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

Only disable depth writing in opaque pipelines #71124

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Jan 9, 2023

Follow up to #70214 and #70884

Fixes: #71249

While working on something else I noticed that there was a regression in master that caused objects with visibility fade alpha to not display unless the visibility was set to 0.0 and the fade was at fully opaque. If any alpha was introduced the object would disappear.

The issue was caused by the depth-prepass optimization added in #70214 which makes it so that objects only draw where their depth matches the depth-prepass depth and it disables writing to the depth during the draw pass. This optimization hugely reduces the cost of drawing opaque geometry. However, it was tricky to apply to the transparent pass. In the end with #70884 we restricted the optimization to just objects that we know will get sent down the opaque pipeline (opaque objects transparent objects with alpha scissor or alpha hash). Even with the restriction we were needlessly applying the optimizations to both the opaque pipeline and the transparent pipeline (if a material passed our checks the object using our material should be sent down the opaque pipeline, not the transparent pipeline, so there is no reason to apply the optimization to the transparent pipeline).

Visibility fade introduces a case where an otherwise opaque object may get sent down the transparent pipeline. In which case, we need the transparent pipeline version to be compiled and ready to be used (i.e. without the optimization). In the end, I realized that we never need to add the optimization to the transparent pipeline as the rendering logic checks for alpha scissor etc. and sends those materials down the opaque pipeline anyway.

before:

Screenshot from 2023-01-09 09-47-28

after:

Screenshot from 2023-01-09 09-46-34

CC @Ansraer

This restores the behaviour of the visibility fade
@Ansraer
Copy link
Contributor

Ansraer commented Jan 10, 2023

I disn't even know we had this visibility fade option, thanks fir catching this.
Sounds like a reasonable change, but I think I had a reason why I placed that check up there initially. Of course, now I can't remember what it was. Let me quickly test this PR with my benchmark scene.

@BastiaanOlij
Copy link
Contributor

I haven't dived deeply into this but it makes sense to me

@akien-mga akien-mga merged commit 201673e into godotengine:master Jan 11, 2023
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[4.0 beta11] GeometryInstance3D transparency parameter hide mesh with value over 0.0
4 participants