Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Atomic counter for render resource ids and Render to Gpu rename #2895

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions crates/bevy_derive/src/id.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use proc_macro::TokenStream;
use quote::quote;
use syn::{parse_macro_input, DeriveInput};

pub fn derive_id(input: TokenStream) -> TokenStream {
let ast = parse_macro_input!(input as DeriveInput);

let struct_name = &ast.ident;
let error_message = format!("The system ran out of unique `{}`s.", struct_name);
let (_impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();

TokenStream::from(quote! {
impl #struct_name #type_generics #where_clause {
pub fn new() -> Self {
use std::sync::atomic::{AtomicU64, Ordering};
static COUNTER: AtomicU64 = AtomicU64::new(0);
COUNTER
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| {
val.checked_add(1)
})
.map(Self)
.expect(#error_message)
}
}
})
}
6 changes: 6 additions & 0 deletions crates/bevy_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod app_plugin;
mod bevy_main;
mod bytes;
mod enum_variant_meta;
mod id;
mod modules;
mod render_resource;
mod render_resources;
Expand Down Expand Up @@ -70,3 +71,8 @@ pub fn derive_app_label(input: TokenStream) -> TokenStream {
trait_path.segments.push(format_ident!("AppLabel").into());
derive_label(input, trait_path)
}

#[proc_macro_derive(Id)]
pub fn derive_id(input: TokenStream) -> TokenStream {
id::derive_id(input)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it a regular macro? Something like define_id!(pub SystemId) which defines the struct and add all appropriate derives.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point! But this makes it also harder to extend those types, right? And a derive trait seems like the more idiomatic choice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not really anything that can or should be extended about those ids. Using more than one field or a non-integer field doesn't make sense for an id. Also you can still use impl SystemId {} of you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that makes sense. I will try it :)
Thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mhh I am not sure whether defining a new type inside a macro is even possible, because my generated struct is not visible across files and thus can't be imported. Any ideas on how to make the compiler know this struct before expanding the macro?

define_id!(pub MyId);


pub struct IdStruct {
    pub vis: Visibility,
    pub ident: Ident,
}

impl Parse for IdStruct {
    fn parse(input: ParseStream) -> syn::Result<Self> {
        Ok(Self {
            vis: input.parse()?,
            ident: input.parse()?,
        })
    }
}

pub fn define_id(input: TokenStream) -> TokenStream {
    let ast = parse_macro_input!(input as ItemStruct);

    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(u64);

        impl #struct_name {
            pub fn new() -> Self {
                use std::sync::atomic::{AtomicU64, Ordering};
                static COUNTER: AtomicU64 = AtomicU64::new(0);
                COUNTER
                    .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| {
                        val.checked_add(1)
                    })
                    .map(Self)
                    .expect(#error_message)
            }
        }
    })
}

14 changes: 3 additions & 11 deletions crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use bevy_utils::tracing::warn;
use bevy_utils::{tracing::warn, Id, IdType};

use crate::{
archetype::{Archetype, ArchetypeComponentId},
Expand All @@ -9,16 +9,8 @@ use crate::{
use std::borrow::Cow;

/// A [`System`] identifier.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub struct SystemId(pub usize);

impl SystemId {
/// Creates a new random `SystemId`.
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
SystemId(rand::random::<usize>())
}
}
#[derive(Id, Copy, Clone, Hash, Eq, PartialEq, Debug)]
pub struct SystemId(IdType);

/// An ECS system that can be added to a [Schedule](crate::schedule::Schedule)
///
Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,19 @@ use crate::{
query::{FilterFetch, QueryState, WorldQuery},
storage::{Column, SparseSet, Storages},
};
use bevy_utils::{Id, IdType};
use std::{
any::TypeId,
fmt,
sync::atomic::{AtomicU32, Ordering},
};

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub struct WorldId(u64);
#[derive(Id, Copy, Clone, Hash, Eq, PartialEq, Debug)]
pub struct WorldId(IdType);

impl Default for WorldId {
fn default() -> Self {
WorldId(rand::random())
WorldId::new()
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_utils/src/id.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub use bevy_derive::Id;
pub type IdType = u64;
2 changes: 2 additions & 0 deletions crates/bevy_utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
mod enum_variant_meta;
mod id;
pub mod label;
pub mod slab;

pub use ahash::AHasher;
pub use enum_variant_meta::*;
pub use id::*;
pub use instant::{Duration, Instant};
pub use tracing;
pub use uuid::Uuid;
Expand Down
14 changes: 5 additions & 9 deletions crates/bevy_window/src/window.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
use bevy_math::{IVec2, Vec2};
use bevy_utils::{tracing::warn, Uuid};
use bevy_utils::{tracing::warn, Id, IdType};
use raw_window_handle::RawWindowHandle;

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct WindowId(Uuid);
#[derive(Id, Copy, Clone, Hash, Eq, PartialEq, Debug)]
pub struct WindowId(pub IdType);

impl WindowId {
pub fn new() -> Self {
WindowId(Uuid::new_v4())
}

pub fn primary() -> Self {
WindowId(Uuid::from_u128(0))
WindowId(0)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is problematic, because the first WindowId generated via new() will be the primary window.
Should we really encode this information into the id itself anyway?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use !0 as primary window id?


pub fn is_primary(&self) -> bool {
Expand All @@ -25,7 +21,7 @@ use crate::raw_window_handle::RawWindowHandleWrapper;

impl fmt::Display for WindowId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.to_simple().fmt(f)
self.0.fmt(f)
}
}

Expand Down
16 changes: 8 additions & 8 deletions examples/shader/custom_shader_pipelined.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use bevy::{
AddRenderCommand, DrawFunctions, RenderCommand, RenderPhase, TrackedRenderPass,
},
render_resource::*,
renderer::RenderDevice,
renderer::GpuDevice,
shader::Shader,
texture::BevyDefault,
view::ExtractedView,
Expand All @@ -44,7 +44,7 @@ pub struct GpuCustomMaterial {
impl RenderAsset for CustomMaterial {
type ExtractedAsset = CustomMaterial;
type PreparedAsset = GpuCustomMaterial;
type Param = (SRes<RenderDevice>, SRes<CustomPipeline>);
type Param = (SRes<GpuDevice>, SRes<CustomPipeline>);
fn extract_asset(&self) -> Self::ExtractedAsset {
self.clone()
}
Expand Down Expand Up @@ -129,11 +129,11 @@ pub struct CustomPipeline {
// TODO: this pattern for initializing the shaders / pipeline isn't ideal. this should be handled by the asset system
impl FromWorld for CustomPipeline {
fn from_world(world: &mut World) -> Self {
let render_device = world.get_resource::<RenderDevice>().unwrap();
let gpu_device = world.get_resource::<GpuDevice>().unwrap();
let shader = Shader::from_wgsl(include_str!("../../assets/shaders/custom.wgsl"));
let shader_module = render_device.create_shader_module(&shader);
let shader_module = gpu_device.create_shader_module(&shader);

let material_layout = render_device.create_bind_group_layout(&BindGroupLayoutDescriptor {
let material_layout = gpu_device.create_bind_group_layout(&BindGroupLayoutDescriptor {
entries: &[BindGroupLayoutEntry {
binding: 0,
visibility: ShaderStage::FRAGMENT,
Expand All @@ -148,7 +148,7 @@ impl FromWorld for CustomPipeline {
});
let pbr_pipeline = world.get_resource::<PbrShaders>().unwrap();

let pipeline_layout = render_device.create_pipeline_layout(&PipelineLayoutDescriptor {
let pipeline_layout = gpu_device.create_pipeline_layout(&PipelineLayoutDescriptor {
label: None,
push_constant_ranges: &[],
bind_group_layouts: &[
Expand All @@ -158,7 +158,7 @@ impl FromWorld for CustomPipeline {
],
});

let pipeline = render_device.create_render_pipeline(&RenderPipelineDescriptor {
let pipeline = gpu_device.create_render_pipeline(&RenderPipelineDescriptor {
label: None,
vertex: VertexState {
buffers: &[VertexBufferLayout {
Expand Down Expand Up @@ -238,8 +238,8 @@ impl FromWorld for CustomPipeline {
});

CustomPipeline {
pipeline,
material_layout,
pipeline,
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pipelined/bevy_core_pipeline/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use bevy_render2::{
render_graph::{EmptyNode, RenderGraph, SlotInfo, SlotType},
render_phase::{sort_phase_system, DrawFunctionId, DrawFunctions, PhaseItem, RenderPhase},
render_resource::*,
renderer::RenderDevice,
renderer::GpuDevice,
texture::{Image, TextureCache},
view::ExtractedView,
RenderApp, RenderStage, RenderWorld,
Expand Down Expand Up @@ -228,12 +228,12 @@ pub fn extract_core_pipeline_camera_phases(
pub fn prepare_core_views_system(
mut commands: Commands,
mut texture_cache: ResMut<TextureCache>,
render_device: Res<RenderDevice>,
gpu_device: Res<GpuDevice>,
views: Query<(Entity, &ExtractedView), With<RenderPhase<Transparent3d>>>,
) {
for (entity, view) in views.iter() {
let cached_texture = texture_cache.get(
&render_device,
&gpu_device,
TextureDescriptor {
label: None,
size: Extent3d {
Expand Down
6 changes: 3 additions & 3 deletions pipelined/bevy_core_pipeline/src/main_pass_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use bevy_render2::{
render_graph::{Node, NodeRunError, RenderGraphContext, SlotInfo, SlotType},
render_phase::{DrawFunctions, RenderPhase, TrackedRenderPass},
render_resource::{LoadOp, Operations, RenderPassColorAttachment, RenderPassDescriptor},
renderer::RenderContext,
renderer::GpuContext,
view::ExtractedView,
};

Expand Down Expand Up @@ -38,7 +38,7 @@ impl Node for MainPass2dNode {
fn run(
&self,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext,
gpu_context: &mut GpuContext,
world: &World,
) -> Result<(), NodeRunError> {
let color_attachment_texture = graph.get_input_texture(Self::IN_COLOR_ATTACHMENT)?;
Expand Down Expand Up @@ -66,7 +66,7 @@ impl Node for MainPass2dNode {
.get_manual(world, view_entity)
.expect("view entity should exist");

let render_pass = render_context
let render_pass = gpu_context
.command_encoder
.begin_render_pass(&pass_descriptor);

Expand Down
6 changes: 3 additions & 3 deletions pipelined/bevy_core_pipeline/src/main_pass_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bevy_render2::{
LoadOp, Operations, RenderPassColorAttachment, RenderPassDepthStencilAttachment,
RenderPassDescriptor,
},
renderer::RenderContext,
renderer::GpuContext,
view::ExtractedView,
};

Expand Down Expand Up @@ -43,7 +43,7 @@ impl Node for MainPass3dNode {
fn run(
&self,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext,
gpu_context: &mut GpuContext,
world: &World,
) -> Result<(), NodeRunError> {
let color_attachment_texture = graph.get_input_texture(Self::IN_COLOR_ATTACHMENT)?;
Expand Down Expand Up @@ -79,7 +79,7 @@ impl Node for MainPass3dNode {
.get_manual(world, view_entity)
.expect("view entity should exist");

let render_pass = render_context
let render_pass = gpu_context
.command_encoder
.begin_render_pass(&pass_descriptor);
let mut draw_functions = draw_functions.write();
Expand Down
4 changes: 2 additions & 2 deletions pipelined/bevy_core_pipeline/src/main_pass_driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use bevy_ecs::world::World;
use bevy_render2::{
camera::{CameraPlugin, ExtractedCamera, ExtractedCameraNames},
render_graph::{Node, NodeRunError, RenderGraphContext, SlotValue},
renderer::RenderContext,
renderer::GpuContext,
view::ExtractedWindows,
};

Expand All @@ -13,7 +13,7 @@ impl Node for MainPassDriverNode {
fn run(
&self,
graph: &mut RenderGraphContext,
_render_context: &mut RenderContext,
_gpu_context: &mut GpuContext,
world: &World,
) -> Result<(), NodeRunError> {
let extracted_cameras = world.get_resource::<ExtractedCameraNames>().unwrap();
Expand Down
8 changes: 2 additions & 6 deletions pipelined/bevy_pbr2/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bevy_render2::{
color::Color,
render_asset::{PrepareAssetError, RenderAsset, RenderAssetPlugin, RenderAssets},
render_resource::{BindGroup, Buffer, BufferInitDescriptor, BufferUsage, Sampler, TextureView},
renderer::RenderDevice,
renderer::GpuDevice,
texture::Image,
};
use crevice::std140::{AsStd140, Std140};
Expand Down Expand Up @@ -144,11 +144,7 @@ pub struct GpuStandardMaterial {
impl RenderAsset for StandardMaterial {
type ExtractedAsset = StandardMaterial;
type PreparedAsset = GpuStandardMaterial;
type Param = (
SRes<RenderDevice>,
SRes<PbrShaders>,
SRes<RenderAssets<Image>>,
);
type Param = (SRes<GpuDevice>, SRes<PbrShaders>, SRes<RenderAssets<Image>>);

fn extract_asset(&self) -> Self::ExtractedAsset {
self.clone()
Expand Down
Loading