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

Strange lights show on ShaderMaterial in 4.2 #84089

Closed
Flynsarmy opened this issue Oct 28, 2023 · 5 comments · Fixed by #84159
Closed

Strange lights show on ShaderMaterial in 4.2 #84089

Flynsarmy opened this issue Oct 28, 2023 · 5 comments · Fixed by #84159

Comments

@Flynsarmy
Copy link
Contributor

Flynsarmy commented Oct 28, 2023

Godot version

v4.2.beta3.official

System information

Windows 11, Forward+, geforce 3070TI

Issue description

An imported cylinder mesh with ShaderMaterial applied has a strange lights show appearing above it when I move rotate the viewport in the 4.2b3. An identical project in 4.1 does not exhibit this behaviour.

Here's a video showing the behaviour in 4.1 then the behaviour in an identical 4.2 project:

2023-10-28.11-02-35.mp4

Steps to reproduce

Import a cylinder OBJ and drag it into the scene. Add a surface material override ShaderMaterial with the following shader:

shader_type spatial;
render_mode blend_mix, unshaded, cull_disabled, depth_draw_opaque, diffuse_lambert, specular_schlick_ggx;

uniform sampler2D noise_tex;
uniform vec2 noise_tex_amount = vec2(1.0, 0.5);
uniform vec2 noise_tex_speed = vec2(0, 0.3);
uniform float noise_tex_power = 3.0;
uniform vec4 color: source_color = vec4(3.34, 2.35, 4.5, 1.0);

void fragment() {
	// pan the UV
	vec2 uv = (UV * noise_tex_amount) + TIME * noise_tex_speed;

	vec4 outTex = texture(noise_tex, uv);
	// Fade out towards the top
	outTex *= vec4(pow(UV.y, noise_tex_power));

	ALBEDO = outTex.xyz * color.xyz;
	ALPHA = 0.2 * length(outTex);
}

Move the camera around in the editor.

Minimal reproduction project

The zip contains a 4.1 project and an identical 4.2 project.
vfx41-42.zip

@Flynsarmy
Copy link
Contributor Author

Flynsarmy commented Oct 28, 2023

This issue appears to be something to do with the way meshes are handled in 4.2. If I replace the imported mesh with a built in CylinderMesh with caps removed and change UV.y to UV.y + 0.25 to account for the differing origins, the light show disappears.

@bitsawer
Copy link
Member

I can repro the visual issues on current master at 3f04595. Seems like selecting cylinder.obj and reimporting it by enabling Force Disable Mesh Compression fixes the issue, so it has possibly something to do with the new mesh compression. Could be a bug in either Godot mesh handling or the actual .obj mesh geometry might have weird or invalid data that now behaves badly.

The flickering visuals seem to be caused by the Glow effect, adding a WorldEnvironment node to the scene and disabling its Glow seems to also remove (or hide) these visual issues. However, the root cause is still possibly caused by the mesh data (bad normals or tangents?), this might need some more investigation. CC @clayjohn

@clayjohn
Copy link
Member

Looks like an issue with UVs. I don't see how normals or tangents would change this behaviour. It could be that the UV is flooring to 0 (because it's using 16 bit precision when compressed) and that is causing issue with the call to pow(). I can take a deeper look when I am in front of a computer

@Flynsarmy
Copy link
Contributor Author

image
The UVs at least on the Blender side seem correct.

If it helps, I've attached the blend file this cylinder came from. I exported to Godot as OBJ with 'export materials' unchecked and every other setting left default.
meshes.zip

@clayjohn
Copy link
Member

Okay, testing locally it looks like UV.y is sometimes negative which is undefined behaviour in GLSL

Changing the pow line to outTex *= vec4(pow(max(0.0, UV.y), noise_tex_power)); fixes the issue.

Looking at the generated UVs, this indeed comes from our UV compression scheme. Basically the way UV compression works is we determine the absolute range (i.e. the furthest we get away from 0) and use that to scale the UVs.

In this case since the UVs are all in the 0-1 range, we get an absolute range of one. So we scale the UVs by doing UV / 2 + 0.5 (this way anything within -1 - 1 is brought within the 0-1 range).

Since 16 bit unorms can't store 0.5 exactly, 0.0 ends up becoming 0.49999 which ends up as -0.0002 in the fragment shader. Taking the pow() of a negative number leads to undefined behaviour which is likely creating NaNs which are triggering the glow.

For now, you have two solutions:

  1. Add that max(0.0, UV.y) line into your shader code. In my opinion, this is the "correct" option as you should always ensure you never pass a negative number into pow(). (some GPU drivers will emit negative UVs even in the normal course of things, so its good to have this line regardless)
  2. Force disable compression. This sort of case is the reason we added the option to force disable compression. But, in my opinion, it kind of sucks to have to disable compression (and lose the performance benefit) to workaround a shader issue.

I'll also investigate if I can improve this on the engine side. It would be nice to guarantee that positive numbers stay positive with the compression scheme.

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

Successfully merging a pull request may close this issue.

3 participants