Fix Javascript exception on repeated BufferSlice::get_mapped_range calls (#4726)

* Add reusable buffer mappings for WASM

* Run cargo fmt

* Update CHANGELOG.md

* Update web.rs

* Add documentation for WebBuffer struct
This commit is contained in:
Douglas Dwyer
2023-12-06 16:24:09 -05:00
committed by GitHub
parent 0f4df52b5a
commit 10253db555
2 changed files with 119 additions and 45 deletions

View File

@@ -153,6 +153,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.

View File

@@ -569,7 +569,7 @@ fn map_texture_view_dimension(
fn map_buffer_copy_view(view: crate::ImageCopyBuffer<'_>) -> web_sys::GpuImageCopyBuffer {
let buffer: &<Context as crate::Context>::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<web_sys::GpuTextureView>;
type SamplerId = Identified<web_sys::GpuSampler>;
type SamplerData = Sendable<web_sys::GpuSampler>;
type BufferId = Identified<web_sys::GpuBuffer>;
type BufferData = Sendable<web_sys::GpuBuffer>;
type BufferId = Identified<WebBuffer>;
type BufferData = Sendable<WebBuffer>;
type TextureId = Identified<web_sys::GpuTexture>;
type TextureData = Sendable<web_sys::GpuTexture>;
type QuerySetId = Identified<web_sys::GpuQuerySet>;
@@ -1616,7 +1616,8 @@ impl crate::context::Context for Context {
}) => {
let buffer: &<Context as crate::Context>::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<wgt::BufferAddress>,
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<wgt::BufferAddress>,
) -> Box<dyn crate::context::BufferMappedRange> {
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<wgt::BufferAddress>,
) -> 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: &<Context as crate::Context>::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<WebBufferMapState>,
}
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<wgt::BufferAddress>) -> 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<wgt::BufferAddress>) -> 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<wgt::BufferAddress>) {
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<js_sys::ArrayBuffer>,
/// The total range which has been mapped in the buffer overall.
pub range: Range<wgt::BufferAddress>,
}
#[derive(Debug)]
pub struct BufferMappedRange {
actual_mapping: js_sys::Uint8Array,