From d8bdbf79ec39503571ce6fcf4dc571786e9159d3 Mon Sep 17 00:00:00 2001 From: s-ol Date: Thu, 13 Nov 2025 15:44:07 +0100 Subject: [PATCH 1/2] unify vk_hal::Texture::{block, external_memory} in enum TextureMemory Instead of having two std::Option<> types that are logically mutually exclusive, this unifies them in an enum. As a result, vk::Device::texture_from_raw() becomes more generic and can be used more consistently in the wgpu-hal::vulkan implementation, which also fixes inconsistent usage of the "textures" counter. --- wgpu-hal/src/vulkan/device.rs | 91 ++++++++----------------- wgpu-hal/src/vulkan/mod.rs | 20 ++++-- wgpu-hal/src/vulkan/swapchain/native.rs | 3 +- 3 files changed, 45 insertions(+), 69 deletions(-) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 961a9182be..ed75515b83 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -488,41 +488,27 @@ impl super::Device { /// - If `drop_callback` is [`None`], wgpu-hal will take ownership of `vk_image`. If /// `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. - /// - If `external_memory` is [`Some`], wgpu-hal will take ownership of the memory (which is presumed to back - /// `vk_image`). If `external_memory` is [`None`], the memory must be valid until `drop_callback` is called. + /// - If `external_memory` is not [`super::TextureMemory::External`], wgpu-hal will take ownership of the + /// memory (which is presumed to back `vk_image`). Otherwise, the memory must remain valid until + /// `drop_callback` is called. pub unsafe fn texture_from_raw( &self, vk_image: vk::Image, desc: &crate::TextureDescriptor, drop_callback: Option, - external_memory: Option, + memory: super::TextureMemory, ) -> super::Texture { - let mut raw_flags = vk::ImageCreateFlags::empty(); - let mut view_formats = vec![]; - for tf in desc.view_formats.iter() { - if *tf == desc.format { - continue; - } - view_formats.push(*tf); - } - if !view_formats.is_empty() { - raw_flags |= - vk::ImageCreateFlags::MUTABLE_FORMAT | vk::ImageCreateFlags::EXTENDED_USAGE; - view_formats.push(desc.format) - } - if desc.format.is_multi_planar_format() { - raw_flags |= vk::ImageCreateFlags::MUTABLE_FORMAT; - } - let identity = self.shared.texture_identity_factory.next(); - let drop_guard = crate::DropGuard::from_option(drop_callback); + if let Some(label) = desc.label { + unsafe { self.shared.set_object_name(vk_image, label) }; + } + super::Texture { raw: vk_image, drop_guard, - external_memory, - block: None, + memory, format: desc.format, copy_size: desc.copy_extent(), identity, @@ -634,7 +620,6 @@ impl super::Device { Ok(ImageWithoutMemory { raw, requirements: req, - copy_size, }) } @@ -695,22 +680,13 @@ impl super::Device { unsafe { self.shared.raw.bind_image_memory(image.raw, memory, 0) } .map_err(super::map_host_device_oom_err)?; - if let Some(label) = desc.label { - 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 { - raw: image.raw, - drop_guard: None, - external_memory: Some(memory), - block: None, - format: desc.format, - copy_size: image.copy_size, - identity, + Ok(unsafe { + self.texture_from_raw( + image.raw, + desc, + None, + super::TextureMemory::Dedicated(memory), + ) }) } @@ -1186,35 +1162,25 @@ impl crate::Device for super::Device { unsafe { self.shared.raw.destroy_image(image.raw, None) }; })?; - if let Some(label) = desc.label { - 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 { - raw: image.raw, - drop_guard: None, - external_memory: None, - block: Some(block), - format: desc.format, - copy_size: image.copy_size, - identity, + Ok(unsafe { + self.texture_from_raw(image.raw, desc, None, super::TextureMemory::Block(block)) }) } + unsafe fn destroy_texture(&self, texture: super::Texture) { if texture.drop_guard.is_none() { unsafe { self.shared.raw.destroy_image(texture.raw, None) }; } - if let Some(memory) = texture.external_memory { - unsafe { self.shared.raw.free_memory(memory, None) }; - } - if let Some(block) = texture.block { - self.counters.texture_memory.sub(block.size() as isize); - unsafe { self.mem_allocator.lock().dealloc(&*self.shared, block) }; + match texture.memory { + super::TextureMemory::Block(block) => unsafe { + self.counters.texture_memory.sub(block.size() as isize); + self.mem_allocator.lock().dealloc(&*self.shared, block); + }, + super::TextureMemory::Dedicated(memory) => unsafe { + self.shared.raw.free_memory(memory, None); + }, + super::TextureMemory::External => {} } self.counters.textures.sub(1); @@ -2879,5 +2845,4 @@ fn handle_unexpected(err: vk::Result) -> ! { struct ImageWithoutMemory { raw: vk::Image, requirements: vk::MemoryRequirements, - copy_size: crate::CopyExtent, } diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 115c6eb209..c0c35c7ed2 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -695,11 +695,23 @@ pub struct AccelerationStructure { impl crate::DynAccelerationStructure for AccelerationStructure {} +#[derive(Debug)] +pub enum TextureMemory { + // shared memory in GPU allocator (owned by wgpu-hal) + Block(gpu_alloc::MemoryBlock), + + // dedicated memory (owned by wgpu-hal) + Dedicated(vk::DeviceMemory), + + // memory not owned by wgpu + External, +} + #[derive(Debug)] pub struct Texture { raw: vk::Image, - external_memory: Option, - block: Option>, + memory: TextureMemory, + format: wgt::TextureFormat, copy_size: crate::CopyExtent, identity: ResourceIdentity, @@ -722,8 +734,8 @@ impl Texture { /// # Safety /// /// - The external memory must not be manually freed - pub unsafe fn external_memory(&self) -> Option { - self.external_memory + pub unsafe fn memory(&self) -> &TextureMemory { + &self.memory } } diff --git a/wgpu-hal/src/vulkan/swapchain/native.rs b/wgpu-hal/src/vulkan/swapchain/native.rs index ac8c07cd11..bae73f3e53 100644 --- a/wgpu-hal/src/vulkan/swapchain/native.rs +++ b/wgpu-hal/src/vulkan/swapchain/native.rs @@ -498,8 +498,7 @@ impl Swapchain for NativeSwapchain { texture: crate::vulkan::Texture { raw: self.images[index as usize], drop_guard: None, - block: None, - external_memory: None, + memory: crate::vulkan::TextureMemory::External, format: self.config.format, copy_size: crate::CopyExtent { width: self.config.extent.width, From 30e0083d134bcf21528524254145810a1c08b145 Mon Sep 17 00:00:00 2001 From: s-ol Date: Thu, 13 Nov 2025 16:01:29 +0100 Subject: [PATCH 2/2] update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f72ca279f0..b76776ae5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,7 +123,7 @@ By @SupaMaggie70Incorporated in [#8206](https://github.com/gfx-rs/wgpu/pull/8206 - Validation errors from `CommandEncoder::finish()` will report the label of the invalid encoder. By @kpreid in [#8449](https://github.com/gfx-rs/wgpu/pull/8449). - Corrected documentation of the minimum alignment of the *end* of a mapped range of a buffer (it is 4, not 8). By @kpreid in [#8450](https://github.com/gfx-rs/wgpu/pull/8450). - `util::StagingBelt` now takes a `Device` when it is created instead of when it is used. By @kpreid in [#8462](https://github.com/gfx-rs/wgpu/pull/8462). -- `wgpu_hal::vulkan::Device::texture_from_raw` now takes an `external_memory` argument. By @s-ol in [#8512](https://github.com/gfx-rs/wgpu/pull/8512) +- `wgpu_hal::vulkan::Texture` API changes to handle externally-created textures and memory more flexibly. By @s-ol in [#8512](https://github.com/gfx-rs/wgpu/pull/8512), [#8521](https://github.com/gfx-rs/wgpu/pull/8521). #### Naga