Information is lost on a round trip through ColorPicker4 for applications that store colors as HSV

[haldean]

Version/Branch of Dear ImGui:

Version: 1.68
Branch: I believe all, but definitely master and docking

Back-end/Renderer/Compiler/OS

Back-ends: all
Compiler: all
Operating System: all

My Issue/Question:

Applications that store colors as HSV values have a hard time using ColorPicker and ColorEdit correctly, because they have to take the round trip of HSV -> RGB -> HSV. The map from HSV to RGB has two singularities: when S is 0 (grayscale values, where hue doesn't matter) and when V is 0 (black, hue and saturation both don't matter).

Applications that store color values in HSV would like to maintain the H and S values when V goes to zero, or the H value when S goes to zero, even though the RGB color is the same. Otherwise, you get odd behavior where fading the saturation from a positive value down to zero and then back up will change the hue of the color you were editing.

From looking at the code of ColorPicker4, it looks like internally the first thing that the picker does is convert from RGB to HSV, and then it uses those HSV values almost everywhere when drawing the UI. I believe this issue could be fixed by adding a ColorPicker flag that specifies that the input float[3] of color is already in HSV format. This is because the problem is not really with ColorPicker, but with the need for using ColorConvertHSVtoRGB and ColorConvertRGBtoHSV, which are inherently lossy. If ColorPicker took and returned HSV values, then it would have the necessary information to maintain the state of hue and saturation when saturation or value went to zero.

Unfortunately ImGuiColorEditFlags_HSV is already taken, but maybe the flag could be named ImGuiColorEditFlags_HSV_IO, to mark that input and output color data are in HSV, or something similar? I'm happy to provide a PR to add a flag like this, but I wanted to make sure this sounded like a good approach before I started coding. Either way, I'm happy to do the work to get native HSV I/O support into ColorPicker; if you're willing to accept a patch, let me know what you'd like to see!

Standalone, minimal, complete and verifiable example: (see #2261)

A dumb example to show how a round-trip through ColorConvertHSVtoRGB and ColorConvertRGBtoHSV can muck up your colors:

    static ImVec4 hsv(0.5f, 1.0f, 1.0f, 1.0f);
    ImVec4 rgb(0, 0, 0, 1);

    ImGui::Begin("Colors");
    ImGui::DragFloat3("color-hsv", &hsv.x, 0.01f, 0.0f, 1.0f);

    ImGui::ColorConvertHSVtoRGB(hsv.x, hsv.y, hsv.z, rgb.x, rgb.y, rgb.z);
    ImGui::DragFloat3("color-rgb", &rgb.x, 0.01f, 0.0f, 1.0f);

    static bool showEdit = false;
    ImGui::Checkbox("show ColorEdit3", &showEdit);
    if (showEdit) {
        ImGui::ColorEdit3("color", &rgb.x);
        ImGui::ColorConvertRGBtoHSV(rgb.x, rgb.y, rgb.z, hsv.x, hsv.y, hsv.z);
    }
    ImGui::End();

Screenshots/Video

The hue starts at 0.5, but is reset to zero when the saturation is dropped to zero, because ColorConvertRGBtoHSV turns (1, 1, 1) into (0, 0, 1), instead of the (0.5, 0, 1) we would like to have. We wouldn't need to call the ColorConvert* functions if ColorEdit3 could take HSV values natively.

[ocornut]

Hello, Thanks for the detailed post. I agree with your suggestion and would very much appreciate a PR for it. ( I suggest the flag could be called _InputHSV (corresponding _InputRGB may be added to make things cleared, as well as __InputDefault). I also suggest __InputsMask (which is private) be renamed to __ShowMask. And we could later consider to rename _RGB/ _HSV/ _HEX to add the “Show” word in then (with redirecting enums for backward compatibility) but that could be done separately. ) Thanks a lot!

[haldean]

Great! I'll start in on this and get you something in the next couple days. Thanks Omar!

[ocornut]

Closing this as the PR itself is enough for tracking progress on this, Thanks again!