Opened 11 years ago

Closed 5 years ago

#778 closed defect (fixed)

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

Reported by: hamish Owned by: grass-dev@…
Priority: major Milestone: 7.0.0
Component: Raster Version: svn-trunk
Keywords: i.rgb.his, i.his.rgb Cc:
CPU: Unspecified Platform: All

Description

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

Change History (17)

comment:1 Changed 8 years ago by cmbarton

Priority: minormajor

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

comment:2 in reply to:  1 Changed 8 years ago by 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.

comment:3 Changed 8 years ago by 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

comment:4 Changed 8 years ago by hamish

backported to devbr6 in r52208 for testing.

thanks, Hamish

comment:5 Changed 8 years ago by cmbarton

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

Michael

comment:6 Changed 8 years ago by 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

comment:7 Changed 8 years ago by cmbarton

By devbr6, you mean 6.5?

Michael

comment:8 Changed 8 years ago by hamish

By devbr6, you mean 6.5?

yes.

comment:9 Changed 8 years ago by cmbarton

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

Michael

comment:10 in reply to:  3 Changed 8 years ago by 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.

comment:11 Changed 7 years ago by mlennert

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

comment:12 in reply to:  11 Changed 7 years ago by 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

comment:13 Changed 7 years ago by cmbarton

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

Michael

comment:14 in reply to:  13 Changed 7 years ago by 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.

comment:15 Changed 6 years ago by neteler

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

comment:16 Changed 5 years ago by 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?

comment:17 Changed 5 years ago by mlennert

Resolution: fixed
Status: newclosed

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

Note: See TracTickets for help on using tickets.