Skip to content

Commit

Permalink
Unbreak shadows by retaining work item buffers corresponding to
Browse files Browse the repository at this point in the history
`ExtractedView`s, not `ViewTarget`s.

OK, so this is tricky. Every frame, `delete_old_work_item_buffers`
deletes the mesh preprocessing index buffers (a.k.a. work item buffers)
for views that don't have `ViewTarget`s. This was always wrong for
shadow map views, as shadow maps only have `ExtractedView` components,
not `ViewTarget`s. However, before bevyengine#16836, the problem was masked,
because uploading the mesh preprocessing index buffers for shadow views
had already completed by the time `delete_old_work_item_buffers` ran.
But PR bevyengine#16836 moved `delete_old_work_item_buffers` from the
`ManageViews` phase to `PrepareResources`, which runs before
`write_batched_instance_buffers` uploads the work item buffers to the
GPU.

This itself isn't wrong, but it exposed the bug, because now it's
possible for work item buffers to get deleted before they're uploaded in
`write_batched_instance_buffers`. This is actually intermittent! It's
possible for the old work item buffers to get deleted, and then
*recreated* in `batch_and_prepare_binned_render_phase`, which runs
during `PrepareResources` as well, and under that system ordering, there
will be no problem other than a little inefficiency arising from
recreating the buffers every frame. But, if
`delete_old_work_item_buffers` runs *after*
`batch_and_prepare_render_phase`, then the work item buffers
corresponding to shadow views will get deleted, and then the shadows
will disappear.

The fact that this is racy is what made it look like bevyengine#16922 solved the
issue. In fact, it didn't solve the issue: it just perturbed the
ordering on the build bots enough that the issue stopped appearing.
However, on my system, the shadows still don't appear with bevyengine#16922.

This commit solves the problem by making `delete_old_work_item_buffers`
look at `ExtractedView`s, not `ViewTarget`s, preventing work item
buffers corresponding to live shadow map views from being deleted.
  • Loading branch information
pcwalton committed Dec 30, 2024
1 parent 0362abd commit 479bb1e
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions crates/bevy_render/src/batching/gpu_preprocessing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,22 +489,22 @@ pub fn clear_batched_gpu_instance_buffers<GFBD>(
}

/// A system that removes GPU preprocessing work item buffers that correspond to
/// deleted [`ViewTarget`]s.
/// deleted [`ExtractedView`]s.
///
/// This is a separate system from [`clear_batched_gpu_instance_buffers`]
/// because [`ViewTarget`]s aren't created until after the extraction phase is
/// completed.
/// because [`ExtractedView`]s aren't created until after the extraction phase
/// is completed.
pub fn delete_old_work_item_buffers<GFBD>(
mut gpu_batched_instance_buffers: ResMut<
BatchedInstanceBuffers<GFBD::BufferData, GFBD::BufferInputData>,
>,
view_targets: Query<Entity, With<ViewTarget>>,
extracted_views: Query<Entity, With<ExtractedView>>,
) where
GFBD: GetFullBatchData,
{
gpu_batched_instance_buffers
.work_item_buffers
.retain(|entity, _| view_targets.contains(*entity));
.retain(|entity, _| extracted_views.contains(*entity));
}

/// Batch the items in a sorted render phase, when GPU instance buffer building
Expand Down

0 comments on commit 479bb1e

Please sign in to comment.