Make shadows respect render layers #18747
Conversation
Fix bevyengine#18000: Shadows do not respect render layers Shadows of objects are still rendered even when camera is rendering a different render layer. This happens because `queue_shadows` does not use the camera render layers when filtering visible entities. To fix it, i just exported the Mesh render layers and filtered them according to the camera render layers.
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
alice-i-cecile
changed the title
alice-i-cecile
added
C-Bug
alice-i-cecile
added
the
S-Needs-Review
Filter was removing shadows of objects without any layers. Changed code to just filter if mesh has RenderLayers assigned. Signed-off-by: Adrian Graur <adrian.graur@tecnico.ulisboa.pt>
Signed-off-by: Adrian Graur <adrian.graur@tecnico.ulisboa.pt>
Signed-off-by: Adrian Graur <adrian.graur@tecnico.ulisboa.pt>
|
Fixed the visual difference, the original fix was filtering shadows by layers even when layers were not assigned. I'm sorry for the multiple commits but i wasn't able to run the CI tests locally for some reason. Ready to be reviewed when possible. |
crates/bevy_pbr/src/render/light.rs
Outdated
| ) -> RenderVisibleMeshEntities { | ||
| RenderVisibleMeshEntities { | ||
| entities: visible_entities | ||
| .iter() | ||
| .map(|e| { | ||
| let render_entity = mapper.get(*e).unwrap_or(Entity::PLACEHOLDER); | ||
| (render_entity, MainEntity::from(*e)) | ||
|
|
||
| // Extract RenderLayers from the mesh entity |
There was a problem hiding this comment.
This is a very hot path and I'm not sure it makes sense to store this information here, particularly because it will duplicate the the mesh instance render layers for every view light in which the entity appears, which will increase memory and hurt locality. I'm not 100% sure that filtering in the queue method is correct, but if we are going to do so, we should probably store the mesh render layers on RenderMeshInstanceShared, which is currently looked up in the queue system anyway and will ensure that they're only stored once per instance.
alice-i-cecile
added
S-Needs-Design
Signed-off-by: Adrian Graur <adrian.graur@tecnico.ulisboa.pt>
Signed-off-by: Adrian Graur <adrian.graur@tecnico.ulisboa.pt>
|
I changed the render layers location to |
| } | ||
| } | ||
|
|
||
| // Skip the entity if it's cached in a bin and up to date. |
There was a problem hiding this comment.
Did you move this on purpose?
| let mesh_layers = mesh_instance.shared.render_layers.as_ref(); | ||
|
|
||
| // Skip the entity if it's not in the camera render layers. | ||
| if let (Some(camera_layers), Some(layers)) = (camera_layers, mesh_layers) { |
There was a problem hiding this comment.
Typically the way we do this is to call unwrap_or_default on each of the respective layers, since a non-specified layer counts as layer 0. See check_point_light_mesh_visibility for example.
Fix #18000: Shadows do not respect render layers
Shadows of objects are still rendered even when camera is rendering a different render layer.
Objective
Solution
RenderLayersof Mesh from the world.queue_shadows(), add filter that checks ifRenderLayersof the Mesh match the current rendered layer of theCamera.Testing
Showcase
ShowCase.mp4