Skip to content

Commit a8b37af

Browse files
committed
Replace UUID based IDs with a atomic-counted ones (#6988)
# Objective - alternative to #2895 - as mentioned in #2535 the uuid based ids in the render module should be replaced with atomic-counted ones ## Solution - instead of generating a random UUID for each render resource, this implementation increases an atomic counter - this might be replaced by the ids of wgpu if they expose them directly in the future - I have not benchmarked this solution yet, but this should be slightly faster in theory. - Bevymark does not seem to be affected much by this change, which is to be expected. - Nothing of our API has changed, other than that the IDs have lost their IMO rather insignificant documentation. - Maybe the documentation could be added back into the macro, but this would complicate the code.
1 parent cb11a94 commit a8b37af

File tree

8 files changed

+65
-91
lines changed

8 files changed

+65
-91
lines changed

crates/bevy_render/src/render_graph/node.rs

+2-18
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,17 @@
11
use crate::{
2+
define_atomic_id,
23
render_graph::{
34
Edge, InputSlotError, OutputSlotError, RenderGraphContext, RenderGraphError,
45
RunSubGraphError, SlotInfo, SlotInfos, SlotType, SlotValue,
56
},
67
renderer::RenderContext,
78
};
89
use bevy_ecs::world::World;
9-
use bevy_utils::Uuid;
1010
use downcast_rs::{impl_downcast, Downcast};
1111
use std::{borrow::Cow, fmt::Debug};
1212
use thiserror::Error;
1313

14-
/// A [`Node`] identifier.
15-
/// It automatically generates its own random uuid.
16-
///
17-
/// This id is used to reference the node internally (edges, etc).
18-
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
19-
pub struct NodeId(Uuid);
20-
21-
impl NodeId {
22-
#[allow(clippy::new_without_default)]
23-
pub fn new() -> Self {
24-
NodeId(Uuid::new_v4())
25-
}
26-
27-
pub fn uuid(&self) -> &Uuid {
28-
&self.0
29-
}
30-
}
14+
define_atomic_id!(NodeId);
3115

3216
/// A render node that can be added to a [`RenderGraph`](super::RenderGraph).
3317
///

crates/bevy_render/src/render_resource/bind_group.rs

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,19 @@
1-
pub use bevy_render_macros::AsBindGroup;
2-
use encase::ShaderType;
3-
41
use crate::{
2+
define_atomic_id,
53
prelude::Image,
64
render_asset::RenderAssets,
7-
render_resource::{BindGroupLayout, Buffer, Sampler, TextureView},
5+
render_resource::{resource_macros::*, BindGroupLayout, Buffer, Sampler, TextureView},
86
renderer::RenderDevice,
97
texture::FallbackImage,
108
};
11-
use bevy_reflect::Uuid;
9+
pub use bevy_render_macros::AsBindGroup;
10+
use encase::ShaderType;
1211
use std::ops::Deref;
1312
use wgpu::BindingResource;
1413

15-
use crate::render_resource::resource_macros::*;
14+
define_atomic_id!(BindGroupId);
1615
render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup);
1716

18-
/// A [`BindGroup`] identifier.
19-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
20-
pub struct BindGroupId(Uuid);
21-
2217
/// Bind groups are responsible for binding render resources (e.g. buffers, textures, samplers)
2318
/// to a [`TrackedRenderPass`](crate::render_phase::TrackedRenderPass).
2419
/// This makes them accessible in the pipeline (shaders) as uniforms.
@@ -42,7 +37,7 @@ impl BindGroup {
4237
impl From<wgpu::BindGroup> for BindGroup {
4338
fn from(value: wgpu::BindGroup) -> Self {
4439
BindGroup {
45-
id: BindGroupId(Uuid::new_v4()),
40+
id: BindGroupId::new(),
4641
value: ErasedBindGroup::new(value),
4742
}
4843
}

crates/bevy_render/src/render_resource/bind_group_layout.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
use crate::render_resource::resource_macros::*;
2-
use bevy_reflect::Uuid;
1+
use crate::{define_atomic_id, render_resource::resource_macros::*};
32
use std::ops::Deref;
43

5-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
6-
pub struct BindGroupLayoutId(Uuid);
7-
4+
define_atomic_id!(BindGroupLayoutId);
85
render_resource_wrapper!(ErasedBindGroupLayout, wgpu::BindGroupLayout);
96

107
#[derive(Clone, Debug)]
@@ -34,7 +31,7 @@ impl BindGroupLayout {
3431
impl From<wgpu::BindGroupLayout> for BindGroupLayout {
3532
fn from(value: wgpu::BindGroupLayout) -> Self {
3633
BindGroupLayout {
37-
id: BindGroupLayoutId(Uuid::new_v4()),
34+
id: BindGroupLayoutId::new(),
3835
value: ErasedBindGroupLayout::new(value),
3936
}
4037
}

crates/bevy_render/src/render_resource/buffer.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
use bevy_utils::Uuid;
1+
use crate::{define_atomic_id, render_resource::resource_macros::render_resource_wrapper};
22
use std::ops::{Bound, Deref, RangeBounds};
33

4-
use crate::render_resource::resource_macros::*;
5-
6-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
7-
pub struct BufferId(Uuid);
8-
4+
define_atomic_id!(BufferId);
95
render_resource_wrapper!(ErasedBuffer, wgpu::Buffer);
106

117
#[derive(Clone, Debug)]
@@ -42,7 +38,7 @@ impl Buffer {
4238
impl From<wgpu::Buffer> for Buffer {
4339
fn from(value: wgpu::Buffer) -> Self {
4440
Buffer {
45-
id: BufferId(Uuid::new_v4()),
41+
id: BufferId::new(),
4642
value: ErasedBuffer::new(value),
4743
}
4844
}

crates/bevy_render/src/render_resource/pipeline.rs

+9-15
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
1-
use crate::render_resource::{BindGroupLayout, Shader};
1+
use super::ShaderDefVal;
2+
use crate::{
3+
define_atomic_id,
4+
render_resource::{resource_macros::render_resource_wrapper, BindGroupLayout, Shader},
5+
};
26
use bevy_asset::Handle;
3-
use bevy_reflect::Uuid;
47
use std::{borrow::Cow, ops::Deref};
58
use wgpu::{
69
BufferAddress, ColorTargetState, DepthStencilState, MultisampleState, PrimitiveState,
710
VertexAttribute, VertexFormat, VertexStepMode,
811
};
912

10-
use super::ShaderDefVal;
11-
use crate::render_resource::resource_macros::*;
12-
13-
/// A [`RenderPipeline`] identifier.
14-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
15-
pub struct RenderPipelineId(Uuid);
16-
13+
define_atomic_id!(RenderPipelineId);
1714
render_resource_wrapper!(ErasedRenderPipeline, wgpu::RenderPipeline);
1815

1916
/// A [`RenderPipeline`] represents a graphics pipeline and its stages (shaders), bindings and vertex buffers.
@@ -36,7 +33,7 @@ impl RenderPipeline {
3633
impl From<wgpu::RenderPipeline> for RenderPipeline {
3734
fn from(value: wgpu::RenderPipeline) -> Self {
3835
RenderPipeline {
39-
id: RenderPipelineId(Uuid::new_v4()),
36+
id: RenderPipelineId::new(),
4037
value: ErasedRenderPipeline::new(value),
4138
}
4239
}
@@ -51,10 +48,7 @@ impl Deref for RenderPipeline {
5148
}
5249
}
5350

54-
/// A [`ComputePipeline`] identifier.
55-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
56-
pub struct ComputePipelineId(Uuid);
57-
51+
define_atomic_id!(ComputePipelineId);
5852
render_resource_wrapper!(ErasedComputePipeline, wgpu::ComputePipeline);
5953

6054
/// A [`ComputePipeline`] represents a compute pipeline and its single shader stage.
@@ -78,7 +72,7 @@ impl ComputePipeline {
7872
impl From<wgpu::ComputePipeline> for ComputePipeline {
7973
fn from(value: wgpu::ComputePipeline) -> Self {
8074
ComputePipeline {
81-
id: ComputePipelineId(Uuid::new_v4()),
75+
id: ComputePipelineId::new(),
8276
value: ErasedComputePipeline::new(value),
8377
}
8478
}

crates/bevy_render/src/render_resource/resource_macros.rs

+28
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,32 @@ macro_rules! render_resource_wrapper {
120120
};
121121
}
122122

123+
#[macro_export]
124+
macro_rules! define_atomic_id {
125+
($atomic_id_type:ident) => {
126+
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
127+
pub struct $atomic_id_type(u32);
128+
129+
// We use new instead of default to indicate that each ID created will be unique.
130+
#[allow(clippy::new_without_default)]
131+
impl $atomic_id_type {
132+
pub fn new() -> Self {
133+
use std::sync::atomic::{AtomicU32, Ordering};
134+
135+
static COUNTER: AtomicU32 = AtomicU32::new(1);
136+
137+
match COUNTER.fetch_add(1, Ordering::Relaxed) {
138+
0 => {
139+
panic!(
140+
"The system ran out of unique `{}`s.",
141+
stringify!($atomic_id_type)
142+
);
143+
}
144+
id => Self(id),
145+
}
146+
}
147+
}
148+
};
149+
}
150+
123151
pub use render_resource_wrapper;

crates/bevy_render/src/render_resource/shader.rs

+6-17
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,16 @@
1+
use super::ShaderDefVal;
2+
use crate::define_atomic_id;
13
use bevy_asset::{AssetLoader, AssetPath, Handle, LoadContext, LoadedAsset};
2-
use bevy_reflect::{TypeUuid, Uuid};
4+
use bevy_reflect::TypeUuid;
35
use bevy_utils::{tracing::error, BoxedFuture, HashMap};
4-
use naga::back::wgsl::WriterFlags;
5-
use naga::valid::Capabilities;
6-
use naga::{valid::ModuleInfo, Module};
6+
use naga::{back::wgsl::WriterFlags, valid::Capabilities, valid::ModuleInfo, Module};
77
use once_cell::sync::Lazy;
88
use regex::Regex;
99
use std::{borrow::Cow, marker::Copy, ops::Deref, path::PathBuf, str::FromStr};
1010
use thiserror::Error;
11-
use wgpu::Features;
12-
use wgpu::{util::make_spirv, ShaderModuleDescriptor, ShaderSource};
13-
14-
use super::ShaderDefVal;
11+
use wgpu::{util::make_spirv, Features, ShaderModuleDescriptor, ShaderSource};
1512

16-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
17-
pub struct ShaderId(Uuid);
18-
19-
impl ShaderId {
20-
#[allow(clippy::new_without_default)]
21-
pub fn new() -> Self {
22-
ShaderId(Uuid::new_v4())
23-
}
24-
}
13+
define_atomic_id!(ShaderId);
2514

2615
#[derive(Error, Debug)]
2716
pub enum ShaderReflectError {

crates/bevy_render/src/render_resource/texture.rs

+8-17
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1-
use bevy_utils::Uuid;
1+
use crate::define_atomic_id;
22
use std::ops::Deref;
33

44
use crate::render_resource::resource_macros::*;
55

6-
/// A [`Texture`] identifier.
7-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
8-
pub struct TextureId(Uuid);
9-
6+
define_atomic_id!(TextureId);
107
render_resource_wrapper!(ErasedTexture, wgpu::Texture);
118

129
/// A GPU-accessible texture.
@@ -35,7 +32,7 @@ impl Texture {
3532
impl From<wgpu::Texture> for Texture {
3633
fn from(value: wgpu::Texture) -> Self {
3734
Texture {
38-
id: TextureId(Uuid::new_v4()),
35+
id: TextureId::new(),
3936
value: ErasedTexture::new(value),
4037
}
4138
}
@@ -50,10 +47,7 @@ impl Deref for Texture {
5047
}
5148
}
5249

53-
/// A [`TextureView`] identifier.
54-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
55-
pub struct TextureViewId(Uuid);
56-
50+
define_atomic_id!(TextureViewId);
5751
render_resource_wrapper!(ErasedTextureView, wgpu::TextureView);
5852
render_resource_wrapper!(ErasedSurfaceTexture, wgpu::SurfaceTexture);
5953

@@ -104,7 +98,7 @@ impl TextureView {
10498
impl From<wgpu::TextureView> for TextureView {
10599
fn from(value: wgpu::TextureView) -> Self {
106100
TextureView {
107-
id: TextureViewId(Uuid::new_v4()),
101+
id: TextureViewId::new(),
108102
value: TextureViewValue::TextureView(ErasedTextureView::new(value)),
109103
}
110104
}
@@ -116,7 +110,7 @@ impl From<wgpu::SurfaceTexture> for TextureView {
116110
let texture = ErasedSurfaceTexture::new(value);
117111

118112
TextureView {
119-
id: TextureViewId(Uuid::new_v4()),
113+
id: TextureViewId::new(),
120114
value: TextureViewValue::SurfaceTexture { texture, view },
121115
}
122116
}
@@ -134,10 +128,7 @@ impl Deref for TextureView {
134128
}
135129
}
136130

137-
/// A [`Sampler`] identifier.
138-
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
139-
pub struct SamplerId(Uuid);
140-
131+
define_atomic_id!(SamplerId);
141132
render_resource_wrapper!(ErasedSampler, wgpu::Sampler);
142133

143134
/// A Sampler defines how a pipeline will sample from a [`TextureView`].
@@ -162,7 +153,7 @@ impl Sampler {
162153
impl From<wgpu::Sampler> for Sampler {
163154
fn from(value: wgpu::Sampler) -> Self {
164155
Sampler {
165-
id: SamplerId(Uuid::new_v4()),
156+
id: SamplerId::new(),
166157
value: ErasedSampler::new(value),
167158
}
168159
}

0 commit comments

Comments
 (0)