ColorPicker: Add an intensity slider in raw mode for HDR#99366
Conversation
fire
left a comment
There was a problem hiding this comment.
I support colour intensity (hdr), but we want to consult the rendering team and reduz since they vetoed the change during the colour picker rework.
2971157 to
e3ebbdd
Compare
|
What change was veto'd? I see no harm in an intensity slider. I would see harm in adding an intensity field in Color, but that's not what this PR does |
|
The way it's exposed in this PR seems less risky than what godotengine/godot-proposals#1031 originally envisioned, since it's only available in RAW mode and does not impact the RGB/HSV modes in any way. The downside is that you can't use HSV editing while having an HDR intensity slider on the same page, but you can switch to RAW mode after editing your HSL color and use the intensity slider there. I also agree with keeping the hexadecimal field visible whenever the color isn't overbright too. |
Calinou
left a comment
There was a problem hiding this comment.
Tested locally, I noticed some issues:
- Switching to RAW mode increases the dialog's height, which can cause the bottom of the dialog to be unreachable if it spawns at the bottom of a window:
colorpicker_at_bottom.mp4
- The icon when the color is overbright doesn't display correctly in the running project:
In contrast, it displays correctly in the editor:
- Switching to the OKHSL Circle picker will result in broken rendering if you were in the RAW mode and intensity is over 1 (this isn't a new issue IIRC, but it's easier to achieve now):
I think it's acceptable, clicking "Swatches" or "Recent colors" increases the dialog's height as well.
I have fixed it in the running project. However, in editor the 2d screen it still can't display correctly, for which I don't have a solution at the moment.
It seems due to the incorrect value of HSV. #99461 should fix it. |
|
I've tested the latest revision of this PR and noticed the color picker can have its hex/code toggle button permanently locked if you raise the intensity above 1 then lower it below 1. Closing and reopening the ColorPicker doesn't fix the issue, you need to reload the saved scene:
colorpicker_hex_lockout.mp4The
|
95f5ce6 to
f5507ae
Compare
|
The button to switch between hexadecimal and code works now, but the button becomes invisible once you switch to code (it's still clickable):
|
It's as expected, because the "Script" icon is only available in the editor. To make it available at runtime, we may add a theme property for it and move the "Script" icon to the default theme. I don't know if it's worth. Originally, this button is only enabled in the editor. |
f5507ae to
52ef932
Compare
897d632 to
07a7bb3
Compare
|
I have added theme property for the "script" icon and fix the issue. |
07a7bb3 to
633d86a
Compare
|
I'm still interested in this. It just helps me a lot to set overbright colors. |
|
I still think this is salvageable. |
|
I accidentally closed it, and the reopen button is disabled 😅 |
This PR adopts a relatively simple approach, without storing intensity or modifying the Color class. Intensity is inferred from raw RGB, and will be 0 and not be retained if RGB does not exceed 1.
Add a intensity slider and reduce the max value of RGB to 1 in ColorPicker raw mode.
Keep the hex LineEdit always visiable and switch it to code if RGB exceeds the valid range.
Resolve: godotengine/godot-proposals#1031