Skip to content

perf(test_apps): every Vulkan test app on dxr::VkFrameRing (INV-4.10) - #1945

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

dfattal merged 6 commits into
mainfrom
feat/cubes-vk-frame-ring

Conversation

@dfattal

@dfattal dfattal commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Why

INV-4.10 (#1942, merged) asks every Vulkan app for >= 2 frames in flight, ONE vkQueueSubmit per frame, never a wait on the frame just submitted, no vkQueueWaitIdle/vkDeviceWaitIdle on the frame path, command buffers allocated once. Every Vulkan test app broke it, and these cubes are what /new-displayxr-app clones, so every future app inherited the pattern.

On Android/Adreno a serialised submit-then-wait loop parks the GPU at about a third of its clock (#1917; displayxr-demo-modelviewer#170 went 13 -> 30 fps on this fix). On desktop it costs the CPU/GPU overlap. The shared fix ships in displayxr-common as dxr::VkFrameRing (common/vk_frame_ring.h, v2.28.0). This PR moves every flagged app onto it.

What changed

Pins. Every test app that fetches displayxr-common is on v2.29.0. That is v2.28.0 (the ring) plus dxr::CursorDepth. main moved test_apps/common to v2.29.0 (#1943) while this branch was open, so I matched it rather than leave the standalone apps one tag behind. This also gives cube_handle_metal_macos the cursor_depth.h it already includes, which main still pins at v2.27.0. The three GL apps pinned at v2.0.0 are untouched. The Android apps did not use displayxr-common before; they now fetch it sources-only with common/ appended last (the modelviewer pattern).

Every converted app:

  • Ring replaces the old scheme. One dxr::VkFrameRing per device/queue replaces the single command buffer and fence: default 3 in flight, DXR_VK_FRAMES_IN_FLIGHT overrides, and Android passes 3 explicitly.
  • Frame shape: acquire -> beginFrame() -> record every tile/eye -> endFrame() (one submit, no wait) -> xrReleaseSwapchainImage -> xrEndFrame.
  • Depth dependency. Each render pass gains a VK_SUBPASS_EXTERNAL dependency. Overlapping frames now write the one shared depth image, and the per-frame fence wait used to order that implicitly.
  • MVPs stay push constants. No per-frame host-written UBO needed viewUbo().
  • Teardown. waitAll() runs before resources are freed, and ring.destroy() before vkDestroyDevice, including on the renderer-init failure path.
App Change
cube_handle_vk_android, cube_zones_vk_android record_atlas() no longer does alloc/submit/vkQueueWaitIdle/free each frame. HUD vertex data the CPU rewrites each frame gets one region per ring slot. zones: the static Local2D band is uploaded once per swapchain image through submitImmediate(), not with a blocking submit every frame.
cube_handle_vk_win RenderScene no longer waits its own fence. The HUD and Local2D panel uploads go through submitImmediate() (no alloc/free/wait-idle); hudCmdPool removed.
cube_zones_vk_win, cube_zones_texture_vk_win All zone layers record into one ring frame (was one submit+wait per zone); zone swapchains are released after the submit. Strip/BGRA uploads go through submitImmediate(). texture: waitAll() before CleanupZones; the shared-texture D3D11 present path is unchanged.
cube_handle_vk_linux, cube_zones_vk_linux Ring; zones: one frame for every zone. The strip clear goes through submitImmediate().
cube_handle_vk_macos, cube_zones_vk_macos Ring (was a single fence, so at most one frame in flight); zones: one frame for every zone. The HUD goes through submitImmediate() and is re-uploaded only when its text changes (2 Hz); an unchanged HUD reuses the swapchain's last released image.
cube_hosted_legacy_vk_{win,linux,macos} RenderScene no longer waits its own fence: the runtime composes on the same queue, so queue order is what makes the image ready.
weave_client_vk_android Ring; present_woven() passes its semaphores through endFrame(); input upload and the debug dump use submitImmediate(). One per-frame waitAll() remains on purpose (commented): the runtime rewrites the single shared woven output on every xrWeaveSubmitDXR and Android has no acquire fence. CPU paint now overlaps the previous blit.

Lint (scripts/check_displayxr_app.py, INV-4.10 rule from #1942)

App Before After
cube_handle_vk_android 0E / 3W (2x INV-4.10) 0E / 1W (INV-9.1)
cube_zones_vk_android 0E / 3W (2x INV-4.10) 0E / 1W (INV-9.1)
cube_handle_vk_win 0E / 4W (3x INV-4.10) 0E / 1W
cube_zones_vk_win 0E / 2W (2x INV-4.10) OK, no issues
cube_zones_texture_vk_win 1E / 3W (2x INV-4.10) 1E / 1W
cube_handle_vk_linux 0E / 1W (1x INV-4.10) OK, no issues
cube_zones_vk_linux 0E / 2W (2x INV-4.10) OK, no issues
cube_hosted_legacy_vk_win 1E / 2W (1x INV-4.10) 1E / 1W
cube_hosted_legacy_vk_linux 0E / 2W (1x INV-4.10) 0E / 1W
cube_hosted_legacy_vk_macos 1E / 2W (1x INV-4.10) 1E / 1W
weave_client_vk_android 0E / 2W (1x INV-4.10) 0E / 1W
cube_handle_vk_macos 1E / 1W + INFO INV-4.10 1E / 1W
cube_zones_vk_macos 1E / 0W + INFO INV-4.10 1E / 0W

Zero INV-4.10 findings remain, and no app has more errors than before. The remaining findings were all there before: INV-5.9 (enable1), INV-3.1, INV-9.1 (no manifest). weave_present_vk_linux and weave_probe_vk_linux had no INV-4.10 findings and are untouched.

Verification

  • macOS, local run (sim-display, MoltenVK, M1 Pro). I ran cube_handle_vk_macos, cube_zones_vk_macos and cube_hosted_legacy_vk_macos for about 12 s each via _package/DisplayXR-macOS/run_cube_*_vk.sh:
    • Each entered the main loop, presented frames and shut down cleanly.
    • No Vulkan errors and no endFrame failures in the logs.
    • The vk_native atlas trigger ($TMPDIR/displayxr_atlas_trigger) was consumed each time, and each PNG shows the rendered crate and grid: handle 2-view atlas; zones per-zone tiles on one run, the base projection on another; legacy SBS.
    • Rebuilt and re-run after moving to v2.29.0.
    • The Khronos validation layer failed to load in this environment, so there is no sync-validation evidence.
  • Android: all three built locally with Gradle + NDK 26.3. cube_handle_vk_android did a full assembleDebug (on v2.28.0, before the v2.29.0 move); zones and weave did externalNativeBuildDebug on v2.29.0. CI builds only cube_handle_vk_android.
  • Windows: MinGW -fsyntax-only on every changed .cpp; the TestApps job is the real gate.
  • Linux: clang -fsyntax-only with the Linux-only binding headers stubbed; the --apps lane is the real gate.

Not verified on an Android device. The fps effect is measured separately (no adb was used for this PR).

Follow-ups / not done

  • cube_handle_vk_win HUD: it still uploads with one blocking submitImmediate() per frame while the HUD is visible (it is hidden by default). Its perf text changes every frame and it has a single staging buffer, so gating on change (as on macOS) wasn't trivial. Per-slot staging recorded into the frame would remove it.
  • weave_client_vk_android: the per-frame waitAll() stays until Android gets a sync_file-style acquire fence for the woven output (weave spec v10 is desktop-Linux only).
  • cube_zones_vk_android Local2D band: the once-per-image upload assumes the runtime keeps a Local2D image's contents between acquires. Check this on hardware; if the band blanks, go back to uploading every frame.
  • Init-time loaders: these still vkQueueWaitIdle (one-off; not flagged).
  • Linter gap: check_displayxr_app.py walks the gitignored Android .cxx/ tree. After a local build it reports false INV-3.1 errors from displayxr-common's tests/; it should skip .cxx.
  • No git clang-format: no .clang-format covers test_apps/, so it would apply the LLVM fallback style against these files' own style. Changes are hand-formatted to match each file.

🤖 Generated with Claude Code

@dfattal
dfattal force-pushed the feat/cubes-vk-frame-ring branch from 77383b6 to 23363ff Compare October 11, 2026 09:47
@dfattal
dfattal marked this pull request as ready for review October 11, 2026 10:00
@dfattal
dfattal requested a review from a team as a code owner October 11, 2026 10:00
@dfattal
dfattal force-pushed the feat/cubes-vk-frame-ring branch from 856ff4a to 1302676 Compare October 11, 2026 18:28
dfattal and others added 6 commits October 11, 2026 11:40
v2.28.0 adds common/vk_frame_ring.h — the shared N-frames-in-flight,
one-submit-per-frame Vulkan ring (INV-4.10, #1917). Every test app that
fetches displayxr-common at v2.27.0 moves to it; the three GL apps pinned
at v2.0.0 are left alone (no Vulkan, and a 28-minor jump is out of scope).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…4.10)

cube_handle_vk_{linux,macos} and cube_zones_vk_{linux,macos} submitted,
then waited the fence they had just submitted (or waited a single fence at
the top of the next frame): zero / one frame in flight, no CPU-GPU overlap.
They are what /new-displayxr-app clones, so every new app inherited it.

- The single command buffer + frame fence become a dxr::VkFrameRing
  (displayxr-common v2.28.0; default 3 in flight, DXR_VK_FRAMES_IN_FLIGHT
  overrides). Per frame: beginFrame() -> record -> endFrame() (ONE submit,
  no wait) -> xrReleaseSwapchainImage.
- Zones: every zone's pass of a frame is recorded into the one ring frame;
  all zone swapchains are released after the single submit.
- Each render pass gains a VK_SUBPASS_EXTERNAL dependency: the shared depth
  image is now written by overlapping frames, which the per-frame fence wait
  used to order implicitly.
- MVPs stay push constants (no per-frame host-written UBO to protect).
- macOS HUD: uploads go through ring.submitImmediate() (no per-call
  alloc/free/vkQueueWaitIdle) and only when the HUD text changes; an
  unchanged HUD reuses the swapchain's last released image.
- Linux zones: the strip clear goes through submitImmediate().
- Teardown destroys the ring before the device, including on the
  renderer-init failure path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cube_handle_vk_win, cube_zones_vk_win and cube_zones_texture_vk_win waited
the fence of the frame they had just submitted (one submit + wait per ZONE
in the zones apps), and cube_handle_vk_win's HUD upload allocated/freed a
command buffer and vkQueueWaitIdle'd on the frame path.

- Single command buffer + frame fence -> dxr::VkFrameRing (default 3 in
  flight). Per frame: beginFrame() -> record -> endFrame() (ONE submit, no
  wait) -> release swapchain image(s) -> xrEndFrame.
- Zones / texture zones: all zone layers record into the one ring frame;
  zone swapchains are released after the single submit.
- Render passes gain a VK_SUBPASS_EXTERNAL dependency for the shared depth
  image (overlapping frames now write it).
- One-off uploads (HUD, Local2D panel, zone strip, BGRA uploads) go through
  ring.submitImmediate(); cube_handle_vk_win's now-unused hudCmdPool is gone.
- texture zones: ring.waitAll() before CleanupZones at shutdown (it relied on
  every frame being synchronous). The shared-texture D3D11 present path is
  unchanged — the app never reads its own Vulkan output.
- MVPs stay push constants.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
main moved test_apps/common to v2.29.0 (dxr::CursorDepth, #1943) while this
branch was open; v2.29.0 is v2.28.0 (dxr::VkFrameRing) plus that one
additive helper. Keep every test app on the same tag rather than leave the
standalone-fetching apps one tag behind. This also gives
cube_handle_metal_macos the cursor_depth.h it already includes (main pins it
at v2.27.0, which has no such header).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…1917)

cube_handle_vk_android and cube_zones_vk_android allocated a command buffer,
submitted, vkQueueWaitIdle'd and freed it in record_atlas() every frame
("no per-frame fence wired yet") — the serialised submit->wait loop that
parks an Adreno GPU at ~1/3 clock (#1917; modelviewer#170 went 13 -> 30 fps
on the same fix).

- displayxr-common v2.29.0 fetched sources-only (its STATIC lib carries
  desktop sources), common/ appended last to the include path — the
  modelviewer pattern.
- One dxr::VkFrameRing (framesInFlight = 3 explicitly; Android has no env):
  beginFrame() -> record every tile -> endFrame() (ONE submit, no wait) ->
  release swapchain image.
- Render pass gains a VK_SUBPASS_EXTERNAL depth dependency (the one atlas
  depth image is now written by overlapping frames).
- CPU-rewritten HUD vertex data gets one region per ring slot (fallback HUD
  buffer sized by framesInFlight; HudFont glyph/panel buffers by
  HUD_FONT_MAX_SLOTS) so a frame in flight never reads the next frame's
  vertices.
- zones: the static Local2D band upload goes through submitImmediate() once
  per swapchain image (uploaded_mask, reset on swapchain create/destroy)
  instead of a blocking submit every frame.
- Teardown: waitAll() before freeing app resources, ring.destroy() before
  the device. Startup-only uploads in crate_scene / hud_font unchanged.

Built locally with Gradle + NDK 26.3 (no device touched).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dxr::VkFrameRing

Same INV-4.10 treatment for the small apps — even tiny ones use the shared
helper rather than a second hand-rolled fence scheme.

- cube_hosted_legacy_vk_{win,linux,macos}: RenderScene() submitted then
  waited its own fence ("runtime needs the image ready"). The runtime
  composes on the same queue, so queue order — not a CPU wait — is what
  makes the image ready. Now beginFrame()/endFrame() (default 3 in flight),
  a VK_SUBPASS_EXTERNAL depth dependency for the shared depth image, and
  ring teardown before the device (incl. the renderer-init failure path).
- weave_client_vk_android: the command pool/buffer/fence become a ring;
  present_woven() is beginFrame()/endFrame() with the acquire/present
  semaphores passed through endFrame() — no wait on the frame just
  submitted. upload_input() and the debug dump use submitImmediate(). One
  waitAll() per frame REMAINS, deliberately and commented: the runtime
  rewrites the single shared woven output on every xrWeaveSubmitDXR and
  Android has no acquire fence, so the previous present blit must finish
  first. CPU painting now overlaps that blit; true overlap needs a
  sync_file-style acquire fence on Android (weave spec v10 is desktop-Linux
  only).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@dfattal
dfattal force-pushed the feat/cubes-vk-frame-ring branch from 1302676 to 517375c Compare October 11, 2026 18:42
@dfattal
dfattal merged commit 37b82e6 into main Oct 11, 2026
36 checks passed
@dfattal
dfattal deleted the feat/cubes-vk-frame-ring branch October 11, 2026 18:55
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