Skip to content

Commit

Permalink
Fix the failures:
Browse files Browse the repository at this point in the history
* The retained view key from bevyengine#16942 was insufficient to uniquely
  identify a shadow cascade when multiple cameras were present. In such
  cases, the stable ID for a shadow cascade is actually (light entity,
  camera entity, cascade index), not (light entity, cascade index) as
  the PR in bevyengine#16942 assumed. This caused failures in the
  `camera_sub_view` example.

* Sorted phase items didn't push batch sets as they were supposed to. I
  updated `batch_and_prepare_sorted_render_phase` to do so. This fixes
  the examples with transparency.

* Unbatchable binned entities didn't push batch sets as they were
  supposed to. This fixes the `morph_targets` example.

* As the `GpuPreprocessNode` now runs per camera (a necessary change for
  occlusion culling), it should only run on views associated with the
  current camera (the camera itself plus the shadow maps). It was
  running again for every view, causing failures in tests with multiple
  views like `split_screen`.

* 3D meshes need to be re-extracted if their assets change, so that the
  first vertex and first index in `MeshInputUniform` are updated. I
  added a system to do so. Note that this system is somewhat inefficient
  when meshes change; once `cold-specialization` lands it can be updated
  to use the asset change infrastructure in that patch to fix the issue.
  This fixes the `query_gltf_primitives` example.

* `specialized_mesh_pipeline` wasn't allocating indirect work items. I
  changed the example to do so.
  • Loading branch information
pcwalton committed Jan 14, 2025
1 parent e614842 commit 4a2bed2
Show file tree
Hide file tree
Showing 12 changed files with 265 additions and 47 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ pub fn extract_core_2d_camera_phases(
}

// This is the main 2D camera, so we use the first subview index (0).
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), 0);
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0);

transparent_2d_phases.insert_or_clear(retained_view_entity);
opaque_2d_phases.insert_or_clear(retained_view_entity, GpuPreprocessingMode::None);
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ pub fn extract_core_3d_camera_phases(
});

// This is the main 3D camera, so use the first subview index (0).
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), 0);
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0);

opaque_3d_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode);
alpha_mask_3d_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode);
Expand Down Expand Up @@ -685,7 +685,7 @@ pub fn extract_camera_prepass_phase(
});

// This is the main 3D camera, so we use the first subview index (0).
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), 0);
let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0);

if depth_prepass || normal_prepass || motion_vector_prepass {
opaque_3d_prepass_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode);
Expand Down
30 changes: 24 additions & 6 deletions crates/bevy_pbr/src/render/gpu_preprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ use crate::{
graph::NodePbr, MeshCullingData, MeshCullingDataBuffer, MeshInputUniform, MeshUniform,
};

use super::ViewLightEntities;

/// The handle to the `mesh_preprocess.wgsl` compute shader.
pub const MESH_PREPROCESS_SHADER_HANDLE: Handle<Shader> =
Handle::weak_from_u128(16991728318640779533);
Expand Down Expand Up @@ -89,6 +91,7 @@ pub struct GpuPreprocessNode {
),
Without<SkipGpuPreprocess>,
>,
main_view_query: QueryState<Read<ViewLightEntities>>,
}

/// The render node for the indirect parameter building pass.
Expand Down Expand Up @@ -298,18 +301,20 @@ impl FromWorld for GpuPreprocessNode {
fn from_world(world: &mut World) -> Self {
Self {
view_query: QueryState::new(world),
main_view_query: QueryState::new(world),
}
}
}

impl Node for GpuPreprocessNode {
fn update(&mut self, world: &mut World) {
self.view_query.update_archetypes(world);
self.main_view_query.update_archetypes(world);
}

fn run<'w>(
&self,
_: &mut RenderGraphContext,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext<'w>,
world: &'w World,
) -> Result<(), NodeRunError> {
Expand All @@ -330,10 +335,23 @@ impl Node for GpuPreprocessNode {
timestamp_writes: None,
});

// Run the compute passes.
for (view, bind_groups, view_uniform_offset, no_indirect_drawing) in
self.view_query.iter_manual(world)
let mut all_views: SmallVec<[_; 8]> = SmallVec::new();
all_views.push(graph.view_entity());
if let Ok(shadow_cascade_views) =
self.main_view_query.get_manual(world, graph.view_entity())
{
all_views.extend(shadow_cascade_views.lights.iter().copied());
}

// Run the compute passes.

for view_entity in all_views {
let Ok((view, bind_groups, view_uniform_offset, no_indirect_drawing)) =
self.view_query.get_manual(world, view_entity)
else {
continue;
};

// Grab the work item buffers for this view.
let Some(view_work_item_buffers) = index_buffers.get(&view) else {
warn!("The preprocessing index buffer wasn't present");
Expand All @@ -351,14 +369,14 @@ impl Node for GpuPreprocessNode {
// Fetch the pipeline.
let Some(preprocess_pipeline_id) = maybe_pipeline_id else {
warn!("The build mesh uniforms pipeline wasn't ready");
return Ok(());
continue;
};

let Some(preprocess_pipeline) =
pipeline_cache.get_compute_pipeline(preprocess_pipeline_id)
else {
// This will happen while the pipeline is being compiled and is fine.
return Ok(());
continue;
};

compute_pass.set_pipeline(preprocess_pipeline);
Expand Down
37 changes: 28 additions & 9 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,18 @@ pub struct ViewShadowBindings {
pub directional_light_depth_texture_view: TextureView,
}

/// A component that holds the shadow cascade views for all shadow cascades
/// associated with a camera.
///
/// Note: Despite the name, this component actually holds the shadow cascade
/// views, not the lights themselves.
#[derive(Component)]
pub struct ViewLightEntities {
/// The shadow cascade views for all shadow cascades associated with a
/// camera.
///
/// Note: Despite the name, this component actually holds the shadow cascade
/// views, not the lights themselves.
pub lights: Vec<Entity>,
}

Expand Down Expand Up @@ -701,6 +711,7 @@ pub fn prepare_lights(
views: Query<
(
Entity,
MainEntity,
&ExtractedView,
&ExtractedClusterConfig,
Option<&RenderLayers>,
Expand Down Expand Up @@ -1115,10 +1126,11 @@ pub fn prepare_lights(
let mut live_views = EntityHashSet::with_capacity(views_count);

// set up light data for each view
for (entity, extracted_view, clusters, maybe_layers, no_indirect_drawing) in sorted_cameras
.0
.iter()
.filter_map(|sorted_camera| views.get(sorted_camera.entity).ok())
for (entity, camera_main_entity, extracted_view, clusters, maybe_layers, no_indirect_drawing) in
sorted_cameras
.0
.iter()
.filter_map(|sorted_camera| views.get(sorted_camera.entity).ok())
{
live_views.insert(entity);
let mut view_lights = Vec::new();
Expand Down Expand Up @@ -1229,8 +1241,11 @@ pub fn prepare_lights(
})
.clone();

let retained_view_entity =
RetainedViewEntity::new(*light_main_entity, face_index as u32);
let retained_view_entity = RetainedViewEntity::new(
*light_main_entity,
Some(camera_main_entity.into()),
face_index as u32,
);

commands.entity(view_light_entity).insert((
ShadowView {
Expand Down Expand Up @@ -1334,7 +1349,8 @@ pub fn prepare_lights(

let view_light_entity = light_view_entities[0];

let retained_view_entity = RetainedViewEntity::new(*light_main_entity, 0);
let retained_view_entity =
RetainedViewEntity::new(*light_main_entity, Some(camera_main_entity.into()), 0);

commands.entity(view_light_entity).insert((
ShadowView {
Expand Down Expand Up @@ -1467,8 +1483,11 @@ pub fn prepare_lights(
frustum.half_spaces[4] =
HalfSpace::new(frustum.half_spaces[4].normal().extend(f32::INFINITY));

let retained_view_entity =
RetainedViewEntity::new(*light_main_entity, cascade_index as u32);
let retained_view_entity = RetainedViewEntity::new(
*light_main_entity,
Some(camera_main_entity.into()),
cascade_index as u32,
);

commands.entity(view_light_entity).insert((
ShadowView {
Expand Down
8 changes: 5 additions & 3 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2756,7 +2756,8 @@ impl<P: PhaseItem> RenderCommand<P> for DrawMesh {
indirect_parameters_buffer.indexed_batch_sets_buffer(),
) else {
warn!(
"Not rendering mesh because indirect parameters buffer wasn't present"
"Not rendering mesh because indexed indirect parameters buffer \
wasn't present",
);
return RenderCommandResult::Skip;
};
Expand Down Expand Up @@ -2806,13 +2807,14 @@ impl<P: PhaseItem> RenderCommand<P> for DrawMesh {
} => {
// Look up the indirect parameters buffer, as well as the
// buffer we're going to use for
// `multi_draw_indexed_indirect_count` (if available).
// `multi_draw_indirect_count` (if available).
let (Some(indirect_parameters_buffer), Some(batch_sets_buffer)) = (
indirect_parameters_buffer.non_indexed_data_buffer(),
indirect_parameters_buffer.non_indexed_batch_sets_buffer(),
) else {
warn!(
"Not rendering mesh because indirect parameters buffer wasn't present"
"Not rendering mesh because non-indexed indirect parameters buffer \
wasn't present"
);
return RenderCommandResult::Skip;
};
Expand Down
39 changes: 31 additions & 8 deletions crates/bevy_render/src/batching/gpu_preprocessing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl PreprocessWorkItemBuffers {
///
/// `no_indirect_drawing` specifies whether we're drawing directly or
/// indirectly.
fn new(no_indirect_drawing: bool) -> Self {
pub fn new(no_indirect_drawing: bool) -> Self {
if no_indirect_drawing {
PreprocessWorkItemBuffers::Direct(BufferVec::new(BufferUsages::STORAGE))
} else {
Expand All @@ -311,7 +311,7 @@ impl PreprocessWorkItemBuffers {
///
/// `indexed` specifies whether the work item corresponds to an indexed
/// mesh.
fn push(&mut self, indexed: bool, preprocess_work_item: PreprocessWorkItem) {
pub fn push(&mut self, indexed: bool, preprocess_work_item: PreprocessWorkItem) {
match *self {
PreprocessWorkItemBuffers::Direct(ref mut buffer) => {
buffer.push(preprocess_work_item);
Expand Down Expand Up @@ -633,7 +633,7 @@ impl IndirectParametersBuffers {
///
/// The `indexed` parameter specifies whether the meshes that these batches
/// correspond to are indexed or not.
fn allocate(&mut self, indexed: bool, count: u32) -> u32 {
pub fn allocate(&mut self, indexed: bool, count: u32) -> u32 {
if indexed {
self.allocate_indexed(count)
} else {
Expand Down Expand Up @@ -819,6 +819,9 @@ where
/// The index of the first instance in this batch in the instance buffer.
instance_start_index: u32,

/// True if the mesh in question has an index buffer; false otherwise.
indexed: bool,

/// The index of the indirect parameters for this batch in the
/// [`IndirectParametersBuffers`].
///
Expand All @@ -841,15 +844,24 @@ where
///
/// `instance_end_index` is the index of the last instance in this batch
/// plus one.
fn flush<I>(self, instance_end_index: u32, phase: &mut SortedRenderPhase<I>)
where
fn flush<I>(
self,
instance_end_index: u32,
phase: &mut SortedRenderPhase<I>,
indirect_parameters_buffers: &mut IndirectParametersBuffers,
) where
I: CachedRenderPipelinePhaseItem + SortedPhaseItem,
{
let (batch_range, batch_extra_index) =
phase.items[self.phase_item_start_index as usize].batch_range_and_extra_index_mut();
*batch_range = self.instance_start_index..instance_end_index;
*batch_extra_index =
PhaseItemExtraIndex::maybe_indirect_parameters_index(self.indirect_parameters_index);

if let Some(indirect_parameters_index) = self.indirect_parameters_index {
indirect_parameters_buffers
.add_batch_set(self.indexed, indirect_parameters_index.into());
}
}
}

Expand Down Expand Up @@ -943,7 +955,11 @@ pub fn batch_and_prepare_sorted_render_phase<I, GFBD>(
let Some((current_input_index, current_meta)) = current_batch_input_index else {
// Break a batch if we need to.
if let Some(batch) = batch.take() {
batch.flush(data_buffer.len() as u32, phase);
batch.flush(
data_buffer.len() as u32,
phase,
&mut indirect_parameters_buffers,
);
}

continue;
Expand All @@ -968,7 +984,7 @@ pub fn batch_and_prepare_sorted_render_phase<I, GFBD>(
if !can_batch {
// Break a batch if we need to.
if let Some(batch) = batch.take() {
batch.flush(output_index, phase);
batch.flush(output_index, phase, &mut indirect_parameters_buffers);
}

let indirect_parameters_index = if no_indirect_drawing {
Expand All @@ -994,6 +1010,7 @@ pub fn batch_and_prepare_sorted_render_phase<I, GFBD>(
batch = Some(SortedRenderBatch {
phase_item_start_index: current_index as u32,
instance_start_index: output_index,
indexed: item_is_indexed,
indirect_parameters_index: indirect_parameters_index.and_then(NonMaxU32::new),
meta: current_meta,
});
Expand Down Expand Up @@ -1024,7 +1041,11 @@ pub fn batch_and_prepare_sorted_render_phase<I, GFBD>(

// Flush the final batch if necessary.
if let Some(batch) = batch.take() {
batch.flush(data_buffer.len() as u32, phase);
batch.flush(
data_buffer.len() as u32,
phase,
&mut indirect_parameters_buffers,
);
}
}
}
Expand Down Expand Up @@ -1337,6 +1358,8 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
batch_set_index: None,
},
});
indirect_parameters_buffers
.add_batch_set(key.0.indexed(), *indirect_parameters_index);
*indirect_parameters_index += 1;
} else {
work_item_buffer.push(
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ pub fn extract_cameras(
hdr: camera.hdr,
},
ExtractedView {
retained_view_entity: RetainedViewEntity::new(main_entity.into(), 0),
retained_view_entity: RetainedViewEntity::new(main_entity.into(), None, 0),
clip_from_view: camera.clip_from_view(),
world_from_view: *transform,
clip_from_world: None,
Expand Down
37 changes: 35 additions & 2 deletions crates/bevy_render/src/mesh/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ use crate::{
mesh::Mesh,
view::{self, Visibility, VisibilityClass},
};
use bevy_asset::{AssetId, Handle};
use bevy_asset::{AssetEvent, AssetId, Handle};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{component::Component, prelude::require, reflect::ReflectComponent};
use bevy_ecs::{
change_detection::DetectChangesMut, component::Component, event::EventReader, prelude::require,
reflect::ReflectComponent, system::Query,
};
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_transform::components::Transform;
use bevy_utils::{FixedHasher, HashSet};
use derive_more::derive::From;

/// A component for 2D meshes. Requires a [`MeshMaterial2d`] to be rendered, commonly using a [`ColorMaterial`].
Expand Down Expand Up @@ -101,3 +105,32 @@ impl From<&Mesh3d> for AssetId<Mesh> {
mesh.id()
}
}

/// A system that marks a [`Mesh3d`] as changed if the associated [`Mesh`] asset
/// has changed.
///
/// This is needed because the systems that extract meshes, such as
/// `extract_meshes_for_gpu_building`, write some metadata about the mesh (like
/// the location within each slab) into the GPU structures that they build that
/// needs to be kept up to date if the contents of the mesh change.
pub fn mark_3d_meshes_as_changed_if_their_assets_changed(
mut meshes_3d: Query<&mut Mesh3d>,
mut mesh_asset_events: EventReader<AssetEvent<Mesh>>,
) {
let mut changed_meshes: HashSet<AssetId<Mesh>, FixedHasher> = HashSet::default();
for mesh_asset_event in mesh_asset_events.read() {
if let AssetEvent::Modified { id } = mesh_asset_event {
changed_meshes.insert(*id);
}
}

if changed_meshes.is_empty() {
return;
}

for mut mesh_3d in &mut meshes_3d {
if changed_meshes.contains(&mesh_3d.0.id()) {
mesh_3d.set_changed();
}
}
}
Loading

0 comments on commit 4a2bed2

Please sign in to comment.