Unbreak shadows by retaining work item buffers corresponding to ExtractedViews, not ViewTargets. (#17039)

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 #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 #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 #16922 solved the
issue. In fact, it didn't: 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 #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.
This commit is contained in:
Patrick Walton 2024-12-30 14:06:40 -06:00 committed by GitHub
parent 0362abd4f4
commit fde7968168
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -24,7 +24,7 @@ use crate::{
},
render_resource::{BufferVec, GpuArrayBufferable, RawBufferVec, UninitBufferVec},
renderer::{RenderAdapter, RenderDevice, RenderQueue},
view::{ExtractedView, NoIndirectDrawing, ViewTarget},
view::{ExtractedView, NoIndirectDrawing},
Render, RenderApp, RenderSet,
};
@ -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