-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Add support for depth function in spatial materials #73527
base: master
Are you sure you want to change the base?
Conversation
6a27928
to
db27e5a
Compare
servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp
Outdated
Show resolved
Hide resolved
default: | ||
depth_stencil_state.depth_compare_operator = RD::COMPARE_OP_LESS_OR_EQUAL; | ||
break; | ||
} | ||
} | ||
bool depth_pre_pass_enabled = bool(GLOBAL_GET("rendering/driver/depth_prepass/enable")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As things are now, it would be difficult to integrate depth functions into the opaque render pass anyways, due to potential sorting issues. I'm currently looking into solutions for this, but it might be beyond the scope of this PR.
Hey, what's the state of this? |
This PR needs to be rebased to account for ed0c378. (I don't know if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected in all 3 rendering methods.
Testing project: test_pr_73527.zip
It's possible to get a transparent x-ray effect to work with an opaque material if you use Grow with -0.001
as a value in the next pass:
However, this is a fiddly setup that is difficult to figure out and Z-fighting will occur in the distance. A custom shader that sets grow distance based on the distance to the camera might work better. (Some outline effects may benefit from a fixed size regardless of camera distance, so it could be worth considering as a core material feature.)
If this caveat is well-documented in the StandardMaterial3D class reference and manual page, this should still be a net usability upgrade overall. However, I wonder if the depth function property should be hidden for opaque materials that have Depth Draw Mode set to a value other than Never, as it appears to do nothing otherwise.
Updated:
|
Previous change broke Visual Shaders, pushed a small fix that involved moving the |
9f20505
to
9c49019
Compare
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
I think this looks good from a design standpoint, I am also concerned about the impact of #73158. So I would like to discuss making the changes described here #73158 (comment) in a future rendering meeting. I'm worried about the usability of this PR. Switching between depth draw modes can be greatly impacted by the order in which nodes are draw (i.e if I render an object with depth_draw GREATER, but it is drawn first, it just won't get drawn. We may want to hold off on this until we have the ability to run multiple opaque passes (and clear depth etc. between passes). Otherwise I fear this feature is going to feel a bit user hostile and difficult to control for anything except simple examples. |
Similarly to the stencil PR, we need to first come up with a design for opaque multipass. |
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Updated stencil features to match new design Fixed stencil opaque pass Added STENCIL_RW_WRITE_ALWAYS Added docs for BaseMaterial3D stencil operations Added mobile renderer support Renamed stencil rw modes Implemented property hiding for stencil fields Updated docs Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Update scene/resources/material.cpp Co-authored-by: RedMser <5117197+RedMser@users.noreply.github.com> Update servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Added missing cases Fixed stencil_mode autocompletion Fixed syntax highlighting of stencil_mode Added stencil support to VisualShader Added stencil support for GLES3 Changed stencil property default values in BaseMaterial3D Revert "Added stencil support for GLES3" This reverts commit c6f581b. Removed mask_uses_ref project setting Changed depth functions to better match PR godotengine#73527 Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Changed set_stencil_effect_color to pass by const ref Fixed errors
Adds "Depth Function" property to spatial materials, which controls the depth comparison operator.
Resolves godotengine/godot-proposals#1298.
Test project: https://github.com/apples/DepthFunctionTest
Seems to work fine in all three rendering modes in my simple testing, but more testing is likely necessary as this touches core rendering logic. I'm a little unsure about the GLES3 implementation, but I tried my best.
Currently only useful for alpha pass materials, due to #73158.