Fix `ColorPicker`'s RAW mode colors and values

[WhalesState]

• Fixes: https://github.com/godotengine/godot/issues/62894
• Fixes: https://github.com/godotengine/godot/issues/88073

Changes:

1. Makes RAW sliders colored same as RGB mode.
2. Fix RAW mode maximum values, from 100 to 1.0.
3. Allow RGB and RAW modes to have greater values for over bright.
4. Changes RAW step value to 1.0 / 255.0, to match the RGB mode values when changed.
5. Fix pressing the sample left side changes the color to black when old color is not displayed.
6. Remove an empty VBoxContainer called vbl.
7. Remove an extra VBoxContainer used as child of real_vbox, and adding it's children to the real_vbox directly.
8. Prevent OKHSL mode from changing the picker shape. The color mode is for sliders only, no need to override the shape.
9. Fix over bright indicator position.
10. Fix conversion between RAW and RGB modes when color is over bright.

[WhalesState]

Please add the bug label too, since it fixes three annoying issues.

[AThousandShips]

Please open a bug report first :)

Remember you need to add "fixes" before each issue number

[WhalesState]

Remember you need to add "fixes" before each issue number

Thanks ^^

[AThousandShips]

See also:

• https://github.com/godotengine/godot/pull/62913

[WhalesState]

See also:

• fix switching RAW mode to RGB mode issue #62913

We share the same fix from float slider_max[4] = { 100, 100, 100, 1 }; to float slider_max[4] = { 1, 1, 1, 1 };

[Cammymoop]

Being able to set values greater than 1.0 in raw mode is a feature, not a bug. #62894 is about not restoring the maximum value of the slider when going back to RGB mode, not changing the max value in raw mode.

[WhalesState]

Being able to set values greater than 1.0 in raw mode is a feature, not a bug. #62894 is about not restoring the maximum value of the slider when going back to RGB mode, not changing the max value in raw mode.

we can still allow this feature by making the SpinBox able to set values higher than 1.0 by enabling allow_greater, but I still don't understand the use of it, since they will not be visualized correctly, is it for glow/emission strength to make some colors more brighter that others? maybe because the glow the strength is a global variable for each environment.

however, allowing both RGB and RAW mode to set greater values will fix this issue and keep the feature.

[Cammymoop]

Yes, it is for colors that are overbright in cases such as emissive colors with glow effects, or using modulate to brighten something.

I wouldn't suggest using allow_greater on the RAW mode sliders and limiting them to 1, because afaik the main reason raw mode is used at all is for overbright colors. RGB mode uses code specifically (see get_slider_max *edit) to set different slider max values in the case where you switched from raw and still want to edit the overbright color I guess but I'm not sure that that part is usefull, and you could just as easily switch to HSV instead (which would be more helpful to edit an overbright color) now that raw is a separate tab from RGB, but HSV does clamp the overbright color so that's pointless currently.

A proper way to deal with overbright colors on all color modes (probably a separate intensity slider) is the only elegant fix I think, but I think making RGB mode use allow_greater or just making it clamp is the way to go for this bug. HSV could use allow_greater theoretically, but I don't think the internal conversion supports that properly, okhsl will definitely not work with unclamped lightness so don't bother with that one.

[WhalesState]

ok, after allowing both RGB and RAW modes to have greater values, we are back again to this issue https://github.com/godotengine/godot/issues/62894, for some reason the slider maximum value changes after converting from RAW to RGB.

!!!

The issue is now clear, maximum slider values are not defined for RGB, I think it do calculates it manually which causes this issue.

here it is.

[MajorMcDoom]

Hi there, I was going to ask, while we're at it, is it difficult to make it possible to accept negative values as well? I know the most common use case is overbright colors, but negative colors are also used in many graphics tricks, and being able to export one would be great. The thing is that each component in Color can, in code, represent any float. But when exported in the inspector, the values are artificially being constrained by the UX of the ColorPicker. I think that's quite a shame, and it would be great if we could get over that hurdle with this PR. But if it's too difficult, I think getting over the limit of 100 is already a pretty big win.

[Calinou]

Hi there, I was going to ask, while we're at it, is it difficult to make it possible to accept negative values as well? I know the most common use case is overbright colors, but negative colors are also used in many graphics tricks, and being able to export one would be great. The thing is that each component in Color can, in code, represent any float. But when exported in the inspector, the values are artificially being constrained by the UX of the ColorPicker. I think that's quite a shame, and it would be great if we could get over that hurdle with this PR. But if it's too difficult, I think getting over the limit of 100 is already a pretty big win.

The issue is that many properties can't handle negative colors by design (or it'll result in unintended effects). We'd need additional property hints to denote that specific properties can't use negative colors, similar to the existing PROPERTY_HINT_COLOR_NO_ALPHA. Once this hint is added, we'd then have to comb through all the existing properties of type Color and add hints as required. It's a lot of work for a feature that won't be needed often, as some nodes like Light3D already have a Negative property.

[KoBeWi]

Still not convinced about slider max value change. While it's possible to input bigger number in the SpinBox, you lose ability to adjust overbright via dragging, and I know some people like doing that. Not to mention that pressing Enter to accept SpinBox value will close the popup.

[WhalesState]

Still not convinced about slider max value change. While it's possible to input bigger number in the SpinBox, you lose ability to adjust overbright via dragging, and I know some people like doing that. Not to mention that pressing Enter to accept SpinBox value will close the popup.

For RAW and RGB modes, We can allow dragging using the SpinBox, they can click and hold the up/down button and move the mouse up or down to change the value, we just need to change the maximum value for the SpinBox only and not the sliders to not lose the colorized sliders for RAW mode, also a maximum value of 10 can be more usable than 100. We can add ColorMode::get_spinbox_max to solve this.

[KoBeWi]

With all these get() methods in ColorMode, I wonder if we shouldn't just introduce a single ColorModeConfig struct, which would hold all these parameters (not in this PR). get_spinbox_max() would solve the problem. I think it should have some default return value, like -1, which would make it match the slider max.

[WhalesState]

Maybe i can use this to return the slider max value by default virtual float get_spinbox_max(int idx) const override { return get_slider_max(idx); }.

[KoBeWi]

Makes sense.

[WhalesState]

Failed because Spinbox and Slider are shared, will have to update them manually for this to work.

[WhalesState]

Added a new class OverbrightSpinbox and used it for RGB and RAW modes.

First i have tried to disconnect the value_changed and connects it again after updating the slider but I was afraid if it will be slow, so i just connected the value_changed of the slider to OverbrightSpinbox::_update which will return early to avoid calling set_value_no_signal twice.

Didn't test it yet since it's still building after the rebase, Just need to know if this approach looks fine.

Also I can reuse ColorMode::can_allow_greater instead of ColorMode::has_overbright_spinbox since both are for overbright.

[WhalesState]

Works as expected.

Only the alpha slider shares it's value with the alpha spinbox, I ended up using the new OverBrightSpinBox for all the other SpinBoxes to avoid deleting/recreating spinboxes.

Screencast from 01-26-2025 02:34:15 AM.webm