Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 6, 2023
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8d3d7ec
Add reusable buffer mappings for WASM
DouglasDwyer Nov 20, 2023
620ee5d
Run cargo fmt
DouglasDwyer Nov 20, 2023
2cff52a
Update CHANGELOG.md
DouglasDwyer Nov 20, 2023
9d5a386
Merge remote-tracking branch 'upstream/trunk' into trunk
DouglasDwyer Dec 2, 2023
cb2f904
Update web.rs
DouglasDwyer Dec 2, 2023
dca0aea
Add documentation for WebBuffer struct
DouglasDwyer Dec 3, 2023
5dcdd3d
Merge remote-tracking branch 'upstream/trunk' into trunk
DouglasDwyer Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,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.
Expand Down
146 changes: 101 additions & 45 deletions wgpu/src/backend/web.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -1018,8 +1018,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>;
Expand Down Expand Up @@ -1611,7 +1611,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);
Expand Down Expand Up @@ -1814,7 +1815,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(
Expand Down Expand Up @@ -2022,12 +2026,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));
}

Expand All @@ -2037,9 +2043,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,
Expand All @@ -2053,14 +2057,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(
Expand Down Expand Up @@ -2100,7 +2102,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) {
Expand Down Expand Up @@ -2236,9 +2238,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,
)
Expand Down Expand Up @@ -2452,14 +2454,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),
}
}

Expand Down Expand Up @@ -2521,7 +2523,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,
);
}
Expand Down Expand Up @@ -2563,7 +2565,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,
Expand All @@ -2580,7 +2582,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");
Expand All @@ -2601,8 +2603,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(())
Expand Down Expand Up @@ -2856,9 +2858,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(
Expand Down Expand Up @@ -2910,15 +2913,15 @@ 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,
);
}
None => {
encoder_data.0.set_index_buffer_with_f64(
&buffer_data.0,
&buffer_data.0.buffer,
map_index_format(index_format),
offset as f64,
);
Expand All @@ -2940,15 +2943,15 @@ 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,
);
}
None => {
encoder_data.0.set_vertex_buffer_with_f64(
slot,
Some(&buffer_data.0),
Some(&buffer_data.0.buffer),
offset as f64,
);
}
Expand Down Expand Up @@ -3012,7 +3015,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(
Expand All @@ -3025,7 +3028,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(
Expand Down Expand Up @@ -3131,15 +3134,15 @@ 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,
);
}
None => {
pass_data.0.set_index_buffer_with_f64(
&buffer_data.0,
&buffer_data.0.buffer,
map_index_format(index_format),
offset as f64,
);
Expand All @@ -3161,15 +3164,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,
);
}
};
}
Expand Down Expand Up @@ -3231,7 +3236,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(
Expand All @@ -3244,7 +3249,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(
Expand Down Expand Up @@ -3462,6 +3467,57 @@ impl QueueWriteBuffer for WebQueueWriteBuffer {
}
}

#[derive(Debug)]
pub struct WebBuffer {
buffer: web_sys::GpuBuffer,
mapping: RefCell<WebBufferMapState>,
}

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<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,
)
}

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,
)
}

fn set_mapped_range(&self, range: Range<wgt::BufferAddress>) {
self.mapping.borrow_mut().range = range;
}
}

#[derive(Debug)]
struct WebBufferMapState {
pub mapped_buffer: Option<js_sys::ArrayBuffer>,
pub range: Range<wgt::BufferAddress>,
}

#[derive(Debug)]
pub struct BufferMappedRange {
actual_mapping: js_sys::Uint8Array,
Expand Down