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

Add support for depth function in spatial materials #73527

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

apples
Copy link
Contributor

@apples apples commented Feb 17, 2023

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.

image

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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to point out that for opaque materials #70214 will overwrite the compare op you just set later on.
This is currently not a problem due to #73158 (which sends custom no-depth draw materials into the alpha pass) but could be a potential problem in the future.

Copy link
Contributor Author

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.

@QbieShay
Copy link
Contributor

QbieShay commented May 8, 2023

Hey, what's the state of this?

@Calinou
Copy link
Member

Calinou commented Aug 9, 2023

Hey, what's the state of this?

This PR needs to be rebased to account for ed0c378. (I don't know if glDepthFunc() should be moved to follow glDepthMask(). In my rebase, I didn't move glDepthFunc() and it worked fine this way.)

Copy link
Member

@Calinou Calinou left a 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:

image

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.

@apples
Copy link
Contributor Author

apples commented Aug 17, 2023

Updated:

  • Rebased to latest master.
  • Squashed commits.
  • Incorporated Calinou's suggested documentation change.
  • Renamed and reordered some enum values, to better match the underlying graphics API enums, and also to be more in line with the expected changes for Expose an intuitive subset of stencil operations godot-proposals#7174.
  • Regarding the glDepthMask change from ed0c378: that change shouldn't affect this PR, so I left it all as-is.

@apples
Copy link
Contributor Author

apples commented Aug 28, 2023

Previous change broke Visual Shaders, pushed a small fix that involved moving the LESS_OR_EQUAL operator to slot 0 in all enums, since Visual Shaders assume 0 is the default value.

apples added a commit to apples/godot that referenced this pull request Sep 10, 2023
apples added a commit to apples/godot that referenced this pull request Sep 10, 2023
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
@QbieShay QbieShay modified the milestones: 4.x, 4.2 Sep 10, 2023
@clayjohn
Copy link
Member

clayjohn commented Oct 7, 2023

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. render_priority can help, but it can't sort between TAA and non-TAA passes or between opaque and transparent, so you can still get into trouble.

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.

@QbieShay
Copy link
Contributor

Similarly to the stencil PR, we need to first come up with a design for opaque multipass.

@akien-mga akien-mga modified the milestones: 4.2, 4.3 Oct 11, 2023
cridenour pushed a commit to cridenour/godot that referenced this pull request Nov 17, 2023
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
cridenour added a commit to cridenour/godot that referenced this pull request Dec 4, 2023
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
cridenour added a commit to cridenour/godot that referenced this pull request Dec 4, 2023
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
pkhead pushed a commit to pkhead/godot that referenced this pull request Jan 6, 2024
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
@akien-mga akien-mga modified the milestones: 4.3, 4.x Jun 12, 2024
cridenour added a commit to cridenour/godot that referenced this pull request Jul 5, 2024
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
apples added a commit to apples/godot that referenced this pull request Jul 20, 2024
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
RadiantUwU pushed a commit to RadiantUwU/godot that referenced this pull request Aug 15, 2024
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
RadiantUwU pushed a commit to RadiantUwU/godot that referenced this pull request Sep 16, 2024
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
RadiantUwU pushed a commit to RadiantUwU/godot that referenced this pull request Sep 30, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: On hold
Development

Successfully merging this pull request may close these issues.

Add support for depth function in spatial materials and/or shaders (GL_GREATER, …)
6 participants