Skip to content

Commit 7cd2270

Browse files
authored
Use boxed storage for material erasure. (#20614)
# Objective #19667 added a `bytemuck::Pod` type constraint on `AsBindGroup::Data` which, while technically a reasonable constraint to expect from material keys, imposed a breaking change on our users which may be confusing / difficult for them to work around. It's not technically invalid to store arbitrary types in a material key, just probably bad practice. Additionally, the performance concerns here were probably overstated: 1. cold specialization makes re-specialization less likely. 2. we can mitigate going through the vtable unnecessarily by pre-computing our hash which should still make the boxed storage fast in almost all cases where we don't get a collision. ## Solution Use `Box<dyn Any>` for erasure.
1 parent c90b53a commit 7cd2270

File tree

9 files changed

+110
-31
lines changed

9 files changed

+110
-31
lines changed

crates/bevy_pbr/src/extended_material.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ where
147147
}
148148
}
149149

150-
#[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, PartialEq, Eq, Hash)]
150+
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
151151
#[repr(C, packed)]
152152
pub struct MaterialExtensionBindGroupData<B, E> {
153153
pub base: B,

crates/bevy_pbr/src/material.rs

Lines changed: 91 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ use bevy_render::{mesh::allocator::MeshAllocator, sync_world::MainEntityHashMap}
5757
use bevy_render::{texture::FallbackImage, view::RenderVisibleEntities};
5858
use bevy_shader::{Shader, ShaderDefVal};
5959
use bevy_utils::Parallel;
60-
use core::any::TypeId;
60+
use core::any::{Any, TypeId};
61+
use core::hash::{BuildHasher, Hasher};
6162
use core::{hash::Hash, marker::PhantomData};
6263
use smallvec::SmallVec;
6364
use tracing::error;
@@ -432,7 +433,7 @@ pub struct MaterialPipelineKey<M: Material> {
432433
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
433434
pub struct ErasedMaterialPipelineKey {
434435
pub mesh_key: MeshPipelineKey,
435-
pub material_key: SmallVec<[u8; 8]>,
436+
pub material_key: ErasedMaterialKey,
436437
pub type_id: TypeId,
437438
}
438439

@@ -1290,6 +1291,85 @@ pub struct DeferredDrawFunction;
12901291
#[derive(DrawFunctionLabel, Debug, Hash, PartialEq, Eq, Clone, Default)]
12911292
pub struct ShadowsDrawFunction;
12921293

1294+
#[derive(Debug)]
1295+
pub struct ErasedMaterialKey {
1296+
type_id: TypeId,
1297+
hash: u64,
1298+
value: Box<dyn Any + Send + Sync>,
1299+
vtable: Arc<ErasedMaterialKeyVTable>,
1300+
}
1301+
1302+
#[derive(Debug)]
1303+
pub struct ErasedMaterialKeyVTable {
1304+
clone_fn: fn(&dyn Any) -> Box<dyn Any + Send + Sync>,
1305+
partial_eq_fn: fn(&dyn Any, &dyn Any) -> bool,
1306+
}
1307+
1308+
impl ErasedMaterialKey {
1309+
pub fn new<T>(material_key: T) -> Self
1310+
where
1311+
T: Clone + Hash + PartialEq + Send + Sync + 'static,
1312+
{
1313+
let type_id = TypeId::of::<T>();
1314+
let hash = FixedHasher::hash_one(&FixedHasher, &material_key);
1315+
1316+
fn clone<T: Clone + Send + Sync + 'static>(any: &dyn Any) -> Box<dyn Any + Send + Sync> {
1317+
Box::new(any.downcast_ref::<T>().unwrap().clone())
1318+
}
1319+
fn partial_eq<T: PartialEq + 'static>(a: &dyn Any, b: &dyn Any) -> bool {
1320+
a.downcast_ref::<T>().unwrap() == b.downcast_ref::<T>().unwrap()
1321+
}
1322+
1323+
Self {
1324+
type_id,
1325+
hash,
1326+
value: Box::new(material_key),
1327+
vtable: Arc::new(ErasedMaterialKeyVTable {
1328+
clone_fn: clone::<T>,
1329+
partial_eq_fn: partial_eq::<T>,
1330+
}),
1331+
}
1332+
}
1333+
1334+
pub fn to_key<T: Clone + 'static>(&self) -> T {
1335+
debug_assert_eq!(self.type_id, TypeId::of::<T>());
1336+
self.value.downcast_ref::<T>().unwrap().clone()
1337+
}
1338+
}
1339+
1340+
impl PartialEq for ErasedMaterialKey {
1341+
fn eq(&self, other: &Self) -> bool {
1342+
self.type_id == other.type_id
1343+
&& (self.vtable.partial_eq_fn)(self.value.as_ref(), other.value.as_ref())
1344+
}
1345+
}
1346+
1347+
impl Eq for ErasedMaterialKey {}
1348+
1349+
impl Clone for ErasedMaterialKey {
1350+
fn clone(&self) -> Self {
1351+
Self {
1352+
type_id: self.type_id,
1353+
hash: self.hash,
1354+
value: (self.vtable.clone_fn)(self.value.as_ref()),
1355+
vtable: self.vtable.clone(),
1356+
}
1357+
}
1358+
}
1359+
1360+
impl Hash for ErasedMaterialKey {
1361+
fn hash<H: Hasher>(&self, state: &mut H) {
1362+
self.type_id.hash(state);
1363+
self.hash.hash(state);
1364+
}
1365+
}
1366+
1367+
impl Default for ErasedMaterialKey {
1368+
fn default() -> Self {
1369+
Self::new(())
1370+
}
1371+
}
1372+
12931373
/// Common [`Material`] properties, calculated for a specific material instance.
12941374
#[derive(Default)]
12951375
pub struct MaterialProperties {
@@ -1332,7 +1412,7 @@ pub struct MaterialProperties {
13321412
>,
13331413
/// The key for this material, typically a bitfield of flags that are used to modify
13341414
/// the pipeline descriptor used for this material.
1335-
pub material_key: SmallVec<[u8; 8]>,
1415+
pub material_key: ErasedMaterialKey,
13361416
/// Whether shadows are enabled for this material
13371417
pub shadows_enabled: bool,
13381418
/// Whether prepass is enabled for this material
@@ -1395,7 +1475,7 @@ pub struct PreparedMaterial {
13951475
// orphan rules T_T
13961476
impl<M: Material> ErasedRenderAsset for MeshMaterial3d<M>
13971477
where
1398-
M::Data: Clone,
1478+
M::Data: PartialEq + Eq + Hash + Clone,
13991479
{
14001480
type SourceAsset = M;
14011481
type ErasedAsset = PreparedMaterial;
@@ -1556,21 +1636,24 @@ where
15561636

15571637
let bindless = material_uses_bindless_resources::<M>(render_device);
15581638
let bind_group_data = material.bind_group_data();
1559-
let material_key = SmallVec::from(bytemuck::bytes_of(&bind_group_data));
1639+
let material_key = ErasedMaterialKey::new(bind_group_data);
15601640
fn specialize<M: Material>(
15611641
pipeline: &MaterialPipeline,
15621642
descriptor: &mut RenderPipelineDescriptor,
15631643
mesh_layout: &MeshVertexBufferLayoutRef,
15641644
erased_key: ErasedMaterialPipelineKey,
1565-
) -> Result<(), SpecializedMeshPipelineError> {
1566-
let material_key = bytemuck::from_bytes(erased_key.material_key.as_slice());
1645+
) -> Result<(), SpecializedMeshPipelineError>
1646+
where
1647+
M::Data: Hash + Clone,
1648+
{
1649+
let material_key = erased_key.material_key.to_key();
15671650
M::specialize(
15681651
pipeline,
15691652
descriptor,
15701653
mesh_layout,
15711654
MaterialPipelineKey {
15721655
mesh_key: erased_key.mesh_key,
1573-
bind_group_data: *material_key,
1656+
bind_group_data: material_key,
15741657
},
15751658
)
15761659
}

crates/bevy_pbr/src/pbr_material.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,7 @@ impl AsBindGroupShaderType<StandardMaterialUniform> for StandardMaterial {
12101210
bitflags! {
12111211
/// The pipeline key for `StandardMaterial`, packed into 64 bits.
12121212
#[repr(C)]
1213-
#[derive(Clone, Copy, PartialEq, Eq, Hash, bytemuck::Pod, bytemuck::Zeroable)]
1213+
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
12141214
pub struct StandardMaterialKey: u64 {
12151215
const CULL_FRONT = 0x000001;
12161216
const CULL_BACK = 0x000002;

crates/bevy_render/src/render_resource/bind_group.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -482,26 +482,24 @@ impl Deref for BindGroup {
482482
/// }
483483
///
484484
/// // Materials keys are intended to be small, cheap to hash, and
485-
/// // uniquely identify a specific material permutation, which
486-
/// // is why they are required to be `bytemuck::Pod` and `bytemuck::Zeroable`
487-
/// // when using the `AsBindGroup` derive macro.
485+
/// // uniquely identify a specific material permutation.
488486
/// #[repr(C)]
489-
/// #[derive(Copy, Clone, Hash, Eq, PartialEq, bytemuck::Pod, bytemuck::Zeroable)]
487+
/// #[derive(Copy, Clone, Hash, Eq, PartialEq)]
490488
/// struct CoolMaterialKey {
491-
/// is_shaded: u32,
489+
/// is_shaded: bool,
492490
/// }
493491
///
494492
/// impl From<&CoolMaterial> for CoolMaterialKey {
495493
/// fn from(material: &CoolMaterial) -> CoolMaterialKey {
496494
/// CoolMaterialKey {
497-
/// is_shaded: material.is_shaded as u32,
495+
/// is_shaded: material.is_shaded,
498496
/// }
499497
/// }
500498
/// }
501499
/// ```
502500
pub trait AsBindGroup {
503501
/// Data that will be stored alongside the "prepared" bind group.
504-
type Data: bytemuck::Pod + bytemuck::Zeroable + Send + Sync;
502+
type Data: Send + Sync;
505503

506504
type Param: SystemParam + 'static;
507505

crates/bevy_sprite_render/src/mesh2d/material.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ where
409409
fn clone(&self) -> Self {
410410
Self {
411411
mesh_key: self.mesh_key,
412-
bind_group_data: self.bind_group_data,
412+
bind_group_data: self.bind_group_data.clone(),
413413
}
414414
}
415415
}
@@ -763,7 +763,7 @@ pub fn specialize_material2d_meshes<M: Material2d>(
763763
&material2d_pipeline,
764764
Material2dKey {
765765
mesh_key,
766-
bind_group_data: material_2d.key,
766+
bind_group_data: material_2d.key.clone(),
767767
},
768768
&mesh.layout,
769769
);

crates/bevy_ui_render/src/ui_material.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ where
143143
fn clone(&self) -> Self {
144144
Self {
145145
hdr: self.hdr,
146-
bind_group_data: self.bind_group_data,
146+
bind_group_data: self.bind_group_data.clone(),
147147
}
148148
}
149149
}

crates/bevy_ui_render/src/ui_material_pipeline.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ pub fn queue_ui_material_nodes<M: UiMaterial>(
614614
&ui_material_pipeline,
615615
UiMaterialKey {
616616
hdr: view.hdr,
617-
bind_group_data: material.key,
617+
bind_group_data: material.key.clone(),
618618
},
619619
);
620620
if transparent_phase.items.capacity() < extracted_uinodes.uinodes.len() {

examples/shader/shader_defs.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl Material for CustomMaterial {
6565
_layout: &MeshVertexBufferLayoutRef,
6666
key: MaterialPipelineKey<Self>,
6767
) -> Result<(), SpecializedMeshPipelineError> {
68-
if key.bind_group_data.is_red == 1 {
68+
if key.bind_group_data.is_red {
6969
let fragment = descriptor.fragment.as_mut().unwrap();
7070
fragment.shader_defs.push("IS_RED".into());
7171
}
@@ -86,18 +86,16 @@ struct CustomMaterial {
8686
// In this case, we specialize on whether or not to configure the "IS_RED" shader def.
8787
// Specialization keys should be kept as small / cheap to hash as possible,
8888
// as they will be used to look up the pipeline for each drawn entity with this material type,
89-
// Which is why they are required to be `bytemuck::Pod` and `bytemuck::Zeroable` for materials
90-
// that use the `AsBindGroup` derive macro.
9189
#[repr(C)]
92-
#[derive(Eq, PartialEq, Hash, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
90+
#[derive(Eq, PartialEq, Hash, Copy, Clone)]
9391
struct CustomMaterialKey {
94-
is_red: u32,
92+
is_red: bool,
9593
}
9694

9795
impl From<&CustomMaterial> for CustomMaterialKey {
9896
fn from(material: &CustomMaterial) -> Self {
9997
Self {
100-
is_red: material.is_red as u32,
98+
is_red: material.is_red,
10199
}
102100
}
103101
}

examples/shader/shader_material_wesl.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,15 @@ struct CustomMaterial {
100100
}
101101

102102
#[repr(C)]
103-
#[derive(Eq, PartialEq, Hash, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
103+
#[derive(Eq, PartialEq, Hash, Copy, Clone)]
104104
struct CustomMaterialKey {
105-
party_mode: u32,
105+
party_mode: bool,
106106
}
107107

108108
impl From<&CustomMaterial> for CustomMaterialKey {
109109
fn from(material: &CustomMaterial) -> Self {
110110
Self {
111-
party_mode: material.party_mode as u32,
111+
party_mode: material.party_mode,
112112
}
113113
}
114114
}
@@ -127,7 +127,7 @@ impl Material for CustomMaterial {
127127
let fragment = descriptor.fragment.as_mut().unwrap();
128128
fragment.shader_defs.push(ShaderDefVal::Bool(
129129
"PARTY_MODE".to_string(),
130-
key.bind_group_data.party_mode == 1,
130+
key.bind_group_data.party_mode,
131131
));
132132
Ok(())
133133
}

0 commit comments

Comments
 (0)