cessen
commented
1 year ago Just dropping a note that we also ran into this recently at Blender when trying to fix a tone mapping issue, and this cut off a possible avenue of fixing that issue.
I agree with @bram0101's proposal here. Although I can understand if there are backwards compatibility concerns.
The current RGB_TO_HSV and HSV_TO_RGB fixed function implementations have some modifications to them, which changes the way they behave. According to the comments next to the implementation, this is to allow it to support extended ranges. I agree with the intent, but I disagree with the implementation.
I wanted to create a simple saturation mapping in my custom view transform. The idea was to convert it into HSV, then apply a curve on the saturation to bring everything in the range of 0.0 to 1.0, and lastly convert it back into RGB. Unfortunately, due to the modifications in the RGB_TO_HSV and HSV_TO_RGB fixed functions, this won't produce correct results. The saturation is already remapped into a range between 0.0 and 2.0, instead of 0.0 and infinity. Next to that, when the saturation is above 1.0, then the V (of HSV) is changed based on the saturation. So, when I then change the saturation, the resulting RGB values will be wrong, because the V wasn't changed as well. A big reason to convert it into HSV, is so that I can change just one value. I shouldn't have to change another value as well.
To give a concrete example. If I have the RGB values (-0.5, 0.0, 1.0) and I convert it into HSV, set the saturation to 1.0, and convert it back to RGB, then the result will be (0.0, 0.166, 0.5). Every other implementation will give (0.0, 0.333, 1.0), which I what I would expect of it as well. Another example is if I have the RGB values (0.0, 0.5, 1.0) and I convert it into HSV, increase the saturation by 1.5 times, and convert it back to RGB, then the result will be (-1.0, 0.5, 2.0). I would've expected (-0.5, 0.25, 1.0). What this means, is that for saturation ranges between 0.0 and 1.0, the behaviour of the algorithm is like normal, but when the saturation exceeds that range, the behaviour changes into something that does not represent HSV anymore.
In my opinion, the fixed functions should be building blocks for people to create their own transforms with. Therefore, these building blocks shouldn't have anything special to them. The RGB_TO_HSV and HSV_TO_RGB fixed functions have special logic added, that causes it to behave unlike traditional HSV, and in my opinion that shouldn't be the case.
My proposal is to use the standard algorithm to convert to and from HSV. But, for the RGB_TO_HSV conversion there is a small change. If all three of the RGB values are negative, then they are negated to make them positive and after the HSV values have been calculated, the V value is negated to make it negative.
In the HSV_TO_RGB conversion, the hue and saturation are used to generate RGB values which are then multiplied by V. This is a viable and correct implementation of the HSV to RGB conversion, which also supports negative values for V.
With these algorithms, extended ranges are still supported and it behaves like any other HSV conversion algorithm. Any RGB value can still be converted into HSV and back into RGB, successfully.