Encapsulate buffer map fields in a separate struct

components/script/dom/gpubuffer.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use crate::dom::bindings::cell::{DomRefCell, RefCell};
+use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::codegen::Bindings::GPUBufferBinding::{GPUBufferMethods, GPUSize64};
 use crate::dom::bindings::codegen::Bindings::GPUMapModeBinding::GPUMapModeConstants;
 use crate::dom::bindings::error::{Error, Fallible};
@@ -19,7 +19,7 @@ use dom_struct::dom_struct;
 use js::jsapi::DetachArrayBuffer;
 use js::jsapi::NewExternalArrayBuffer;
 use js::jsapi::{Heap, JSObject};
-use std::cell::Cell;
+use std::cell::{Cell, RefCell};
 use std::ffi::c_void;
 use std::ops::Range;
 use std::ptr::NonNull;
@@ -41,6 +41,17 @@ pub enum GPUBufferState {
     Destroyed,
 }
 
+#[derive(JSTraceable, MallocSizeOf)]
+pub struct GPUBufferMapInfo {
+    #[ignore_malloc_size_of = "Rc"]
+    pub mapping: Rc<RefCell<Vec<u8>>>,
+    pub mapping_range: Range<u64>,
+    pub mapped_ranges: Vec<Range<u64>>,
+    #[ignore_malloc_size_of = "defined in mozjs"]
+    pub js_buffers: Vec<Box<Heap<*mut JSObject>>>,
+    pub map_mode: Option<u32>,
+}
+
 #[dom_struct]
 pub struct GPUBuffer {
     reflector_: Reflector,
@@ -51,16 +62,10 @@ pub struct GPUBuffer {
     buffer: WebGPUBuffer,
     device: WebGPUDevice,
     valid: Cell<bool>,
-    #[ignore_malloc_size_of = "defined in mozjs"]
-    mapping: Rc<RefCell<Option<Vec<u8>>>>,
-    mapping_range: DomRefCell<Option<Range<u64>>>,
-    mapped_ranges: DomRefCell<Option<Vec<Range<u64>>>>,
-    #[ignore_malloc_size_of = "defined in mozjs"]
-    js_buffers: DomRefCell<Option<Vec<Box<Heap<*mut JSObject>>>>>,
-    #[ignore_malloc_size_of = "defined in mozjs"]
-    map_promise: DomRefCell<Option<Rc<Promise>>>,
     size: GPUSize64,
-    map_mode: Cell<Option<u32>>,
+    #[ignore_malloc_size_of = "promises are hard"]
+    map_promise: DomRefCell<Option<Rc<Promise>>>,
+    map_info: DomRefCell<Option<GPUBufferMapInfo>>,
 }
 
 impl GPUBuffer {
@@ -71,8 +76,7 @@ impl GPUBuffer {
         state: GPUBufferState,
         size: GPUSize64,
         valid: bool,
-        mapping: Rc<RefCell<Option<Vec<u8>>>>,
+        map_info: DomRefCell<Option<GPUBufferMapInfo>>,
-        mapping_range: Range<u64>,
     ) -> Self {
         Self {
             reflector_: Reflector::new(),
@@ -82,13 +86,9 @@ impl GPUBuffer {
             valid: Cell::new(valid),
             device,
             buffer,
-            mapping,
-            mapped_ranges: DomRefCell::new(None),
-            js_buffers: DomRefCell::new(None),
             map_promise: DomRefCell::new(None),
             size,
-            mapping_range: DomRefCell::new(Some(mapping_range)),
+            map_info,
-            map_mode: Cell::new(None),
         }
     }
 
@@ -101,19 +101,11 @@ impl GPUBuffer {
         state: GPUBufferState,
         size: GPUSize64,
         valid: bool,
-        mapping: Rc<RefCell<Option<Vec<u8>>>>,
+        map_info: DomRefCell<Option<GPUBufferMapInfo>>,
-        mapping_range: Range<u64>,
     ) -> DomRoot<Self> {
         reflect_dom_object(
             Box::new(GPUBuffer::new_inherited(
-                channel,
+                channel, buffer, device, state, size, valid, map_info,
-                buffer,
-                device,
-                state,
-                size,
-                valid,
-                mapping,
-                mapping_range,
             )),
             global,
         )
@@ -153,23 +145,22 @@ impl GPUBufferMethods for GPUBuffer {
             },
             // Step 3
             GPUBufferState::Mapped | GPUBufferState::MappedAtCreation => {
+                let mut info = self.map_info.borrow_mut();
+                let m_info = info.as_mut().unwrap();
+                let m_range = m_info.mapping_range.clone();
                 if let Err(e) = self.channel.0.send(WebGPURequest::UnmapBuffer {
                     buffer_id: self.id().0,
-                    array_buffer: self.mapping.borrow().as_ref().unwrap().clone(),
+                    array_buffer: m_info.mapping.borrow().clone(),
-                    is_map_read: self.map_mode.get() == Some(GPUMapModeConstants::READ),
+                    is_map_read: m_info.map_mode == Some(GPUMapModeConstants::READ),
-                    offset: self.mapping_range.borrow().as_ref().unwrap().start,
+                    offset: m_range.start,
-                    size: self.mapping_range.borrow().as_ref().unwrap().end -
+                    size: m_range.end - m_range.start,
-                        self.mapping_range.borrow().as_ref().unwrap().start,
                 }) {
                     warn!("Failed to send Buffer unmap ({:?}) ({})", self.buffer.0, e);
                 }
                 // Step 3.3
-                let mut bufs = self.js_buffers.borrow_mut().take().unwrap();
+                m_info.js_buffers.drain(..).for_each(|obj| unsafe {
-                bufs.drain(..).for_each(|obj| unsafe {
                     DetachArrayBuffer(*cx, obj.handle());
                 });
-                *self.mapped_ranges.borrow_mut() = None;
-                *self.mapping.borrow_mut() = None;
             },
             // Step 2
             GPUBufferState::MappingPending => {
@@ -179,8 +170,7 @@ impl GPUBufferMethods for GPUBuffer {
         };
         // Step 4
         self.state.set(GPUBufferState::Unmapped);
-        self.map_mode.set(None);
+        *self.map_info.borrow_mut() = None;
-        *self.mapping_range.borrow_mut() = None;
     }
 
     /// https://gpuweb.github.io/gpuweb/#dom-gpubuffer-destroy
@@ -254,8 +244,13 @@ impl GPUBufferMethods for GPUBuffer {
         }
 
         self.state.set(GPUBufferState::MappingPending);
-        self.map_mode.set(Some(mode));
+        *self.map_info.borrow_mut() = Some(GPUBufferMapInfo {
-        *self.mapping_range.borrow_mut() = Some(map_range);
+            mapping: Rc::new(RefCell::new(Vec::with_capacity(0))),
+            mapping_range: map_range,
+            mapped_ranges: Vec::new(),
+            js_buffers: Vec::new(),
+            map_mode: Some(mode),
+        });
         *self.map_promise.borrow_mut() = Some(promise.clone());
         promise
     }
@@ -268,28 +263,22 @@ impl GPUBufferMethods for GPUBuffer {
         offset: GPUSize64,
         size: GPUSize64,
     ) -> Fallible<NonNull<JSObject>> {
-        if self.mapped_ranges.borrow().is_none() {
+        let m_end = if size == 0 { self.size } else { offset + size };
-            *self.mapped_ranges.borrow_mut() = Some(Vec::new());
+        let mut info = self.map_info.borrow_mut();
-        }
+        let m_info = info.as_mut().unwrap();
-        if self.js_buffers.borrow().is_none() {
+
-            *self.js_buffers.borrow_mut() = Some(Vec::new());
-        }
-        let act_size = if size == 0 { self.size - offset } else { size };
         let mut valid = match self.state.get() {
             GPUBufferState::Mapped | GPUBufferState::MappedAtCreation => true,
             _ => false,
         };
         valid &= offset % RANGE_OFFSET_ALIGN_MASK == 0 &&
-            act_size % RANGE_SIZE_ALIGN_MASK == 0 &&
+            (m_end - offset) % RANGE_SIZE_ALIGN_MASK == 0 &&
-            offset >= self.mapping_range.borrow().as_ref().unwrap().start &&
+            offset >= m_info.mapping_range.start &&
-            offset + act_size <= self.mapping_range.borrow().as_ref().unwrap().end;
+            m_end <= m_info.mapping_range.end;
-        valid &= self
+        valid &= m_info
             .mapped_ranges
-            .borrow()
-            .as_ref()
-            .unwrap()
             .iter()
-            .all(|range| range.start > offset + act_size || range.end < offset);
+            .all(|range| range.start >= m_end || range.end <= offset);
         if !valid {
             return Err(Error::Operation);
         }
@@ -301,22 +290,15 @@ impl GPUBufferMethods for GPUBuffer {
         let array_buffer = unsafe {
             NewExternalArrayBuffer(
                 *cx,
-                act_size as usize,
+                (m_end - offset) as usize,
-                self.mapping.borrow_mut().as_mut().unwrap()
+                m_info.mapping.borrow_mut()[offset as usize..m_end as usize].as_mut_ptr() as _,
-                    [offset as usize..(offset + act_size) as usize]
-                    .as_mut_ptr() as _,
                 Some(free_func),
-                Rc::into_raw(self.mapping.clone()) as _,
+                Rc::into_raw(m_info.mapping.clone()) as _,
             )
         };
-        self.mapped_ranges
+
-            .borrow_mut()
+        m_info.mapped_ranges.push(offset..m_end);
-            .as_mut()
+        m_info.js_buffers.push(Heap::boxed(array_buffer));
-            .map(|v| v.push(offset..offset + act_size));
-        self.js_buffers
-            .borrow_mut()
-            .as_mut()
-            .map(|a| a.push(Heap::boxed(array_buffer)));
 
         Ok(NonNull::new(array_buffer).unwrap())
     }
@@ -337,7 +319,13 @@ impl AsyncWGPUListener for GPUBuffer {
     fn handle_response(&self, response: WebGPUResponse, promise: &Rc<Promise>) {
         match response {
             WebGPUResponse::BufferMapAsync(bytes) => {
-                *self.mapping.borrow_mut() = Some(bytes);
+                *self
+                    .map_info
+                    .borrow_mut()
+                    .as_mut()
+                    .unwrap()
+                    .mapping
+                    .borrow_mut() = bytes;
                 promise.resolve_native(&());
                 self.state.set(GPUBufferState::Mapped);
             },

components/script/dom/gpudevice.rs

@@ -4,7 +4,7 @@
 
 #![allow(unsafe_code)]
 
-use crate::dom::bindings::cell::{DomRefCell, RefCell};
+use crate::dom::bindings::cell::DomRefCell;
 use crate::dom::bindings::codegen::Bindings::GPUBindGroupBinding::{
     GPUBindGroupDescriptor, GPUBindingResource,
 };
@@ -41,7 +41,7 @@ use crate::dom::globalscope::GlobalScope;
 use crate::dom::gpuadapter::GPUAdapter;
 use crate::dom::gpubindgroup::GPUBindGroup;
 use crate::dom::gpubindgrouplayout::GPUBindGroupLayout;
-use crate::dom::gpubuffer::{GPUBuffer, GPUBufferState};
+use crate::dom::gpubuffer::{GPUBuffer, GPUBufferMapInfo, GPUBufferState};
 use crate::dom::gpucommandencoder::GPUCommandEncoder;
 use crate::dom::gpucomputepipeline::GPUComputePipeline;
 use crate::dom::gpupipelinelayout::GPUPipelineLayout;
@@ -54,6 +54,7 @@ use crate::script_runtime::JSContext as SafeJSContext;
 use arrayvec::ArrayVec;
 use dom_struct::dom_struct;
 use js::jsapi::{Heap, JSObject};
+use std::cell::RefCell;
 use std::ptr::NonNull;
 use std::rc::Rc;
 use webgpu::wgpu::binding_model::BufferBinding;
@@ -177,18 +178,21 @@ impl GPUDeviceMethods for GPUDevice {
             .expect("Failed to create WebGPU buffer");
 
         let buffer = webgpu::WebGPUBuffer(id);
-        let mapping;
+        let map_info;
         let state;
-        let mapping_range;
         if descriptor.mappedAtCreation {
             let buf_data = vec![0u8; descriptor.size as usize];
-            mapping = Rc::new(RefCell::new(Some(buf_data)));
+            map_info = DomRefCell::new(Some(GPUBufferMapInfo {
+                mapping: Rc::new(RefCell::new(buf_data)),
+                mapping_range: 0..descriptor.size,
+                mapped_ranges: Vec::new(),
+                js_buffers: Vec::new(),
+                map_mode: None,
+            }));
             state = GPUBufferState::MappedAtCreation;
-            mapping_range = 0..descriptor.size;
         } else {
-            mapping = Rc::new(RefCell::new(None));
+            map_info = DomRefCell::new(None);
             state = GPUBufferState::Unmapped;
-            mapping_range = 0..0;
         }
 
         GPUBuffer::new(
@@ -199,8 +203,7 @@ impl GPUDeviceMethods for GPUDevice {
             state,
             descriptor.size,
             true,
-            mapping,
+            map_info,
-            mapping_range,
         )
     }