Skip to content

i.his.rgb: reimplement conversion code #5730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jayneel-shah18
Copy link
Contributor

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 rowbuffer overwrite style used in main.c

File Changed

  • his2rgb.c — fixed all cases via modular hue2rgb() 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 #5659

Please review and suggest further improvements if necessary.

@github-actions github-actions bot added C Related code is in C module imagery labels May 22, 2025
@petrasovaa
Copy link
Contributor

I realized that there are also tools r.his and d.his, could you check how different is that implementation?

@jayneel-shah18
Copy link
Contributor Author

Thank you for the suggestion. I reviewed the implementations of r.his and d.his.

  • r.his performs RGB to HIS conversion and shares the same theoretical model that I have used in this patch. Its hue interpolation logic is similar to the hue2rgb function in this patch for i.his.rgb.

  • d.his is a display module designed for interactive screen rendering. Unlike i.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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C imagery module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] i.his.rgb: incorrect RGB values for grayscale and primary hue inputs
2 participants