i.rgb.his, i.his.rgb: propagate NULLs

[hamish]

Hi,

NULL cells in i.rgb.his R,G,B input appear as non-NULL cells (0) in H,I,S output maps. Same happens in the reverse i.his.rgb process.

Hamish

[cmbarton]

This remains a serious hidden bug. At the very least, it should be documented in the manuals for these routines. Especially for remote sensing imagery, 0 is a meaningful value. To mix 0's with nulls is to corrupt the information in the image. Because of the potential for data corruption and because it is an undocumented behavior that differers from most (all?) other raster modules, I'm also upping the priority to major.

Note that because neither i.rgb.his or i.his.rgb deal with nulls correctly, they cannot be used with a MASK.

Is this difficult to fix?

Michael

[glynn]

Replying to cmbarton:

Is this difficult to fix?

Not really.

In 7.0, nulls resulted in garbage. 7.0 doesn't have an equivalent of G_get_map_row(), but rgb2his/his2rgb don't explicitly check for nulls, so they will be treated as the literal value -2147483648. This should be fixed by r52207.

In 6.x, change G_get_map_row() to G_get_c_raster_row() in main(), then manually back-port r52207.

Also, we might want to do something about the hard-coded 0..255 range (r.composite uses the colour table to convert cell values to intensities), but that's a separate issue.

[cmbarton]

Thanks much. I agree about the 0-255 range. Can it simply be a setting, like in i.pca, with a default of 0,255 but changeable to a different min,max?

Michael

[hamish]

backported to devbr6 in r52208 for testing.

thanks, Hamish

[cmbarton]

I'll recompile and test in GRASS 7. Do I need to test separately in GRASS 6?

Michael

[hamish]

Do I need to test separately in GRASS 6?

the more testing the better :). Once it is confirmed working in devbr6 with real data we can backport to 6.4.

thanks, Hamish

[cmbarton]

By devbr6, you mean 6.5?

Michael

[hamish]

By devbr6, you mean 6.5?

yes.

[cmbarton]

This works in GRASS 7 at least. Still compiling devbr6.

Michael

[glynn]

Replying to cmbarton:

I agree about the 0-255 range. Can it simply be a setting, like in i.pca, with a default of 0,255 but changeable to a different min,max?

It could be, but that's limited to a linear scale. Using the colour table is more flexible, although that limits you to 8-bit resolution.

Probably the easiest solution is to just add a "divisor" option, which would default to 255 (or maybe to 1.0 for floating-point maps), and maybe a flag to use the colour table. That would allow for more than 8-bit resolution, and for floating-point maps using 0.0..1.0. If you want anything more complex (e.g. "gamma" law), you have to pre/post-process the maps with r.mapcalc.

[mlennert]

This was never backported to the 64 release branch. It probably should, or ?

[hamish]

Replying to mlennert:

This was never backported to the 64 release branch. It probably should, or ?

I have just begun backports for the 6.4.4 round, currently I'm going through module by module seeing what should be backported and what is still a work in progress or experiment. I'm starting with simple stuff in scripts/ & haven't got to i.rgb.his yet. (kdiff3 seems to be useful here, I've had trouble with regex filtering of false positives using meld et al. tips on the wiki)

regards, Hamish

[cmbarton]

For pan sharpening at least, it would be helpful if these modules could deal with floating point values instead of just integers.

Michael

[mlennert]

Replying to cmbarton:

For pan sharpening at least, it would be helpful if these modules could deal with floating point values instead of just integers.

I think this is a different issue and shouldn't be part of this ticket. It was already mentioned in #774.

[neteler]

Hamish, do you plan to backport to 6.4.svn?

[wenzeslaus]

So only backport is missing? Or can we even close the ticket without the backport? (since the change is in 7.0.0 (the latest release) as fas as I understand from the comments) There is something about the backport in comment:2 but it is really that easy and who will test it?

[mlennert]

Actually the change was backported by Hamish in r57791 two years ago, so I'm closing this ticket.