vidya/design-plans/descriptor-set-ownership.md
2026-10-26 02:11:29 +01:00

26 KiB

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<Buffer>, Arc<ImageView>, Arc<Sampler>, and texel-buffer views as appropriate. ImageView retains its Arc<Image> (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<ImageView> 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<Buffer> 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<Image>.

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<Buffer>, offset, range };
    • sampled/storage image and combined image sampler: { Arc<ImageView>, optional Arc<Sampler>, normalized subresource range, layout } (combined requires a sampler);
    • sampler-only: { Arc<Sampler> } with no image/view;
    • uniform/storage texel buffer: { Arc<BufferView>, 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<vk::DescriptorSet>) 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<DescriptorSet>, initial_states, dynamic_offsets) -> Result<GraphDescriptorSet, GraphError>. 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<DescriptorSet>, 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

// renderer_resources
impl DescriptorPool {
    pub fn allocate_untracked(&self, descs: &[DescriptorSetAllocDesc])
        -> VkResult<Vec<vk::DescriptorSet>>;
    pub fn allocate_managed(&self, desc: DescriptorSetAllocDesc) -> Result<DescriptorSet>;
}
pub struct DescriptorSet { /* pool token, raw handle, pool generation snapshot, layout facts, binding map */ }
pub enum DescriptorResourceBinding {
    Buffer { buffer: Arc<Buffer>, offset: u64, range: u64, dynamic: bool },
    Image { view: Arc<ImageView>, sampler: Option<Arc<Sampler>>,
            range: vk::ImageSubresourceRange, layout: vk::ImageLayout },
    Sampler { sampler: Arc<Sampler> },
    TexelBuffer { view: Arc<BufferView>, 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<DescriptorWritePlan>;
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<vk::DescriptorSet>;
    pub fn bindings(&self) -> Result<impl Iterator<Item = &TrackedDescriptorWrite>>;
    pub fn writer(&mut self) -> Result<DescriptorWriter<'_>>;
}
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<Sampler>]) -> Result<&mut Self>;
    pub fn write_texel_buffer(&mut self, binding: u32, first_element: u32,
                              values: &[Arc<BufferView>]) -> Result<&mut Self>;
    pub fn commit(self, set: &mut DescriptorSet) -> Result<()>;
}
// renderer_rendergraph
impl RenderGraph {
    pub fn import_descriptor_set(
        &mut self,
        set: Arc<DescriptorSet>,
        initial_states: &[(DescriptorResourceIdentity, ResourceState)],
        dynamic_offsets: &[u32],
    ) -> Result<GraphDescriptorSet, GraphError>;
}
pub struct GraphDescriptorSet {
    /* private raw handle + retained Arc<DescriptorSet>; no unchecked public raw-handle field */
    pub accesses: Vec<DescriptorResourceAccess>,
    pub dynamic_offsets: Vec<u32>,
    /* retained Arc<DescriptorSet> */
}

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<ImageView>; 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<Image> (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.