hal/vulkan: generate separate hash identity for Texture/TextureViews (#7972)

Author: cwfitzgerald

wgpu-hal/src/vulkan/adapter.rs

@@ -2162,6 +2162,9 @@ impl super::Adapter {
                 self.private_caps.maximum_samplers,
             )),
             memory_allocations_counter: Default::default(),
+
+            texture_identity_factory: super::ResourceIdentityFactory::new(),
+            texture_view_identity_factory: super::ResourceIdentityFactory::new(),
         });
 
         let relay_semaphores = super::RelaySemaphores::new(&shared)?;

wgpu-hal/src/vulkan/command.rs

@@ -61,7 +61,8 @@ impl super::CommandEncoder {
             Entry::Vacant(e) => {
                 let super::FramebufferKey {
                     raw_pass,
-                    ref attachments,
+                    ref attachment_views,
+                    attachment_identities: _,
                     extent,
                 } = *e.key();
 
@@ -70,7 +71,7 @@ impl super::CommandEncoder {
                     .width(extent.width)
                     .height(extent.height)
                     .layers(extent.depth_or_array_layers)
-                    .attachments(attachments);
+                    .attachments(attachment_views);
 
                 let raw = unsafe { self.device.raw.create_framebuffer(&vk_info, None).unwrap() };
                 *e.insert(raw)
@@ -81,12 +82,13 @@ impl super::CommandEncoder {
     fn make_temp_texture_view(
         &mut self,
         key: super::TempTextureViewKey,
-    ) -> Result<vk::ImageView, crate::DeviceError> {
+    ) -> Result<super::IdentifiedTextureView, crate::DeviceError> {
         Ok(match self.temp_texture_views.entry(key) {
             Entry::Occupied(e) => *e.get(),
             Entry::Vacant(e) => {
                 let super::TempTextureViewKey {
                     texture,
+                    texture_identity: _,
                     format,
                     mip_level,
                     depth_slice,
@@ -105,7 +107,10 @@ impl super::CommandEncoder {
                     });
                 let raw = unsafe { self.device.raw.create_image_view(&vk_info, None) }
                     .map_err(super::map_host_device_oom_and_ioca_err)?;
-                *e.insert(raw)
+
+                let identity = self.device.texture_view_identity_factory.next();
+
+                *e.insert(super::IdentifiedTextureView { raw, identity })
             }
         })
     }
@@ -779,7 +784,8 @@ impl crate::CommandEncoder for super::CommandEncoder {
         };
         let mut fb_key = super::FramebufferKey {
             raw_pass: vk::RenderPass::null(),
-            attachments: ArrayVec::default(),
+            attachment_views: ArrayVec::default(),
+            attachment_identities: ArrayVec::default(),
             extent: desc.extent,
         };
 
@@ -788,13 +794,14 @@ impl crate::CommandEncoder for super::CommandEncoder {
                 let color_view = if cat.target.view.dimension == wgt::TextureViewDimension::D3 {
                     let key = super::TempTextureViewKey {
                         texture: cat.target.view.raw_texture,
+                        texture_identity: cat.target.view.texture_identity,
                         format: cat.target.view.raw_format,
                         mip_level: cat.target.view.base_mip_level,
                         depth_slice: cat.depth_slice.unwrap(),
                     };
                     self.make_temp_texture_view(key)?
                 } else {
-                    cat.target.view.raw
+                    cat.target.view.identified_raw_view()
                 };
 
                 vk_clear_values.push(vk::ClearValue {
@@ -809,10 +816,10 @@ impl crate::CommandEncoder for super::CommandEncoder {
                 };
 
                 rp_key.colors.push(Some(color));
-                fb_key.attachments.push(color_view);
+                fb_key.push_view(color_view);
                 if let Some(ref at) = cat.resolve_target {
                     vk_clear_values.push(unsafe { mem::zeroed() });
-                    fb_key.attachments.push(at.view.raw);
+                    fb_key.push_view(at.view.identified_raw_view());
                 }
 
                 // Assert this attachment is valid for the detected multiview, as a sanity check
@@ -838,7 +845,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
                 base: ds.target.make_attachment_key(ds.depth_ops),
                 stencil_ops: ds.stencil_ops,
             });
-            fb_key.attachments.push(ds.target.view.raw);
+            fb_key.push_view(ds.target.view.identified_raw_view());
 
             // Assert this attachment is valid for the detected multiview, as a sanity check
             // The driver crash for this is really bad on AMD, so the check is worth it

wgpu-hal/src/vulkan/device.rs

@@ -603,6 +603,7 @@ impl super::Device {
     ///   `drop_callback` is [`Some`], `vk_image` must be valid until the callback is called.
     /// - If the `ImageCreateFlags` does not contain `MUTABLE_FORMAT`, the `view_formats` of `desc` must be empty.
     pub unsafe fn texture_from_raw(
+        &self,
         vk_image: vk::Image,
         desc: &crate::TextureDescriptor,
         drop_callback: Option<crate::DropCallback>,
@@ -624,6 +625,8 @@ impl super::Device {
             raw_flags |= vk::ImageCreateFlags::MUTABLE_FORMAT;
         }
 
+        let identity = self.shared.texture_identity_factory.next();
+
         let drop_guard = crate::DropGuard::from_option(drop_callback);
 
         super::Texture {
@@ -633,6 +636,7 @@ impl super::Device {
             block: None,
             format: desc.format,
             copy_size: desc.copy_extent(),
+            identity,
         }
     }
 
@@ -796,6 +800,8 @@ impl super::Device {
             unsafe { self.shared.set_object_name(image.raw, label) };
         }
 
+        let identity = self.shared.texture_identity_factory.next();
+
         self.counters.textures.add(1);
 
         Ok(super::Texture {
@@ -805,6 +811,7 @@ impl super::Device {
             block: None,
             format: desc.format,
             copy_size: image.copy_size,
+            identity,
         })
     }
 
@@ -1274,6 +1281,8 @@ impl crate::Device for super::Device {
             unsafe { self.shared.set_object_name(image.raw, label) };
         }
 
+        let identity = self.shared.texture_identity_factory.next();
+
         self.counters.textures.add(1);
 
         Ok(super::Texture {
@@ -1283,6 +1292,7 @@ impl crate::Device for super::Device {
             block: Some(block),
             format: desc.format,
             copy_size: image.copy_size,
+            identity,
         })
     }
     unsafe fn destroy_texture(&self, texture: super::Texture) {
@@ -1335,6 +1345,8 @@ impl crate::Device for super::Device {
             unsafe { self.shared.set_object_name(raw, label) };
         }
 
+        let identity = self.shared.texture_view_identity_factory.next();
+
         self.counters.texture_views.add(1);
 
         Ok(super::TextureView {
@@ -1345,6 +1357,8 @@ impl crate::Device for super::Device {
             raw_format,
             base_mip_level: desc.range.base_mip_level,
             dimension: desc.dimension,
+            texture_identity: texture.identity,
+            view_identity: identity,
         })
     }
     unsafe fn destroy_texture_view(&self, view: super::TextureView) {

wgpu-hal/src/vulkan/instance.rs

@@ -1139,6 +1139,8 @@ impl crate::Surface for super::Surface {
             return Err(crate::SurfaceError::Outdated);
         }
 
+        let identity = swapchain.device.texture_identity_factory.next();
+
         let texture = super::SurfaceTexture {
             index,
             texture: super::Texture {
@@ -1152,6 +1154,7 @@ impl crate::Surface for super::Surface {
                     height: swapchain.config.extent.height,
                     depth: 1,
                 },
+                identity,
             },
             surface_semaphores: swapchain_semaphores_arc,
         };

wgpu-hal/src/vulkan/mod.rs

@@ -651,6 +651,14 @@ struct DeviceShared {
     render_passes: Mutex<FastHashMap<RenderPassKey, vk::RenderPass>>,
     sampler_cache: Mutex<sampler::SamplerCache>,
     memory_allocations_counter: InternalCounter,
+
+    /// Because we have cached framebuffers which are not deleted from until
+    /// the device is destroyed, if the implementation of vulkan re-uses handles
+    /// we need some way to differentiate between the old handle and the new handle.
+    /// This factory allows us to have a dedicated identity value for each texture.
+    texture_identity_factory: ResourceIdentityFactory<vk::Image>,
+    /// As above, for texture views.
+    texture_view_identity_factory: ResourceIdentityFactory<vk::ImageView>,
 }
 
 impl Drop for DeviceShared {
@@ -864,6 +872,7 @@ pub struct Texture {
     block: Option<gpu_alloc::MemoryBlock<vk::DeviceMemory>>,
     format: wgt::TextureFormat,
     copy_size: crate::CopyExtent,
+    identity: ResourceIdentity<vk::Image>,
 }
 
 impl crate::DynTexture for Texture {}
@@ -886,6 +895,8 @@ pub struct TextureView {
     raw_format: vk::Format,
     base_mip_level: u32,
     dimension: wgt::TextureViewDimension,
+    texture_identity: ResourceIdentity<vk::Image>,
+    view_identity: ResourceIdentity<vk::ImageView>,
 }
 
 impl crate::DynTextureView for TextureView {}
@@ -897,6 +908,14 @@ impl TextureView {
     pub unsafe fn raw_handle(&self) -> vk::ImageView {
         self.raw
     }
+
+    /// Returns the raw texture view, along with its identity.
+    fn identified_raw_view(&self) -> IdentifiedTextureView {
+        IdentifiedTextureView {
+            raw: self.raw,
+            identity: self.view_identity,
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -956,16 +975,97 @@ impl Temp {
     }
 }
 
+/// Generates unique IDs for each resource of type `T`.
+///
+/// Because vk handles are not permanently unique, this
+/// provides a way to generate unique IDs for each resource.
+struct ResourceIdentityFactory<T> {
+    #[cfg(not(target_has_atomic = "64"))]
+    next_id: Mutex<u64>,
+    #[cfg(target_has_atomic = "64")]
+    next_id: core::sync::atomic::AtomicU64,
+    _phantom: PhantomData<T>,
+}
+
+impl<T> ResourceIdentityFactory<T> {
+    fn new() -> Self {
+        Self {
+            #[cfg(not(target_has_atomic = "64"))]
+            next_id: Mutex::new(0),
+            #[cfg(target_has_atomic = "64")]
+            next_id: core::sync::atomic::AtomicU64::new(0),
+            _phantom: PhantomData,
+        }
+    }
+
+    /// Returns a new unique ID for a resource of type `T`.
+    fn next(&self) -> ResourceIdentity<T> {
+        #[cfg(not(target_has_atomic = "64"))]
+        {
+            let mut next_id = self.next_id.lock();
+            let id = *next_id;
+            *next_id += 1;
+            ResourceIdentity {
+                id,
+                _phantom: PhantomData,
+            }
+        }
+
+        #[cfg(target_has_atomic = "64")]
+        ResourceIdentity {
+            id: self
+                .next_id
+                .fetch_add(1, core::sync::atomic::Ordering::Relaxed),
+            _phantom: PhantomData,
+        }
+    }
+}
+
+/// A unique identifier for a resource of type `T`.
+///
+/// This is used as a hashable key for resources, which
+/// is permanently unique through the lifetime of the program.
+#[derive(Debug, Copy, Clone, Eq, Hash, PartialEq)]
+struct ResourceIdentity<T> {
+    id: u64,
+    _phantom: PhantomData<T>,
+}
+
 #[derive(Clone, Eq, Hash, PartialEq)]
 struct FramebufferKey {
     raw_pass: vk::RenderPass,
-    attachments: ArrayVec<vk::ImageView, { MAX_TOTAL_ATTACHMENTS }>,
+    /// Because this is used as a key in a hash map, we need to include the identity
+    /// so that this hashes differently, even if the ImageView handles are the same
+    /// between different views.
+    attachment_identities: ArrayVec<ResourceIdentity<vk::ImageView>, { MAX_TOTAL_ATTACHMENTS }>,
+    /// While this is redundant for calculating the hash, we need access to an array
+    /// of all the raw ImageViews when we are creating the actual framebuffer,
+    /// so we store this here.
+    attachment_views: ArrayVec<vk::ImageView, { MAX_TOTAL_ATTACHMENTS }>,
     extent: wgt::Extent3d,
 }
 
+impl FramebufferKey {
+    fn push_view(&mut self, view: IdentifiedTextureView) {
+        self.attachment_identities.push(view.identity);
+        self.attachment_views.push(view.raw);
+    }
+}
+
+/// A texture view paired with its identity.
+#[derive(Copy, Clone)]
+struct IdentifiedTextureView {
+    raw: vk::ImageView,
+    identity: ResourceIdentity<vk::ImageView>,
+}
+
 #[derive(Clone, Eq, Hash, PartialEq)]
 struct TempTextureViewKey {
     texture: vk::Image,
+    /// As this is used in a hashmap, we need to
+    /// include the identity so that this hashes differently,
+    /// even if the Image handles are the same between different images.
+    texture_identity: ResourceIdentity<vk::Image>,
     format: vk::Format,
     mip_level: u32,
     depth_slice: u32,
@@ -1008,7 +1108,7 @@ pub struct CommandEncoder {
     end_of_pass_timer_query: Option<(vk::QueryPool, u32)>,
 
     framebuffers: FastHashMap<FramebufferKey, vk::Framebuffer>,
-    temp_texture_views: FastHashMap<TempTextureViewKey, vk::ImageView>,
+    temp_texture_views: FastHashMap<TempTextureViewKey, IdentifiedTextureView>,
 
     counters: Arc<wgt::HalCounters>,
 }
@@ -1037,7 +1137,7 @@ impl Drop for CommandEncoder {
         }
 
         for (_, view) in self.temp_texture_views.drain() {
-            unsafe { self.device.raw.destroy_image_view(view, None) };
+            unsafe { self.device.raw.destroy_image_view(view.raw, None) };
         }
 
         self.counters.command_encoders.sub(1);