Remove render_resource_wrapper (#15441)

# Objective

* Remove all uses of render_resource_wrapper.
* Make it easier to share a `wgpu::Device` between Bevy and application
code.

## Solution

Removed the `render_resource_wrapper` macro.

To improve the `RenderCreation:: Manual ` API, `ErasedRenderDevice` was
replaced by `Arc`. Unfortunately I had to introduce one more usage of
`WgpuWrapper` which seems like an unwanted constraint on the caller.

## Testing

- Did you test these changes? If so, how?
    - Ran `cargo test`.
    - Ran a few examples.
    - Used `RenderCreation::Manual` in my own project
    - Exercised `RenderCreation::Automatic` through examples

- Are there any parts that need more testing?
    - No

- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
    - Run examples
    - Use `RenderCreation::Manual` in their own project
This commit is contained in:
Erik Živković 2024-09-30 19:37:07 +02:00 committed by GitHub
parent f97eba2082
commit 72aaa41603
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 62 additions and 207 deletions

View File

@ -1,10 +1,12 @@
use crate::renderer::WgpuWrapper;
use crate::{
define_atomic_id,
render_asset::RenderAssets,
render_resource::{resource_macros::*, BindGroupLayout, Buffer, Sampler, TextureView},
render_resource::{BindGroupLayout, Buffer, Sampler, TextureView},
renderer::RenderDevice,
texture::GpuImage,
};
use alloc::sync::Arc;
use bevy_ecs::system::{SystemParam, SystemParamItem};
pub use bevy_render_macros::AsBindGroup;
use core::ops::Deref;
@ -13,7 +15,6 @@ use thiserror::Error;
use wgpu::{BindGroupEntry, BindGroupLayoutEntry, BindingResource};
define_atomic_id!(BindGroupId);
render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup);
/// Bind groups are responsible for binding render resources (e.g. buffers, textures, samplers)
/// to a [`TrackedRenderPass`](crate::render_phase::TrackedRenderPass).
@ -24,7 +25,7 @@ render_resource_wrapper!(ErasedBindGroup, wgpu::BindGroup);
#[derive(Clone, Debug)]
pub struct BindGroup {
id: BindGroupId,
value: ErasedBindGroup,
value: Arc<WgpuWrapper<wgpu::BindGroup>>,
}
impl BindGroup {
@ -39,7 +40,7 @@ impl From<wgpu::BindGroup> for BindGroup {
fn from(value: wgpu::BindGroup) -> Self {
BindGroup {
id: BindGroupId::new(),
value: ErasedBindGroup::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}

View File

@ -1,13 +1,14 @@
use crate::{define_atomic_id, render_resource::resource_macros::*};
use crate::define_atomic_id;
use crate::renderer::WgpuWrapper;
use alloc::sync::Arc;
use core::ops::Deref;
define_atomic_id!(BindGroupLayoutId);
render_resource_wrapper!(ErasedBindGroupLayout, wgpu::BindGroupLayout);
#[derive(Clone, Debug)]
pub struct BindGroupLayout {
id: BindGroupLayoutId,
value: ErasedBindGroupLayout,
value: Arc<WgpuWrapper<wgpu::BindGroupLayout>>,
}
impl PartialEq for BindGroupLayout {
@ -32,7 +33,7 @@ impl From<wgpu::BindGroupLayout> for BindGroupLayout {
fn from(value: wgpu::BindGroupLayout) -> Self {
BindGroupLayout {
id: BindGroupLayoutId::new(),
value: ErasedBindGroupLayout::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}

View File

@ -1,13 +1,14 @@
use crate::{define_atomic_id, render_resource::resource_macros::render_resource_wrapper};
use crate::define_atomic_id;
use crate::renderer::WgpuWrapper;
use alloc::sync::Arc;
use core::ops::{Bound, Deref, RangeBounds};
define_atomic_id!(BufferId);
render_resource_wrapper!(ErasedBuffer, wgpu::Buffer);
#[derive(Clone, Debug)]
pub struct Buffer {
id: BufferId,
value: ErasedBuffer,
value: Arc<WgpuWrapper<wgpu::Buffer>>,
size: wgpu::BufferAddress,
}
@ -48,7 +49,7 @@ impl From<wgpu::Buffer> for Buffer {
Buffer {
id: BufferId::new(),
size: value.size(),
value: ErasedBuffer::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}

View File

@ -1,9 +1,11 @@
use super::ShaderDefVal;
use crate::renderer::WgpuWrapper;
use crate::{
define_atomic_id,
render_resource::{resource_macros::render_resource_wrapper, BindGroupLayout, Shader},
render_resource::{BindGroupLayout, Shader},
};
use alloc::borrow::Cow;
use alloc::sync::Arc;
use bevy_asset::Handle;
use core::ops::Deref;
use wgpu::{
@ -12,7 +14,6 @@ use wgpu::{
};
define_atomic_id!(RenderPipelineId);
render_resource_wrapper!(ErasedRenderPipeline, wgpu::RenderPipeline);
/// A [`RenderPipeline`] represents a graphics pipeline and its stages (shaders), bindings and vertex buffers.
///
@ -21,7 +22,7 @@ render_resource_wrapper!(ErasedRenderPipeline, wgpu::RenderPipeline);
#[derive(Clone, Debug)]
pub struct RenderPipeline {
id: RenderPipelineId,
value: ErasedRenderPipeline,
value: Arc<WgpuWrapper<wgpu::RenderPipeline>>,
}
impl RenderPipeline {
@ -35,7 +36,7 @@ impl From<wgpu::RenderPipeline> for RenderPipeline {
fn from(value: wgpu::RenderPipeline) -> Self {
RenderPipeline {
id: RenderPipelineId::new(),
value: ErasedRenderPipeline::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}
@ -50,7 +51,6 @@ impl Deref for RenderPipeline {
}
define_atomic_id!(ComputePipelineId);
render_resource_wrapper!(ErasedComputePipeline, wgpu::ComputePipeline);
/// A [`ComputePipeline`] represents a compute pipeline and its single shader stage.
///
@ -59,7 +59,7 @@ render_resource_wrapper!(ErasedComputePipeline, wgpu::ComputePipeline);
#[derive(Clone, Debug)]
pub struct ComputePipeline {
id: ComputePipelineId,
value: ErasedComputePipeline,
value: Arc<WgpuWrapper<wgpu::ComputePipeline>>,
}
impl ComputePipeline {
@ -74,7 +74,7 @@ impl From<wgpu::ComputePipeline> for ComputePipeline {
fn from(value: wgpu::ComputePipeline) -> Self {
ComputePipeline {
id: ComputePipelineId::new(),
value: ErasedComputePipeline::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}

View File

@ -1,3 +1,4 @@
use crate::renderer::WgpuWrapper;
use crate::{
render_resource::*,
renderer::{RenderAdapter, RenderDevice},
@ -27,11 +28,6 @@ use wgpu::{
VertexBufferLayout as RawVertexBufferLayout,
};
use crate::render_resource::resource_macros::*;
render_resource_wrapper!(ErasedShaderModule, ShaderModule);
render_resource_wrapper!(ErasedPipelineLayout, PipelineLayout);
/// A descriptor for a [`Pipeline`].
///
/// Used to store an heterogenous collection of render and compute pipeline descriptors together.
@ -126,7 +122,7 @@ impl CachedPipelineState {
#[derive(Default)]
struct ShaderData {
pipelines: HashSet<CachedPipelineId>,
processed_shaders: HashMap<Box<[ShaderDefVal]>, ErasedShaderModule>,
processed_shaders: HashMap<Box<[ShaderDefVal]>, Arc<WgpuWrapper<ShaderModule>>>,
resolved_imports: HashMap<ShaderImport, AssetId<Shader>>,
dependents: HashSet<AssetId<Shader>>,
}
@ -225,7 +221,7 @@ impl ShaderCache {
pipeline: CachedPipelineId,
id: AssetId<Shader>,
shader_defs: &[ShaderDefVal],
) -> Result<ErasedShaderModule, PipelineCacheError> {
) -> Result<Arc<WgpuWrapper<ShaderModule>>, PipelineCacheError> {
let shader = self
.shaders
.get(&id)
@ -338,7 +334,7 @@ impl ShaderCache {
return Err(PipelineCacheError::CreateShaderModule(description));
}
entry.insert(ErasedShaderModule::new(shader_module))
entry.insert(Arc::new(WgpuWrapper::new(shader_module)))
}
};
@ -410,7 +406,7 @@ impl ShaderCache {
type LayoutCacheKey = (Vec<BindGroupLayoutId>, Vec<PushConstantRange>);
#[derive(Default)]
struct LayoutCache {
layouts: HashMap<LayoutCacheKey, ErasedPipelineLayout>,
layouts: HashMap<LayoutCacheKey, Arc<WgpuWrapper<PipelineLayout>>>,
}
impl LayoutCache {
@ -419,7 +415,7 @@ impl LayoutCache {
render_device: &RenderDevice,
bind_group_layouts: &[BindGroupLayout],
push_constant_ranges: Vec<PushConstantRange>,
) -> ErasedPipelineLayout {
) -> Arc<WgpuWrapper<PipelineLayout>> {
let bind_group_ids = bind_group_layouts.iter().map(BindGroupLayout::id).collect();
self.layouts
.entry((bind_group_ids, push_constant_ranges))
@ -428,13 +424,13 @@ impl LayoutCache {
.iter()
.map(BindGroupLayout::value)
.collect::<Vec<_>>();
ErasedPipelineLayout::new(render_device.create_pipeline_layout(
Arc::new(WgpuWrapper::new(render_device.create_pipeline_layout(
&PipelineLayoutDescriptor {
bind_group_layouts: &bind_group_layouts,
push_constant_ranges,
..default()
},
))
)))
})
.clone()
}
@ -746,7 +742,7 @@ impl PipelineCache {
multiview: None,
depth_stencil: descriptor.depth_stencil.clone(),
label: descriptor.label.as_deref(),
layout: layout.as_deref(),
layout: layout.as_ref().map(|layout| -> &PipelineLayout { layout }),
multisample: descriptor.multisample,
primitive: descriptor.primitive,
vertex: RawVertexState {
@ -814,7 +810,7 @@ impl PipelineCache {
let descriptor = RawComputePipelineDescriptor {
label: descriptor.label.as_deref(),
layout: layout.as_deref(),
layout: layout.as_ref().map(|layout| -> &PipelineLayout { layout }),
module: &compute_module,
entry_point: &descriptor.entry_point,
// TODO: Expose this somehow

View File

@ -1,150 +1,3 @@
// 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 type-erasing with the `render_resource_wrapper` macro. The resulting type behaves like Arc<$wgpu_type>,
// but avoids explicitly storing an Arc<$wgpu_type> member.
// analysis from https://github.com/bevyengine/bevy/pull/5950#issuecomment-1243473071 indicates this is
// due to `evaluate_obligations`. we should check if this can be removed after a fix lands for
// https://github.com/rust-lang/rust/issues/99188 (and after other `evaluate_obligations`-related changes).
#[cfg(debug_assertions)]
#[macro_export]
macro_rules! render_resource_wrapper {
($wrapper_type:ident, $wgpu_type:ty) => {
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
#[derive(Debug)]
// SAFETY: while self is live, self.0 comes from `into_raw` of an Arc<$wgpu_type> with a strong ref.
pub struct $wrapper_type(*const ());
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
#[derive(Debug)]
pub struct $wrapper_type(send_wrapper::SendWrapper<*const ()>);
impl $wrapper_type {
pub fn new(value: $wgpu_type) -> Self {
let arc = alloc::sync::Arc::new(value);
let value_ptr = alloc::sync::Arc::into_raw(arc);
let unit_ptr = value_ptr.cast::<()>();
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
return Self(unit_ptr);
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
return Self(send_wrapper::SendWrapper::new(unit_ptr));
}
pub fn try_unwrap(self) -> Option<$wgpu_type> {
let value_ptr = self.0.cast::<$wgpu_type>();
// SAFETY: pointer refers to a valid Arc, and was created from Arc::into_raw.
let arc = unsafe { alloc::sync::Arc::from_raw(value_ptr) };
// we forget ourselves here since the reconstructed arc will be dropped/decremented within this scope
core::mem::forget(self);
alloc::sync::Arc::try_unwrap(arc).ok()
}
}
impl core::ops::Deref for $wrapper_type {
type Target = $wgpu_type;
fn deref(&self) -> &Self::Target {
let value_ptr = self.0.cast::<$wgpu_type>();
// SAFETY: the arc lives for 'self, so the ref lives for 'self
let value_ref = unsafe { value_ptr.as_ref() };
value_ref.unwrap()
}
}
impl Drop for $wrapper_type {
fn drop(&mut self) {
let value_ptr = self.0.cast::<$wgpu_type>();
// SAFETY: pointer refers to a valid Arc, and was created from Arc::into_raw.
// this reconstructed arc is dropped/decremented within this scope.
unsafe { alloc::sync::Arc::from_raw(value_ptr) };
}
}
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
// SAFETY: We manually implement Send and Sync, which is valid for Arc<T> when T: Send + Sync.
// We ensure correctness by checking that $wgpu_type does implement Send and Sync.
// If in future there is a case where a wrapper is required for a non-send/sync type
// we can implement a macro variant that omits these manual Send + Sync impls
unsafe impl Send for $wrapper_type {}
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
// SAFETY: As explained above, we ensure correctness by checking that $wgpu_type implements Send and Sync.
unsafe impl Sync for $wrapper_type {}
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
const _: () = {
trait AssertSendSyncBound: Send + Sync {}
impl AssertSendSyncBound for $wgpu_type {}
};
impl Clone for $wrapper_type {
fn clone(&self) -> Self {
let value_ptr = self.0.cast::<$wgpu_type>();
// SAFETY: pointer refers to a valid Arc, and was created from Arc::into_raw.
let arc = unsafe { alloc::sync::Arc::from_raw(value_ptr.cast::<$wgpu_type>()) };
let cloned = alloc::sync::Arc::clone(&arc);
// we forget the reconstructed Arc to avoid decrementing the ref counter, as self is still live.
core::mem::forget(arc);
let cloned_value_ptr = alloc::sync::Arc::into_raw(cloned);
let cloned_unit_ptr = cloned_value_ptr.cast::<()>();
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
return Self(cloned_unit_ptr);
// Note: this implementation means that this Clone will panic
// when called off the wgpu thread.
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
return Self(send_wrapper::SendWrapper::new(cloned_unit_ptr));
}
}
};
}
#[cfg(not(debug_assertions))]
#[macro_export]
macro_rules! render_resource_wrapper {
($wrapper_type:ident, $wgpu_type:ty) => {
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
#[derive(Clone, Debug)]
pub struct $wrapper_type(std::sync::Arc<$wgpu_type>);
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
#[derive(Clone, Debug)]
pub struct $wrapper_type(std::sync::Arc<send_wrapper::SendWrapper<$wgpu_type>>);
impl $wrapper_type {
pub fn new(value: $wgpu_type) -> Self {
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
return Self(std::sync::Arc::new(value));
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
return Self(std::sync::Arc::new(send_wrapper::SendWrapper::new(value)));
}
pub fn try_unwrap(self) -> Option<$wgpu_type> {
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
return std::sync::Arc::try_unwrap(self.0).ok();
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
return std::sync::Arc::try_unwrap(self.0).ok().map(|p| p.take());
}
}
impl core::ops::Deref for $wrapper_type {
type Target = $wgpu_type;
fn deref(&self) -> &Self::Target {
self.0.as_ref()
}
}
#[cfg(not(all(target_arch = "wasm32", target_feature = "atomics")))]
const _: () = {
trait AssertSendSyncBound: Send + Sync {}
impl AssertSendSyncBound for $wgpu_type {}
};
};
}
#[macro_export]
macro_rules! define_atomic_id {
($atomic_id_type:ident) => {
@ -182,5 +35,3 @@ macro_rules! define_atomic_id {
}
};
}
pub use render_resource_wrapper;

View File

@ -1,10 +1,9 @@
use crate::define_atomic_id;
use crate::renderer::WgpuWrapper;
use alloc::sync::Arc;
use core::ops::Deref;
use crate::render_resource::resource_macros::*;
define_atomic_id!(TextureId);
render_resource_wrapper!(ErasedTexture, wgpu::Texture);
/// A GPU-accessible texture.
///
@ -13,7 +12,7 @@ render_resource_wrapper!(ErasedTexture, wgpu::Texture);
#[derive(Clone, Debug)]
pub struct Texture {
id: TextureId,
value: ErasedTexture,
value: Arc<WgpuWrapper<wgpu::Texture>>,
}
impl Texture {
@ -33,7 +32,7 @@ impl From<wgpu::Texture> for Texture {
fn from(value: wgpu::Texture) -> Self {
Texture {
id: TextureId::new(),
value: ErasedTexture::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}
@ -48,23 +47,23 @@ impl Deref for Texture {
}
define_atomic_id!(TextureViewId);
render_resource_wrapper!(ErasedTextureView, wgpu::TextureView);
render_resource_wrapper!(ErasedSurfaceTexture, wgpu::SurfaceTexture);
/// Describes a [`Texture`] with its associated metadata required by a pipeline or [`BindGroup`](super::BindGroup).
#[derive(Clone, Debug)]
pub struct TextureView {
id: TextureViewId,
value: ErasedTextureView,
value: Arc<WgpuWrapper<wgpu::TextureView>>,
}
pub struct SurfaceTexture {
value: ErasedSurfaceTexture,
value: Arc<WgpuWrapper<wgpu::SurfaceTexture>>,
}
impl SurfaceTexture {
pub fn try_unwrap(self) -> Option<wgpu::SurfaceTexture> {
self.value.try_unwrap()
Arc::try_unwrap(self.value)
.map(WgpuWrapper::into_inner)
.ok()
}
}
@ -80,7 +79,7 @@ impl From<wgpu::TextureView> for TextureView {
fn from(value: wgpu::TextureView) -> Self {
TextureView {
id: TextureViewId::new(),
value: ErasedTextureView::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}
@ -88,7 +87,7 @@ impl From<wgpu::TextureView> for TextureView {
impl From<wgpu::SurfaceTexture> for SurfaceTexture {
fn from(value: wgpu::SurfaceTexture) -> Self {
SurfaceTexture {
value: ErasedSurfaceTexture::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}
@ -112,7 +111,6 @@ impl Deref for SurfaceTexture {
}
define_atomic_id!(SamplerId);
render_resource_wrapper!(ErasedSampler, wgpu::Sampler);
/// A Sampler defines how a pipeline will sample from a [`TextureView`].
/// They define image filters (including anisotropy) and address (wrapping) modes, among other things.
@ -122,7 +120,7 @@ render_resource_wrapper!(ErasedSampler, wgpu::Sampler);
#[derive(Clone, Debug)]
pub struct Sampler {
id: SamplerId,
value: ErasedSampler,
value: Arc<WgpuWrapper<wgpu::Sampler>>,
}
impl Sampler {
@ -137,7 +135,7 @@ impl From<wgpu::Sampler> for Sampler {
fn from(value: wgpu::Sampler) -> Self {
Sampler {
id: SamplerId::new(),
value: ErasedSampler::new(value),
value: Arc::new(WgpuWrapper::new(value)),
}
}
}

View File

@ -142,6 +142,10 @@ impl<T> WgpuWrapper<T> {
pub fn new(t: T) -> Self {
Self(t)
}
pub fn into_inner(self) -> T {
self.0
}
}
#[cfg(all(target_arch = "wasm32", target_feature = "atomics"))]
@ -149,6 +153,10 @@ impl<T> WgpuWrapper<T> {
pub fn new(t: T) -> Self {
Self(send_wrapper::SendWrapper::new(t))
}
pub fn into_inner(self) -> T {
self.0.take()
}
}
/// This queue is used to enqueue tasks for the GPU to execute asynchronously.

View File

@ -1,34 +1,33 @@
use super::RenderQueue;
use crate::render_resource::{
BindGroup, BindGroupLayout, Buffer, ComputePipeline, RawRenderPipelineDescriptor,
RenderPipeline, Sampler, Texture,
};
use crate::WgpuWrapper;
use alloc::sync::Arc;
use bevy_ecs::system::Resource;
use wgpu::{
util::DeviceExt, BindGroupDescriptor, BindGroupEntry, BindGroupLayoutDescriptor,
BindGroupLayoutEntry, BufferAsyncError, BufferBindingType, MaintainResult,
};
use super::RenderQueue;
use crate::{render_resource::resource_macros::*, WgpuWrapper};
render_resource_wrapper!(ErasedRenderDevice, wgpu::Device);
/// This GPU device is responsible for the creation of most rendering and compute resources.
#[derive(Resource, Clone)]
pub struct RenderDevice {
device: WgpuWrapper<ErasedRenderDevice>,
device: Arc<WgpuWrapper<wgpu::Device>>,
}
impl From<wgpu::Device> for RenderDevice {
fn from(device: wgpu::Device) -> Self {
Self {
device: WgpuWrapper::new(ErasedRenderDevice::new(device)),
}
Self::new(Arc::new(WgpuWrapper::new(device)))
}
}
impl RenderDevice {
pub fn new(device: Arc<WgpuWrapper<wgpu::Device>>) -> Self {
Self { device }
}
/// List all [`Features`](wgpu::Features) that may be used with this device.
///
/// Functions may panic if you use unsupported features.