rewrite batcher to be fully generic

crates/bevy_asset/src/lib.rs

@@ -81,6 +81,12 @@ pub struct HandleUntyped {
     pub type_id: TypeId,
 }
 
+impl HandleUntyped {
+    pub fn is_handle<T: 'static>(untyped: &HandleUntyped) -> bool {
+        TypeId::of::<T>() == untyped.type_id
+    }
+}
+
 impl<T> From<Handle<T>> for HandleUntyped
 where
     T: 'static,

crates/bevy_pbr/src/lib.rs

@@ -20,15 +20,10 @@ pub struct PbrPlugin;
 // NOTE: this isn't PBR yet. consider this name "aspirational" :)
 impl AppPlugin for PbrPlugin {
     fn build(&self, app: &mut AppBuilder) {
-        // asset_batchers.batch_types2::<Mesh, StandardMaterial>();
         app.add_resource(AssetStorage::<StandardMaterial>::new())
             .add_system_to_stage(
                 stage::POST_UPDATE,
                 shader::asset_handle_shader_def_system::<StandardMaterial>.system(),
-            )
-            .add_system_to_stage(
-                stage::POST_UPDATE,
-                shader::asset_handle_batcher_system::<StandardMaterial>(),
             );
         let resources = app.resources();
         let mut render_graph = resources.get_mut::<RenderGraph>().unwrap();

crates/bevy_render/Cargo.toml

@@ -27,6 +27,7 @@ uuid = { version = "0.8", features = ["v4", "serde"] }
 glam = "0.8.6"
 zerocopy = "0.3"
 bitflags = "1.0"
+smallvec = "1.4.0"
 # TODO: replace once_cell with std equivalent if/when this lands: https://github.com/rust-lang/rfcs/pull/2788
 once_cell = "1.3.1"
 downcast-rs = "1.1.1"

crates/bevy_render/src/batch/asset_batcher.rs

@@ -1,269 +0,0 @@
-use super::{AssetSetBatcher2, AssetSetBatcherKey2, Batch, BatchKey2};
-use bevy_asset::{Handle, HandleId};
-use legion::prelude::Entity;
-use std::{any::TypeId, collections::HashMap};
-
-pub trait AssetBatcher {
-    fn set_entity_handle(&mut self, entity: Entity, handle_type: TypeId, handle_id: HandleId);
-    fn get_batch2(&self, key: &BatchKey2) -> Option<&Batch>;
-    // TODO: add pipeline handle here
-    fn get_batches2(&self) -> std::collections::hash_map::Iter<'_, BatchKey2, Batch>;
-    fn get_batches<'a>(&'a self) -> Box<dyn Iterator<Item = &Batch> + 'a>;
-    fn get_batches_mut<'a>(&'a mut self) -> Box<dyn Iterator<Item = &mut Batch> + 'a>;
-}
-
-#[derive(Default)]
-pub struct AssetBatchers {
-    asset_batchers: Vec<Box<dyn AssetBatcher + Send + Sync>>,
-    asset_batcher_indices2: HashMap<AssetSetBatcherKey2, usize>,
-    handle_batchers: HashMap<TypeId, Vec<usize>>,
-}
-
-impl AssetBatchers {
-    pub fn set_entity_handle<T>(&mut self, entity: Entity, handle: Handle<T>)
-    where
-        T: 'static,
-    {
-        let handle_type = TypeId::of::<T>();
-        if let Some(batcher_indices) = self.handle_batchers.get(&handle_type) {
-            for index in batcher_indices.iter() {
-                self.asset_batchers[*index].set_entity_handle(entity, handle_type, handle.id);
-            }
-        }
-    }
-
-    pub fn batch_types2<T1, T2>(&mut self)
-    where
-        T1: 'static,
-        T2: 'static,
-    {
-        let key = AssetSetBatcherKey2 {
-            handle1_type: TypeId::of::<T1>(),
-            handle2_type: TypeId::of::<T2>(),
-        };
-
-        self.asset_batchers
-            .push(Box::new(AssetSetBatcher2::new(key.clone())));
-
-        let index = self.asset_batchers.len() - 1;
-
-        let handle1_batchers = self
-            .handle_batchers
-            .entry(key.handle1_type.clone())
-            .or_insert_with(|| Vec::new());
-        handle1_batchers.push(index);
-
-        let handle2_batchers = self
-            .handle_batchers
-            .entry(key.handle2_type.clone())
-            .or_insert_with(|| Vec::new());
-        handle2_batchers.push(index);
-
-        self.asset_batcher_indices2.insert(key, index);
-    }
-
-    pub fn get_batches2<T1, T2>(
-        &self,
-    ) -> Option<std::collections::hash_map::Iter<'_, BatchKey2, Batch>>
-    where
-        T1: 'static,
-        T2: 'static,
-    {
-        let key = AssetSetBatcherKey2 {
-            handle1_type: TypeId::of::<T1>(),
-            handle2_type: TypeId::of::<T2>(),
-        };
-
-        if let Some(index) = self.asset_batcher_indices2.get(&key) {
-            Some(self.asset_batchers[*index].get_batches2())
-        } else {
-            None
-        }
-    }
-
-    pub fn get_batch2<T1, T2>(&self, handle1: Handle<T1>, handle2: Handle<T2>) -> Option<&Batch>
-    where
-        T1: 'static,
-        T2: 'static,
-    {
-        let key = AssetSetBatcherKey2 {
-            handle1_type: TypeId::of::<T1>(),
-            handle2_type: TypeId::of::<T2>(),
-        };
-
-        let batch_key = BatchKey2 {
-            handle1: handle1.id,
-            handle2: handle2.id,
-        };
-
-        if let Some(index) = self.asset_batcher_indices2.get(&key) {
-            self.asset_batchers[*index].get_batch2(&batch_key)
-        } else {
-            None
-        }
-    }
-
-    pub fn get_batches(&self) -> impl Iterator<Item = &Batch> {
-        self.asset_batchers
-            .iter()
-            .map(|a| a.get_batches())
-            .flatten()
-    }
-
-    pub fn get_batches_mut(&mut self) -> impl Iterator<Item = &mut Batch> {
-        self.asset_batchers
-            .iter_mut()
-            .map(|a| a.get_batches_mut())
-            .flatten()
-    }
-
-    pub fn get_handle_batches<T>(&self) -> Option<impl Iterator<Item = &Batch>>
-    where
-        T: 'static,
-    {
-        let handle_type = TypeId::of::<T>();
-        if let Some(batcher_indices) = self.handle_batchers.get(&handle_type) {
-            Some(
-                // NOTE: it would be great to use batcher_indices.iter().map(|i| self.asset_batchers[*i].get_batches()) here
-                // but unfortunately the lifetimes don't work out for some reason
-                self.asset_batchers
-                    .iter()
-                    .enumerate()
-                    .filter(move |(index, _a)| batcher_indices.contains(index))
-                    .map(|(_index, a)| a.get_batches())
-                    .flatten(),
-            )
-        } else {
-            None
-        }
-    }
-
-    pub fn get_handle_batches_mut<T>(&mut self) -> Option<impl Iterator<Item = &mut Batch>>
-    where
-        T: 'static,
-    {
-        let handle_type = TypeId::of::<T>();
-        if let Some(batcher_indices) = self.handle_batchers.get(&handle_type) {
-            Some(
-                self.asset_batchers
-                    .iter_mut()
-                    .enumerate()
-                    .filter(move |(index, _a)| batcher_indices.contains(index))
-                    .map(|(_index, a)| a.get_batches_mut())
-                    .flatten(),
-            )
-        } else {
-            None
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use legion::prelude::*;
-    struct A;
-    struct B;
-    struct C;
-
-    #[test]
-    fn test_batching() {
-        let mut asset_batchers = AssetBatchers::default();
-        asset_batchers.batch_types2::<A, B>();
-
-        let mut world = World::new();
-        let a1: Handle<A> = Handle::new(1);
-        let b1: Handle<B> = Handle::new(1);
-        let c1: Handle<C> = Handle::new(1);
-
-        let a2: Handle<A> = Handle::new(2);
-        let b2: Handle<B> = Handle::new(2);
-
-        let entities = world.insert((), (0..3).map(|_| ()));
-        asset_batchers.set_entity_handle(entities[0], a1);
-        // batch is empty when Handle<B> is missing
-        assert_eq!(asset_batchers.get_batch2(a1, b1), None);
-        asset_batchers.set_entity_handle(entities[0], b1);
-        // entity[0] is added to batch when it has both Handle<A> and Handle<B>
-        let mut expected_batch = Batch {
-            handles: vec![a1.into(), b1.into()],
-            ..Default::default()
-        };
-        expected_batch.add_entity(entities[0]);
-        let actual_batch = asset_batchers.get_batch2(a1, b1).unwrap();
-        copy_ignored_fields(actual_batch, &mut expected_batch);
-        assert_eq!(actual_batch, &expected_batch);
-        asset_batchers.set_entity_handle(entities[0], c1);
-
-        asset_batchers.set_entity_handle(entities[1], a1);
-        asset_batchers.set_entity_handle(entities[1], b1);
-
-        // all entities with Handle<A> and Handle<B> are returned
-        let mut expected_batch = Batch {
-            handles: vec![a1.into(), b1.into()],
-            ..Default::default()
-        };
-        expected_batch.add_entity(entities[0]);
-        expected_batch.add_entity(entities[1]);
-        let actual_batch = asset_batchers.get_batch2(a1, b1).unwrap();
-        copy_ignored_fields(actual_batch, &mut expected_batch);
-        assert_eq!(actual_batch, &expected_batch);
-
-        // uncreated batches are empty
-        assert_eq!(asset_batchers.get_batch2(a1, c1), None);
-
-        // batch iteration works
-        asset_batchers.set_entity_handle(entities[2], a2);
-        asset_batchers.set_entity_handle(entities[2], b2);
-
-        let mut batches = asset_batchers
-            .get_batches2::<A, B>()
-            .unwrap()
-            .collect::<Vec<(&BatchKey2, &Batch)>>();
-
-        batches.sort_by(|a, b| a.0.cmp(b.0));
-        let mut expected_batch1 = Batch {
-            handles: vec![a1.into(), b1.into()],
-            ..Default::default()
-        };
-        expected_batch1.add_entity(entities[0]);
-        expected_batch1.add_entity(entities[1]);
-        let mut expected_batch2 = Batch {
-            handles: vec![a2.into(), b2.into()],
-            ..Default::default()
-        };
-        expected_batch2.add_entity(entities[2]);
-        let mut expected_batches = vec![
-            (
-                BatchKey2 {
-                    handle1: a1.id,
-                    handle2: b1.id,
-                },
-                expected_batch1,
-            ),
-            (
-                BatchKey2 {
-                    handle1: a2.id,
-                    handle2: b2.id,
-                },
-                expected_batch2,
-            ),
-        ];
-        expected_batches.sort_by(|a, b| a.0.cmp(&b.0));
-        // copy ignored fields
-        batches.iter().zip(expected_batches.iter_mut()).for_each(
-            |((_, ref actual), (_, ref mut expected))| copy_ignored_fields(actual, expected),
-        );
-        assert_eq!(
-            batches,
-            expected_batches
-                .iter()
-                .map(|(a, b)| (a, b))
-                .collect::<Vec<(&BatchKey2, &Batch)>>()
-        );
-    }
-
-    fn copy_ignored_fields(source: &Batch, destination: &mut Batch) {
-        destination.render_resource_assignments.id = source.render_resource_assignments.id;
-    }
-}

crates/bevy_render/src/batch/asset_batcher2.rs

@@ -1,144 +0,0 @@
-use super::{AssetBatcher, Batch};
-use bevy_asset::{HandleId, HandleUntyped};
-use legion::prelude::Entity;
-use std::{any::TypeId, collections::HashMap, hash::Hash};
-
-// TODO: if/when const generics land, revisit this design in favor of generic array lengths
-
-// TODO: add sorting by primary / secondary handle to reduce rebinds of data
-
-#[derive(Hash, Eq, PartialEq, Debug, Ord, PartialOrd)]
-pub struct BatchKey2 {
-    pub handle1: HandleId,
-    pub handle2: HandleId,
-}
-
-#[derive(Hash, Eq, PartialEq, Clone, Debug)]
-pub struct AssetSetBatcherKey2 {
-    pub handle1_type: TypeId,
-    pub handle2_type: TypeId,
-}
-
-struct EntitySetState2 {
-    handle1: Option<HandleId>,
-    handle2: Option<HandleId>,
-}
-
-impl EntitySetState2 {
-    fn is_full(&self) -> bool {
-        self.handle1.is_some() && self.handle2.is_some()
-    }
-}
-
-pub struct AssetSetBatcher2 {
-    key: AssetSetBatcherKey2,
-    set_batches: HashMap<BatchKey2, Batch>,
-    entity_set_states: HashMap<Entity, EntitySetState2>,
-}
-
-impl AssetSetBatcher2 {
-    pub fn new(key: AssetSetBatcherKey2) -> Self {
-        AssetSetBatcher2 {
-            key,
-            set_batches: HashMap::new(),
-            entity_set_states: HashMap::new(),
-        }
-    }
-
-    pub fn add_entity_to_set(&mut self, entity: Entity) {
-        // these unwraps are safe because this function is only called from set_entity_handle on a "full" state
-        let state = self.entity_set_states.get(&entity).unwrap();
-        let key = BatchKey2 {
-            handle1: state.handle1.unwrap(),
-            handle2: state.handle2.unwrap(),
-        };
-
-        match self.set_batches.get_mut(&key) {
-            Some(batch) => {
-                batch.add_entity(entity);
-            }
-            None => {
-                let mut batch = Batch::default();
-
-                batch.handles.push(HandleUntyped {
-                    id: key.handle1,
-                    type_id: self.key.handle1_type,
-                });
-                batch.handles.push(HandleUntyped {
-                    id: key.handle2,
-                    type_id: self.key.handle2_type,
-                });
-
-                batch.add_entity(entity);
-                self.set_batches.insert(key, batch);
-            }
-        }
-    }
-
-    pub fn set_entity_handle1(&mut self, entity: Entity, handle_id: HandleId) {
-        match self.entity_set_states.get_mut(&entity) {
-            None => {
-                // TODO: when generalizing to set size 1, ensure you treat set as "full" here
-                self.entity_set_states.insert(
-                    entity,
-                    EntitySetState2 {
-                        handle1: Some(handle_id),
-                        handle2: None,
-                    },
-                );
-            }
-            Some(state) => {
-                state.handle1 = Some(handle_id);
-                if state.is_full() {
-                    self.add_entity_to_set(entity);
-                }
-            }
-        }
-    }
-
-    pub fn set_entity_handle2(&mut self, entity: Entity, handle_id: HandleId) {
-        match self.entity_set_states.get_mut(&entity) {
-            None => {
-                // TODO: when generalizing to set size 1, ensure you treat set as "full" here
-                self.entity_set_states.insert(
-                    entity,
-                    EntitySetState2 {
-                        handle1: None,
-                        handle2: Some(handle_id),
-                    },
-                );
-            }
-            Some(state) => {
-                state.handle2 = Some(handle_id);
-                if state.is_full() {
-                    self.add_entity_to_set(entity);
-                }
-            }
-        }
-    }
-}
-
-impl AssetBatcher for AssetSetBatcher2 {
-    fn set_entity_handle(&mut self, entity: Entity, handle_type: TypeId, handle_id: HandleId) {
-        if handle_type == self.key.handle1_type {
-            self.set_entity_handle1(entity, handle_id);
-        } else if handle_type == self.key.handle2_type {
-            self.set_entity_handle2(entity, handle_id);
-        }
-    }
-    fn get_batch2(&self, key: &BatchKey2) -> Option<&Batch> {
-        self.set_batches.get(key)
-    }
-
-    fn get_batches2(&self) -> std::collections::hash_map::Iter<'_, BatchKey2, Batch> {
-        self.set_batches.iter()
-    }
-
-    fn get_batches<'a>(&'a self) -> Box<dyn Iterator<Item = &'a Batch> + 'a> {
-        Box::new(self.set_batches.values())
-    }
-
-    fn get_batches_mut<'a>(&'a mut self) -> Box<dyn Iterator<Item = &'a mut Batch> + 'a> {
-        Box::new(self.set_batches.values_mut())
-    }
-}

crates/bevy_render/src/batch/batch.rs

@@ -1,38 +1,36 @@
-use crate::render_resource::RenderResourceAssignments;
+use super::{BatchKey, Key};
-use bevy_asset::{Handle, HandleUntyped};
-use legion::prelude::Entity;
-use std::collections::{HashMap, HashSet};
 
-#[derive(PartialEq, Eq, Debug, Default)]
+#[derive(Debug, Eq, PartialEq)]
-pub struct Batch {
+pub struct Batch<TKey, TValue, TData>
-    pub handles: Vec<HandleUntyped>,
+where
-    pub entities: HashSet<Entity>,
+    TKey: Key,
-    pub instanced_entity_indices: HashMap<Entity, usize>,
+{
-    pub current_instanced_entity_index: usize,
+    pub batch_key: BatchKey<TKey>,
-    pub render_resource_assignments: RenderResourceAssignments,
+    pub values: Vec<TValue>,
+    pub data: TData,
 }
 
-impl Batch {
+impl<TKey, TValue, TData> Batch<TKey, TValue, TData>
-    pub fn add_entity(&mut self, entity: Entity) {
+where
-        self.entities.insert(entity);
+    TKey: Key,
-    }
+{
-
+    pub fn new(batch_key: BatchKey<TKey>, data: TData) -> Self {
-    pub fn add_instanced_entity(&mut self, entity: Entity) {
+        Batch {
-        if let None = self.instanced_entity_indices.get(&entity) {
+            data,
-            self.instanced_entity_indices
+            values: Vec::new(),
-                .insert(entity, self.current_instanced_entity_index);
+            batch_key,
-            self.current_instanced_entity_index += 1;
         }
     }
 
-    pub fn get_handle<T>(&self) -> Option<Handle<T>>
+    pub fn add(&mut self, value: TValue) {
-    where
+        self.values.push(value);
-        T: 'static,
+    }
-    {
+
-        self.handles
+    pub fn iter(&self) -> impl Iterator<Item = &TValue> {
-            .iter()
+        self.values.iter()
-            .map(|h| Handle::from_untyped(*h))
+    }
-            .find(|h| h.is_some())
+
-            .map(|h| h.unwrap())
+    pub fn get_key(&self, index: usize) -> Option<&TKey> {
+        self.batch_key.0.get(index)
     }
 }

crates/bevy_render/src/batch/batcher.rs

@@ -0,0 +1,262 @@
+use super::Batch;
+use smallvec::{smallvec, SmallVec};
+use std::{borrow::Cow, collections::HashMap, hash::Hash};
+
+// TODO: add sorting by primary / secondary handle to reduce rebinds of data
+
+// TValue: entityid
+// TKey: handleuntyped
+
+pub trait Key: Clone + Eq + Hash + 'static {}
+impl<T: Clone + Eq + Hash + 'static> Key for T {}
+
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+pub struct BatchKey<TKey: Key>(pub Cow<'static, SmallVec<[TKey; 2]>>);
+
+impl<TKey: Key> BatchKey<TKey> {
+    pub fn key1(key: TKey) -> Self {
+        BatchKey(Cow::Owned(smallvec![key]))
+    }
+
+    pub fn key2(key1: TKey, key2: TKey) -> Self {
+        BatchKey(Cow::Owned(smallvec![key1, key2]))
+    }
+
+    pub fn key3(key1: TKey, key2: TKey, key3: TKey) -> Self {
+        BatchKey(Cow::Owned(smallvec![key1, key2, key3]))
+    }
+}
+
+pub struct BatcherKeyState<TKey: Key> {
+    batch_key: Option<BatchKey<TKey>>,
+    keys: SmallVec<[Option<TKey>; 2]>,
+}
+
+impl<TKey: Key> BatcherKeyState<TKey> {
+    pub fn new(size: usize) -> Self {
+        BatcherKeyState {
+            keys: smallvec![None; size],
+            batch_key: None,
+        }
+    }
+
+    pub fn set(&mut self, index: usize, key: TKey) {
+        self.keys[index] = Some(key);
+    }
+
+    pub fn finish(&mut self) -> Option<BatchKey<TKey>> {
+        let finished = self.keys.iter().filter(|x| x.is_some()).count() == self.keys.len();
+        if finished {
+            let batch_key = BatchKey(Cow::Owned(
+                self.keys
+                    .drain(..)
+                    .map(|k| k.unwrap())
+                    .collect::<SmallVec<[TKey; 2]>>(),
+            ));
+            self.batch_key = Some(batch_key);
+            self.batch_key.clone()
+        } else {
+            None
+        }
+    }
+}
+
+/// An unordered batcher intended to support an arbitrary number of keys of the same type (but with some distinguishing factor)
+/// NOTE: this may or may not be useful for anything. when paired with a higher-level "BatcherSet" it would allow updating batches
+// per-key (ex: material, mesh) with no global knowledge of the number of batch types (ex: (Mesh), (Material, Mesh)) that key belongs
+// to. The downside is that it is completely unordered, so it probably isn't useful for front->back or back->front rendering. But
+// _maybe_ for gpu instancing?
+pub struct Batcher<TKey, TValue, TData>
+where
+    TKey: Key,
+{
+    pub batches: HashMap<BatchKey<TKey>, Batch<TKey, TValue, TData>>,
+    pub is_index: Vec<fn(&TKey) -> bool>,
+    pub key_states: HashMap<TValue, BatcherKeyState<TKey>>,
+    pub key_count: usize,
+}
+
+impl<TKey, TValue, TData> Batcher<TKey, TValue, TData>
+where
+    TKey: Key,
+    TValue: Clone + Eq + Hash,
+    TData: Default,
+{
+    pub fn new(is_index: Vec<fn(&TKey) -> bool>) -> Self {
+        Batcher {
+            batches: HashMap::default(),
+            key_states: HashMap::default(),
+            key_count: is_index.len(),
+            is_index,
+        }
+    }
+
+    pub fn get_batch(&self, batch_key: &BatchKey<TKey>) -> Option<&Batch<TKey, TValue, TData>> {
+        self.batches.get(batch_key)
+    }
+
+    pub fn get_batch_mut(
+        &mut self,
+        batch_key: &BatchKey<TKey>,
+    ) -> Option<&mut Batch<TKey, TValue, TData>> {
+        self.batches.get_mut(batch_key)
+    }
+
+    pub fn add(&mut self, key: TKey, value: TValue) -> bool {
+        let batch_key = {
+            let key_count = self.key_count;
+            let key_state = self
+                .key_states
+                .entry(value.clone())
+                .or_insert_with(|| BatcherKeyState::new(key_count));
+
+            // if all key states are set, the value is already in the batch
+            if key_state.batch_key.is_some() {
+                // TODO: if weights are ever added, make sure to get the batch and set the weight here
+                return true;
+            }
+
+            let key_index = self
+                .is_index
+                .iter()
+                .enumerate()
+                .find(|(_i, is_index)| is_index(&key))
+                .map(|(i, _)| i);
+            if let Some(key_index) = key_index {
+                key_state.set(key_index, key.clone());
+                key_state.finish()
+            } else {
+                return false;
+            }
+        };
+
+        if let Some(batch_key) = batch_key {
+            let batch = self
+                .batches
+                .entry(batch_key.clone())
+                .or_insert_with(|| Batch::new(batch_key, TData::default()));
+
+            batch.add(value);
+        }
+
+        return true;
+    }
+
+    pub fn iter(&self) -> impl Iterator<Item = &Batch<TKey, TValue, TData>> {
+        self.batches.values()
+    }
+
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut Batch<TKey, TValue, TData>> {
+        self.batches.values_mut()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{Batch, BatchKey, Batcher};
+    use bevy_asset::{Handle, HandleUntyped};
+
+    #[derive(Debug, Eq, PartialEq)]
+    struct A;
+    #[derive(Debug, Eq, PartialEq)]
+    struct B;
+    #[derive(Debug, Eq, PartialEq)]
+    struct C;
+    #[derive(Debug, Eq, PartialEq, Default)]
+    struct Data;
+    #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+    struct Entity(usize);
+    #[test]
+    fn test_batcher_2() {
+        let mut batcher: Batcher<HandleUntyped, Entity, Data> = Batcher::new(vec![
+            HandleUntyped::is_handle::<A>,
+            HandleUntyped::is_handle::<B>,
+        ]);
+
+        let e1 = Entity(1);
+        let e2 = Entity(2);
+        let e3 = Entity(3);
+
+        let a1: HandleUntyped = Handle::<A>::new(1).into();
+        let b1: HandleUntyped = Handle::<B>::new(1).into();
+        let c1: HandleUntyped = Handle::<C>::new(1).into();
+
+        let a2: HandleUntyped = Handle::<A>::new(2).into();
+        let b2: HandleUntyped = Handle::<B>::new(2).into();
+
+        let a1_b1 = BatchKey::key2(a1, b1);
+        let a2_b2 = BatchKey::key2(a2, b2);
+
+        assert_eq!(
+            batcher.get_batch(&a1_b1),
+            None,
+            "a1_b1 batch should not exist yet"
+        );
+        batcher.add(a1, e1);
+        assert_eq!(
+            batcher.get_batch(&a1_b1),
+            None,
+            "a1_b1 batch should not exist yet"
+        );
+        batcher.add(b1, e1);
+
+        let a1_b1_batch = Batch {
+            batch_key: a1_b1.clone(),
+            values: vec![e1],
+            data: Data,
+        };
+
+        assert_eq!(
+            batcher.get_batch(&a1_b1),
+            Some(&a1_b1_batch),
+            "a1_b1 batch should exist"
+        );
+
+        assert_eq!(
+            batcher.get_batch(&a2_b2),
+            None,
+            "a2_b2 batch should not exist yet"
+        );
+        batcher.add(a2, e2);
+        assert_eq!(
+            batcher.get_batch(&a2_b2),
+            None,
+            "a2_b2 batch should not exist yet"
+        );
+        batcher.add(b2, e2);
+
+        let expected_batch = Batch {
+            batch_key: a2_b2.clone(),
+            values: vec![e2],
+            data: Data,
+        };
+
+        assert_eq!(
+            batcher.get_batch(&a2_b2),
+            Some(&expected_batch),
+            "a2_b2 batch should have e2"
+        );
+
+        batcher.add(a2, e3);
+        batcher.add(b2, e3);
+        batcher.add(c1, e3); // this should be ignored
+        let a2_b2_batch = Batch {
+            batch_key: a2_b2.clone(),
+            values: vec![e2, e3],
+            data: Data,
+        };
+
+        assert_eq!(
+            batcher.get_batch(&a2_b2),
+            Some(&a2_b2_batch),
+            "a2_b2 batch should have e2 and e3"
+        );
+
+        assert_eq!(
+            batcher
+                .iter()
+                .collect::<Vec<&Batch<HandleUntyped, Entity, Data>>>(),
+            vec![&a1_b1_batch, &a2_b2_batch]
+        )
+    }
+}

crates/bevy_render/src/batch/mod.rs

@@ -1,7 +1,9 @@
-mod asset_batcher;
+// mod asset_batcher;
-mod asset_batcher2;
+// mod asset_batcher2;
 mod batch;
+mod batcher;
 
-pub use asset_batcher::*;
+// pub use asset_batcher::*;
-pub use asset_batcher2::*;
+// pub use asset_batcher2::*;
 pub use batch::*;
+pub use batcher::*;

crates/bevy_render/src/draw_target/draw_targets/assigned_batches_draw_target.rs

@@ -1,120 +0,0 @@
-use crate::{
-    batch::AssetBatchers,
-    draw_target::DrawTarget,
-    pass::RenderPass,
-    pipeline::PipelineDescriptor,
-    render_resource::{resource_name, RenderResourceAssignments},
-    renderer::RenderContext,
-    Renderable,
-};
-use bevy_asset::Handle;
-use legion::prelude::*;
-
-#[derive(Default)]
-pub struct AssignedBatchesDrawTarget;
-
-impl DrawTarget for AssignedBatchesDrawTarget {
-    fn draw(
-        &self,
-        world: &World,
-        resources: &Resources,
-        render_pass: &mut dyn RenderPass,
-        pipeline_handle: Handle<PipelineDescriptor>,
-        pipeline_descriptor: &PipelineDescriptor,
-    ) {
-        log::trace!("drawing batches for pipeline {:?}", pipeline_handle);
-        let asset_batches = resources.get::<AssetBatchers>().unwrap();
-        let global_render_resource_assignments =
-            resources.get::<RenderResourceAssignments>().unwrap();
-        render_pass.set_render_resources(pipeline_descriptor, &global_render_resource_assignments);
-        for batch in asset_batches.get_batches() {
-            let indices = render_pass
-                .set_render_resources(pipeline_descriptor, &batch.render_resource_assignments);
-            log::trace!("drawing batch {:?}", batch.render_resource_assignments.id);
-            log::trace!("{:#?}", batch);
-            for batched_entity in batch.entities.iter() {
-                let renderable = world.get_component::<Renderable>(*batched_entity).unwrap();
-                if !renderable.is_visible {
-                    continue;
-                }
-
-                log::trace!("start drawing batched entity: {:?}", batched_entity);
-                log::trace!("{:#?}", renderable.render_resource_assignments);
-                let entity_indices = render_pass.set_render_resources(
-                    pipeline_descriptor,
-                    &renderable.render_resource_assignments,
-                );
-                let mut draw_indices = &indices;
-                if entity_indices.is_some() {
-                    if indices.is_some() {
-                        // panic!("entities overriding their batch's vertex buffer is not currently supported");
-                        log::trace!("using batch vertex indices");
-                        draw_indices = &entity_indices;
-                    } else {
-                        log::trace!("using entity vertex indices");
-                        draw_indices = &entity_indices;
-                    }
-                }
-
-                if draw_indices.is_none() {
-                    continue;
-                }
-
-                render_pass.draw_indexed(draw_indices.as_ref().unwrap().clone(), 0, 0..1);
-                log::trace!("finish drawing batched entity: {:?}", batched_entity);
-            }
-        }
-    }
-
-    fn setup(
-        &mut self,
-        world: &World,
-        resources: &Resources,
-        render_context: &mut dyn RenderContext,
-        pipeline_handle: Handle<PipelineDescriptor>,
-        pipeline_descriptor: &PipelineDescriptor,
-    ) {
-        let asset_batches = resources.get::<AssetBatchers>().unwrap();
-
-        let global_render_resource_assignments =
-            resources.get_mut::<RenderResourceAssignments>().unwrap();
-
-        log::trace!(
-            "setting up batch bind groups for pipeline: {:?} {:?}",
-            pipeline_handle,
-            pipeline_descriptor.name,
-        );
-        log::trace!("setting up global bind groups");
-        render_context.setup_bind_groups(pipeline_descriptor, &global_render_resource_assignments);
-
-        for batch in asset_batches.get_batches() {
-            log::trace!(
-                "setting up batch bind groups: {:?}",
-                batch.render_resource_assignments.id
-            );
-            log::trace!("{:#?}", batch);
-            render_context
-                .setup_bind_groups(pipeline_descriptor, &batch.render_resource_assignments);
-            for batched_entity in batch.entities.iter() {
-                let renderable = world.get_component::<Renderable>(*batched_entity).unwrap();
-                if !renderable.is_visible || renderable.is_instanced {
-                    continue;
-                }
-
-                log::trace!(
-                    "setting up entity bind group {:?} for batch {:?}",
-                    batched_entity,
-                    batch.render_resource_assignments.id
-                );
-                render_context.setup_bind_groups(
-                    pipeline_descriptor,
-                    &renderable.render_resource_assignments,
-                );
-            }
-        }
-    }
-
-    fn get_name(&self) -> String {
-        resource_name::draw_target::ASSIGNED_BATCHES.to_string()
-    }
-}

crates/bevy_render/src/draw_target/draw_targets/mod.rs

@@ -1,9 +1,7 @@
-mod assigned_batches_draw_target;
 mod assigned_meshes_draw_target;
 mod meshes_draw_target;
 mod ui_draw_target;
 
-pub use assigned_batches_draw_target::*;
 pub use assigned_meshes_draw_target::*;
 pub use meshes_draw_target::*;
 pub use ui_draw_target::*;

crates/bevy_render/src/lib.rs

@@ -38,7 +38,6 @@ use self::{
     texture::Texture,
 };
 
-use batch::AssetBatchers;
 use bevy_app::{stage, AppBuilder, AppPlugin};
 use bevy_asset::AssetStorage;
 use mesh::mesh_resource_provider_system;
@@ -67,12 +66,10 @@ impl AppPlugin for RenderPlugin {
             .add_resource(RenderResourceAssignments::default())
             .add_resource(VertexBufferDescriptors::default())
             .add_resource(EntityRenderResourceAssignments::default())
-            .add_resource(AssetBatchers::default())
             // core systems
             .add_system(entity_render_resource_assignments_system())
             .init_system_to_stage(stage::POST_UPDATE, camera::camera_update_system)
             .add_system_to_stage(stage::POST_UPDATE, mesh::mesh_specializer_system())
-            .add_system_to_stage(stage::POST_UPDATE, mesh::mesh_batcher_system())
             // render resource provider systems
             .init_system_to_stage(RENDER_RESOURCE_STAGE, mesh_resource_provider_system);
     }

crates/bevy_render/src/mesh.rs

@@ -1,5 +1,4 @@
 use crate::{
-    batch::AssetBatchers,
     pipeline::{
         state_descriptors::{IndexFormat, PrimitiveTopology},
         VertexBufferDescriptor, VertexBufferDescriptors, VertexFormat,
@@ -311,19 +310,6 @@ pub mod shape {
     }
 }
 
-pub fn mesh_batcher_system() -> Box<dyn Schedulable> {
-    SystemBuilder::new("mesh_batcher")
-        .write_resource::<AssetBatchers>()
-        .with_query(
-            <(Read<Handle<Mesh>>, Read<Renderable>)>::query().filter(changed::<Handle<Mesh>>()),
-        )
-        .build(|_, world, asset_batchers, query| {
-            for (entity, (mesh_handle, _renderable)) in query.iter_entities(world) {
-                asset_batchers.set_entity_handle(entity, *mesh_handle);
-            }
-        })
-}
-
 pub fn mesh_specializer_system() -> Box<dyn Schedulable> {
     SystemBuilder::new("mesh_specializer")
         .read_resource::<AssetStorage<Mesh>>()
@@ -396,24 +382,10 @@ pub fn mesh_resource_provider_system(resources: &mut Resources) -> Box<dyn Sched
     SystemBuilder::new("mesh_resource_provider")
         .read_resource::<GlobalRenderResourceContext>()
         .read_resource::<AssetStorage<Mesh>>()
-        .write_resource::<AssetBatchers>()
         .with_query(<(Read<Handle<Mesh>>, Write<Renderable>)>::query())
         .build(
-            move |_, world, (render_resource_context, meshes, asset_batchers), query| {
+            move |_, world, (render_resource_context, meshes,/* asset_batchers*/), query| {
                 let render_resources = &*render_resource_context.context;
-                if let Some(batches) = asset_batchers.get_handle_batches_mut::<Mesh>() {
-                    for batch in batches {
-                        let handle = batch.get_handle::<Mesh>().unwrap();
-                        setup_mesh_resource(
-                            render_resources,
-                            &mut batch.render_resource_assignments,
-                            &vertex_buffer_descriptor,
-                            handle,
-                            &meshes,
-                        );
-                    }
-                }
-
                 // TODO: remove this once batches are pipeline specific and deprecate assigned_meshes draw target
                 for (handle, mut renderable) in query.iter_mut(world) {
                     setup_mesh_resource(

crates/bevy_render/src/shader/uniform.rs

@@ -1,5 +1,4 @@
 use crate::{
-    batch::AssetBatchers,
     color::ColorSource,
     pipeline::{BindType, VertexBufferDescriptor},
     texture::Texture,
@@ -56,27 +55,6 @@ pub fn asset_handle_shader_def_system<T>(
     }
 }
 
-pub fn asset_handle_batcher_system<T>() -> Box<dyn Schedulable>
-where
-    T: AsUniforms + Send + Sync + 'static,
-{
-    SystemBuilder::new(format!(
-        "asset_handle_batcher::{}",
-        std::any::type_name::<T>()
-    ))
-    .write_resource::<AssetBatchers>()
-    .with_query(<(Read<Handle<T>>, Read<Renderable>)>::query())
-    .build(|_, world, asset_batchers, query| {
-        for (entity, (uniform_handle, renderable)) in query.iter_entities(world) {
-            if !renderable.is_visible || renderable.is_instanced {
-                continue;
-            }
-
-            asset_batchers.set_entity_handle(entity, *uniform_handle);
-        }
-    })
-}
-
 pub trait ShaderDefSuffixProvider {
     fn get_shader_def(&self) -> Option<&'static str>;
 }

src/prelude.rs

@@ -13,7 +13,6 @@ pub use crate::diagnostic::DiagnosticsPlugin;
 pub use crate::pbr::{entity::*, light::Light, material::StandardMaterial};
 #[cfg(feature = "render")]
 pub use crate::render::{
-    batch::AssetBatchers,
     draw_target,
     entity::*,
     mesh::{shape, Mesh},