ColorPicker: Add an intensity slider in raw mode for HDR

[beicause]

• Close Add an HDR Color toggle and a separated slider for power godot-proposals#1031

Close #99366

This PR adopts a relatively simple approach, without storing extra intensity field in Color class.

Adds an intensity slider and limit the max value of RGB to 1 in ColorPicker raw mode. Intensity is inferred from raw RGB and may change after switch color mode. The final color is calculated as color * pow(2, intensity).

Also keep the hex LineEdit always visiable and switch it to code if RGB exceeds the valid range.

Edit:

This depends on #102240 to allow greater values.

[allenwp]

From looking at this, it appears that the intensity slider is the exposure stops (log2/pow2). So an intensity of 0 is +0 stops, which uses the RGB values as-is.

If my understanding is correct, I think this is a little confusing to users who are not familiar with stops. Even though I'm used to working in stops, I was still a bit confused and assumed the intensity slider was simply a multiplier on the RGB values -- This meant that I expected an intensity of 1.0 to result in no modification of the RGB values. (But this is not the case because this is actually +1 stops, not a 1.0 intensity multiplier).

I think having the + or - indicator on the value would help distinguish that this is stops and not an intensity multiplier. Also, the label should read "stops" or something like that, ideally. Here's an example of how photoshop shows this sort of slider:

[beicause]

pow 2 is better than multiplier as it's easier to control the appearence of glow. Unity's intensity slider also uses pow 2. If we want to allow negative intensity, more works need to be done.

[allenwp]

pow 2 is better than multiplier as it's easier to control the appearence of glow. Unity's intensity slider also uses pow 2. If we want to allow negative intensity, more works need to be done.

My apologies, I did not mean to suggest it should be a multiplier. The functionality should definitely be in stops, as implemented in the PR. I only brought up intensity multiplier because the current GUI of this PR makes me think this is how it behaves (even though it doesn't behave that way).

[allenwp]

If we want to allow negative intensity, more works need to be done.

I don't think this is strictly needed, but allowing negative stops is very much a standard convention for this sort of a slider, as shown in the Photoshop example I posted.

[beicause]

Added support for negative intensity.
When the HDR threshold of the glow is below 1.0 , adjust the intensity to negative makes sense.

[beicause]

#102240 adds get_allow_geater for sliders. After that, I think the max value of intensity can be 4 for common use.

[allenwp]

#102240 adds get_allow_geater for sliders. After that, I think the max value of intensity can be 4 for common use.

I was also thinking about whether having the slider max at +6.0 or +4.0. I believe +6.0 is nice because it results in 0.0 being placed in the center of the GUI, which gives a pleasant appearance that reinforces that a value of 0.0 is "neutral" (as in, makes no difference to the colour value). But I agree that in practice, the user shouldn't need to go higher than +4.0...

[allenwp]

Added support for negative intensity. When the HDR threshold of the glow is below 1.0 , adjust the intensity to negative makes sense.

This looks great to me, thank you!

Is it possible to use the prifix property on the spinbox to add a + symbol when intensity > 0? Or maybe there is another way to achieve this sort of number formatting?

[allenwp]

My review of the code was not very thorough, but the user experience of this PR looks great to me, now that the intensity slider extends to negative stops and the +/- signs are shown on the intensity value.

Thanks!

[beicause]

I also colorize the sliders in the RAW mode, like RGB mode. At this point, I think the RAW mode and the RGB mode are similar, maybe we could remove the RAW mode and move the Intensity slider to the RGB mode?

[allenwp]

maybe we could remove the RAW mode and move the Intensity slider to the RGB mode?

The power-user part of me likes this, but this would require giving users the option to input colour values in either 255 integer scale or 1.0 float scale in the RGB mode of the colour picker. This might make the RGB mode too cluttered and too much of a visual overload for users who are entirely new to game development and colour picking.

For the reasons of making it accessible to new users, maybe the current approach is fine...

That said, I do have some related thoughts...

My complaint is that RAW mode picks colours in gamma encoding rather than linear, but that's more of an issue with how the Color class stores data and there's no way that should be changed... But, maybe there should be an HDR mode that allows colour picking in linear encoding that handles encoding conversions for the user. This is actually the way that Photoshop behaves:

8-bit traditional sRGB colour picker:

32-bit HDR sRGB colour picker:

Notice how the HDR colour picker is entirely in linear encoding, which is different than the 8-bit traditional sRGB picker. In this way, maybe the intensity slider should exist in an HDR mode that allows selecting colours in linear encoding rather than being added to a RAW mode (like this PR's current state), since gamma encoded colours don't make sense when working in HDR.

(To be honest, I actually wanted this sort of behaviour before even looking at Photoshop -- I was pleasantly surprised that Photoshop behaved this way, and I think there is value in matching the behaviour of art tools like this...)

So after writing all of this it makes me wonder: maybe the RAW mode should be removed and a new (linear) HDR mode should be added with an intensity slider and automatic linear-to-gamma conversions. The intensity slider would not be added to RGB mode, but an option to input colour values in 1.0 float scale would be added to the RGB mode in a way that wouldn't overload new users with complexity...

(Please let me know if any of what I wrote is at all confusing and I can whip up a couple of mockups to describe what I'm suggesting be considered.)