From 8d3d7ec059f74b6d477106c4db5d436bce0b5cea Mon Sep 17 00:00:00 2001 From: Douglas Dwyer Date: Sun, 19 Nov 2023 20:54:28 -0500 Subject: [PATCH 1/5] Add reusable buffer mappings for WASM --- wgpu/src/backend/web.rs | 110 ++++++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 37 deletions(-) diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 73d5120aad..eb753bcf8c 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -567,7 +567,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); } @@ -1016,8 +1016,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; @@ -1609,7 +1609,7 @@ 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); @@ -1812,7 +1812,7 @@ 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( @@ -2016,12 +2016,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)); } @@ -2031,9 +2033,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, @@ -2047,14 +2047,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( @@ -2094,7 +2092,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) { @@ -2230,9 +2228,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, ) @@ -2447,13 +2445,13 @@ 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, + &buffer.0.buffer, offset as f64, size.get() 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), } } @@ -2515,7 +2513,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, ); } @@ -2557,7 +2555,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, @@ -2574,7 +2572,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"); @@ -2595,8 +2593,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(()) @@ -2852,7 +2850,7 @@ impl crate::context::Context for Context { ) { pass_data .0 - .dispatch_workgroups_indirect_with_f64(&indirect_buffer_data.0, indirect_offset as f64); + .dispatch_workgroups_indirect_with_f64(&indirect_buffer_data.0.buffer, indirect_offset as f64); } fn render_bundle_encoder_set_pipeline( @@ -2902,7 +2900,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, @@ -2910,7 +2908,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, ); @@ -2932,7 +2930,7 @@ impl crate::context::Context for Context { Some(s) => { encoder_data.0.set_vertex_buffer_with_f64_and_f64( slot, - &buffer_data.0, + &buffer_data.0.buffer, offset as f64, s.get() as f64, ); @@ -2940,7 +2938,7 @@ impl crate::context::Context for Context { None => { encoder_data .0 - .set_vertex_buffer_with_f64(slot, &buffer_data.0, offset as f64); + .set_vertex_buffer_with_f64(slot, &buffer_data.0.buffer, offset as f64); } }; } @@ -3002,7 +3000,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( @@ -3015,7 +3013,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( @@ -3121,7 +3119,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, @@ -3129,7 +3127,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, ); @@ -3151,7 +3149,7 @@ impl crate::context::Context for Context { Some(s) => { pass_data.0.set_vertex_buffer_with_f64_and_f64( slot, - &buffer_data.0, + &buffer_data.0.buffer, offset as f64, s.get() as f64, ); @@ -3159,7 +3157,7 @@ impl crate::context::Context for Context { None => { pass_data .0 - .set_vertex_buffer_with_f64(slot, &buffer_data.0, offset as f64); + .set_vertex_buffer_with_f64(slot, &buffer_data.0.buffer, offset as f64); } }; } @@ -3221,7 +3219,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( @@ -3234,7 +3232,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( @@ -3452,6 +3450,44 @@ impl QueueWriteBuffer for WebQueueWriteBuffer { } } +#[derive(Debug)] +pub struct WebBuffer { + buffer: web_sys::GpuBuffer, + mapping: RefCell +} + +impl WebBuffer { + fn new(buffer: web_sys::GpuBuffer, desc: &crate::BufferDescriptor<'_>) -> Self { + Self { + buffer, + mapping: RefCell::new(WebBufferMapState { mapped_buffer: None, range: 0..desc.size }) + } + } + + 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) + } + + 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) + } + + fn set_mapped_range(&self, range: Range) { + self.mapping.borrow_mut().range = range; + } +} + +#[derive(Debug)] +struct WebBufferMapState { + pub mapped_buffer: Option, + pub range: Range +} + #[derive(Debug)] pub struct BufferMappedRange { actual_mapping: js_sys::Uint8Array, From 620ee5d69b411d352bf8569c9aab77737f7dd767 Mon Sep 17 00:00:00 2001 From: Douglas Dwyer Date: Sun, 19 Nov 2023 20:56:40 -0500 Subject: [PATCH 2/5] Run cargo fmt --- wgpu/src/backend/web.rs | 48 +++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index eb753bcf8c..428bcf8f82 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -1609,7 +1609,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.buffer); + 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); @@ -1812,7 +1813,10 @@ impl crate::context::Context for Context { if let Some(label) = desc.label { mapped_desc.label(label); } - create_identified(WebBuffer::new(device_data.0.create_buffer(&mapped_desc), desc)) + create_identified(WebBuffer::new( + device_data.0.create_buffer(&mapped_desc), + desc, + )) } fn device_create_texture( @@ -2848,9 +2852,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.buffer, 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( @@ -2936,9 +2941,11 @@ impl crate::context::Context for Context { ); } None => { - encoder_data - .0 - .set_vertex_buffer_with_f64(slot, &buffer_data.0.buffer, offset as f64); + encoder_data.0.set_vertex_buffer_with_f64( + slot, + &buffer_data.0.buffer, + offset as f64, + ); } }; } @@ -3453,28 +3460,41 @@ impl QueueWriteBuffer for WebQueueWriteBuffer { #[derive(Debug)] pub struct WebBuffer { buffer: web_sys::GpuBuffer, - mapping: RefCell + mapping: RefCell, } impl WebBuffer { fn new(buffer: web_sys::GpuBuffer, desc: &crate::BufferDescriptor<'_>) -> Self { Self { buffer, - mapping: RefCell::new(WebBufferMapState { mapped_buffer: None, range: 0..desc.size }) + mapping: RefCell::new(WebBufferMapState { + mapped_buffer: None, + range: 0..desc.size, + }), } } 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) + self.buffer.get_mapped_range_with_f64_and_f64( + sub_range.start as f64, + (sub_range.end - sub_range.start) as f64, + ) } 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) + 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) + 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, + ) } fn set_mapped_range(&self, range: Range) { @@ -3485,7 +3505,7 @@ impl WebBuffer { #[derive(Debug)] struct WebBufferMapState { pub mapped_buffer: Option, - pub range: Range + pub range: Range, } #[derive(Debug)] From 2cff52a5f664bf948f37417e041c519b36229ca9 Mon Sep 17 00:00:00 2001 From: Douglas Dwyer Date: Sun, 19 Nov 2023 21:22:53 -0500 Subject: [PATCH 3/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 539aee7ef8..27ce123702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,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. From cb2f904082e360fa3e26c592afa7b772322a2fc9 Mon Sep 17 00:00:00 2001 From: Douglas Dwyer Date: Sat, 2 Dec 2023 15:15:42 -0500 Subject: [PATCH 4/5] Update web.rs --- wgpu/src/backend/web.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index f8d4f16edc..ed121fa013 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -2454,11 +2454,11 @@ 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.buffer, offset as f64), @@ -3170,9 +3170,11 @@ impl crate::context::Context for Context { ); } None => { - pass_data - .0 - .set_vertex_buffer_with_f64(slot, Some(&buffer_data.0.buffer), offset as f64); + pass_data.0.set_vertex_buffer_with_f64( + slot, + Some(&buffer_data.0.buffer), + offset as f64, + ); } }; } From dca0aeadda6620af9278e7cb7488858573467265 Mon Sep 17 00:00:00 2001 From: Douglas Dwyer Date: Sun, 3 Dec 2023 10:11:36 -0500 Subject: [PATCH 5/5] Add documentation for WebBuffer struct --- wgpu/src/backend/web.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index ed121fa013..c87d842f74 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -3467,13 +3467,20 @@ 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, @@ -3484,6 +3491,7 @@ impl WebBuffer { } } + /// 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, @@ -3491,6 +3499,7 @@ impl WebBuffer { ) } + /// 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(); @@ -3507,14 +3516,19 @@ impl WebBuffer { ) } + /// 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, }