Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an HDR Color toggle and a separated slider for power #1031

Open
ghsoares opened this issue Jun 10, 2020 · 12 comments · May be fixed by godotengine/godot#99366
Open

Add an HDR Color toggle and a separated slider for power #1031

ghsoares opened this issue Jun 10, 2020 · 12 comments · May be fixed by godotengine/godot#99366

Comments

@ghsoares
Copy link

Describe the project you are working on:
any project that uses HDR color.

Describe the problem or limitation you are having in your project:
In Godot currently we are able to use HDR colors, but it is not intuitive and you have to make the calculations manually. It would be nicer if we could enable/disable a color as a HDR and use a slider or something to increase/decrease the power separated.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
The feature would be simple as defining a new parameter to the class Color to define the HDR power, using the "raw" color picker parameter renaming to "HDR" and adding a simple HDR power slider to it.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
The class "Color" would have the HDR power parameter, multiplying the RGB channels by the power.
In the color picker we could enable/disable if the current color is HDR and if so, it would appear a slider to change the power like so:
Temp1

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Not with a few lines of script, as it would be needed to create a custom editor plugin, with a custom Color class and the scripts would only accept this custom Color class to use easily the HDR function.

Is there a reason why this should be core and not an add-on in the asset library?:
Because is a main feature for those that creates custom shader effects with HDR that would accept the color in HDR, as many other things.

@Calinou
Copy link
Member

Calinou commented Jun 10, 2020

This is pretty much what the "RAW mode" currently achieves. It just needs to be made a bit clearer. We should also try to improve its overall usability, but I think it's mainly a matter of ensuring slider ranges are small enough to be easily adjustable with a mouse.

After enabling RAW mode, you can increase colors' components past 1.0 by entering a value in the text field (there's no real upper limit).

I'm not sure if we should rename the RAW mode to HDR mode, as you can use the RAW mode to select colors whose components are below 1.0. Maybe we should still do it for discoverability reasons, even if it's technically incorrect.

@ghsoares
Copy link
Author

@Calinou I know that we can use the raw mode, but it changes the main color, what I'm trying to propose is a way to manipulate separately the power of the color so we can increase the brightness without changing the main color. This makes easier to maintenance the color without needing to calculate the values manually for each channel.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 10, 2020

what I'm trying to propose is a way to manipulate separately the power of the color so we can increase the brightness without changing the main color

This sounds like HSV with value beyond 1 (if that makes sense), which reminds me of my old issue: godotengine/godot#31679
If we could combine HSV and RAW, this would be possible probably.

@ghsoares
Copy link
Author

This sounds like HSV with value beyond 1 (if that makes sense), which reminds me of my old issue: godotengine/godot#31679
If we could combine HSV and RAW, this would be possible probably.

This can work, but as someone mentioned in the post you did reference:

RAW makes little sense outside of RGB

Because HSV is a color mode as RGB, and the brightness is a color channel in this mode, the only change is that this mode fallsback to RGB mode as output to Graphics Card. So increasing the brightness value beyond 100% only makes this unintuitive. So the solution would be create a separate HDR value so it don't mess up with the main color.

@ghsoares
Copy link
Author

If we take as example the Unity engine, in the materials we have HDR colors as emission, for example, were the power is a separated parameter, this makes easier to control how bright is a material without changing the emission color, but how bright the color is.

@Calinou Calinou changed the title HDR Color toggle and a separated slider for power Add an HDR Color toggle and a separated slider for power Mar 28, 2022
@Calinou
Copy link
Member

Calinou commented Jun 26, 2022

If I recall correctly, reduz was more of the opinion that nodes/resources where using overbright colors is relevant should have an Energy property associated (like Light3D), rather than adding an energy slider to ColorPicker. The multiplication is done on the CPU side when passing the uniform, so the shader remains identical. This is why the wireframes in #4353 (comment) don't have an energy slider.

Overbright/HDR colors tend to be poorly understood; even if they're exposed in ColorPicker in a more visible way, they may not be used effectively as often as we'd hope.

@QbieShay
Copy link

Up for this proposal :D

reduz was more of the opinion that nodes/resources where using overbright colors is relevant should have an Energy property associated (like Light3D)

This seems way more complicated structurally than a color picker with value buttons, and prone to situations where you want to integrate it with your own nodes/shaders and the engine failing to adapt to your pipeline.

@fire
Copy link
Member

fire commented Jul 10, 2022

#4353 has discussion for an intensity slider that multiplies the rgba8 values.

This was discussed with redlamp and others in rocketchat few months ago.

@RyanGatts
Copy link

I'm inclined to agree with @KoBeWi , Having the V component of the HSV go past 100 makes a lot of intuitive sense to me and doesn't involve adding any new controls. Tolerating that kind of input is at least useful, because it makes it so that you maintain access to the hue control for adjusting HDR colors (which is fairly common in vfx) while preserving their brightness. Currently, attempting to use the HSV controls when working on an HDR color crushes the color back into SDR, and it must be multiplied back up to range in a separate color picker, which is inconsistent, and kind of annoying to do.

@fire
Copy link
Member

fire commented Nov 17, 2023

Colour is not defined on srgb to have hsv values > 1.0. I suggest an intensity multiplier or raw.

My thought is raw numbers is hard to use.

I believe hdr intensity was rejected but I don’t agree. Does anyone have the historical argument for it?

@QbieShay
Copy link

Would this be in the color picker or in the Color class?

@Calinou
Copy link
Member

Calinou commented Nov 17, 2023

I believe hdr intensity was rejected but I don’t agree. Does anyone have the historical argument for it?

I believe the argument was that it's not always possible to infer the previously used intensity for a given color, unless you store intensity as a separate field (with memory usage and complexity concerns).

For instance, in RGB mode, if you set the color to 20, 40, 30 and intensity 2.0, the final color will be 40, 80, 60. However, if you exit the color picker and open it again, you'll see a color of 40, 80, 60 with intensity 1.0. The color picker is unable to know the previously used intensity value, as this color can be input with an intensity of 1.0 just fine.

Also, separate energy properties in nodes that need it encourage the use of them to animate energy separately from the color itself. This can't be done with a color picker intensity slider, unless we implement the intensity as a full-fledged property of Color that can be changed in scripts and keyed in animations.

I'd personally be OK with having an intensity slider that works purely as a convenience tool, but it needs to made very clear that the intensity value won't be preserved if the final color isn't overbright (i.e. at least one of its components exceeds 255 in RGB mode).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants