depth_draw_never moves objects into the transparent pipeline for no reason

[celyk]

Godot version

v4.0.rc1.official [8843d9a]

System information

Windows 11, Vulkan

Issue description

Disabling depth testing or disabling depth drawing unexpectedly removes objects from the opaque pass, so they disappear from screen textures. This is problematic because the user may intend to keep the objects opaque. The user may also wish to write directly to the depth buffer at any time, this is why depth settings and transparency should be decoupled.

Example:
In Godot 3.5 I am able to create this x-ray effect entirely in the opaque pipeline by utilizing render priority and render_mode depth_draw_never in my shader, this is not possible anymore

Steps to reproduce

1. Create a new MeshInstance3D and assign a mesh
2. Assign a StandardMaterial3D
3. Set depth_draw_mode = disabled, or set no_depth_test = true
4. Check to see if they appear in the screen textures (SCREEN_TEXTURE or DEPTH_TEXTURE) using a screen-reading shader

In the reproduction you'll find 3 objects corresponding to each setting, and a box for peeking at the screen texture. If something doesn't appear in the screen texture, that implies it is not in the opaque pipeline

Minimal reproduction project

issue-reproduction.zip

[clayjohn]

Looks like it works if you change render priority and then run the project or reload the editor. The issue is that the mesh isn't updating its surface cache when render priority changes. Should be an easy fix

[clayjohn]

Also, to add to this. Nothing relevant to this issue has changed between 3.5 and 4.1. Disabling depth test or force disabling depth write always moved the object to the transparent pass (which is why render_priority worked for those objects. render_priority only applies to objects in the transparent pipeline).

[celyk]

Also, to add to this. Nothing relevant to this issue has changed between 3.5 and 4.1. Disabling depth test or force disabling depth write always moved the object to the transparent pass (which is why render_priority worked for those objects. render_priority only applies to objects in the transparent pipeline).

That's not my experience. In 3.5, render_priority consistently affects opaque objects as well as transparent ones, and depth_draw_never does not put the object into the transparent pipeline. Some of my effects depend on these facts.

I have recreated the setup in 3.5.1 to demonstrate these subtle differences:

The opaque objects have been duplicated and colored red. Because the object named default_red perfectly overlaps with the original, render priority determines which of them is shown. Likewise, the object named depth_draw_never_red always renders on top of the original, unless you change the render_priority.

Let me know if you can reproduce these results
issue-reproduction-gd3.zip

[celyk]

It doesn't make sense to me for these depth settings by themselves to affect when the object is rendered, or at least to give the user next to no choice on that. I can make a proposal about this if that is more appropriate

[clayjohn]

That's not my experience. In 3.5, render_priority consistently affects opaque objects as well as transparent ones, and depth_draw_never does not put the object into the transparent pipeline. Some of my effects depend on these facts.

My above statement was a little too strong. render_priority does impact opaque objects, just not in the same way as transparent objects. For transparent objects it totally reorders the order in which they render (low priority always get rendered before higher priority and are thus "behind" higher priority). Opaque objects use priority as part of the "key" that they are sorted by. So priority is one factor of several that determines draw order. That being said, saying that render_priority only impacts transparent objects was wrong.

I'll take a look at your 3.x MRP and see if the same effect can be done in current master (with #73263 merged)

Edit: I looked at your 3.x MRP and I can confirm it looks like in 3.x depth_draw_never kept objects in the opaque pass, but in 4.0 it moves objects to the alpha pass Tracking down the change it looks like it happened in #44838 and was likely not intentional

[clayjohn]

There were two pieces to this bug report:

1. render_priority couldn't be used to reorder draw calls (fixed by Notify mesh surface when render_priority changes #73263)
2. depth_draw_never moves objects into the transparent pipeline for no reason

Only the first was fixed, so reopening and renaming to track the second

edit: marking as 4.1 as the changes won't be trivial.

Basically, right now this block ensures that depth_draw_never always goes through the transparent pipeline:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 3449 to 3460 in 8c7b98d

	if (has_alpha || has_read_screen_alpha || p_material->shader_data->depth_draw == SceneShaderForwardClustered::ShaderData::DEPTH_DRAW_DISABLED || p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED) {
	//material is only meant for alpha pass
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_ALPHA;
	if ((p_material->shader_data->uses_depth_prepass_alpha || p_material->shader_data->uses_alpha_antialiasing) && !(p_material->shader_data->depth_draw == SceneShaderForwardClustered::ShaderData::DEPTH_DRAW_DISABLED || p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED)) {
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH;
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_SHADOW;
	}
	} else {
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE;
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH;
	flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_SHADOW;
	}

We could change it to:

	if (has_alpha || has_read_screen_alpha || p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED) {
		//material is only meant for alpha pass
		flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_ALPHA;
		if ((p_material->shader_data->uses_depth_prepass_alpha || p_material->shader_data->uses_alpha_antialiasing) && !(p_material->shader_data->depth_draw == SceneShaderForwardClustered::ShaderData::DEPTH_DRAW_DISABLED || p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED)) {
			flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH;
			flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_SHADOW;
		}
	} else {
		flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE;
		if (p_material->shader_data->depth_draw != SceneShaderForwardClustered::ShaderData::DEPTH_DRAW_DISABLED) {
			flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH;
			flags |= GeometryInstanceSurfaceDataCache::FLAG_PASS_SHADOW;
		}
	}

But we run into another issue, Opaque objects are always added to the depth pass:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 1041 to 1043 in 8c7b98d

	if (surf->flags & (GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH | GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE)) {
	rl->add_element(surf);
	}

We could remove that, but we would risk breaking other areas. So this change needs to be discussed after 4.0 releases and then made for 4.1 if other rendering contributors agree that it is necessary

[apples]

I ran into this while experimentally trying to implement godotengine/godot-proposals#1298.

I tried implementing a simple silhouette material by setting the depth function to greater, disabling depth write, and setting the model's actual material (a standard opaque material) to render on top (using any means, e.g. next_pass). My expectation was that the silhouette material would be drawn in the opaque pass before the second material, but it was of course deferred to the transparent pass.

In this screenshot, there are red artifacts over the green material due to the red silhouette material being drawn in the transparent pass, while the green material is opaque. (The "Depth Function" is the property I added, it merely sets the depth comparison operator.)

Such a silhouette material would rely on the fact that it would be drawn in the opaque pass, but it also should not write depth, as doing so would cause other rendering artifacts.

But we run into another issue, Opaque objects are always added to the depth pass:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 1041 to 1043 in 8c7b98d

	if (surf->flags & (GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH | GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE)) {
	rl->add_element(surf);
	}

We could remove that, but we would risk breaking other areas.

But depth_draw_never materials are already excluded from the depth pass, right?

I think the bigger breaking change would just be that people might already be relying on depth_draw_never using the transparent pass. Perhaps we need another flag on the material to indicate which pass(es) should be used?

As an aside - the behavior of the materials I was testing with seems to differ a bit between the Forward+ and Mobile render modes. Not sure if there's a related issue there.

[clayjohn]

But depth_draw_never materials are already excluded from the depth pass, right?

Currently in Godot 4.0 depth_draw_never materials are always sent into the alpha pass and excluded from the depth pass. However, opaque materials are always sent to the depth pass. So we would need to add a new code path for opaque objects that aren't included in the depth pass.

I think the bigger breaking change would just be that people might already be relying on depth_draw_never using the transparent pass. Perhaps we need another flag on the material to indicate which pass(es) should be used?

We may have to do that, but I hope to avoid doing that if possible. We will have to discuss with other interested parties what the best solution is.

As an aside - the behavior of the materials I was testing with seems to differ a bit between the Forward+ and Mobile render modes. Not sure if there's a related issue there.

There might be. Also keep in mind that the Mobile renderer never uses a depth prepass which may break some assumptions about what information is available when.

[apples]

I tried implementing the changes suggested in #73158 (comment). My final changes can be found here: apples@451b04a. No GLES3 yet. I have not made any changes to the shadow flag in my commit, but I do believe the shadow flag should be moved as well.

One additional change I had to make was changing this check:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Lines 1003 to 1005 in 61d2c85

	if (!force_alpha && (surf->flags & (GeometryInstanceSurfaceDataCache::FLAG_PASS_DEPTH | GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE))) {
	rl->add_element(surf);
	}

To this:

if (!force_alpha && (surf->flags & GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE)) {
	rl->add_element(surf);
}

That change was only necessary in the Forward+ renderer, the Mobile renderer check was already similar to mine. I assume this is related to the depth prepass.

Additionally, I had to modify this check:

godot/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

Line 3459 in 61d2c85

if (has_alpha || has_read_screen_alpha || p_material->shader_data->depth_draw == SceneShaderForwardClustered::ShaderData::DEPTH_DRAW_DISABLED || p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED) {

As such:

if (has_alpha || has_read_screen_alpha) {

Overall, I personally think these changes would match my expectations of how materials should be assigned to the different rendering passes. Essentially, I find it odd that there are various rules for how materials are considered "transparent", when there is an explicit flag for it on the material. I feel like only materials with the transparent flag set should be in the alpha pass, regardless of depth flags.

However, these changes really seem to me like it would be an annoyance to people who have already made materials under these assumptions. For my purposes, I would be fine with adding a pass_override property with flags like NO_OVERRIDE, ALPHA_ONLY, OPAQUE_ONLY, and OPAQUE_AND_DEPTH.

Adding a new property like this would make the rendering logic more complicated.

Also, I haven't tested how these changes might interact with sky/fog/etc.

EDIT: Just to add: My motivation in implementing these changes, is to create a better "character behind wall" silhouette shader in the opaque pass. For me, another viable solution would be to simply add a silhouette flag to the material. But I think that resolving this issue in a more general way would open the pathway for more creative shaders.

[clayjohn]

@apples The changes seem good except the removal of the check for DEPTH_TEST_DISABLED p_material->shader_data->depth_test == SceneShaderForwardClustered::ShaderData::DEPTH_TEST_DISABLED. When depth test is disabled the object needs to be rendered in the transparent pass so that it is sorted back to front. Otherwise you will have very little control over how it gets rendered and it would always render behind transparent objects

[yoont4]

Just wanted to ask what the outlook is for the depth_draw_never transparency bug? I am running into a problem where I wanted to make a screen-reading distortion shader (heat vents, soap bubbles, etc.) that is in-world (NOT a post-processing effect).

I understand the hint_screen_texture flag is handled after opaque geometry, but before transparent geometry. Ideally I'd be able to sample from the screen after everything, but I'm happy to settle with only opaque pixels getting the distortion effect, and transparent effects being undistorted. I was gonna use the depth_draw_never flag to make it so the cards are ignored by transparent objects drawn in the pass afterwards, but I was running into them still getting occluded! This thread makes sense as to why it appears that way for me: if the card is being rendered in the transparency pass, it will still render on top of the transparent materials, thereby re-projecting the opaque-only information onto itself and hiding the transparent materials behind it.

Here's a screenshot of the issue:

And the simplified shader:

shader_type spatial;
render_mode unshaded, depth_draw_never;

uniform sampler2D screen_texture : hint_screen_texture, repeat_disable, filter_nearest;

void fragment() {
	ALBEDO = textureLod(screen_texture, SCREEN_UV, 0.0).rgb;
}

[yoont4]

I realize now that I can just use a render priority of -1 to make it appear behind 💀

[pineapplemachine]

Noting here that I ran into confusing behavior with depth_draw_opaque in a spatial shader. Even when I set ALPHA = 1.0 in the shader for every fragment, depth behavior was not working as expected. (Expected behavior would have been the same visual result as depth_draw_always.)

It worked after adding ALPHA_HASH_SCALE = 1.0; to the fragment shader. It isn't clear to me why this fixed it.

When I reported this on the forum here, @Calinou suggested that it may be related, so I'm putting this here in case it's of relevance to anyone working on this issue: https://forum.godotengine.org/t/depth-draw-opaque-not-working-as-expected-depth-is-not-drawn-even-when-alpha-is-1/59492