Add Depth Offset property to BaseMaterial3D and fix collision shape gizmo flicker#100211
Add Depth Offset property to BaseMaterial3D and fix collision shape gizmo flicker#100211Calinou wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
For those curious, this part is the exact same as the DEPTH adjustment code featured in #97646.
There was a problem hiding this comment.
This looks suspiciously like it's dependent on Vulkan-style NDCs where the NDC.z is in the range [0,1], and therefore won't work correctly in Compatibility? But I only know just enough to use these in typical cases in user shaders, not quite enough to suggest a proper correction here.
There was a problem hiding this comment.
Yes, it likely needs an #ifdef check depending on the current rendering method to alter the generated code. I don't know what's the conversion formula for this is though.
(This should use the shader preprocessor, so that it keeps working if you convert the BaseMaterial3D to a ShaderMaterial then switch rendering methods.)
There was a problem hiding this comment.
I think the Compatibility view position snippet is this, a variation of what is documented here:
#if CURRENT_RENDERER == RENDERER_COMPATIBILITY
vec4 view_position = INV_PROJECTION_MATRIX * vec4(SCREEN_UV * 2. - 1.0, FRAGCOORD.z * 2. - 1.0, 1.0);
#else
vec4 view_position = INV_PROJECTION_MATRIX * vec4(SCREEN_UV * 2. - 1.0, FRAGCOORD.z, 1.0);
#endif
or
#if CURRENT_RENDERER == RENDERER_COMPATIBILITY
vec4 view_position = INV_PROJECTION_MATRIX * vec4(vec3(SCREEN_UV, FRAGCOORD.z) * 2. - 1.0, 1.0);
#else
vec4 view_position = INV_PROJECTION_MATRIX * vec4(SCREEN_UV * 2. - 1.0, FRAGCOORD.z, 1.0);
#endif
But you probably also need to convert NDC.z again when setting depth:
#if CURRENT_RENDERER == RENDERER_COMPATIBILITY
DEPTH = ndc_position.z * 0.5 + 0.5;
#else
DEPTH = ndc_position.z;
#endif
Again, I only have casual user shader knowledge of this, and my understanding of the underlying coordinate spaces is still fuzzy.
Since the material writes out a GDShader, not GLSL directly, we still have access to the preprocessor here, right?
There was a problem hiding this comment.
Thanks! I'll look into integrating this.
Since the material writes out a GDShader, not GLSL directly, we still have access to the preprocessor here, right?
Yes 🙂
There was a problem hiding this comment.
It seems preprocessor macros don't work within the BaseMaterial3D shader: #97646 (comment)
This is likely an engine bug unless I missed something obvious.
62e893c to
3384e6e
Compare
|
The test scene seems broken in the Compatibility renderer: godot.windows.editor.x86_64_HJe8TusW6S.mp4However, it only seems to happen when |
|
I don't see justification for why this feature is modifying the depth offset in the fragment function when this NDC z manipulation can be done in vertex with minimal performance overhead Writing to DEPTH is very performance heavy since it disables early-z optimization. Any option with a hidden performance cost ought to be justified and explained to the user, assuming it is even desirable in the first place.
Why? It does not write to alpha. If writing to DEPTH changes the transparency mode, that sounds like a renderer bug. In my opinion, Opaque vs transparent pipeline should be an explicit choice and not left up to seemingly opaque logic that relates to whether or not half a dozen variables are referenced in the code. |
Doing this at the vertex level would be preferable if there are no edge cases (e.g. with complex meshes or huge planes), but I don't know how it should be done. Is it just a matter of shifting
It doesn't, but the depth prepass will introduce random black pixels (or make the material fully black if you push pixels away from the camera). Using For reference, these are the random black pixels I'm referring to: I'm making sure If we can find a reliable way to do this (or use a vertex shader instead), then I can make it so the material doesn't have to go through the transparent pipeline anymore when depth offset is enabled. |
|
Sorry, I didn't read the whole conversation. Has the depth offset from visual instance been tried to solve the problem? |
I came here to say the same thing. This PR seems to be recreated the |
It's a different thing, as we are not dealing with transparency sorting issues here. (Note that the collision shape gizmo is already marked as a transparent material.) We're dealing with Z-fighting when a transparent object (the collision shape) is at the same depth as an opaque object (the visual representation of the source mesh). Sorting Offset can't do anything here, as it does not actually offset vertices or pixels in depth space. It only impacts how sorting is done relative to other transparent objects. |
Yes, this can be done. I have done this quite often to achieve larger depth offsets, often in units of meters or the like (for example, the Godot editor uses shifting of POSITION.z to draw the skeleton on top of the mesh without entirely disabling depth test). EDIT: I forgot that I was the one who said this in the first place. Yes, that is sufficient to do what the PR is doing. I am unconvinced that shifting POSITION.z belongs as a core material property: it can remain as custom vertex code on the specific materials that need it. (The rest of this comment is trying to explore the solution to the actual issue being addressed: z fighting of a collision outline against the identical mesh. If that is not the case, then you can ignore this comment and likely best to focus on writing the custom vertex shader code for the specific application rather than an engine change) However, from what I understand, what is being asked here is not adequately solved with POSITION shifting, because you want something to be at the same logical position, but drawn on top without affecting depth. In a sense, what is wanted is more akin to an additive pass a la extra lights in forward mobile. Also I just want it to be clear that I am not a core rendering developer, and my comment does not carry any weight. I am just diving in to clarify some misunderstandings and illustrate options. so there are a few things I want to check. first of all, I am a little unclear on why glDepthFunc(GL_LEQUAL) isn't sufficient to solve this (as the equality depth test is commonly used for additive lighting). If the vertex calculation is identical in both the base and the overlay material, this should work. Indeed, the comment at https://stackoverflow.com/a/68157205 actually says roughly the same thing: in principle this should work. so, for the specific case of wireframes we might need the second part of the comment, which is what the original poster is asking for: glPolygonOffset(1,1). The thing is, this seems to me a really specific case where the math hits different rounding error between two draw passes. (Another similar case I've seen is two overlay materials doing matrix math and division in a different order.) finally, there is what clayjohn asked: what about doing these outlines as a transparent overlay material which can be drawn last. In my opinion, this wouldn't solve the z fighting by itself but it would allow disabling depth write and pushing position a bit in front, or simply rendering on top. (I'm not sure if this is what clayjohn is getting at, but using a trimesh with a barycentric outline shader, possible with custom vertex data, it would be possible to draw this as an actual transparent overlay and precisely equal depth and have properly antialiased line rendering) I think it is worth asking if this usecase is niche enough to warrant a new feature, when position shifting is possible (even if inconsistent from some angles). Or, for example, maybe offset 1,1 makes sense to generally apply to line meshes without needing to expose the setting, since that is the specific issue being solved. Let me know if I missed something or I am wrong about some of these points. I just wanted to explore the options exhaustively and explain why this seems to be a real problem with a less-than-ideal workaround |
|
Sorry, my comment warrants a TLDR. Here it is. As written, using POSITION offset in vertex is too niche and, in the case of fragment DEPTH, also too unperformant. Make the collision gizmo code use a ShaderMaterial and put the shader code there. Later, if more people also want to use it, we can discuss making it a standard material feature (godot design). if you want specific behaviors of glPolygonOffset or glDepthFunc due to the combination of line and tri mesh then likely make a new pr for what is needed based on what I wrote in my comment. |
…izmo flicker This has several use cases: - Prevent collision shape gizmos that overlap with geometry from Z-fighting, which causes flickering in motion. - Draw meshes that overlap other meshes without Z-fighting, which is useful for mesh-based decals (without needing to offset them from the surface manually). - In general, allow meshes to draw in front of other meshes even if they're actually slightly behind, which can be useful for some VFX.
TODO: - Fix depth adjustment being too much when far away from the gizmo.
3384e6e to
418a493
Compare
|
@Calinou depth biasing has been in Vulkan since 1.0, so why can't we use that? It's the direct modern equivalent of VkPipelineRasterizationStateCreateInfo {
depthBiasEnable = VK_TRUE,
depthBiasConstantFactor = ...,
depthBiasClamp = ...,
depthBiasSlopeFactor = ...
} |
I wasn't aware about it 🙂 That said, does it have an equivalent in both D3D12 and Metal? What about OpenGL (specifically OpenGL ES 3.0 and WebGL 2.0)? We want this functionality to be as universal as possible. See #106330 where vendor or graphics API limitations are problematic. |
|
@Calinou Yes, and apparently the code is already inside the Godot driver initializers for all of them https://github.com/search?q=repo%3Agodotengine%2Fgodot%20depthbias&type=code I'm just glad it can help, I could use this for a couple of projects. I will add, I would like to see this available in general materials since it allows separation of testing from writing unlike writing |
|
I have a question for this. Has it been suggested to use a custom shader for the collision gizmos? |
Yes, this was mentioned above: #100211 (comment) It's not too difficult to make it use a custom shader, but we need to figure out a way to avoid code duplication as this shader is needed in at least 3 locations:
Also, it would still need to be modified to use a more efficient approach than writing to |

This has several use cases:
Testing project: test_depth_offset.zip
Preview
Debug collisions
Without depth offset
With depth offset
Debug collisions (supersampled)
2.0 render scale + 8x MSAA 3D.
Without depth offset
With depth offset
Debug collisions in motion
Without depth offset
truck_town_shapes_before.mp4
With depth offset
truck_town_shapes_after.mp4
CSG node gizmos
Without depth offset
With depth offset
Mesh-based decal
Both planes are at the same height as the box they're resting on (no offset is applied to the mesh's vertices).
Without depth offset
Z-fighting occurs and is most visible when the camera moves or rotates.
With depth offset
No more Z-fighting.