From a8f1d402061e408c93c73540aa00f7a459af4dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurt=20K=C3=BChnert?= Date: Tue, 20 Dec 2022 13:04:17 +0100 Subject: [PATCH 1/4] replaced the uuid based ids of the render resources with a derive macro based on atomic counters --- crates/bevy_render/macros/src/atomic_id.rs | 47 +++++++++++++++++++ crates/bevy_render/macros/src/lib.rs | 6 +++ crates/bevy_render/src/render_graph/node.rs | 20 +------- .../src/render_resource/bind_group.rs | 16 ++----- .../src/render_resource/bind_group_layout.rs | 8 ++-- .../bevy_render/src/render_resource/buffer.rs | 8 ++-- .../src/render_resource/pipeline.rs | 16 ++----- .../src/render_resource/resource_macros.rs | 9 ++++ .../bevy_render/src/render_resource/shader.rs | 23 +++------ .../src/render_resource/texture.rs | 25 ++++------ 10 files changed, 94 insertions(+), 84 deletions(-) create mode 100644 crates/bevy_render/macros/src/atomic_id.rs diff --git a/crates/bevy_render/macros/src/atomic_id.rs b/crates/bevy_render/macros/src/atomic_id.rs new file mode 100644 index 0000000000000..a5aaad83f0136 --- /dev/null +++ b/crates/bevy_render/macros/src/atomic_id.rs @@ -0,0 +1,47 @@ +use proc_macro::TokenStream; +use quote::quote; +use syn::parse::{Parse, ParseStream}; +use syn::{parse_macro_input, Ident, Visibility}; + +pub struct AtomicIdStruct { + pub vis: Visibility, + pub ident: Ident, +} + +impl Parse for AtomicIdStruct { + fn parse(input: ParseStream) -> syn::Result { + Ok(Self { + vis: input.parse()?, + ident: input.parse()?, + }) + } +} + +pub fn define_atomic_id(input: TokenStream) -> TokenStream { + let ast = parse_macro_input!(input as AtomicIdStruct); + + let struct_name = &ast.ident; + let visibility = &ast.vis; + let error_message = format!("The system ran out of unique `{}`s.", struct_name); + + TokenStream::from(quote! { + + #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] + #visibility struct #struct_name(crate::render_resource::resource_macros::AtomicIdType); + + impl #struct_name { + pub fn new() -> Self { + use crate::render_resource::resource_macros::AtomicIdCounter; + use std::sync::atomic::Ordering; + + static COUNTER: AtomicIdCounter = AtomicIdCounter::new(0); + COUNTER + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| { + val.checked_add(1) + }) + .map(Self) + .expect(#error_message) + } + } + }) +} diff --git a/crates/bevy_render/macros/src/lib.rs b/crates/bevy_render/macros/src/lib.rs index 863b48baf5fa0..8dc2d6cf8c249 100644 --- a/crates/bevy_render/macros/src/lib.rs +++ b/crates/bevy_render/macros/src/lib.rs @@ -1,4 +1,5 @@ mod as_bind_group; +mod atomic_id; mod extract_resource; use bevy_macro_utils::BevyManifest; @@ -23,3 +24,8 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { as_bind_group::derive_as_bind_group(input).unwrap_or_else(|err| err.to_compile_error().into()) } + +#[proc_macro] +pub fn define_atomic_id(input: TokenStream) -> TokenStream { + atomic_id::define_atomic_id(input) +} diff --git a/crates/bevy_render/src/render_graph/node.rs b/crates/bevy_render/src/render_graph/node.rs index ab60925e6fbe1..198d8286ccee7 100644 --- a/crates/bevy_render/src/render_graph/node.rs +++ b/crates/bevy_render/src/render_graph/node.rs @@ -6,28 +6,12 @@ use crate::{ renderer::RenderContext, }; use bevy_ecs::world::World; -use bevy_utils::Uuid; +use bevy_render_macros::define_atomic_id; 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!(pub 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 88d3c6cc927e1..401e6b6dea366 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -1,24 +1,18 @@ -pub use bevy_render_macros::AsBindGroup; -use encase::ShaderType; - use crate::{ 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::{define_atomic_id, AsBindGroup}; +use encase::ShaderType; use std::ops::Deref; use wgpu::BindingResource; -use crate::render_resource::resource_macros::*; +define_atomic_id!(pub 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 +36,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 47edbbd113580..722bc91a7585a 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,8 @@ use crate::render_resource::resource_macros::*; -use bevy_reflect::Uuid; +use bevy_render_macros::define_atomic_id; use std::ops::Deref; -#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] -pub struct BindGroupLayoutId(Uuid); - +define_atomic_id!(pub BindGroupLayoutId); render_resource_wrapper!(ErasedBindGroupLayout, wgpu::BindGroupLayout); #[derive(Clone, Debug)] @@ -34,7 +32,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 6bae2e529ab41..c86de6d0978a4 100644 --- a/crates/bevy_render/src/render_resource/buffer.rs +++ b/crates/bevy_render/src/render_resource/buffer.rs @@ -1,11 +1,9 @@ -use bevy_utils::Uuid; +use bevy_render_macros::define_atomic_id; 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!(pub BufferId); render_resource_wrapper!(ErasedBuffer, wgpu::Buffer); #[derive(Clone, Debug)] @@ -42,7 +40,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 3a379b60522f2..e6ce173474170 100644 --- a/crates/bevy_render/src/render_resource/pipeline.rs +++ b/crates/bevy_render/src/render_resource/pipeline.rs @@ -1,6 +1,6 @@ use crate::render_resource::{BindGroupLayout, Shader}; use bevy_asset::Handle; -use bevy_reflect::Uuid; +use bevy_render_macros::define_atomic_id; use std::{borrow::Cow, ops::Deref}; use wgpu::{ BufferAddress, ColorTargetState, DepthStencilState, MultisampleState, PrimitiveState, @@ -10,10 +10,7 @@ use wgpu::{ 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!(pub 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!(pub 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 4f1a69673b043..19b6994807a8a 100644 --- a/crates/bevy_render/src/render_resource/resource_macros.rs +++ b/crates/bevy_render/src/render_resource/resource_macros.rs @@ -1,3 +1,12 @@ +#[cfg(target_has_atomic = "64")] +pub type AtomicIdCounter = std::sync::atomic::AtomicU64; +#[cfg(target_has_atomic = "64")] +pub type AtomicIdType = u64; +#[cfg(not(target_has_atomic = "64"))] +pub type AtomicIdCounter = std::sync::atomic::AtomicUsize; +#[cfg(not(target_has_atomic = "64"))] +pub type AtomicIdType = usize; + // structs containing wgpu types take a long time to compile. this is particularly bad for generic // structs containing wgpu structs. we avoid that in debug builds (and for cargo check and rust analyzer) // by boxing and type-erasing with the `render_resource_wrapper` macro. diff --git a/crates/bevy_render/src/render_resource/shader.rs b/crates/bevy_render/src/render_resource/shader.rs index 569234abeeb6d..3a5382d78e5fa 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 bevy_asset::{AssetLoader, AssetPath, Handle, LoadContext, LoadedAsset}; -use bevy_reflect::{TypeUuid, Uuid}; +use bevy_reflect::TypeUuid; +use bevy_render_macros::define_atomic_id; 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!(pub 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 330d7cdb0b85e..c1551a1defbc0 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 bevy_render_macros::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!(pub 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!(pub 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!(pub 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), } } From 6f64b666538bf47da72def220cd9638111f60263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurt=20K=C3=BChnert?= Date: Tue, 20 Dec 2022 21:44:34 +0100 Subject: [PATCH 2/4] changed the define_atomic_id macro implementation to macro_rules using default trait instead of a new constructor --- crates/bevy_render/macros/src/atomic_id.rs | 47 ------------------- crates/bevy_render/macros/src/lib.rs | 6 --- crates/bevy_render/src/render_graph/graph.rs | 2 +- crates/bevy_render/src/render_graph/node.rs | 4 +- .../src/render_resource/bind_group.rs | 8 ++-- .../src/render_resource/bind_group_layout.rs | 8 ++-- .../bevy_render/src/render_resource/buffer.rs | 9 ++-- .../src/render_resource/pipeline.rs | 19 ++++---- .../src/render_resource/resource_macros.rs | 34 ++++++++++---- .../bevy_render/src/render_resource/shader.rs | 4 +- .../src/render_resource/texture.rs | 17 +++---- 11 files changed, 62 insertions(+), 96 deletions(-) delete mode 100644 crates/bevy_render/macros/src/atomic_id.rs diff --git a/crates/bevy_render/macros/src/atomic_id.rs b/crates/bevy_render/macros/src/atomic_id.rs deleted file mode 100644 index a5aaad83f0136..0000000000000 --- a/crates/bevy_render/macros/src/atomic_id.rs +++ /dev/null @@ -1,47 +0,0 @@ -use proc_macro::TokenStream; -use quote::quote; -use syn::parse::{Parse, ParseStream}; -use syn::{parse_macro_input, Ident, Visibility}; - -pub struct AtomicIdStruct { - pub vis: Visibility, - pub ident: Ident, -} - -impl Parse for AtomicIdStruct { - fn parse(input: ParseStream) -> syn::Result { - Ok(Self { - vis: input.parse()?, - ident: input.parse()?, - }) - } -} - -pub fn define_atomic_id(input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as AtomicIdStruct); - - let struct_name = &ast.ident; - let visibility = &ast.vis; - let error_message = format!("The system ran out of unique `{}`s.", struct_name); - - TokenStream::from(quote! { - - #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] - #visibility struct #struct_name(crate::render_resource::resource_macros::AtomicIdType); - - impl #struct_name { - pub fn new() -> Self { - use crate::render_resource::resource_macros::AtomicIdCounter; - use std::sync::atomic::Ordering; - - static COUNTER: AtomicIdCounter = AtomicIdCounter::new(0); - COUNTER - .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| { - val.checked_add(1) - }) - .map(Self) - .expect(#error_message) - } - } - }) -} diff --git a/crates/bevy_render/macros/src/lib.rs b/crates/bevy_render/macros/src/lib.rs index 8dc2d6cf8c249..863b48baf5fa0 100644 --- a/crates/bevy_render/macros/src/lib.rs +++ b/crates/bevy_render/macros/src/lib.rs @@ -1,5 +1,4 @@ mod as_bind_group; -mod atomic_id; mod extract_resource; use bevy_macro_utils::BevyManifest; @@ -24,8 +23,3 @@ pub fn derive_as_bind_group(input: TokenStream) -> TokenStream { as_bind_group::derive_as_bind_group(input).unwrap_or_else(|err| err.to_compile_error().into()) } - -#[proc_macro] -pub fn define_atomic_id(input: TokenStream) -> TokenStream { - atomic_id::define_atomic_id(input) -} diff --git a/crates/bevy_render/src/render_graph/graph.rs b/crates/bevy_render/src/render_graph/graph.rs index e034f345f4e67..8e65db0544fd3 100644 --- a/crates/bevy_render/src/render_graph/graph.rs +++ b/crates/bevy_render/src/render_graph/graph.rs @@ -110,7 +110,7 @@ impl RenderGraph { where T: Node, { - let id = NodeId::new(); + let id = NodeId::default(); let name = name.into(); let mut node_state = NodeState::new(id, node); node_state.name = Some(name.clone()); diff --git a/crates/bevy_render/src/render_graph/node.rs b/crates/bevy_render/src/render_graph/node.rs index 198d8286ccee7..9e3e884ba8b08 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,12 +7,11 @@ use crate::{ renderer::RenderContext, }; use bevy_ecs::world::World; -use bevy_render_macros::define_atomic_id; use downcast_rs::{impl_downcast, Downcast}; use std::{borrow::Cow, fmt::Debug}; use thiserror::Error; -define_atomic_id!(pub NodeId); +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 401e6b6dea366..77ce20db0838f 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -1,16 +1,18 @@ use crate::{ + define_atomic_id, prelude::Image, render_asset::RenderAssets, render_resource::{resource_macros::*, BindGroupLayout, Buffer, Sampler, TextureView}, renderer::RenderDevice, texture::FallbackImage, }; -pub use bevy_render_macros::{define_atomic_id, AsBindGroup}; +pub use bevy_render_macros::AsBindGroup; +use bevy_utils::default; use encase::ShaderType; use std::ops::Deref; use wgpu::BindingResource; -define_atomic_id!(pub BindGroupId); +define_atomic_id!(BindGroupId); render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup); /// Bind groups are responsible for binding render resources (e.g. buffers, textures, samplers) @@ -36,7 +38,7 @@ impl BindGroup { impl From for BindGroup { fn from(value: wgpu::BindGroup) -> Self { BindGroup { - id: BindGroupId::new(), + id: default(), 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 722bc91a7585a..8835adde50633 100644 --- a/crates/bevy_render/src/render_resource/bind_group_layout.rs +++ b/crates/bevy_render/src/render_resource/bind_group_layout.rs @@ -1,8 +1,8 @@ -use crate::render_resource::resource_macros::*; -use bevy_render_macros::define_atomic_id; +use crate::{define_atomic_id, render_resource::resource_macros::*}; +use bevy_utils::default; use std::ops::Deref; -define_atomic_id!(pub BindGroupLayoutId); +define_atomic_id!(BindGroupLayoutId); render_resource_wrapper!(ErasedBindGroupLayout, wgpu::BindGroupLayout); #[derive(Clone, Debug)] @@ -32,7 +32,7 @@ impl BindGroupLayout { impl From for BindGroupLayout { fn from(value: wgpu::BindGroupLayout) -> Self { BindGroupLayout { - id: BindGroupLayoutId::new(), + id: default(), 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 c86de6d0978a4..65b5289fdf938 100644 --- a/crates/bevy_render/src/render_resource/buffer.rs +++ b/crates/bevy_render/src/render_resource/buffer.rs @@ -1,9 +1,8 @@ -use bevy_render_macros::define_atomic_id; +use crate::{define_atomic_id, render_resource::resource_macros::render_resource_wrapper}; +use bevy_utils::default; use std::ops::{Bound, Deref, RangeBounds}; -use crate::render_resource::resource_macros::*; - -define_atomic_id!(pub BufferId); +define_atomic_id!(BufferId); render_resource_wrapper!(ErasedBuffer, wgpu::Buffer); #[derive(Clone, Debug)] @@ -40,7 +39,7 @@ impl Buffer { impl From for Buffer { fn from(value: wgpu::Buffer) -> Self { Buffer { - id: BufferId::new(), + id: default(), 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 e6ce173474170..eeedeab636e8e 100644 --- a/crates/bevy_render/src/render_resource/pipeline.rs +++ b/crates/bevy_render/src/render_resource/pipeline.rs @@ -1,16 +1,17 @@ -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_render_macros::define_atomic_id; +use bevy_utils::default; 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::*; - -define_atomic_id!(pub RenderPipelineId); +define_atomic_id!(RenderPipelineId); render_resource_wrapper!(ErasedRenderPipeline, wgpu::RenderPipeline); /// A [`RenderPipeline`] represents a graphics pipeline and its stages (shaders), bindings and vertex buffers. @@ -33,7 +34,7 @@ impl RenderPipeline { impl From for RenderPipeline { fn from(value: wgpu::RenderPipeline) -> Self { RenderPipeline { - id: RenderPipelineId::new(), + id: default(), value: ErasedRenderPipeline::new(value), } } @@ -48,7 +49,7 @@ impl Deref for RenderPipeline { } } -define_atomic_id!(pub ComputePipelineId); +define_atomic_id!(ComputePipelineId); render_resource_wrapper!(ErasedComputePipeline, wgpu::ComputePipeline); /// A [`ComputePipeline`] represents a compute pipeline and its single shader stage. @@ -72,7 +73,7 @@ impl ComputePipeline { impl From for ComputePipeline { fn from(value: wgpu::ComputePipeline) -> Self { ComputePipeline { - id: ComputePipelineId::new(), + id: default(), 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 19b6994807a8a..5706225325929 100644 --- a/crates/bevy_render/src/render_resource/resource_macros.rs +++ b/crates/bevy_render/src/render_resource/resource_macros.rs @@ -1,12 +1,3 @@ -#[cfg(target_has_atomic = "64")] -pub type AtomicIdCounter = std::sync::atomic::AtomicU64; -#[cfg(target_has_atomic = "64")] -pub type AtomicIdType = u64; -#[cfg(not(target_has_atomic = "64"))] -pub type AtomicIdCounter = std::sync::atomic::AtomicUsize; -#[cfg(not(target_has_atomic = "64"))] -pub type AtomicIdType = usize; - // structs containing wgpu types take a long time to compile. this is particularly bad for generic // structs containing wgpu structs. we avoid that in debug builds (and for cargo check and rust analyzer) // by boxing and type-erasing with the `render_resource_wrapper` macro. @@ -129,4 +120,29 @@ 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); + + impl Default for $atomic_id_type { + fn default() -> Self { + use std::sync::atomic::{AtomicU32, Ordering}; + + static COUNTER: AtomicU32 = AtomicU32::new(0); + COUNTER + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| { + val.checked_add(1) + }) + .map(Self) + .expect(&format!( + "The system ran out of unique `{}`s.", + stringify!($atomic_id_type) + )) + } + } + }; +} + 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 3a5382d78e5fa..08f01f64fa81c 100644 --- a/crates/bevy_render/src/render_resource/shader.rs +++ b/crates/bevy_render/src/render_resource/shader.rs @@ -1,7 +1,7 @@ use super::ShaderDefVal; +use crate::define_atomic_id; use bevy_asset::{AssetLoader, AssetPath, Handle, LoadContext, LoadedAsset}; use bevy_reflect::TypeUuid; -use bevy_render_macros::define_atomic_id; use bevy_utils::{tracing::error, BoxedFuture, HashMap}; use naga::{back::wgsl::WriterFlags, valid::Capabilities, valid::ModuleInfo, Module}; use once_cell::sync::Lazy; @@ -10,7 +10,7 @@ use std::{borrow::Cow, marker::Copy, ops::Deref, path::PathBuf, str::FromStr}; use thiserror::Error; use wgpu::{util::make_spirv, Features, ShaderModuleDescriptor, ShaderSource}; -define_atomic_id!(pub ShaderId); +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 c1551a1defbc0..4fce96b8cd5a2 100644 --- a/crates/bevy_render/src/render_resource/texture.rs +++ b/crates/bevy_render/src/render_resource/texture.rs @@ -1,9 +1,10 @@ -use bevy_render_macros::define_atomic_id; +use crate::define_atomic_id; +use bevy_utils::default; use std::ops::Deref; use crate::render_resource::resource_macros::*; -define_atomic_id!(pub TextureId); +define_atomic_id!(TextureId); render_resource_wrapper!(ErasedTexture, wgpu::Texture); /// A GPU-accessible texture. @@ -32,7 +33,7 @@ impl Texture { impl From for Texture { fn from(value: wgpu::Texture) -> Self { Texture { - id: TextureId::new(), + id: default(), value: ErasedTexture::new(value), } } @@ -47,7 +48,7 @@ impl Deref for Texture { } } -define_atomic_id!(pub TextureViewId); +define_atomic_id!(TextureViewId); render_resource_wrapper!(ErasedTextureView, wgpu::TextureView); render_resource_wrapper!(ErasedSurfaceTexture, wgpu::SurfaceTexture); @@ -98,7 +99,7 @@ impl TextureView { impl From for TextureView { fn from(value: wgpu::TextureView) -> Self { TextureView { - id: TextureViewId::new(), + id: default(), value: TextureViewValue::TextureView(ErasedTextureView::new(value)), } } @@ -110,7 +111,7 @@ impl From for TextureView { let texture = ErasedSurfaceTexture::new(value); TextureView { - id: TextureViewId::new(), + id: default(), value: TextureViewValue::SurfaceTexture { texture, view }, } } @@ -128,7 +129,7 @@ impl Deref for TextureView { } } -define_atomic_id!(pub SamplerId); +define_atomic_id!(SamplerId); render_resource_wrapper!(ErasedSampler, wgpu::Sampler); /// A Sampler defines how a pipeline will sample from a [`TextureView`]. @@ -153,7 +154,7 @@ impl Sampler { impl From for Sampler { fn from(value: wgpu::Sampler) -> Self { Sampler { - id: SamplerId::new(), + id: default(), value: ErasedSampler::new(value), } } From 6bd5a0ecd43feb4f311f7991f5f5d774c4dd170c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurt=20K=C3=BChnert?= Date: Wed, 21 Dec 2022 23:33:31 +0100 Subject: [PATCH 3/4] improved the define_atomic_id macro --- .../src/render_resource/resource_macros.rs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/bevy_render/src/render_resource/resource_macros.rs b/crates/bevy_render/src/render_resource/resource_macros.rs index 5706225325929..3e306ca8d6704 100644 --- a/crates/bevy_render/src/render_resource/resource_macros.rs +++ b/crates/bevy_render/src/render_resource/resource_macros.rs @@ -130,16 +130,17 @@ macro_rules! define_atomic_id { fn default() -> Self { use std::sync::atomic::{AtomicU32, Ordering}; - static COUNTER: AtomicU32 = AtomicU32::new(0); - COUNTER - .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| { - val.checked_add(1) - }) - .map(Self) - .expect(&format!( - "The system ran out of unique `{}`s.", - stringify!($atomic_id_type) - )) + 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), + } } } }; From 3217bf9fccba834fe24d3137fbf9c20e8c02588e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kurt=20K=C3=BChnert?= Date: Thu, 22 Dec 2022 17:06:00 +0100 Subject: [PATCH 4/4] changed back from default to new and disabled the lint --- crates/bevy_render/src/render_graph/graph.rs | 2 +- crates/bevy_render/src/render_resource/bind_group.rs | 3 +-- .../bevy_render/src/render_resource/bind_group_layout.rs | 3 +-- crates/bevy_render/src/render_resource/buffer.rs | 3 +-- crates/bevy_render/src/render_resource/pipeline.rs | 5 ++--- .../bevy_render/src/render_resource/resource_macros.rs | 6 ++++-- crates/bevy_render/src/render_resource/texture.rs | 9 ++++----- 7 files changed, 14 insertions(+), 17 deletions(-) diff --git a/crates/bevy_render/src/render_graph/graph.rs b/crates/bevy_render/src/render_graph/graph.rs index 8e65db0544fd3..e034f345f4e67 100644 --- a/crates/bevy_render/src/render_graph/graph.rs +++ b/crates/bevy_render/src/render_graph/graph.rs @@ -110,7 +110,7 @@ impl RenderGraph { where T: Node, { - let id = NodeId::default(); + let id = NodeId::new(); let name = name.into(); let mut node_state = NodeState::new(id, node); node_state.name = Some(name.clone()); diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index 77ce20db0838f..5bc13b1d1f47a 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -7,7 +7,6 @@ use crate::{ texture::FallbackImage, }; pub use bevy_render_macros::AsBindGroup; -use bevy_utils::default; use encase::ShaderType; use std::ops::Deref; use wgpu::BindingResource; @@ -38,7 +37,7 @@ impl BindGroup { impl From for BindGroup { fn from(value: wgpu::BindGroup) -> Self { BindGroup { - id: default(), + 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 8835adde50633..9793f1391d188 100644 --- a/crates/bevy_render/src/render_resource/bind_group_layout.rs +++ b/crates/bevy_render/src/render_resource/bind_group_layout.rs @@ -1,5 +1,4 @@ use crate::{define_atomic_id, render_resource::resource_macros::*}; -use bevy_utils::default; use std::ops::Deref; define_atomic_id!(BindGroupLayoutId); @@ -32,7 +31,7 @@ impl BindGroupLayout { impl From for BindGroupLayout { fn from(value: wgpu::BindGroupLayout) -> Self { BindGroupLayout { - id: default(), + 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 65b5289fdf938..9867945673efb 100644 --- a/crates/bevy_render/src/render_resource/buffer.rs +++ b/crates/bevy_render/src/render_resource/buffer.rs @@ -1,5 +1,4 @@ use crate::{define_atomic_id, render_resource::resource_macros::render_resource_wrapper}; -use bevy_utils::default; use std::ops::{Bound, Deref, RangeBounds}; define_atomic_id!(BufferId); @@ -39,7 +38,7 @@ impl Buffer { impl From for Buffer { fn from(value: wgpu::Buffer) -> Self { Buffer { - id: default(), + 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 eeedeab636e8e..edd3d8f6d67cb 100644 --- a/crates/bevy_render/src/render_resource/pipeline.rs +++ b/crates/bevy_render/src/render_resource/pipeline.rs @@ -4,7 +4,6 @@ use crate::{ render_resource::{resource_macros::render_resource_wrapper, BindGroupLayout, Shader}, }; use bevy_asset::Handle; -use bevy_utils::default; use std::{borrow::Cow, ops::Deref}; use wgpu::{ BufferAddress, ColorTargetState, DepthStencilState, MultisampleState, PrimitiveState, @@ -34,7 +33,7 @@ impl RenderPipeline { impl From for RenderPipeline { fn from(value: wgpu::RenderPipeline) -> Self { RenderPipeline { - id: default(), + id: RenderPipelineId::new(), value: ErasedRenderPipeline::new(value), } } @@ -73,7 +72,7 @@ impl ComputePipeline { impl From for ComputePipeline { fn from(value: wgpu::ComputePipeline) -> Self { ComputePipeline { - id: default(), + 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 3e306ca8d6704..e27380426a169 100644 --- a/crates/bevy_render/src/render_resource/resource_macros.rs +++ b/crates/bevy_render/src/render_resource/resource_macros.rs @@ -126,8 +126,10 @@ macro_rules! define_atomic_id { #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] pub struct $atomic_id_type(u32); - impl Default for $atomic_id_type { - fn default() -> Self { + // 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); diff --git a/crates/bevy_render/src/render_resource/texture.rs b/crates/bevy_render/src/render_resource/texture.rs index 4fce96b8cd5a2..02cc818408073 100644 --- a/crates/bevy_render/src/render_resource/texture.rs +++ b/crates/bevy_render/src/render_resource/texture.rs @@ -1,5 +1,4 @@ use crate::define_atomic_id; -use bevy_utils::default; use std::ops::Deref; use crate::render_resource::resource_macros::*; @@ -33,7 +32,7 @@ impl Texture { impl From for Texture { fn from(value: wgpu::Texture) -> Self { Texture { - id: default(), + id: TextureId::new(), value: ErasedTexture::new(value), } } @@ -99,7 +98,7 @@ impl TextureView { impl From for TextureView { fn from(value: wgpu::TextureView) -> Self { TextureView { - id: default(), + id: TextureViewId::new(), value: TextureViewValue::TextureView(ErasedTextureView::new(value)), } } @@ -111,7 +110,7 @@ impl From for TextureView { let texture = ErasedSurfaceTexture::new(value); TextureView { - id: default(), + id: TextureViewId::new(), value: TextureViewValue::SurfaceTexture { texture, view }, } } @@ -154,7 +153,7 @@ impl Sampler { impl From for Sampler { fn from(value: wgpu::Sampler) -> Self { Sampler { - id: default(), + id: SamplerId::new(), value: ErasedSampler::new(value), } }