Skip to content

feat(vk): shared Vulkan frame ring (vk_frame_ring.h) + FencedStage reuses its command buffer - #74

Merged
dfattal merged 2 commits into
mainfrom
feat/vk-frame-ring
Oct 11, 2026
Merged

dfattal merged 2 commits into
mainfrom
feat/vk-frame-ring

Conversation

@dfattal

@dfattal dfattal commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Problem

On Android (Adreno 740, msm-adreno-tz governor) the GPU clock follows GPU busy %. An app that submits → waits the fence → submits again, per frame or per tile, leaves the GPU idle while the CPU waits. The governor reads ~50 % busy and parks the GPU at ~1/3 clock (220 MHz), which makes each frame slower, which keeps it idle: ~10 fps. displayxr-demo-modelviewer #170 (b060835) fixed this with a 3-deep command-buffer/fence/UBO ring, ONE vkQueueSubmit per frame, a memory barrier between tiles, and endFrame() before xrReleaseSwapchainImage: 13 → 30 fps.

The rule: keep ≥ 2 frames in flight, make one submit per frame, never wait the fence of the frame just submitted, and never allocate command buffers per frame.

displayxr-common has had no shared helper, so the rest of the ecosystem still gets it wrong:

App Vulkan frame pattern today
modelviewer (post-#170) OK: 3-deep ring, one submit per frame
earthview 2 submits per eye + vkQueueWaitIdle per eye + a command-buffer alloc per eye
mediaplayer waits its own fence right after submitting
avatar (Android) never calls beginFrame → ring overflow → vkQueueWaitIdle
gaussiansplat ring is per-view, not per-frame
runtime cube_*_vk_android command-buffer alloc + vkQueueWaitIdle every frame

The only Vulkan sync code here, vk_overlay_kit.h's detail::FencedStage, had one slot and allocated and freed a command buffer on every upload. That is the same per-frame Adreno cost runtime #1936 just removed from the compositor.

What this PR adds

common/vk_frame_ring.h: dxr::VkFrameRing. Header-only, C++17, no Vulkan link dependency (it calls the loader-exported vk* symbols the same way vk_overlay_kit.h does).

  • N slots, default 3. Config::framesInFlight sets it explicitly (the Android path). The env var DXR_VK_FRAMES_IN_FLIGHT overrides it for A/B tests, clamped to 1..8; 1 reproduces the anti-pattern.
  • Allocated once at init: a RESET-capable pool, N command buffers and N fences, plus one transient command buffer and fence for submitImmediate.
  • Optional UBO: one persistently-mapped HOST_VISIBLE|HOST_COHERENT buffer, sliced slot × view at minUniformBufferOffsetAlignment. Because the slots are disjoint ranges of one buffer, a single UNIFORM_BUFFER_DYNAMIC descriptor set serves every slot.
  • beginFrame() waits only the fence of the slot's previous use (N frames ago), runs that slot's deferred deletions, then resets and begins the command buffer.
  • endFrame(...) ends the command buffer and makes exactly one vkQueueSubmit, with no wait. It returns the VkResult. A begun frame is always submitted, even an empty one. Call it before xrReleaseSwapchainImage.
  • tileBarrier() is the full ALL_COMMANDS memory barrier from model_renderer.cpp. colorAttachmentBarrier() is a narrower colour-only variant.
  • Lifetime and blocking calls: deferDestroy(fn), waitAll() (for resize and teardown), submitImmediate(record) (the only sanctioned blocking submit), destroy() / RAII.
  • The header doc block covers the DVFS rationale, the rule, a usage skeleton (acquire → beginFrame → tiles + tileBarrier → endFrame → release → xrEndFrame) and a do-NOT list.

vk_overlay_kit.h: FencedStage now allocates its command buffer once and calls vkResetCommandBuffer on reuse (the pool already had RESET_COMMAND_BUFFER_BIT). The public API is unchanged. The header comment no longer claims "the runtime's per-frame commit already drains the queue"; that stopped being true with runtime #1923 / #1936.

Wire-up: the header is added to DISPLAYXR_COMMON_SOURCES and is reachable through the displayxr::rules INTERFACE target (it exposes common/), with a comment there. The README gets rows for vk_frame_ring.h and vk_overlay_kit.h (the latter was missing) and a Vulkan frame discipline section. VERSION is set to 2.28.0, following the file's "tracks the NEXT tag" convention (latest tag is v2.27.0).

Test evidence

  • tests/vk_frame_ring_test.cpp covers the ring schedule for N = 1..8 (the fence waited at frame k is frame k−N's, and never frame k−1's when N ≥ 2), env parsing and clamping, alignUp, UBO slice offsets (aligned, disjoint, in bounds) and DeferredQueue order (FIFO, no re-entrancy).
    • Bare CI runners have no Vulkan SDK, so the pure helpers build with DXR_VK_FRAME_RING_PURE_ONLY and run there.
    • When the Vulkan headers are found, the test also compile-checks the full class and vk_overlay_kit.h, and static_asserts the public signatures.
  • Local results on macOS:
    • ctest: 6/6 pass, both with Vulkan headers (brew vulkan-headers) and in the pure-only build.
    • A throwaway GPU run outside the repo, on MoltenVK with the Khronos validation layer forced on, reported zero validation messages. It covered 30 frames × 4 tiles with UBO slices, tile and colour barriers and deferDestroy: 27 deletions ran before waitAll (30 − N) and 30 after. It also covered submitImmediate, an empty frame, a double endFrame (returns VK_NOT_READY), DXR_VK_FRAMES_IN_FLIGHT=1, and 5 consecutive CachedLayerUploader uploads reusing the one command buffer.
  • Not tested: an actual Adreno device. The fps claim comes from modelviewer #170, not from this header.

Follow-ups (not in this PR)

  1. Runtime: an INV rule in docs/guides/displayxr-app-rules.md plus a check_displayxr_app.py lint (flag vkQueueWaitIdle / per-frame vkAllocateCommandBuffers on the frame path), then convert the cube_*_vk_* test apps.
  2. Demo adoptions: earthview, mediaplayer, avatar, gaussiansplat; then modelviewer, from its private ring to this header.

🤖 Generated with Claude Code

dfattal and others added 2 commits October 11, 2026 01:41
…t per use

FencedStage allocated and freed a command buffer on every upload/readback —
the per-frame allocate/free cost on Adreno that runtime #1936 removed from the
compositor. The stage now allocates ONE command buffer at init (the pool
already had RESET_COMMAND_BUFFER_BIT) and vkResetCommandBuffer()s it in
beginCmd(). Public API (CachedLayerUploader, PipelinedReadback) unchanged.

Also drop the header's claim that the runtime's per-frame commit drains the
queue: it no longer does (runtime #1923 / #1936).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e (vk_frame_ring.h)

dxr::VkFrameRing is the generic half of displayxr-demo-modelviewer #170's
fix (13 -> 30 fps on Adreno 740): N slots (default 3, DXR_VK_FRAMES_IN_FLIGHT
1..8 for A/B), command buffers + fences allocated once, an optional
persistently-mapped dynamic-offset UBO sliced per slot x view, tile barriers,
per-slot deferred deletion, waitAll(), and submitImmediate() as the one named
blocking escape hatch. endFrame() makes exactly one vkQueueSubmit and never
waits.

Header-only, no Vulkan link dependency; reachable through displayxr::rules
for the Android legs. tests/vk_frame_ring_test.cpp checks the ring
arithmetic, env clamp, UBO slice alignment and deferred-deletion order
GPU-free (pure helpers compile without the Vulkan SDK), and compile-checks
the full class when Vulkan headers are present. VERSION -> 2.28.0 (tracks
the next tag).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@dfattal
dfattal merged commit 5caea09 into main Oct 11, 2026
6 checks passed
@dfattal
dfattal deleted the feat/vk-frame-ring branch October 11, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant