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 note to shadow_mesh docs to warn about improper usage #94547

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #72071

Using a different mesh for shadow_mesh breaks rendering right now because the shadow mesh is used for the depth prepass.

If we don't use it for the depth prepass, we will hurt performance. Further, the rendering will still appear broken as we will get all kinds of self-shadowing artifacts. Therefore, we just shouldn't support using a different shaped mesh for shadows. Users should use the old workflow which is to have a child MeshInstance3D with SHADOW_CASTING_SETTING_SHADOWS_ONLY and have the source mesh use SHADOW_CASTING_SETTING_OFF. Doing so has no additional cost over using shadow_mesh, but gives the user full control over how the shadow is drawn.

@clayjohn clayjohn added enhancement documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jul 19, 2024
@clayjohn clayjohn added this to the 4.3 milestone Jul 19, 2024
@clayjohn clayjohn requested a review from a team as a code owner July 19, 2024 20:13
@Calinou
Copy link
Member

Calinou commented Jul 19, 2024

I guess this also invalidates godotengine/godot-proposals#2600 as a proposal? It seemed to work from my quick testing, but I didn't test on more complex meshes.

That is, unless the proposal would allow performing this automatically:

Users should use the old workflow which is to have a child MeshInstance3D with SHADOW_CASTING_SETTING_SHADOWS_ONLY and have the source mesh use SHADOW_CASTING_SETTING_OFF. Doing so has no additional cost over using shadow_mesh, but gives the user full control over how the shadow is drawn.

Performance gains can be significant with 4-split PSSM (or in scenes with lots of point lights with shadows enabled) if you have lots of complex meshes.

@clayjohn
Copy link
Member Author

I guess this also invalidates godotengine/godot-proposals#2600 as a proposal? It seemed to work from my quick testing, but I didn't test on more complex meshes.

That is, unless the proposal would allow performing this automatically:

Users should use the old workflow which is to have a child MeshInstance3D with SHADOW_CASTING_SETTING_SHADOWS_ONLY and have the source mesh use SHADOW_CASTING_SETTING_OFF. Doing so has no additional cost over using shadow_mesh, but gives the user full control over how the shadow is drawn.

Performance gains can be significant with 4-split PSSM (or in scenes with lots of point lights with shadows enabled) if you have lots of complex meshes.

Using nearby LODs won't be as bad as a completely different mesh for shadows (like in the OP issue). I think godotengine/godot-proposals#2600 would be better off with an LOD bias for shadows specifically. Remember, when used in the depth prepass, using a different mesh (even a different LOD of the same mesh) will totally break rendering. However, using a lower LOD for shadows only maybe be feasible.

@akien-mga akien-mga changed the title Add note to shadow_mesh docs to warn about improper usage Add note to shadow_mesh docs to warn about improper usage Jul 19, 2024
@akien-mga akien-mga merged commit e3b8525 into godotengine:master Jul 20, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shadow_mesh on ArrayMesh replaces the original mesh
3 participants