From b0d0a24eacd2e4208bef04122f8cc5611e460286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurt=20K=C3=BChnert?= Date: Sun, 25 Dec 2022 00:23:15 +0000 Subject: [PATCH] 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. --- crates/bevy_render/src/render_graph/node.rs | 20 ++----------- .../src/render_resource/bind_group.rs | 17 ++++------- .../src/render_resource/bind_group_layout.rs | 9 ++---- .../bevy_render/src/render_resource/buffer.rs | 10 ++----- .../src/render_resource/pipeline.rs | 24 ++++++---------- .../src/render_resource/resource_macros.rs | 28 +++++++++++++++++++ .../bevy_render/src/render_resource/shader.rs | 23 ++++----------- .../src/render_resource/texture.rs | 25 ++++++----------- 8 files changed, 65 insertions(+), 91 deletions(-) diff --git a/crates/bevy_render/src/render_graph/node.rs b/crates/bevy_render/src/render_graph/node.rs index ab60925e6fbe14..9e3e884ba8b082 100644 --- a/crates/bevy_render/src/render_graph/node.rs +++ b/crates/bevy_render/src/render_graph/node.rs @@ -1,4 +1,5 @@ use crate::{ + define_atomic_id, render_graph::{ Edge, InputSlotError, OutputSlotError, RenderGraphContext, RenderGraphError, RunSubGraphError, SlotInfo, SlotInfos, SlotType, SlotValue, @@ -6,28 +7,11 @@ use crate::{ renderer::RenderContext, }; use bevy_ecs::world::World; -use bevy_utils::Uuid; use downcast_rs::{impl_downcast, Downcast}; use std::{borrow::Cow, fmt::Debug}; use thiserror::Error; -/// A [`Node`] identifier. -/// It automatically generates its own random uuid. -/// -/// This id is used to reference the node internally (edges, etc). -#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub struct NodeId(Uuid); - -impl NodeId { - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - NodeId(Uuid::new_v4()) - } - - pub fn uuid(&self) -> &Uuid { - &self.0 - } -} +define_atomic_id!(NodeId); /// A render node that can be added to a [`RenderGraph`](super::RenderGraph). /// diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 88d3c6cc927e1b..5bc13b1d1f47ad 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -1,24 +1,19 @@ -pub use bevy_render_macros::AsBindGroup; -use encase::ShaderType; - use crate::{ + define_atomic_id, prelude::Image, render_asset::RenderAssets, - render_resource::{BindGroupLayout, Buffer, Sampler, TextureView}, + render_resource::{resource_macros::*, BindGroupLayout, Buffer, Sampler, TextureView}, renderer::RenderDevice, texture::FallbackImage, }; -use bevy_reflect::Uuid; +pub use bevy_render_macros::AsBindGroup; +use encase::ShaderType; use std::ops::Deref; use wgpu::BindingResource; -use crate::render_resource::resource_macros::*; +define_atomic_id!(BindGroupId); render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup); -/// A [`BindGroup`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct BindGroupId(Uuid); - /// Bind groups are responsible for binding render resources (e.g. buffers, textures, samplers) /// to a [`TrackedRenderPass`](crate::render_phase::TrackedRenderPass). /// This makes them accessible in the pipeline (shaders) as uniforms. @@ -42,7 +37,7 @@ impl BindGroup { impl From for BindGroup { fn from(value: wgpu::BindGroup) -> Self { BindGroup { - id: BindGroupId(Uuid::new_v4()), + id: BindGroupId::new(), value: ErasedBindGroup::new(value), } } diff --git a/crates/bevy_render/src/render_resource/bind_group_layout.rs b/crates/bevy_render/src/render_resource/bind_group_layout.rs index 47edbbd113580c..9793f1391d1883 100644 --- a/crates/bevy_render/src/render_resource/bind_group_layout.rs +++ b/crates/bevy_render/src/render_resource/bind_group_layout.rs @@ -1,10 +1,7 @@ -use crate::render_resource::resource_macros::*; -use bevy_reflect::Uuid; +use crate::{define_atomic_id, render_resource::resource_macros::*}; use std::ops::Deref; -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct BindGroupLayoutId(Uuid); - +define_atomic_id!(BindGroupLayoutId); render_resource_wrapper!(ErasedBindGroupLayout, wgpu::BindGroupLayout); #[derive(Clone, Debug)] @@ -34,7 +31,7 @@ impl BindGroupLayout { impl From for BindGroupLayout { fn from(value: wgpu::BindGroupLayout) -> Self { BindGroupLayout { - id: BindGroupLayoutId(Uuid::new_v4()), + id: BindGroupLayoutId::new(), value: ErasedBindGroupLayout::new(value), } } diff --git a/crates/bevy_render/src/render_resource/buffer.rs b/crates/bevy_render/src/render_resource/buffer.rs index 6bae2e529ab417..9867945673efba 100644 --- a/crates/bevy_render/src/render_resource/buffer.rs +++ b/crates/bevy_render/src/render_resource/buffer.rs @@ -1,11 +1,7 @@ -use bevy_utils::Uuid; +use crate::{define_atomic_id, render_resource::resource_macros::render_resource_wrapper}; use std::ops::{Bound, Deref, RangeBounds}; -use crate::render_resource::resource_macros::*; - -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct BufferId(Uuid); - +define_atomic_id!(BufferId); render_resource_wrapper!(ErasedBuffer, wgpu::Buffer); #[derive(Clone, Debug)] @@ -42,7 +38,7 @@ impl Buffer { impl From for Buffer { fn from(value: wgpu::Buffer) -> Self { Buffer { - id: BufferId(Uuid::new_v4()), + id: BufferId::new(), value: ErasedBuffer::new(value), } } diff --git a/crates/bevy_render/src/render_resource/pipeline.rs b/crates/bevy_render/src/render_resource/pipeline.rs index 3a379b60522f22..edd3d8f6d67cbf 100644 --- a/crates/bevy_render/src/render_resource/pipeline.rs +++ b/crates/bevy_render/src/render_resource/pipeline.rs @@ -1,19 +1,16 @@ -use crate::render_resource::{BindGroupLayout, Shader}; +use super::ShaderDefVal; +use crate::{ + define_atomic_id, + render_resource::{resource_macros::render_resource_wrapper, BindGroupLayout, Shader}, +}; use bevy_asset::Handle; -use bevy_reflect::Uuid; use std::{borrow::Cow, ops::Deref}; use wgpu::{ BufferAddress, ColorTargetState, DepthStencilState, MultisampleState, PrimitiveState, VertexAttribute, VertexFormat, VertexStepMode, }; -use super::ShaderDefVal; -use crate::render_resource::resource_macros::*; - -/// A [`RenderPipeline`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct RenderPipelineId(Uuid); - +define_atomic_id!(RenderPipelineId); render_resource_wrapper!(ErasedRenderPipeline, wgpu::RenderPipeline); /// A [`RenderPipeline`] represents a graphics pipeline and its stages (shaders), bindings and vertex buffers. @@ -36,7 +33,7 @@ impl RenderPipeline { impl From for RenderPipeline { fn from(value: wgpu::RenderPipeline) -> Self { RenderPipeline { - id: RenderPipelineId(Uuid::new_v4()), + id: RenderPipelineId::new(), value: ErasedRenderPipeline::new(value), } } @@ -51,10 +48,7 @@ impl Deref for RenderPipeline { } } -/// A [`ComputePipeline`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct ComputePipelineId(Uuid); - +define_atomic_id!(ComputePipelineId); render_resource_wrapper!(ErasedComputePipeline, wgpu::ComputePipeline); /// A [`ComputePipeline`] represents a compute pipeline and its single shader stage. @@ -78,7 +72,7 @@ impl ComputePipeline { impl From for ComputePipeline { fn from(value: wgpu::ComputePipeline) -> Self { ComputePipeline { - id: ComputePipelineId(Uuid::new_v4()), + id: ComputePipelineId::new(), value: ErasedComputePipeline::new(value), } } diff --git a/crates/bevy_render/src/render_resource/resource_macros.rs b/crates/bevy_render/src/render_resource/resource_macros.rs index 4f1a69673b043f..e27380426a169b 100644 --- a/crates/bevy_render/src/render_resource/resource_macros.rs +++ b/crates/bevy_render/src/render_resource/resource_macros.rs @@ -120,4 +120,32 @@ macro_rules! render_resource_wrapper { }; } +#[macro_export] +macro_rules! define_atomic_id { + ($atomic_id_type:ident) => { + #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] + pub struct $atomic_id_type(u32); + + // We use new instead of default to indicate that each ID created will be unique. + #[allow(clippy::new_without_default)] + impl $atomic_id_type { + pub fn new() -> Self { + use std::sync::atomic::{AtomicU32, Ordering}; + + static COUNTER: AtomicU32 = AtomicU32::new(1); + + match COUNTER.fetch_add(1, Ordering::Relaxed) { + 0 => { + panic!( + "The system ran out of unique `{}`s.", + stringify!($atomic_id_type) + ); + } + id => Self(id), + } + } + } + }; +} + pub use render_resource_wrapper; diff --git a/crates/bevy_render/src/render_resource/shader.rs b/crates/bevy_render/src/render_resource/shader.rs index 569234abeeb6db..08f01f64fa81c9 100644 --- a/crates/bevy_render/src/render_resource/shader.rs +++ b/crates/bevy_render/src/render_resource/shader.rs @@ -1,27 +1,16 @@ +use super::ShaderDefVal; +use crate::define_atomic_id; use bevy_asset::{AssetLoader, AssetPath, Handle, LoadContext, LoadedAsset}; -use bevy_reflect::{TypeUuid, Uuid}; +use bevy_reflect::TypeUuid; use bevy_utils::{tracing::error, BoxedFuture, HashMap}; -use naga::back::wgsl::WriterFlags; -use naga::valid::Capabilities; -use naga::{valid::ModuleInfo, Module}; +use naga::{back::wgsl::WriterFlags, valid::Capabilities, valid::ModuleInfo, Module}; use once_cell::sync::Lazy; use regex::Regex; use std::{borrow::Cow, marker::Copy, ops::Deref, path::PathBuf, str::FromStr}; use thiserror::Error; -use wgpu::Features; -use wgpu::{util::make_spirv, ShaderModuleDescriptor, ShaderSource}; - -use super::ShaderDefVal; +use wgpu::{util::make_spirv, Features, ShaderModuleDescriptor, ShaderSource}; -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct ShaderId(Uuid); - -impl ShaderId { - #[allow(clippy::new_without_default)] - pub fn new() -> Self { - ShaderId(Uuid::new_v4()) - } -} +define_atomic_id!(ShaderId); #[derive(Error, Debug)] pub enum ShaderReflectError { diff --git a/crates/bevy_render/src/render_resource/texture.rs b/crates/bevy_render/src/render_resource/texture.rs index 330d7cdb0b85e8..02cc8184080734 100644 --- a/crates/bevy_render/src/render_resource/texture.rs +++ b/crates/bevy_render/src/render_resource/texture.rs @@ -1,12 +1,9 @@ -use bevy_utils::Uuid; +use crate::define_atomic_id; use std::ops::Deref; use crate::render_resource::resource_macros::*; -/// A [`Texture`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct TextureId(Uuid); - +define_atomic_id!(TextureId); render_resource_wrapper!(ErasedTexture, wgpu::Texture); /// A GPU-accessible texture. @@ -35,7 +32,7 @@ impl Texture { impl From for Texture { fn from(value: wgpu::Texture) -> Self { Texture { - id: TextureId(Uuid::new_v4()), + id: TextureId::new(), value: ErasedTexture::new(value), } } @@ -50,10 +47,7 @@ impl Deref for Texture { } } -/// A [`TextureView`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct TextureViewId(Uuid); - +define_atomic_id!(TextureViewId); render_resource_wrapper!(ErasedTextureView, wgpu::TextureView); render_resource_wrapper!(ErasedSurfaceTexture, wgpu::SurfaceTexture); @@ -104,7 +98,7 @@ impl TextureView { impl From for TextureView { fn from(value: wgpu::TextureView) -> Self { TextureView { - id: TextureViewId(Uuid::new_v4()), + id: TextureViewId::new(), value: TextureViewValue::TextureView(ErasedTextureView::new(value)), } } @@ -116,7 +110,7 @@ impl From for TextureView { let texture = ErasedSurfaceTexture::new(value); TextureView { - id: TextureViewId(Uuid::new_v4()), + id: TextureViewId::new(), value: TextureViewValue::SurfaceTexture { texture, view }, } } @@ -134,10 +128,7 @@ impl Deref for TextureView { } } -/// A [`Sampler`] identifier. -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct SamplerId(Uuid); - +define_atomic_id!(SamplerId); render_resource_wrapper!(ErasedSampler, wgpu::Sampler); /// A Sampler defines how a pipeline will sample from a [`TextureView`]. @@ -162,7 +153,7 @@ impl Sampler { impl From for Sampler { fn from(value: wgpu::Sampler) -> Self { Sampler { - id: SamplerId(Uuid::new_v4()), + id: SamplerId::new(), value: ErasedSampler::new(value), } }