diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e03e6c83e..30fa02f7aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,6 +154,10 @@ Passing an owned value `window` to `Surface` will return a `Surface<'static>`. S ### Bug Fixes +#### WebGPU + +- Allow calling `BufferSlice::get_mapped_range` multiple times on the same buffer slice (instead of throwing a Javascript exception): By @DouglasDwyer in [#4726](https://github.com/gfx-rs/wgpu/pull/4726) + #### WGL - Create a hidden window per `wgpu::Instance` instead of sharing a global one. diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 1faf25bfee..9b59a2d4ba 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -569,7 +569,7 @@ fn map_texture_view_dimension( fn map_buffer_copy_view(view: crate::ImageCopyBuffer<'_>) -> web_sys::GpuImageCopyBuffer { let buffer: &::BufferData = downcast_ref(view.buffer.data.as_ref()); - let mut mapped = web_sys::GpuImageCopyBuffer::new(&buffer.0); + let mut mapped = web_sys::GpuImageCopyBuffer::new(&buffer.0.buffer); if let Some(bytes_per_row) = view.layout.bytes_per_row { mapped.bytes_per_row(bytes_per_row); } @@ -1022,8 +1022,8 @@ impl crate::context::Context for Context { type TextureViewData = Sendable; type SamplerId = Identified; type SamplerData = Sendable; - type BufferId = Identified; - type BufferData = Sendable; + type BufferId = Identified; + type BufferData = Sendable; type TextureId = Identified; type TextureData = Sendable; type QuerySetId = Identified; @@ -1616,7 +1616,8 @@ impl crate::context::Context for Context { }) => { let buffer: &::BufferData = downcast_ref(buffer.data.as_ref()); - let mut mapped_buffer_binding = web_sys::GpuBufferBinding::new(&buffer.0); + let mut mapped_buffer_binding = + web_sys::GpuBufferBinding::new(&buffer.0.buffer); mapped_buffer_binding.offset(offset as f64); if let Some(s) = size { mapped_buffer_binding.size(s.get() as f64); @@ -1819,7 +1820,10 @@ impl crate::context::Context for Context { if let Some(label) = desc.label { mapped_desc.label(label); } - create_identified(device_data.0.create_buffer(&mapped_desc)) + create_identified(WebBuffer::new( + device_data.0.create_buffer(&mapped_desc), + desc, + )) } fn device_create_texture( @@ -2027,12 +2031,14 @@ impl crate::context::Context for Context { range: Range, callback: crate::context::BufferMapCallback, ) { - let map_promise = buffer_data.0.map_async_with_f64_and_f64( + let map_promise = buffer_data.0.buffer.map_async_with_f64_and_f64( map_map_mode(mode), range.start as f64, (range.end - range.start) as f64, ); + buffer_data.0.set_mapped_range(range); + register_then_closures(&map_promise, callback, Ok(()), Err(crate::BufferAsyncError)); } @@ -2042,9 +2048,7 @@ impl crate::context::Context for Context { buffer_data: &Self::BufferData, sub_range: Range, ) -> Box { - let array_buffer = - self.buffer_get_mapped_range_as_array_buffer(_buffer, buffer_data, sub_range); - let actual_mapping = js_sys::Uint8Array::new(&array_buffer); + let actual_mapping = buffer_data.0.get_mapped_range(sub_range); let temporary_mapping = actual_mapping.to_vec(); Box::new(BufferMappedRange { actual_mapping, @@ -2058,14 +2062,12 @@ impl crate::context::Context for Context { buffer_data: &Self::BufferData, sub_range: Range, ) -> js_sys::ArrayBuffer { - buffer_data.0.get_mapped_range_with_f64_and_f64( - sub_range.start as f64, - (sub_range.end - sub_range.start) as f64, - ) + buffer_data.0.get_mapped_array_buffer(sub_range) } fn buffer_unmap(&self, _buffer: &Self::BufferId, buffer_data: &Self::BufferData) { - buffer_data.0.unmap(); + buffer_data.0.buffer.unmap(); + buffer_data.0.mapping.borrow_mut().mapped_buffer = None; } fn texture_create_view( @@ -2105,7 +2107,7 @@ impl crate::context::Context for Context { } fn buffer_destroy(&self, _buffer: &Self::BufferId, buffer_data: &Self::BufferData) { - buffer_data.0.destroy(); + buffer_data.0.buffer.destroy(); } fn buffer_drop(&self, _buffer: &Self::BufferId, _buffer_data: &Self::BufferData) { @@ -2241,9 +2243,9 @@ impl crate::context::Context for Context { encoder_data .0 .copy_buffer_to_buffer_with_f64_and_f64_and_f64( - &source_data.0, + &source_data.0.buffer, source_offset as f64, - &destination_data.0, + &destination_data.0.buffer, destination_offset as f64, copy_size as f64, ) @@ -2457,14 +2459,14 @@ impl crate::context::Context for Context { ) { let buffer: &::BufferData = downcast_ref(buffer.data.as_ref()); match size { - Some(size) => { - encoder_data - .0 - .clear_buffer_with_f64_and_f64(&buffer.0, offset as f64, size as f64) - } + Some(size) => encoder_data.0.clear_buffer_with_f64_and_f64( + &buffer.0.buffer, + offset as f64, + size as f64, + ), None => encoder_data .0 - .clear_buffer_with_f64(&buffer.0, offset as f64), + .clear_buffer_with_f64(&buffer.0.buffer, offset as f64), } } @@ -2526,7 +2528,7 @@ impl crate::context::Context for Context { &query_set_data.0, first_query, query_count, - &destination_data.0, + &destination_data.0.buffer, destination_offset as u32, ); } @@ -2568,7 +2570,7 @@ impl crate::context::Context for Context { queue_data .0 .write_buffer_with_f64_and_buffer_source_and_f64_and_f64( - &buffer_data.0, + &buffer_data.0.buffer, offset as f64, &js_sys::Uint8Array::from(data).buffer(), 0f64, @@ -2585,7 +2587,7 @@ impl crate::context::Context for Context { offset: wgt::BufferAddress, size: wgt::BufferSize, ) -> Option<()> { - let usage = wgt::BufferUsages::from_bits_truncate(buffer_data.0.usage()); + let usage = wgt::BufferUsages::from_bits_truncate(buffer_data.0.buffer.usage()); // TODO: actually send this down the error scope if !usage.contains(wgt::BufferUsages::COPY_DST) { log::error!("Destination buffer is missing the `COPY_DST` usage flag"); @@ -2606,8 +2608,8 @@ impl crate::context::Context for Context { ); return None; } - if write_size + offset > buffer_data.0.size() as u64 { - log::error!("copy of {}..{} would end up overrunning the bounds of the destination buffer of size {}", offset, offset + write_size, buffer_data.0.size()); + if write_size + offset > buffer_data.0.buffer.size() as u64 { + log::error!("copy of {}..{} would end up overrunning the bounds of the destination buffer of size {}", offset, offset + write_size, buffer_data.0.buffer.size()); return None; } Some(()) @@ -2861,9 +2863,10 @@ impl crate::context::Context for Context { indirect_buffer_data: &Self::BufferData, indirect_offset: wgt::BufferAddress, ) { - pass_data - .0 - .dispatch_workgroups_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + pass_data.0.dispatch_workgroups_indirect_with_f64( + &indirect_buffer_data.0.buffer, + indirect_offset as f64, + ); } fn render_bundle_encoder_set_pipeline( @@ -2915,7 +2918,7 @@ impl crate::context::Context for Context { match size { Some(s) => { encoder_data.0.set_index_buffer_with_f64_and_f64( - &buffer_data.0, + &buffer_data.0.buffer, map_index_format(index_format), offset as f64, s.get() as f64, @@ -2923,7 +2926,7 @@ impl crate::context::Context for Context { } None => { encoder_data.0.set_index_buffer_with_f64( - &buffer_data.0, + &buffer_data.0.buffer, map_index_format(index_format), offset as f64, ); @@ -2945,7 +2948,7 @@ impl crate::context::Context for Context { Some(s) => { encoder_data.0.set_vertex_buffer_with_f64_and_f64( slot, - Some(&buffer_data.0), + Some(&buffer_data.0.buffer), offset as f64, s.get() as f64, ); @@ -2953,7 +2956,7 @@ impl crate::context::Context for Context { None => { encoder_data.0.set_vertex_buffer_with_f64( slot, - Some(&buffer_data.0), + Some(&buffer_data.0.buffer), offset as f64, ); } @@ -3017,7 +3020,7 @@ impl crate::context::Context for Context { ) { encoder_data .0 - .draw_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + .draw_indirect_with_f64(&indirect_buffer_data.0.buffer, indirect_offset as f64); } fn render_bundle_encoder_draw_indexed_indirect( @@ -3030,7 +3033,7 @@ impl crate::context::Context for Context { ) { encoder_data .0 - .draw_indexed_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + .draw_indexed_indirect_with_f64(&indirect_buffer_data.0.buffer, indirect_offset as f64); } fn render_bundle_encoder_multi_draw_indirect( @@ -3136,7 +3139,7 @@ impl crate::context::Context for Context { match size { Some(s) => { pass_data.0.set_index_buffer_with_f64_and_f64( - &buffer_data.0, + &buffer_data.0.buffer, map_index_format(index_format), offset as f64, s.get() as f64, @@ -3144,7 +3147,7 @@ impl crate::context::Context for Context { } None => { pass_data.0.set_index_buffer_with_f64( - &buffer_data.0, + &buffer_data.0.buffer, map_index_format(index_format), offset as f64, ); @@ -3166,15 +3169,17 @@ impl crate::context::Context for Context { Some(s) => { pass_data.0.set_vertex_buffer_with_f64_and_f64( slot, - Some(&buffer_data.0), + Some(&buffer_data.0.buffer), offset as f64, s.get() as f64, ); } None => { - pass_data - .0 - .set_vertex_buffer_with_f64(slot, Some(&buffer_data.0), offset as f64); + pass_data.0.set_vertex_buffer_with_f64( + slot, + Some(&buffer_data.0.buffer), + offset as f64, + ); } }; } @@ -3236,7 +3241,7 @@ impl crate::context::Context for Context { ) { pass_data .0 - .draw_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + .draw_indirect_with_f64(&indirect_buffer_data.0.buffer, indirect_offset as f64); } fn render_pass_draw_indexed_indirect( @@ -3249,7 +3254,7 @@ impl crate::context::Context for Context { ) { pass_data .0 - .draw_indexed_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + .draw_indexed_indirect_with_f64(&indirect_buffer_data.0.buffer, indirect_offset as f64); } fn render_pass_multi_draw_indirect( @@ -3467,6 +3472,71 @@ impl QueueWriteBuffer for WebQueueWriteBuffer { } } +/// Stores the state of a GPU buffer and a reference to its mapped `ArrayBuffer` (if any). +/// The WebGPU specification forbids calling `getMappedRange` on a `web_sys::GpuBuffer` more than +/// once, so this struct stores the initial mapped range and re-uses it, allowing for multiple `get_mapped_range` +/// calls on the Rust-side. +#[derive(Debug)] +pub struct WebBuffer { + /// The associated GPU buffer. + buffer: web_sys::GpuBuffer, + /// The mapped array buffer and mapped range. + mapping: RefCell, +} + +impl WebBuffer { + /// Creates a new web buffer for the given Javascript object and description. + fn new(buffer: web_sys::GpuBuffer, desc: &crate::BufferDescriptor<'_>) -> Self { + Self { + buffer, + mapping: RefCell::new(WebBufferMapState { + mapped_buffer: None, + range: 0..desc.size, + }), + } + } + + /// Creates a raw Javascript array buffer over the provided range. + fn get_mapped_array_buffer(&self, sub_range: Range) -> js_sys::ArrayBuffer { + self.buffer.get_mapped_range_with_f64_and_f64( + sub_range.start as f64, + (sub_range.end - sub_range.start) as f64, + ) + } + + /// Obtains a reference to the re-usable buffer mapping as a Javascript array view. + fn get_mapped_range(&self, sub_range: Range) -> js_sys::Uint8Array { + let mut mapping = self.mapping.borrow_mut(); + let range = mapping.range.clone(); + let array_buffer = mapping.mapped_buffer.get_or_insert_with(|| { + self.buffer.get_mapped_range_with_f64_and_f64( + range.start as f64, + (range.end - range.start) as f64, + ) + }); + js_sys::Uint8Array::new_with_byte_offset_and_length( + array_buffer, + (sub_range.start - range.start) as u32, + (sub_range.end - sub_range.start) as u32, + ) + } + + /// Sets the range of the buffer which is presently mapped. + fn set_mapped_range(&self, range: Range) { + self.mapping.borrow_mut().range = range; + } +} + +/// Remembers which portion of a buffer has been mapped, along with a reference +/// to the mapped portion. +#[derive(Debug)] +struct WebBufferMapState { + /// The mapped memory of the buffer. + pub mapped_buffer: Option, + /// The total range which has been mapped in the buffer overall. + pub range: Range, +} + #[derive(Debug)] pub struct BufferMappedRange { actual_mapping: js_sys::Uint8Array,