Skip to content

Commit

Permalink
Pass BufferAccessResult instead of BufferMapAsyncStatus to the rust c…
Browse files Browse the repository at this point in the history
…allback in map_async

It provides with more information about the errors. BufferMapAsyncStatus is only needed for the C callback which needs an ffi-frendly payload.
  • Loading branch information
nical committed Aug 3, 2022
1 parent c9bd036 commit 5affada
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 44 deletions.
6 changes: 3 additions & 3 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,11 +888,11 @@ impl<A: HalApi> LifetimeTracker<A> {
range: mapping.range.start..mapping.range.start + size,
host,
};
resource::BufferMapAsyncStatus::Success
Ok(())
}
Err(e) => {
log::error!("Mapping failed {:?}", e);
resource::BufferMapAsyncStatus::Error
Err(e)
}
}
} else {
Expand All @@ -901,7 +901,7 @@ impl<A: HalApi> LifetimeTracker<A> {
range: mapping.range,
host: mapping.op.host,
};
resource::BufferMapAsyncStatus::Success
Ok(())
};
pending_callbacks.push((mapping.op, status));
}
Expand Down
44 changes: 9 additions & 35 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::{
TextureInitTracker, TextureInitTrackerAction,
},
instance, pipeline, present,
resource::{self, BufferMapState},
resource::{BufferAccessError, BufferMapAsyncStatus, BufferMapOperation},
resource::{self, BufferAccessResult, BufferMapState},
resource::{BufferAccessError, BufferMapOperation},
track::{BindGroupStates, TextureSelector, Tracker},
validation::{self, check_buffer_usage, check_texture_usage},
FastHashMap, Label, LabelHelpers as _, LifeGuard, MultiRefCount, RefCount, Stored,
Expand Down Expand Up @@ -132,7 +132,7 @@ impl RenderPassContext {
}
}

pub type BufferMapPendingClosure = (BufferMapOperation, BufferMapAsyncStatus);
pub type BufferMapPendingClosure = (BufferMapOperation, BufferAccessResult);

#[derive(Default)]
pub struct UserClosures {
Expand Down Expand Up @@ -3433,7 +3433,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
buffer_id: id::BufferId,
offset: BufferAddress,
data: &[u8],
) -> Result<(), BufferAccessError> {
) -> BufferAccessResult {
profiling::scope!("Device::set_buffer_sub_data");

let hub = A::hub(self);
Expand Down Expand Up @@ -3490,7 +3490,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
buffer_id: id::BufferId,
offset: BufferAddress,
data: &mut [u8],
) -> Result<(), BufferAccessError> {
) -> BufferAccessResult {
profiling::scope!("Device::get_buffer_sub_data");

let hub = A::hub(self);
Expand Down Expand Up @@ -5382,35 +5382,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
buffer_id: id::BufferId,
range: Range<BufferAddress>,
op: BufferMapOperation,
) -> Result<(), BufferAccessError> {
) -> BufferAccessResult {
// User callbacks must not be called while holding buffer_map_async_inner's locks, so we
// defer the error callback if it needs to be called immediately (typically when running
// into errors).
if let Err((op, err)) = self.buffer_map_async_inner::<A>(buffer_id, range, op) {
let status = match &err {
&BufferAccessError::Device(_) => BufferMapAsyncStatus::ContextLost,
&BufferAccessError::Invalid | &BufferAccessError::Destroyed => {
BufferMapAsyncStatus::Invalid
}
&BufferAccessError::AlreadyMapped => BufferMapAsyncStatus::AlreadyMapped,
&BufferAccessError::MapAlreadyPending => BufferMapAsyncStatus::MapAlreadyPending,
&BufferAccessError::MissingBufferUsage(_) => {
BufferMapAsyncStatus::InvalidUsageFlags
}
&BufferAccessError::UnalignedRange
| &BufferAccessError::UnalignedRangeSize { .. }
| &BufferAccessError::UnalignedOffset { .. } => {
BufferMapAsyncStatus::InvalidAlignment
}
&BufferAccessError::OutOfBoundsUnderrun { .. }
| &BufferAccessError::OutOfBoundsOverrun { .. }
| &BufferAccessError::NegativeRange { .. } => {
BufferMapAsyncStatus::InvalidRange
}
_ => BufferMapAsyncStatus::Error,
};

op.callback.call(status);
op.callback.call(Err(err.clone()));

return Err(err);
}
Expand Down Expand Up @@ -5646,7 +5623,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(BufferAccessError::NotMapped);
}
resource::BufferMapState::Waiting(pending) => {
return Ok(Some((pending.op, resource::BufferMapAsyncStatus::Aborted)));
return Ok(Some((pending.op, Err(BufferAccessError::MapAborted))));
}
resource::BufferMapState::Active { ptr, range, host } => {
if host == HostMap::Write {
Expand Down Expand Up @@ -5677,10 +5654,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(None)
}

pub fn buffer_unmap<A: HalApi>(
&self,
buffer_id: id::BufferId,
) -> Result<(), BufferAccessError> {
pub fn buffer_unmap<A: HalApi>(&self, buffer_id: id::BufferId) -> BufferAccessResult {
profiling::scope!("unmap", "Buffer");

let closure;
Expand Down
41 changes: 36 additions & 5 deletions wgpu-core/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::{borrow::Borrow, num::NonZeroU8, ops::Range, ptr::NonNull};

/// The status code provided to the buffer mapping callback.
///
/// This is very similar to `Result<(), BufferAccessError>`, except that this is FFI-friendly.
/// This is very similar to `BufferAccessResult`, except that this is FFI-friendly.
#[repr(C)]
#[derive(Debug)]
pub enum BufferMapAsyncStatus {
Expand Down Expand Up @@ -84,15 +84,15 @@ pub struct BufferMapCallback {

enum BufferMapCallbackInner {
Rust {
callback: Box<dyn FnOnce(BufferMapAsyncStatus) + Send + 'static>,
callback: Box<dyn FnOnce(BufferAccessResult) + Send + 'static>,
},
C {
inner: BufferMapCallbackC,
},
}

impl BufferMapCallback {
pub fn from_rust(callback: Box<dyn FnOnce(BufferMapAsyncStatus) + Send + 'static>) -> Self {
pub fn from_rust(callback: Box<dyn FnOnce(BufferAccessResult) + Send + 'static>) -> Self {
Self {
inner: Some(BufferMapCallbackInner::Rust { callback }),
}
Expand All @@ -108,13 +108,39 @@ impl BufferMapCallback {
}
}

pub(crate) fn call(mut self, status: BufferMapAsyncStatus) {
pub(crate) fn call(mut self, result: BufferAccessResult) {
match self.inner.take() {
Some(BufferMapCallbackInner::Rust { callback }) => {
callback(status);
callback(result);
}
// SAFETY: the contract of the call to from_c says that this unsafe is sound.
Some(BufferMapCallbackInner::C { inner }) => unsafe {
let status = match result {
Ok(()) => BufferMapAsyncStatus::Success,
Err(BufferAccessError::Device(_)) => BufferMapAsyncStatus::ContextLost,
Err(BufferAccessError::Invalid) | Err(BufferAccessError::Destroyed) => {
BufferMapAsyncStatus::Invalid
}
Err(BufferAccessError::AlreadyMapped) => BufferMapAsyncStatus::AlreadyMapped,
Err(BufferAccessError::MapAlreadyPending) => {
BufferMapAsyncStatus::MapAlreadyPending
}
Err(BufferAccessError::MissingBufferUsage(_)) => {
BufferMapAsyncStatus::InvalidUsageFlags
}
Err(BufferAccessError::UnalignedRange)
| Err(BufferAccessError::UnalignedRangeSize { .. })
| Err(BufferAccessError::UnalignedOffset { .. }) => {
BufferMapAsyncStatus::InvalidAlignment
}
Err(BufferAccessError::OutOfBoundsUnderrun { .. })
| Err(BufferAccessError::OutOfBoundsOverrun { .. })
| Err(BufferAccessError::NegativeRange { .. }) => {
BufferMapAsyncStatus::InvalidRange
}
Err(_) => BufferMapAsyncStatus::Error,
};

(inner.callback)(status, inner.user_data);
},
None => {
Expand All @@ -141,6 +167,8 @@ pub struct BufferMapOperation {
pub enum BufferAccessError {
#[error(transparent)]
Device(#[from] DeviceError),
#[error("buffer map failed")]
Failed,
#[error("buffer is invalid")]
Invalid,
#[error("buffer is destroyed")]
Expand Down Expand Up @@ -178,8 +206,11 @@ pub enum BufferAccessError {
start: wgt::BufferAddress,
end: wgt::BufferAddress,
},
#[error("buffer map aborted")]
MapAborted,
}

pub type BufferAccessResult = Result<(), BufferAccessError>;
pub(crate) struct BufferPendingMapping {
pub range: Range<wgt::BufferAddress>,
pub op: BufferMapOperation,
Expand Down
2 changes: 1 addition & 1 deletion wgpu/src/backend/direct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ impl crate::Context for Context {
},
callback: wgc::resource::BufferMapCallback::from_rust(Box::new(|status| {
let res = match status {
wgc::resource::BufferMapAsyncStatus::Success => Ok(()),
Ok(()) => Ok(()),
_ => Err(crate::BufferAsyncError),
};
callback(res);
Expand Down

0 comments on commit 5affada

Please sign in to comment.