Skip to content

Commit c40b485

Browse files
mockersfmockersf
authored
use a u64 for MeshPipelineKey (#13015)
# Objective - `MeshPipelineKey` use some bits for two things - First commit in this PR adds an assertion that doesn't work currently on main - This leads to some mesh topology not working anymore, for example `LineStrip` - With examples `lines`, there should be two groups of lines, the blue one doesn't display currently ## Solution - Change the `MeshPipelineKey` to be backed by a `u64` instead, to have enough bits
1 parent 13cac2e commit c40b485

File tree

2 files changed

+50
-41
lines changed

2 files changed

+50
-41
lines changed

‎crates/bevy_pbr/src/render/mesh.rs

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,7 +1031,7 @@ bitflags::bitflags! {
10311031
#[repr(transparent)]
10321032
// NOTE: Apparently quadro drivers support up to 64x MSAA.
10331033
/// MSAA uses the highest 3 bits for the MSAA log2(sample count) to support up to 128x MSAA.
1034-
pub struct MeshPipelineKey: u32 {
1034+
pub struct MeshPipelineKey: u64 {
10351035
// Nothing
10361036
const NONE = 0;
10371037

@@ -1058,13 +1058,13 @@ bitflags::bitflags! {
10581058
const LAST_FLAG = Self::IRRADIANCE_VOLUME.bits();
10591059

10601060
// Bitfields
1061-
const BLEND_RESERVED_BITS = Self::BLEND_MASK_BITS << Self::BLEND_SHIFT_BITS; // ← Bitmask reserving bits for the blend state
1062-
const BLEND_OPAQUE = 0 << Self::BLEND_SHIFT_BITS; // ← Values are just sequential within the mask, and can range from 0 to 3
1063-
const BLEND_PREMULTIPLIED_ALPHA = 1 << Self::BLEND_SHIFT_BITS; //
1064-
const BLEND_MULTIPLY = 2 << Self::BLEND_SHIFT_BITS; // ← We still have room for one more value without adding more bits
1065-
const BLEND_ALPHA = 3 << Self::BLEND_SHIFT_BITS;
1066-
const BLEND_ALPHA_TO_COVERAGE = 4 << Self::BLEND_SHIFT_BITS;
10671061
const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS;
1062+
const BLEND_RESERVED_BITS = Self::BLEND_MASK_BITS << Self::BLEND_SHIFT_BITS; // ← Bitmask reserving bits for the blend state
1063+
const BLEND_OPAQUE = 0 << Self::BLEND_SHIFT_BITS; // ← Values are just sequential within the mask
1064+
const BLEND_PREMULTIPLIED_ALPHA = 1 << Self::BLEND_SHIFT_BITS; // ← As blend states is on 3 bits, it can range from 0 to 7
1065+
const BLEND_MULTIPLY = 2 << Self::BLEND_SHIFT_BITS; // ← See `BLEND_MASK_BITS` for the number of bits available
1066+
const BLEND_ALPHA = 3 << Self::BLEND_SHIFT_BITS; //
1067+
const BLEND_ALPHA_TO_COVERAGE = 4 << Self::BLEND_SHIFT_BITS; // ← We still have room for three more values without adding more bits
10681068
const TONEMAP_METHOD_RESERVED_BITS = Self::TONEMAP_METHOD_MASK_BITS << Self::TONEMAP_METHOD_SHIFT_BITS;
10691069
const TONEMAP_METHOD_NONE = 0 << Self::TONEMAP_METHOD_SHIFT_BITS;
10701070
const TONEMAP_METHOD_REINHARD = 1 << Self::TONEMAP_METHOD_SHIFT_BITS;
@@ -1099,31 +1099,32 @@ bitflags::bitflags! {
10991099
}
11001100

11011101
impl MeshPipelineKey {
1102-
const MSAA_MASK_BITS: u32 = 0b111;
1103-
const MSAA_SHIFT_BITS: u32 = Self::LAST_FLAG.bits().trailing_zeros() + 1;
1102+
const MSAA_MASK_BITS: u64 = 0b111;
1103+
const MSAA_SHIFT_BITS: u64 = Self::LAST_FLAG.bits().trailing_zeros() as u64 + 1;
11041104

1105-
const BLEND_MASK_BITS: u32 = 0b111;
1106-
const BLEND_SHIFT_BITS: u32 = Self::MSAA_MASK_BITS.count_ones() + Self::MSAA_SHIFT_BITS;
1105+
const BLEND_MASK_BITS: u64 = 0b111;
1106+
const BLEND_SHIFT_BITS: u64 = Self::MSAA_MASK_BITS.count_ones() as u64 + Self::MSAA_SHIFT_BITS;
11071107

1108-
const TONEMAP_METHOD_MASK_BITS: u32 = 0b111;
1109-
const TONEMAP_METHOD_SHIFT_BITS: u32 =
1110-
Self::BLEND_MASK_BITS.count_ones() + Self::BLEND_SHIFT_BITS;
1108+
const TONEMAP_METHOD_MASK_BITS: u64 = 0b111;
1109+
const TONEMAP_METHOD_SHIFT_BITS: u64 =
1110+
Self::BLEND_MASK_BITS.count_ones() as u64 + Self::BLEND_SHIFT_BITS;
11111111

1112-
const SHADOW_FILTER_METHOD_MASK_BITS: u32 = 0b11;
1113-
const SHADOW_FILTER_METHOD_SHIFT_BITS: u32 =
1114-
Self::TONEMAP_METHOD_MASK_BITS.count_ones() + Self::TONEMAP_METHOD_SHIFT_BITS;
1112+
const SHADOW_FILTER_METHOD_MASK_BITS: u64 = 0b11;
1113+
const SHADOW_FILTER_METHOD_SHIFT_BITS: u64 =
1114+
Self::TONEMAP_METHOD_MASK_BITS.count_ones() as u64 + Self::TONEMAP_METHOD_SHIFT_BITS;
11151115

1116-
const VIEW_PROJECTION_MASK_BITS: u32 = 0b11;
1117-
const VIEW_PROJECTION_SHIFT_BITS: u32 =
1118-
Self::SHADOW_FILTER_METHOD_MASK_BITS.count_ones() + Self::SHADOW_FILTER_METHOD_SHIFT_BITS;
1116+
const VIEW_PROJECTION_MASK_BITS: u64 = 0b11;
1117+
const VIEW_PROJECTION_SHIFT_BITS: u64 = Self::SHADOW_FILTER_METHOD_MASK_BITS.count_ones()
1118+
as u64
1119+
+ Self::SHADOW_FILTER_METHOD_SHIFT_BITS;
11191120

1120-
const SCREEN_SPACE_SPECULAR_TRANSMISSION_MASK_BITS: u32 = 0b11;
1121-
const SCREEN_SPACE_SPECULAR_TRANSMISSION_SHIFT_BITS: u32 =
1122-
Self::VIEW_PROJECTION_MASK_BITS.count_ones() + Self::VIEW_PROJECTION_SHIFT_BITS;
1121+
const SCREEN_SPACE_SPECULAR_TRANSMISSION_MASK_BITS: u64 = 0b11;
1122+
const SCREEN_SPACE_SPECULAR_TRANSMISSION_SHIFT_BITS: u64 =
1123+
Self::VIEW_PROJECTION_MASK_BITS.count_ones() as u64 + Self::VIEW_PROJECTION_SHIFT_BITS;
11231124

11241125
pub fn from_msaa_samples(msaa_samples: u32) -> Self {
11251126
let msaa_bits =
1126-
(msaa_samples.trailing_zeros() & Self::MSAA_MASK_BITS) << Self::MSAA_SHIFT_BITS;
1127+
(msaa_samples.trailing_zeros() as u64 & Self::MSAA_MASK_BITS) << Self::MSAA_SHIFT_BITS;
11271128
Self::from_bits_retain(msaa_bits)
11281129
}
11291130

@@ -1140,7 +1141,7 @@ impl MeshPipelineKey {
11401141
}
11411142

11421143
pub fn from_primitive_topology(primitive_topology: PrimitiveTopology) -> Self {
1143-
let primitive_topology_bits = ((primitive_topology as u32)
1144+
let primitive_topology_bits = ((primitive_topology as u64)
11441145
& BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS)
11451146
<< BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS;
11461147
Self::from_bits_retain(primitive_topology_bits)
@@ -1151,11 +1152,11 @@ impl MeshPipelineKey {
11511152
>> BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS)
11521153
& BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS;
11531154
match primitive_topology_bits {
1154-
x if x == PrimitiveTopology::PointList as u32 => PrimitiveTopology::PointList,
1155-
x if x == PrimitiveTopology::LineList as u32 => PrimitiveTopology::LineList,
1156-
x if x == PrimitiveTopology::LineStrip as u32 => PrimitiveTopology::LineStrip,
1157-
x if x == PrimitiveTopology::TriangleList as u32 => PrimitiveTopology::TriangleList,
1158-
x if x == PrimitiveTopology::TriangleStrip as u32 => PrimitiveTopology::TriangleStrip,
1155+
x if x == PrimitiveTopology::PointList as u64 => PrimitiveTopology::PointList,
1156+
x if x == PrimitiveTopology::LineList as u64 => PrimitiveTopology::LineList,
1157+
x if x == PrimitiveTopology::LineStrip as u64 => PrimitiveTopology::LineStrip,
1158+
x if x == PrimitiveTopology::TriangleList as u64 => PrimitiveTopology::TriangleList,
1159+
x if x == PrimitiveTopology::TriangleStrip as u64 => PrimitiveTopology::TriangleStrip,
11591160
_ => PrimitiveTopology::default(),
11601161
}
11611162
}
@@ -1168,6 +1169,14 @@ const_assert_eq!(
11681169
0
11691170
);
11701171

1172+
// Ensure that the reserved bits don't overlap with the topology bits
1173+
const_assert_eq!(
1174+
(BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS
1175+
<< BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS)
1176+
& MeshPipelineKey::ALL_RESERVED_BITS.bits(),
1177+
0
1178+
);
1179+
11711180
fn is_skinned(layout: &MeshVertexBufferLayoutRef) -> bool {
11721181
layout.0.contains(Mesh::ATTRIBUTE_JOINT_INDEX)
11731182
&& layout.0.contains(Mesh::ATTRIBUTE_JOINT_WEIGHT)

‎crates/bevy_render/src/mesh/mesh/mod.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,18 +1401,18 @@ bitflags! {
14011401
/// go upward. This allows the PBR bits in the downstream crate `bevy_pbr`
14021402
/// to coexist in the same field without any shifts.
14031403
#[derive(Clone, Debug)]
1404-
pub struct BaseMeshPipelineKey: u32 {
1405-
const MORPH_TARGETS = 1 << 31;
1404+
pub struct BaseMeshPipelineKey: u64 {
1405+
const MORPH_TARGETS = 1 << (u64::BITS - 1);
14061406
}
14071407
}
14081408

14091409
impl BaseMeshPipelineKey {
1410-
pub const PRIMITIVE_TOPOLOGY_MASK_BITS: u32 = 0b111;
1411-
pub const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u32 =
1412-
31 - Self::PRIMITIVE_TOPOLOGY_MASK_BITS.count_ones();
1410+
pub const PRIMITIVE_TOPOLOGY_MASK_BITS: u64 = 0b111;
1411+
pub const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u64 =
1412+
(u64::BITS - 1 - Self::PRIMITIVE_TOPOLOGY_MASK_BITS.count_ones()) as u64;
14131413

14141414
pub fn from_primitive_topology(primitive_topology: PrimitiveTopology) -> Self {
1415-
let primitive_topology_bits = ((primitive_topology as u32)
1415+
let primitive_topology_bits = ((primitive_topology as u64)
14161416
& Self::PRIMITIVE_TOPOLOGY_MASK_BITS)
14171417
<< Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS;
14181418
Self::from_bits_retain(primitive_topology_bits)
@@ -1422,11 +1422,11 @@ impl BaseMeshPipelineKey {
14221422
let primitive_topology_bits = (self.bits() >> Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS)
14231423
& Self::PRIMITIVE_TOPOLOGY_MASK_BITS;
14241424
match primitive_topology_bits {
1425-
x if x == PrimitiveTopology::PointList as u32 => PrimitiveTopology::PointList,
1426-
x if x == PrimitiveTopology::LineList as u32 => PrimitiveTopology::LineList,
1427-
x if x == PrimitiveTopology::LineStrip as u32 => PrimitiveTopology::LineStrip,
1428-
x if x == PrimitiveTopology::TriangleList as u32 => PrimitiveTopology::TriangleList,
1429-
x if x == PrimitiveTopology::TriangleStrip as u32 => PrimitiveTopology::TriangleStrip,
1425+
x if x == PrimitiveTopology::PointList as u64 => PrimitiveTopology::PointList,
1426+
x if x == PrimitiveTopology::LineList as u64 => PrimitiveTopology::LineList,
1427+
x if x == PrimitiveTopology::LineStrip as u64 => PrimitiveTopology::LineStrip,
1428+
x if x == PrimitiveTopology::TriangleList as u64 => PrimitiveTopology::TriangleList,
1429+
x if x == PrimitiveTopology::TriangleStrip as u64 => PrimitiveTopology::TriangleStrip,
14301430
_ => PrimitiveTopology::default(),
14311431
}
14321432
}

0 commit comments

Comments
 (0)