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

[Merged by Bors] - improve compile time by type-erasing wgpu structs #5950

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
move unsafety to call scope
  • Loading branch information
robtfm committed Sep 12, 2022
commit f688540019c24ae1aedd12d29fc1bc880a12b9b5
6 changes: 4 additions & 2 deletions crates/bevy_render/src/render_resource/bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ impl Deref for BindGroup {

#[inline]
fn deref(&self) -> &Self::Target {
render_resource_ref!(&self.value, wgpu::BindGroup)
unsafe { render_resource_ref!(&self.value, wgpu::BindGroup) }
}
}

impl Drop for BindGroup {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::BindGroup);
unsafe {
render_resource_drop!(&mut self.value, wgpu::BindGroup);
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_render/src/render_resource/bind_group_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl BindGroupLayout {

#[inline]
pub fn value(&self) -> &wgpu::BindGroupLayout {
render_resource_ref!(&self.value, wgpu::BindGroupLayout)
unsafe { render_resource_ref!(&self.value, wgpu::BindGroupLayout) }
}
}

Expand All @@ -49,6 +49,8 @@ impl Deref for BindGroupLayout {

impl Drop for BindGroupLayout {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::BindGroupLayout);
unsafe {
render_resource_drop!(&mut self.value, wgpu::BindGroupLayout);
}
}
}
10 changes: 6 additions & 4 deletions crates/bevy_render/src/render_resource/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ impl Buffer {
Bound::Excluded(&bound) => bound + 1,
Bound::Unbounded => 0,
},
value: render_resource_ref!(self.value, wgpu::Buffer).slice(bounds),
value: unsafe { render_resource_ref!(&self.value, wgpu::Buffer).slice(bounds) },
}
}

#[inline]
pub fn unmap(&self) {
render_resource_ref!(self.value, wgpu::Buffer).unmap();
unsafe { render_resource_ref!(&self.value, wgpu::Buffer).unmap() };
}
}

Expand All @@ -51,13 +51,15 @@ impl Deref for Buffer {

#[inline]
fn deref(&self) -> &Self::Target {
render_resource_ref!(&self.value, wgpu::Buffer)
unsafe { render_resource_ref!(&self.value, wgpu::Buffer) }
}
}

impl Drop for Buffer {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::Buffer);
unsafe {
render_resource_drop!(&mut self.value, wgpu::Buffer);
}
}
}

Expand Down
12 changes: 8 additions & 4 deletions crates/bevy_render/src/render_resource/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,15 @@ impl Deref for RenderPipeline {

#[inline]
fn deref(&self) -> &Self::Target {
render_resource_ref!(&self.value, wgpu::RenderPipeline)
unsafe { render_resource_ref!(&self.value, wgpu::RenderPipeline) }
}
}

impl Drop for RenderPipeline {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::RenderPipeline);
unsafe {
render_resource_drop!(&mut self.value, wgpu::RenderPipeline);
}
}
}

Expand Down Expand Up @@ -90,13 +92,15 @@ impl Deref for ComputePipeline {

#[inline]
fn deref(&self) -> &Self::Target {
render_resource_ref!(&self.value, wgpu::ComputePipeline)
unsafe { render_resource_ref!(&self.value, wgpu::ComputePipeline) }
}
}

impl Drop for ComputePipeline {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::ComputePipeline);
unsafe {
render_resource_drop!(&mut self.value, wgpu::ComputePipeline);
}
}
}

Expand Down
20 changes: 13 additions & 7 deletions crates/bevy_render/src/render_resource/pipeline_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ impl ShaderCache {
while let Some(handle) = shaders_to_clear.pop() {
if let Some(data) = self.data.get_mut(&handle) {
for (_, mut shader) in data.processed_shaders.drain() {
render_resource_drop!(&mut shader, wgpu::ShaderModule);
unsafe {
render_resource_drop!(&mut shader, wgpu::ShaderModule);
}
}
pipelines_to_queue.extend(data.pipelines.iter().cloned());
shaders_to_clear.extend(data.dependents.iter().map(|h| h.clone_weak()));
Expand Down Expand Up @@ -279,7 +281,9 @@ impl Drop for ShaderCache {
fn drop(&mut self) {
for (_, mut data) in self.data.drain() {
for (_, mut render_resource) in data.processed_shaders.drain() {
render_resource_drop!(&mut render_resource, ShaderModule);
unsafe {
render_resource_drop!(&mut render_resource, ShaderModule);
}
}
}
}
Expand Down Expand Up @@ -309,14 +313,16 @@ impl LayoutCache {
})
)
});
render_resource_ref!(untyped, wgpu::PipelineLayout)
unsafe { render_resource_ref!(untyped, wgpu::PipelineLayout) }
}
}

impl Drop for LayoutCache {
fn drop(&mut self) {
for (_, mut render_resource) in self.layouts.drain() {
render_resource_drop!(&mut render_resource, wgpu::PipelineLayout);
unsafe {
render_resource_drop!(&mut render_resource, wgpu::PipelineLayout);
}
}
}
}
Expand Down Expand Up @@ -571,13 +577,13 @@ impl PipelineCache {
vertex: RawVertexState {
buffers: &vertex_buffer_layouts,
entry_point: descriptor.vertex.entry_point.deref(),
module: render_resource_ref!(&vertex_module, ShaderModule),
module: unsafe { render_resource_ref!(&vertex_module, ShaderModule) },
},
fragment: fragment_data
.as_ref()
.map(|(module, entry_point, targets)| RawFragmentState {
entry_point,
module: render_resource_ref!(&module, ShaderModule),
module: unsafe { render_resource_ref!(module, ShaderModule) },
targets,
}),
};
Expand Down Expand Up @@ -613,7 +619,7 @@ impl PipelineCache {
let descriptor = RawComputePipelineDescriptor {
label: descriptor.label.as_deref(),
layout,
module: render_resource_ref!(&compute_module, ShaderModule),
module: unsafe { render_resource_ref!(&compute_module, ShaderModule) },
entry_point: descriptor.entry_point.as_ref(),
};

Expand Down
39 changes: 27 additions & 12 deletions crates/bevy_render/src/render_resource/resource_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::sync::Arc;

#[cfg(debug_assertions)]
#[derive(Clone, Debug)]
#[allow(clippy::redundant_allocation)]
pub struct BlackBox(Option<Arc<Box<()>>>);

#[cfg(debug_assertions)]
Expand All @@ -11,6 +12,9 @@ impl BlackBox {
unsafe { Self(Some(Arc::new(std::mem::transmute(Box::new(value))))) }
}

/// # Safety
///
/// Caller must ensure the contained value is of type T
pub unsafe fn typed_ref<T>(&self) -> &T {
let untyped_box = self
.0
Expand All @@ -22,6 +26,9 @@ impl BlackBox {
typed_box.as_ref()
}

/// # Safety
///
/// Caller must ensure the contained value is of type T
pub unsafe fn try_unwrap<T>(mut self) -> Option<T> {
let inner = self.0.take();
if let Some(inner) = inner {
Expand All @@ -39,6 +46,10 @@ impl BlackBox {
None
}
}

/// # Safety
///
/// Caller must ensure the contained value is of type T
pub unsafe fn drop_inner<T>(&mut self) {
let inner = self.0.take();
if let Some(inner) = inner {
Expand All @@ -51,7 +62,7 @@ impl BlackBox {
impl Drop for BlackBox {
fn drop(&mut self) {
if let Some(inner) = &self.0 {
if Arc::strong_count(&inner) == 1 {
if Arc::strong_count(inner) == 1 {
panic!("undropped inner");
}
}
Expand All @@ -69,7 +80,7 @@ macro_rules! render_resource_type {
#[macro_export]
macro_rules! render_resource_ref {
($value:expr, $wgpu_type:ty) => {
unsafe { $value.typed_ref::<$wgpu_type>() }
$value.typed_ref::<$wgpu_type>()
};
}
#[cfg(debug_assertions)]
Expand All @@ -83,16 +94,14 @@ macro_rules! render_resource_new {
#[macro_export]
macro_rules! render_resource_drop {
($value:expr, $wgpu_type:ty) => {
unsafe {
$value.drop_inner::<$wgpu_type>();
}
$value.drop_inner::<$wgpu_type>();
};
}
#[cfg(debug_assertions)]
#[macro_export]
macro_rules! render_resource_try_unwrap {
($value:expr, $wgpu_type:ty) => {{
unsafe { $value.try_unwrap::<$wgpu_type>() }
$value.try_unwrap::<$wgpu_type>()
}};
}

Expand All @@ -106,9 +115,11 @@ macro_rules! render_resource_type {
#[cfg(not(debug_assertions))]
#[macro_export]
macro_rules! render_resource_ref {
($value:expr, $wgpu_type:ty) => {
($value:expr, $wgpu_type:ty) => {{
// remove unused unsafe warning
std::mem::transmute::<(), ()>(());
$value
};
}};
}
#[cfg(not(debug_assertions))]
#[macro_export]
Expand All @@ -120,16 +131,20 @@ macro_rules! render_resource_new {
#[cfg(not(debug_assertions))]
#[macro_export]
macro_rules! render_resource_drop {
($value:expr, $wgpu_type:ty) => {
($value:expr, $wgpu_type:ty) => {{
// remove unused unsafe warning
std::mem::transmute::<(), ()>(());
let _ = $value;
};
}};
}
#[cfg(not(debug_assertions))]
#[macro_export]
macro_rules! render_resource_try_unwrap {
($value:expr, $wgpu_type:ty) => {
($value:expr, $wgpu_type:ty) => {{
// remove unused unsafe warning
std::mem::transmute::<(), ()>(());
std::sync::Arc::try_unwrap($value).ok()
};
}};
}

pub use render_resource_drop;
Expand Down
34 changes: 22 additions & 12 deletions crates/bevy_render/src/render_resource/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl Texture {
}

fn value(&self) -> &wgpu::Texture {
render_resource_ref!(&self.value, wgpu::Texture)
unsafe { render_resource_ref!(&self.value, wgpu::Texture) }
}
}

Expand All @@ -54,7 +54,9 @@ impl Deref for Texture {

impl Drop for Texture {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::Texture);
unsafe {
render_resource_drop!(&mut self.value, wgpu::Texture);
}
}
}

Expand Down Expand Up @@ -102,7 +104,7 @@ impl TextureView {
if let TextureViewValue::SurfaceTexture { texture, .. } = &mut self.value {
let texture = texture.take();
if let Some(texture) = texture {
return render_resource_try_unwrap!(texture, wgpu::SurfaceTexture);
return unsafe { render_resource_try_unwrap!(texture, wgpu::SurfaceTexture) };
}
}

Expand Down Expand Up @@ -137,25 +139,31 @@ impl Deref for TextureView {
#[inline]
fn deref(&self) -> &Self::Target {
match &self.value {
TextureViewValue::TextureView(value) => render_resource_ref!(value, wgpu::TextureView),
TextureViewValue::SurfaceTexture { view, .. } => {
TextureViewValue::TextureView(value) => unsafe {
render_resource_ref!(value, wgpu::TextureView)
},
TextureViewValue::SurfaceTexture { view, .. } => unsafe {
render_resource_ref!(view, wgpu::TextureView)
}
},
}
}
}

impl Drop for TextureView {
fn drop(&mut self) {
match &mut self.value {
TextureViewValue::TextureView(value) => {
TextureViewValue::TextureView(value) => unsafe {
render_resource_drop!(value, wgpu::TextureView);
}
},
TextureViewValue::SurfaceTexture { texture, view } => {
if let Some(texture) = texture {
render_resource_drop!(texture, wgpu::SurfaceTexture);
unsafe {
render_resource_drop!(texture, wgpu::SurfaceTexture);
}
}
unsafe {
render_resource_drop!(view, wgpu::TextureView);
}
render_resource_drop!(view, wgpu::TextureView);
}
}
}
Expand Down Expand Up @@ -198,12 +206,14 @@ impl Deref for Sampler {

#[inline]
fn deref(&self) -> &Self::Target {
render_resource_ref!(&self.value, wgpu::Sampler)
unsafe { render_resource_ref!(&self.value, wgpu::Sampler) }
}
}

impl Drop for Sampler {
fn drop(&mut self) {
render_resource_drop!(&mut self.value, wgpu::Sampler);
unsafe {
render_resource_drop!(&mut self.value, wgpu::Sampler);
}
}
}
6 changes: 4 additions & 2 deletions crates/bevy_render/src/renderer/render_device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl From<wgpu::Device> for RenderDevice {
impl RenderDevice {
#[inline]
fn device(&self) -> &wgpu::Device {
render_resource_ref!(&self.device, wgpu::Device)
unsafe { render_resource_ref!(&self.device, wgpu::Device) }
}

/// List all [`Features`](wgpu::Features) that may be used with this device.
Expand Down Expand Up @@ -207,6 +207,8 @@ impl RenderDevice {

impl Drop for RenderDevice {
fn drop(&mut self) {
render_resource_drop!(&mut self.device, wgpu::Device);
unsafe {
render_resource_drop!(&mut self.device, wgpu::Device);
}
}
}