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

shadow_mesh on ArrayMesh replaces the original mesh #72071

Closed
aXu-AP opened this issue Jan 25, 2023 · 9 comments · Fixed by #94547
Closed

shadow_mesh on ArrayMesh replaces the original mesh #72071

aXu-AP opened this issue Jan 25, 2023 · 9 comments · Fixed by #94547

Comments

@aXu-AP
Copy link
Contributor

aXu-AP commented Jan 25, 2023

Godot version

v4.0.beta.custom_build [fab9926]

System information

Windows 10

Issue description

Adding shadow_mesh to ArrayMesh causes the mesh itself not to render, only shadow mesh gets rendered. If shadow_mesh has different vertex positions than the main mesh, rendering the main mesh fails on Forward+ renderer.

Check this image explanation:
shadowmesh

Steps to reproduce

Make an ArrayMesh and add shadow_mesh to it. I used following code (it rotates and offsets the shadow mesh to make the problem more visible):

@tool
extends MeshInstance3D


func _ready() -> void:
	var arr_mesh = create_mesh(0, Vector3.ZERO)
	arr_mesh.shadow_mesh = create_mesh(1, Vector3.UP * 1)
	mesh = arr_mesh


func create_mesh(rotate : float, offset : Vector3) -> ArrayMesh:
	var vertices = PackedVector3Array()
	vertices.push_back(Vector3(0, 1, 0).rotated(Vector3.UP, rotate) + offset)
	vertices.push_back(Vector3(1, 0, 0).rotated(Vector3.UP, rotate) + offset)
	vertices.push_back(Vector3(0, 0, 1).rotated(Vector3.UP, rotate) + offset)
	
	# Initialize the ArrayMesh.
	var arr_mesh = ArrayMesh.new()
	var arrays = []
	arrays.resize(Mesh.ARRAY_MAX)
	arrays[Mesh.ARRAY_VERTEX] = vertices

	# Create the Mesh.
	arr_mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, arrays)
	return arr_mesh

Minimal reproduction project

ShadowMeshBug.zip

@clayjohn
Copy link
Member

Does this happen on all the renderers?

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Jan 25, 2023

It seems to be exclusive to the Forward+ renderer. On Mobile seems to work as intended. (and Compatibility doesn't have shadows whatsoever).

@clayjohn
Copy link
Member

clayjohn commented Jan 25, 2023

Oh, I know whats going on. The mesh isn't rendering at all. The depth prepass uses the shadow mesh, but the opaque pass only draws meshes where the depth is the same as the depth from the depth prepass. Since you are using a completely different mesh, it means the object isn't getting drawn at all. But since it was drawn to the depth prepass, there is a hole where nothing has been drawn.

This is another edge case where #70214 breaks down. We need to add some logic to avoid this issue when a shadow mesh is used

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Jan 25, 2023

Ah, I see. I can confirm that it renders normally if the shadow mesh is the same as the main mesh. However, there is an use case where shadow mesh is simpler version of the actual mesh, so this should be eventually fixed.

@Calinou
Copy link
Member

Calinou commented Jan 25, 2023

We need to add some logic to avoid this issue when a shadow mesh is used

Shadow meshes are generated by default for imported 3D scenes, so this would make the optimization useless in most cases 🙁

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Jan 25, 2023

I'm not expert on this area, but if solving this requires removing optimization from more frequent cases, it isn't worth it. In that case we should document this behaviour. I'm currently writing description for shadow_mesh, which is how I encountered this bug in first place.

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Jan 25, 2023

Altough if making custom shadow meshes is not supported, exposing shadow_mesh doesn't make much sense. A safer way would be having create_shadow_mesh() function, which copies vertex and index arrays from the actual mesh in similiar manner how it's done for imported meshes.

@clayjohn
Copy link
Member

We need to add some logic to avoid this issue when a shadow mesh is used

Shadow meshes are generated by default for imported 3D scenes, so this would make the optimization useless in most cases slightly_frowning_face

Not really, the optimization is only really for alpha hash and alpha scissor materials. For regular opaque materials it doesn't make much of a difference

@clayjohn
Copy link
Member

In the end I think documenting the requirement for positions to be the same is the way to go. We discussed this at the rendering meeting yesterday and realized that any attempt to create the behaviour sought by the OP will hurt performance in a way that we don't want to support.

Further, using a different mesh for shadows than the one used for rendering will always result in uncontrollable shadow artifacts (i.e. very bad shadow acne).

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

Successfully merging a pull request may close this issue.

3 participants