Skip to content

Commit

Permalink
Remove panics in Deref impl of QueueWriteBufferView and `BufferVi…
Browse files Browse the repository at this point in the history
…ewMut` (#3336)
  • Loading branch information
botahamec authored Jan 12, 2023
1 parent 48fbb92 commit f2d2a0c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non
- Add missing `DEPTH_BIAS_CLAMP` and `FULL_DRAW_INDEX_UINT32` downlevel flags. By @teoxoy in [#3316](https://github.com/gfx-rs/wgpu/pull/3316)
- Make `ObjectId` structure and invariants idiomatic. By @teoxoy in [#3347](https://github.com/gfx-rs/wgpu/pull/3347)
- Add validation in accordance with WebGPU `GPUSamplerDescriptor` valid usage for `lodMinClamp` and `lodMaxClamp`. By @James2022-rgb in [#3353](https://github.com/gfx-rs/wgpu/pull/3353)
- Remove panics in `Deref` implementations for `QueueWriteBufferView` and `BufferViewMut`. Instead, warnings are logged, since reading from these types is not recommended. By @botahamec in [#3336]

#### WebGPU

Expand Down
63 changes: 35 additions & 28 deletions wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::{
future::Future,
marker::PhantomData,
num::{NonZeroU32, NonZeroU8},
ops::{Bound, Range, RangeBounds},
ops::{Bound, Deref, DerefMut, Range, RangeBounds},
sync::Arc,
thread,
};
Expand Down Expand Up @@ -2297,6 +2297,9 @@ pub struct BufferView<'a> {
}

/// Write only view into mapped buffer.
///
/// It is possible to read the buffer using this view, but doing so is not
/// recommended, as it is likely to be slow.
#[derive(Debug)]
pub struct BufferViewMut<'a> {
slice: BufferSlice<'a>,
Expand All @@ -2313,37 +2316,34 @@ impl std::ops::Deref for BufferView<'_> {
}
}

impl std::ops::Deref for BufferViewMut<'_> {
type Target = [u8];

impl AsRef<[u8]> for BufferView<'_> {
#[inline]
fn deref(&self) -> &[u8] {
assert!(
self.readable,
"Attempting to read a write-only mapping for buffer {:?}",
self.slice.buffer.id
);
fn as_ref(&self) -> &[u8] {
self.data.slice()
}
}

impl std::ops::DerefMut for BufferViewMut<'_> {
impl AsMut<[u8]> for BufferViewMut<'_> {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
fn as_mut(&mut self) -> &mut [u8] {
self.data.slice_mut()
}
}

impl AsRef<[u8]> for BufferView<'_> {
#[inline]
fn as_ref(&self) -> &[u8] {
impl Deref for BufferViewMut<'_> {
type Target = [u8];

fn deref(&self) -> &Self::Target {
if !self.readable {
log::warn!("Reading from a BufferViewMut is slow and not recommended.");
}

self.data.slice()
}
}

impl AsMut<[u8]> for BufferViewMut<'_> {
#[inline]
fn as_mut(&mut self) -> &mut [u8] {
impl DerefMut for BufferViewMut<'_> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.data.slice_mut()
}
}
Expand Down Expand Up @@ -3768,7 +3768,11 @@ impl<'a> RenderBundleEncoder<'a> {
}
}

/// A write-only view into a staging buffer
/// A read-only view into a staging buffer.
///
/// Reading into this buffer won't yield the contents of the buffer from the
/// GPU and is likely to be slow. Because of this, although [`AsMut`] is
/// implemented for this type, [`AsRef`] is not.
pub struct QueueWriteBufferView<'a> {
queue: &'a Queue,
buffer: &'a Buffer,
Expand All @@ -3777,21 +3781,27 @@ pub struct QueueWriteBufferView<'a> {
}
static_assertions::assert_impl_all!(QueueWriteBufferView: Send, Sync);

impl<'a> std::ops::Deref for QueueWriteBufferView<'a> {
impl Deref for QueueWriteBufferView<'_> {
type Target = [u8];

fn deref(&self) -> &Self::Target {
panic!("QueueWriteBufferView is write-only!");
log::warn!("Reading from a QueueWriteBufferView won't yield the contents of the buffer and may be slow.");
self.inner.slice()
}
}

impl<'a> std::ops::DerefMut for QueueWriteBufferView<'a> {
#[inline]
impl DerefMut for QueueWriteBufferView<'_> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.inner.slice_mut()
}
}

impl<'a> AsMut<[u8]> for QueueWriteBufferView<'a> {
fn as_mut(&mut self) -> &mut [u8] {
self.inner.slice_mut()
}
}

impl<'a> Drop for QueueWriteBufferView<'a> {
fn drop(&mut self) {
DynContext::queue_write_staging_buffer(
Expand Down Expand Up @@ -3827,12 +3837,9 @@ impl Queue {
}

/// Schedule a data write into `buffer` starting at `offset` via the returned
/// [QueueWriteBufferView].
/// [`QueueWriteBufferView`].
///
/// The returned value can be dereferenced to a `&mut [u8]`; dereferencing it to a
/// `&[u8]` panics!
/// (It is not unsound to read through the `&mut [u8]` anyway, but doing so will not
/// yield the existing contents of `buffer` from the GPU, and it is likely to be slow.)
/// Reading from this buffer is slow and will not yield the actual contents of the buffer.
///
/// This method is intended to have low performance costs.
/// As such, the write is not immediately submitted, and instead enqueued
Expand Down

0 comments on commit f2d2a0c

Please sign in to comment.