fix(gles): Tidy locking in struct Buffer (#9505)

This commit is contained in:
Andy Leiserson
2026-05-22 16:58:36 -07:00
committed by GitHub
parent ce445e882e
commit 22bea009e6
5 changed files with 61 additions and 44 deletions

1
Cargo.lock generated
View File

@@ -4865,6 +4865,7 @@ dependencies = [
"raw-window-metal",
"renderdoc-sys",
"smallvec",
"static_assertions",
"thiserror 2.0.18",
"wasm-bindgen",
"wayland-sys",

View File

@@ -215,6 +215,7 @@ bytemuck = { workspace = true, optional = true, features = ["derive"] }
hashbrown = { workspace = true }
ordered-float = { workspace = true, optional = true }
profiling = { workspace = true, optional = true, default-features = false }
static_assertions = { workspace = true }
# Backend: GLES
glow = { workspace = true, optional = true }

View File

@@ -571,9 +571,11 @@ impl crate::Device for super::Device {
target,
size: desc.size,
map_flags: 0,
mapped: false.into(),
data: Some(Arc::new(MaybeMutex::new(vec![0; desc.size as usize]))),
offset_of_current_mapping: Arc::new(MaybeMutex::new(0)),
map_state: Arc::new(MaybeMutex::new(super::BufferMapState {
mapped: false,
data: Some(vec![0; desc.size as usize]),
offset_of_current_mapping: 0,
})),
});
}
@@ -660,7 +662,7 @@ impl crate::Device for super::Device {
}
let data = if emulate_map && desc.usage.contains(wgt::BufferUses::MAP_READ) {
Some(Arc::new(MaybeMutex::new(vec![0; desc.size as usize])))
Some(vec![0; desc.size as usize])
} else {
None
};
@@ -671,10 +673,12 @@ impl crate::Device for super::Device {
raw,
target,
size: desc.size,
mapped: false.into(),
map_flags,
data,
offset_of_current_mapping: Arc::new(MaybeMutex::new(0)),
map_state: Arc::new(MaybeMutex::new(super::BufferMapState {
mapped: false,
data,
offset_of_current_mapping: 0,
})),
})
}
@@ -699,20 +703,21 @@ impl crate::Device for super::Device {
let is_coherent = buffer.map_flags & glow::MAP_COHERENT_BIT != 0;
let ptr = match buffer.raw {
None => {
let mut vec = lock(buffer.data.as_ref().unwrap());
let mut map_state = lock(&buffer.map_state);
let vec = map_state.data.as_mut().unwrap();
let slice = &mut vec.as_mut_slice()[range.start as usize..range.end as usize];
slice.as_mut_ptr()
}
Some(raw) => {
let gl = &self.shared.context.lock();
unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
let ptr = if let Some(ref map_read_allocation) = buffer.data {
let mut guard = lock(map_read_allocation);
let slice = guard.as_mut_slice();
let mut map_state = lock(&buffer.map_state);
let ptr = if let Some(map_read_allocation) = map_state.data.as_mut() {
let slice = map_read_allocation.as_mut_slice();
unsafe { self.shared.get_buffer_sub_data(gl, buffer.target, 0, slice) };
slice.as_mut_ptr()
} else {
*lock(&buffer.offset_of_current_mapping) = range.start;
map_state.offset_of_current_mapping = range.start;
// glMapBufferRange throws an error if length is 0.
// We want to allow mapping 0-sized buffer slices, so perform a workaround
// if the range length is 0. The resulting pointer must never be dereferenced.
@@ -724,7 +729,7 @@ impl crate::Device for super::Device {
.try_into()
.expect("Buffer range invalid for GLES");
if range_length != 0 {
buffer.mapped.set(true);
map_state.mapped = true;
unsafe {
gl.map_buffer_range(
buffer.target,
@@ -747,14 +752,15 @@ impl crate::Device for super::Device {
})
}
unsafe fn unmap_buffer(&self, buffer: &super::Buffer) {
if buffer.mapped.replace(false) {
let gl = &self.shared.context.lock();
let mut map_state = lock(&buffer.map_state);
if core::mem::replace(&mut map_state.mapped, false) {
if let Some(raw) = buffer.raw {
if buffer.data.is_none() {
let gl = &self.shared.context.lock();
if map_state.data.is_none() {
unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
unsafe { gl.unmap_buffer(buffer.target) };
unsafe { gl.bind_buffer(buffer.target, None) };
*lock(&buffer.offset_of_current_mapping) = 0;
map_state.offset_of_current_mapping = 0;
}
}
}
@@ -763,13 +769,14 @@ impl crate::Device for super::Device {
where
I: Iterator<Item = crate::MemoryRange>,
{
if buffer.mapped.get() {
let gl = &self.shared.context.lock();
let map_state = lock(&buffer.map_state);
if map_state.mapped {
if let Some(raw) = buffer.raw {
if buffer.data.is_none() {
let gl = &self.shared.context.lock();
if map_state.data.is_none() {
unsafe { gl.bind_buffer(buffer.target, Some(raw)) };
for range in ranges {
let offset_of_current_mapping = *lock(&buffer.offset_of_current_mapping);
let offset_of_current_mapping = map_state.offset_of_current_mapping;
unsafe {
gl.flush_mapped_buffer_range(
buffer.target,

View File

@@ -115,7 +115,6 @@ pub use self::wgl::{Instance, Surface};
use alloc::{boxed::Box, string::String, string::ToString as _, sync::Arc, vec::Vec};
use core::{
cell::Cell,
fmt,
ops::Range,
sync::atomic::{AtomicU32, AtomicU8},
@@ -345,17 +344,23 @@ pub struct Buffer {
size: wgt::BufferAddress,
/// Flags to use within calls to [`Device::map_buffer`](crate::Device::map_buffer).
map_flags: u32,
/// Buffer mapping state.
///
/// If locked concurrently with the GL context, the GL context should be locked first.
map_state: Arc<MaybeMutex<BufferMapState>>,
}
#[derive(Clone, Debug)]
struct BufferMapState {
/// True if the GL buffer is actually mapped, i.e. not "fake-mapped" with
/// an empty slice
mapped: Cell<bool>,
data: Option<Arc<MaybeMutex<Vec<u8>>>>,
offset_of_current_mapping: Arc<MaybeMutex<wgt::BufferAddress>>,
mapped: bool,
data: Option<Vec<u8>>,
offset_of_current_mapping: wgt::BufferAddress,
}
#[cfg(send_sync)]
unsafe impl Sync for Buffer {}
#[cfg(send_sync)]
unsafe impl Send for Buffer {}
static_assertions::assert_impl_all!(Buffer: Send, Sync);
impl crate::DynBuffer for Buffer {}

View File

@@ -377,7 +377,8 @@ impl super::Queue {
}
}
None => {
lock(dst.data.as_ref().unwrap()).as_mut_slice()
let mut map_state = lock(&dst.map_state);
map_state.data.as_mut().unwrap().as_mut_slice()
[range.start as usize..range.end as usize]
.fill(0);
}
@@ -419,8 +420,8 @@ impl super::Queue {
};
}
(Some(src), None) => {
let mut data = lock(dst.data.as_ref().unwrap());
let dst_data = &mut data.as_mut_slice()
let mut map_state = lock(&dst.map_state);
let dst_data = &mut map_state.data.as_mut().unwrap().as_mut_slice()
[copy.dst_offset as usize..copy.dst_offset as usize + size];
unsafe { gl.bind_buffer(copy_src_target, Some(src)) };
@@ -434,8 +435,8 @@ impl super::Queue {
};
}
(None, Some(dst)) => {
let data = lock(src.data.as_ref().unwrap());
let src_data = &data.as_slice()
let map_state = lock(&src.map_state);
let src_data = &map_state.data.as_ref().unwrap().as_slice()
[copy.src_offset as usize..copy.src_offset as usize + size];
unsafe { gl.bind_buffer(copy_dst_target, Some(dst)) };
unsafe {
@@ -775,7 +776,7 @@ impl super::Queue {
unsafe { gl.pixel_store_i32(glow::UNPACK_IMAGE_HEIGHT, column_texels as i32) };
let mut unbind_unpack_buffer = false;
if !dst_format.is_compressed() {
let buffer_data;
let map_state;
let unpack_data = match src.raw {
Some(buffer) => {
unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(buffer)) };
@@ -783,9 +784,9 @@ impl super::Queue {
glow::PixelUnpackData::BufferOffset(copy.buffer_layout.offset as u32)
}
None => {
buffer_data = lock(src.data.as_ref().unwrap());
let src_data =
&buffer_data.as_slice()[copy.buffer_layout.offset as usize..];
map_state = lock(&src.map_state);
let src_data = &map_state.data.as_ref().unwrap().as_slice()
[copy.buffer_layout.offset as usize..];
glow::PixelUnpackData::Slice(Some(src_data))
}
};
@@ -837,7 +838,7 @@ impl super::Queue {
(bytes_per_image * (copy.size.depth - 1)) + minimum_bytes_per_image;
let offset = copy.buffer_layout.offset as u32;
let buffer_data;
let map_state;
let unpack_data = match src.raw {
Some(buffer) => {
unsafe { gl.bind_buffer(glow::PIXEL_UNPACK_BUFFER, Some(buffer)) };
@@ -847,8 +848,8 @@ impl super::Queue {
)
}
None => {
buffer_data = lock(src.data.as_ref().unwrap());
let src_data = &buffer_data.as_slice()
map_state = lock(&src.map_state);
let src_data = &map_state.data.as_ref().unwrap().as_slice()
[(offset as usize)..(offset + bytes_in_upload) as usize];
glow::CompressedPixelUnpackData::Slice(src_data)
}
@@ -920,7 +921,7 @@ impl super::Queue {
unsafe { gl.bind_framebuffer(glow::READ_FRAMEBUFFER, Some(self.copy_fbo)) };
let read_pixels = |offset| {
let mut buffer_data;
let mut map_state;
let unpack_data = match dst.raw {
Some(buffer) => {
unsafe { gl.pixel_store_i32(glow::PACK_ROW_LENGTH, row_texels as i32) };
@@ -928,8 +929,9 @@ impl super::Queue {
glow::PixelPackData::BufferOffset(offset as u32)
}
None => {
buffer_data = lock(dst.data.as_ref().unwrap());
let dst_data = &mut buffer_data.as_mut_slice()[offset as usize..];
map_state = lock(&dst.map_state);
let dst_data = &mut map_state.data.as_mut().unwrap().as_mut_slice()
[offset as usize..];
glow::PixelPackData::Slice(Some(dst_data))
}
};
@@ -1094,7 +1096,8 @@ impl super::Queue {
};
}
None => {
let data = &mut lock(dst.data.as_ref().unwrap());
let mut map_state = lock(&dst.map_state);
let data = map_state.data.as_mut().unwrap();
let len = query_data.len().min(data.len());
data[..len].copy_from_slice(&query_data[..len]);
}