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

Depth buffer resolution is unexpectedly small when sampled in spatial shader #89701

Open
jake-low opened this issue Mar 20, 2024 · 4 comments
Open

Comments

@jake-low
Copy link

jake-low commented Mar 20, 2024

Tested versions

Noticed in: v4.2.1.stable (when porting a working project from 3.x)
Also found when testing: v4.0.0, v4.2.2-rc2, v4.3-dev1, v4.3-dev2
Partially fixed in: v4.3-dev3 and later (Forward+ renderer is fixed, Mobile renderer still has same issue)

System information

Godot v4.2.1.stable - macOS 14.3.1 - Vulkan (Forward+) - dedicated AMD Radeon Pro 555X - Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz (12 Threads)

Issue description

On my system (macOS, Intel CPU, AMD GPU), 3D shaders which sample the depth buffer have visual artifacts that suggest that the depth buffer being rendered at a much lower resolution than the main camera.

Here's a scene with a plane above a torus. The plane has a simple shader attached (included in sample project zip, see below) which samples the depth buffer and uses it to set the albedo. The result appears chunky, with depth buffer resolution appearing ~8x less in both dimensions than the main camera's output.

repro-1.mp4

A similar effect can be seen when using the Proximity Fade property on the standard material.

repro-2.mp4

I encountered this when porting a shader that uses the depth buffer from Godot 3.4 to Godot 4.2.1. That shader worked as expected on the same computer under that version of Godot (and the GLES 2 backend) - i.e. the depth texture was the same resolution as the scene.

Steps to reproduce

I can repro this on my machine with either the Forward+ or Mobile rendering method. I guess depth buffer sampling isn't supported in Compatibility mode so this bug isn't relevant there.

Minimal reproduction project (MRP)

This test project contains the scene shown above. It's configured with the standard material with the proximity fade property enabled. The shader code shown above is also included.

DepthBufferBugRepro.zip

@Zireael07
Copy link
Contributor

IIRC depth works differently in Vulkan than in OpenGL - have you accounted for this fact when porting?

@jake-low
Copy link
Author

IIRC depth works differently in Vulkan than in OpenGL - have you accounted for this fact when porting?

Yes, I think so. For convenience, here's the code from the shader example above (video 1), which is also included in the zipped project.

shader_type spatial;

uniform sampler2D DEPTH_TEXTURE: hint_depth_texture;

void fragment() {
	float depth = textureLod(DEPTH_TEXTURE, SCREEN_UV, 0.0).r;
	vec3 ndc = vec3(SCREEN_UV * 2.0 - 1.0, depth);
	vec4 view = INV_PROJECTION_MATRIX * vec4(ndc, 1.0);
	view.xyz /= view.w;
	ALBEDO = vec3(smoothstep(view.z + 0.5, view.z, VERTEX.z));
}

I think only relevant difference between Vulkan and OpenGL in this situation is that Vulkan's normalized device coordinates go from 0 to 1 in Z, instead of -1 to 1 like in OpenGL. So the line vec3 ndc = vec3(SCREEN_UV * 2.0 - 1.0, depth); would be vec3 ndc = vec3(SCREEN_UV, depth) * 2.0 - 1.0; in OpenGL.

I realized that there's an even simpler repro. This shader:

shader_type spatial;

uniform sampler2D DEPTH_TEXTURE: hint_depth_texture;

void fragment() {
	ALBEDO = vec3(1.0 - textureLod(DEPTH_TEXTURE, SCREEN_UV, 0.0).r);
}

Should look something like this (screenshot from this video)

image

But on my machine it looks like this:

image

Maybe this can help narrow down the issue. I think it illustrates that two things are wrong: the depth texture is lower resolution than the scene (big chunky pixels visible) and the values at each pixel are surprising (pixels of the plane which are in front of the torus are all white, rather than showing a gray ramp).

@jake-low
Copy link
Author

I tested a few more versions and discovered that this bug appears to have been fixed in v4.3-dev3 for the Forward+ renderer. But it can still be observed when using the Mobile renderer.

Here's a screenshot of v4.3-dev3 using the Forward+ renderer, showing the shader working as expected:
image

And here's a screenshot using the Mobile renderer where the bug still exists.

image

The same can be seen on the latest v4.3 build (dev5).

I looked at the v4.3-dev3 changelog and saw that there's a change that was made to the default bit depth for the depth buffer when using the Forward+ renderer. Seems like it could be related? #87057

Anyway, this solves my personal use case, so I'm happy, but I will leave this issue open since it appears that there is still a bug affecting the Mobile render method.

@Calinou
Copy link
Member

Calinou commented Apr 22, 2024

Anyway, this solves my personal use case, so I'm happy, but I will leave this issue open since it appears that there is still a bug affecting the Mobile render method.

This makes me wonder if 32-bit depth should be used on desktop platforms regardless of the rendering method (Forward+ or Mobile), since there's virtually no performance penalty in this scenario. See #87057.

The downside of this approach is that it may result in some projects looking slightly different between desktop and mobile platforms when using the Mobile rendering method on both (i.e. your issue would still occur on mobile platforms, but not on desktop).

It's possible that the 24-bit depth implementation in the macOS GPU drivers for AMD is broken too. As mentioned in the proposal, AMD hardware has never really had real support for 24-bit depth – instead, it's emulated with 32-bit depth.

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

4 participants