i.his.rgb: reimplement conversion code
This pull request fixes incorrect RGB outputs in i.his.rgb GRASS module for certain combinations of hue, intensity, and saturation values, particularly:
- When saturation = 0 (expected grayscale output)
- When intensity = 0 (expected black output)
- When intensity = 255 and saturation = 255 (expected pure hue)
The prior implementation failed to correctly compute RGB for these edge cases due to formula limitations, incorrect assumptions, and redundancy in the his2rgb.c logic.
This patch:
- Correctly computes pure grayscale and black cases
- Fixes hue interpolation logic with proper clamping and rounding
- Replaces the core conversion logic with a mathematically accurate implementation
- Preserves the
rowbufferoverwrite style used inmain.c
File Changed
his2rgb.c— fixed all cases via modularhue2rgb()logic and improved edge-case handling.
Validation Table (Old vs. Reference RGB Values)
| IDX | H | I | S | OLD_RGB | REF_RGB | Match | Justification |
|---|---|---|---|---|---|---|---|
| 0 | 0 | 0 | 0 | [0, 0, 0] | [0, 0, 0] | Yes | S=0: grayscale expected |
| 1 | 0 | 0 | 128 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 2 | 0 | 0 | 255 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 3 | 0 | 128 | 0 | [0, 0, 0] | [128,128,128] | No | S=0: grayscale expected |
| 4 | 0 | 128 | 128 | [192, 64, 64] | [192, 64, 64] | Yes | Match |
| 5 | 0 | 128 | 255 | [255, 1, 1] | [255, 1, 1] | Yes | Match |
| 6 | 0 | 255 | 0 | [0, 0, 0] | [255,255,255] | No | S=0: grayscale expected |
| 7 | 0 | 255 | 128 | [255,255,255] | [255,255,255] | Yes | Match |
| 8 | 0 | 255 | 255 | [255,255,255] | [255, 0, 0] | No | Expected pure hue |
| 9 | 60 | 0 | 0 | [0, 0, 0] | [0, 0, 0] | Yes | S=0: grayscale expected |
| 10 | 60 | 0 | 128 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 11 | 60 | 0 | 255 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 12 | 60 | 128 | 0 | [0, 0, 0] | [128,128,128] | No | S=0: grayscale expected |
| 13 | 60 | 128 | 128 | [139,192, 64] | [139,192, 64] | Yes | Match |
| 14 | 60 | 128 | 255 | [150,255, 1] | [150,255, 1] | Yes | Match |
| 15 | 60 | 255 | 0 | [0, 0, 0] | [255,255,255] | No | S=0: grayscale expected |
| 16 | 60 | 255 | 128 | [255,255,255] | [255,255,255] | Yes | Match |
| 17 | 60 | 255 | 255 | [255,255,255] | [0,255,0] | No | Expected pure hue |
| 18 | 120 | 0 | 0 | [0, 0, 0] | [0, 0, 0] | Yes | S=0: grayscale expected |
| 19 | 120 | 0 | 128 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 20 | 120 | 0 | 255 | [0, 0, 0] | [0, 0, 0] | Yes | I=0: black expected |
| 21 | 120 | 128 | 0 | [0, 0, 0] | [128,128,128] | No | S=0: grayscale expected |
| 22 | 120 | 128 | 128 | [64,192,169] | [64,192,169] | Yes | Match |
| 23 | 120 | 128 | 255 | [1,255,210] | [1,255,210] | Yes | Match |
| 24 | 120 | 255 | 0 | [0, 0, 0] | [255,255,255] | No | S=0: grayscale expected |
| 25 | 120 | 255 | 128 | [255,255,255] | [255,255,255] | Yes | Match |
| 26 | 120 | 255 | 255 | [255,255,255] | [0,255,0] | No | Expected pure hue |
| 27 | 45 | 64 | 192 | [107,112, 16] | [107,112, 16] | Yes | Match |
| 28 | 90 | 128 | 64 | [96,160,104] | [96,160,104] | Yes | Match |
| 29 | 135 | 192 | 255 | [129,233,255] | [129,233,255] | Yes | Match |
| 30 | 180 | 128 | 128 | [94,64,192] | [94,64,192] | Yes | Match |
| 31 | 210 | 64 | 255 | [120,0,128] | [120,0,128] | Yes | Match |
| 32 | 240 | 192 | 64 | [208,176,187] | [208,176,187] | Yes | Match |
| 33 | 270 | 128 | 192 | [224,100, 32] | [128,32,224] | No | Hue interpolation mismatch |
| 34 | 300 | 255 | 128 | [255,255,255] | [255,255,255] | Yes | Match |
| 35 | 330 | 192 | 255 | [159,255,129] | [255,129,192] | No | Hue interpolation mismatch |
| 36 | 30 | 192 | 192 | [239,212,145] | [239,212,145] | Yes | Match |
Fixes https://github.com/OSGeo/grass/issues/5659
Please review and suggest further improvements if necessary.
I realized that there are also tools r.his and d.his, could you check how different is that implementation?
Thank you for the suggestion. I reviewed the implementations of r.his and d.his.
-
r.hisperforms RGB to HIS conversion and shares the same theoretical model that I have used in this patch. Its hue interpolation logic is similar to thehue2rgbfunction in this patch fori.his.rgb. -
d.hisis a display module designed for interactive screen rendering. Unlikei.his.rgb, it operates at the graphics level and does not use raster CELL buffers.
This patch complements r.his by implementing the inverse HIS to RGB transformation, using the same color space model and consistent logic.