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

SCREEN_TEXTURE is not receiving colors defined with RAW picker correctly #59530

Closed
claychinasky opened this issue Mar 25, 2022 · 6 comments
Closed

Comments

@claychinasky
Copy link

Godot version

Godot 4.0alpha5

System information

Windows 10, RTX 2080, Vulkan clustered

Issue description

SCREEN_TEXTURE is not receiving colors defined with RAW picker which values are higher than 1.0f but normalizes them somewhere in the chain.
Sprite2D, Polygon2D and modulate color values are acting the same way.
As shader code describes, screen_texture conditions should capture RAW values and turn them into green.

This issue wasn't present in Godot 3.x and it is working as expected.

Steps to reproduce

Create Polygon2D color or sprite2D modulate with RAW colors higher than 1.0
Create a ColorRect with custom shader ;

shader_type canvas_item;
render_mode blend_mix;

void fragment() {
	vec4 tex = textureLod(SCREEN_TEXTURE, SCREEN_UV, 0.0);
	tex.a = 0.0;
	
//  post-multiply to see if values can exceed higher than 1.0, which it can
//	tex.r *= 3.0;

	if (tex.r > 1.0){
		tex.r = 0.0;
		tex.g = 1.0;
		tex.b = 0.0;
		tex.a = 1.0;
	}	
	COLOR = tex;
}

Minimal reproduction project

issue_raw_colors.zip

@clayjohn
Copy link
Member

In Godot 4.0, the 2D render target is LDR (meaning it stores values between 0-1 only). In Godot 3.x, the 2D render target uses the same precision as the 3D render target which is configurable, but defaults to HDR. So this isn't really a bug, it is a change in behaviour.

I guess what we need to figure out is if the precision of the 2D render target should be configurable in 4.0 as well, or if it makes more sense for you to workaround this new limitation another way.

@Calinou
Copy link
Member

Calinou commented Mar 25, 2022

I guess what we need to figure out is if the precision of the 2D render target should be configurable in 4.0 as well, or if it makes more sense for you to workaround this new limitation another way.

A 2D HDR render target is required for 2D glow to work as it does in 3.x. While it is possible to use glow in LDR too, I think many people will miss the ability to have physically accurate glow in 2D.

@claychinasky
Copy link
Author

A 2D HDR render target is required for 2D glow to work as it does in 3.x. While it is possible to use glow in LDR too, I think many people will miss the ability to have physically accurate glow in 2D.

I feel that it's important for glow to work with precision but also I don't see a reason to handle 2D and 3D differently (besides technical reasons). Since 2D and 3D is being rasterized in screen same way and becomes 2D image. For example without that, engine won't be able to render or process a real life photo taken by those ranges correctly.

@clayjohn
Copy link
Member

clayjohn commented Mar 28, 2022

I feel that it's important for glow to work with precision but also I don't see a reason to handle 2D and 3D differently (besides technical reasons)

Its mostly about performance/memory. Most users are rendering to LDR screens, which means our final output needs to be LDR. In 3D the HDR rendering is converted to LDR when the 3D scene is copied into the canvas backbuffer (which is used for 2D rendering). Most 2D rendering takes place in LDR anyway (in particular, very few 2D games use a post-processing tonemap), so it makes sense to keep 2D in LDR. This benefits performance by reducing the memory required to do operations on the backbuffer. The other main benefit is simplicity in the render pipeline, with Vulkan, things can become pretty unwieldy from a code standpoint if you support many different configurations.

I tend to agree that there should be an HDR option, importantly, it is necessary if users want to SubViewports as generic render targets for GPGPU.

@claychinasky
Copy link
Author

claychinasky commented Mar 29, 2022

Thanks for the explanation.
If I understood correctly, even if screen_texture is read from 3D scene, it will still be in LDR because its already converted. So other effects like built-in tonemapping happen to take place before the conversation? (not sure if workflow of SubViewports/ViewportContainers will takes place before the conversation, which documented for custom post-processing)
However, I'm hopeful that it will get discussed and implemented on the roadmap as you pointed out also GPGPU as a thing.

@Calinou
Copy link
Member

Calinou commented Jun 16, 2022

Duplicate of #54122 (same cause).

@Calinou Calinou closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants