Don't create a raw bind group layout for duplicates.

wgpu-core/src/binding_model.rs

@@ -446,25 +446,64 @@ pub type BindGroupLayouts<A> = crate::storage::Storage<BindGroupLayout<A>, BindG
 ///  - produced bind groups
 ///  - produced pipeline layouts
 ///  - pipelines with implicit layouts
-#[derive(Debug)]
 pub struct BindGroupLayout<A: hal::Api> {
-    pub(crate) raw: A::BindGroupLayout,
     pub(crate) device_id: Stored<DeviceId>,
     pub(crate) multi_ref_count: MultiRefCount,
-    pub(crate) entries: BindEntryMap,
     // When a layout created and there already exists a compatible layout the new layout
     // keeps a reference to the older compatible one. In some places we substitute the
     // bind group layout id with its compatible sibling.
     // Since this substitution can come at a cost, it is skipped when wgpu-core generates
     // its own resource IDs.
-    pub(crate) compatible_layout: Option<Valid<BindGroupLayoutId>>,
+    pub(crate) inner: BglOrDuplicate<A>,
+}
+
+pub(crate) enum BglOrDuplicate<A: hal::Api> {
+    Inner(BindGroupLayoutInner<A>),
+    Duplicate(Valid<BindGroupLayoutId>),
+}
+
+pub struct BindGroupLayoutInner<A: hal::Api> {
+    pub(crate) raw: A::BindGroupLayout,
+    pub(crate) entries: BindEntryMap,
     #[allow(unused)]
     pub(crate) dynamic_count: usize,
     pub(crate) count_validator: BindingTypeMaxCountValidator,
     #[cfg(debug_assertions)]
     pub(crate) label: String,
 }
 
+impl<A: hal::Api> BindGroupLayout<A> {
+    pub(crate) fn assume_deduplicated(&self) -> &BindGroupLayoutInner<A> {
+        match &self.inner {
+            BglOrDuplicate::Inner(inner) => inner,
+            BglOrDuplicate::Duplicate(_) => {
+                panic!()
+            }
+        }
+    }
+
+    pub(crate) fn as_inner(&self) -> Option<&BindGroupLayoutInner<A>> {
+        match &self.inner {
+            BglOrDuplicate::Inner(inner) => Some(inner),
+            BglOrDuplicate::Duplicate(_) => None,
+        }
+    }
+
+    pub(crate) fn into_inner(self) -> Option<BindGroupLayoutInner<A>> {
+        match self.inner {
+            BglOrDuplicate::Inner(inner) => Some(inner),
+            BglOrDuplicate::Duplicate(_) => None,
+        }
+    }
+
+    pub(crate) fn as_duplicate(&self) -> Option<Valid<BindGroupLayoutId>> {
+        match self.inner {
+            BglOrDuplicate::Duplicate(id) => Some(id),
+            BglOrDuplicate::Inner(_) => None,
+        }
+    }
+}
+
 impl<A: hal::Api> Resource for BindGroupLayout<A> {
     const TYPE: &'static str = "BindGroupLayout";
 
@@ -474,7 +513,11 @@ impl<A: hal::Api> Resource for BindGroupLayout<A> {
 
     fn label(&self) -> &str {
         #[cfg(debug_assertions)]
-        return &self.label;
+        return if let BglOrDuplicate::Inner(inner) = &self.inner {
+            &inner.label
+        } else {
+            ""
+        };
         #[cfg(not(debug_assertions))]
         return "";
     }
@@ -486,8 +529,8 @@ pub(crate) fn try_get_bind_group_layout<A: HalApi>(
     id: BindGroupLayoutId,
 ) -> Option<&BindGroupLayout<A>> {
     let layout = layouts.get(id).ok()?;
-    if let Some(compat) = layout.compatible_layout {
-        return Some(&layouts[compat]);
+    if let BglOrDuplicate::Duplicate(original_id) = layout.inner {
+        return Some(&layouts[original_id]);
     }
 
     Some(layout)
@@ -498,10 +541,10 @@ pub(crate) fn get_bind_group_layout<A: HalApi>(
     id: Valid<BindGroupLayoutId>,
 ) -> (Valid<BindGroupLayoutId>, &BindGroupLayout<A>) {
     let layout = &layouts[id];
-    layout
-        .compatible_layout
-        .map(|compat| (compat, &layouts[compat]))
-        .unwrap_or((id, layout))
+    if let BglOrDuplicate::Duplicate(original_id) = layout.inner {
+        return (original_id, &layouts[original_id]);
+    }
+    (id, layout)
 }
 
 #[derive(Clone, Debug, Error)]

wgpu-core/src/command/bind.rs

@@ -38,7 +38,7 @@ mod compat {
             }
 
             if let Some(id) = self.assigned {
-                return bind_group_layouts[id].compatible_layout == self.expected;
+                return bind_group_layouts[id].as_duplicate() == self.expected;
             }
 
             false

wgpu-core/src/device/global.rs

@@ -1,7 +1,8 @@
 #[cfg(feature = "trace")]
 use crate::device::trace;
 use crate::{
-    binding_model, command, conv,
+    binding_model::{self, BindGroupLayout},
+    command, conv,
     device::{life::WaitIdleError, map_buffer, queue, Device, DeviceError, HostMap},
     global::Global,
     hal_api::HalApi,
@@ -1115,7 +1116,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             }
 
             let mut compatible_layout = None;
-            {
+            let layout = {
                 let (bgl_guard, _) = hub.bind_group_layouts.read(&mut token);
                 if let Some(id) =
                     Device::deduplicate_bind_group_layout(device_id, &entry_map, &*bgl_guard)
@@ -1134,19 +1135,26 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
                     compatible_layout = Some(id::Valid(id));
                 }
-            }
 
-            let mut layout = match device.create_bind_group_layout(
-                device_id,
-                desc.label.borrow_option(),
-                entry_map,
-            ) {
-                Ok(layout) => layout,
-                Err(e) => break e,
+                if let Some(original_id) = compatible_layout {
+                    let original = &bgl_guard[original_id];
+                    BindGroupLayout {
+                        device_id: original.device_id.clone(),
+                        inner: crate::binding_model::BglOrDuplicate::Duplicate(original_id),
+                        multi_ref_count: crate::MultiRefCount::new(),
+                    }
+                } else {
+                    match device.create_bind_group_layout(
+                        device_id,
+                        desc.label.borrow_option(),
+                        entry_map,
+                    ) {
+                        Ok(layout) => layout,
+                        Err(e) => break e,
+                    }
+                }
             };
 
-            layout.compatible_layout = compatible_layout;
-
             let id = fid.assign(layout, &mut token);
 
             if let Some(dupe) = compatible_layout {
@@ -1319,7 +1327,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             };
 
             let mut layout_id = id::Valid(desc.layout);
-            if let Some(id) = bind_group_layout.compatible_layout {
+            if let Some(id) = bind_group_layout.as_duplicate() {
                 layout_id = id;
                 bind_group_layout = &bind_group_layout_guard[id];
             }

wgpu-core/src/device/life.rs

@@ -1,6 +1,7 @@
 #[cfg(feature = "trace")]
 use crate::device::trace;
 use crate::{
+    binding_model::BglOrDuplicate,
     device::{
         queue::{EncoderInFlight, SubmittedWorkDoneClosure, TempResource},
         DeviceError,
@@ -776,7 +777,7 @@ impl<A: HalApi> LifetimeTracker<A> {
                     if bgl.multi_ref_count.dec_and_check_empty() {
                         // If This layout points to a compatible one, go over the latter
                         // to decrement the ref count and potentially destroy it.
-                        bgl_to_check = bgl.compatible_layout;
+                        bgl_to_check = bgl.as_duplicate();
 
                         log::debug!("Bind group layout {:?} will be destroyed", id);
                         #[cfg(feature = "trace")]
@@ -786,7 +787,9 @@ impl<A: HalApi> LifetimeTracker<A> {
                         if let Some(lay) =
                             hub.bind_group_layouts.unregister_locked(id.0, &mut *guard)
                         {
-                            self.free_resources.bind_group_layouts.push(lay.raw);
+                            if let BglOrDuplicate::Inner(inner) = lay.inner {
+                                self.free_resources.bind_group_layouts.push(inner.raw);
+                            }
                         }
                     }
                 }

wgpu-core/src/device/resource.rs

@@ -1,7 +1,10 @@
 #[cfg(feature = "trace")]
 use crate::device::trace;
 use crate::{
-    binding_model::{self, get_bind_group_layout, try_get_bind_group_layout},
+    binding_model::{
+        self, get_bind_group_layout, try_get_bind_group_layout, BglOrDuplicate,
+        BindGroupLayoutInner,
+    },
     command, conv,
     device::life::WaitIdleError,
     device::{
@@ -1391,9 +1394,15 @@ impl<A: HalApi> Device<A> {
         guard
             .iter(self_id.backend())
             .find(|&(_, bgl)| {
-                bgl.device_id.value.0 == self_id
-                    && bgl.compatible_layout.is_none()
-                    && bgl.entries == *entry_map
+                if bgl.device_id.value.0 != self_id {
+                    return false;
+                }
+
+                if let Some(inner) = bgl.as_inner() {
+                    return inner.entries == *entry_map;
+                }
+
+                false
             })
             .map(|(id, value)| {
                 value.multi_ref_count.inc();
@@ -1408,7 +1417,12 @@ impl<A: HalApi> Device<A> {
         pipeline_layout
             .bind_group_layout_ids
             .iter()
-            .map(|&id| &bgl_guard[id].entries)
+            .map(|&id| {
+                &get_bind_group_layout(bgl_guard, id)
+                    .1
+                    .assume_deduplicated()
+                    .entries
+            })
             .collect()
     }
 
@@ -1427,7 +1441,9 @@ impl<A: HalApi> Device<A> {
             .iter()
             .enumerate()
             .map(|(group_index, &bgl_id)| pipeline::LateSizedBufferGroup {
-                shader_sizes: bgl_guard[bgl_id]
+                shader_sizes: get_bind_group_layout(bgl_guard, bgl_id)
+                    .1
+                    .assume_deduplicated()
                     .entries
                     .values()
                     .filter_map(|entry| match entry.ty {
@@ -1639,21 +1655,22 @@ impl<A: HalApi> Device<A> {
             .map_err(binding_model::CreateBindGroupLayoutError::TooManyBindings)?;
 
         Ok(binding_model::BindGroupLayout {
-            raw,
             device_id: Stored {
                 value: id::Valid(self_id),
                 ref_count: self.life_guard.add_ref(),
             },
             multi_ref_count: MultiRefCount::new(),
-            dynamic_count: entry_map
-                .values()
-                .filter(|b| b.ty.has_dynamic_offset())
-                .count(),
-            count_validator,
-            entries: entry_map,
-            #[cfg(debug_assertions)]
-            label: label.unwrap_or("").to_string(),
-            compatible_layout: None,
+            inner: BglOrDuplicate::Inner(BindGroupLayoutInner {
+                raw,
+                dynamic_count: entry_map
+                    .values()
+                    .filter(|b| b.ty.has_dynamic_offset())
+                    .count(),
+                count_validator,
+                entries: entry_map,
+                #[cfg(debug_assertions)]
+                label: label.unwrap_or("").to_string(),
+            }),
         })
     }
 
@@ -1823,6 +1840,8 @@ impl<A: HalApi> Device<A> {
         Ok(())
     }
 
+    // This function expects the provided bind group layout to be resolved
+    // (not passing a duplicate) beforehand.
     pub(super) fn create_bind_group<G: GlobalIdentityHandlerFactory>(
         &self,
         self_id: id::DeviceId,
@@ -1837,7 +1856,7 @@ impl<A: HalApi> Device<A> {
             // Check that the number of entries in the descriptor matches
             // the number of entries in the layout.
             let actual = desc.entries.len();
-            let expected = layout.entries.len();
+            let expected = layout.assume_deduplicated().entries.len();
             if actual != expected {
                 return Err(Error::BindingsNumMismatch { expected, actual });
             }
@@ -1868,6 +1887,7 @@ impl<A: HalApi> Device<A> {
             let binding = entry.binding;
             // Find the corresponding declaration in the layout
             let decl = layout
+                .assume_deduplicated()
                 .entries
                 .get(&binding)
                 .ok_or(Error::MissingBindingDeclaration(binding))?;
@@ -2044,9 +2064,11 @@ impl<A: HalApi> Device<A> {
             }
         }
 
+        let layout_inner = layout.assume_deduplicated();
+
         let hal_desc = hal::BindGroupDescriptor {
             label: desc.label.borrow_option(),
-            layout: &layout.raw,
+            layout: &layout_inner.raw,
             entries: &hal_entries,
             buffers: &hal_buffers,
             samplers: &hal_samplers,
@@ -2074,7 +2096,7 @@ impl<A: HalApi> Device<A> {
             used_texture_ranges,
             dynamic_binding_info,
             // collect in the order of BGL iteration
-            late_buffer_binding_sizes: layout
+            late_buffer_binding_sizes: layout_inner
                 .entries
                 .keys()
                 .flat_map(|binding| late_buffer_binding_sizes.get(binding).cloned())
@@ -2304,10 +2326,13 @@ impl<A: HalApi> Device<A> {
 
         // validate total resource counts
         for &id in desc.bind_group_layouts.iter() {
-            let bind_group_layout = bgl_guard
-                .get(id)
-                .map_err(|_| Error::InvalidBindGroupLayout(id))?;
-            count_validator.merge(&bind_group_layout.count_validator);
+            let bind_group_layout = match try_get_bind_group_layout(bgl_guard, id) {
+                Some(layout) => layout,
+                None => {
+                    return Err(Error::InvalidBindGroupLayout(id));
+                }
+            };
+            count_validator.merge(&bind_group_layout.assume_deduplicated().count_validator);
         }
         count_validator
             .validate(&self.limits)
@@ -2316,7 +2341,12 @@ impl<A: HalApi> Device<A> {
         let bgl_vec = desc
             .bind_group_layouts
             .iter()
-            .map(|&id| &try_get_bind_group_layout(bgl_guard, id).unwrap().raw)
+            .map(|&id| {
+                &try_get_bind_group_layout(bgl_guard, id)
+                    .unwrap()
+                    .assume_deduplicated()
+                    .raw
+            })
             .collect::<Vec<_>>();
         let hal_desc = hal::PipelineLayoutDescriptor {
             label: desc.label.borrow_option(),

wgpu-core/src/hub.rs

@@ -575,8 +575,10 @@ impl<A: HalApi, F: GlobalIdentityHandlerFactory> Hub<A, F> {
         for element in self.bind_group_layouts.data.write().map.drain(..) {
             if let Element::Occupied(bgl, _) = element {
                 let device = &devices[bgl.device_id.value];
-                unsafe {
-                    device.raw.destroy_bind_group_layout(bgl.raw);
+                if let Some(inner) = bgl.into_inner() {
+                    unsafe {
+                        device.raw.destroy_bind_group_layout(inner.raw);
+                    }
                 }
             }
         }