From c40b4850951f9a64fe65465c50c9c8f16ef7c1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Sun, 21 Apr 2024 22:01:45 +0200 Subject: [PATCH] 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 --- crates/bevy_pbr/src/render/mesh.rs | 69 ++++++++++++++----------- crates/bevy_render/src/mesh/mesh/mod.rs | 22 ++++---- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 93b7ed9918..ed5a4d7549 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -1031,7 +1031,7 @@ bitflags::bitflags! { #[repr(transparent)] // NOTE: Apparently quadro drivers support up to 64x MSAA. /// MSAA uses the highest 3 bits for the MSAA log2(sample count) to support up to 128x MSAA. - pub struct MeshPipelineKey: u32 { + pub struct MeshPipelineKey: u64 { // Nothing const NONE = 0; @@ -1058,13 +1058,13 @@ bitflags::bitflags! { const LAST_FLAG = Self::IRRADIANCE_VOLUME.bits(); // Bitfields - const BLEND_RESERVED_BITS = Self::BLEND_MASK_BITS << Self::BLEND_SHIFT_BITS; // ← Bitmask reserving bits for the blend state - const BLEND_OPAQUE = 0 << Self::BLEND_SHIFT_BITS; // ← Values are just sequential within the mask, and can range from 0 to 3 - const BLEND_PREMULTIPLIED_ALPHA = 1 << Self::BLEND_SHIFT_BITS; // - const BLEND_MULTIPLY = 2 << Self::BLEND_SHIFT_BITS; // ← We still have room for one more value without adding more bits - const BLEND_ALPHA = 3 << Self::BLEND_SHIFT_BITS; - const BLEND_ALPHA_TO_COVERAGE = 4 << Self::BLEND_SHIFT_BITS; const MSAA_RESERVED_BITS = Self::MSAA_MASK_BITS << Self::MSAA_SHIFT_BITS; + const BLEND_RESERVED_BITS = Self::BLEND_MASK_BITS << Self::BLEND_SHIFT_BITS; // ← Bitmask reserving bits for the blend state + const BLEND_OPAQUE = 0 << Self::BLEND_SHIFT_BITS; // ← Values are just sequential within the mask + const BLEND_PREMULTIPLIED_ALPHA = 1 << Self::BLEND_SHIFT_BITS; // ← As blend states is on 3 bits, it can range from 0 to 7 + const BLEND_MULTIPLY = 2 << Self::BLEND_SHIFT_BITS; // ← See `BLEND_MASK_BITS` for the number of bits available + const BLEND_ALPHA = 3 << Self::BLEND_SHIFT_BITS; // + const BLEND_ALPHA_TO_COVERAGE = 4 << Self::BLEND_SHIFT_BITS; // ← We still have room for three more values without adding more bits const TONEMAP_METHOD_RESERVED_BITS = Self::TONEMAP_METHOD_MASK_BITS << Self::TONEMAP_METHOD_SHIFT_BITS; const TONEMAP_METHOD_NONE = 0 << Self::TONEMAP_METHOD_SHIFT_BITS; const TONEMAP_METHOD_REINHARD = 1 << Self::TONEMAP_METHOD_SHIFT_BITS; @@ -1099,31 +1099,32 @@ bitflags::bitflags! { } impl MeshPipelineKey { - const MSAA_MASK_BITS: u32 = 0b111; - const MSAA_SHIFT_BITS: u32 = Self::LAST_FLAG.bits().trailing_zeros() + 1; + const MSAA_MASK_BITS: u64 = 0b111; + const MSAA_SHIFT_BITS: u64 = Self::LAST_FLAG.bits().trailing_zeros() as u64 + 1; - const BLEND_MASK_BITS: u32 = 0b111; - const BLEND_SHIFT_BITS: u32 = Self::MSAA_MASK_BITS.count_ones() + Self::MSAA_SHIFT_BITS; + const BLEND_MASK_BITS: u64 = 0b111; + const BLEND_SHIFT_BITS: u64 = Self::MSAA_MASK_BITS.count_ones() as u64 + Self::MSAA_SHIFT_BITS; - const TONEMAP_METHOD_MASK_BITS: u32 = 0b111; - const TONEMAP_METHOD_SHIFT_BITS: u32 = - Self::BLEND_MASK_BITS.count_ones() + Self::BLEND_SHIFT_BITS; + const TONEMAP_METHOD_MASK_BITS: u64 = 0b111; + const TONEMAP_METHOD_SHIFT_BITS: u64 = + Self::BLEND_MASK_BITS.count_ones() as u64 + Self::BLEND_SHIFT_BITS; - const SHADOW_FILTER_METHOD_MASK_BITS: u32 = 0b11; - const SHADOW_FILTER_METHOD_SHIFT_BITS: u32 = - Self::TONEMAP_METHOD_MASK_BITS.count_ones() + Self::TONEMAP_METHOD_SHIFT_BITS; + const SHADOW_FILTER_METHOD_MASK_BITS: u64 = 0b11; + const SHADOW_FILTER_METHOD_SHIFT_BITS: u64 = + Self::TONEMAP_METHOD_MASK_BITS.count_ones() as u64 + Self::TONEMAP_METHOD_SHIFT_BITS; - const VIEW_PROJECTION_MASK_BITS: u32 = 0b11; - const VIEW_PROJECTION_SHIFT_BITS: u32 = - Self::SHADOW_FILTER_METHOD_MASK_BITS.count_ones() + Self::SHADOW_FILTER_METHOD_SHIFT_BITS; + const VIEW_PROJECTION_MASK_BITS: u64 = 0b11; + const VIEW_PROJECTION_SHIFT_BITS: u64 = Self::SHADOW_FILTER_METHOD_MASK_BITS.count_ones() + as u64 + + Self::SHADOW_FILTER_METHOD_SHIFT_BITS; - const SCREEN_SPACE_SPECULAR_TRANSMISSION_MASK_BITS: u32 = 0b11; - const SCREEN_SPACE_SPECULAR_TRANSMISSION_SHIFT_BITS: u32 = - Self::VIEW_PROJECTION_MASK_BITS.count_ones() + Self::VIEW_PROJECTION_SHIFT_BITS; + const SCREEN_SPACE_SPECULAR_TRANSMISSION_MASK_BITS: u64 = 0b11; + const SCREEN_SPACE_SPECULAR_TRANSMISSION_SHIFT_BITS: u64 = + Self::VIEW_PROJECTION_MASK_BITS.count_ones() as u64 + Self::VIEW_PROJECTION_SHIFT_BITS; pub fn from_msaa_samples(msaa_samples: u32) -> Self { let msaa_bits = - (msaa_samples.trailing_zeros() & Self::MSAA_MASK_BITS) << Self::MSAA_SHIFT_BITS; + (msaa_samples.trailing_zeros() as u64 & Self::MSAA_MASK_BITS) << Self::MSAA_SHIFT_BITS; Self::from_bits_retain(msaa_bits) } @@ -1140,7 +1141,7 @@ impl MeshPipelineKey { } pub fn from_primitive_topology(primitive_topology: PrimitiveTopology) -> Self { - let primitive_topology_bits = ((primitive_topology as u32) + let primitive_topology_bits = ((primitive_topology as u64) & BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS) << BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS; Self::from_bits_retain(primitive_topology_bits) @@ -1151,11 +1152,11 @@ impl MeshPipelineKey { >> BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS) & BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS; match primitive_topology_bits { - x if x == PrimitiveTopology::PointList as u32 => PrimitiveTopology::PointList, - x if x == PrimitiveTopology::LineList as u32 => PrimitiveTopology::LineList, - x if x == PrimitiveTopology::LineStrip as u32 => PrimitiveTopology::LineStrip, - x if x == PrimitiveTopology::TriangleList as u32 => PrimitiveTopology::TriangleList, - x if x == PrimitiveTopology::TriangleStrip as u32 => PrimitiveTopology::TriangleStrip, + x if x == PrimitiveTopology::PointList as u64 => PrimitiveTopology::PointList, + x if x == PrimitiveTopology::LineList as u64 => PrimitiveTopology::LineList, + x if x == PrimitiveTopology::LineStrip as u64 => PrimitiveTopology::LineStrip, + x if x == PrimitiveTopology::TriangleList as u64 => PrimitiveTopology::TriangleList, + x if x == PrimitiveTopology::TriangleStrip as u64 => PrimitiveTopology::TriangleStrip, _ => PrimitiveTopology::default(), } } @@ -1168,6 +1169,14 @@ const_assert_eq!( 0 ); +// Ensure that the reserved bits don't overlap with the topology bits +const_assert_eq!( + (BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_MASK_BITS + << BaseMeshPipelineKey::PRIMITIVE_TOPOLOGY_SHIFT_BITS) + & MeshPipelineKey::ALL_RESERVED_BITS.bits(), + 0 +); + fn is_skinned(layout: &MeshVertexBufferLayoutRef) -> bool { layout.0.contains(Mesh::ATTRIBUTE_JOINT_INDEX) && layout.0.contains(Mesh::ATTRIBUTE_JOINT_WEIGHT) diff --git a/crates/bevy_render/src/mesh/mesh/mod.rs b/crates/bevy_render/src/mesh/mesh/mod.rs index 8452b3f310..d861794d87 100644 --- a/crates/bevy_render/src/mesh/mesh/mod.rs +++ b/crates/bevy_render/src/mesh/mesh/mod.rs @@ -1401,18 +1401,18 @@ bitflags! { /// go upward. This allows the PBR bits in the downstream crate `bevy_pbr` /// to coexist in the same field without any shifts. #[derive(Clone, Debug)] - pub struct BaseMeshPipelineKey: u32 { - const MORPH_TARGETS = 1 << 31; + pub struct BaseMeshPipelineKey: u64 { + const MORPH_TARGETS = 1 << (u64::BITS - 1); } } impl BaseMeshPipelineKey { - pub const PRIMITIVE_TOPOLOGY_MASK_BITS: u32 = 0b111; - pub const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u32 = - 31 - Self::PRIMITIVE_TOPOLOGY_MASK_BITS.count_ones(); + pub const PRIMITIVE_TOPOLOGY_MASK_BITS: u64 = 0b111; + pub const PRIMITIVE_TOPOLOGY_SHIFT_BITS: u64 = + (u64::BITS - 1 - Self::PRIMITIVE_TOPOLOGY_MASK_BITS.count_ones()) as u64; pub fn from_primitive_topology(primitive_topology: PrimitiveTopology) -> Self { - let primitive_topology_bits = ((primitive_topology as u32) + let primitive_topology_bits = ((primitive_topology as u64) & Self::PRIMITIVE_TOPOLOGY_MASK_BITS) << Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS; Self::from_bits_retain(primitive_topology_bits) @@ -1422,11 +1422,11 @@ impl BaseMeshPipelineKey { let primitive_topology_bits = (self.bits() >> Self::PRIMITIVE_TOPOLOGY_SHIFT_BITS) & Self::PRIMITIVE_TOPOLOGY_MASK_BITS; match primitive_topology_bits { - x if x == PrimitiveTopology::PointList as u32 => PrimitiveTopology::PointList, - x if x == PrimitiveTopology::LineList as u32 => PrimitiveTopology::LineList, - x if x == PrimitiveTopology::LineStrip as u32 => PrimitiveTopology::LineStrip, - x if x == PrimitiveTopology::TriangleList as u32 => PrimitiveTopology::TriangleList, - x if x == PrimitiveTopology::TriangleStrip as u32 => PrimitiveTopology::TriangleStrip, + x if x == PrimitiveTopology::PointList as u64 => PrimitiveTopology::PointList, + x if x == PrimitiveTopology::LineList as u64 => PrimitiveTopology::LineList, + x if x == PrimitiveTopology::LineStrip as u64 => PrimitiveTopology::LineStrip, + x if x == PrimitiveTopology::TriangleList as u64 => PrimitiveTopology::TriangleList, + x if x == PrimitiveTopology::TriangleStrip as u64 => PrimitiveTopology::TriangleStrip, _ => PrimitiveTopology::default(), } }