Pass BufferAccessResult instead of BufferMapAsyncStatus to the rust c…

…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.

deno_webgpu/src/buffer.rs

@@ -86,7 +86,7 @@ pub async fn op_webgpu_buffer_get_map_async(
         let callback = Box::new(move |status| {
             sender
                 .send(match status {
-                    wgpu_core::resource::BufferMapAsyncStatus::Success => Ok(()),
+                    Ok(_) => Ok(()),
                     _ => unreachable!(), // TODO
                 })
                 .unwrap();

player/tests/test.rs

@@ -55,10 +55,9 @@ struct Test<'a> {
     actions: Vec<wgc::device::trace::Action<'a>>,
 }
 
-fn map_callback(status: wgc::resource::BufferMapAsyncStatus) {
-    match status {
-        wgc::resource::BufferMapAsyncStatus::Success => (),
-        _ => panic!("Unable to map"),
+fn map_callback(status: Result<(), wgc::resource::BufferAccessError>) {
+    if let Err(e) = status {
+        panic!("Buffer map error: {}", e);
     }
 }
 

wgpu-core/src/device/life.rs

@@ -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 {
@@ -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));
             }

wgpu-core/src/device/mod.rs

@@ -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,
@@ -132,7 +132,7 @@ impl RenderPassContext {
     }
 }
 
-pub type BufferMapPendingClosure = (BufferMapOperation, BufferMapAsyncStatus);
+pub type BufferMapPendingClosure = (BufferMapOperation, BufferAccessResult);
 
 #[derive(Default)]
 pub struct UserClosures {
@@ -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);
@@ -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);
@@ -5382,33 +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);
         }
@@ -5644,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 {
@@ -5675,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;

wgpu-core/src/resource.rs

@@ -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 {
@@ -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 }),
         }
@@ -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 => {
@@ -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")]
@@ -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,

wgpu/src/backend/direct.rs

@@ -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);