address review comments

Cargo.lock

@@ -6814,7 +6814,7 @@ dependencies = [
 [[package]]
 name = "wgpu-core"
 version = "0.5.0"
-source = "git+https://github.com/gfx-rs/wgpu#fc4baa31072f96479f691a8d134eaf4ceb418df3"
+source = "git+https://github.com/gfx-rs/wgpu#71e853d6ce289cb529ce39c03263e7f101dd4db5"
 dependencies = [
  "arrayvec 0.5.1",
  "bitflags",
@@ -6842,7 +6842,7 @@ dependencies = [
 [[package]]
 name = "wgpu-types"
 version = "0.5.0"
-source = "git+https://github.com/gfx-rs/wgpu#fc4baa31072f96479f691a8d134eaf4ceb418df3"
+source = "git+https://github.com/gfx-rs/wgpu#71e853d6ce289cb529ce39c03263e7f101dd4db5"
 dependencies = [
  "bitflags",
  "serde",

components/script/dom/gpudevice.rs

@@ -60,13 +60,15 @@ use crate::script_runtime::JSContext as SafeJSContext;
 use arrayvec::ArrayVec;
 use dom_struct::dom_struct;
 use js::jsapi::{Heap, JSObject};
-use std::cell::{Cell, RefCell};
+use std::cell::RefCell;
 use std::collections::HashMap;
 use std::ptr::NonNull;
 use std::rc::Rc;
 use webgpu::wgpu::binding_model::BufferBinding;
 use webgpu::{self, wgt, WebGPU, WebGPUBindings, WebGPURequest};
 
+type ErrorScopeId = u64;
+
 #[derive(JSTraceable, MallocSizeOf)]
 struct ErrorScopeInfo {
     filter: GPUErrorFilter,
@@ -77,6 +79,13 @@ struct ErrorScopeInfo {
     promise: Option<Rc<Promise>>,
 }
 
+#[derive(JSTraceable, MallocSizeOf)]
+struct ScopeContext {
+    error_scopes: HashMap<ErrorScopeId, ErrorScopeInfo>,
+    scope_stack: Vec<ErrorScopeId>,
+    next_scope_id: ErrorScopeId,
+}
+
 #[dom_struct]
 pub struct GPUDevice {
     eventtarget: EventTarget,
@@ -90,13 +99,12 @@ pub struct GPUDevice {
     label: DomRefCell<Option<DOMString>>,
     device: webgpu::WebGPUDevice,
     default_queue: Dom<GPUQueue>,
-    error_scopes: DomRefCell<HashMap<u64, ErrorScopeInfo>>,
+    scope_context: DomRefCell<ScopeContext>,
-    scope_stack: DomRefCell<Vec<u64>>,
-    next_scope_id: Cell<u64>,
     #[ignore_malloc_size_of = "promises are hard"]
     lost_promise: DomRefCell<Option<Rc<Promise>>>,
     #[ignore_malloc_size_of = "defined in webgpu"]
-    bind_group_layouts: DomRefCell<Vec<(Vec<wgt::BindGroupLayoutEntry>, Dom<GPUBindGroupLayout>)>>,
+    bind_group_layouts:
+        DomRefCell<HashMap<Vec<wgt::BindGroupLayoutEntry>, Dom<GPUBindGroupLayout>>>,
 }
 
 impl GPUDevice {
@@ -117,11 +125,13 @@ impl GPUDevice {
             label: DomRefCell::new(None),
             device,
             default_queue: Dom::from_ref(queue),
-            error_scopes: DomRefCell::new(HashMap::new()),
+            scope_context: DomRefCell::new(ScopeContext {
-            scope_stack: DomRefCell::new(Vec::new()),
+                error_scopes: HashMap::new(),
-            next_scope_id: Cell::new(0),
+                scope_stack: Vec::new(),
+                next_scope_id: 0,
+            }),
             lost_promise: DomRefCell::new(None),
-            bind_group_layouts: DomRefCell::new(Vec::new()),
+            bind_group_layouts: DomRefCell::new(HashMap::new()),
         }
     }
 
@@ -149,35 +159,49 @@ impl GPUDevice {
         self.device
     }
 
-    pub fn handle_server_msg(&self, scope: u64, result: Result<(), GPUError>) {
+    pub fn handle_server_msg(&self, scope: ErrorScopeId, result: Result<(), GPUError>) {
-        let mut err_scope;
+        let mut context = self.scope_context.borrow_mut();
-        {
+        let remove = if let Some(mut err_scope) = context.error_scopes.get_mut(&scope) {
-            err_scope = self.error_scopes.borrow_mut().remove(&scope).unwrap();
+            err_scope.op_count -= 1;
-        }
+            match result {
-        err_scope.op_count -= 1;
+                Ok(()) => {},
-        match result {
+                Err(e) => {
-            Ok(()) => {},
+                    if err_scope.error.is_none() {
-            Err(e) => {
+                        err_scope.error = Some(e);
-                if err_scope.error.is_none() {
+                    }
-                    err_scope.error = Some(e);
+                },
-                }
+            }
-            },
+            if let Some(ref promise) = err_scope.promise {
-        }
+                if !promise.is_fulfilled() {
-        if let Some(ref promise) = err_scope.promise {
+                    if let Some(ref e) = err_scope.error {
-            if !promise.is_fulfilled() {
+                        match e {
-                if let Some(ref e) = err_scope.error {
+                            GPUError::GPUValidationError(v) => promise.resolve_native(&v),
-                    match e {
+                            GPUError::GPUOutOfMemoryError(w) => promise.resolve_native(&w),
-                        GPUError::GPUValidationError(v) => promise.resolve_native(&v),
+                        }
-                        GPUError::GPUOutOfMemoryError(w) => promise.resolve_native(&w),
+                    } else if err_scope.op_count == 0 {
+                        promise.resolve_native(&None::<GPUError>);
                     }
-                } else if err_scope.op_count == 0 {
-                    promise.resolve_native(&None::<GPUError>);
                 }
             }
+            err_scope.op_count == 0 && err_scope.promise.is_some()
+        } else {
+            warn!("Could not find ErrroScope with Id({})", scope);
+            false
+        };
+        if remove {
+            let _ = context.error_scopes.remove(&scope);
         }
-        if err_scope.op_count > 0 || err_scope.promise.is_none() {
+    }
-            let _ = self.error_scopes.borrow_mut().insert(scope, err_scope);
+
-        }
+    fn use_current_scope(&self) -> Option<ErrorScopeId> {
+        let mut context = self.scope_context.borrow_mut();
+        let scope_id = context.scope_stack.last().cloned();
+        scope_id.and_then(|s_id| {
+            context.error_scopes.get_mut(&s_id).map(|mut scope| {
+                scope.op_count += 1;
+                s_id
+            })
+        })
     }
 }
 
@@ -356,25 +380,20 @@ impl GPUDeviceMethods for GPUDevice {
             })
             .collect::<Vec<_>>();
 
-        let mut scope_id = None;
+        let scope_id = self.use_current_scope();
-        if let Some(s_id) = self.scope_stack.borrow_mut().last() {
+
-            if let Some(mut scope) = self.error_scopes.borrow_mut().get_mut(&s_id) {
-                scope.op_count += 1;
-                scope_id = Some(*s_id);
-            } else {
-                warn!("Could not find Error Scope for id {}", s_id);
-            }
-        }
         // Check for equivalent GPUBindGroupLayout
         {
-            for (bgl_ent, bgl) in self.bind_group_layouts.borrow().iter() {
+            let layout = self
-                if *bgl_ent == entries {
+                .bind_group_layouts
-                    let layout = DomRoot::from_ref(&**bgl);
+                .borrow()
-                    if let Some(i) = scope_id {
+                .get(&entries)
-                        self.handle_server_msg(i, Ok(()));
+                .map(|bgl| DomRoot::from_ref(&**bgl));
-                    }
+            if let Some(l) = layout {
-                    return layout;
+                if let Some(i) = scope_id {
+                    self.handle_server_msg(i, Ok(()));
                 }
+                return l;
             }
         }
 
@@ -399,7 +418,7 @@ impl GPUDeviceMethods for GPUDevice {
 
         self.bind_group_layouts
             .borrow_mut()
-            .push((entries, Dom::from_ref(&*layout)));
+            .insert(entries, Dom::from_ref(&*layout));
 
         layout
     }
@@ -416,15 +435,7 @@ impl GPUDeviceMethods for GPUDevice {
             }
         });
 
-        let mut scope_id = None;
+        let scope_id = self.use_current_scope();
-        if let Some(s_id) = self.scope_stack.borrow_mut().last() {
-            if let Some(mut scope) = self.error_scopes.borrow_mut().get_mut(&s_id) {
-                scope.op_count += 1;
-                scope_id = Some(*s_id);
-            } else {
-                warn!("Could not find Error Scope for id {}", s_id);
-            }
-        }
 
         let pipeline_layout_id = self
             .global()
@@ -476,15 +487,7 @@ impl GPUDeviceMethods for GPUDevice {
             })
             .collect::<Vec<_>>();
 
-        let mut scope_id = None;
+        let scope_id = self.use_current_scope();
-        if let Some(s_id) = self.scope_stack.borrow_mut().last() {
-            if let Some(mut scope) = self.error_scopes.borrow_mut().get_mut(&s_id) {
-                scope.op_count += 1;
-                scope_id = Some(*s_id);
-            } else {
-                warn!("Could not find Error Scope for id {}", s_id);
-            }
-        }
 
         let bind_group_id = self
             .global()
@@ -555,15 +558,7 @@ impl GPUDeviceMethods for GPUDevice {
             .lock()
             .create_compute_pipeline_id(self.device.0.backend());
 
-        let mut scope_id = None;
+        let scope_id = self.use_current_scope();
-        if let Some(s_id) = self.scope_stack.borrow_mut().last() {
-            if let Some(mut scope) = self.error_scopes.borrow_mut().get_mut(&s_id) {
-                scope.op_count += 1;
-                scope_id = Some(*s_id);
-            } else {
-                warn!("Could not find Error Scope for id {}", s_id);
-            }
-        }
 
         self.channel
             .0
@@ -813,15 +808,7 @@ impl GPUDeviceMethods for GPUDevice {
             .lock()
             .create_render_pipeline_id(self.device.0.backend());
 
-        let mut scope_id = None;
+        let scope_id = self.use_current_scope();
-        if let Some(s_id) = self.scope_stack.borrow_mut().last() {
-            if let Some(mut scope) = self.error_scopes.borrow_mut().get_mut(&s_id) {
-                scope.op_count += 1;
-                scope_id = Some(*s_id);
-            } else {
-                warn!("Could not find Error Scope for id {}", s_id);
-            }
-        }
 
         self.channel
             .0
@@ -852,43 +839,47 @@ impl GPUDeviceMethods for GPUDevice {
 
     /// https://gpuweb.github.io/gpuweb/#dom-gpudevice-pusherrorscope
     fn PushErrorScope(&self, filter: GPUErrorFilter) {
-        let scope_id = self.next_scope_id.get();
+        let mut context = self.scope_context.borrow_mut();
-        self.next_scope_id.set(scope_id + 1);
+        let scope_id = context.next_scope_id;
+        context.next_scope_id += 1;
         let err_scope = ErrorScopeInfo {
             filter,
             op_count: 0,
             error: None,
             promise: None,
         };
-        let res = self.error_scopes.borrow_mut().insert(scope_id, err_scope);
+        let res = context.error_scopes.insert(scope_id, err_scope);
-        self.scope_stack.borrow_mut().push(scope_id);
+        context.scope_stack.push(scope_id);
         assert!(res.is_none());
     }
 
     /// https://gpuweb.github.io/gpuweb/#dom-gpudevice-poperrorscope
     fn PopErrorScope(&self, comp: InRealm) -> Rc<Promise> {
+        let mut context = self.scope_context.borrow_mut();
         let promise = Promise::new_in_current_realm(&self.global(), comp);
-        let scope_id = if let Some(e) = self.scope_stack.borrow_mut().pop() {
+        let scope_id = if let Some(e) = context.scope_stack.pop() {
             e
         } else {
             promise.reject_error(Error::Operation);
             return promise;
         };
-        let mut err_scope;
+        let remove = if let Some(mut err_scope) = context.error_scopes.get_mut(&scope_id) {
-        {
+            if let Some(ref e) = err_scope.error {
-            err_scope = self.error_scopes.borrow_mut().remove(&scope_id).unwrap();
+                match e {
-        }
+                    GPUError::GPUValidationError(ref v) => promise.resolve_native(&v),
-        if let Some(ref e) = err_scope.error {
+                    GPUError::GPUOutOfMemoryError(ref w) => promise.resolve_native(&w),
-            match e {
+                }
-                GPUError::GPUValidationError(ref v) => promise.resolve_native(&v),
+            } else if err_scope.op_count == 0 {
-                GPUError::GPUOutOfMemoryError(ref w) => promise.resolve_native(&w),
+                promise.resolve_native(&None::<GPUError>);
             }
-        } else if err_scope.op_count == 0 {
+            err_scope.promise = Some(promise.clone());
-            promise.resolve_native(&None::<GPUError>);
+            err_scope.op_count == 0
-        }
+        } else {
-        err_scope.promise = Some(promise.clone());
+            error!("Could not find ErrorScope with Id({})", scope_id);
-        if err_scope.op_count > 0 {
+            false
-            self.error_scopes.borrow_mut().insert(scope_id, err_scope);
+        };
+        if remove {
+            let _ = context.error_scopes.remove(&scope_id);
         }
         promise
     }

components/webgpu/lib.rs

@@ -83,6 +83,7 @@ pub enum WebGPURequest {
     },
     CreateBindGroup {
         device_id: id::DeviceId,
+        // TODO: Consider using NonZeroU64 to reduce enum size
         scope_id: Option<u64>,
         bind_group_id: id::BindGroupId,
         bind_group_layout_id: id::BindGroupLayoutId,
@@ -480,25 +481,7 @@ impl<'a> WGPU<'a> {
                         let result = gfx_select!(bind_group_id =>
                             global.device_create_bind_group(device_id, &descriptor, bind_group_id));
                         if let Some(s_id) = scope_id {
-                            let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+                            self.send_result(device_id, s_id, result);
-                            let op_result;
-                            if let Err(e) = result {
-                                let error_msg = format!("{:?}", e);
-                                op_result = WebGPUOpResult::ValidationError(error_msg);
-                            } else {
-                                op_result = WebGPUOpResult::Success;
-                            }
-                            if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
-                                device: WebGPUDevice(device_id),
-                                scope_id: s_id,
-                                pipeline_id,
-                                result: op_result,
-                            }) {
-                                warn!(
-                                    "Failed to send BindGroupResult({:?}) ({})",
-                                    bind_group_id, w
-                                );
-                            }
                         }
                     },
                     WebGPURequest::CreateBindGroupLayout {
@@ -515,25 +498,7 @@ impl<'a> WGPU<'a> {
                         let result = gfx_select!(bind_group_layout_id =>
                             global.device_create_bind_group_layout(device_id, &descriptor, bind_group_layout_id));
                         if let Some(s_id) = scope_id {
-                            let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+                            self.send_result(device_id, s_id, result);
-                            let op_result;
-                            if let Err(e) = result {
-                                let error_msg = format!("{:?}", e);
-                                op_result = WebGPUOpResult::ValidationError(error_msg);
-                            } else {
-                                op_result = WebGPUOpResult::Success;
-                            }
-                            if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
-                                device: WebGPUDevice(device_id),
-                                pipeline_id,
-                                scope_id: s_id,
-                                result: op_result,
-                            }) {
-                                warn!(
-                                    "Failed to send BindGroupLayoutResult({:?}) ({})",
-                                    bind_group_layout_id, w
-                                );
-                            }
                         }
                     },
                     WebGPURequest::CreateBuffer {
@@ -574,25 +539,7 @@ impl<'a> WGPU<'a> {
                         let result = gfx_select!(compute_pipeline_id =>
                             global.device_create_compute_pipeline(device_id, &descriptor, compute_pipeline_id));
                         if let Some(s_id) = scope_id {
-                            let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+                            self.send_result(device_id, s_id, result);
-                            let op_result;
-                            if let Err(e) = result {
-                                let error_msg = format!("{:?}", e);
-                                op_result = WebGPUOpResult::ValidationError(error_msg);
-                            } else {
-                                op_result = WebGPUOpResult::Success;
-                            }
-                            if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
-                                device: WebGPUDevice(device_id),
-                                scope_id: s_id,
-                                pipeline_id,
-                                result: op_result,
-                            }) {
-                                warn!(
-                                    "Failed to send ComputePipelineResult({:?}) ({})",
-                                    compute_pipeline_id, w
-                                );
-                            }
                         }
                     },
                     WebGPURequest::CreateContext(sender) => {
@@ -619,25 +566,7 @@ impl<'a> WGPU<'a> {
                         let result = gfx_select!(pipeline_layout_id =>
                             global.device_create_pipeline_layout(device_id, &descriptor, pipeline_layout_id));
                         if let Some(s_id) = scope_id {
-                            let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+                            self.send_result(device_id, s_id, result);
-                            let op_result;
-                            if let Err(e) = result {
-                                let error_msg = format!("{:?}", e);
-                                op_result = WebGPUOpResult::ValidationError(error_msg);
-                            } else {
-                                op_result = WebGPUOpResult::Success;
-                            }
-                            if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
-                                device: WebGPUDevice(device_id),
-                                scope_id: s_id,
-                                pipeline_id,
-                                result: op_result,
-                            }) {
-                                warn!(
-                                    "Failed to send PipelineLayoutResult({:?}) ({})",
-                                    pipeline_layout_id, w
-                                );
-                            }
                         }
                     },
                     //TODO: consider https://github.com/gfx-rs/wgpu/issues/684
@@ -707,25 +636,7 @@ impl<'a> WGPU<'a> {
                         let result = gfx_select!(render_pipeline_id =>
                             global.device_create_render_pipeline(device_id, &descriptor, render_pipeline_id));
                         if let Some(s_id) = scope_id {
-                            let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+                            self.send_result(device_id, s_id, result);
-                            let op_result;
-                            if let Err(e) = result {
-                                let error_msg = format!("{:?}", e);
-                                op_result = WebGPUOpResult::ValidationError(error_msg);
-                            } else {
-                                op_result = WebGPUOpResult::Success;
-                            }
-                            if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
-                                device: WebGPUDevice(device_id),
-                                scope_id: s_id,
-                                pipeline_id,
-                                result: op_result,
-                            }) {
-                                warn!(
-                                    "Failed to send RenderPipelineResult({:?}) ({})",
-                                    render_pipeline_id, w
-                                );
-                            }
                         }
                     },
                     WebGPURequest::CreateSampler {
@@ -937,7 +848,7 @@ impl<'a> WGPU<'a> {
                         compute_pass,
                     } => {
                         let global = &self.global;
-                        gfx_select!(command_encoder_id => global.command_encoder_run_compute_pass(
+                        let _ = gfx_select!(command_encoder_id => global.command_encoder_run_compute_pass(
                             command_encoder_id,
                             &compute_pass
                         ));
@@ -947,7 +858,7 @@ impl<'a> WGPU<'a> {
                         render_pass,
                     } => {
                         let global = &self.global;
-                        gfx_select!(command_encoder_id => global.command_encoder_run_render_pass(
+                        let _ = gfx_select!(command_encoder_id => global.command_encoder_run_render_pass(
                             command_encoder_id,
                             &render_pass
                         ));
@@ -1179,6 +1090,27 @@ impl<'a> WGPU<'a> {
             }
         }
     }
+
+    fn send_result<U: id::TypedId, T: std::fmt::Debug>(
+        &self,
+        device_id: id::DeviceId,
+        scope_id: u64,
+        result: Result<U, T>,
+    ) {
+        let &pipeline_id = self.devices.get(&WebGPUDevice(device_id)).unwrap();
+        if let Err(w) = self.script_sender.send(WebGPUMsg::WebGPUOpResult {
+            device: WebGPUDevice(device_id),
+            scope_id,
+            pipeline_id,
+            result: if let Err(e) = result {
+                WebGPUOpResult::ValidationError(format!("{:?}", e))
+            } else {
+                WebGPUOpResult::Success
+            },
+        }) {
+            warn!("Failed to send WebGPUOpResult ({})", w);
+        }
+    }
 }
 
 fn convert_to_pointer<T: Sized>(obj: Rc<T>) -> *mut u8 {