- Add BeginRendering/EndRendering commands with imported/created attachment
views, view plumbing, and aspect-gated depth/stencil slots
- Derive and validate render-pass spans at compile time; contract spans into
single DAG nodes before hazard analysis; hoist cross-boundary barriers to
pass begins with in-pass policy and created-view resolution
- Replace the four-pass ordering pipeline with a single command walk
(build_command_orderings): span derivation, validation, lifetime-only
injection, dependency edges, and Kahn batching in one pass; batches are
ranges into execution_order and RenderPassSpan keeps only begin/end
- Perf: node-collapsed frontier analysis, dense command/resource indexing
(no SipHash on hot paths), inline barrier storage, exact preallocation,
lazy Kahn ready heap, scoped barrier fast paths
- Scope buffer and image alias barriers to the incoming resource per the
Vulkan resource-memory-aliasing rules (availability unscoped, visibility
scoped to the new owner)
- Add criterion compile benchmarks (two_pass/{no_pass,pass}/per_{20,50,100})
576 lines
34 KiB
Markdown
576 lines
34 KiB
Markdown
# Plan: Render-Pass Support (`BeginRendering` / `EndRendering`) in `renderer_rendergraph`
|
||
|
||
Status: proposed — rev 5 (rev-1 correctness, rev-2 performance, rev-3 adversarial, and rev-4 re-review applied; rev-4 verdict was REJECT, findings resolved here)
|
||
Date: 2026-10-27
|
||
Scope: `crates/renderer_rendergraph`
|
||
|
||
## Goal
|
||
|
||
Add dynamic-rendering render-pass support to the rendergraph: `BeginRendering`
|
||
and `EndRendering` commands that bracket a span of graph commands, with the
|
||
compiler guaranteeing that every command recorded between the two is emitted
|
||
inside the rendering instance — i.e. commands that must be in a rendergraph
|
||
pass are ordered by the begin/end rendering commands, and no foreign command
|
||
may be scheduled between them.
|
||
|
||
## Current state (verified against code)
|
||
|
||
- Commands implement `Command` (declares `side_effects` → `SubresourceScope`
|
||
accesses) + `RecordedCommand` (`preflight`/`apply`), and are pushed via
|
||
`RenderGraph::push` into `CommandList` (`commands.rs`, `recorder.rs`).
|
||
- `GraphCompiler::dependencies()` (`graph.rs:820-863`) builds hazard edges
|
||
purely from resource conflicts; `topo_batches_and_dependencies()`
|
||
(`graph.rs:870-889`) Kahn-sorts commands into `batches`;
|
||
`record_commands` (`graph.rs:2500-2554`) flattens batches, emitting barriers
|
||
(keyed `before_command` via `grouped_barrier_indices`) immediately before
|
||
each command.
|
||
- There is no notion of a render pass. Nothing prevents the flattener from
|
||
emitting an unrelated command between two commands the user recorded
|
||
adjacently: Kahn batching interleaves hazard-free commands freely.
|
||
- `CommandRecorder` (`recorder.rs:159-164`) maps `ResourceId → vk::Buffer` /
|
||
`vk::Image` only; there is no image-view plumbing. Descriptor imports read
|
||
`Arc<ImageView>` bindings at import time (`graph.rs:507-513`) but retain
|
||
only the underlying image (`StagedImportResource::Texture(view.image().clone())`,
|
||
`graph.rs:524-532`) — attachment views are new plumbing, not an existing
|
||
pattern to reuse.
|
||
- `CommandList` stores type-erased `Box<dyn RecordedCommand>` (`recorder.rs:97-109`);
|
||
the `Command` trait exposes only `side_effects` (`commands.rs:14-16`), so
|
||
there is currently no way to classify a command or introspect its render-pass
|
||
descriptor.
|
||
- The graph documents a single-queue submission contract (`graph.rs:8-10`);
|
||
ordinary barriers set `queue_transfer: None` (`graph.rs:972-982, 1001-1012`).
|
||
- Compile-time cost profile (recomputed independently): `dependencies()` is
|
||
`O(Σ_r u_r²)` pair checks (plus a per-pair scope cross-product factor);
|
||
`HappensBefore::from_dag` allocates one `FixedBitSet` row per node
|
||
(`graph.rs:213-217`, `O(N²/64)` memory) and unions per edge
|
||
(`O(E·N/64)` time). `ResourceInterference::from_resource_uses` **already**
|
||
computes terminal/initial frontier sets once per resource
|
||
(`graph.rs:1348-1356`); the remaining recomputations are in
|
||
`first_fit_placement` (`graph.rs:1642-1649`), the `overlapping` loop in
|
||
`plan_placements` (`graph.rs:2384-2399`), and `build_alias_barriers`
|
||
(`graph.rs:1088, 1109-1117`). Contraction does not remove those; see D8.
|
||
- The crate is not yet referenced by `renderer`/`bevy_vulkan_render`; the
|
||
current per-frame path is the legacy `renderer::render_graph`. If this
|
||
crate replaces it, `compile()` becomes per-frame, so compile cost matters.
|
||
- `dynamic_rendering` and `synchronization2` are requested/enabled
|
||
(`crates/renderer/src/lib.rs:352-357`, `crates/renderer_device/src/testing.rs:166-172`).
|
||
- **Pre-existing defect (reproduced):** `build_barriers` records only the last
|
||
*conflicting* producer's layout (`graph.rs:951-982`) and never chains
|
||
`old_layout` through earlier barriers. Two same-layout readers of one
|
||
producer yield two image barriers both claiming `oldLayout = producerLayout`;
|
||
the second is stale (Vulkan requires `oldLayout` be the current layout or
|
||
`UNDEFINED`). Fixing this generally (per-subresource current-layout map) is
|
||
part of Task 3 — the plan's own tests hit it.
|
||
- `Image::create_view` returns `Result<ManuallyDrop<ImageView>>`
|
||
(`renderer_resources/src/images.rs:558-621`); storing it directly leaks the
|
||
`vk::ImageView` and the `Arc<Image>` (the existing renderer calls `.raw()`
|
||
on the `ManuallyDrop`, `crates/renderer/src/rendering/mod.rs:332`, which
|
||
leaks today). New code must use `ManuallyDrop::into_inner`.
|
||
- `ImageView` holds `Arc<Image>` (`images.rs:1123-1127`), so a view cache
|
||
keeps its image alive; drop-order interaction with `CompiledGraph` is benign
|
||
provided the `ManuallyDrop` is unwrapped.
|
||
|
||
## Design decisions
|
||
|
||
### D1 — Dynamic rendering, not `VkRenderPass`
|
||
|
||
New commands `BeginRendering` / `EndRendering` wrapping `vkCmdBeginRendering`
|
||
/ `vkCmdEndRendering` (Vulkan 1.3 `dynamic_rendering`). Attachments are graph
|
||
`Texture` handles plus a view specification (see D9 for the created-texture
|
||
problem).
|
||
|
||
### D2 — The pass span is an atomic scheduling unit, contracted *before* hazard-edge construction
|
||
|
||
The compiler contracts the `[begin, end]` command span into a single DAG node
|
||
("pass node"). This is the mechanism that satisfies the ordering contract:
|
||
|
||
- No foreign command can ever be scheduled inside the span (Kahn batching
|
||
would otherwise interleave hazard-free commands into it).
|
||
- The whole span is ordered relative to everything else by the begin/end
|
||
hazard edges.
|
||
|
||
Contraction happens **before** `dependencies()` builds hazard edges. This
|
||
removes the `O(Σ u_r²)` interior-draw edge construction (edges that would be
|
||
discarded as self-edges anyway) and shrinks the `O(N²/64)` closure memory/time
|
||
by roughly `(N_c/N)²`.
|
||
|
||
The contraction mapping used by every consumer:
|
||
|
||
- **Type-safe node indexing:** use `NodeId(NonZeroU32)` newtypes and
|
||
`Option<NodeId>` rather than raw `u32` sentinels mixed into `CommandIndex`
|
||
spaces. To minimize churn, keep `batches: Vec<Vec<CommandIndex>>` with the
|
||
`BeginRendering` command index as the documented pass-node representative;
|
||
`spans: Vec<Span>` on `CompiledGraph` records `{ begin, end, interior }`
|
||
for expansion. Document the semantic change to the public `batches()` /
|
||
`dependency_edges()` accessors. Closure rows are indexed `0..N_c`.
|
||
- **HappensBefore is span-aware internally:** `HappensBefore` (public,
|
||
`lib.rs:10`) stores the command→node map and span slices and translates in
|
||
`contains`/`happens_before` (`graph.rs:244-251`), so command-space queries
|
||
keep working and interior indices never silently return `false` from a
|
||
node-space row lookup.
|
||
- **Intra-span record order is NOT a happens-before fact.** Interior commands
|
||
map to the pass node and same-node uses are treated as **unordered** by the
|
||
closure used for aliasing/interference. This is a memory-correctness
|
||
requirement, not a style choice: if intra-span record order were visible to
|
||
`resources_alias_in_order`, a created attachment `A` (single use at `Begin`)
|
||
and a created storage image `B` used only by an interior command would be
|
||
judged ordered, `first_fit_placement` could place `B` over `A`'s memory, and
|
||
`build_alias_barriers` would either emit no barrier or an illegal in-pass
|
||
`UNDEFINED → layout` transition — discarding a live render target mid-pass.
|
||
Intra-span record order is needed only by `build_barriers` (which iterates
|
||
execution order directly) and by recording; it must not leak into the
|
||
aliasing/interference closure.
|
||
- **Attachment liveness is modeled through `End`.** `EndRendering` carries
|
||
synthetic accesses (the attachment layout, `COLOR_ATTACHMENT_WRITE` /
|
||
`DEPTH_STENCIL_ATTACHMENT_WRITE` at the attachment stage) used **only** for
|
||
lifetime/alias analysis — excluded from barrier generation (the begin's
|
||
merged scope already covers synchronization) and from usage derivation.
|
||
This makes `terminal_uses(attachment) == End`, so an attachment is never
|
||
judged dead before the store.
|
||
- **Barrier analysis:** `build_barriers` walks scheduled command indices and
|
||
reads only `accesses[command_index]` (`graph.rs:930-934`). Barrier analysis
|
||
runs over the *uncontracted* access list in **execution order** (see D6),
|
||
with each interior command's scheduling position mapped to the pass node.
|
||
This preserves, e.g., the barrier for an outside producer consumed only by
|
||
an interior `Draw`.
|
||
- **Lifetimes:** `bind_lifetimes` indexes `batch_for_command` for every
|
||
original access (`graph.rs:1258-1275`); interior commands map to the pass
|
||
node's batch index so they are never left at `usize::MAX`. Placement uses
|
||
command-level interference, not batch intervals, so placement quality is
|
||
unaffected; external consumers of `resource_lifetimes()` get coarser
|
||
intervals for interior commands (documented).
|
||
- **Self-edges:** intra-span hazard pairs are skipped during edge
|
||
construction (they become intra-span execution-order constraints handled by
|
||
`build_barriers`, not DAG edges). Note `dependencies()` only adds
|
||
prior→current edges (`graph.rs:825-861`), so the DAG is acyclic regardless;
|
||
only a literal self-edge would trip `Kahn`, and skipping intra-span pairs
|
||
prevents exactly that.
|
||
|
||
### D3 — Access declaration: one merged scope per attachment; interior accesses preserved
|
||
|
||
- `BeginRendering` declares **one access per attachment** with
|
||
`access = COLOR_ATTACHMENT_WRITE | (LOAD ? COLOR_ATTACHMENT_READ : 0)` at
|
||
the same stage (`COLOR_ATTACHMENT_OUTPUT` / `EARLY|LATE_FRAGMENT_TESTS`),
|
||
subresource range from the view (concrete range rules in D9). Declaring
|
||
read and write as two separate accesses is a blocker:
|
||
`SideEffectMap::insert` deliberately keeps opposite-polarity scopes separate
|
||
(`recorder.rs:55-82`), and `build_barriers` then emits **two**
|
||
`ImageMemoryBarrier2` entries for the same image/range/layout in one
|
||
`vkCmdPipelineBarrier2` (reproduced). A single merged scope is handled
|
||
correctly by `SideEffectMap`, `conflicts`, `derive_image_usage`
|
||
(`graph.rs:2696-2709`), and the barrier builder.
|
||
- `EndRendering`'s synthetic accesses (D2) are tagged so barrier generation
|
||
and usage derivation skip them.
|
||
- Interior command accesses are *not* replaced by the begin declarations;
|
||
they remain in the access list (see D2 barrier analysis) so the compiler
|
||
can emit barriers for outside producers consumed inside the pass.
|
||
- Because the span is contracted, any later consumer is ordered after the
|
||
*entire pass node*, i.e. after `EndRendering` — matching store-visibility
|
||
semantics (verified: the consumer's barrier takes
|
||
`COLOR_ATTACHMENT_OUTPUT`/`COLOR_ATTACHMENT_WRITE` as src, and hoisting a
|
||
barrier before `BeginRendering` with those dst stages is sufficient
|
||
visibility for LOAD reads and in-pass attachment reads).
|
||
- **Intra-span hazard policy (narrowed):**
|
||
- **Allowed inside a span:** any barrier with `old_layout == new_layout` —
|
||
memory barriers, buffer barriers, *and* image barriers with no layout
|
||
change (a storage-image read/write hazard at `GENERAL` needs an
|
||
`ImageMemoryBarrier2`; buffer barriers do not synchronize image
|
||
accesses). These are legal inside a rendering instance.
|
||
- **Rejected at compile time** (`GraphError::UnsupportedIntraPassDependency`):
|
||
intra-span hazards requiring an image layout transition (`old != new`).
|
||
The rejection operates on the **generated barrier plan** (D5), not on an
|
||
independent declared-layout scan.
|
||
- **Feedback loops:** defined as any interior access (read *or* write)
|
||
whose `(aspect, mip, layer)` set intersects a `Begin` attachment
|
||
subresource, excluding the begin's own load operation. A storage-image
|
||
write to an attachment subresource is equally a feedback loop; an
|
||
interior read of a disjoint mip/layer is legal. Genuine feedback loops
|
||
require splitting.
|
||
- In-pass barrier emission is part of Task 4, not a follow-up: the recorder
|
||
emits the barrier between the two interior commands in execution order.
|
||
- **Aliasing vs. interior first uses is a placement constraint, not a
|
||
validation message.** For image resources, add an interference edge (or an
|
||
explicit `aliasing_allowed` predicate consulted by `plan_placements` /
|
||
`first_fit_placement`) whenever a required alias transition would land at a
|
||
command that maps to a pass node — covering interior *and* boundary
|
||
(`Begin`/`End`) positions. There is no "reject aliasing" hook after
|
||
`plan_placements` commits offsets, so this must be enforced before
|
||
placement. Attachments themselves are safe: with D2's `End` modeling,
|
||
their first use is `Begin` and their terminal use is `End`, both boundary
|
||
commands of the pass node itself.
|
||
|
||
### D4 — Barrier placement: hoist cross-boundary barriers to BeginRendering, with per-subresource layout handling
|
||
|
||
Barriers whose `before_command` targets an interior command but whose origin
|
||
lies outside the span are re-keyed to the span's `BeginRendering` command,
|
||
after batching when spans are known. Constraints:
|
||
|
||
- **Barrier origin must be explicit.** `BarrierResource` (`graph.rs:84-95`)
|
||
records only masks/layout — no source command and no initial-vs-command
|
||
origin — so a post-pass hoisting pass cannot distinguish an outside-producer
|
||
barrier from an initial-state transition. Add
|
||
`src_command: Option<CommandIndex>` or a
|
||
`BarrierOrigin::{Initial, Command(CommandIndex)}` to `BarrierResource`.
|
||
**Initial-state transitions for interior first uses are hoisted to
|
||
`Begin`** and are not covered by the intra-span rejection (a transient
|
||
storage image first used inside a pass is the common case, not an edge
|
||
case).
|
||
- **Coalesce hoisted image transitions per `(resource, exact range,
|
||
target_layout)` into one barrier** with a unioned `dst_stage/dst_access`
|
||
(safe: the dst scope is a wait target; unioning only makes all consumers
|
||
wait). Layout chaining is defined **per `(aspect, mip, layer)`**, not per
|
||
barrier: for overlapping-but-unequal ranges, transition only the
|
||
subresources still in the producer layout; for disjoint ranges, emit
|
||
independent `producer → layout` transitions. The earlier "chain different
|
||
target layouts" rule is removed: for same-pass overlapping ranges it is
|
||
unreachable (D3 rejects the required in-pass transition), and for disjoint
|
||
ranges chaining `producer → A → B` is simply wrong (the second range is
|
||
still in the producer layout).
|
||
- The general `build_barriers` fix (per-subresource current-layout map, not
|
||
"last conflicting access's layout") makes non-hoisted multi-reader cases
|
||
correct too; D4's rules then only govern the hoisted subset.
|
||
- After hoisting, rebuild `barrier_indices_by_command` and
|
||
`ordinary_barrier_count` (`O(B)`), and apply the same coalescing in
|
||
`rebuild_barriers` (`graph.rs:2092-2106`) for alias barriers.
|
||
**Sequencing:** hoisting runs before `build_alias_barriers`, and hoisted
|
||
barriers live strictly within the ordinary prefix so `rebuild_barriers`'
|
||
truncation to `ordinary_barrier_count` (`graph.rs:2098`) cannot drop or
|
||
duplicate them; `rebuild_barriers` re-runs hoisting after the alias rebuild.
|
||
- Consumers after the pass keep their own barrier keys; expansion emits them
|
||
after `EndRendering` naturally. No barrier targets `EndRendering` itself
|
||
(its synthetic accesses are excluded from barrier generation).
|
||
- In-pass barriers (per D3) are emitted between the two interior commands in
|
||
execution order; they are not hoisted.
|
||
|
||
### D5 — Compile-time validation on the generated barrier plan, all linear
|
||
|
||
New `GraphError` variants for:
|
||
|
||
- unbalanced begin/end; nested/overlapping spans (dynamic rendering cannot
|
||
nest); `EndRendering` before its `BeginRendering` in record order;
|
||
- graphics commands outside any span (`GraphError::DrawOutsideRenderPass`),
|
||
and **transfer/clear commands inside a span** (`CopyBuffer`,
|
||
`ClearTexture`, and any custom command not marked render-pass-legal —
|
||
`vkCmdCopyBuffer2`/`vkCmdClearColorImage` are illegal inside a rendering
|
||
instance). Classification uses a `fn kind(&self) -> CommandKind` method on
|
||
**`RecordedCommand`** (not `Command` — a method on `Command` is
|
||
unreachable after `Box<dyn RecordedCommand>` erasure), returning
|
||
`CommandKind::{Begin, End, Graphics, Transfer, Compute, Other}` plus a
|
||
`fn allowed_in_render_pass(&self) -> bool` (default false; `Draw` → true;
|
||
test/custom no-op commands may override so Task 5's in-pass barrier tests
|
||
can exist). **Note:** five existing unit tests compile graphs containing
|
||
`Draw` with no pass (pushes at `graph.rs:3563, 3749, 3815, 4329, 4469`)
|
||
and must be updated (wrap in begin/end or make the restriction opt-in) —
|
||
added to Task 5.
|
||
- unsupported intra-span dependencies (per D3). **The check operates on the
|
||
generated barrier plan after hoisting/chaining, not on an independent
|
||
declared-layout scan:** after hoisting, any barrier whose `before_command`
|
||
is interior and whose `old_layout != new_layout` is either hoistable
|
||
(initial/outside origin) or must be rejected. This is exact, `O(B)`, and
|
||
removes the fragile per-subresource validation machinery whose
|
||
`previous`-selection (`graph.rs:951-982`) can diverge from declared
|
||
layouts (e.g. producer `P` at layout `P0`, outside reader `R1` transitions
|
||
to `A`, interior reader `R2` reads at `A`: a declared-layout scan sees
|
||
`A → A` and accepts, while the un-fixed builder would emit `P0 → A` at
|
||
`R2` inside the pass).
|
||
- attachment consistency: each attachment view must refer to the same image
|
||
as its graph `Texture` — compare **raw handles** (`Image: PartialEq`
|
||
compares raw handles, `images.rs:484-488`; `Arc::ptr_eq` would
|
||
false-positive), consistent with import dedup by raw handle
|
||
(`graph.rs:389-399`). Applies to the imported case at compile time and to
|
||
the created case after view creation (D9). The recorder binds graph images
|
||
by `ResourceId` (`graph.rs:2513-2529`); this check closes that gap.
|
||
- attachment layout legality: the caller-specified layout must be a legal
|
||
rendering layout (`COLOR_ATTACHMENT_OPTIMAL`, `GENERAL`, etc.). The
|
||
previously proposed "layout must match compiler-produced state" rule is
|
||
vacuous — the barrier's `new_layout` is derived from the begin's own
|
||
declared access, so no independent state exists.
|
||
- duplicate attachment subresources: two attachments covering the same
|
||
subresource would be silently merged by `SideEffectMap::insert`
|
||
(`recorder.rs:55-82`), making them invisible from `command_accesses`;
|
||
reject via the introspection hook (below), using a per-subresource set
|
||
(`O(covered subresources)`, not pairwise `O(a²)`).
|
||
- queue ownership: passes cannot cross queues or require ownership
|
||
transfers; the single-queue submission contract (`graph.rs:8-10`) applies
|
||
unchanged, and any access requiring a queue transfer inside or across a
|
||
span is rejected until paired transfer/barrier support exists.
|
||
- **Introspection hook:** commands are type-erased, and `CommandKind` carries
|
||
no data, so Task 2's attachment validations (view/image identity, duplicate
|
||
subresources, layout legality) need a `fn render_pass_desc(&self) ->
|
||
Option<&RenderPassDesc>` method on `RecordedCommand` (or a validation hook
|
||
called from `compile()`, or an attachment summary stored in
|
||
`CommandList::push`, which is generic over `C: Command`). Without one,
|
||
these checks are not implementable as written.
|
||
- **Error-path safety:** `record_commands` returning mid-pass must not leave
|
||
an unclosed rendering instance; track an `in_render_pass` flag in the
|
||
recorder/compiled graph, end the rendering instance (or discard the command
|
||
buffer) on the error path, and reset the flag (`graph.rs:2540-2553`).
|
||
|
||
### D6 — Execution order is precomputed at compile time
|
||
|
||
Compile an `execution_order: Vec<CommandIndex>` once: expanded contracted
|
||
batches — begin → interiors in **original record order** → end. Barrier
|
||
analysis and recording both iterate it; `batches` is retained (begin-index
|
||
representative) for future parallel recording. This avoids per-frame
|
||
expansion branches and keeps `recording_steps_cached` (`graph.rs:2595-2608`)
|
||
allocation-free. Interior commands are emitted in record order, not topo
|
||
order: ordering within the pass is the user's responsibility, matching the
|
||
"ordered by begin/end" contract.
|
||
|
||
**Batch-index assertion fix:** `build_barriers_with_initial_states` asserts
|
||
`batch_indices[previous] < batch_indices[current]` (`graph.rs:962-966`).
|
||
With interior commands mapped to the pass node's batch index, an allowed
|
||
intra-span hazard gives equal batch indices → debug panic. Use
|
||
**execution-order positions** for that assertion, not batch indices.
|
||
|
||
### D7 — Recording is allocation-free per frame
|
||
|
||
- `BeginRendering::apply` precomputes `vk::RenderingAttachmentInfo`s in the
|
||
command (or uses `ArrayVec`/thread-local scratch, mirroring
|
||
`pipeline_barrier`'s `BARRIER_SCRATCH` pattern, `recorder.rs:217-292`) so
|
||
no `Vec` is built per frame. Resolving attachment images per frame is
|
||
`O(a log R)` `BTreeMap` lookups — negligible.
|
||
- **View plumbing into `apply`:** `RecordedCommand::apply` receives only
|
||
`CommandRecorder` (`recorder.rs:108`), which cannot see `CompiledGraph`'s
|
||
view cache. Extend `CommandRecorder` with a view map
|
||
(`ResourceId → Arc<ImageView>`, or `ResourceId → Arc<Image>` plus
|
||
command-local view cache populated via `&mut self`) populated by
|
||
`record_commands`. `BeginRendering::apply` looks up the view; a missing or
|
||
unresolved view is a new `RecordError::MissingView`. Views are created
|
||
with `ManuallyDrop::into_inner(create_view(...))` — never store the
|
||
`ManuallyDrop` directly (leak).
|
||
- **Resolve idempotence:** views are created only after a fully successful
|
||
placement pass, atomically; a failed view creation must not leave a
|
||
partially-populated cache that later makes `Arc::get_mut`
|
||
(`graph.rs:2472-2480`) fail on a retried placement. Repeated `resolve()`
|
||
calls are idempotent; `create_view` failures map to `GraphError` variants.
|
||
|
||
### D8 — Frontier reuse for placement and alias barriers (performance amendment, corrected)
|
||
|
||
`ResourceInterference::from_resource_uses` already caches frontier sets per
|
||
resource (`graph.rs:1348-1356`); the plan does not claim to change it. The
|
||
actual recomputation hot spots contraction does not touch are
|
||
`first_fit_placement` (`graph.rs:1642-1649`), the `overlapping` loop in
|
||
`plan_placements` (`graph.rs:2384-2399`), and `build_alias_barriers`
|
||
(`terminal_uses` per old resource per new resource at `graph.rs:1109-1117`;
|
||
`initial_uses` per new at `graph.rs:1088`). Compute terminal/initial
|
||
frontiers **once per resource** and reuse them across all three sites, so
|
||
the performance rationale is honest: contraction removes the dependency-edge
|
||
and closure quadratics; frontier reuse removes the placement/alias ones.
|
||
|
||
### D9 — Attachment API for created textures
|
||
|
||
`RenderGraph::create_texture` returns only a `Texture` id (`graph.rs:352-354`);
|
||
the `Arc<Image>` is created privately in `plan_placements`/`resolve`
|
||
(`graph.rs:2129-2495`) and `CompiledGraph` exposes no accessor, so a caller
|
||
can never supply an `Arc<ImageView>` for a transient render target — the
|
||
main use case. Resolution:
|
||
|
||
- `ColorAttachment.view` is an enum:
|
||
`AttachmentView::Created(ImageViewDesc) | Imported(Arc<ImageView>)`.
|
||
- **`Created(ImageViewDesc)` must carry concrete aspect/mip/layer ranges**
|
||
(`REMAINING_*` sentinels are rejected at validation): `side_effects` runs
|
||
at `push` time (`recorder.rs:119-124`), before `resolve()` creates any
|
||
image, `image_view_subresource_range` is private (`images.rs:1183`), and
|
||
`create_view` normalizes/mutates the desc (`set_max_end`, format fill-in,
|
||
`images.rs:566-589`; `MipRange` addition panics on overflow in debug for
|
||
`base > 0` with a `REMAINING` count, `images.rs:1319-1333`). The declared
|
||
access range therefore comes from the desc, and is validated against the
|
||
image's mip/layer counts at compile time (from `GraphTextureDesc` /
|
||
imported `ImageDesc`).
|
||
- The crate creates and caches views after `resolve()` (`Image::create_view`
|
||
needs `&Arc<Image>`, `images.rs:558`), keyed by `(Texture, ImageViewDesc)`;
|
||
`BeginRendering::apply` consumes the cached view via the D7 recorder view
|
||
map.
|
||
- The D5 view/image consistency check applies to the imported case at
|
||
compile time and to the created case after view creation.
|
||
|
||
## Work items (all in `crates/renderer_rendergraph`)
|
||
|
||
### Task 1 — New commands in `src/commands.rs`
|
||
|
||
- `BeginRendering { color: Vec<ColorAttachment>, depth: Option<DepthStencilAttachment>, ... }`
|
||
with `ColorAttachment { texture: Texture, view: AttachmentView (D9),
|
||
layout: vk::ImageLayout, load_op, store_op, clear_value }` (depth/stencil
|
||
analogous). `EndRendering` (unit struct).
|
||
- `Command::side_effects`: `BeginRendering` inserts **one merged scope per
|
||
attachment** (write + conditional LOAD read, same stage, concrete range
|
||
from the desc/view — D3, D9). `EndRendering` inserts synthetic
|
||
lifetime-only accesses (D2), tagged to be skipped by barrier generation
|
||
and usage derivation.
|
||
- Add `fn kind(&self) -> CommandKind` and
|
||
`fn allowed_in_render_pass(&self) -> bool` and
|
||
`fn render_pass_desc(&self) -> Option<&RenderPassDesc>` to
|
||
`RecordedCommand` (D5): `Draw` → `Graphics`/allowed; begin/end →
|
||
`Begin`/`End`; `CopyBuffer`/`ClearTexture` → `Transfer`/not allowed;
|
||
default `Other`/not allowed.
|
||
- `RecordedCommand::apply`: `BeginRendering` builds `vk::RenderingInfo` +
|
||
precomputed `vk::RenderingAttachmentInfo`s (resolving `vk::Image` via
|
||
`recorder.image(...)`, using the cached/created view via the D7 view map)
|
||
and calls `cmd_begin_rendering`; `EndRendering` calls `cmd_end_rendering`.
|
||
Allocation-free per D7.
|
||
- Export both from `lib.rs`.
|
||
|
||
### Task 2 — Span tracking & validation in `src/graph.rs`
|
||
|
||
- Derive spans in `compile()` by scanning command record order using
|
||
`CommandKind`; build `node_of_command` and `spans`.
|
||
- Validation pass implementing the D5 rules, including the raw-handle
|
||
view/image consistency check (imported case), layout legality, duplicate
|
||
attachment subresources (via the introspection hook), and the
|
||
classification-based inside/outside-pass checks. Linear per D5.
|
||
- Update the five existing unit tests that compile `Draw` without a pass
|
||
(pushes at `graph.rs:3563, 3749, 3815, 4329, 4469`).
|
||
|
||
### Task 3 — Span contraction in the compiler
|
||
|
||
- Contract spans **before** `dependencies()` per D2: aggregate access scopes
|
||
per pass node, skip intra-span pairs during edge construction, begin-index
|
||
node representation, contracted `dependency_edges`; `HappensBefore` stores
|
||
the command→node map and translates in queries.
|
||
- Fix the pre-existing stale-`old_layout` defect in `build_barriers`
|
||
(`graph.rs:951-982`) with a per-subresource current-layout map (D4).
|
||
- Add `BarrierOrigin` to `BarrierResource` (D4).
|
||
- Barrier analysis runs over the uncontracted access list in execution order
|
||
with interior positions mapped to the pass node (D2); switch the
|
||
`batch_indices` debug assertion to execution-order positions (D6).
|
||
- `bind_lifetimes`, `HappensBefore::from_dag`, `uses_by_resource`, and
|
||
`build_alias_barriers` thread the span map through per D2; all closure
|
||
consumers route through the span-aware query; same-node uses are unordered
|
||
for aliasing/interference (D2 blocker fix).
|
||
- Precompute `execution_order` per D6.
|
||
- Frontier reuse per D8 (`first_fit_placement`, `plan_placements` overlapping
|
||
loop, `build_alias_barriers`).
|
||
|
||
### Task 4 — Barrier hoisting, in-pass barriers & recording
|
||
|
||
- Hoist cross-boundary barriers per D4, including initial-state transitions
|
||
for interior first uses (`BarrierOrigin::Initial`); coalesce per
|
||
`(resource, exact range, target_layout)` with unioned dst masks;
|
||
per-`(aspect, mip, layer)` layout handling for overlapping/disjoint ranges.
|
||
- Enforce the aliasing placement constraint per D3 (interference edge /
|
||
`aliasing_allowed` predicate) before placement commits offsets.
|
||
- Emit in-pass barriers (any `old_layout == new_layout`) between interior
|
||
commands in execution order for allowed intra-span hazards (D3).
|
||
- Hoisting sequenced before `build_alias_barriers`, hoisted barriers within
|
||
the ordinary prefix, `rebuild_barriers` re-runs hoisting (D4).
|
||
- `recording_steps_cached` iterates `execution_order`, expanding pass nodes:
|
||
barriers (hoisted) → `BeginRendering` → interior commands in record order
|
||
(with in-pass barriers interleaved) → `EndRendering`.
|
||
- `record_commands`: expanded steps, D7 view map population, and the D5
|
||
error-path safety (`in_render_pass` flag, no unclosed rendering instance
|
||
on mid-pass `RecordError`).
|
||
|
||
### Task 5 — Tests
|
||
|
||
- Unit tests in `graph.rs`:
|
||
- span validation errors (unbalanced, nested, draw-outside-pass,
|
||
transfer-inside-pass, view/image mismatch, layout legality, duplicate
|
||
attachment subresources, feedback-loop rejection);
|
||
- contraction correctness (foreign command never appears between begin and
|
||
end in `execution_order`; interior accesses still produce barriers;
|
||
lifetimes resolved for interior commands);
|
||
- barrier hoisting (barrier for an outside producer consumed by an interior
|
||
command is keyed to begin; **two interior consumers of one
|
||
outside-produced image coalesce into one barrier with unioned dst masks
|
||
and correct per-subresource `old_layout`** — regression test for the
|
||
reproduced blocker; also cover the non-hoisted two-reader case fixed by
|
||
the general `build_barriers` change);
|
||
- initial-state hoisting (transient storage image first used inside a pass
|
||
gets its `UNDEFINED → GENERAL` transition before `BeginRendering`);
|
||
- in-pass barrier emission (interior storage-buffer *and* storage-image
|
||
same-layout hazards produce barriers between the interior commands, no
|
||
pass split; layout-transition hazard is rejected);
|
||
- **aliasing safety (regression test for the rev-4 blocker):** created
|
||
attachment `A` + interior-only storage image `B` must **not** share
|
||
memory, and no barrier may target an interior command for `A`/`B`;
|
||
attachment `terminal_uses == End`;
|
||
- ordering of both incoming and outgoing dependencies around the whole
|
||
span;
|
||
- span-aware closure queries (`happens_before` with interior endpoints;
|
||
same-node uses unordered for aliasing).
|
||
- Integration tests in `tests/graph_validation.rs` (following the existing
|
||
`TestVulkan` pattern):
|
||
- skip when `core13.dynamic_rendering != TRUE` in addition to the existing
|
||
`synchronization2` check (`tests/graph_validation.rs:108-111`);
|
||
- the previously proposed `Draw`-based graph is **not constructible**
|
||
(`CopyBuffer` accepts only buffers, `commands.rs:30-79`; `Draw` has no
|
||
pipeline handle and never binds one, `commands.rs:143-153, 226-335`).
|
||
Use a custom no-op command (like `ScopedNoop`) with a texture read
|
||
access as the post-`End` consumer:
|
||
`ClearTexture → BeginRendering(color=target) → allowed no-op (texture
|
||
read) → EndRendering → no-op (texture read)` compiles, records,
|
||
submits, and validates. Adding a real `BindPipeline` command (with a
|
||
documented sequence edge to the following `Draw`, since pipeline binding
|
||
declares no resources) is an optional separate work item;
|
||
- a case proving a hazard-free foreign command is emitted *after*
|
||
`EndRendering`, not inside the pass;
|
||
- an outside-producer → interior-consumer case asserting the hoisted
|
||
barrier appears before `BeginRendering`.
|
||
|
||
### Task 6 — Existing test/API churn
|
||
|
||
- `batches()` assertions and helpers that assume command-space batches:
|
||
`graph.rs:3774, 4285, 4388-4393, 4674, 5237, 5245, 5257, 5337, 5343, 5397,
|
||
5409, 5414` (including `recording_steps(compiled.batches(), …)` at 5343).
|
||
- `GraphCompiler::new` / `build_barriers` signature changes (execution order +
|
||
span map) break direct unit-test call sites at `graph.rs:3177-3178, 4125,
|
||
4242, 4716-4718, 4792-4800, 4846-4854, 4893-4901, 4962-4976, 5051-5052,
|
||
5102-5104, 5142-5145, 5171-5173`; `HappensBefore::from_dag` is called
|
||
directly by a test (`graph.rs:4140-4155`) and its signature changes.
|
||
- Wrapping a `Draw` in a pass shifts barrier indices in tests such as
|
||
`sampled_image_import_sets_usage_and_layout_barrier` (asserts
|
||
`compiled.barriers()[0]`, `graph.rs:3837-3845`).
|
||
- Provide compatibility constructors where cheap, or update all listed call
|
||
sites; `cargo test -p renderer_rendergraph` must pass at the end.
|
||
|
||
## Sequencing
|
||
|
||
Tasks 1–2 and 3–4 share types, so execute strictly in order: **1 → 2 → 3 →
|
||
4 → 5 → 6**, with `cargo check -p renderer_rendergraph` after each and
|
||
`cargo test -p renderer_rendergraph` at the end. No changes to other crates;
|
||
`renderer`'s legacy `render_graph` module is out of scope.
|
||
|
||
## Performance summary (from rev-2 review, corrected in revs 3–5)
|
||
|
||
- **Contraction before `dependencies()`** removes the interior-draw
|
||
`O(Σ u_r²)` edge construction and shrinks the `O(N²/64)` closure
|
||
memory/time by roughly `(N_c/N)²` (a pass spanning 90% of commands →
|
||
~100× less closure work).
|
||
- **Contraction does not touch** the placement/alias frontiers; D8's
|
||
frontier reuse addresses `first_fit_placement`, `plan_placements`'
|
||
overlapping loop, and `build_alias_barriers` (interference frontier
|
||
caching already exists).
|
||
- **Barrier hoisting is cost-neutral to slightly positive** provided
|
||
coalescing follows D4 (per-target-layout, per-subresource handling,
|
||
unioned dst masks only for identical ranges/layouts).
|
||
- **In-pass barriers for same-layout hazards** avoid the significant
|
||
tiled-GPU regression of forced pass splits (attachment store/load
|
||
round-trips); only layout transitions and feedback loops force splits.
|
||
- **Validation is linear** with the barrier-plan-based intra-span check
|
||
(`O(B)`) and per-subresource-set duplicate detection; the existing
|
||
layout-conflict precheck grows by `O(a²)` pair iterations per pass
|
||
(28 for 8 attachments with the merged single access) — negligible.
|
||
- **Recording stays allocation-free** via precomputed attachment infos,
|
||
thread-local scratch, precomputed `execution_order`, and begin-index node
|
||
representation.
|
||
|
||
## Risks / open points
|
||
|
||
- **API shape:** raw `push(BeginRendering)` / `push(EndRendering)` vs. a
|
||
scoped `render_pass(desc, |graph| ...)` builder that pushes begin, runs the
|
||
closure, and pushes end while recording span command indices. The scoped
|
||
builder makes unbalanced spans unrepresentable; raw push is more flexible.
|
||
Decide before Task 1.
|
||
- **Created-texture views (D9):** view creation after `resolve()` adds a
|
||
cache keyed by `(Texture, ImageViewDesc)`; `ImageView` holds `Arc<Image>`
|
||
so lifetime is safe; resolve idempotence and atomic creation per D7.
|
||
- **Public API semantics:** `batches()` / `dependency_edges()` keep
|
||
command-space indices with the begin index as pass-node representative;
|
||
document the semantics and consider adding `execution_order()` as the
|
||
primary accessor.
|
||
- **`BindPipeline`:** out of scope for this plan; without it, submitted
|
||
tests cannot include real draws. If added later, it needs a sequence edge
|
||
to the following `Draw` (pipeline binding declares no resources, so the
|
||
hazard graph will not order it).
|