Closed TranquilMarmot closed 1 month ago
It looks like this is the same issue mentioned in https://github.com/godotengine/godot-docs/issues/9591 (see also https://github.com/godotengine/godot/pull/86316)
Applying this function to each of the normal samples fixes the issue:
vec4 normal_roughness_compatibility(vec4 p_normal_roughness) {
float roughness = p_normal_roughness.w;
if (roughness > 0.5) {
roughness = 1.0 - roughness;
}
roughness /= (127.0 / 255.0);
return vec4(normalize(p_normal_roughness.xyz * 2.0 - 1.0) * 0.5 + 0.5, roughness);
}
float detectEdgesNormal(UVNeighbors uvs, sampler2D normTex, vec3 camDirWorld){
vec3 n_u = normal_roughness_compatibility(texture(normTex, uvs.up)).xyz;
vec3 n_d = normal_roughness_compatibility(texture(normTex, uvs.down)).xyz;
vec3 n_l = normal_roughness_compatibility(texture(normTex, uvs.left)).xyz;
vec3 n_r = normal_roughness_compatibility(texture(normTex, uvs.right)).xyz;
vec3 n_tl = normal_roughness_compatibility(texture(normTex, uvs.top_left)).xyz;
vec3 n_tr = normal_roughness_compatibility(texture(normTex, uvs.top_right)).xyz;
vec3 n_bl = normal_roughness_compatibility(texture(normTex, uvs.bottom_left)).xyz;
vec3 n_br = normal_roughness_compatibility(texture(normTex, uvs.bottom_right)).xyz;
vec3 normalFiniteDifference0 = n_tr - n_bl;
vec3 normalFiniteDifference1 = n_tl - n_br;
vec3 normalFiniteDifference2 = n_l - n_r;
vec3 normalFiniteDifference3 = n_u - n_d;
float edgeNormal = sqrt(
dot(normalFiniteDifference0, normalFiniteDifference0) +
dot(normalFiniteDifference1, normalFiniteDifference1) +
dot(normalFiniteDifference2, normalFiniteDifference2) +
dot(normalFiniteDifference3, normalFiniteDifference3)
);
return smoothstep(normal_threshold - normal_smoothing, normal_threshold + normal_smoothing, edgeNormal);
}
I don't think the change mentioned above for detectEdgesDepth
is needed.
Interesting! I opened the docs issue for CompositorEffects, and I was only able to discover the problem because the Spatial shader on my quadmesh WAS receiving the correct (converted) normals values, and my Compute shader wasn't. I just confirmed the Spatial shader seems to still be getting the correct normals values in 4.3-rc1.
However, when I open your MRP, I am able to reproduce the same artifacts.
Here's my Spatial shader. Results were the same with linear sampling.
shader_type spatial;
uniform sampler2D normal_roughness_texture : hint_normal_roughness_texture, repeat_disable, filter_nearest;
void vertex() {
POSITION = vec4(VERTEX.xy, 1.0, 1.0);
}
void fragment() {
vec3 screen_normal = texture(normal_roughness_texture, SCREEN_UV).xyz;
screen_normal = screen_normal * 2.0 - 1.0;
ALBEDO = screen_normal;
}
Very interesting catch!
We have code that automatically detects when the normal_roughness buffer is used and it automatically wraps the texture()
call with normal_roughness_compatibility()
. However, it looks like this is failing when the normal_roughness texture is passed to a function.
Here is the generated code:
fragment()
{
float m_aspect=(float(read_viewport_size.y) / float(read_viewport_size.x));
m_UVNeighbors m_n=m_getNeighbors(screen_uv, material.m_max_thickness, m_aspect);
m_NeighborDepthSamples m_depth_samples=m_getLinearDepthSamples(m_n, depth_buffer, inv_projection_matrix);
float m_min_d=m_getMinimumDepth(m_depth_samples);
float m_thickness=clamp(m_remap(m_min_d, material.m_min_distance, material.m_max_distance, material.m_max_thickness, material.m_min_thickness), material.m_min_thickness, material.m_max_thickness);
float m_fade_a=clamp(m_remap(m_min_d, material.m_min_distance, material.m_max_distance, 1.0, 0.0), 0.0, 1.0);
m_n=m_getNeighbors(screen_uv, m_thickness, m_aspect);
m_depth_samples=m_getLinearDepthSamples(m_n, depth_buffer, inv_projection_matrix);
vec3 m_pixel_normal=normal_roughness_compatibility(texture(sampler2D(normal_roughness_buffer, SAMPLER_LINEAR_CLAMP), screen_uv)).xyz;
float m_depthEdges=m_detectEdgesDepth(m_depth_samples, m_pixel_normal, view);
float m_normEdges=min(m_detectEdgesNormal(m_n, normal_roughness_buffer, scene_data.inv_view_matrix[2].xyz), 1.0);
albedo.rgb=material.m_outlineColor.rgb;
alpha=((max(m_depthEdges, m_normEdges) * material.m_outlineColor.a) * m_fade_a);
}
And
float m_detectEdgesNormal(m_UVNeighbors m_uvs, texture2D m_normTex, vec3 m_camDirWorld)
{
vec3 m_n_u=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.up).xyz;
vec3 m_n_d=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.down).xyz;
vec3 m_n_l=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.left).xyz;
vec3 m_n_r=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.right).xyz;
vec3 m_n_tl=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.top_left).xyz;
vec3 m_n_tr=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.top_right).xyz;
vec3 m_n_bl=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.bottom_left).xyz;
vec3 m_n_br=texture(sampler2D(m_normTex, SAMPLER_LINEAR_CLAMP), m_uvs.bottom_right).xyz;
vec3 m_normalFiniteDifference0=(m_n_tr - m_n_bl);
vec3 m_normalFiniteDifference1=(m_n_tl - m_n_br);
vec3 m_normalFiniteDifference2=(m_n_l - m_n_r);
vec3 m_normalFiniteDifference3=(m_n_u - m_n_d);
float m_edgeNormal=sqrt((((dot(m_normalFiniteDifference0, m_normalFiniteDifference0) + dot(m_normalFiniteDifference1, m_normalFiniteDifference1)) + dot(m_normalFiniteDifference2, m_normalFiniteDifference2)) + dot(m_normalFiniteDifference3, m_normalFiniteDifference3)));
return smoothstep((material.m_normal_threshold - material.m_normal_smoothing), (material.m_normal_threshold + material.m_normal_smoothing), m_edgeNormal); }
You can see the compatibility code is only added in the first case. Looking into a fix now
I have a patch that fixes this issue, but it introduces another issue that we have run into before. If we add the compatibility code to ensure that the normal is read correctly, it makes it so that function will always have the compatibility code. Therefore if you call the function with both the normal_roughness buffer and some other texture, it will fail for that other texture.
We ran into this issue for XR and ended up just banning passing the screen textures as arguments in custom functions when using XR. I think i can fix both cases by just banning using a screen texture as an argument in one place while using a different texture in another place
Edit: That was unexpectedly easy. PR incoming
Tested versions
Reproduceable in: Godot v4.3.rc1.mono
Not reproduceable in: Godot v4.2.2.stable.mono
System information
macOS 14.5.0 - Vulkan (Forward+) - integrated Apple M1 - Apple M1 (8 Threads)
Issue description
I'm using this shader: https://godotshaders.com/shader/high-quality-post-process-outline/
Here it is for Godot 4.2 with all of the irrelevant mobile-related code stripped out:
Behavior in 4.2
Nice outlines, no artifacts 👍
https://github.com/user-attachments/assets/55bc71c2-6b10-4928-b062-722f9c921aae
Behavior in 4.3
After reading through the Introducing Reverse Z (AKA I'm sorry I broke your shader) article, I made the following changes:
However, running this in Godot 4.3 there are artifacts on sloped surfaces.
https://github.com/user-attachments/assets/51c4f939-27d2-4563-8d03-4cfa68fa1b7d
Steps to reproduce
MeshInstance3D
in the sceneQuadMesh
ShaderMaterial
using the shaderMinimal reproduction project (MRP)
Here is a repo with a reproduction:
https://github.com/TranquilMarmot/godot_4.3_shader_artifacts
There are two folders:
4.2
: Open with Godot 4.2 to see intended behavior4.3
: Open with Godot 4.3 to see broken behavior