diff --git a/player/src/lib.rs b/player/src/lib.rs index 2b0001a5c..a17ad1abb 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -390,7 +390,8 @@ impl GlobalPlay for wgc::hub::Global { self.queue_write_buffer::(device, id, range.start, &bin); } else { self.device_wait_for_buffer::(device, id).unwrap(); - self.device_set_buffer_sub_data::(device, id, range.start, &bin[..size]); + self.device_set_buffer_sub_data::(device, id, range.start, &bin[..size]) + .unwrap(); } } A::WriteTexture { diff --git a/player/tests/test.rs b/player/tests/test.rs index a223b41b7..67132ed90 100644 --- a/player/tests/test.rs +++ b/player/tests/test.rs @@ -97,7 +97,8 @@ impl Test { callback: map_callback, user_data: ptr::null_mut(), } - )); + )) + .unwrap(); } println!("\t\t\tWaiting..."); @@ -107,7 +108,8 @@ impl Test { println!("\t\t\tChecking {}", expect.name); let buffer = wgc::id::TypedId::zip(expect.buffer.index, expect.buffer.epoch, backend); let ptr = - gfx_select!(device => global.buffer_get_mapped_range(buffer, expect.offset, None)); + gfx_select!(device => global.buffer_get_mapped_range(buffer, expect.offset, None)) + .unwrap(); let contents = unsafe { slice::from_raw_parts(ptr, expect.data.len()) }; assert_eq!(&expect.data[..], contents); } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index e984aef7c..9ba76360f 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -9,7 +9,8 @@ use crate::{ hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Hub, Input, Token}, id, pipeline, resource, span, swap_chain, track::{BufferState, TextureState, TrackerSet}, - validation, FastHashMap, LifeGuard, MultiRefCount, PrivateFeatures, Stored, SubmissionIndex, + validation::{self, check_buffer_usage, MissingBufferUsageError}, + FastHashMap, LifeGuard, MultiRefCount, PrivateFeatures, Stored, SubmissionIndex, MAX_BIND_GROUPS, }; @@ -125,7 +126,6 @@ impl RenderPassContext { } } -type BufferMapResult = Result, hal::device::MapError>; type BufferMapPendingCallback = (resource::BufferMapOperation, resource::BufferMapAsyncStatus); fn map_buffer( @@ -133,7 +133,7 @@ fn map_buffer( buffer: &mut resource::Buffer, sub_range: hal::buffer::SubRange, kind: HostMap, -) -> BufferMapResult { +) -> Result, BufferMapError> { let (ptr, segment, needs_sync) = { let segment = hal::memory::Segment { offset: sub_range.offset, @@ -783,7 +783,7 @@ impl Global { buffer_id: id::BufferId, offset: BufferAddress, data: &[u8], - ) { + ) -> Result<(), BufferMapError> { span!(_guard, INFO, "Device::set_buffer_sub_data"); let hub = B::hub(self); @@ -793,11 +793,7 @@ impl Global { let (mut buffer_guard, _) = hub.buffers.write(&mut token); let device = &device_guard[device_id]; let mut buffer = &mut buffer_guard[buffer_id]; - assert!( - buffer.usage.contains(wgt::BufferUsage::MAP_WRITE), - "Buffer usage {:?} must contain usage flag MAP_WRITE", - buffer.usage - ); + check_buffer_usage(buffer.usage, wgt::BufferUsage::MAP_WRITE)?; //assert!(buffer isn't used by the GPU); #[cfg(feature = "trace")] @@ -815,7 +811,7 @@ impl Global { None => (), }; - match map_buffer( + let ptr = map_buffer( &device.raw, &mut buffer, hal::buffer::SubRange { @@ -823,17 +819,15 @@ impl Global { size: Some(data.len() as BufferAddress), }, HostMap::Write, - ) { - Ok(ptr) => unsafe { - ptr::copy_nonoverlapping(data.as_ptr(), ptr.as_ptr(), data.len()); - }, - Err(e) => { - log::error!("failed to map a buffer: {:?}", e); - return; - } + )?; + + unsafe { + ptr::copy_nonoverlapping(data.as_ptr(), ptr.as_ptr(), data.len()); } unmap_buffer(&device.raw, buffer); + + Ok(()) } pub fn device_get_buffer_sub_data( @@ -2852,7 +2846,7 @@ impl Global { buffer_id: id::BufferId, range: Range, op: resource::BufferMapOperation, - ) { + ) -> Result<(), BufferMapError> { span!(_guard, INFO, "Device::buffer_map_async"); let hub = B::hub(self); @@ -2863,26 +2857,24 @@ impl Global { HostMap::Write => (wgt::BufferUsage::MAP_WRITE, resource::BufferUse::MAP_WRITE), }; - assert_eq!(range.start % wgt::COPY_BUFFER_ALIGNMENT, 0); - assert_eq!(range.end % wgt::COPY_BUFFER_ALIGNMENT, 0); + if range.start % wgt::COPY_BUFFER_ALIGNMENT != 0 + || range.end % wgt::COPY_BUFFER_ALIGNMENT != 0 + { + return Err(BufferMapError::UnalignedRange); + } let (device_id, ref_count) = { let (mut buffer_guard, _) = hub.buffers.write(&mut token); let buffer = &mut buffer_guard[buffer_id]; - assert!( - buffer.usage.contains(pub_usage), - "Buffer usage {:?} must contain usage flag(s) {:?}", - buffer.usage, - pub_usage - ); + check_buffer_usage(buffer.usage, pub_usage)?; buffer.map_state = match buffer.map_state { resource::BufferMapState::Init { .. } | resource::BufferMapState::Active { .. } => { - panic!("Buffer already mapped") + return Err(BufferMapError::AlreadyMapped); } resource::BufferMapState::Waiting(_) => { op.call_error(); - return; + return Ok(()); } resource::BufferMapState::Idle => { resource::BufferMapState::Waiting(resource::BufferPendingMapping { @@ -2908,6 +2900,8 @@ impl Global { .change_replace(buffer_id, &ref_count, (), internal_use); device.lock_life(&mut token).map(buffer_id, ref_count); + + Ok(()) } pub fn buffer_get_mapped_range( @@ -2915,7 +2909,7 @@ impl Global { buffer_id: id::BufferId, offset: BufferAddress, _size: Option, - ) -> *mut u8 { + ) -> Result<*mut u8, BufferNotMappedError> { span!(_guard, INFO, "Device::buffer_get_mapped_range"); let hub = B::hub(self); @@ -2926,16 +2920,18 @@ impl Global { match buffer.map_state { resource::BufferMapState::Init { ptr, .. } | resource::BufferMapState::Active { ptr, .. } => unsafe { - ptr.as_ptr().offset(offset as isize) + Ok(ptr.as_ptr().offset(offset as isize)) }, resource::BufferMapState::Idle | resource::BufferMapState::Waiting(_) => { - log::error!("Buffer is not mapped"); - ptr::null_mut() + Err(BufferNotMappedError) } } } - pub fn buffer_unmap(&self, buffer_id: id::BufferId) { + pub fn buffer_unmap( + &self, + buffer_id: id::BufferId, + ) -> Result<(), BufferNotMappedError> { span!(_guard, INFO, "Device::buffer_unmap"); let hub = B::hub(self); @@ -3005,7 +3001,7 @@ impl Global { .consume_temp(stage_buffer, stage_memory); } resource::BufferMapState::Idle => { - log::error!("Buffer is not mapped"); + return Err(BufferNotMappedError); } resource::BufferMapState::Waiting(_) => {} resource::BufferMapState::Active { @@ -3036,6 +3032,7 @@ impl Global { unmap_buffer(&device.raw, buffer); } } + Ok(()) } } @@ -3057,3 +3054,19 @@ pub enum CreateTextureError { )] InvalidMipLevelCount(u32), } + +#[derive(Clone, Debug, Error)] +pub enum BufferMapError { + #[error(transparent)] + MissingBufferUsage(#[from] MissingBufferUsageError), + #[error(transparent)] + MapError(#[from] hal::device::MapError), + #[error("buffer map range is not aligned to {}", wgt::COPY_BUFFER_ALIGNMENT)] + UnalignedRange, + #[error("buffer is already mapped")] + AlreadyMapped, +} + +#[derive(Clone, Debug, Error)] +#[error("buffer is not mapped")] +pub struct BufferNotMappedError;