ocornut/imgui

Fork 0

mirror of https://github.com/ocornut/imgui.git synced 2026-11-14 19:04:44 +00:00

ColorPicker / ColorEdit Widget Wrong Conversion from HSV to RGB #2722

New issue

Closed

opened 2019-08-11 10:24:00 +00:00 by MegaBor · 8 comments

MegaBor commented

2019-08-11 10:24:00 +00:00

(Migrated from github.com)

Version/Branch of Dear ImGui:

Version: 1.72b
Branch: master

My Issue:

Hi, I think color picker doesn't work well when doing conversion from HSV to RGB.
When I set S to 0, it jumps to red. In Photoshop color picker doesn't do that.

**Version/Branch of Dear ImGui:** Version: 1.72b Branch: master **My Issue:** Hi, I think color picker doesn't work well when doing conversion from HSV to RGB. When I set S to 0, it jumps to red. In Photoshop color picker doesn't do that. ![ezgif com-video-to-gif](https://user-images.githubusercontent.com/52722806/62832685-cc17a880-bc32-11e9-81cb-a7ff38e4125a.gif)

👍 2 👎 2 😕 2 👀 2 ❤️ 3 🎉 2 😆 2 🚀 2

MegaBor commented

2019-08-11 12:43:39 +00:00

(Migrated from github.com)

Is there a way to force the color picker to not use HSV format? So I don't have to bother regarding this issue. I saw ImGuiColorEditFlags_DisplayRGB flag, but although I'm using it, it still displays HSV when I pick a color. How about also displaying ImGuiColorEditFlags_DisplayRGB and ImGuiColorEditFlags_DisplayHex? The assertion doesn't allow that.

Is there a way to force the color picker to not use HSV format? So I don't have to bother regarding this issue. I saw `ImGuiColorEditFlags_DisplayRGB` flag, but although I'm using it, it still displays HSV when I pick a color. How about also displaying `ImGuiColorEditFlags_DisplayRGB` and `ImGuiColorEditFlags_DisplayHex`? The assertion doesn't allow that.

❤️ 1

IAmEki commented

2019-08-13 16:36:48 +00:00

(Migrated from github.com)

Have you tried ImGuiColorEditFlags_InputRGB?

Have you tried `ImGuiColorEditFlags_InputRGB`?

👍 1 ❤️ 1 🎉 1 🚀 1

MegaBor commented

2019-08-13 20:55:56 +00:00

(Migrated from github.com)

@IAmEki, Thanks! I see with the new changes it does display RGB with this flag, but the problem where the color transition jumps to red still there. I think the color values needs to be clamped.

👍 1

ocornut commented

2019-08-14 15:45:58 +00:00

(Migrated from github.com)

See #2383 (and #2384) to understand the issue.
If your code is storing RGB i don't think this is fully solvable.

You can store HSV values and use the InputHSV flag to avoid those singularities.
We could potentially make an open color picker instance store HSV intermediate values internally to work-around this... for the lifetime of the picker you would be protected. However this wouldn't work with HSV sliders in any regular in-line ColorEdit widget, so that would create a discrepancy between the two usages (in-line vs picker popup).

I don't know if there is a better solution. @haldean would know perhaps ?

See #2383 (and #2384) to understand the issue. If your code is storing RGB i don't think this is fully solvable. - You can store HSV values and use the `InputHSV` flag to avoid those singularities. - We could potentially make an open color picker instance store HSV intermediate values internally to work-around this... for the lifetime of the picker you would be protected. However this wouldn't work with HSV sliders in any regular in-line ColorEdit widget, so that would create a discrepancy between the two usages (in-line vs picker popup). I don't know if there is a better solution. @haldean would know perhaps ?

👍 2 ❤️ 1 🎉 1 🚀 1

haldean commented

2019-08-14 16:19:22 +00:00

(Migrated from github.com)

I agree 100% with what you said Omar; as long as the application is holding all of the color state, the widget doesn't have enough information to keep the hue slider in the same place when S/V go to 0. To fix, you'd have to pass HSV values (using InputHSV) or the library would need to store a "last hue" somewhere.

I agree 100% with what you said Omar; as long as the application is holding all of the color state, the widget doesn't have enough information to keep the hue slider in the same place when S/V go to 0. To fix, you'd have to pass HSV values (using `InputHSV`) or the library would need to store a "last hue" somewhere.

👍 1 👀 1 ❤️ 1 🎉 1 🚀 1

ocornut commented

2019-08-14 17:08:03 +00:00

(Migrated from github.com)

Hmmm, we could probably store a "last hue" associated to the last active widget ID, and have ColorEdit4/ColorPicker4 restore that hue on edit.. That would maybe attenuate the issue enough?

👍 2 ❤️ 1

MegaBor commented

2019-08-14 18:45:19 +00:00

(Migrated from github.com)

Thank you @ocornut. Looking into this issue more in depth, I believe storing last hue could potentially solve this issue. However, I'm pretty new to using ImGui, so I'm not confident enough with the sources providing a pull request to this, but I'll try to see if something comes up in mind.

👍 1 🎉 1

ocornut commented

2019-09-20 13:34:18 +00:00

(Migrated from github.com)

Merged a fix for this, thank you @rokups !

The fixes stores last Hue for a given RGB set.
Note that the picker has another singularity of the same kind, when setting V=0 breaks Saturation. A fix for that other issue could be nice sometimes.

Merged a fix for this, thank you @rokups ! The fixes stores last Hue for a given RGB set. Note that the picker has another singularity of the same kind, when setting V=0 breaks Saturation. A fix for that other issue could be nice sometimes.