diff --git a/crates/renderer/src/egui.rs b/crates/renderer/src/egui.rs index a8c90a3..df1614b 100644 --- a/crates/renderer/src/egui.rs +++ b/crates/renderer/src/egui.rs @@ -1,5 +1,6 @@ use std::{ collections::{BTreeMap, HashMap}, + mem::ManuallyDrop, sync::Arc, }; @@ -485,6 +486,13 @@ pub fn egui_pass( textures_indices.push(idx as u32); } + const TEXTURE_DESCRIPTOR_CAPACITY: usize = 10; + if textures.len() > TEXTURE_DESCRIPTOR_CAPACITY { + return Err(crate::Error::Todo( + "egui frame references more textures than the descriptor array supports", + )); + } + let num_draw_calls = draw_calls.len(); let vertices_size = vertices.len() * size_of::(); let indices_size = indices.len() * size_of::(); @@ -558,57 +566,110 @@ pub fn egui_pass( (staging, vertices, indices, draw_calls, texture_ids) }; - let descriptor_infos = textures + // This map contains only unique textures referenced by these meshes. Its insertion order + // is the exact index stored in `texture_ids`, matching `texture[index]` in egui.slang. + let descriptor_resources = textures .values() .map(|entry| { let texture = texture_handler.get_texture(entry.id).unwrap(); - vk::DescriptorImageInfo { - sampler: samplers.get_sampler(entry.as_sampler_desc()).unwrap(), - image_view: texture + let view = Arc::new(ManuallyDrop::into_inner( + texture .create_view( ImageViewDesc::color_2d() .with_mip_range(0..1) .with_layer_range(0..1), ) - .unwrap() - .raw(), - image_layout: vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + .unwrap(), + )); + let sampler = samplers.get_sampler_owned(entry.as_sampler_desc()).unwrap(); + (view, sampler) + }) + .collect::>(); + + let texture_ids = Arc::new(texture_ids); + // Each pass gets its own managed set, so later passes/frames cannot mutate the contents + // captured by this callback. + let descriptor_pool = crate::pipeline::DescriptorPool::new( + dev.clone(), + crate::pipeline::DescriptorPoolDesc { + flags: vk::DescriptorPoolCreateFlags::UPDATE_AFTER_BIND, + name: Some("egui-pass-descriptor-pool".into()), + sizes: &[ + vk::DescriptorPoolSize { + ty: vk::DescriptorType::COMBINED_IMAGE_SAMPLER, + descriptor_count: TEXTURE_DESCRIPTOR_CAPACITY as u32, + }, + vk::DescriptorPoolSize { + ty: vk::DescriptorType::STORAGE_BUFFER, + descriptor_count: 1, + }, + ], + max_sets: 1, + }, + )?; + let mut descriptor_set = descriptor_pool + .allocate_managed(crate::pipeline::DescriptorSetAllocDesc { + name: None, + layout: &egui_state.descriptor_layout, + }) + .map_err(|_| crate::Error::Todo("egui managed descriptor allocation failed"))?; + { + let mut writer = descriptor_set + .writer() + .map_err(|_| crate::Error::Todo("egui descriptor set is stale"))?; + writer + .clear_binding(EguiState::TEXTURE_BINDING) + .map_err(|_| crate::Error::Todo("egui descriptor binding clear failed"))?; + // Managed bindings are partially bound, but Vulkan descriptor contents do not have an + // unset operation. Populate every unused slot with a live descriptor to avoid stale + // shader-array entries while preserving the mesh texture indices above. + if let Some((first_view, first_sampler)) = descriptor_resources.first() { + for array_element in descriptor_resources.len()..TEXTURE_DESCRIPTOR_CAPACITY { + writer + .write_image( + EguiState::TEXTURE_BINDING, + array_element as u32, + &[crate::pipeline::ImageDescriptor { + view: first_view, + sampler: Some(first_sampler), + layout: vk::ImageLayout::GENERAL, + }], + ) + .map_err(|_| crate::Error::Todo("egui descriptor image padding failed"))?; } - }) - .collect::>(); - - let uniform_info = vk::DescriptorBufferInfo { - buffer: texture_ids.raw(), - offset: 0, - range: texture_ids.len(), - }; - - let descriptor_writes = descriptor_infos - .iter() - .enumerate() - .map(|(i, info)| { - vk::WriteDescriptorSet::default() - .image_info(core::slice::from_ref(info)) - .descriptor_count(1) - .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) - .dst_set(egui_state.descriptor_set) - .dst_binding(EguiState::TEXTURE_BINDING) - .dst_array_element(i as u32) - }) - .chain(core::iter::once({ - vk::WriteDescriptorSet::default() - .buffer_info(core::slice::from_ref(&uniform_info)) - .descriptor_count(1) - .dst_binding(EguiState::UNIFORM_BINDING) - .descriptor_type(vk::DescriptorType::STORAGE_BUFFER) - .dst_array_element(0) - .dst_set(egui_state.descriptor_set) - })) - .collect::>(); - - unsafe { - dev.dev().update_descriptor_sets(&descriptor_writes, &[]); + } + for (array_element, (view, sampler)) in descriptor_resources.iter().enumerate() { + writer + .write_image( + EguiState::TEXTURE_BINDING, + array_element as u32, + &[crate::pipeline::ImageDescriptor { + view, + sampler: Some(sampler), + layout: vk::ImageLayout::GENERAL, + }], + ) + .map_err(|_| crate::Error::Todo("egui descriptor image write failed"))?; + } + writer + .write_buffer( + EguiState::UNIFORM_BINDING, + 0, + &[crate::pipeline::BufferDescriptor { + buffer: &texture_ids, + offset: 0, + range: texture_ids.len(), + }], + ) + .map_err(|_| crate::Error::Todo("egui descriptor buffer write failed"))?; + writer + .commit() + .map_err(|_| crate::Error::Todo("egui descriptor commit failed"))?; } + let descriptor_set_raw = descriptor_set + .raw() + .map_err(|_| crate::Error::Todo("egui descriptor set is stale"))?; + let descriptor_set_owner = Arc::new(descriptor_set); let to_remove_tex_ids = output .textures_delta @@ -631,10 +692,15 @@ pub fn egui_pass( let record: Box = Box::new({ let pipeline = egui_state.pipeline.clone(); - let descriptor_set = egui_state.descriptor_set; + let descriptor_set_owner = descriptor_set_owner.clone(); let screen_rect = egui.screen_rect(); move |ctx: &RenderContext| -> crate::Result<()> { + // Keep this managed wrapper alive through recording, and validate its generation + // immediately before binding the raw handle captured for this frame. + let _descriptor_set = descriptor_set_owner + .raw() + .map_err(|_| crate::Error::Todo("egui descriptor set is stale"))?; let cmd = &ctx.cmd; let staging = ctx.get_buffer(draw_staging).unwrap(); let vertices = ctx.get_buffer(vertices).unwrap(); @@ -742,7 +808,7 @@ pub fn egui_pass( cmd.bind_descriptor_sets( pipeline.layout(), vk::PipelineBindPoint::GRAPHICS, - &[descriptor_set], + &[descriptor_set_raw], ); cmd.draw_indexed_indirect( draw_calls.raw(), diff --git a/crates/renderer/src/lib.rs b/crates/renderer/src/lib.rs index 9d2c7ba..907aa08 100644 --- a/crates/renderer/src/lib.rs +++ b/crates/renderer/src/lib.rs @@ -1,8 +1,4 @@ -#![feature( - negative_impls, - map_try_insert, - debug_closure_helpers, -)] +#![feature(negative_impls, map_try_insert, debug_closure_helpers)] use std::{collections::HashMap, sync::Arc}; @@ -22,15 +18,14 @@ pub mod util; pub(crate) use renderer_resources::{buffers, images, pipeline}; -pub use renderer_device::{device, instance, queue, sync}; pub use renderer_device::{ - compatible_extension_properties, extension_intersection, - get_physical_device_features, get_physical_device_properties, make_extension, DebugUtils, DebugUtilsCreateInfo, Device, DeviceOwned, DeviceRef, Error, ExtendsDeviceFeatures2Debug, ExtendsDeviceProperties2Debug, Extension, Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, PhysicalDeviceProperties, Result, - SurfaceCapabilities, + SurfaceCapabilities, compatible_extension_properties, extension_intersection, + get_physical_device_features, get_physical_device_properties, make_extension, }; +pub use renderer_device::{device, instance, queue, sync}; pub use renderer_macros::DeviceRef; mod texture { @@ -75,11 +70,9 @@ mod texture { use util::Rgba; - - pub struct SamplerCache { device: Device, - samplers: HashMap, + samplers: HashMap>, } impl SamplerCache { @@ -91,16 +84,22 @@ impl SamplerCache { } pub fn get_sampler(&mut self, desc: pipeline::SamplerDesc) -> VkResult { - use std::collections::hash_map::Entry; - let entry = match self.samplers.entry(desc) { - Entry::Occupied(entry) => entry.get().raw(), - Entry::Vacant(entry) => { - let sampler = pipeline::Sampler::new(self.device.clone(), entry.key())?; - entry.insert(sampler).raw() - } - }; + Ok(self.get_sampler_owned(desc)?.raw()) + } - Ok(entry) + pub(crate) fn get_sampler_owned( + &mut self, + desc: pipeline::SamplerDesc, + ) -> VkResult> { + use std::collections::hash_map::Entry; + match self.samplers.entry(desc) { + Entry::Occupied(entry) => Ok(entry.get().clone()), + Entry::Vacant(entry) => { + let sampler = Arc::new(pipeline::Sampler::new(self.device.clone(), entry.key())?); + entry.insert(sampler.clone()); + Ok(sampler) + } + } } } @@ -108,9 +107,6 @@ impl SamplerCache { pub struct EguiState { textures: HashMap, #[allow(unused)] - descriptor_pool: pipeline::DescriptorPool, - descriptor_set: vk::DescriptorSet, - #[allow(unused)] descriptor_layout: pipeline::DescriptorSetLayout, pipeline: Arc, } @@ -155,60 +151,35 @@ impl EguiState { const TEXTURE_BINDING: u32 = 0; const UNIFORM_BINDING: u32 = 1; pub fn new(device: Device) -> Result { - let (descriptor_pool, descriptor_layout, sets) = - util::timed("Create Descriptor Set", || { - let descriptor_pool = pipeline::DescriptorPool::new( - device.clone(), - pipeline::DescriptorPoolDesc { - flags: vk::DescriptorPoolCreateFlags::UPDATE_AFTER_BIND, - name: Some("egui-descriptorpool".into()), - sizes: &[ - vk::DescriptorPoolSize { - ty: vk::DescriptorType::COMBINED_IMAGE_SAMPLER, - descriptor_count: 10, - }, - vk::DescriptorPoolSize { - ty: vk::DescriptorType::STORAGE_BUFFER, - descriptor_count: 1, - }, - ], - max_sets: 1, - }, - )?; - let descriptor_layout = pipeline::DescriptorSetLayout::new( - device.clone(), - pipeline::DescriptorSetLayoutDesc { - flags: vk::DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL, - name: Some("egui-descriptor-layout".into()), - bindings: &[ - pipeline::DescriptorSetLayoutBindingDesc { - binding: Self::TEXTURE_BINDING, - count: 10, - kind: vk::DescriptorType::COMBINED_IMAGE_SAMPLER, - stage: vk::ShaderStageFlags::FRAGMENT, - flags: Some( - vk::DescriptorBindingFlags::PARTIALLY_BOUND - | vk::DescriptorBindingFlags::UPDATE_AFTER_BIND, - ), - }, - pipeline::DescriptorSetLayoutBindingDesc { - binding: Self::UNIFORM_BINDING, - count: 1, - kind: vk::DescriptorType::STORAGE_BUFFER, - stage: vk::ShaderStageFlags::FRAGMENT, - flags: None, - }, - ], - }, - )?; - - let sets = descriptor_pool.allocate(&[pipeline::DescriptorSetAllocDesc { - name: None, - layout: &descriptor_layout, - }])?; - - Result::Ok((descriptor_pool, descriptor_layout, sets)) - })?; + let descriptor_layout = util::timed("Create Descriptor Set", || { + let descriptor_layout = pipeline::DescriptorSetLayout::new( + device.clone(), + pipeline::DescriptorSetLayoutDesc { + flags: vk::DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL, + name: Some("egui-descriptor-layout".into()), + bindings: &[ + pipeline::DescriptorSetLayoutBindingDesc { + binding: Self::TEXTURE_BINDING, + count: 10, + kind: vk::DescriptorType::COMBINED_IMAGE_SAMPLER, + stage: vk::ShaderStageFlags::FRAGMENT, + flags: Some( + vk::DescriptorBindingFlags::PARTIALLY_BOUND + | vk::DescriptorBindingFlags::UPDATE_AFTER_BIND, + ), + }, + pipeline::DescriptorSetLayoutBindingDesc { + binding: Self::UNIFORM_BINDING, + count: 1, + kind: vk::DescriptorType::STORAGE_BUFFER, + stage: vk::ShaderStageFlags::FRAGMENT, + flags: None, + }, + ], + }, + )?; + Result::Ok(descriptor_layout) + })?; let pipeline_layout = Arc::new(pipeline::PipelineLayout::new( device.clone(), @@ -332,9 +303,7 @@ impl EguiState { Ok(Self { textures: HashMap::new(), - descriptor_pool, descriptor_layout, - descriptor_set: sets[0], pipeline: Arc::new(pipeline), }) } @@ -474,9 +443,7 @@ impl Renderer2 { pub use vk::Extent2D; -use crate::{ - instance::InstanceDesc, -}; +use crate::instance::InstanceDesc; pub mod utils { #![allow(dead_code)] diff --git a/crates/renderer_device/AGENTS.md b/crates/renderer_device/AGENTS.md index 8900136..083c9da 100644 --- a/crates/renderer_device/AGENTS.md +++ b/crates/renderer_device/AGENTS.md @@ -16,6 +16,7 @@ ## Key types and ownership - `Device(Arc)` is the cloneable owning handle. It dereferences to `DeviceInner`; equality is `Arc::ptr_eq`. - `DeviceInner` keeps the raw `ash::Device`, allocator mutex, adapter and instance, queues, synchronization threadpool, extension loaders, and inline `DevicePools`. +- `Device::enabled_features()` returns the feature snapshot requested for logical-device creation; it is distinct from `Device::features()`, which reports adapter-supported features. Descriptor capability validation must use the enabled snapshot, not support data. - `PhysicalDeviceInfo::create_logical_device` selects queues, enables requested/supported extensions and features, initializes the allocator/cache, and constructs `DeviceInner` with `Arc::new_cyclic`. - `DeviceInner::dev`, `instance`, `queues`, `physical_device`, `main_queue`, `compute_queue`, and `transfer_queue` are the main convenience accessors for downstream code. - `DevicePools` contains a `PipelineCache` wrapper plus pools for raw fences and binary/timeline semaphore newtypes. A `Pool` stores bare handles in `Mutex>` and a `Weak`; `get()` reuses a handle or creates one through `Pooled`. @@ -61,6 +62,7 @@ - `_drop: DeviceDrop` is the private shutdown path: it waits for the device to idle, exports and destroys the pipeline cache to `pipeline_cache.bin`, then destroys the Vulkan device. The cache is also exposed as a copyable wrapper through `DevicePools` for pipeline creation. - `Device` cloning is cheap and prolongs the full device lifetime; any live `DeviceObject`, `PoolObject`, sync wrapper, or other owner retaining a `Device` can delay shutdown. - Device-owned wrapper structs live outside `DeviceInner` and its pools. Pools hold bare handles, never `DeviceObject` or `PoolObject` wrappers. +- Keep enabled-feature state distinct from adapter-supported feature queries: `enabled_features()` records what the logical device was created with, while `features()` is support information and must not be treated as enabled capability. - `Pool::device()` upgrades its `Weak` and panics if the pool outlives its device; don't retain pools independently. - `PoolObject` uses `ManuallyDrop` to transfer its inner handle to `Recycle` exactly once. New `Recycle` implementations must route to the matching pool and handle type. - The derive parser requires a named-field struct with exactly one `#[device]` field and does not add generic bounds itself; generic field types must satisfy `DeviceRef` through the input declaration. Its generated paths require the crate self-alias in `lib.rs`. diff --git a/crates/renderer_device/src/device.rs b/crates/renderer_device/src/device.rs index c223581..21f6b85 100644 --- a/crates/renderer_device/src/device.rs +++ b/crates/renderer_device/src/device.rs @@ -6,11 +6,7 @@ use std::{ sync::{Arc, Weak}, }; -use ash::{ - ext, khr, - prelude::VkResult, - vk, -}; +use ash::{ext, khr, prelude::VkResult, vk}; use parking_lot::Mutex; use raw_window_handle::RawDisplayHandle; @@ -69,8 +65,6 @@ impl From for DebugName { } } } - - use crate::{ Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, Result, device::asdf::traits::ExternallyManagedObject, @@ -162,6 +156,8 @@ pub struct DeviceInner { pub sync_threadpool: sync::SyncThreadpool, pub device_extensions: DeviceExtensions, pub pools: DevicePools, + /// Features enabled when this logical device was created. + pub enabled_features: PhysicalDeviceFeatures, #[allow(dead_code)] pub enabled_extensions: Vec<&'static CStr>, @@ -410,6 +406,8 @@ impl PhysicalDeviceInfo { let create_info = vk::DeviceCreateInfo::default() .queue_create_infos(&queue_create_infos) .enabled_extension_names(&extensions); + // Preserve the values before push_next links the feature structs together for Vulkan. + let enabled_features = enabled_features_snapshot(&features); let create_info = features.push_to_device_create_info(create_info); let device = unsafe { @@ -464,6 +462,7 @@ impl PhysicalDeviceInfo { queues: device_queues, device_extensions, enabled_extensions, + enabled_features, sync_threadpool: sync::SyncThreadpool::new(), pools: DevicePools::new(inner.clone(), pipeline_cache), _drop: DeviceDrop { @@ -499,6 +498,10 @@ impl PhysicalDeviceInfo { } } +fn enabled_features_snapshot(features: &PhysicalDeviceFeatures) -> PhysicalDeviceFeatures { + features.clone() +} + #[derive(Debug)] pub struct DevicePools { pub pipeline_cache: PipelineCache, @@ -594,6 +597,12 @@ impl DeviceInner { pub fn features(&self) -> &crate::PhysicalDeviceFeatures { &self.adapter.features } + /// Returns the feature values actually requested for logical-device creation. + /// + /// This is distinct from [`Self::features`], which reports adapter-supported features. + pub fn enabled_features(&self) -> &crate::PhysicalDeviceFeatures { + &self.enabled_features + } pub fn properties(&self) -> &crate::PhysicalDeviceProperties { &self.adapter.properties } @@ -906,8 +915,8 @@ pub mod asdf { use ash::vk; use ash::vk::Handle; - use crate::device::{DebugName, DeviceInner}; use crate::Device; + use crate::device::{DebugName, DeviceInner}; pub mod traits { /// A trait describing an object owned by some manager-type, which is @@ -1022,10 +1031,7 @@ pub mod asdf { let inner: &T = &self.inner; fmt.field_with("device", |f| { - write!( - f, - "0x{:x}", self.inner.owner.raw.handle().as_raw() - ) + write!(f, "0x{:x}", self.inner.owner.raw.handle().as_raw()) }) .field_with("handle", |f| write!(f, "{inner:?}")) .field("name", &self.name); @@ -1132,4 +1138,21 @@ pub mod asdf { let _pool_owned: DeviceObject = DeviceObject::new(summon::(), summon::()); } + + #[test] + fn enabled_feature_snapshot_is_independent_of_adapter_support() { + let requested = crate::PhysicalDeviceFeatures { + core12: vk::PhysicalDeviceVulkan12Features::default() + .descriptor_binding_partially_bound(true), + ..Default::default() + }; + let snapshot = super::enabled_features_snapshot(&requested); + let supported = crate::PhysicalDeviceFeatures::default(); + + assert_eq!(snapshot.core12.descriptor_binding_partially_bound, vk::TRUE); + assert_eq!( + supported.core12.descriptor_binding_partially_bound, + vk::FALSE + ); + } } diff --git a/crates/renderer_device/src/instance.rs b/crates/renderer_device/src/instance.rs index ba17fab..c1552d3 100644 --- a/crates/renderer_device/src/instance.rs +++ b/crates/renderer_device/src/instance.rs @@ -261,25 +261,19 @@ impl Instance { raw: vk::SurfaceKHR, present_mode: Option, ) -> crate::Result { - let formats = unsafe { - functor.get_physical_device_surface_formats(pdev, raw)? - }; + let formats = unsafe { functor.get_physical_device_surface_formats(pdev, raw)? }; - let present_modes = unsafe { - functor.get_physical_device_surface_present_modes(pdev, raw)? - }; + let present_modes = + unsafe { functor.get_physical_device_surface_present_modes(pdev, raw)? }; - let generic_caps = unsafe { - functor.get_physical_device_surface_capabilities(pdev, raw)? - }; + let generic_caps = unsafe { functor.get_physical_device_surface_capabilities(pdev, raw)? }; let (capabilities, present_modes) = if let Some(ref functor) = self.extensions.get_surface_capabilities2 && self.extensions.surface_maintenance1.is_some() { let mut capabilities = vk::SurfaceCapabilities2KHR::default(); - let mut surface_info = - vk::PhysicalDeviceSurfaceInfo2KHR::default().surface(raw); + let mut surface_info = vk::PhysicalDeviceSurfaceInfo2KHR::default().surface(raw); let present_mode = present_mode.unwrap_or(present_modes[0]); let mut surface_present_mode = @@ -318,10 +312,7 @@ impl Instance { }) } - pub fn expose_adapter( - &self, - pdev: vk::PhysicalDevice, - ) -> crate::Result { + pub fn expose_adapter(&self, pdev: vk::PhysicalDevice) -> crate::Result { let properties = get_physical_device_properties(&self.inner, pdev)?; let features = get_physical_device_features(&self.inner, pdev, &properties); diff --git a/crates/renderer_device/src/lib.rs b/crates/renderer_device/src/lib.rs index 24abc2a..6a83abe 100644 --- a/crates/renderer_device/src/lib.rs +++ b/crates/renderer_device/src/lib.rs @@ -168,7 +168,7 @@ pub struct SurfaceCapabilities { pub present_modes: Vec, } -#[derive(Default, Debug)] +#[derive(Default, Debug, Clone)] pub struct PhysicalDeviceFeatures { pub core: vk::PhysicalDeviceFeatures, pub core11: vk::PhysicalDeviceVulkan11Features<'static>, diff --git a/crates/renderer_device/src/sync.rs b/crates/renderer_device/src/sync.rs index a38eaea..a0c826b 100644 --- a/crates/renderer_device/src/sync.rs +++ b/crates/renderer_device/src/sync.rs @@ -7,10 +7,7 @@ use std::{ use crate::device::{ Pool, PoolObject, Pooled, Recycle, - asdf::{ - DeviceObject, - traits::ExternallyManagedObject as ExternallyManagedObjectTrait, - }, + asdf::{DeviceObject, traits::ExternallyManagedObject as ExternallyManagedObjectTrait}, }; use crate::{Result, device::DeviceInner}; @@ -159,12 +156,8 @@ impl SyncThreadpool { } pub enum Fence { - Dedicated { - fence: DeviceObject, - }, - Pooled { - fence: PoolObject, - }, + Dedicated { fence: DeviceObject }, + Pooled { fence: PoolObject }, } impl Pooled for vk::Fence { @@ -279,12 +272,14 @@ pub enum SemaphoreInner { impl Recycle for SemaphoreInner { fn recycle(self, owner: &Device) { match self { - SemaphoreInner::Binary(semaphore) => { - owner.pools.binary_semaphores.push(BinarySemaphore(semaphore)) - } - SemaphoreInner::Timeline(semaphore) => { - owner.pools.timeline_semaphores.push(TimelineSemaphore(semaphore)) - } + SemaphoreInner::Binary(semaphore) => owner + .pools + .binary_semaphores + .push(BinarySemaphore(semaphore)), + SemaphoreInner::Timeline(semaphore) => owner + .pools + .timeline_semaphores + .push(TimelineSemaphore(semaphore)), } } } diff --git a/crates/renderer_rendergraph/Cargo.toml b/crates/renderer_rendergraph/Cargo.toml index a08ef86..1a368d7 100644 --- a/crates/renderer_rendergraph/Cargo.toml +++ b/crates/renderer_rendergraph/Cargo.toml @@ -9,3 +9,6 @@ gpu-allocator.workspace = true renderer_device = { path = "../renderer_device" } renderer_resources = { path = "../renderer_resources" } ash-formats = { git = "https://github.com/janis-bhm/ash-formats", branch = "main" } + +[dev-dependencies] +renderer_resources = { path = "../renderer_resources", features = ["testing"] } diff --git a/crates/renderer_rendergraph/src/commands.rs b/crates/renderer_rendergraph/src/commands.rs index c598d78..9ee0095 100644 --- a/crates/renderer_rendergraph/src/commands.rs +++ b/crates/renderer_rendergraph/src/commands.rs @@ -219,6 +219,10 @@ impl Command for Draw { } impl RecordedCommand for Draw { + fn preflight(&self) -> Result<(), RecordError> { + validate_managed_descriptor_sets(&self.descriptor_sets).map(|_| ()) + } + fn apply(&mut self, recorder: &mut CommandRecorder<'_>) -> Result<(), RecordError> { for vertex in &self.vertices { let buffers = [recorder.buffer(vertex.slice.buffer.id())?]; @@ -258,7 +262,12 @@ impl RecordedCommand for Draw { ); } } - let descriptor_sets: Vec<_> = self.descriptor_sets.iter().map(|set| set.raw).collect(); + let descriptor_sets = validate_managed_descriptor_sets(&self.descriptor_sets)?; + let dynamic_offsets = self + .descriptor_sets + .iter() + .flat_map(|set| set.dynamic_offsets.iter().copied()) + .collect::>(); if !descriptor_sets.is_empty() { unsafe { recorder.device.cmd_bind_descriptor_sets( @@ -267,7 +276,7 @@ impl RecordedCommand for Draw { self.pipeline_layout, 0, &descriptor_sets, - &[], + &dynamic_offsets, ); } } @@ -326,6 +335,31 @@ impl RecordedCommand for Draw { } } +fn validate_managed_descriptor_sets( + sets: &[GraphDescriptorSet], +) -> Result, RecordError> { + sets.iter() + .map(|set| { + if let Some(managed) = &set.managed { + let raw = managed.raw().map_err(|error| match error { + renderer_resources::descriptors::DescriptorSetError::StaleSet { .. } => { + RecordError::StaleDescriptorSet + } + error => RecordError::DescriptorSet(error.to_string()), + })?; + if raw != set.raw { + return Err(RecordError::DescriptorSet( + "managed descriptor set handle changed after import".into(), + )); + } + Ok(raw) + } else { + Ok(set.raw) + } + }) + .collect() +} + pub fn resource_id(buffer: Buffer) -> ResourceId { buffer.id() } diff --git a/crates/renderer_rendergraph/src/graph.rs b/crates/renderer_rendergraph/src/graph.rs index 6e545e6..fa1918d 100644 --- a/crates/renderer_rendergraph/src/graph.rs +++ b/crates/renderer_rendergraph/src/graph.rs @@ -9,8 +9,10 @@ use ash_formats::FormatExt; use renderer_resources::{ Device, buffers::{Buffer as BufferResource, BufferDesc}, + descriptors::DescriptorResourceBinding, device::{AllocationStrategy, QueueFlags}, - images::{Image as ImageResource, ImageDesc}, + images::{Image as ImageResource, ImageDesc, ImageSubresourceExt}, + pipeline::DescriptorSet, }; use renderer_device::QueueFamilyIndex; @@ -18,9 +20,9 @@ use renderer_device::QueueFamilyIndex; use crate::{ recorder::{CommandIndex, CommandList, CommandRecorder, RecordError, scope_range}, resources::{ - Buffer, DescriptorResourceAccess, GraphBufferDesc, GraphDescriptorSet, GraphTextureDesc, - ResourceDesc, ResourceId, Subresource, SubresourceScope, SubresourceScopeInner, Texture, - access_flag_is_write, + Buffer, BufferRange, DescriptorResourceAccess, DescriptorResourceIdentity, GraphBufferDesc, + GraphDescriptorSet, GraphTextureDesc, ResourceDesc, ResourceId, ResourceState, Subresource, + SubresourceScope, SubresourceScopeInner, Texture, access_flag_is_write, }, }; @@ -28,6 +30,12 @@ use crate::{ pub enum GraphError { UnknownResource(ResourceId), DependencyCycle, + DescriptorImport(String), + StaleDescriptorSet, + ConflictingInitialState(ResourceId), + ConflictingDrawLayouts(ResourceId), + ConflictingDescriptorInitialStates, + ConflictingDescriptorDrawLayouts, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -196,6 +204,10 @@ impl Kahn { pub struct RenderGraph { next_id: u64, resources: Vec, + imported_buffers_by_ptr: HashMap, + imported_buffers_by_raw: HashMap, + imported_textures_by_ptr: HashMap, + imported_textures_by_raw: HashMap, pub commands: CommandList, } @@ -204,6 +216,10 @@ impl RenderGraph { Self { next_id: 0, resources: Vec::new(), + imported_buffers_by_ptr: HashMap::new(), + imported_buffers_by_raw: HashMap::new(), + imported_textures_by_ptr: HashMap::new(), + imported_textures_by_raw: HashMap::new(), commands: CommandList::new(), } } @@ -227,35 +243,370 @@ impl RenderGraph { &mut self, resource: impl Into>, initial: crate::resources::ResourceState, - ) -> Buffer { - Buffer(self.add_resource(ResourceDesc::ImportedBuffer { - resource: resource.into(), - initial, - })) + ) -> Result { + let resource = resource.into(); + let ptr = Arc::as_ptr(&resource) as usize; + let raw = resource.raw(); + let pointer_id = self.imported_buffers_by_ptr.get(&ptr).copied(); + let found_id = pointer_id.or_else(|| self.imported_buffers_by_raw.get(&raw).copied()); + if let Some(id) = found_id { + let prior = self.resources[id.index()].initial_state(); + if !same_state(prior, initial) { + return Err(GraphError::ConflictingInitialState(id)); + } + if pointer_id.is_some() { + self.imported_buffers_by_raw.insert(raw, id); + } + return Ok(Buffer(id)); + } + let id = self.add_resource(ResourceDesc::ImportedBuffer { resource, initial }); + self.imported_buffers_by_ptr.insert(ptr, id); + self.imported_buffers_by_raw.insert(raw, id); + Ok(Buffer(id)) } pub fn import_texture( &mut self, resource: impl Into>, initial: crate::resources::ResourceState, - ) -> Texture { - Texture(self.add_resource(ResourceDesc::ImportedTexture { - resource: resource.into(), - initial, - })) + ) -> Result { + let resource = resource.into(); + let ptr = Arc::as_ptr(&resource) as usize; + let raw = resource.raw(); + let pointer_id = self.imported_textures_by_ptr.get(&ptr).copied(); + let found_id = pointer_id.or_else(|| self.imported_textures_by_raw.get(&raw).copied()); + if let Some(id) = found_id { + let prior = self.resources[id.index()].initial_state(); + if !same_state(prior, initial) { + return Err(GraphError::ConflictingInitialState(id)); + } + if pointer_id.is_some() { + self.imported_textures_by_raw.insert(raw, id); + } + return Ok(Texture(id)); + } + let id = self.add_resource(ResourceDesc::ImportedTexture { resource, initial }); + self.imported_textures_by_ptr.insert(ptr, id); + self.imported_textures_by_raw.insert(raw, id); + Ok(Texture(id)) } - /// Associate graph-visible resource accesses with a raw Vulkan descriptor set. + /// Transitively import resources retained by a managed descriptor set. /// - /// The caller is responsible for declaring every resource referenced by the - /// set accurately; compilation validates that each declared resource belongs - /// to this graph, but cannot inspect the raw descriptor contents. + /// `initial_states` must provide one explicit initial state for every unique + /// buffer/image referenced by the set. Descriptor access and image layout are + /// draw-use states and are not used as initial state. + pub fn import_descriptor_set( + &mut self, + set: Arc, + initial_states: &[(DescriptorResourceIdentity, ResourceState)], + dynamic_offsets: &[u32], + ) -> Result { + let fail = |message: String| GraphError::DescriptorImport(message); + let raw = set.raw().map_err(map_descriptor_set_error)?; + let layout = set.layout_bindings().map_err(map_descriptor_set_error)?; + let tracked = set + .bindings() + .map_err(map_descriptor_set_error)? + .cloned() + .collect::>(); + + let dynamic_slots = dynamic_offset_slots(layout); + if dynamic_slots.len() != dynamic_offsets.len() { + return Err(fail(format!( + "expected {} dynamic offsets, received {}", + dynamic_slots.len(), + dynamic_offsets.len() + ))); + } + for (index, &(_, _, kind)) in dynamic_slots.iter().enumerate() { + let alignment = set + .dynamic_offset_alignment(kind) + .map_err(map_descriptor_set_error)?; + if !is_dynamic_offset_aligned(dynamic_offsets[index], alignment) { + return Err(fail(format!( + "dynamic offset at index {index} is misaligned" + ))); + } + } + for &(binding, element, _) in &dynamic_slots { + if !tracked + .iter() + .any(|entry| entry.binding == binding && entry.array_element == element) + { + return Err(fail(format!( + "dynamic descriptor binding {binding} element {element} has no retained buffer" + ))); + } + } + + let mut staged_accesses = Vec::new(); + let mut staged_resources = Vec::new(); + let mut access_resource_indices: Vec = Vec::new(); + for entry in &tracked { + let stage = shader_stages_to_pipeline(entry.stages) + .ok_or_else(|| fail(format!("unsupported shader stage mask {:?}", entry.stages)))?; + match (&entry.resource, entry.descriptor_type) { + ( + DescriptorResourceBinding::Buffer { + buffer, + offset, + range, + dynamic, + }, + kind, + ) if matches!( + kind, + vk::DescriptorType::UNIFORM_BUFFER + | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + ) => + { + let dynamic_offset = if *dynamic { + let index = dynamic_slots + .iter() + .position(|&(binding, element, _)| { + binding == entry.binding && element == entry.array_element + }) + .ok_or_else(|| fail("dynamic descriptor ordering mismatch".into()))?; + let value = u64::from(dynamic_offsets[index]); + value + } else { + 0 + }; + let range = + effective_buffer_range(buffer.len(), *offset, *range, dynamic_offset) + .map_err(|message| fail(message.into()))?; + let (access, _) = descriptor_access(kind) + .ok_or_else(|| fail(format!("unsupported descriptor type {kind:?}")))?; + staged_accesses.push(DescriptorResourceAccess::Buffer { + buffer: Buffer(ResourceId(u64::MAX)), + range, + stage, + access, + }); + staged_resources.push(StagedImportResource::Buffer(buffer.clone())); + access_resource_indices.push(staged_resources.len() - 1); + } + ( + DescriptorResourceBinding::Image { + view, + range, + layout, + .. + }, + kind, + ) if matches!( + kind, + vk::DescriptorType::SAMPLED_IMAGE + | vk::DescriptorType::COMBINED_IMAGE_SAMPLER + | vk::DescriptorType::STORAGE_IMAGE + ) => + { + let (access, forced_layout) = descriptor_access(kind) + .ok_or_else(|| fail(format!("unsupported descriptor type {kind:?}")))?; + let layout = forced_layout.unwrap_or(*layout); + staged_accesses.push(DescriptorResourceAccess::Texture { + texture: Texture(ResourceId(u64::MAX)), + range: *range, + layout, + stage, + access, + }); + staged_resources.push(StagedImportResource::Texture(view.image().clone())); + access_resource_indices.push(staged_resources.len() - 1); + } + (DescriptorResourceBinding::Sampler { .. }, vk::DescriptorType::SAMPLER) => {} + (DescriptorResourceBinding::TexelBuffer { view, range }, kind) + if matches!( + kind, + vk::DescriptorType::UNIFORM_TEXEL_BUFFER + | vk::DescriptorType::STORAGE_TEXEL_BUFFER + ) => + { + let buffer = view.buffer(); + let start = range.offset; + let size = range.size; + if start.checked_add(size).is_none_or(|end| end > buffer.len()) { + return Err(fail("texel buffer range exceeds buffer length".into())); + } + let (access, _) = descriptor_access(kind) + .ok_or_else(|| fail(format!("unsupported descriptor type {kind:?}")))?; + staged_accesses.push(DescriptorResourceAccess::Buffer { + buffer: Buffer(ResourceId(u64::MAX)), + range: BufferRange { + offset: start, + size, + }, + stage, + access, + }); + staged_resources.push(StagedImportResource::Buffer(buffer.clone())); + access_resource_indices.push(staged_resources.len() - 1); + } + _ => { + return Err(fail(format!( + "descriptor type {:?} has incompatible or unsupported owner", + entry.descriptor_type + ))); + } + } + } + + // Validate that every distinct referenced owner has exactly one usable state + // before mutating graph resources, then route imports through the deduping APIs. + let mut resolved_states = Vec::with_capacity(staged_resources.len()); + for owner in &staged_resources { + let state = state_for_identity(initial_states, owner).ok_or_else(|| { + fail("missing or conflicting explicit initial state for descriptor resource".into()) + })?; + resolved_states.push(state); + } + for (identity, state) in initial_states { + let supplied = match identity { + DescriptorResourceIdentity::Buffer(buffer) => { + StagedImportResource::Buffer(buffer.clone()) + } + DescriptorResourceIdentity::Texture(image) => { + StagedImportResource::Texture(image.clone()) + } + }; + if !staged_resources + .iter() + .any(|resource| same_imported_identity(resource, &supplied)) + { + return Err(fail( + "initial state supplied for a resource not referenced by the descriptor set" + .into(), + )); + } + if let Some((id, existing)) = self.find_imported(&supplied) + && !same_state(existing, *state) + { + return Err(GraphError::ConflictingInitialState(id)); + } + } + for (owner, state) in staged_resources.iter().zip(&resolved_states) { + if let Some((id, existing_state)) = self.find_imported(owner) + && !same_state(existing_state, *state) + { + return Err(GraphError::ConflictingInitialState(id)); + } + } + for index in 0..staged_resources.len() { + for other in (index + 1)..staged_resources.len() { + if same_imported_identity(&staged_resources[index], &staged_resources[other]) + && !same_state(resolved_states[index], resolved_states[other]) + { + if let Some((id, _)) = self.find_imported(&staged_resources[index]) { + return Err(GraphError::ConflictingInitialState(id)); + } + return Err(GraphError::ConflictingDescriptorInitialStates); + } + } + } + let mut texture_scopes = Vec::new(); + for (access, &resource_index) in staged_accesses.iter().zip(&access_resource_indices) { + if let DescriptorResourceAccess::Texture { range, layout, .. } = access { + texture_scopes.push((resource_index, *range, *layout)); + } + } + for (index, &(resource_a, range_a, layout_a)) in texture_scopes.iter().enumerate() { + for &(resource_b, range_b, layout_b) in &texture_scopes[index + 1..] { + if same_imported_identity( + &staged_resources[resource_a], + &staged_resources[resource_b], + ) && texture_layouts_conflict(range_a, layout_a, range_b, layout_b) + { + return Err(GraphError::ConflictingDescriptorDrawLayouts); + } + } + } + for (index, first) in staged_accesses.iter().enumerate() { + if !matches!(first, DescriptorResourceAccess::Texture { .. }) { + continue; + } + let DescriptorResourceAccess::Texture { + range: range_a, + layout: layout_a, + .. + } = first + else { + continue; + }; + let first_resource = access_resource_indices[index]; + for (second_index, second) in staged_accesses.iter().enumerate().skip(index + 1) { + let DescriptorResourceAccess::Texture { + range: range_b, + layout: layout_b, + .. + } = second + else { + continue; + }; + let second_resource = access_resource_indices[second_index]; + if same_imported_identity( + &staged_resources[first_resource], + &staged_resources[second_resource], + ) && texture_layouts_conflict(*range_a, *layout_a, *range_b, *layout_b) + { + return Err(GraphError::ConflictingDescriptorDrawLayouts); + } + } + } + + let resolved_ids = self.import_transaction(|graph| { + let mut resolved_ids = Vec::with_capacity(staged_resources.len()); + for (owner, state) in staged_resources.iter().zip(resolved_states) { + let id = match owner { + StagedImportResource::Buffer(resource) => graph + .import_buffer(resource.clone(), state) + .map(|id| id.id()), + StagedImportResource::Texture(resource) => graph + .import_texture(resource.clone(), state) + .map(|id| id.id()), + }?; + resolved_ids.push(id); + } + Ok(resolved_ids) + })?; + for (access, &resource_index) in staged_accesses.iter_mut().zip(&access_resource_indices) { + let id = resolved_ids[resource_index]; + match access { + DescriptorResourceAccess::Buffer { buffer, .. } => { + *buffer = Buffer(id); + } + DescriptorResourceAccess::Texture { texture, .. } => { + *texture = Texture(id); + } + } + } + + Ok(GraphDescriptorSet { + raw, + accesses: staged_accesses, + managed: Some(set), + dynamic_offsets: dynamic_offsets.to_vec(), + }) + } + + /// Tracks a raw descriptor set without ownership or content verification. + /// + /// This is the untracked escape hatch. The caller must retain every bound + /// resource and declare all accesses accurately; prefer `import_descriptor_set` + /// for managed sets. The graph cannot verify raw descriptor contents. pub fn track_descriptor_set( &self, raw: vk::DescriptorSet, accesses: Vec, ) -> GraphDescriptorSet { - GraphDescriptorSet { raw, accesses } + GraphDescriptorSet { + raw, + accesses, + managed: None, + dynamic_offsets: Vec::new(), + } } pub fn push(&mut self, command: C) -> CommandIndex { @@ -263,11 +614,36 @@ impl RenderGraph { } pub fn compile(self) -> Result { + // Check for conflicting draw layouts before building the dependency graph. + for accesses in self.commands.side_effects.command_accesses() { + for (index, first) in accesses.iter().enumerate() { + for second in &accesses[index + 1..] { + if first.resource == second.resource + && let ( + SubresourceScopeInner::Texture { + range: a, + layout: la, + }, + SubresourceScopeInner::Texture { + range: b, + layout: lb, + }, + ) = (first.scope.subresource, second.scope.subresource) + && texture_layouts_conflict(a, la, b, lb) + { + return Err(GraphError::ConflictingDrawLayouts(first.resource)); + } + } + } + } + let command_count = self.commands.len(); let accesses = self.commands.side_effects.command_accesses(); let compiler = GraphCompiler::new(&self.resources, accesses, command_count); let batches = compiler.topo_batches()?; + compiler.validate_resources()?; + let barriers = compiler.build_barriers(&batches); let (lifetimes, interference) = compiler.bind_lifetimes(); @@ -551,6 +927,259 @@ impl Default for RenderGraph { } } +#[derive(Clone)] +enum StagedImportResource { + Buffer(Arc), + Texture(Arc), +} + +impl RenderGraph { + // Import staging is transactional for ordinary errors. Panics are intentionally not caught; + // callers must not rely on graph rollback after unwinding. + fn import_transaction( + &mut self, + operation: impl FnOnce(&mut Self) -> Result, + ) -> Result { + let resources_len = self.resources.len(); + let next_id = self.next_id; + let buffer_ptrs = self.imported_buffers_by_ptr.clone(); + let buffer_raw = self.imported_buffers_by_raw.clone(); + let texture_ptrs = self.imported_textures_by_ptr.clone(); + let texture_raw = self.imported_textures_by_raw.clone(); + + match operation(self) { + Ok(value) => Ok(value), + Err(error) => { + self.resources.truncate(resources_len); + self.next_id = next_id; + self.imported_buffers_by_ptr = buffer_ptrs; + self.imported_buffers_by_raw = buffer_raw; + self.imported_textures_by_ptr = texture_ptrs; + self.imported_textures_by_raw = texture_raw; + Err(error) + } + } + } + + fn find_imported( + &self, + resource: &StagedImportResource, + ) -> Option<(ResourceId, ResourceState)> { + let id = match resource { + StagedImportResource::Buffer(resource) => self + .imported_buffers_by_ptr + .get(&(Arc::as_ptr(resource) as usize)) + .copied() + .or_else(|| self.imported_buffers_by_raw.get(&resource.raw()).copied()), + StagedImportResource::Texture(resource) => self + .imported_textures_by_ptr + .get(&(Arc::as_ptr(resource) as usize)) + .copied() + .or_else(|| self.imported_textures_by_raw.get(&resource.raw()).copied()), + }?; + Some((id, self.resources[id.index()].initial_state())) + } +} + +fn map_descriptor_set_error( + error: renderer_resources::descriptors::DescriptorSetError, +) -> GraphError { + match error { + renderer_resources::descriptors::DescriptorSetError::StaleSet { .. } => { + GraphError::StaleDescriptorSet + } + error => GraphError::DescriptorImport(error.to_string()), + } +} + +fn same_state(a: ResourceState, b: ResourceState) -> bool { + a.stage == b.stage && a.access == b.access && a.layout == b.layout +} + +fn same_imported_identity(a: &StagedImportResource, b: &StagedImportResource) -> bool { + match (a, b) { + (StagedImportResource::Buffer(a), StagedImportResource::Buffer(b)) => { + Arc::ptr_eq(a, b) || a.raw() == b.raw() + } + (StagedImportResource::Texture(a), StagedImportResource::Texture(b)) => { + Arc::ptr_eq(a, b) || a.raw() == b.raw() + } + _ => false, + } +} + +fn effective_buffer_range( + buffer_len: u64, + base_offset: u64, + range: u64, + dynamic_offset: u64, +) -> Result { + if base_offset > buffer_len { + return Err("descriptor base offset exceeds buffer length"); + } + let base_range = if range == vk::WHOLE_SIZE { + buffer_len - base_offset + } else { + range + }; + if base_offset + .checked_add(base_range) + .is_none_or(|end| end > buffer_len) + { + return Err("descriptor base range exceeds buffer length"); + } + let offset = base_offset + .checked_add(dynamic_offset) + .ok_or("effective descriptor offset overflow")?; + if offset + .checked_add(base_range) + .is_none_or(|end| end > buffer_len) + { + return Err("effective descriptor range exceeds buffer length"); + } + Ok(BufferRange { + offset, + size: base_range, + }) +} + +fn dynamic_offset_slots( + layout: &[renderer_resources::pipeline::DescriptorSetLayoutBindingDesc], +) -> Vec<(u32, u32, vk::DescriptorType)> { + let mut dynamic_bindings = layout.iter().collect::>(); + dynamic_bindings.sort_by_key(|binding| binding.binding); + dynamic_bindings + .into_iter() + .filter(|binding| { + matches!( + binding.kind, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + ) + }) + .flat_map(|binding| { + (0..binding.count).map(move |element| (binding.binding, element, binding.kind)) + }) + .collect() +} + +fn is_dynamic_offset_aligned(offset: u32, alignment: u64) -> bool { + alignment != 0 && u64::from(offset).is_multiple_of(alignment) +} + +fn texture_layouts_conflict( + a: vk::ImageSubresourceRange, + layout_a: vk::ImageLayout, + b: vk::ImageSubresourceRange, + layout_b: vk::ImageLayout, +) -> bool { + layout_a != layout_b && a.intersects(&b) +} + +fn descriptor_access( + kind: vk::DescriptorType, +) -> Option<(vk::AccessFlags2, Option)> { + match kind { + vk::DescriptorType::UNIFORM_BUFFER | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC => { + Some((vk::AccessFlags2::UNIFORM_READ, None)) + } + vk::DescriptorType::STORAGE_BUFFER | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC => Some(( + vk::AccessFlags2::SHADER_STORAGE_READ | vk::AccessFlags2::SHADER_STORAGE_WRITE, + None, + )), + vk::DescriptorType::SAMPLED_IMAGE | vk::DescriptorType::COMBINED_IMAGE_SAMPLER => { + Some((vk::AccessFlags2::SHADER_SAMPLED_READ, None)) + } + vk::DescriptorType::STORAGE_IMAGE => Some(( + vk::AccessFlags2::SHADER_STORAGE_READ | vk::AccessFlags2::SHADER_STORAGE_WRITE, + Some(vk::ImageLayout::GENERAL), + )), + vk::DescriptorType::UNIFORM_TEXEL_BUFFER => Some((vk::AccessFlags2::SHADER_READ, None)), + vk::DescriptorType::STORAGE_TEXEL_BUFFER => Some(( + vk::AccessFlags2::SHADER_STORAGE_READ | vk::AccessFlags2::SHADER_STORAGE_WRITE, + None, + )), + _ => None, + } +} + +fn state_for_identity( + states: &[(DescriptorResourceIdentity, ResourceState)], + resource: &StagedImportResource, +) -> Option { + let matches = states.iter().filter_map(|(identity, state)| { + let same = match (resource, identity) { + (StagedImportResource::Buffer(resource), DescriptorResourceIdentity::Buffer(other)) => { + Arc::ptr_eq(resource, other) || resource.raw() == other.raw() + } + ( + StagedImportResource::Texture(resource), + DescriptorResourceIdentity::Texture(other), + ) => Arc::ptr_eq(resource, other) || resource.raw() == other.raw(), + _ => false, + }; + same.then_some(*state) + }); + let mut matches = matches; + let first = matches.next()?; + matches + .all(|state| same_state(first, state)) + .then_some(first) +} + +fn shader_stages_to_pipeline(stages: vk::ShaderStageFlags) -> Option { + if stages == vk::ShaderStageFlags::ALL { + return Some(vk::PipelineStageFlags2::ALL_COMMANDS); + } + let mut pipeline = vk::PipelineStageFlags2::NONE; + let mappings = [ + ( + vk::ShaderStageFlags::VERTEX, + vk::PipelineStageFlags2::VERTEX_SHADER, + ), + ( + vk::ShaderStageFlags::TESSELLATION_CONTROL, + vk::PipelineStageFlags2::TESSELLATION_CONTROL_SHADER, + ), + ( + vk::ShaderStageFlags::TESSELLATION_EVALUATION, + vk::PipelineStageFlags2::TESSELLATION_EVALUATION_SHADER, + ), + ( + vk::ShaderStageFlags::GEOMETRY, + vk::PipelineStageFlags2::GEOMETRY_SHADER, + ), + ( + vk::ShaderStageFlags::FRAGMENT, + vk::PipelineStageFlags2::FRAGMENT_SHADER, + ), + ( + vk::ShaderStageFlags::COMPUTE, + vk::PipelineStageFlags2::COMPUTE_SHADER, + ), + ( + vk::ShaderStageFlags::TASK_EXT, + vk::PipelineStageFlags2::TASK_SHADER_EXT, + ), + ( + vk::ShaderStageFlags::MESH_EXT, + vk::PipelineStageFlags2::MESH_SHADER_EXT, + ), + ]; + let mut recognized = vk::ShaderStageFlags::empty(); + if stages.contains(vk::ShaderStageFlags::ALL_GRAPHICS) { + recognized |= vk::ShaderStageFlags::ALL_GRAPHICS; + pipeline |= vk::PipelineStageFlags2::ALL_GRAPHICS; + } + for (shader, stage) in mappings { + if stages.intersects(shader) { + recognized |= shader; + pipeline |= stage; + } + } + (recognized == stages && !pipeline.is_empty()).then_some(pipeline) +} + pub struct CompiledGraph { resources: Vec, resolved: HashMap, @@ -653,6 +1282,9 @@ impl CompiledGraph { &mut self, recorder: &mut CommandRecorder<'_>, ) -> Result<(), RecordError> { + // Validate every wrapper before resource binding or barriers can emit Vulkan commands. + self.commands.preflight()?; + for (index, desc) in self.resources.iter().enumerate() { let id = ResourceId(index as u64); if !self.lifetimes.contains_key(&id) { @@ -939,10 +1571,884 @@ mod tests { use crate::{ commands::{ClearTexture, CopyBuffer, Draw, DrawArgs}, resources::{ - BufferRange, BufferSlice, GraphBufferDesc, GraphTextureDesc, IndexBinding, IndexFormat, - IndirectBinding, ResourceDesc, VertexBinding, + BufferRange, BufferSlice, DescriptorResourceIdentity, GraphBufferDesc, + GraphTextureDesc, IndexBinding, IndexFormat, IndirectBinding, ResourceDesc, + ResourceState, VertexBinding, }, }; + use ash::vk::Handle; + use renderer_resources::{ + buffers::Buffer as BufferResource, + descriptors::{DescriptorResourceBinding, TrackedDescriptorWrite}, + images::{Image as ImageResource, ImageDesc, ImageView}, + pipeline::{DescriptorSet, DescriptorSetLayoutBindingDesc, Sampler}, + }; + + static RECORDING_MARKER_APPLIED: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + + struct RecordingMarker(Buffer); + + impl crate::commands::Command for RecordingMarker { + fn side_effects( + &self, + command_index: CommandIndex, + map: &mut crate::recorder::SideEffectMap, + ) { + map.insert( + command_index, + self.0.id(), + SubresourceScope { + stage: vk::PipelineStageFlags2::TRANSFER, + access: vk::AccessFlags2::TRANSFER_READ, + subresource: SubresourceScopeInner::Buffer(BufferRange { + offset: 0, + size: 32, + }), + }, + ); + } + } + + impl crate::recorder::RecordedCommand for RecordingMarker { + fn apply(&mut self, _recorder: &mut CommandRecorder<'_>) -> Result<(), RecordError> { + RECORDING_MARKER_APPLIED.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + } + + fn test_buffer(handle: u64, size: u64) -> Arc { + Arc::new(BufferResource::for_testing( + vk::Buffer::from_raw(handle), + size, + )) + } + + fn test_image(handle: u64) -> Arc { + Arc::new(ImageResource::for_testing( + vk::Image::from_raw(handle), + ImageDesc { + format: vk::Format::R8G8B8A8_UNORM, + extent: vk::Extent3D { + width: 16, + height: 16, + depth: 1, + }, + usage: vk::ImageUsageFlags::SAMPLED, + ..Default::default() + }, + )) + } + + fn test_view(image: Arc) -> Arc { + Arc::new(ImageView::for_testing( + vk::ImageView::from_raw(1), + image, + texture_range(vk::ImageAspectFlags::COLOR, 0, 1, 0, 1), + )) + } + + fn layout_binding(binding: u32, kind: vk::DescriptorType) -> DescriptorSetLayoutBindingDesc { + DescriptorSetLayoutBindingDesc { + binding, + count: 1, + kind, + stage: vk::ShaderStageFlags::FRAGMENT, + flags: None, + } + } + + fn test_set( + handle: u64, + layout: Vec, + bindings: Vec, + ) -> Arc { + Arc::new(DescriptorSet::for_testing( + vk::DescriptorSet::from_raw(handle), + layout, + 0, + 0, + bindings, + )) + } + + fn buffer_entry( + binding: u32, + kind: vk::DescriptorType, + buffer: Arc, + offset: u64, + range: u64, + ) -> TrackedDescriptorWrite { + TrackedDescriptorWrite { + binding, + array_element: 0, + descriptor_type: kind, + stages: vk::ShaderStageFlags::FRAGMENT, + resource: DescriptorResourceBinding::Buffer { + buffer, + offset, + range, + dynamic: matches!( + kind, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + ), + }, + } + } + + fn image_entry( + binding: u32, + kind: vk::DescriptorType, + view: Arc, + layout: vk::ImageLayout, + ) -> TrackedDescriptorWrite { + TrackedDescriptorWrite { + binding, + array_element: 0, + descriptor_type: kind, + stages: vk::ShaderStageFlags::FRAGMENT, + resource: DescriptorResourceBinding::Image { + range: view.subresource_range(), + view, + sampler: None, + layout, + }, + } + } + + fn import_buffer_state( + buffer: &Arc, + state: ResourceState, + ) -> (DescriptorResourceIdentity, ResourceState) { + (DescriptorResourceIdentity::Buffer(buffer.clone()), state) + } + + fn import_texture_state( + image: &Arc, + state: ResourceState, + ) -> (DescriptorResourceIdentity, ResourceState) { + (DescriptorResourceIdentity::Texture(image.clone()), state) + } + + #[test] + fn managed_descriptor_import_transitively_retains_buffer_and_image_owners() { + let buffer = test_buffer(101, 128); + let image = test_image(201); + let view = test_view(image.clone()); + let buffer_weak = Arc::downgrade(&buffer); + let image_weak = Arc::downgrade(&image); + let view_weak = Arc::downgrade(&view); + let set = test_set( + 1, + vec![ + layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER), + layout_binding(1, vk::DescriptorType::SAMPLED_IMAGE), + ], + vec![ + buffer_entry(0, vk::DescriptorType::UNIFORM_BUFFER, buffer.clone(), 8, 32), + image_entry( + 1, + vk::DescriptorType::SAMPLED_IMAGE, + view.clone(), + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + ), + ], + ); + drop(buffer); + drop(image); + drop(view); + + let buffer = buffer_weak.upgrade().unwrap(); + let image = image_weak.upgrade().unwrap(); + let state = ResourceState::default(); + let mut graph = RenderGraph::new(); + let imported = graph + .import_descriptor_set( + set.clone(), + &[ + import_buffer_state(&buffer, state), + import_texture_state(&image, state), + ], + &[], + ) + .unwrap(); + + assert!(buffer_weak.strong_count() >= 2); + assert!(image_weak.strong_count() >= 2); + assert!(view_weak.strong_count() >= 1); + assert_eq!(graph.resources.len(), 2); + assert_eq!(imported.accesses.len(), 2); + assert!(graph.resources.iter().any(|desc| matches!( + desc, + ResourceDesc::ImportedBuffer { resource, .. } if Arc::ptr_eq(resource, &buffer) + ))); + assert!(graph.resources.iter().any(|desc| matches!( + desc, + ResourceDesc::ImportedTexture { resource, .. } if Arc::ptr_eq(resource, &image) + ))); + } + + #[test] + fn managed_descriptor_import_deduplicates_buffer_in_both_import_orders() { + let state = ResourceState::default(); + let buffer = test_buffer(102, 64); + let set = test_set( + 2, + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + ); + let mut graph = RenderGraph::new(); + let direct = graph.import_buffer(buffer.clone(), state).unwrap(); + let imported = graph + .import_descriptor_set(set, &[import_buffer_state(&buffer, state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Buffer { + buffer: transitive, .. + } = imported.accesses[0] + else { + panic!("buffer descriptor import produces a buffer scope"); + }; + assert_eq!(direct, transitive); + assert_eq!(graph.resources.len(), 1); + + let buffer = test_buffer(103, 64); + let set = test_set( + 3, + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + ); + let mut graph = RenderGraph::new(); + let imported = graph + .import_descriptor_set(set, &[import_buffer_state(&buffer, state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Buffer { + buffer: transitive, .. + } = imported.accesses[0] + else { + panic!("buffer descriptor import produces a buffer scope"); + }; + let direct = graph.import_buffer(buffer, state).unwrap(); + assert_eq!(direct, transitive); + assert_eq!(graph.resources.len(), 1); + } + + #[test] + fn managed_descriptor_import_deduplicates_texture_in_both_import_orders() { + let state = ResourceState::default(); + let image = test_image(202); + let view = test_view(image.clone()); + let set = test_set( + 4, + vec![layout_binding(0, vk::DescriptorType::SAMPLED_IMAGE)], + vec![image_entry( + 0, + vk::DescriptorType::SAMPLED_IMAGE, + view, + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + )], + ); + let mut graph = RenderGraph::new(); + let direct = graph.import_texture(image.clone(), state).unwrap(); + let imported = graph + .import_descriptor_set(set, &[import_texture_state(&image, state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Texture { + texture: transitive, + .. + } = imported.accesses[0] + else { + panic!("image descriptor import produces a texture scope"); + }; + assert_eq!(direct, transitive); + assert_eq!(graph.resources.len(), 1); + + let image = test_image(203); + let view = test_view(image.clone()); + let set = test_set( + 5, + vec![layout_binding(0, vk::DescriptorType::SAMPLED_IMAGE)], + vec![image_entry( + 0, + vk::DescriptorType::SAMPLED_IMAGE, + view, + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + )], + ); + let mut graph = RenderGraph::new(); + let imported = graph + .import_descriptor_set(set, &[import_texture_state(&image, state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Texture { + texture: transitive, + .. + } = imported.accesses[0] + else { + panic!("image descriptor import produces a texture scope"); + }; + let direct = graph.import_texture(image, state).unwrap(); + assert_eq!(direct, transitive); + assert_eq!(graph.resources.len(), 1); + } + + #[test] + fn descriptor_import_rejects_conflicting_initial_state_in_both_orders() { + let buffer_state = ResourceState::default(); + let conflicting = ResourceState { + stage: vk::PipelineStageFlags2::HOST, + access: vk::AccessFlags2::HOST_WRITE, + layout: None, + }; + let buffer = test_buffer(104, 64); + let set = test_set( + 6, + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + ); + let mut graph = RenderGraph::new(); + let direct = graph.import_buffer(buffer.clone(), buffer_state).unwrap(); + assert!(matches!( + graph.import_descriptor_set( + set, + &[import_buffer_state(&buffer, conflicting)], + &[], + ), + Err(GraphError::ConflictingInitialState(id)) if id == direct.id() + )); + + let buffer = test_buffer(105, 64); + let set = test_set( + 7, + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + ); + let mut graph = RenderGraph::new(); + let imported = graph + .import_descriptor_set(set, &[import_buffer_state(&buffer, buffer_state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Buffer { buffer: id, .. } = imported.accesses[0] else { + panic!("buffer descriptor import produces a buffer scope"); + }; + assert_eq!( + graph.import_buffer(buffer, conflicting), + Err(GraphError::ConflictingInitialState(id.id())) + ); + } + + #[test] + fn stale_managed_descriptor_set_is_rejected_at_import() { + let buffer = test_buffer(106, 64); + let set = Arc::new(DescriptorSet::for_testing( + vk::DescriptorSet::from_raw(8), + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + 3, + 4, + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + )); + let mut graph = RenderGraph::new(); + assert!(matches!( + graph.import_descriptor_set( + set, + &[import_buffer_state(&buffer, ResourceState::default())], + &[], + ), + Err(GraphError::StaleDescriptorSet) + )); + } + + #[test] + fn recording_preflight_rejects_stale_draw_before_graph_recording_steps() { + let buffer = test_buffer(109, 64); + let state = ResourceState { + stage: vk::PipelineStageFlags2::HOST, + access: vk::AccessFlags2::HOST_WRITE, + layout: None, + }; + let mut graph = RenderGraph::new(); + let imported = graph.import_buffer(buffer.clone(), state).unwrap(); + let stale_set = Arc::new(DescriptorSet::for_testing( + vk::DescriptorSet::from_raw(9), + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + 1, + 2, + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer, + 0, + 32, + )], + )); + graph.push(RecordingMarker(imported)); + graph.push(Draw { + args: DrawArgs::Direct { + vertex_count: 3, + instance_count: 1, + first_vertex: 0, + first_instance: 0, + }, + pipeline_layout: vk::PipelineLayout::null(), + vertices: Vec::new(), + index: None, + indirect: None, + descriptor_sets: vec![GraphDescriptorSet { + raw: vk::DescriptorSet::from_raw(9), + accesses: Vec::new(), + managed: Some(stale_set), + dynamic_offsets: Vec::new(), + }], + push_constants: Vec::new(), + }); + + let mut compiled = graph.compile().unwrap(); + assert!(compiled.barriers().iter().any(|barrier| { + barrier.before_command == CommandIndex(0) && !barrier.resources.is_empty() + })); + + static BARRIER_CALLS: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + unsafe extern "system" fn count_barrier( + _device: vk::Device, + _command_buffer: vk::CommandBuffer, + _dependency: *const vk::DependencyInfo<'_>, + ) { + BARRIER_CALLS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + } + let device = unsafe { + ash::Device::load_with( + |name| { + if name.to_bytes() == b"vkCmdPipelineBarrier2" { + count_barrier as *const () as *const std::ffi::c_void + } else { + std::ptr::null() + } + }, + vk::Device::null(), + ) + }; + RECORDING_MARKER_APPLIED.store(0, std::sync::atomic::Ordering::SeqCst); + BARRIER_CALLS.store(0, std::sync::atomic::Ordering::SeqCst); + let mut recorder = CommandRecorder::new(&device, vk::CommandBuffer::null()); + assert_eq!( + compiled.record_commands(&mut recorder), + Err(RecordError::StaleDescriptorSet) + ); + assert_eq!(BARRIER_CALLS.load(std::sync::atomic::Ordering::SeqCst), 0); + assert_eq!( + RECORDING_MARKER_APPLIED.load(std::sync::atomic::Ordering::SeqCst), + 0 + ); + } + + #[test] + fn failed_descriptor_import_does_not_append_earlier_valid_resources() { + let buffer = test_buffer(110, 64); + let set = test_set( + 16, + vec![ + layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER), + layout_binding(1, vk::DescriptorType::INPUT_ATTACHMENT), + ], + vec![ + buffer_entry(0, vk::DescriptorType::UNIFORM_BUFFER, buffer.clone(), 0, 32), + TrackedDescriptorWrite { + binding: 1, + array_element: 0, + descriptor_type: vk::DescriptorType::INPUT_ATTACHMENT, + stages: vk::ShaderStageFlags::FRAGMENT, + resource: DescriptorResourceBinding::Sampler { + sampler: Arc::new(Sampler::for_testing(vk::Sampler::from_raw(3))), + }, + }, + ], + ); + let mut graph = RenderGraph::new(); + assert!(matches!( + graph.import_descriptor_set( + set, + &[import_buffer_state(&buffer, ResourceState::default())], + &[], + ), + Err(GraphError::DescriptorImport(_)) + )); + assert!(graph.resources.is_empty()); + assert_eq!(graph.next_id, 0); + assert!(graph.imported_buffers_by_ptr.is_empty()); + assert!(graph.imported_buffers_by_raw.is_empty()); + } + + #[test] + fn failed_import_transaction_restores_exact_raw_identity_mapping() { + let first = test_buffer(111, 64); + let second = test_buffer(111, 64); + assert!(!Arc::ptr_eq(&first, &second)); + let state = ResourceState::default(); + let mut graph = RenderGraph::new(); + + let first_id = graph.import_buffer(first.clone(), state).unwrap().id(); + // Model an existing graph state with two pointer identities sharing a raw handle; the + // raw-key choice is deliberately not the resource-order reconstruction's last entry. + let second_id = graph.add_resource(ResourceDesc::ImportedBuffer { + resource: second.clone(), + initial: state, + }); + graph + .imported_buffers_by_ptr + .insert(Arc::as_ptr(&second) as usize, second_id); + graph.imported_buffers_by_raw.insert(first.raw(), first_id); + + let ptr_before = graph.imported_buffers_by_ptr.clone(); + let raw_before = graph.imported_buffers_by_raw.clone(); + let resources_before = graph.resources.len(); + let failed = graph.import_transaction::<()>(|graph| { + // Pointer identity selects the second resource and updates its shared raw-key entry; + // the subsequent import error must restore the prior raw-key choice exactly. + graph.import_buffer(second.clone(), state)?; + Err(GraphError::ConflictingDescriptorInitialStates) + }); + + assert!(matches!( + failed, + Err(GraphError::ConflictingDescriptorInitialStates) + )); + assert_eq!(graph.resources.len(), resources_before); + assert_eq!(graph.imported_buffers_by_ptr, ptr_before); + assert_eq!(graph.imported_buffers_by_raw, raw_before); + assert_eq!( + graph.imported_buffers_by_raw.get(&first.raw()), + Some(&first_id) + ); + + // Pointer identity still selects the second resource, while a fresh Arc using the raw + // handle follows the exact pre-transaction raw index. + let fresh = test_buffer(111, 64); + assert_eq!(graph.import_buffer(fresh, state).unwrap().id(), first_id); + assert_eq!(graph.import_buffer(second, state).unwrap().id(), second_id); + } + + #[test] + fn descriptor_bound_uniform_copy_orders_before_draw_and_infers_usage() { + let buffer = test_buffer(107, 64); + let set = test_set( + 9, + vec![layout_binding(0, vk::DescriptorType::UNIFORM_BUFFER)], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + buffer.clone(), + 0, + 32, + )], + ); + let state = ResourceState::default(); + let mut graph = RenderGraph::new(); + let destination = graph + .import_descriptor_set(set.clone(), &[import_buffer_state(&buffer, state)], &[]) + .unwrap(); + let DescriptorResourceAccess::Buffer { + buffer: uniform, .. + } = destination.accesses[0] + else { + panic!("uniform descriptor maps to buffer access"); + }; + let source = graph.create_buffer(GraphBufferDesc { + size: 64, + ..Default::default() + }); + let range = BufferRange { + offset: 0, + size: 32, + }; + graph.push(CopyBuffer { + src: source, + src_range: range, + dst: uniform, + dst_range: range, + }); + graph.push(Draw { + args: DrawArgs::Direct { + vertex_count: 3, + instance_count: 1, + first_vertex: 0, + first_instance: 0, + }, + pipeline_layout: vk::PipelineLayout::null(), + vertices: Vec::new(), + index: None, + indirect: None, + descriptor_sets: vec![destination], + push_constants: Vec::new(), + }); + + let accesses = graph.commands.side_effects.command_accesses(); + assert!( + derive_buffer_usage( + &accesses_for_resource(uniform.id(), accesses), + vk::BufferUsageFlags::empty() + ) + .contains(vk::BufferUsageFlags::UNIFORM_BUFFER) + ); + let compiled = graph.compile().unwrap(); + assert_eq!( + compiled.batches(), + &[vec![CommandIndex(0)], vec![CommandIndex(1)]] + ); + let barrier = compiled + .barriers() + .iter() + .find(|barrier| barrier.before_command == CommandIndex(1)) + .unwrap(); + assert!(barrier.resources.iter().any(|resource| { + resource.id == uniform.id() + && resource.dst_access == vk::AccessFlags2::UNIFORM_READ + && resource.dst_stage == vk::PipelineStageFlags2::FRAGMENT_SHADER + })); + } + + #[test] + fn sampled_image_import_sets_usage_and_layout_barrier() { + let image = test_image(204); + let view = test_view(image.clone()); + let set = test_set( + 10, + vec![layout_binding(0, vk::DescriptorType::SAMPLED_IMAGE)], + vec![image_entry( + 0, + vk::DescriptorType::SAMPLED_IMAGE, + view, + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + )], + ); + let initial = ResourceState { + stage: vk::PipelineStageFlags2::TRANSFER, + access: vk::AccessFlags2::TRANSFER_WRITE, + layout: Some(vk::ImageLayout::GENERAL), + }; + let mut graph = RenderGraph::new(); + let descriptor_set = graph + .import_descriptor_set(set, &[import_texture_state(&image, initial)], &[]) + .unwrap(); + let DescriptorResourceAccess::Texture { texture, .. } = descriptor_set.accesses[0] else { + panic!("sampled image maps to texture access"); + }; + graph.push(Draw { + args: DrawArgs::Direct { + vertex_count: 3, + instance_count: 1, + first_vertex: 0, + first_instance: 0, + }, + pipeline_layout: vk::PipelineLayout::null(), + vertices: Vec::new(), + index: None, + indirect: None, + descriptor_sets: vec![descriptor_set], + push_constants: Vec::new(), + }); + let accesses = graph.commands.side_effects.command_accesses(); + assert!( + derive_image_usage( + &accesses_for_resource(texture.id(), accesses), + vk::ImageUsageFlags::empty() + ) + .contains(vk::ImageUsageFlags::SAMPLED) + ); + let compiled = graph.compile().unwrap(); + let barrier = &compiled.barriers()[0].resources[0]; + assert_eq!(barrier.old_layout, Some(vk::ImageLayout::GENERAL)); + assert_eq!( + barrier.new_layout, + Some(vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL) + ); + assert_eq!(barrier.dst_access, vk::AccessFlags2::SHADER_SAMPLED_READ); + } + + #[test] + fn sampler_only_managed_binding_has_no_graph_resource_or_memory_access() { + let sampler = Arc::new(Sampler::for_testing(vk::Sampler::from_raw(1))); + let set = test_set( + 11, + vec![layout_binding(0, vk::DescriptorType::SAMPLER)], + vec![TrackedDescriptorWrite { + binding: 0, + array_element: 0, + descriptor_type: vk::DescriptorType::SAMPLER, + stages: vk::ShaderStageFlags::FRAGMENT, + resource: DescriptorResourceBinding::Sampler { sampler }, + }], + ); + let mut graph = RenderGraph::new(); + let imported = graph.import_descriptor_set(set, &[], &[]).unwrap(); + assert!(imported.accesses.is_empty()); + assert!(graph.resources.is_empty()); + } + + #[test] + fn dynamic_offsets_across_sets_follow_binding_order_and_validate_effective_ranges() { + let buffer = test_buffer(108, 256); + let first = test_set( + 12, + vec![ + DescriptorSetLayoutBindingDesc { + binding: 8, + count: 1, + ..layout_binding(8, vk::DescriptorType::STORAGE_BUFFER_DYNAMIC) + }, + DescriptorSetLayoutBindingDesc { + binding: 2, + count: 1, + ..layout_binding(2, vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC) + }, + ], + vec![ + buffer_entry( + 2, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + buffer.clone(), + 16, + 32, + ), + buffer_entry( + 8, + vk::DescriptorType::STORAGE_BUFFER_DYNAMIC, + buffer.clone(), + 64, + 32, + ), + ], + ); + let second = test_set( + 13, + vec![layout_binding( + 1, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + )], + vec![buffer_entry( + 1, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + buffer.clone(), + 128, + 32, + )], + ); + let state = ResourceState::default(); + let mut graph = RenderGraph::new(); + let initial = [import_buffer_state(&buffer, state)]; + let first = graph + .import_descriptor_set(first, &initial, &[8, 16]) + .unwrap(); + let second = graph + .import_descriptor_set(second, &initial, &[24]) + .unwrap(); + assert_eq!(first.dynamic_offsets(), &[8, 16]); + assert_eq!(second.dynamic_offsets(), &[24]); + assert_eq!( + first + .dynamic_offsets() + .iter() + .chain(second.dynamic_offsets()) + .copied() + .collect::>(), + [8, 16, 24] + ); + let ranges = first + .accesses + .iter() + .chain(&second.accesses) + .map(|access| match access { + DescriptorResourceAccess::Buffer { range, .. } => *range, + _ => panic!("dynamic buffer descriptor maps to buffer range"), + }) + .collect::>(); + assert_eq!( + ranges, + [ + BufferRange { + offset: 24, + size: 32 + }, + BufferRange { + offset: 80, + size: 32 + }, + BufferRange { + offset: 152, + size: 32 + }, + ] + ); + + let out_of_bounds = test_set( + 14, + vec![layout_binding( + 0, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + )], + vec![buffer_entry( + 0, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + buffer.clone(), + 224, + 32, + )], + ); + assert!( + graph + .import_descriptor_set(out_of_bounds, &initial, &[8]) + .is_err() + ); + } + + #[test] + fn managed_descriptor_import_rejects_unsupported_descriptor_kind() { + let set = test_set( + 15, + vec![layout_binding(0, vk::DescriptorType::INPUT_ATTACHMENT)], + vec![TrackedDescriptorWrite { + binding: 0, + array_element: 0, + descriptor_type: vk::DescriptorType::INPUT_ATTACHMENT, + stages: vk::ShaderStageFlags::FRAGMENT, + resource: DescriptorResourceBinding::Sampler { + sampler: Arc::new(Sampler::for_testing(vk::Sampler::from_raw(2))), + }, + }], + ); + assert!(matches!( + RenderGraph::new().import_descriptor_set(set, &[], &[]), + Err(GraphError::DescriptorImport(_)) + )); + } #[test] fn kahn_emits_dag_in_deterministic_order() { @@ -1327,6 +2833,106 @@ mod tests { ); } + #[test] + fn descriptor_import_helpers_validate_effective_ranges_and_dynamic_order() { + assert_eq!( + effective_buffer_range(128, 16, 32, 64), + Ok(BufferRange { + offset: 80, + size: 32 + }) + ); + assert!(effective_buffer_range(128, 96, 32, 16).is_err()); + assert!(effective_buffer_range(128, u64::MAX, 16, 1).is_err()); + assert!(effective_buffer_range(128, 16, vk::WHOLE_SIZE, 0).is_ok()); + assert!(effective_buffer_range(128, 16, vk::WHOLE_SIZE, 4).is_err()); + assert!(is_dynamic_offset_aligned(64, 32)); + assert!(!is_dynamic_offset_aligned(16, 32)); + assert!(!is_dynamic_offset_aligned(0, 0)); + + let layout = [ + renderer_resources::pipeline::DescriptorSetLayoutBindingDesc { + binding: 9, + count: 2, + kind: vk::DescriptorType::STORAGE_BUFFER_DYNAMIC, + ..Default::default() + }, + renderer_resources::pipeline::DescriptorSetLayoutBindingDesc { + binding: 2, + count: 1, + kind: vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + ..Default::default() + }, + ]; + assert_eq!( + dynamic_offset_slots(&layout), + [ + (2, 0, vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC), + (9, 0, vk::DescriptorType::STORAGE_BUFFER_DYNAMIC), + (9, 1, vk::DescriptorType::STORAGE_BUFFER_DYNAMIC), + ] + ); + assert_eq!( + descriptor_access(vk::DescriptorType::UNIFORM_BUFFER), + Some((vk::AccessFlags2::UNIFORM_READ, None)) + ); + assert_eq!( + descriptor_access(vk::DescriptorType::SAMPLED_IMAGE), + Some((vk::AccessFlags2::SHADER_SAMPLED_READ, None)) + ); + assert_eq!( + descriptor_access(vk::DescriptorType::STORAGE_IMAGE), + Some(( + vk::AccessFlags2::SHADER_STORAGE_READ | vk::AccessFlags2::SHADER_STORAGE_WRITE, + Some(vk::ImageLayout::GENERAL), + )) + ); + assert_eq!(descriptor_access(vk::DescriptorType::SAMPLER), None); + assert_eq!( + shader_stages_to_pipeline( + vk::ShaderStageFlags::VERTEX | vk::ShaderStageFlags::FRAGMENT + ), + Some(vk::PipelineStageFlags2::VERTEX_SHADER | vk::PipelineStageFlags2::FRAGMENT_SHADER) + ); + assert_eq!( + shader_stages_to_pipeline(vk::ShaderStageFlags::ALL), + Some(vk::PipelineStageFlags2::ALL_COMMANDS) + ); + assert!(texture_layouts_conflict( + texture_range(vk::ImageAspectFlags::COLOR, 0, 1, 0, 1), + vk::ImageLayout::GENERAL, + texture_range(vk::ImageAspectFlags::COLOR, 0, 1, 0, 1), + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + )); + assert!(!texture_layouts_conflict( + texture_range(vk::ImageAspectFlags::COLOR, 0, 1, 0, 1), + vk::ImageLayout::GENERAL, + texture_range(vk::ImageAspectFlags::COLOR, 1, 1, 0, 1), + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + )); + } + + #[test] + fn raw_and_managed_descriptor_binding_offsets_are_concatenated_by_set_order() { + let first = GraphDescriptorSet { + raw: vk::DescriptorSet::null(), + accesses: Vec::new(), + managed: None, + dynamic_offsets: vec![4, 8], + }; + let second = GraphDescriptorSet { + raw: vk::DescriptorSet::null(), + accesses: Vec::new(), + managed: None, + dynamic_offsets: vec![12], + }; + let combined = [first, second] + .iter() + .flat_map(|set| set.dynamic_offsets().iter().copied()) + .collect::>(); + assert_eq!(combined, [4, 8, 12]); + } + #[test] fn write_then_read_creates_order_and_barrier() { let mut graph = RenderGraph::new(); diff --git a/crates/renderer_rendergraph/src/lib.rs b/crates/renderer_rendergraph/src/lib.rs index 525216a..2bb4501 100644 --- a/crates/renderer_rendergraph/src/lib.rs +++ b/crates/renderer_rendergraph/src/lib.rs @@ -9,7 +9,7 @@ pub mod resources; pub use graph::{CompiledGraph, GraphError, RenderGraph, ResourceInterference}; pub use recorder::{CommandIndex, CommandList, CommandRecorder, SideEffectMap}; pub use resources::{ - Buffer, BufferRange, BufferSlice, DescriptorResourceAccess, GraphBufferDesc, - GraphDescriptorSet, GraphTextureDesc, IndexBinding, IndexFormat, IndirectBinding, - PushConstantData, ResourceId, Texture, VertexBinding, + Buffer, BufferRange, BufferSlice, DescriptorResourceAccess, DescriptorResourceIdentity, + GraphBufferDesc, GraphDescriptorSet, GraphTextureDesc, IndexBinding, IndexFormat, + IndirectBinding, PushConstantData, ResourceId, ResourceState, Texture, VertexBinding, }; diff --git a/crates/renderer_rendergraph/src/recorder.rs b/crates/renderer_rendergraph/src/recorder.rs index 16912b1..6235940 100644 --- a/crates/renderer_rendergraph/src/recorder.rs +++ b/crates/renderer_rendergraph/src/recorder.rs @@ -100,6 +100,11 @@ pub struct CommandList { } pub trait RecordedCommand { + /// Validate wrapper-backed state before any command in the graph is emitted. + fn preflight(&self) -> Result<(), RecordError> { + Ok(()) + } + fn apply(&mut self, recorder: &mut CommandRecorder<'_>) -> Result<(), RecordError>; } @@ -125,6 +130,13 @@ impl CommandList { pub fn is_empty(&self) -> bool { self.commands.is_empty() } + + pub(crate) fn preflight(&self) -> Result<(), RecordError> { + for command in &self.commands { + command.preflight()?; + } + Ok(()) + } } impl Default for CommandList { @@ -139,6 +151,8 @@ pub enum RecordError { MissingImage(ResourceId), UnresolvedCreatedBuffer(ResourceId), UnresolvedCreatedImage(ResourceId), + DescriptorSet(String), + StaleDescriptorSet, } /// Vulkan recording context. Handle maps are populated by the compiled graph. diff --git a/crates/renderer_rendergraph/src/resources.rs b/crates/renderer_rendergraph/src/resources.rs index deb821d..d3f16a6 100644 --- a/crates/renderer_rendergraph/src/resources.rs +++ b/crates/renderer_rendergraph/src/resources.rs @@ -4,6 +4,7 @@ use ash::vk; use renderer_resources::{ buffers::Buffer as BufferResource, images::{Image as ImageResource, ImageSubresourceExt}, + pipeline::DescriptorSet, }; /// Minimal graph-level buffer description. Buffer usage, queue sharing, and @@ -185,15 +186,30 @@ pub enum DescriptorResourceAccess { } /// A Vulkan descriptor set accompanied by graph-visible resource accesses. -/// -/// Callers must list every resource used by the set with its accurate range, -/// pipeline stage, access mask, and (for textures) image layout. The graph can -/// track these declarations, but cannot inspect or verify raw descriptor-set -/// contents. +/// Managed instances retain their descriptor wrapper and dynamic offsets; +/// raw instances created through `track_descriptor_set` are untracked and +/// require caller-managed resource lifetime and access declarations. #[derive(Debug, Clone)] pub struct GraphDescriptorSet { - pub raw: vk::DescriptorSet, + pub(crate) raw: vk::DescriptorSet, pub accesses: Vec, + /// Retained managed set, when this packet was created by transitive import. + pub(crate) managed: Option>, + /// Offsets ordered by the descriptor set's binding and array-element order. + pub(crate) dynamic_offsets: Vec, +} + +impl GraphDescriptorSet { + pub fn dynamic_offsets(&self) -> &[u32] { + &self.dynamic_offsets + } +} + +/// A resource identity used to supply explicit initial state during descriptor import. +#[derive(Debug, Clone)] +pub enum DescriptorResourceIdentity { + Buffer(Arc), + Texture(Arc), } #[derive(Debug, Clone)] diff --git a/crates/renderer_resources/AGENTS.md b/crates/renderer_resources/AGENTS.md index 7391d9a..05a16b4 100644 --- a/crates/renderer_resources/AGENTS.md +++ b/crates/renderer_resources/AGENTS.md @@ -4,10 +4,11 @@ `renderer_resources` owns Vulkan buffer, image, view, descriptor, sampler, shader-module, and pipeline construction on the `renderer_device` layer. It handles resource memory and handle lifetimes, but contains no window, swapchain, or presentation orchestration; those remain in `renderer`. It must not depend on `renderer`. ## Module map -- `lib.rs`: re-exports `Device`, `Error`, `Result`, `device`, and `make_extension` from `renderer_device`; declares four public resource modules. +- `lib.rs`: re-exports `Device`, `Error`, `Result`, `device`, and `make_extension` from `renderer_device`; declares the public resource modules, including descriptor metadata and validation. - `buffers.rs`: `BufferDesc` and `Buffer`; Vulkan buffer creation, GPU allocation/binding, raw access, and host mapping. - `images.rs`: `ImageDesc`, `Image`, `ImageViewDesc`, `ImageView`, subresource ranges, extents, offsets, and queue ownership. `ImageInner` distinguishes allocated images from borrowed swapchain images. - `pipeline.rs`: shader-stage and graphics/compute pipeline descriptions; `DescriptorPool`, `DescriptorSetLayout`, `PipelineLayout`, `Sampler`, `ShaderModule`, and `Pipeline`. +- `descriptors.rs`: managed descriptor binding/resource metadata, validation plans, errors, and pool-generation helpers; it remains independent of render-graph types. - `util.rs`: `eq_f32`, `ord_f32`, `hash_f32`, `F32`, and the crate-private `weak_vec` module / `WeakVec` used by image view tracking. ## Key types and API @@ -25,8 +26,13 @@ - Fixed-function state records are `VertexInputState`, `TessellationState`, `InputAssemblyState`, `ViewportState`, `DepthBiasState`, `RasterizationState`, `MultisampleState`, `DepthBounds`, `DepthState`, `StencilState`, `DepthStencilState`, `ColorBlendState`, `RenderingState`, and `DynamicState`. - `Pipeline` wraps graphics or compute Vulkan pipelines and retains its `PipelineLayout` in an `Arc`. Creation uses `device.pools.pipeline_cache.raw`. - `DescriptorPool` serializes allocation/reset with a mutex; descriptor set layouts and pipeline layouts own their Vulkan handles. -- `DescriptorPool::allocate` returns raw descriptor-set handles from the provided `DescriptorSetLayout` values and optionally assigns names from `DescriptorSetAllocDesc`. -- `DescriptorSetLayout::new` applies per-binding descriptor flags when the device supports Vulkan 1.2 or `VK_EXT_descriptor_indexing`. +- `DescriptorPool::allocate_untracked` returns raw descriptor-set handles without retaining layouts or bound resources. `allocate_managed` returns a generation-checked `DescriptorSet` that retains its pool, layout facts, and written resource owners. +- A managed `DescriptorSet` retains its pool-allocation token, layout facts, and the `Arc`, `Arc`, `Arc`, or `Arc` owners supplied for its written descriptors. Image views in turn retain their parent images. +- `DescriptorSet::raw()` is fallible and checks the pool generation before exposing the Vulkan handle. Generation checks also apply to binding metadata and writer operations; they detect sets invalidated by reset but cannot revoke escaped raw handles or make concurrent reset/use safe. +- `DescriptorSet::writer` stages buffer, image, sampler, and texel-buffer updates. `DescriptorWriter` validates the descriptor type, slot bounds, ranges, resource facts, and required enabled features before Vulkan updates; retained owners are replaced only after the Vulkan writes succeed. Partially-bound bindings may keep unwritten slots; other bindings must be complete at commit. +- Managed writes support `UNIFORM_BUFFER`, `UNIFORM_BUFFER_DYNAMIC`, `STORAGE_BUFFER`, `STORAGE_BUFFER_DYNAMIC`, `SAMPLED_IMAGE`, `STORAGE_IMAGE`, `COMBINED_IMAGE_SAMPLER`, `SAMPLER`, `UNIFORM_TEXEL_BUFFER`, and `STORAGE_TEXEL_BUFFER`. Input attachments, acceleration structures, inline uniform blocks, and all other descriptor types are rejected until explicitly implemented. +- Managed image writes reject swapchain-backed images (including combined image samplers) before updating Vulkan descriptors, because retaining an `ImageView` does not retain its swapchain. Use the untracked route only when the caller separately guarantees the external image/swapchain lifetime. +- `DescriptorSetLayout::new` chains per-binding descriptor flags when the device API/extension supports the Vulkan structure and validates requested flags against enabled descriptor-indexing features. - `PipelineLayoutDesc` references descriptor-set layouts and push-constant ranges; `PipelineLayout` owns the created Vulkan layout. - `SamplerDesc` describes filtering, addressing, anisotropy, compare, LOD, and border settings; `Sampler::new` creates the Vulkan sampler. - `ShaderModule::new_from_path` reads a shader file relative to the process working directory; file-open failures map to `Error::AssetMissing`, while read/Vulkan failures use their corresponding `Error` conversions. `new_from_memory` accepts SPIR-V words. @@ -45,10 +51,13 @@ - `ImageView` retains `Arc` so the source image outlives the view. `Image::create_view` returns `ManuallyDrop`; callers must transfer/drop it deliberately. - `Image` tracks views through `WeakVec` to avoid a strong-reference cycle. The weak list does not keep views alive. - `ImageView`'s `DeviceObject` owns and destroys the Vulkan view; its parent `Arc` must remain alive through view destruction. +- `Image::create_view` returns `ManuallyDrop`; when a managed descriptor needs an owned view, deliberately transfer it with `ManuallyDrop::into_inner` and place it in an `Arc`. - `ImageDesc` and `BufferDesc` omit `alloc_scheme` from both `Hash` and `Eq`; their `Hash` implementations omit `name`, and `Eq` also omits `name`. Do not treat these descriptors as complete resource identities without accounting for these fields. - Descriptor hashing/equality covers resource shape, usage, sharing, and memory location but not every construction input. Update both implementations together if adding descriptor-keyed resource deduplication. - `Pipeline` retains its layout with `Arc`; descriptor-pool operations are mutex-serialized. Keep these lifetime and synchronization guarantees when changing constructors or wrappers. -- `DescriptorPool::allocate` returns raw `vk::DescriptorSet` handles; keep the owning pool alive while those sets are in use. Descriptor sets are not wrapped in owning resource objects here. +- Descriptor indexing validation uses `Device::enabled_features()` (the feature snapshot requested for logical-device creation), not `Device::features()` (adapter-supported features). Managed layouts validate binding-flag and descriptor-type-specific feature requirements, and managed writes validate descriptor requirements against the same enabled snapshot. +- `DescriptorPool::allocate_untracked` (and its backwards-compatible `allocate` alias) is a raw escape hatch: it retains no layout or binding owners, owns no managed-set token, and is not eligible for transitive managed import. Any raw handles are the caller's responsibility. +- Managed pool `reset` and `destroy` are unsafe caller-synchronized operations. Before reset/destruction, callers must ensure relevant GPU work has completed and exclude all managed-set use, including allocation, writes, graph recording/import, and command-buffer recording/binding. Final pool destruction/drop has the same synchronization requirement. Generation checks are defense-in-depth and do not close concurrent reset/use races; wrapper drops do not prove GPU retirement. Any raw handle returned by `raw()` escapes these checks and remains the caller's responsibility. Raw/untracked reset and use are likewise caller-synchronized. - Descriptor-set layout binding flags are chained into layout creation when the device API version or descriptor-indexing extension supports them. - Pipeline cache lifetime and teardown belong to `renderer_device`; use the device's existing cache rather than adding a resource-crate cache. - Renderer shader assets remain under `crates/renderer/shaders`; this crate does not own those assets. diff --git a/crates/renderer_resources/Cargo.toml b/crates/renderer_resources/Cargo.toml index e51badd..7966eb7 100644 --- a/crates/renderer_resources/Cargo.toml +++ b/crates/renderer_resources/Cargo.toml @@ -3,6 +3,9 @@ name = "renderer_resources" version = "0.1.0" edition = "2024" +[features] +testing = [] + [dependencies] renderer_device = { path = "../renderer_device" } ash = { workspace = true } diff --git a/crates/renderer_resources/src/buffers.rs b/crates/renderer_resources/src/buffers.rs index 05c36c3..4babd82 100644 --- a/crates/renderer_resources/src/buffers.rs +++ b/crates/renderer_resources/src/buffers.rs @@ -1,4 +1,4 @@ -use std::borrow::Cow; +use std::{borrow::Cow, sync::Arc}; use ash::vk; use gpu_allocator::vulkan::AllocationScheme; @@ -73,19 +73,34 @@ impl Default for BufferDesc { #[derive(Debug)] pub struct Buffer { - buffer: DeviceObject, + buffer: BufferHandle, pub desc: BufferDesc, alloc: Allocation, } +#[derive(Debug)] +enum BufferHandle { + Owned(DeviceObject), + #[cfg(feature = "testing")] + Testing(vk::Buffer), +} + impl Eq for Buffer {} impl PartialEq for Buffer { fn eq(&self, other: &Self) -> bool { - *self.buffer == *other.buffer + self.raw() == other.raw() } } impl Buffer { + fn owned(&self) -> Option<&DeviceObject> { + match &self.buffer { + BufferHandle::Owned(buffer) => Some(buffer), + #[cfg(feature = "testing")] + BufferHandle::Testing(_) => None, + } + } + pub fn new(device: Device, desc: BufferDesc) -> crate::Result { let (buffer, requirements) = Self::new_raw(device.clone(), &desc)?; @@ -113,7 +128,11 @@ impl Buffer { } Ok(Self { - buffer: DeviceObject::new_debug_named(device.clone(), buffer, desc.name.clone()), + buffer: BufferHandle::Owned(DeviceObject::new_debug_named( + device.clone(), + buffer, + desc.name.clone(), + )), desc, alloc: Allocation::Owned(DeviceObject::new(device, alloc)), }) @@ -163,9 +182,78 @@ impl Buffer { } pub fn raw(&self) -> vk::Buffer { - *self.buffer + match &self.buffer { + BufferHandle::Owned(buffer) => **buffer, + #[cfg(feature = "testing")] + BufferHandle::Testing(buffer) => *buffer, + } } pub fn len(&self) -> u64 { self.desc.size } + + /// Creates a non-owning buffer fixture for device-free integration tests. + /// + /// This constructor is only available with the crate's `testing` feature. The raw handle is + /// never destroyed and must not be used for Vulkan calls. + #[cfg(feature = "testing")] + #[doc(hidden)] + pub fn for_testing(raw: vk::Buffer, size: u64) -> Self { + Self { + buffer: BufferHandle::Testing(raw), + desc: BufferDesc { + size, + ..Default::default() + }, + alloc: Allocation::Unmanaged, + } + } +} + +/// Byte range selected by a buffer view. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BufferRange { + pub offset: u64, + pub size: u64, +} + +/// An owned Vulkan buffer view, retaining the buffer it refers to. +#[derive(Debug)] +pub struct BufferView { + raw: vk::BufferView, + buffer: Arc, + range: BufferRange, +} + +impl BufferView { + pub fn new(buffer: Arc, format: vk::Format, range: BufferRange) -> crate::Result { + let info = vk::BufferViewCreateInfo::default() + .buffer(buffer.raw()) + .format(format) + .offset(range.offset) + .range(range.size); + let Some(buffer_handle) = buffer.owned() else { + return Err(crate::Error::Unspecified); + }; + let raw = unsafe { buffer_handle.device().raw.create_buffer_view(&info, None)? }; + Ok(Self { raw, buffer, range }) + } + + pub fn raw(&self) -> vk::BufferView { + self.raw + } + pub fn buffer(&self) -> &Arc { + &self.buffer + } + pub fn range(&self) -> BufferRange { + self.range + } +} + +impl Drop for BufferView { + fn drop(&mut self) { + if let Some(buffer) = self.buffer.owned() { + unsafe { buffer.device().raw.destroy_buffer_view(self.raw, None) }; + } + } } diff --git a/crates/renderer_resources/src/descriptors.rs b/crates/renderer_resources/src/descriptors.rs new file mode 100644 index 0000000..1d5318c --- /dev/null +++ b/crates/renderer_resources/src/descriptors.rs @@ -0,0 +1,987 @@ +//! Graph-independent descriptor resource metadata and write validation. + +use std::sync::Arc; + +use ash::vk; +use renderer_device::PhysicalDeviceFeatures; + +use crate::{ + buffers::{Buffer, BufferView}, + images::ImageView, + pipeline::Sampler, +}; + +/// An owned resource entry suitable for retention by a tracked descriptor set. +#[derive(Debug, Clone)] +pub enum DescriptorResourceBinding { + Buffer { + buffer: Arc, + offset: u64, + range: u64, + dynamic: bool, + }, + Image { + view: Arc, + sampler: Option>, + range: vk::ImageSubresourceRange, + layout: vk::ImageLayout, + }, + Sampler { + sampler: Arc, + }, + TexelBuffer { + view: Arc, + range: crate::buffers::BufferRange, + }, +} + +/// Errors produced by managed descriptor-set operations. +#[derive(Debug)] +pub enum DescriptorSetError { + Vulkan(crate::Error), + Validation(DescriptorValidationError), + StaleSet { + allocated_generation: u64, + pool_generation: u64, + }, + IncompleteBinding { + binding: u32, + element: u32, + }, + PoolStillRetained, + PoolGenerationExhausted, +} + +pub type DescriptorSetResult = Result; + +impl std::fmt::Display for DescriptorSetError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Vulkan(error) => error.fmt(f), + Self::Validation(error) => error.fmt(f), + Self::StaleSet { + allocated_generation, + pool_generation, + } => write!( + f, + "descriptor set is stale (allocated at generation {allocated_generation}, pool is at generation {pool_generation})" + ), + Self::IncompleteBinding { binding, element } => write!( + f, + "descriptor binding {binding} has unwritten array element {element}" + ), + Self::PoolStillRetained => { + f.write_str("managed descriptor sets still retain this descriptor pool") + } + Self::PoolGenerationExhausted => f.write_str("descriptor pool generation exhausted"), + } + } +} + +impl std::error::Error for DescriptorSetError {} + +impl From for DescriptorSetError { + fn from(error: crate::Error) -> Self { + Self::Vulkan(error) + } +} + +impl From for DescriptorSetError { + fn from(error: DescriptorValidationError) -> Self { + Self::Validation(error) + } +} + +pub(crate) fn check_pool_generation( + allocated_generation: u64, + pool_generation: u64, +) -> DescriptorSetResult<()> { + if allocated_generation == pool_generation { + Ok(()) + } else { + Err(DescriptorSetError::StaleSet { + allocated_generation, + pool_generation, + }) + } +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum PoolResetError { + Reset(E), + GenerationExhausted, +} + +pub(crate) fn pool_generation_after_reset( + generation: u64, + reset: impl FnOnce() -> Result<(), E>, +) -> Result> { + let next = generation + .checked_add(1) + .ok_or(PoolResetError::GenerationExhausted)?; + reset().map_err(PoolResetError::Reset)?; + Ok(next) +} + +pub(crate) fn replace_binding_entries( + current: &std::collections::BTreeMap<(u32, u32), T>, + writes: impl IntoIterator, +) -> std::collections::BTreeMap<(u32, u32), T> { + let mut replacement = current.clone(); + replacement.extend(writes); + replacement +} + +pub(crate) fn clear_binding_entries( + entries: &mut std::collections::BTreeMap<(u32, u32), T>, + binding: u32, +) { + entries.retain(|(entry_binding, _), _| *entry_binding != binding); +} + +pub(crate) fn validate_binding_completeness( + layout: &[crate::pipeline::DescriptorSetLayoutBindingDesc], + entries: &std::collections::BTreeMap<(u32, u32), T>, +) -> DescriptorSetResult<()> { + for binding in layout { + if binding + .flags + .unwrap_or_default() + .contains(vk::DescriptorBindingFlags::PARTIALLY_BOUND) + { + continue; + } + for element in 0..binding.count { + if !entries.contains_key(&(binding.binding, element)) { + return Err(DescriptorSetError::IncompleteBinding { + binding: binding.binding, + element, + }); + } + } + } + Ok(()) +} + +/// An owned descriptor write. Texel-buffer kind is carried by `descriptor_type`. +#[derive(Debug, Clone)] +pub struct TrackedDescriptorWrite { + pub binding: u32, + pub array_element: u32, + pub descriptor_type: vk::DescriptorType, + pub stages: vk::ShaderStageFlags, + pub resource: DescriptorResourceBinding, +} + +/// Resource facts attached to a raw Vulkan write, independent of Vulkan handles. +#[derive(Debug, Clone, Copy)] +pub enum RawDescriptorResourceFacts { + Buffer { + buffer_len: u64, + offset: u64, + range: u64, + dynamic: bool, + }, + Image { + swapchain_backed: bool, + has_view: bool, + has_sampler: bool, + }, + Sampler { + has_sampler: bool, + }, + TexelBuffer { + buffer_len: u64, + offset: u64, + range: u64, + }, +} + +/// The descriptor-specific facts needed to validate a pending write. +#[derive(Debug, Clone, Copy)] +pub struct RawDescriptorWriteFacts { + pub binding: u32, + pub array_element: u32, + pub descriptor_type: vk::DescriptorType, + pub resource: RawDescriptorResourceFacts, +} + +/// A validated write, with whole-size ranges normalized to concrete lengths. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ValidatedDescriptorWrite { + pub binding: u32, + pub array_element: u32, + pub descriptor_type: vk::DescriptorType, + pub normalized_range: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DescriptorValidationError { + UnknownBinding(u32), + ArrayElementOutOfBounds { + binding: u32, + element: u32, + count: u32, + }, + TypeMismatch { + binding: u32, + expected: vk::DescriptorType, + actual: vk::DescriptorType, + }, + UnsupportedDescriptorType(vk::DescriptorType), + ResourceTypeMismatch(vk::DescriptorType), + BufferRangeOutOfBounds, + InvalidDynamicAlignment, + MisalignedDynamicOffset, + MissingImageView, + MissingImageSampler, + MissingSampler, + SwapchainImage, + BindingNotPartiallyBound(u32), + FeatureNotEnabled { + binding: u32, + feature: &'static str, + }, +} + +impl std::fmt::Display for DescriptorValidationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::UnknownBinding(binding) => write!( + f, + "descriptor binding {binding} does not exist in the layout" + ), + Self::ArrayElementOutOfBounds { + binding, + element, + count, + } => write!( + f, + "descriptor array element {element} is outside binding {binding}'s count {count}" + ), + Self::TypeMismatch { + binding, + expected, + actual, + } => write!( + f, + "descriptor type {actual:?} does not match binding {binding}'s type {expected:?}" + ), + Self::UnsupportedDescriptorType(kind) => write!( + f, + "descriptor type {kind:?} is unsupported for tracked writes" + ), + Self::ResourceTypeMismatch(kind) => write!( + f, + "descriptor resource facts do not match descriptor type {kind:?}" + ), + Self::BufferRangeOutOfBounds => { + f.write_str("descriptor buffer range exceeds the buffer length") + } + Self::InvalidDynamicAlignment => { + f.write_str("dynamic buffer descriptor has a zero alignment") + } + Self::MisalignedDynamicOffset => { + f.write_str("dynamic buffer descriptor offset is not aligned") + } + Self::MissingImageView => f.write_str("image descriptor requires an image view"), + Self::MissingImageSampler => { + f.write_str("combined image sampler descriptor requires a sampler") + } + Self::MissingSampler => f.write_str("sampler descriptor requires a sampler"), + Self::SwapchainImage => { + f.write_str("managed descriptor writes cannot reference swapchain-backed images") + } + Self::BindingNotPartiallyBound(binding) => write!( + f, + "descriptor binding {binding} cannot be cleared because it is not partially bound" + ), + Self::FeatureNotEnabled { binding, feature } => write!( + f, + "binding {binding} requires an enabled descriptor-indexing feature: {feature}" + ), + } + } +} + +impl std::error::Error for DescriptorValidationError {} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DescriptorWritePlan { + pub writes: Vec, +} + +/// Checks all descriptor-indexing flags against the logical-device enabled-feature snapshot. +pub fn validate_descriptor_binding_features( + bindings: &[crate::pipeline::DescriptorSetLayoutBindingDesc], + enabled: &PhysicalDeviceFeatures, +) -> Result<(), DescriptorValidationError> { + for binding in bindings { + if !is_supported_descriptor_type(binding.kind) { + return Err(DescriptorValidationError::UnsupportedDescriptorType( + binding.kind, + )); + } + let flags = binding.flags.unwrap_or_default(); + let feature_check = |enabled: bool, name| { + if enabled { + Ok(()) + } else { + Err(DescriptorValidationError::FeatureNotEnabled { + binding: binding.binding, + feature: name, + }) + } + }; + + if flags.contains(vk::DescriptorBindingFlags::PARTIALLY_BOUND) { + feature_check( + enabled.core12.descriptor_binding_partially_bound != vk::FALSE, + "descriptorBindingPartiallyBound", + )?; + } + if flags.contains(vk::DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT) { + feature_check( + enabled.core12.descriptor_binding_variable_descriptor_count != vk::FALSE, + "descriptorBindingVariableDescriptorCount", + )?; + feature_check( + enabled.core12.runtime_descriptor_array != vk::FALSE, + "runtimeDescriptorArray", + )?; + } + if flags.contains(vk::DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING) { + feature_check( + enabled + .core12 + .descriptor_binding_update_unused_while_pending + != vk::FALSE, + "descriptorBindingUpdateUnusedWhilePending", + )?; + feature_check( + descriptor_type_update_after_bind_enabled(binding.kind, enabled), + descriptor_type_update_after_bind_feature(binding.kind), + )?; + } + if flags.contains(vk::DescriptorBindingFlags::UPDATE_AFTER_BIND) { + feature_check( + descriptor_type_update_after_bind_enabled(binding.kind, enabled), + descriptor_type_update_after_bind_feature(binding.kind), + )?; + } + } + Ok(()) +} + +/// Validates layout membership, array bounds, supported type, owner facts, and buffer ranges. +/// Dynamic alignment values are Vulkan's `min*BufferOffsetAlignment` limits. +pub fn validate_descriptor_writes( + layout: &[crate::pipeline::DescriptorSetLayoutBindingDesc], + writes: &[RawDescriptorWriteFacts], + enabled_features: &PhysicalDeviceFeatures, + min_uniform_buffer_offset_alignment: u64, + min_storage_buffer_offset_alignment: u64, +) -> Result { + validate_descriptor_binding_features(layout, enabled_features)?; + + let mut validated = Vec::with_capacity(writes.len()); + for write in writes { + let binding = layout + .iter() + .find(|binding| binding.binding == write.binding) + .ok_or(DescriptorValidationError::UnknownBinding(write.binding))?; + + if write.array_element >= binding.count { + return Err(DescriptorValidationError::ArrayElementOutOfBounds { + binding: write.binding, + element: write.array_element, + count: binding.count, + }); + } + + if write.descriptor_type != binding.kind { + return Err(DescriptorValidationError::TypeMismatch { + binding: write.binding, + expected: binding.kind, + actual: write.descriptor_type, + }); + } + + if !is_supported_descriptor_type(write.descriptor_type) { + return Err(DescriptorValidationError::UnsupportedDescriptorType( + write.descriptor_type, + )); + } + + let normalized_range = match (write.descriptor_type, write.resource) { + ( + vk::DescriptorType::UNIFORM_BUFFER + | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC, + RawDescriptorResourceFacts::Buffer { + buffer_len, + offset, + range, + dynamic, + }, + ) => { + let range = normalize_buffer_range(buffer_len, offset, range)?; + + if dynamic + != (write.descriptor_type == vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + || write.descriptor_type == vk::DescriptorType::STORAGE_BUFFER_DYNAMIC) + { + return Err(DescriptorValidationError::ResourceTypeMismatch( + write.descriptor_type, + )); + } + + if dynamic { + let alignment = + if write.descriptor_type == vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC { + min_uniform_buffer_offset_alignment + } else { + min_storage_buffer_offset_alignment + }; + if alignment == 0 { + return Err(DescriptorValidationError::InvalidDynamicAlignment); + } + if offset % alignment != 0 { + return Err(DescriptorValidationError::MisalignedDynamicOffset); + } + } + + Some(range) + } + ( + vk::DescriptorType::UNIFORM_TEXEL_BUFFER | vk::DescriptorType::STORAGE_TEXEL_BUFFER, + RawDescriptorResourceFacts::TexelBuffer { + buffer_len, + offset, + range, + }, + ) => Some(normalize_buffer_range(buffer_len, offset, range)?), + ( + vk::DescriptorType::SAMPLED_IMAGE | vk::DescriptorType::STORAGE_IMAGE, + RawDescriptorResourceFacts::Image { + swapchain_backed, + has_view, + .. + }, + ) => { + check_image_facts(swapchain_backed, has_view, false, false)?; + None + } + ( + vk::DescriptorType::COMBINED_IMAGE_SAMPLER, + RawDescriptorResourceFacts::Image { + swapchain_backed, + has_view, + has_sampler, + }, + ) => { + check_image_facts(swapchain_backed, has_view, has_sampler, true)?; + None + } + (vk::DescriptorType::SAMPLER, RawDescriptorResourceFacts::Sampler { has_sampler }) => { + if !has_sampler { + return Err(DescriptorValidationError::MissingSampler); + } + None + } + _ => { + return Err(DescriptorValidationError::ResourceTypeMismatch( + write.descriptor_type, + )); + } + }; + + validated.push(ValidatedDescriptorWrite { + binding: write.binding, + array_element: write.array_element, + descriptor_type: write.descriptor_type, + normalized_range, + }); + } + + Ok(DescriptorWritePlan { writes: validated }) +} + +fn normalize_buffer_range( + buffer_len: u64, + offset: u64, + range: u64, +) -> Result { + if offset > buffer_len { + return Err(DescriptorValidationError::BufferRangeOutOfBounds); + } + let range = if range == vk::WHOLE_SIZE { + buffer_len - offset + } else { + range + }; + if offset.checked_add(range).is_none_or(|end| end > buffer_len) { + return Err(DescriptorValidationError::BufferRangeOutOfBounds); + } + Ok(range) +} + +fn check_image_facts( + swapchain_backed: bool, + has_view: bool, + has_sampler: bool, + sampler_required: bool, +) -> Result<(), DescriptorValidationError> { + if swapchain_backed { + return Err(DescriptorValidationError::SwapchainImage); + } + if !has_view { + return Err(DescriptorValidationError::MissingImageView); + } + if sampler_required && !has_sampler { + return Err(DescriptorValidationError::MissingImageSampler); + } + Ok(()) +} + +fn is_supported_descriptor_type(kind: vk::DescriptorType) -> bool { + matches!( + kind, + vk::DescriptorType::UNIFORM_BUFFER + | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + | vk::DescriptorType::SAMPLED_IMAGE + | vk::DescriptorType::STORAGE_IMAGE + | vk::DescriptorType::COMBINED_IMAGE_SAMPLER + | vk::DescriptorType::SAMPLER + | vk::DescriptorType::UNIFORM_TEXEL_BUFFER + | vk::DescriptorType::STORAGE_TEXEL_BUFFER + ) +} + +fn descriptor_type_update_after_bind_enabled( + kind: vk::DescriptorType, + features: &PhysicalDeviceFeatures, +) -> bool { + let core12 = &features.core12; + match kind { + vk::DescriptorType::UNIFORM_BUFFER | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC => { + core12.descriptor_binding_uniform_buffer_update_after_bind != vk::FALSE + } + vk::DescriptorType::SAMPLED_IMAGE + | vk::DescriptorType::COMBINED_IMAGE_SAMPLER + | vk::DescriptorType::SAMPLER => { + core12.descriptor_binding_sampled_image_update_after_bind != vk::FALSE + } + vk::DescriptorType::STORAGE_IMAGE => { + core12.descriptor_binding_storage_image_update_after_bind != vk::FALSE + } + vk::DescriptorType::STORAGE_BUFFER | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC => { + core12.descriptor_binding_storage_buffer_update_after_bind != vk::FALSE + } + vk::DescriptorType::UNIFORM_TEXEL_BUFFER => { + core12.descriptor_binding_uniform_texel_buffer_update_after_bind != vk::FALSE + } + vk::DescriptorType::STORAGE_TEXEL_BUFFER => { + core12.descriptor_binding_storage_texel_buffer_update_after_bind != vk::FALSE + } + _ => false, + } +} + +fn descriptor_type_update_after_bind_feature(kind: vk::DescriptorType) -> &'static str { + match kind { + vk::DescriptorType::UNIFORM_BUFFER | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC => { + "descriptorBindingUniformBufferUpdateAfterBind" + } + vk::DescriptorType::SAMPLED_IMAGE + | vk::DescriptorType::COMBINED_IMAGE_SAMPLER + | vk::DescriptorType::SAMPLER => "descriptorBindingSampledImageUpdateAfterBind", + vk::DescriptorType::STORAGE_IMAGE => "descriptorBindingStorageImageUpdateAfterBind", + vk::DescriptorType::STORAGE_BUFFER | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC => { + "descriptorBindingStorageBufferUpdateAfterBind" + } + vk::DescriptorType::UNIFORM_TEXEL_BUFFER => { + "descriptorBindingUniformTexelBufferUpdateAfterBind" + } + vk::DescriptorType::STORAGE_TEXEL_BUFFER => { + "descriptorBindingStorageTexelBufferUpdateAfterBind" + } + _ => "descriptor type update-after-bind feature", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::pipeline::DescriptorSetLayoutBindingDesc; + + fn binding( + binding: u32, + kind: vk::DescriptorType, + count: u32, + ) -> DescriptorSetLayoutBindingDesc { + DescriptorSetLayoutBindingDesc { + binding, + kind, + count, + stage: vk::ShaderStageFlags::FRAGMENT, + flags: None, + } + } + + fn write( + binding: u32, + kind: vk::DescriptorType, + resource: RawDescriptorResourceFacts, + ) -> RawDescriptorWriteFacts { + RawDescriptorWriteFacts { + binding, + array_element: 0, + descriptor_type: kind, + resource, + } + } + + fn validate( + layout: &[DescriptorSetLayoutBindingDesc], + writes: &[RawDescriptorWriteFacts], + ) -> Result { + validate_descriptor_writes(layout, writes, &PhysicalDeviceFeatures::default(), 16, 32) + } + + #[test] + fn validates_supported_buffer_image_sampler_and_texel_writes() { + let layout = [ + binding(0, vk::DescriptorType::UNIFORM_BUFFER, 1), + binding(1, vk::DescriptorType::SAMPLED_IMAGE, 1), + binding(2, vk::DescriptorType::SAMPLER, 1), + binding(3, vk::DescriptorType::UNIFORM_TEXEL_BUFFER, 1), + binding(4, vk::DescriptorType::COMBINED_IMAGE_SAMPLER, 1), + ]; + let writes = [ + write( + 0, + vk::DescriptorType::UNIFORM_BUFFER, + RawDescriptorResourceFacts::Buffer { + buffer_len: 128, + offset: 16, + range: vk::WHOLE_SIZE, + dynamic: false, + }, + ), + write( + 1, + vk::DescriptorType::SAMPLED_IMAGE, + RawDescriptorResourceFacts::Image { + swapchain_backed: false, + has_view: true, + has_sampler: false, + }, + ), + write( + 2, + vk::DescriptorType::SAMPLER, + RawDescriptorResourceFacts::Sampler { has_sampler: true }, + ), + write( + 3, + vk::DescriptorType::UNIFORM_TEXEL_BUFFER, + RawDescriptorResourceFacts::TexelBuffer { + buffer_len: 64, + offset: 0, + range: 32, + }, + ), + write( + 4, + vk::DescriptorType::COMBINED_IMAGE_SAMPLER, + RawDescriptorResourceFacts::Image { + swapchain_backed: false, + has_view: true, + has_sampler: true, + }, + ), + ]; + + let plan = validate(&layout, &writes).unwrap(); + assert_eq!(plan.writes[0].normalized_range, Some(112)); + assert_eq!(plan.writes[1].normalized_range, None); + assert_eq!(plan.writes[2].normalized_range, None); + assert_eq!(plan.writes[3].normalized_range, Some(32)); + assert_eq!(plan.writes[4].normalized_range, None); + } + + #[test] + fn reports_binding_element_type_and_buffer_range_errors() { + let layout = [binding(4, vk::DescriptorType::UNIFORM_BUFFER, 2)]; + let facts = RawDescriptorResourceFacts::Buffer { + buffer_len: 32, + offset: 0, + range: 16, + dynamic: false, + }; + assert_eq!( + validate( + &layout, + &[write(5, vk::DescriptorType::UNIFORM_BUFFER, facts)] + ), + Err(DescriptorValidationError::UnknownBinding(5)) + ); + + let mut out_of_bounds = write(4, vk::DescriptorType::UNIFORM_BUFFER, facts); + out_of_bounds.array_element = 2; + assert_eq!( + validate(&layout, &[out_of_bounds]), + Err(DescriptorValidationError::ArrayElementOutOfBounds { + binding: 4, + element: 2, + count: 2 + }) + ); + assert!(matches!( + validate( + &layout, + &[write(4, vk::DescriptorType::STORAGE_BUFFER, facts)] + ), + Err(DescriptorValidationError::TypeMismatch { .. }) + )); + + let outside = RawDescriptorResourceFacts::Buffer { + buffer_len: 32, + offset: 24, + range: 16, + dynamic: false, + }; + assert_eq!( + validate( + &layout, + &[write(4, vk::DescriptorType::UNIFORM_BUFFER, outside)] + ), + Err(DescriptorValidationError::BufferRangeOutOfBounds) + ); + } + + #[test] + fn rejects_unsupported_and_swapchain_descriptors() { + let input_attachment = [binding(0, vk::DescriptorType::INPUT_ATTACHMENT, 1)]; + assert_eq!( + validate( + &input_attachment, + &[write( + 0, + vk::DescriptorType::INPUT_ATTACHMENT, + RawDescriptorResourceFacts::Sampler { has_sampler: false }, + )] + ), + Err(DescriptorValidationError::UnsupportedDescriptorType( + vk::DescriptorType::INPUT_ATTACHMENT + )) + ); + + let image_layout = [binding(0, vk::DescriptorType::COMBINED_IMAGE_SAMPLER, 1)]; + assert_eq!( + validate( + &image_layout, + &[write( + 0, + vk::DescriptorType::COMBINED_IMAGE_SAMPLER, + RawDescriptorResourceFacts::Image { + swapchain_backed: true, + has_view: true, + has_sampler: true, + }, + )] + ), + Err(DescriptorValidationError::SwapchainImage) + ); + } + + #[test] + fn checks_binding_flags_against_enabled_features_not_adapter_support() { + let layout = [DescriptorSetLayoutBindingDesc { + flags: Some(vk::DescriptorBindingFlags::PARTIALLY_BOUND), + ..binding(7, vk::DescriptorType::SAMPLED_IMAGE, 4) + }]; + let mut enabled = PhysicalDeviceFeatures::default(); + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Err(DescriptorValidationError::FeatureNotEnabled { + binding: 7, + feature: "descriptorBindingPartiallyBound", + }) + ); + + enabled.core12.descriptor_binding_partially_bound = vk::TRUE; + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Ok(()) + ); + } + + #[test] + fn validates_dynamic_buffer_alignment_and_flags() { + let dynamic = [binding(0, vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, 1)]; + let misaligned = RawDescriptorResourceFacts::Buffer { + buffer_len: 128, + offset: 4, + range: 16, + dynamic: true, + }; + assert_eq!( + validate( + &dynamic, + &[write( + 0, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC, + misaligned + )] + ), + Err(DescriptorValidationError::MisalignedDynamicOffset) + ); + } + + #[test] + fn generation_and_binding_map_helpers_track_invalidation_and_replacement() { + assert!(check_pool_generation(3, 3).is_ok()); + assert!(matches!( + check_pool_generation(3, 4), + Err(DescriptorSetError::StaleSet { + allocated_generation: 3, + pool_generation: 4 + }) + )); + + let layout = [ + DescriptorSetLayoutBindingDesc { + flags: Some(vk::DescriptorBindingFlags::PARTIALLY_BOUND), + ..binding(0, vk::DescriptorType::SAMPLED_IMAGE, 3) + }, + binding(1, vk::DescriptorType::SAMPLER, 1), + ]; + let mut entries = std::collections::BTreeMap::new(); + entries.insert((0, 0), "old"); + entries.insert((1, 0), "sampler"); + assert!(validate_binding_completeness(&layout, &entries).is_ok()); + + let replaced = replace_binding_entries(&entries, [((0, 0), "new"), ((0, 2), "image")]); + assert_eq!(replaced.get(&(0, 0)), Some(&"new")); + assert_eq!(replaced.get(&(0, 1)), None); + assert_eq!(replaced.get(&(0, 2)), Some(&"image")); + assert_eq!(entries.get(&(0, 0)), Some(&"old")); + + let incomplete = [binding(2, vk::DescriptorType::SAMPLER, 2)]; + assert!(matches!( + validate_binding_completeness(&incomplete, &entries), + Err(DescriptorSetError::IncompleteBinding { + binding: 2, + element: 0 + }) + )); + + let unchanged = pool_generation_after_reset(4, || Err("reset failed")); + assert_eq!(unchanged, Err(PoolResetError::Reset("reset failed"))); + assert_eq!(pool_generation_after_reset(4, || Ok::<_, ()>(())), Ok(5)); + } + + #[test] + fn clearing_binding_entries_releases_all_tracked_slots_only_for_that_binding() { + let retained = Arc::new(String::from("texture")); + let weak = Arc::downgrade(&retained); + let mut entries = std::collections::BTreeMap::from([ + ((0, 0), retained.clone()), + ((0, 3), retained.clone()), + ((1, 0), Arc::new(String::from("buffer"))), + ]); + drop(retained); + + clear_binding_entries(&mut entries, 0); + + assert_eq!(entries.keys().copied().collect::>(), [(1, 0)]); + assert_eq!(weak.strong_count(), 0); + } + + #[test] + fn binding_map_replacement_retains_arc_owners() { + let owner = Arc::new(String::from("retained")); + let weak = Arc::downgrade(&owner); + let current = std::collections::BTreeMap::from([((0, 0), owner.clone())]); + drop(owner); + assert_eq!(weak.strong_count(), 1); + + let replacement = + replace_binding_entries(¤t, [((0, 1), Arc::new(String::from("second")))]); + drop(current); + assert_eq!( + weak.upgrade().as_deref().map(String::as_str), + Some("retained") + ); + drop(replacement); + assert_eq!(weak.strong_count(), 0); + } + + #[test] + fn update_after_bind_uses_feature_for_binding_descriptor_type() { + let layout = [DescriptorSetLayoutBindingDesc { + flags: Some(vk::DescriptorBindingFlags::UPDATE_AFTER_BIND), + ..binding(2, vk::DescriptorType::STORAGE_BUFFER, 1) + }]; + let mut enabled = PhysicalDeviceFeatures::default(); + enabled + .core12 + .descriptor_binding_uniform_buffer_update_after_bind = vk::TRUE; + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Err(DescriptorValidationError::FeatureNotEnabled { + binding: 2, + feature: "descriptorBindingStorageBufferUpdateAfterBind", + }) + ); + + enabled + .core12 + .descriptor_binding_storage_buffer_update_after_bind = vk::TRUE; + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Ok(()) + ); + } + + #[test] + fn update_unused_while_pending_requires_type_specific_update_after_bind_feature() { + let layout = [DescriptorSetLayoutBindingDesc { + flags: Some(vk::DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING), + ..binding(3, vk::DescriptorType::STORAGE_IMAGE, 1) + }]; + let mut enabled = PhysicalDeviceFeatures::default(); + enabled + .core12 + .descriptor_binding_update_unused_while_pending = vk::TRUE; + enabled + .core12 + .descriptor_binding_sampled_image_update_after_bind = vk::TRUE; + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Err(DescriptorValidationError::FeatureNotEnabled { + binding: 3, + feature: "descriptorBindingStorageImageUpdateAfterBind", + }) + ); + + enabled + .core12 + .descriptor_binding_storage_image_update_after_bind = vk::TRUE; + assert_eq!( + validate_descriptor_binding_features(&layout, &enabled), + Ok(()) + ); + } + + #[test] + fn buffer_is_arc_compatible() { + fn assert_send_sync() {} + assert_send_sync::(); + } +} diff --git a/crates/renderer_resources/src/images.rs b/crates/renderer_resources/src/images.rs index 8acb9de..2c6bb69 100644 --- a/crates/renderer_resources/src/images.rs +++ b/crates/renderer_resources/src/images.rs @@ -133,19 +133,29 @@ impl Default for ImageDesc { enum ImageInner { Swapchain(vk::Image, Device), Allocated(DeviceObject, Allocation), + #[cfg(feature = "testing")] + Testing(vk::Image), } impl ImageInner { + fn is_swapchain_backed(&self) -> bool { + matches!(self, Self::Swapchain(_, _)) + } + fn image(&self) -> vk::Image { match self { Self::Swapchain(image, _) => *image, Self::Allocated(image, _) => **image, + #[cfg(feature = "testing")] + Self::Testing(image) => *image, } } fn device(&self) -> &Device { match self { Self::Swapchain(_, device) => device, Self::Allocated(image, _) => image.device(), + #[cfg(feature = "testing")] + Self::Testing(_) => panic!("test image has no Vulkan device"), } } @@ -153,12 +163,16 @@ impl ImageInner { match self { Self::Swapchain(_, _) => None, Self::Allocated(_, alloc) => Some(alloc), + #[cfg(feature = "testing")] + Self::Testing(_) => None, } } fn allocation_mut(&mut self) -> Option<&mut Allocation> { match self { Self::Swapchain(_, _) => None, Self::Allocated(_, alloc) => Some(alloc), + #[cfg(feature = "testing")] + Self::Testing(_) => None, } } } @@ -279,6 +293,20 @@ impl Image { } } + /// Creates a non-owning image fixture for device-free integration tests. + /// + /// This constructor is only available with the crate's `testing` feature. The raw handle is + /// never destroyed and must not be used for Vulkan calls. + #[cfg(feature = "testing")] + #[doc(hidden)] + pub fn for_testing(raw: vk::Image, desc: ImageDesc) -> Self { + Self { + image: ImageInner::Testing(raw), + desc, + views: Default::default(), + } + } + fn new_raw( device: Device, desc: &ImageDesc, @@ -391,6 +419,11 @@ impl Image { self.image.allocation() } + /// Whether this image wraps a swapchain-owned Vulkan image. + pub fn is_swapchain_backed(&self) -> bool { + self.image.is_swapchain_backed() + } + pub fn allocation_mut(&mut self) -> Option<&mut Allocation> { self.image.allocation_mut() } @@ -481,20 +514,17 @@ impl Image { .view_type(desc.kind) .format(desc.format) .components(desc.components) - .subresource_range( - vk::ImageSubresourceRange::default() - .aspect_mask(desc.aspect) - .base_mip_level(desc.mip_range.start) - .level_count(desc.mip_range.len()) - .base_array_layer(desc.layer_range.start) - .layer_count(desc.layer_range.len()), - ); + .subresource_range(image_view_subresource_range(&desc)); let device = self.image.device(); let view = unsafe { device.raw.create_image_view(&create_info, None)? }; Ok(ManuallyDrop::new(ImageView { - view: DeviceObject::new_debug_named(device.clone(), view, desc.name.clone()), + view: ImageViewHandle::Owned(DeviceObject::new_debug_named( + device.clone(), + view, + desc.name.clone(), + )), desc, image: self.clone(), })) @@ -992,19 +1022,92 @@ impl PartialEq for ImageViewDesc { #[derive(Debug)] pub struct ImageView { - view: DeviceObject, + view: ImageViewHandle, #[allow(dead_code)] desc: ImageViewDesc, image: Arc, } +#[derive(Debug)] +enum ImageViewHandle { + Owned(DeviceObject), + #[cfg(feature = "testing")] + Testing(vk::ImageView), +} + impl ImageView { pub fn raw(&self) -> vk::ImageView { - *self.view + match &self.view { + ImageViewHandle::Owned(view) => **view, + #[cfg(feature = "testing")] + ImageViewHandle::Testing(view) => *view, + } } pub fn image(&self) -> &Arc { &self.image } + + /// Returns the concrete subresource range used to create this view. + pub fn subresource_range(&self) -> vk::ImageSubresourceRange { + image_view_subresource_range(&self.desc) + } + + /// Creates a non-owning image-view fixture for device-free integration tests. + /// + /// This constructor is only available with the crate's `testing` feature. The raw handle is + /// never destroyed and must not be used for Vulkan calls. + #[cfg(feature = "testing")] + #[doc(hidden)] + pub fn for_testing( + raw: vk::ImageView, + image: Arc, + range: vk::ImageSubresourceRange, + ) -> Self { + let mut desc = ImageViewDesc::color_2d(); + desc.aspect = range.aspect_mask; + desc.mip_range = MipRange::new( + range.base_mip_level, + range.base_mip_level + range.level_count, + ); + desc.layer_range = MipRange::new( + range.base_array_layer, + range.base_array_layer + range.layer_count, + ); + Self { + view: ImageViewHandle::Testing(raw), + desc, + image, + } + } +} + +fn image_view_subresource_range(desc: &ImageViewDesc) -> vk::ImageSubresourceRange { + vk::ImageSubresourceRange::default() + .aspect_mask(desc.aspect) + .base_mip_level(desc.mip_range.start) + .level_count(desc.mip_range.len()) + .base_array_layer(desc.layer_range.start) + .layer_count(desc.layer_range.len()) +} + +#[cfg(test)] +mod descriptor_range_tests { + use super::*; + + #[test] + fn normalized_range_accessor_resolves_remaining_sentinels() { + let mut desc = ImageViewDesc::color_2d() + .with_mip_range(1..) + .with_layer_range(2..); + desc.mip_range.set_max_end(6); + desc.layer_range.set_max_end(9); + let range = image_view_subresource_range(&desc); + assert_eq!(range.aspect_mask, vk::ImageAspectFlags::COLOR); + assert_eq!(range.base_mip_level, 1); + assert_eq!(range.level_count, 5); + assert_eq!(range.base_array_layer, 2); + assert_eq!(range.layer_count, 7); + } } pub struct QueueOwnership { diff --git a/crates/renderer_resources/src/lib.rs b/crates/renderer_resources/src/lib.rs index 2518d7b..e9dd614 100644 --- a/crates/renderer_resources/src/lib.rs +++ b/crates/renderer_resources/src/lib.rs @@ -1,6 +1,7 @@ pub use renderer_device::{Device, Error, Result, device, make_extension}; pub mod buffers; +pub mod descriptors; pub mod images; pub mod pipeline; pub mod util; diff --git a/crates/renderer_resources/src/pipeline.rs b/crates/renderer_resources/src/pipeline.rs index 3f3293c..a0586e4 100644 --- a/crates/renderer_resources/src/pipeline.rs +++ b/crates/renderer_resources/src/pipeline.rs @@ -1,12 +1,18 @@ -use std::{borrow::Cow, path::Path, sync::Arc}; +use std::{ + borrow::Cow, + collections::BTreeMap, + path::Path, + sync::{ + Arc, + atomic::{AtomicU64, Ordering}, + }, +}; use ash::{ext, prelude::*, vk}; use parking_lot::Mutex; use crate::{ - device::{ - Device, DeviceObject, - }, + device::{Device, DeviceObject}, make_extension, }; @@ -29,7 +35,7 @@ impl ShaderStageDesc<'_> { } } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone, Copy)] pub struct DescriptorSetLayoutBindingDesc { pub binding: u32, pub count: u32, @@ -229,10 +235,21 @@ pub struct DescriptorSetAllocDesc<'a> { pub layout: &'a DescriptorSetLayout, } +/// Descriptor pool wrapper. +/// +/// Pool reset and final destruction require caller synchronization with all GPU work and every +/// managed-set use. Managed sets retain the pool allocation object so dropping this wrapper alone +/// does not destroy a pool that still has live managed wrappers. #[derive(Debug)] pub struct DescriptorPool { + inner: Arc, +} + +#[derive(Debug)] +struct DescriptorPoolInner { pool: DeviceObject, lock: Mutex<()>, + generation: AtomicU64, } impl DescriptorPool { @@ -245,54 +262,855 @@ impl DescriptorPool { let handle = unsafe { device.dev().create_descriptor_pool(info, None)? }; Ok(Self { - pool: DeviceObject::new_debug_named(device, handle, desc.name), - lock: Mutex::new(()), + inner: Arc::new(DescriptorPoolInner { + pool: DeviceObject::new_debug_named(device, handle, desc.name), + lock: Mutex::new(()), + generation: AtomicU64::new(0), + }), }) } - pub fn allocate(&self, descs: &[DescriptorSetAllocDesc]) -> VkResult> { + /// Allocates raw sets without retaining layouts or bound resources. + /// + /// These handles are untracked and cannot be transitively imported as managed sets. + pub fn allocate_untracked( + &self, + descs: &[DescriptorSetAllocDesc], + ) -> VkResult> { let layouts = descs .iter() .map(|desc| desc.layout.raw()) .collect::>(); let info = &vk::DescriptorSetAllocateInfo::default() - .descriptor_pool(*self.pool) + .descriptor_pool(*self.inner.pool) .set_layouts(&layouts); let sets = unsafe { - let _lock = self.lock.lock(); - self.pool.device().raw.allocate_descriptor_sets(info)? + let _lock = self.inner.lock.lock(); + self.inner + .pool + .device() + .raw + .allocate_descriptor_sets(info)? }; for (&set, desc) in sets.iter().zip(descs) { if let Some(name) = desc.name.as_ref() { - unsafe { self.pool.device().debug_name_object(set, name) }; + unsafe { self.inner.pool.device().debug_name_object(set, name) }; } } Ok(sets) } - #[allow(dead_code)] - pub fn reset(&self) -> VkResult<()> { - let _lock = self.lock.lock(); - unsafe { - self.pool - .device() - .raw - .reset_descriptor_pool(*self.pool, vk::DescriptorPoolResetFlags::empty()) + /// Backwards-compatible raw allocation alias for [`Self::allocate_untracked`]. + /// New code should prefer the explicitly untracked name. + pub fn allocate(&self, descs: &[DescriptorSetAllocDesc]) -> VkResult> { + self.allocate_untracked(descs) + } + + /// Allocates a managed descriptor set retaining its pool lifetime and layout facts. + pub fn allocate_managed( + &self, + desc: DescriptorSetAllocDesc<'_>, + ) -> crate::descriptors::DescriptorSetResult { + crate::descriptors::validate_descriptor_binding_features( + desc.layout.bindings(), + self.inner.pool.device().enabled_features(), + )?; + + let layouts = [desc.layout.raw()]; + let info = vk::DescriptorSetAllocateInfo::default() + .descriptor_pool(*self.inner.pool) + .set_layouts(&layouts); + let (raw, generation) = { + let lock = self.inner.lock.lock(); + let raw = unsafe { self.inner.pool.device().raw.allocate_descriptor_sets(&info) } + .map_err(crate::Error::from)?[0]; + let generation = self.inner.generation.load(Ordering::Acquire); + drop(lock); + (raw, generation) + }; + if let Some(name) = desc.name.as_ref() { + unsafe { self.inner.pool.device().debug_name_object(raw, name) }; } + Ok(DescriptorSet { + pool: Some(self.inner.clone()), + raw, + generation, + test_pool_generation: None, + uniform_alignment: self + .inner + .pool + .device() + .properties() + .core + .limits + .min_uniform_buffer_offset_alignment, + storage_alignment: self + .inner + .pool + .device() + .properties() + .core + .limits + .min_storage_buffer_offset_alignment, + layout: desc.layout.bindings().to_vec(), + bindings: BTreeMap::new(), + }) + } + + /// Resets the pool and invalidates all managed sets allocated from it. + /// + /// # Safety + /// The caller must ensure every GPU submission that could access any set or retained owner + /// from this pool has completed, prevent new submissions and descriptor updates, and exclude + /// all concurrent use of managed sets until reset finishes. This includes allocation, writer + /// updates/commit, graph recording/import, and command-buffer recording/binding. Dropping set + /// wrappers or holding the pool mutex does not establish GPU retirement or exclude all users. + /// Generation checks detect stale wrappers but do not make concurrent reset/use safe. + pub unsafe fn reset(&self) -> crate::descriptors::DescriptorSetResult<()> { + let _lock = self.inner.lock.lock(); + let generation = self.inner.generation.load(Ordering::Acquire); + let next_generation = + crate::descriptors::pool_generation_after_reset(generation, || unsafe { + self.inner + .pool + .device() + .raw + .reset_descriptor_pool(*self.inner.pool, vk::DescriptorPoolResetFlags::empty()) + }) + .map_err(|error| match error { + crate::descriptors::PoolResetError::Reset(error) => { + crate::descriptors::DescriptorSetError::Vulkan(error.into()) + } + crate::descriptors::PoolResetError::GenerationExhausted => { + crate::descriptors::DescriptorSetError::PoolGenerationExhausted + } + })?; + self.inner + .generation + .store(next_generation, Ordering::Release); + Ok(()) + } + + /// Explicitly destroys the pool when no managed-set wrapper still owns it. + /// + /// # Safety + /// The caller must satisfy the same GPU-completion and concurrent-use exclusion requirements + /// as [`DescriptorPool::reset`]. Dropping a pool also requires those conditions when its final + /// owner is dropped. + pub unsafe fn destroy(self) -> crate::descriptors::DescriptorSetResult<()> { + if Arc::strong_count(&self.inner) != 1 { + return Err(crate::descriptors::DescriptorSetError::PoolStillRetained); + } + drop(self); + Ok(()) + } +} + +/// A managed descriptor set. It retains its pool, layout metadata, and all written owners. +#[derive(Debug)] +pub struct DescriptorSet { + pool: Option>, + raw: vk::DescriptorSet, + generation: u64, + test_pool_generation: Option>, + uniform_alignment: u64, + storage_alignment: u64, + layout: Vec, + bindings: BTreeMap<(u32, u32), crate::descriptors::TrackedDescriptorWrite>, +} + +impl DescriptorSet { + fn validate_generation(&self) -> crate::descriptors::DescriptorSetResult<()> { + let pool_generation = self + .pool + .as_ref() + .map(|pool| pool.generation.load(Ordering::Acquire)) + .or_else(|| { + self.test_pool_generation + .as_ref() + .map(|generation| generation.load(Ordering::Acquire)) + }) + .unwrap_or(self.generation); + crate::descriptors::check_pool_generation(self.generation, pool_generation) + } + + /// Returns the raw handle if this wrapper has not been invalidated by pool reset. + /// The returned handle escapes generation checking; its caller must uphold pool synchronization. + pub fn raw(&self) -> crate::descriptors::DescriptorSetResult { + self.validate_generation()?; + Ok(self.raw) + } + + /// Returns retained resource bindings after checking the pool generation. + pub fn bindings( + &self, + ) -> crate::descriptors::DescriptorSetResult< + impl Iterator, + > { + self.validate_generation()?; + Ok(self.bindings.values()) + } + + /// Returns the retained layout facts after checking this set's pool generation. + pub fn layout_bindings( + &self, + ) -> crate::descriptors::DescriptorSetResult<&[DescriptorSetLayoutBindingDesc]> { + self.validate_generation()?; + Ok(&self.layout) + } + + /// Returns the device alignment required for a dynamic descriptor type. + pub fn dynamic_offset_alignment( + &self, + descriptor_type: vk::DescriptorType, + ) -> crate::descriptors::DescriptorSetResult { + self.validate_generation()?; + match descriptor_type { + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC => Ok(self.uniform_alignment), + vk::DescriptorType::STORAGE_BUFFER_DYNAMIC => Ok(self.storage_alignment), + _ => Err( + crate::descriptors::DescriptorValidationError::UnsupportedDescriptorType( + descriptor_type, + ) + .into(), + ), + } + } + + /// Creates a staged writer after validating that this set is current. + pub fn writer(&mut self) -> crate::descriptors::DescriptorSetResult> { + self.validate_generation()?; + Ok(DescriptorWriter { + set: self, + staged: Vec::new(), + cleared_bindings: Vec::new(), + }) + } + + /// Builds a non-owning managed-set fixture for device-free integration tests. + /// + /// This is available only with the resource crate's `testing` feature. The raw descriptor + /// handle is never owned or validated by Vulkan. `generation` is the generation captured by + /// the wrapper; `pool_generation` is the simulated current pool generation used to exercise + /// stale-set validation. + #[cfg(feature = "testing")] + #[doc(hidden)] + pub fn for_testing( + raw: vk::DescriptorSet, + layout: Vec, + generation: u64, + pool_generation: u64, + bindings: Vec, + ) -> Self { + Self { + pool: None, + raw, + generation, + test_pool_generation: Some(Arc::new(AtomicU64::new(pool_generation))), + uniform_alignment: 1, + storage_alignment: 1, + layout, + bindings: bindings + .into_iter() + .map(|entry| ((entry.binding, entry.array_element), entry)) + .collect(), + } + } +} + +/// A buffer descriptor input. The selected descriptor type comes from the set layout. +#[derive(Debug, Clone, Copy)] +pub struct BufferDescriptor<'a> { + pub buffer: &'a Arc, + pub offset: u64, + pub range: u64, +} + +/// An image descriptor input. Combined-image-sampler bindings require `sampler`. +#[derive(Debug, Clone, Copy)] +pub struct ImageDescriptor<'a> { + pub view: &'a Arc, + pub sampler: Option<&'a Arc>, + pub layout: vk::ImageLayout, +} + +#[derive(Debug)] +enum PendingDescriptorResource { + Buffer(vk::DescriptorBufferInfo), + Image(vk::DescriptorImageInfo), + Texel(vk::BufferView), +} + +#[derive(Debug)] +struct PendingDescriptorWrite { + tracked: crate::descriptors::TrackedDescriptorWrite, + facts: crate::descriptors::RawDescriptorWriteFacts, + raw: PendingDescriptorResource, +} + +/// Stages validated descriptor writes for an existing managed set. +/// +/// Replacing owners is externally synchronized by the caller against GPU work: do not commit +/// while submitted work may still use the descriptor set's previous bindings. Vulkan's +/// update-after-bind flags do not remove this owner-lifetime/pending-use requirement. +pub struct DescriptorWriter<'a> { + set: &'a mut DescriptorSet, + staged: Vec, + cleared_bindings: Vec, +} + +impl DescriptorWriter<'_> { + /// Removes all tracked owners from a partially-bound binding before staging its new contents. + /// + /// Vulkan has no descriptor-set "unset" operation. Cleared elements remain physically + /// unchanged in the Vulkan set and must not be dynamically accessed; this is valid for + /// `PARTIALLY_BOUND` bindings. The operation removes their managed ownership metadata. If + /// stale raw descriptor contents must not remain, overwrite those elements with valid + /// descriptors (for example, egui fills unused array entries with its first live texture). + pub fn clear_binding( + &mut self, + binding: u32, + ) -> crate::descriptors::DescriptorSetResult<&mut Self> { + self.set.validate_generation()?; + let desc = self.descriptor_type(binding)?; + if !desc + .flags + .unwrap_or_default() + .contains(vk::DescriptorBindingFlags::PARTIALLY_BOUND) + { + return Err( + crate::descriptors::DescriptorValidationError::BindingNotPartiallyBound(binding) + .into(), + ); + } + if !self.cleared_bindings.contains(&binding) { + self.cleared_bindings.push(binding); + } + Ok(self) + } + + fn validate_staged(&self) -> crate::descriptors::DescriptorSetResult<()> { + self.set.validate_generation()?; + + let facts = self + .staged + .iter() + .map(|write| write.facts) + .collect::>(); + + let device = &self + .set + .pool + .as_ref() + .expect("descriptor writer requires a Vulkan-backed set") + .pool + .device(); + + crate::descriptors::validate_descriptor_writes( + &self.set.layout, + &facts, + device.enabled_features(), + device + .properties() + .core + .limits + .min_uniform_buffer_offset_alignment, + device + .properties() + .core + .limits + .min_storage_buffer_offset_alignment, + )?; + Ok(()) + } + + fn descriptor_type( + &self, + binding: u32, + ) -> crate::descriptors::DescriptorSetResult { + self.set + .layout + .iter() + .find(|desc| desc.binding == binding) + .copied() + .ok_or(crate::descriptors::DescriptorValidationError::UnknownBinding(binding).into()) + } + + pub fn write_buffer( + &mut self, + binding: u32, + first_element: u32, + values: &[BufferDescriptor<'_>], + ) -> crate::descriptors::DescriptorSetResult<&mut Self> { + self.set.validate_generation()?; + let staged_len = self.staged.len(); + let desc = self.descriptor_type(binding)?; + if !matches!( + desc.kind, + vk::DescriptorType::UNIFORM_BUFFER + | vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC + | vk::DescriptorType::STORAGE_BUFFER + | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + ) { + return Err( + crate::descriptors::DescriptorValidationError::ResourceTypeMismatch(desc.kind) + .into(), + ); + } + if values + .len() + .checked_sub(1) + .and_then(|last| u32::try_from(last).ok()) + .and_then(|last| first_element.checked_add(last)) + .is_none() + && !values.is_empty() + { + return Err( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + } + .into(), + ); + } + let dynamic = matches!( + desc.kind, + vk::DescriptorType::UNIFORM_BUFFER_DYNAMIC | vk::DescriptorType::STORAGE_BUFFER_DYNAMIC + ); + for (offset, value) in values.iter().enumerate() { + let array_element = first_element.checked_add(offset as u32).ok_or( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + }, + )?; + let buffer_info = vk::DescriptorBufferInfo::default() + .buffer(value.buffer.raw()) + .offset(value.offset) + .range(value.range); + self.staged.push(PendingDescriptorWrite { + tracked: crate::descriptors::TrackedDescriptorWrite { + binding, + array_element, + descriptor_type: desc.kind, + stages: desc.stage, + resource: crate::descriptors::DescriptorResourceBinding::Buffer { + buffer: value.buffer.clone(), + offset: value.offset, + range: value.range, + dynamic, + }, + }, + facts: crate::descriptors::RawDescriptorWriteFacts { + binding, + array_element, + descriptor_type: desc.kind, + resource: crate::descriptors::RawDescriptorResourceFacts::Buffer { + buffer_len: value.buffer.len(), + offset: value.offset, + range: value.range, + dynamic, + }, + }, + raw: PendingDescriptorResource::Buffer(buffer_info), + }); + } + if let Err(error) = self.validate_staged() { + self.staged.truncate(staged_len); + return Err(error); + } + Ok(self) + } + + pub fn write_image( + &mut self, + binding: u32, + first_element: u32, + values: &[ImageDescriptor<'_>], + ) -> crate::descriptors::DescriptorSetResult<&mut Self> { + self.set.validate_generation()?; + let staged_len = self.staged.len(); + let desc = self.descriptor_type(binding)?; + if !matches!( + desc.kind, + vk::DescriptorType::SAMPLED_IMAGE + | vk::DescriptorType::STORAGE_IMAGE + | vk::DescriptorType::COMBINED_IMAGE_SAMPLER + ) { + return Err( + crate::descriptors::DescriptorValidationError::ResourceTypeMismatch(desc.kind) + .into(), + ); + } + if values + .len() + .checked_sub(1) + .and_then(|last| u32::try_from(last).ok()) + .and_then(|last| first_element.checked_add(last)) + .is_none() + && !values.is_empty() + { + return Err( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + } + .into(), + ); + } + for (offset, value) in values.iter().enumerate() { + let array_element = first_element.checked_add(offset as u32).ok_or( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + }, + )?; + let sampler = if desc.kind == vk::DescriptorType::COMBINED_IMAGE_SAMPLER { + value.sampler.cloned() + } else { + None + }; + let image_info = vk::DescriptorImageInfo::default() + .sampler( + sampler + .as_ref() + .map_or(vk::Sampler::null(), |sampler| sampler.raw()), + ) + .image_view(value.view.raw()) + .image_layout(value.layout); + self.staged.push(PendingDescriptorWrite { + tracked: crate::descriptors::TrackedDescriptorWrite { + binding, + array_element, + descriptor_type: desc.kind, + stages: desc.stage, + resource: crate::descriptors::DescriptorResourceBinding::Image { + view: value.view.clone(), + sampler, + range: value.view.subresource_range(), + layout: value.layout, + }, + }, + facts: crate::descriptors::RawDescriptorWriteFacts { + binding, + array_element, + descriptor_type: desc.kind, + resource: crate::descriptors::RawDescriptorResourceFacts::Image { + swapchain_backed: value.view.image().is_swapchain_backed(), + has_view: true, + has_sampler: value.sampler.is_some(), + }, + }, + raw: PendingDescriptorResource::Image(image_info), + }); + } + if let Err(error) = self.validate_staged() { + self.staged.truncate(staged_len); + return Err(error); + } + Ok(self) + } + + pub fn write_sampler( + &mut self, + binding: u32, + first_element: u32, + values: &[Arc], + ) -> crate::descriptors::DescriptorSetResult<&mut Self> { + self.set.validate_generation()?; + let staged_len = self.staged.len(); + let desc = self.descriptor_type(binding)?; + if desc.kind != vk::DescriptorType::SAMPLER { + return Err( + crate::descriptors::DescriptorValidationError::ResourceTypeMismatch(desc.kind) + .into(), + ); + } + if values + .len() + .checked_sub(1) + .and_then(|last| u32::try_from(last).ok()) + .and_then(|last| first_element.checked_add(last)) + .is_none() + && !values.is_empty() + { + return Err( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + } + .into(), + ); + } + for (offset, sampler) in values.iter().enumerate() { + let array_element = first_element.checked_add(offset as u32).ok_or( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + }, + )?; + self.staged.push(PendingDescriptorWrite { + tracked: crate::descriptors::TrackedDescriptorWrite { + binding, + array_element, + descriptor_type: desc.kind, + stages: desc.stage, + resource: crate::descriptors::DescriptorResourceBinding::Sampler { + sampler: sampler.clone(), + }, + }, + facts: crate::descriptors::RawDescriptorWriteFacts { + binding, + array_element, + descriptor_type: desc.kind, + resource: crate::descriptors::RawDescriptorResourceFacts::Sampler { + has_sampler: true, + }, + }, + raw: PendingDescriptorResource::Image( + vk::DescriptorImageInfo::default().sampler(sampler.raw()), + ), + }); + } + if let Err(error) = self.validate_staged() { + self.staged.truncate(staged_len); + return Err(error); + } + Ok(self) + } + + pub fn write_texel_buffer( + &mut self, + binding: u32, + first_element: u32, + values: &[Arc], + ) -> crate::descriptors::DescriptorSetResult<&mut Self> { + self.set.validate_generation()?; + let staged_len = self.staged.len(); + let desc = self.descriptor_type(binding)?; + if !matches!( + desc.kind, + vk::DescriptorType::UNIFORM_TEXEL_BUFFER | vk::DescriptorType::STORAGE_TEXEL_BUFFER + ) { + return Err( + crate::descriptors::DescriptorValidationError::ResourceTypeMismatch(desc.kind) + .into(), + ); + } + if values + .len() + .checked_sub(1) + .and_then(|last| u32::try_from(last).ok()) + .and_then(|last| first_element.checked_add(last)) + .is_none() + && !values.is_empty() + { + return Err( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + } + .into(), + ); + } + for (offset, view) in values.iter().enumerate() { + let array_element = first_element.checked_add(offset as u32).ok_or( + crate::descriptors::DescriptorValidationError::ArrayElementOutOfBounds { + binding, + element: u32::MAX, + count: desc.count, + }, + )?; + let range = view.range(); + self.staged.push(PendingDescriptorWrite { + tracked: crate::descriptors::TrackedDescriptorWrite { + binding, + array_element, + descriptor_type: desc.kind, + stages: desc.stage, + resource: crate::descriptors::DescriptorResourceBinding::TexelBuffer { + view: view.clone(), + range, + }, + }, + facts: crate::descriptors::RawDescriptorWriteFacts { + binding, + array_element, + descriptor_type: desc.kind, + resource: crate::descriptors::RawDescriptorResourceFacts::TexelBuffer { + buffer_len: view.buffer().len(), + offset: range.offset, + range: range.size, + }, + }, + raw: PendingDescriptorResource::Texel(view.raw()), + }); + } + if let Err(error) = self.validate_staged() { + self.staged.truncate(staged_len); + return Err(error); + } + Ok(self) + } + + /// Applies all staged writes in one Vulkan update operation, then replaces retained owners. + /// The caller must ensure no pending GPU use depends on owners being replaced; see + /// [`DescriptorWriter`] for the synchronization contract. + pub fn commit(self) -> crate::descriptors::DescriptorSetResult<()> { + self.set.validate_generation()?; + let facts = self + .staged + .iter() + .map(|write| write.facts) + .collect::>(); + let device = &self + .set + .pool + .as_ref() + .expect("descriptor writer requires a Vulkan-backed set") + .pool + .device(); + let plan = crate::descriptors::validate_descriptor_writes( + &self.set.layout, + &facts, + device.enabled_features(), + device + .properties() + .core + .limits + .min_uniform_buffer_offset_alignment, + device + .properties() + .core + .limits + .min_storage_buffer_offset_alignment, + )?; + + let mut normalized_staged = self.staged; + for (write, validated) in normalized_staged.iter_mut().zip(plan.writes) { + if let Some(range) = validated.normalized_range { + match &mut write.tracked.resource { + crate::descriptors::DescriptorResourceBinding::Buffer { + range: retained, + .. + } => { + *retained = range; + } + crate::descriptors::DescriptorResourceBinding::TexelBuffer { + range: retained, + .. + } => { + retained.size = range; + } + _ => {} + } + if let PendingDescriptorResource::Buffer(info) = &mut write.raw { + info.range = range; + } + } + } + let mut current_bindings = self.set.bindings.clone(); + for binding in &self.cleared_bindings { + crate::descriptors::clear_binding_entries(&mut current_bindings, *binding); + } + let replacements = crate::descriptors::replace_binding_entries( + ¤t_bindings, + normalized_staged.iter().map(|write| { + ( + (write.tracked.binding, write.tracked.array_element), + write.tracked.clone(), + ) + }), + ); + crate::descriptors::validate_binding_completeness(&self.set.layout, &replacements)?; + + let writes = normalized_staged + .iter() + .map(|pending| match &pending.raw { + PendingDescriptorResource::Buffer(info) => vk::WriteDescriptorSet::default() + .dst_set(self.set.raw) + .dst_binding(pending.tracked.binding) + .dst_array_element(pending.tracked.array_element) + .descriptor_type(pending.tracked.descriptor_type) + .buffer_info(std::slice::from_ref(info)), + PendingDescriptorResource::Image(info) => vk::WriteDescriptorSet::default() + .dst_set(self.set.raw) + .dst_binding(pending.tracked.binding) + .dst_array_element(pending.tracked.array_element) + .descriptor_type(pending.tracked.descriptor_type) + .image_info(std::slice::from_ref(info)), + PendingDescriptorResource::Texel(view) => vk::WriteDescriptorSet::default() + .dst_set(self.set.raw) + .dst_binding(pending.tracked.binding) + .dst_array_element(pending.tracked.array_element) + .descriptor_type(pending.tracked.descriptor_type) + .texel_buffer_view(std::slice::from_ref(view)), + }) + .collect::>(); + + // Vulkan exposes no result for this command; update all entries as one externally + // synchronized operation and only then publish their replacement owners in the map. + unsafe { device.raw.update_descriptor_sets(&writes, &[]) }; + self.set.bindings = replacements; + Ok(()) + } +} + +#[cfg(test)] +mod descriptor_writer_tests { + use super::*; + + #[test] + fn clearing_partially_bound_slots_removes_managed_entries_but_preserves_other_bindings() { + let entries = BTreeMap::from([ + ((0, 0), "first texture"), + ((0, 7), "stale texture"), + ((1, 0), "uniform buffer"), + ]); + let mut retained = entries.clone(); + crate::descriptors::clear_binding_entries(&mut retained, 0); + + assert_eq!(retained, BTreeMap::from([((1, 0), "uniform buffer")])); + assert_eq!(entries.len(), 3); } } #[derive(Debug)] pub struct DescriptorSetLayout { layout: DeviceObject, + bindings: Vec, } impl DescriptorSetLayout { pub fn new(device: Device, desc: DescriptorSetLayoutDesc) -> crate::Result { + crate::descriptors::validate_descriptor_binding_features( + desc.bindings, + device.enabled_features(), + ) + .map_err(|error| { + let message = match error { + crate::descriptors::DescriptorValidationError::UnsupportedDescriptorType(_) => { + "descriptor layout contains an unsupported descriptor type" + } + crate::descriptors::DescriptorValidationError::FeatureNotEnabled { .. } => { + "descriptor layout requests a feature not enabled on the device" + } + _ => "descriptor layout contains invalid binding metadata", + }; + crate::Error::Todo(message) + })?; + let (flags, bindings): (Vec<_>, Vec<_>) = desc .bindings .iter() @@ -327,12 +1145,54 @@ impl DescriptorSetLayout { Ok(Self { layout: DeviceObject::new_debug_named(device, layout, desc.name), + bindings: retained_binding_descriptions(desc.bindings), }) } pub fn raw(&self) -> vk::DescriptorSetLayout { *self.layout } + + /// Retained binding number, type, count, shader stages, and binding flags. + pub fn bindings(&self) -> &[DescriptorSetLayoutBindingDesc] { + &self.bindings + } +} + +fn retained_binding_descriptions( + bindings: &[DescriptorSetLayoutBindingDesc], +) -> Vec { + bindings.to_vec() +} + +#[cfg(test)] +mod descriptor_layout_tests { + use super::*; + + #[test] + fn retained_binding_descriptions_preserve_all_layout_facts() { + let bindings = [DescriptorSetLayoutBindingDesc { + binding: 3, + count: 5, + kind: vk::DescriptorType::STORAGE_BUFFER_DYNAMIC, + stage: vk::ShaderStageFlags::COMPUTE | vk::ShaderStageFlags::FRAGMENT, + flags: Some(vk::DescriptorBindingFlags::PARTIALLY_BOUND), + }]; + + let retained = retained_binding_descriptions(&bindings); + assert_eq!(retained.len(), 1); + assert_eq!(retained[0].binding, 3); + assert_eq!(retained[0].count, 5); + assert_eq!(retained[0].kind, vk::DescriptorType::STORAGE_BUFFER_DYNAMIC); + assert_eq!( + retained[0].stage, + vk::ShaderStageFlags::COMPUTE | vk::ShaderStageFlags::FRAGMENT + ); + assert_eq!( + retained[0].flags, + Some(vk::DescriptorBindingFlags::PARTIALLY_BOUND) + ); + } } #[derive(Debug)] @@ -426,7 +1286,14 @@ impl std::hash::Hash for SamplerDesc { #[derive(Debug)] pub struct Sampler { - sampler: DeviceObject, + sampler: SamplerHandle, +} + +#[derive(Debug)] +enum SamplerHandle { + Owned(DeviceObject), + #[cfg(feature = "testing")] + Testing(vk::Sampler), } impl Sampler { @@ -452,12 +1319,28 @@ impl Sampler { let handle = unsafe { device.dev().create_sampler(info, None)? }; Ok(Self { - sampler: DeviceObject::new(device, handle), + sampler: SamplerHandle::Owned(DeviceObject::new(device, handle)), }) } pub fn raw(&self) -> vk::Sampler { - *self.sampler + match &self.sampler { + SamplerHandle::Owned(sampler) => **sampler, + #[cfg(feature = "testing")] + SamplerHandle::Testing(sampler) => *sampler, + } + } + + /// Creates a non-owning sampler fixture for device-free integration tests. + /// + /// This constructor is only available with the resource crate's `testing` feature. The raw + /// sampler handle is never destroyed and must not be used for Vulkan calls. + #[cfg(feature = "testing")] + #[doc(hidden)] + pub fn for_testing(raw: vk::Sampler) -> Self { + Self { + sampler: SamplerHandle::Testing(raw), + } } } diff --git a/design-plans/descriptor-set-ownership.md b/design-plans/descriptor-set-ownership.md new file mode 100644 index 0000000..9c2fca0 --- /dev/null +++ b/design-plans/descriptor-set-ownership.md @@ -0,0 +1,375 @@ +# Descriptor Set Ownership & Tracking Plan + +Status: approved direction, not yet implemented +Scope: `crates/renderer_device` (enabled-feature source), `crates/renderer_resources` (primary), `crates/renderer_rendergraph` (consumer), `crates/renderer` (egui migration, separate change) + +## Problem + +`renderer_resources::DescriptorPool::allocate` returns raw `vk::DescriptorSet` handles with no +record of what they bind (`pipeline.rs:233-287`). The render graph's draw packets therefore +require callers to manually declare every descriptor-bound resource with accurate +range/stage/access/layout (`GraphDescriptorSet` in `renderer_rendergraph/src/resources.rs:170-197`) +— manual, error-prone, and unverifiable. Bound resources can also be dropped while a set is +still in use; nothing detects this. + +## Approved direction + +1. **Managed descriptor sets retain owners of the resources and objects they bind** — + `Arc`, `Arc`, `Arc`, and texel-buffer views as appropriate. + `ImageView` retains its `Arc` (`images.rs:994-1008`), but this is not a swapchain + lifetime token (see below). This ownership protects CPU object lifetimes, not submitted GPU + work by itself. +2. **Importing a managed descriptor set into the graph transitively imports its resources.** + Identity uses `Arc::ptr_eq` first and raw-handle equality fallback. The graph deduplicates + resources and merges per-command draw-access scopes; descriptor use never changes an + imported resource's initial state. +3. **Future, out of scope:** graph-created descriptor sets binding graph-created transient + resources via a graph-owned writer built on a reusable validation/write-plan interface in + `renderer_resources`. It has no graph types and avoids forking descriptor validation. +4. The raw escape hatch stays, clearly named untracked, owning nothing and never eligible for + transitive import. +5. `renderer_resources` stays graph-agnostic: metadata is generic Vulkan facts; graph-side + mapping lives in `renderer_rendergraph`. Zero graph-shaped types in `renderer_resources`. +6. Descriptor owner replacement, managed-set/pool destruction, and reset are caller-synchronized + against GPU work for this skeleton. Dropping wrappers is not evidence that GPU use retired. + A later design may replace this contract with explicit retirement tokens/GPU-usage tracking. + Pool reset also invalidates every Vulkan set allocated from that pool, independently of GPU + retirement. Managed wrappers surviving a successful reset must fail validation before + wrapper-mediated import, update, or bind attempts; the generation check detects invalidation, + but cannot revoke raw handles already exposed by `raw()`. Reset must also be externally + synchronized against every managed-set use; generation validation does not make concurrent + reset/use safe. +7. Swapchain-backed images are rejected by managed image writes with an explicit error. A + managed `Arc` does not keep its swapchain alive (`renderer_resources/AGENTS.md:39-47`); + rejecting these images avoids representing a false ownership guarantee. +8. Documented invariant changes must be reflected in `crates/renderer_resources/AGENTS.md` + (it currently says descriptor sets are not wrapped in owning resource objects). Any + `renderer` re-export decisions are flagged separately. + +## Design decisions + +### Buffer Arc model +- Store `Arc` in managed sets; callers wrap buffers after construction. + Do NOT make `Buffer` internally Arc-backed. +- Rationale: least invasive; preserves value-based `Buffer::new`/`map`/`map_mut` + (`buffers.rs:75-89,145-160`) and raw-handle equality (`buffers.rs:81-86`). +- Same pattern as `ImageView` -> `Arc`. + +### Managed set layout and supported descriptor types +- A managed set owns: its pool-allocation lifetime token, raw handle, retained layout facts + (binding number/type/count/stages/flags), and a binding map keyed by + `(binding, array_element)`. +- Supported managed descriptor types are `UNIFORM_BUFFER`, `UNIFORM_BUFFER_DYNAMIC`, + `STORAGE_BUFFER`, `STORAGE_BUFFER_DYNAMIC`, `SAMPLED_IMAGE`, `STORAGE_IMAGE`, + `COMBINED_IMAGE_SAMPLER`, `SAMPLER`, `UNIFORM_TEXEL_BUFFER`, and `STORAGE_TEXEL_BUFFER`. + Input attachments, acceleration structures, inline uniform blocks, and every other type + are explicitly rejected by managed allocation/write validation until separately designed. +- Binding entries retain the corresponding owners: + - uniform/storage buffer (including dynamic): `{ Arc, offset, range }`; + - sampled/storage image and combined image sampler: `{ Arc, optional Arc, + normalized subresource range, layout }` (combined requires a sampler); + - sampler-only: `{ Arc }` with no image/view; + - uniform/storage texel buffer: `{ Arc, texel range }` (or an equivalent + resource-crate-owned buffer-view wrapper that owns its Vulkan view and buffer). +- Swapchain-backed images are rejected for every managed image descriptor kind, including + combined image sampler. The error is returned before updating Vulkan descriptors; callers + needing swapchain images use the raw untracked route and must separately retain the frame / + swapchain token through completion. +- Slots may be empty only for `PARTIALLY_BOUND` bindings (egui needs sparse arrays + + update-after-bind). Writes replace retained entries only after the Vulkan update succeeds. +- Validate descriptor-indexing features for the actual binding flags and descriptor types + before creating a managed layout or set. Layout flags alone are not a capability check. + In particular validate each requested `PARTIALLY_BOUND`, `UPDATE_AFTER_BIND`, + `VARIABLE_DESCRIPTOR_COUNT`, and update-unused-while-pending behavior against enabled device + features and the specific descriptor type's feature bit; reject unsupported requests. This check + must use the enabled-feature snapshot exposed by `renderer_device::Device`, never the adapter's + queried-supported feature data (`DeviceInner::features()` currently returns + `adapter.features`; `renderer_device/src/device.rs:594-596`). `DeviceInner` currently retains + `adapter` and `enabled_extensions` but not enabled features (`device.rs:156-168`), although + `create_logical_device` receives the feature chain used for device creation (`device.rs:398-413`). + Add retained enabled-feature state and an accessor in `renderer_device`; layout creation and + managed write validation in `renderer_resources` must consult that accessor for binding flags and + type-specific requirements (including partially-bound, update-after-bind, runtime arrays, and + update-unused-while-pending). Keep this within the existing dependency direction: + `renderer_resources` consumes the data through its existing `renderer_device` re-exports; add + no dependency edge. + +### Shared validation/write path +- Managed `DescriptorWriter::write_*` produces staged binding entries and raw Vulkan write + infos, then invokes a reusable `renderer_resources` validation/write-plan API. The API accepts + retained layout facts plus raw write infos/resource facts and returns a validated write plan + or structured errors. It checks descriptor type, binding/array bounds, descriptor ranges, + completeness/partial-binding rules, and type-specific owner facts before Vulkan is updated. +- Commit applies the validated Vulkan writes and only then swaps the retained owner entries. + The same graph-agnostic validation/write-plan interface is the future graph-owned writer's + route; it does not take graph types or rely on `write_raw_untracked`. +- `write_raw_untracked` remains a separate Vulkan-only escape hatch. It performs no managed + validation, retains no owners, and cannot be passed off as the shared managed write path. + +### Untracked escape hatch and reset synchronization +- Renamed `allocate_untracked`: returns raw handles, retains no bindings, owns nothing, and + cannot be transitively imported. +- Managed-pool reset and destruction are explicitly externally synchronized operations. Before + reset/destruction, the caller must ensure every GPU submission that could access any set or + descriptor owner from the pool has completed, prevent new submissions and descriptor updates, + and exclude every concurrent managed-set use until reset/destruction is complete. This exclusion + covers allocation from the pool, writer updates/commit, graph recording and descriptor-set + import, and command-buffer recording/binding (as well as other operations that inspect or use a + managed set). This mirrors Vulkan's external-synchronization requirement for pool reset versus + descriptor-set use; it does not impose a stricter guarantee than Vulkan. This is an API safety + contract (prefer an `unsafe` reset entry point with this requirement in its safety docs); + wrapper-drop checks alone are insufficient. +- Every pool owns a monotonically increasing generation counter shared with its managed sets. + Allocation records the current generation in each wrapper. On a successful reset, while holding + the pool mutex and after Vulkan reset succeeds, increment the generation; failed reset leaves it + unchanged. Each managed-set operation that can expose or use the raw handle (`raw`, bindings/ + writer access, write/commit, graph import, and command binding) must compare the recorded + generation with the pool's current generation and return a stale-set error on mismatch. Make + raw-handle access fallible (`raw() -> Result`) rather than exposing an + unchecked handle from a surviving wrapper. Recheck at graph import and command binding so a set + imported before reset is detected as stale afterward. These generation checks are defense-in- + depth for stale wrappers after reset, not a substitute for external synchronization: the pool + mutex is not held across command use, so a check immediately before binding cannot close the + check/use race if reset is concurrent. The generation mechanism detects invalidation of all + managed sets, including wrappers still strongly held by callers; reset does not need to find or + mutate those wrappers. `allocate_untracked` remains outside this managed-wrapper check and its + raw handles remain caller responsibility. +- A handle returned by `raw()` escapes the wrapper's control. Its generation check can reject a + wrapper already stale when `raw()` is called, but cannot eliminate the interval between the + check and use or protect an escaped handle if reset later occurs. Once a raw handle escapes, + the caller owns its validity and must uphold the same external-synchronization contract. +- The existing pool mutex only serializes CPU operations, not in-flight GPU work + (`pipeline.rs:253-285`). Dropping all managed wrappers does not prove GPU retirement. The same + caller synchronization applies to replacing/dropping retained binding owners: the caller must + not release or replace an owner while submitted GPU work may still use that descriptor + version. A future retirement-token/GPU-usage-tracking hook can automate this discipline. +- Raw/untracked reset remains caller-synchronized as well. + +### Dynamic offsets +- Dynamic offsets are bind-time data, not owned resources. The graph packet carries offsets + per descriptor set, and the draw path constructs the single correctly ordered offset slice + required by `cmd_bind_descriptor_sets`: set order, then dynamic binding number and array + element order as specified by the bound pipeline layouts. +- Validate exact offset count, per-type Vulkan alignment, overflow, and effective range bounds + (`descriptor base offset + dynamic offset + range <= buffer size`) before accepting/recording + the draw. Keep base descriptor ranges for ownership and use effective ranges for draw scopes. + +## Transitive import design + +- `RenderGraph::import_descriptor_set(Arc, initial_states, dynamic_offsets) + -> Result`. `initial_states` explicitly supplies the initial + `ResourceState` for each unique resource referenced by the set (or a documented caller + default expanded to each resource); it is never inferred from descriptor layout/access. + The draw packet retains the `Arc`, `raw()`, graph-derived draw accesses, and + per-set dynamic offsets. +- Descriptor-derived access/layout describes what a draw will USE. It contributes only + per-command draw-access scopes, which the command compiler already derives from + `GraphDescriptorSet.accesses` (`renderer_rendergraph/src/commands.rs:190-216`). The resource's + initial `ResourceState` describes state before graph execution and remains caller-supplied, + because initial barrier generation reads `ResourceDesc::initial_state` + (`renderer_rendergraph/src/graph.rs:352-358,422-447`). +- **Identity index / dedupe:** maintain a graph-local identity index for imported buffers and + textures. Look up by `Arc::ptr_eq` first and Vulkan raw-handle equality fallback. Both + `import_buffer` and `import_texture` use this index rather than unconditionally appending + (`graph.rs:226-245`); `import_descriptor_set` routes every retained buffer/image through those + same methods. A direct import followed by set import (or vice versa) yields one `ResourceId`. +- **Initial-state merge:** on a duplicate explicit direct import, validate the newly supplied + initial state against the resource's recorded initial state; if they differ in stage, access, + or layout, return a conflicting-initial-state graph error. Do not OR-merge initial states. + Set import requires caller input for each resource; if it encounters a resource already + imported, the explicit state must match exactly or import errors. This prevents a fabricated + initial state and ensures the initial barrier source is unambiguous. +- **Draw-access scopes:** descriptor access is derived per binding and per draw, never folded + into `ResourceState`: + - uniform buffer -> `UNIFORM_READ`, exact effective offset/range; + - storage buffer -> conservative `SHADER_STORAGE_READ | SHADER_STORAGE_WRITE`, effective + offset/range; + - sampled image -> shader sampled read + descriptor image layout and normalized view range; + - storage image -> storage read|write in `GENERAL` and normalized view range; + - combined image sampler -> sampled-image read scope (sampler itself has no memory resource + hazard); + - sampler-only -> no graph memory access; + - uniform/storage texel buffer -> corresponding shader read or conservative storage read|write + over the view's buffer range; + - unsupported descriptor kinds or missing required owners -> explicit import/write error, + never guessed access. + Shader stages map from retained layout shader stages to graph pipeline stages. +- **Draw-scope merge:** deduplicate scopes by imported resource identity within the command and + merge compatible scopes by OR'ing stage/access masks while preserving their covered ranges. + Buffer ranges may remain separate if they are not safely coalescible. Texture layout + conflicts apply to overlapping draw scopes and produce an import error; disjoint subresource + ranges may retain distinct layouts. No draw-scope merge alters the resource's initial state. +- `track_descriptor_set` remains an explicitly unsafe-by-contract/deprecated raw route: it + cannot own or verify resources, and must not be used as though managed imports were safe. + +## API sketches + +```rust +// renderer_resources +impl DescriptorPool { + pub fn allocate_untracked(&self, descs: &[DescriptorSetAllocDesc]) + -> VkResult>; + pub fn allocate_managed(&self, desc: DescriptorSetAllocDesc) -> Result; +} +pub struct DescriptorSet { /* pool token, raw handle, pool generation snapshot, layout facts, binding map */ } +pub enum DescriptorResourceBinding { + Buffer { buffer: Arc, offset: u64, range: u64, dynamic: bool }, + Image { view: Arc, sampler: Option>, + range: vk::ImageSubresourceRange, layout: vk::ImageLayout }, + Sampler { sampler: Arc }, + TexelBuffer { view: Arc, range: BufferRange }, +} +pub struct TrackedDescriptorWrite { + pub binding: u32, pub array_element: u32, + pub descriptor_type: vk::DescriptorType, + pub stages: vk::ShaderStageFlags, + pub resource: DescriptorResourceBinding, +} +pub struct DescriptorWritePlan { /* validated Vulkan writes + staged retained entries */ } +pub fn validate_descriptor_writes( + layout: &DescriptorLayoutFacts, + writes: &[RawDescriptorWriteFacts], +) -> Result; +impl DescriptorSet { + // Checks generation as defense-in-depth; caller must externally synchronize reset against + // every use. Once returned, raw-handle validity is the caller's responsibility. + pub fn raw(&self) -> Result; + pub fn bindings(&self) -> Result>; + pub fn writer(&mut self) -> Result>; +} +impl DescriptorWriter<'_> { + pub fn write_buffer(&mut self, binding: u32, first_element: u32, + values: &[BufferDescriptor<'_>]) -> Result<&mut Self>; + pub fn write_image(&mut self, binding: u32, first_element: u32, + values: &[ImageDescriptor<'_>]) -> Result<&mut Self>; + pub fn write_sampler(&mut self, binding: u32, first_element: u32, + values: &[Arc]) -> Result<&mut Self>; + pub fn write_texel_buffer(&mut self, binding: u32, first_element: u32, + values: &[Arc]) -> Result<&mut Self>; + pub fn commit(self, set: &mut DescriptorSet) -> Result<()>; +} +``` + +```rust +// renderer_rendergraph +impl RenderGraph { + pub fn import_descriptor_set( + &mut self, + set: Arc, + initial_states: &[(DescriptorResourceIdentity, ResourceState)], + dynamic_offsets: &[u32], + ) -> Result; +} +pub struct GraphDescriptorSet { + /* private raw handle + retained Arc; no unchecked public raw-handle field */ + pub accesses: Vec, + pub dynamic_offsets: Vec, + /* retained Arc */ +} +``` + +Exact signatures may be adjusted to existing project conventions during implementation. + +## Staged tasks + +### Task 1 — Enabled-feature source (`renderer_device`) +Retain the exact `PhysicalDeviceFeatures` core and versioned feature chain passed to +`create_logical_device` in `DeviceInner` as the enabled-feature snapshot, and expose it through a +clearly named accessor distinct from the existing adapter-supported `features()` accessor +(`device.rs:156-168,398-413,594-596`; feature-chain type and push logic in +`renderer_device/src/lib.rs:171-194`). Preserve the chain's enabled values after passing it to +`push_to_device_create_info`; do not substitute `adapter.features`, which represents queried +support. Keep the change in `renderer_device`, with `renderer_resources` consuming the accessor +through its existing re-exports and no new dependency edge. +Tests: device-level accessor reflects the values supplied for logical-device creation and remains +distinct from adapter support data. +Accept: `cargo test -p renderer_device`; `cargo check -p renderer_device`. + +### Task 2 — Foundation (`renderer_resources`, no graph changes; depends on 1) +Retain layout binding descriptions; add normalized image-view range accessor; Arc-backed +binding entry types for the supported descriptor set; explicit unsupported-type and +swapchain-image errors; descriptor-indexing feature checks using Task 1's enabled-feature +accessor (both layout binding flags and type-specific descriptor features); and the reusable +graph-agnostic validation/write-plan interface. +Tests: binding lookup, range/type/count validation, partial binding, array element replacement, +dynamic binding metadata, sampler-only and texel-buffer owner retention, unsupported type/feature +rejection (including enabled-versus-supported feature cases), swapchain-backed image rejection, +and validation-plan errors. +Accept: `cargo test -p renderer_resources`; `cargo check -p renderer_resources`. + +### Task 3 — Managed allocation / writer / reset (depends on 2) +Managed allocation, owned write updates using the shared plan, `allocate_untracked` naming, and +caller-synchronized managed-pool reset/destruction and owner replacement contract. Implement the +pool generation counter: capture it at managed allocation, increment only after successful reset, +and reject every operation on surviving stale wrappers, including raw access, writes, import, and +bind. Reset should be unsafe or clearly documented as requiring GPU completion and exclusion of +concurrent use; wrapper-drop alone does not unlock reset. +Tests: owners remain alive after caller drops Arcs; invalid writes leave old entries intact; +reset invalidates a still-live managed wrapper and subsequent raw access/write returns a stale-set +error; failed reset does not advance the generation; reset contract is tested at the API boundary; +tests verify generation checks reject already-stale wrappers but do not treat those checks as +concurrent-use protection; reset's external-synchronization requirement covers allocation, writer +updates, graph recording/import, and command-buffer recording/binding; no test treats wrapper drop +as proof of GPU retirement. +Accept: `cargo test -p renderer_resources`; `cargo check --workspace`. + +### Task 4 — Graph transitive import (`renderer_rendergraph`, depends on 3) +Implement identity index and state-validation behavior in both `import_buffer` and +`import_texture` (replacing their current unconditional append behavior) and route descriptor +imports through those methods. Keep caller-supplied initial state separate from descriptor +draw-access scopes; define explicit per-resource initial state input and conflict errors. +Implement metadata-to-draw-scope mapping, layout conflict behavior, managed set retention, +dynamic-offset storage and ordered draw binding. Revalidate the managed-set generation at graph +import and immediately before command binding, returning an error for a wrapper already stale at +the check. This is defense-in-depth only: callers must externally synchronize reset against graph +recording/import and command-buffer recording/binding because revalidation cannot close a +concurrent reset check/use race. +`Draw::apply` must pass the combined offsets in +Vulkan-required order to `cmd_bind_descriptor_sets` rather than an empty slice. +Tests: direct+transitive dedupe in both import orders; duplicate direct imports with matching and +conflicting initial states; descriptor import initial-state conflicts; descriptor draw scopes do +not alter initial barrier sources; buffer/image access merge, incompatible overlapping texture +draw-layout error, sampled/storage mappings and ranges; an already-stale managed set is rejected at +import or bind after reset (not a claim that generation revalidation makes concurrent reset/use +safe); multiple dynamic bindings across +multiple sets, offset count/order, alignment, overflow/effective-range bounds; raw tracking +behavior. +Accept: `cargo test -p renderer_rendergraph`; `cargo check --workspace`. + +### Task 5 — Documentation + egui migration (separate `renderer` change, depends on 3) +Revise resource invariants in `crates/renderer_resources/AGENTS.md` (reverse the current +raw-only statement). Decide public re-exports (egui can use managed set types internally without +public re-exports; `renderer::{buffers,images,pipeline}` are crate-private aliases, +`renderer/src/lib.rs:23`). Migrate egui's set creation/storage (`renderer/src/lib.rs:108-115, +155-210,337`), update and bind flow (`egui.rs:561-611,621-630,742-746`), preserving its sparse +update-after-bind array and uniform/storage buffer binding. Egui uses the legacy graph; its +manual texture-read declarations (`egui.rs:621-630`) remain necessary because Task 4 changes +only `renderer_rendergraph`, not the legacy graph. Its swapchain/external lifetime and submission +ordering must continue to satisfy the caller-synchronized contract. +Accept: `cargo check -p renderer`; `cargo check --workspace`. + +## Risks / open questions + +- `Image::create_view` returns `ManuallyDrop`; callers must deliberately transfer + it into an `Arc` (`images.rs:1001-1008`). +- Swapchain-backed images are deliberately rejected in managed image writes because `Image` + wrappers do not own swapchain image handles, and `ImageView` retains only `Arc` + (`renderer_resources/AGENTS.md:39-47`). The untracked path remains caller responsibility. +- GPU retirement is not mechanically tracked in this skeleton: managed owner replacement/drop, + set/pool destruction, and pool reset require caller synchronization with completed submissions. + Reset must additionally be externally synchronized against all managed-set use (allocation, + writer updates, graph recording/import, and command-buffer recording/binding), consistent with + Vulkan's own external-synchronization model. Dropping wrappers or taking the pool CPU mutex is + insufficient. Retirement tokens or a GPU-usage tracking hook are the explicit future path. + Separately, successful pool reset advances the pool generation, making every surviving managed + wrapper detectably stale; generation checks are defense-in-depth and do not replace the + GPU-completion/exclusion contract or close a concurrent check/use race. Escaped raw handles remain + the caller's validity responsibility. +- Supported managed descriptor types and rejected types are enumerated above; feature checks + must use the retained enabled-feature snapshot from `renderer_device` and cover + descriptor-indexing binding flags and type-specific requirements, not adapter support queries or + layout flags alone. Never silently omit bound resources from graph hazards. +- A raw Vulkan set or external `update_descriptor_sets` call bypasses tracking; the API must + distinguish "tracked through this wrapper" from merely possessing a raw set. +- Dynamic offsets are bound per draw, validated against binding order/alignment/effective + buffer bounds, and included in draw scopes; they are not implicit descriptor-set state.