Editor ColorPicker color wheel preview incorrect in HSV mode

lostminds commented 1 year ago

Godot version

4.0.beta4

System information

macOS 12.6.1

Issue description

It seems the color picker panel color wheel previews are rendering incorrectly if you modify the color in HSV mode. Possibly it's interpreting the values as something other than HSV? It works correctly as long as you use the RGB sliders, but if you switch to HSV mode the selected color is still correct, but the preview color wheel and gradient thing are incorrect. Possibly color wheel and gradient above is rendering the values as HSL (where lightness L=1 is white) instead of the intended HSV (where V=1 is a bright color). For difference see https://en.wikipedia.org/wiki/HSL_and_HSV

See the attached screen capture.

https://user-images.githubusercontent.com/17763524/200117422-aec4de9d-7187-4124-880d-869e4cdc5632.mov

Steps to reproduce

Click a color parameter in the editor (for example a material Albedo color) to bring up the color picker
Switch to HSV sliders by clicking the HSV tab
Change the HSV sliders and observe the color wheel/gradient above to see the indicated location doesn't match the selected color

Minimal reproduction project

This should be a general editor issue, rather than project specific

lostminds commented 1 year ago

Still the same in 4.0b13, the issue more specifically seems to be the "OKHSL" circle picker (that was the default, I only recently found there were others that work fine) that is should represent the points in the circle and bar as HSL values somehow confuses the input HSV bar values as if they were HSL instead of first converting them from HSV if they're changed.

Screenshot 2023-01-18 at 21 52 08

hsandt commented 1 year ago

It also surprised me, then I found these older issue https://github.com/godotengine/godot/issues/62120 and its PR https://github.com/godotengine/godot/pull/67534 which show that the 3rd slider was L (lightness) at some point, which would explain the pure white at 255.

Either a. the wanted decision is to edit L and display it as V is a bug (a regression since it was L for a moment) or b. the wanted design is to edit V and the fact that the 3rd slider changes lightness is a bug (this would mean design changed since the time we were displaying L)

In case a. though, I'd expect the tab to change its name from HSV to HSL. We could also just have both tabs HSV and HSL always visible.

lostminds commented 8 months ago

Fixed when testing in 4.2.beta1

Norrox commented 2 months ago

this is still a thing in master - fresh compile 7529c0bec597d70bc61975a82063bb5112ac8879

EDIT: It's the OKHSL Circle that is broken, the other ones are OK

lostminds commented 2 months ago

Seeing the same issue, so re-opening. Might be that I was mistaken when not seeing the issue in 4.2b1 not checking the OKHSL mode properly.

Still seems to be the issue that in the OKHSL mode color circle it's using the HSV V value as the L value for the HSL color, resulting in full white L=1 when V=1, which is not correct.

lostminds commented 2 weeks ago

Still present in 4.3beta2. And more specifically it looks like the issue is that when you set the V value (for example using the slider) in the OKHSL mode of the picker this is interpreted as a HSL L value, which results in incorrect color preview. However, if you just switch to the OKHSL mode and use the color areas to change the color the values are correctly interpreted.

https://github.com/godotengine/godot/assets/17763524/05e630d7-8619-483c-8fbb-ba797dd5ed71

godotengine / godot