Introduce two-level bins for multidrawable meshes. (#16898)

Currently, our batchable binned items are stored in a hash table that
maps bin key, which includes the batch set key, to a list of entities.
Multidraw is handled by sorting the bin keys and accumulating adjacent
bins that can be multidrawn together (i.e. have the same batch set key)
into multidraw commands during `batch_and_prepare_binned_render_phase`.

This is reasonably efficient right now, but it will complicate future
work to retain indirect draw parameters from frame to frame. Consider
what must happen when we have retained indirect draw parameters and the
application adds a bin (i.e. a new mesh) that shares a batch set key
with some pre-existing meshes. (That is, the new mesh can be multidrawn
with the pre-existing meshes.) To be maximally efficient, our goal in
that scenario will be to update *only* the indirect draw parameters for
the batch set (i.e. multidraw command) containing the mesh that was
added, while leaving the others alone. That means that we have to
quickly locate all the bins that belong to the batch set being modified.

In the existing code, we would have to sort the list of bin keys so that
bins that can be multidrawn together become adjacent to one another in
the list. Then we would have to do a binary search through the sorted
list to find the location of the bin that was just added. Next, we would
have to widen our search to adjacent indexes that contain the same batch
set, doing expensive comparisons against the batch set key every time.
Finally, we would reallocate the indirect draw parameters and update the
stored pointers to the indirect draw parameters that the bins store.

By contrast, it'd be dramatically simpler if we simply changed the way
bins are stored to first map from batch set key (i.e. multidraw command)
to the bins (i.e. meshes) within that batch set key, and then from each
individual bin to the mesh instances. That way, the scenario above in
which we add a new mesh will be simpler to handle. First, we will look
up the batch set key corresponding to that mesh in the outer map to find
an inner map corresponding to the single multidraw command that will
draw that batch set. We will know how many meshes the multidraw command
is going to draw by the size of that inner map. Then we simply need to
reallocate the indirect draw parameters and update the pointers to those
parameters within the bins as necessary. There will be no need to do any
binary search or expensive batch set key comparison: only a single hash
lookup and an iteration over the inner map to update the pointers.

This patch implements the above technique. Because we don't have
retained bins yet, this PR provides no performance benefits. However, it
opens the door to maximally efficient updates when only a small number
of meshes change from frame to frame.

The main churn that this patch causes is that the *batch set key* (which
uniquely specifies a multidraw command) and *bin key* (which uniquely
specifies a mesh *within* that multidraw command) are now separate,
instead of the batch set key being embedded *within* the bin key.

In order to isolate potential regressions, I think that at least #16890,
#16836, and #16825 should land before this PR does.

## Migration Guide

* The *batch set key* is now separate from the *bin key* in
`BinnedPhaseItem`. The batch set key is used to collect multidrawable
meshes together. If you aren't using the multidraw feature, you can
safely set the batch set key to `()`.

Author: pcwalton

assets/shaders/storage_buffer.wgsl

@@ -4,6 +4,7 @@
 }
 
 @group(2) @binding(0) var<storage, read> colors: array<vec4<f32>, 5>;
+@group(2) @binding(1) var<uniform> color_id: u32;
 
 struct Vertex {
     @builtin(instance_index) instance_index: u32,
@@ -23,10 +24,7 @@ fn vertex(vertex: Vertex) -> VertexOutput {
     out.world_position = mesh_functions::mesh_position_local_to_world(world_from_local, vec4(vertex.position, 1.0));
     out.clip_position = position_world_to_clip(out.world_position.xyz);
 
-    // We have 5 colors in the storage buffer, but potentially many instances of the mesh, so
-    // we use the instance index to select a color from the storage buffer.
-    out.color = colors[vertex.instance_index % 5];
-
+    out.color = colors[color_id];
     return out;
 }
 

crates/bevy_core_pipeline/src/core_2d/mod.rs

@@ -33,9 +33,7 @@ pub mod graph {
 use core::ops::Range;
 
 use bevy_asset::UntypedAssetId;
-use bevy_render::{
-    batching::gpu_preprocessing::GpuPreprocessingMode, render_phase::PhaseItemBinKey,
-};
+use bevy_render::batching::gpu_preprocessing::GpuPreprocessingMode;
 use bevy_utils::HashMap;
 pub use camera_2d::*;
 pub use main_opaque_pass_2d_node::*;
@@ -127,8 +125,13 @@ impl Plugin for Core2dPlugin {
 
 /// Opaque 2D [`BinnedPhaseItem`]s.
 pub struct Opaque2d {
+    /// Determines which objects can be placed into a *batch set*.
+    ///
+    /// Objects in a single batch set can potentially be multi-drawn together,
+    /// if it's enabled and the current platform supports it.
+    pub batch_set_key: (),
     /// The key, which determines which can be batched.
-    pub key: Opaque2dBinKey,
+    pub bin_key: Opaque2dBinKey,
     /// An entity from which data will be fetched, including the mesh if
     /// applicable.
     pub representative_entity: (Entity, MainEntity),
@@ -155,14 +158,6 @@ pub struct Opaque2dBinKey {
     pub material_bind_group_id: Option<BindGroupId>,
 }
 
-impl PhaseItemBinKey for Opaque2dBinKey {
-    type BatchSetKey = ();
-
-    fn get_batch_set_key(&self) -> Option<Self::BatchSetKey> {
-        None
-    }
-}
-
 impl PhaseItem for Opaque2d {
     #[inline]
     fn entity(&self) -> Entity {
@@ -175,7 +170,7 @@ impl PhaseItem for Opaque2d {
 
     #[inline]
     fn draw_function(&self) -> DrawFunctionId {
-        self.key.draw_function
+        self.bin_key.draw_function
     }
 
     #[inline]
@@ -198,16 +193,22 @@ impl PhaseItem for Opaque2d {
 }
 
 impl BinnedPhaseItem for Opaque2d {
+    // Since 2D meshes presently can't be multidrawn, the batch set key is
+    // irrelevant.
+    type BatchSetKey = ();
+
     type BinKey = Opaque2dBinKey;
 
     fn new(
-        key: Self::BinKey,
+        batch_set_key: Self::BatchSetKey,
+        bin_key: Self::BinKey,
         representative_entity: (Entity, MainEntity),
         batch_range: Range<u32>,
         extra_index: PhaseItemExtraIndex,
     ) -> Self {
         Opaque2d {
-            key,
+            batch_set_key,
+            bin_key,
             representative_entity,
             batch_range,
             extra_index,
@@ -218,14 +219,19 @@ impl BinnedPhaseItem for Opaque2d {
 impl CachedRenderPipelinePhaseItem for Opaque2d {
     #[inline]
     fn cached_pipeline(&self) -> CachedRenderPipelineId {
-        self.key.pipeline
+        self.bin_key.pipeline
     }
 }
 
 /// Alpha mask 2D [`BinnedPhaseItem`]s.
 pub struct AlphaMask2d {
+    /// Determines which objects can be placed into a *batch set*.
+    ///
+    /// Objects in a single batch set can potentially be multi-drawn together,
+    /// if it's enabled and the current platform supports it.
+    pub batch_set_key: (),
     /// The key, which determines which can be batched.
-    pub key: AlphaMask2dBinKey,
+    pub bin_key: AlphaMask2dBinKey,
     /// An entity from which data will be fetched, including the mesh if
     /// applicable.
     pub representative_entity: (Entity, MainEntity),
@@ -265,7 +271,7 @@ impl PhaseItem for AlphaMask2d {
 
     #[inline]
     fn draw_function(&self) -> DrawFunctionId {
-        self.key.draw_function
+        self.bin_key.draw_function
     }
 
     #[inline]
@@ -288,35 +294,33 @@ impl PhaseItem for AlphaMask2d {
 }
 
 impl BinnedPhaseItem for AlphaMask2d {
+    // Since 2D meshes presently can't be multidrawn, the batch set key is
+    // irrelevant.
+    type BatchSetKey = ();
+
     type BinKey = AlphaMask2dBinKey;
 
     fn new(
-        key: Self::BinKey,
+        batch_set_key: Self::BatchSetKey,
+        bin_key: Self::BinKey,
         representative_entity: (Entity, MainEntity),
         batch_range: Range<u32>,
         extra_index: PhaseItemExtraIndex,
     ) -> Self {
         AlphaMask2d {
-            key,
+            batch_set_key,
+            bin_key,
             representative_entity,
             batch_range,
             extra_index,
         }
     }
 }
 
-impl PhaseItemBinKey for AlphaMask2dBinKey {
-    type BatchSetKey = ();
-
-    fn get_batch_set_key(&self) -> Option<Self::BatchSetKey> {
-        None
-    }
-}
-
 impl CachedRenderPipelinePhaseItem for AlphaMask2d {
     #[inline]
     fn cached_pipeline(&self) -> CachedRenderPipelineId {
-        self.key.pipeline
+        self.bin_key.pipeline
     }
 }
 

crates/bevy_core_pipeline/src/core_3d/mod.rs

@@ -68,7 +68,6 @@ use core::ops::Range;
 use bevy_render::{
     batching::gpu_preprocessing::{GpuPreprocessingMode, GpuPreprocessingSupport},
     mesh::allocator::SlabId,
-    render_phase::PhaseItemBinKey,
     view::NoIndirectDrawing,
 };
 pub use camera_3d::*;
@@ -115,8 +114,8 @@ use crate::{
     dof::DepthOfFieldNode,
     prepass::{
         node::PrepassNode, AlphaMask3dPrepass, DeferredPrepass, DepthPrepass, MotionVectorPrepass,
-        NormalPrepass, Opaque3dPrepass, OpaqueNoLightmap3dBinKey, ViewPrepassTextures,
-        MOTION_VECTOR_PREPASS_FORMAT, NORMAL_PREPASS_FORMAT,
+        NormalPrepass, Opaque3dPrepass, OpaqueNoLightmap3dBatchSetKey, OpaqueNoLightmap3dBinKey,
+        ViewPrepassTextures, MOTION_VECTOR_PREPASS_FORMAT, NORMAL_PREPASS_FORMAT,
     },
     skybox::SkyboxPlugin,
     tonemapping::TonemappingNode,
@@ -219,8 +218,13 @@ impl Plugin for Core3dPlugin {
 
 /// Opaque 3D [`BinnedPhaseItem`]s.
 pub struct Opaque3d {
+    /// Determines which objects can be placed into a *batch set*.
+    ///
+    /// Objects in a single batch set can potentially be multi-drawn together,
+    /// if it's enabled and the current platform supports it.
+    pub batch_set_key: Opaque3dBatchSetKey,
     /// The key, which determines which can be batched.
-    pub key: Opaque3dBinKey,
+    pub bin_key: Opaque3dBinKey,
     /// An entity from which data will be fetched, including the mesh if
     /// applicable.
     pub representative_entity: (Entity, MainEntity),
@@ -270,27 +274,13 @@ pub struct Opaque3dBatchSetKey {
 /// Note that a *batch set* (if multi-draw is in use) contains multiple batches.
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Opaque3dBinKey {
-    /// The key of the *batch set*.
-    ///
-    /// As batches belong to a batch set, meshes in a batch must obviously be
-    /// able to be placed in a single batch set.
-    pub batch_set_key: Opaque3dBatchSetKey,
-
     /// The asset that this phase item is associated with.
     ///
     /// Normally, this is the ID of the mesh, but for non-mesh items it might be
     /// the ID of another type of asset.
     pub asset_id: UntypedAssetId,
 }
 
-impl PhaseItemBinKey for Opaque3dBinKey {
-    type BatchSetKey = Opaque3dBatchSetKey;
-
-    fn get_batch_set_key(&self) -> Option<Self::BatchSetKey> {
-        Some(self.batch_set_key.clone())
-    }
-}
-
 impl PhaseItem for Opaque3d {
     #[inline]
     fn entity(&self) -> Entity {
@@ -304,7 +294,7 @@ impl PhaseItem for Opaque3d {
 
     #[inline]
     fn draw_function(&self) -> DrawFunctionId {
-        self.key.batch_set_key.draw_function
+        self.batch_set_key.draw_function
     }
 
     #[inline]
@@ -327,17 +317,20 @@ impl PhaseItem for Opaque3d {
 }
 
 impl BinnedPhaseItem for Opaque3d {
+    type BatchSetKey = Opaque3dBatchSetKey;
     type BinKey = Opaque3dBinKey;
 
     #[inline]
     fn new(
-        key: Self::BinKey,
+        batch_set_key: Self::BatchSetKey,
+        bin_key: Self::BinKey,
         representative_entity: (Entity, MainEntity),
         batch_range: Range<u32>,
         extra_index: PhaseItemExtraIndex,
     ) -> Self {
         Opaque3d {
-            key,
+            batch_set_key,
+            bin_key,
             representative_entity,
             batch_range,
             extra_index,
@@ -348,12 +341,18 @@ impl BinnedPhaseItem for Opaque3d {
 impl CachedRenderPipelinePhaseItem for Opaque3d {
     #[inline]
     fn cached_pipeline(&self) -> CachedRenderPipelineId {
-        self.key.batch_set_key.pipeline
+        self.batch_set_key.pipeline
     }
 }
 
 pub struct AlphaMask3d {
-    pub key: OpaqueNoLightmap3dBinKey,
+    /// Determines which objects can be placed into a *batch set*.
+    ///
+    /// Objects in a single batch set can potentially be multi-drawn together,
+    /// if it's enabled and the current platform supports it.
+    pub batch_set_key: OpaqueNoLightmap3dBatchSetKey,
+    /// The key, which determines which can be batched.
+    pub bin_key: OpaqueNoLightmap3dBinKey,
     pub representative_entity: (Entity, MainEntity),
     pub batch_range: Range<u32>,
     pub extra_index: PhaseItemExtraIndex,
@@ -371,7 +370,7 @@ impl PhaseItem for AlphaMask3d {
 
     #[inline]
     fn draw_function(&self) -> DrawFunctionId {
-        self.key.batch_set_key.draw_function
+        self.batch_set_key.draw_function
     }
 
     #[inline]
@@ -397,16 +396,19 @@ impl PhaseItem for AlphaMask3d {
 
 impl BinnedPhaseItem for AlphaMask3d {
     type BinKey = OpaqueNoLightmap3dBinKey;
+    type BatchSetKey = OpaqueNoLightmap3dBatchSetKey;
 
     #[inline]
     fn new(
-        key: Self::BinKey,
+        batch_set_key: Self::BatchSetKey,
+        bin_key: Self::BinKey,
         representative_entity: (Entity, MainEntity),
         batch_range: Range<u32>,
         extra_index: PhaseItemExtraIndex,
     ) -> Self {
         Self {
-            key,
+            batch_set_key,
+            bin_key,
             representative_entity,
             batch_range,
             extra_index,
@@ -417,7 +419,7 @@ impl BinnedPhaseItem for AlphaMask3d {
 impl CachedRenderPipelinePhaseItem for AlphaMask3d {
     #[inline]
     fn cached_pipeline(&self) -> CachedRenderPipelineId {
-        self.key.batch_set_key.pipeline
+        self.batch_set_key.pipeline
     }
 }