Skip to content

Commit

Permalink
fix shadow batching (#11645)
Browse files Browse the repository at this point in the history
# Objective

`RenderMeshInstance::material_bind_group_id` is only set from
`queue_material_meshes::<M>`. this field is used (only) for determining
batch groups, so some items may be batched incorrectly if they have
never been in the camera's view or if they don't use the Material
abstraction.

in particular, shadow views render more meshes than the main camera, and
currently batch some meshes where the object has never entered the
camera view together. this is quite hard to trigger, but should occur in
a scene with out-of-view alpha-mask materials (so that the material
instance actually affects the shadow) in the path of a light.

this is also a footgun for custom pipelines: failing to set the
material_bind_group_id will result in all meshes being batched together
and all using the closest/furthest material to the camera (depending on
sort order).

## Solution

- queue_shadows now sets the material_bind_group_id correctly
- `MeshPipeline` doesn't attempt to batch meshes if the
material_bind_group_id has not been set. custom pipelines still need to
set this field to take advantage of batching, but will at least render
correctly if it is not set
  • Loading branch information
robtfm authored Feb 14, 2024
1 parent d3c9c61 commit 73bf730
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
6 changes: 4 additions & 2 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ pub fn queue_shadows<M: Material>(
shadow_draw_functions: Res<DrawFunctions<Shadow>>,
prepass_pipeline: Res<PrepassPipeline<M>>,
render_meshes: Res<RenderAssets<Mesh>>,
render_mesh_instances: Res<RenderMeshInstances>,
mut render_mesh_instances: ResMut<RenderMeshInstances>,
render_materials: Res<RenderMaterials<M>>,
render_material_instances: Res<RenderMaterialInstances<M>>,
mut pipelines: ResMut<SpecializedMeshPipelines<PrepassPipeline<M>>>,
Expand Down Expand Up @@ -1636,7 +1636,7 @@ pub fn queue_shadows<M: Material>(
// NOTE: Lights with shadow mapping disabled will have no visible entities
// so no meshes will be queued
for entity in visible_entities.iter().copied() {
let Some(mesh_instance) = render_mesh_instances.get(&entity) else {
let Some(mesh_instance) = render_mesh_instances.get_mut(&entity) else {
continue;
};
if !mesh_instance.shadow_caster {
Expand Down Expand Up @@ -1686,6 +1686,8 @@ pub fn queue_shadows<M: Material>(
}
};

mesh_instance.material_bind_group_id = material.get_bind_group_id();

shadow_phase.add(Shadow {
draw_function: draw_shadow_mesh,
pipeline: pipeline_id,
Expand Down
8 changes: 7 additions & 1 deletion crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,12 @@ pub struct RenderMeshInstance {
pub automatic_batching: bool,
}

impl RenderMeshInstance {
pub fn should_batch(&self) -> bool {
self.automatic_batching && self.material_bind_group_id.is_some()
}
}

#[derive(Default, Resource, Deref, DerefMut)]
pub struct RenderMeshInstances(EntityHashMap<RenderMeshInstance>);

Expand Down Expand Up @@ -466,7 +472,7 @@ impl GetBatchData for MeshPipeline {
&mesh_instance.transforms,
maybe_lightmap.map(|lightmap| lightmap.uv_rect),
),
mesh_instance.automatic_batching.then_some((
mesh_instance.should_batch().then_some((
mesh_instance.material_bind_group_id,
mesh_instance.mesh_asset_id,
maybe_lightmap.map(|lightmap| lightmap.image),
Expand Down

0 comments on commit 73bf730

Please sign in to comment.