Nikos Alexandris:
Updated in sandbox, see: https://trac.osgeo.org/grass/changeset/69365 Is it though correct?
Vaclav Petras:
I was not looking at the logic but if (hue == -1.0) { is not safe. Although it will probably work most of the time if you set it to the exactly same value (`hue = -1;`), but it is not portable because it is exact comparison for floats/doubles. Generally, you need to do something like if (fabs(hue - (-1.0)) < GRASS_EPSILON) { However, here it might be more appropriate to signal the good or bad state of hue by some other variable: hue_good = FALSE; /* setting hue here together with hue_good = TRUE */ if (!hue_good) { /* do the null set */
Thanks Vaclav. Will try that out -- a quick run tells me that I didn't understand yet how to do it. Will get to it.
Also, it seems that the code is not consistent with FCELL versus DCELL. I can see DCELL *rowbuffer and Rast_is_d_null_value but also (FCELL)saturation as well as floats and doubles. Since the input claims to be always DCELL, use DCELL/double everywhere.
See my other post, if you have some time. Ideally, it would be smart to have the input be autodetected, be it CELL or DCELL, ranging in [0, 2^bitness-of-input-data) while the output will be always FCELL (hue in [0.0,360.0] and saturation/lightness in [0.0,1.0].
Related to that, although I'm never sure about where is its appropriate to use DCELL and where double (DCELL currently gets evaluated to double: typedef double DCELL;). I would probably use DCELL everywhere in this function; just to make it clear that we deal purely with cell values and there is no need to switch to anything else.
But using DCELL everywhere is a bit of waisting, if I understand correctly. I'd like to keep things clean, even if it appears to be unnecessary to do so on first sight. Nikos _______________________________________________ grass-dev mailing list grass-dev@lists.osgeo.org http://lists.osgeo.org/mailman/listinfo/grass-dev