Skip to content

Commit

Permalink
Replace UUID based IDs with a atomic-counted ones (bevyengine#6988)
Browse files Browse the repository at this point in the history
# Objective

- alternative to bevyengine#2895 
- as mentioned in bevyengine#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.
  • Loading branch information
kurtkuehnert authored and ItsDoot committed Feb 1, 2023
1 parent 43d955f commit b0d0a24
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 91 deletions.
20 changes: 2 additions & 18 deletions crates/bevy_render/src/render_graph/node.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,17 @@
use crate::{
define_atomic_id,
render_graph::{
Edge, InputSlotError, OutputSlotError, RenderGraphContext, RenderGraphError,
RunSubGraphError, SlotInfo, SlotInfos, SlotType, SlotValue,
},
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).
///
Expand Down
17 changes: 6 additions & 11 deletions crates/bevy_render/src/render_resource/bind_group.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -42,7 +37,7 @@ impl BindGroup {
impl From<wgpu::BindGroup> for BindGroup {
fn from(value: wgpu::BindGroup) -> Self {
BindGroup {
id: BindGroupId(Uuid::new_v4()),
id: BindGroupId::new(),
value: ErasedBindGroup::new(value),
}
}
Expand Down
9 changes: 3 additions & 6 deletions crates/bevy_render/src/render_resource/bind_group_layout.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -34,7 +31,7 @@ impl BindGroupLayout {
impl From<wgpu::BindGroupLayout> for BindGroupLayout {
fn from(value: wgpu::BindGroupLayout) -> Self {
BindGroupLayout {
id: BindGroupLayoutId(Uuid::new_v4()),
id: BindGroupLayoutId::new(),
value: ErasedBindGroupLayout::new(value),
}
}
Expand Down
10 changes: 3 additions & 7 deletions crates/bevy_render/src/render_resource/buffer.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -42,7 +38,7 @@ impl Buffer {
impl From<wgpu::Buffer> for Buffer {
fn from(value: wgpu::Buffer) -> Self {
Buffer {
id: BufferId(Uuid::new_v4()),
id: BufferId::new(),
value: ErasedBuffer::new(value),
}
}
Expand Down
24 changes: 9 additions & 15 deletions crates/bevy_render/src/render_resource/pipeline.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -36,7 +33,7 @@ impl RenderPipeline {
impl From<wgpu::RenderPipeline> for RenderPipeline {
fn from(value: wgpu::RenderPipeline) -> Self {
RenderPipeline {
id: RenderPipelineId(Uuid::new_v4()),
id: RenderPipelineId::new(),
value: ErasedRenderPipeline::new(value),
}
}
Expand All @@ -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.
Expand All @@ -78,7 +72,7 @@ impl ComputePipeline {
impl From<wgpu::ComputePipeline> for ComputePipeline {
fn from(value: wgpu::ComputePipeline) -> Self {
ComputePipeline {
id: ComputePipelineId(Uuid::new_v4()),
id: ComputePipelineId::new(),
value: ErasedComputePipeline::new(value),
}
}
Expand Down
28 changes: 28 additions & 0 deletions crates/bevy_render/src/render_resource/resource_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
23 changes: 6 additions & 17 deletions crates/bevy_render/src/render_resource/shader.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
25 changes: 8 additions & 17 deletions crates/bevy_render/src/render_resource/texture.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -35,7 +32,7 @@ impl Texture {
impl From<wgpu::Texture> for Texture {
fn from(value: wgpu::Texture) -> Self {
Texture {
id: TextureId(Uuid::new_v4()),
id: TextureId::new(),
value: ErasedTexture::new(value),
}
}
Expand All @@ -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);

Expand Down Expand Up @@ -104,7 +98,7 @@ impl TextureView {
impl From<wgpu::TextureView> for TextureView {
fn from(value: wgpu::TextureView) -> Self {
TextureView {
id: TextureViewId(Uuid::new_v4()),
id: TextureViewId::new(),
value: TextureViewValue::TextureView(ErasedTextureView::new(value)),
}
}
Expand All @@ -116,7 +110,7 @@ impl From<wgpu::SurfaceTexture> for TextureView {
let texture = ErasedSurfaceTexture::new(value);

TextureView {
id: TextureViewId(Uuid::new_v4()),
id: TextureViewId::new(),
value: TextureViewValue::SurfaceTexture { texture, view },
}
}
Expand All @@ -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`].
Expand All @@ -162,7 +153,7 @@ impl Sampler {
impl From<wgpu::Sampler> for Sampler {
fn from(value: wgpu::Sampler) -> Self {
Sampler {
id: SamplerId(Uuid::new_v4()),
id: SamplerId::new(),
value: ErasedSampler::new(value),
}
}
Expand Down

0 comments on commit b0d0a24

Please sign in to comment.