diff --git a/CMakeLists.txt b/CMakeLists.txt index 277f3da..c848485 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.21) # VERSION tracks the NEXT git tag. The tags are the version of record (consumers # pin GIT_TAG, nothing reads this field) — it had drifted to 0.4.0 against v2.13.0. project(displayxr_common - VERSION 2.27.0 + VERSION 2.28.0 DESCRIPTION "Shared off-axis Kooima projection math + C++ app scaffolding for DisplayXR 3D displays" LANGUAGES C CXX) @@ -266,6 +266,7 @@ set(DISPLAYXR_COMMON_SOURCES common/auto_fit_canvas.h # header-only runtime-canvas viewport source + aspect gate common/frame_stage_timer.h # header-only env-gated per-stage frame timing (#837) common/vk_overlay_kit.h # header-only VK cached-layer-upload + pipelined-readback kit (#837) + common/vk_frame_ring.h # header-only VK frame ring: N in flight, one submit/frame (Adreno DVFS) common/vk_clickthrough_region.h # header-only coverage->SetWindowRgn punch-through (#833, Win32) common/win_window_drag.h # header-only RMB window-drag for borderless overlays (Win32) common/launch_args.h # header-only viewer launch contract: CLI flags + displayxr-view: URL @@ -330,6 +331,12 @@ endif() # the include path (, and ideally the consumer's # XR_DXR_display_info.h) — which every leg that calls OpenXR already has — so # this target still adds no dependency of its own. +# +# common/vk_frame_ring.h (and vk_overlay_kit.h) are reachable through this +# target too — the Android Vulkan legs are exactly the consumers that need the +# frame ring most (Adreno DVFS, modelviewer #170). They need +# on the include path and the consumer's existing Vulkan link; nothing is added +# here. add_library(displayxr_rules INTERFACE) add_library(displayxr::rules ALIAS displayxr_rules) target_include_directories(displayxr_rules INTERFACE diff --git a/README.md b/README.md index f4bc8d9..403097f 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,8 @@ The `common/` directory is the lib's second target (epic #396 W4, re-scoped [#39 | Content-bounds ROI for the rear-depth budget: project a world-space AABB to a canvas-normalised rect, rebase a zoned app's rect into window space, chain it as `XrContentBoundsDXR` | `content_bounds.h` | both | | Content-MASK ROI for the rear-depth budget: any-coverage downsample of the app's own silhouette/alpha coverage onto the extension's cell grid (whole-window or zone-placed), union, cell count, chain it as `XrContentMaskDXR` | `content_mask.h` | both | | View-configuration opt-in: `DxrSelectViewConfigType()` — begin the session with `PRIMARY_MULTIVIEW_DXR` when the runtime advertises it; `DxrAliasInactiveViews()` — submit every located view, aliasing the ones the active mode does not use (ADR-041) | `dxr_view_config.h` | both (also `displayxr::rules`, so Linux/Android legs get it) | +| Vulkan **frame ring**: N frames in flight (default 3), ONE `vkQueueSubmit` per frame, per-slot command buffer + fence + dynamic-offset UBO slice allocated once, tile barriers, deferred deletion, one named blocking escape hatch (`submitImmediate`) — see *Vulkan frame discipline* below | `vk_frame_ring.h` | all (also `displayxr::rules`, so Linux/Android legs get it; needs ``) | +| Vulkan overlay kit: content-hash-gated layer uploads (`CachedLayerUploader`) + fence-pipelined readbacks (`PipelinedReadback`), each with ONE reused command buffer ([runtime#837](https://github.com/DisplayXR/displayxr-runtime/issues/837)) | `vk_overlay_kit.h` | all (needs ``) | **Divergence policy:** behavior differences between consumers are parameterized at the call site (e.g. `InputState::hudToggleRequiresShift`, `EndFrame(..., projectionLayerFlags)`) — never `#ifdef APP` in the lib. Request-flag fields that only one app consumes (file picker, clip playback, transparency toggle) are fine: unconsumed flags are inert. @@ -204,6 +206,45 @@ The type value comes from the consumer's `XR_DXR_display_info.h` when one is on compiles everywhere. `tests/view_config_test.c` + `view_config_test_cxx.cpp` pin that behaviour, and the aliasing contract (C11 and C++17, GPU- and loader-free — they script a fake `xrEnumerateViewConfigurations`). +### Vulkan frame discipline (`vk_frame_ring.h`) + +**The rule:** keep at least two frames in flight, make ONE `vkQueueSubmit` per frame (every tile/view of +the atlas in one command buffer, ordered by a barrier between tiles — not by a CPU wait), never wait the +fence of the frame you just submitted, and never allocate command buffers per frame. Submit **before** +`xrReleaseSwapchainImage` — the runtime composes on the same `VkQueue`. + +**Why:** mobile GPU governors (Adreno's `msm-adreno-tz`) clock the GPU from its busy %. Submit → wait → +submit leaves it idle half the time, so it parks at ~1/3 clock (measured: 220 MHz, ~50 % busy, ~10 fps on +an Adreno 740), which makes each frame slower, which keeps it idle. `displayxr-demo-modelviewer` went +13 → 30 fps with a 3-deep ring (PR #170); `dxr::VkFrameRing` is the generic half of that fix. + +```cpp +#include "vk_frame_ring.h" + +dxr::VkFrameRing ring; // once +dxr::VkFrameRing::Config cfg; +cfg.perViewUboBytes = sizeof(ViewUbo); // 0 = no UBO +cfg.maxViewsPerFrame = kMaxViews; +ring.init(device, physDevice, queue, queueFamily, cfg); + +// per frame, after xrAcquire/WaitSwapchainImage: +VkCommandBuffer cmd = ring.beginFrame(); // waits only the fence from N frames ago +for (uint32_t i = 0; i < viewCount; ++i) { + if (i > 0) dxr::VkFrameRing::tileBarrier(cmd); + uint32_t dynOff; + *static_cast(ring.viewUbo(i, &dynOff)) = ...; // slot-private slice + // bind with dynOff (UNIFORM_BUFFER_DYNAMIC on ring.uboBuffer()), record tile i +} +ring.endFrame(); // ONE submit, no wait +// xrReleaseSwapchainImage, xrEndFrame +``` + +Resources an in-flight frame may still read go through `ring.deferDestroy(fn)`; `ring.waitAll()` is for +resize/teardown; `ring.submitImmediate(record)` is the one sanctioned blocking submit (one-off uploads). +`DXR_VK_FRAMES_IN_FLIGHT=1..8` overrides the depth for A/B tests (`1` reproduces the anti-pattern); +Android has no usable environment, so pass `Config::framesInFlight` there. `tests/vk_frame_ring_test.cpp` +pins the ring arithmetic, env clamp, UBO slice alignment and deferred-deletion order GPU-free. + ### Color: honest sRGB by default (`color_policy.h`, ADR-021 / [#1589](https://github.com/DisplayXR/displayxr-runtime/issues/1589)) > **Behaviour change in `v2.15.0`.** Apps built against this lib now ask for an **`_SRGB`** diff --git a/common/vk_frame_ring.h b/common/vk_frame_ring.h new file mode 100644 index 0000000..a90057a --- /dev/null +++ b/common/vk_frame_ring.h @@ -0,0 +1,563 @@ +// Copyright 2026, DisplayXR +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Vulkan frame ring: N frames in flight, ONE vkQueueSubmit per frame, + * never a wait on the frame just submitted. + * + * Header-only (displayxr-common links no Vulkan; only VK apps include this — + * same convention as vk_overlay_kit.h: the loader-exported vk* symbols are + * called directly, so the consumer's existing Vulkan link satisfies them). + * + * ## Why + * + * Mobile GPU governors (Adreno's `msm-adreno-tz` is the measured case) pick + * the clock from GPU *busy %*. An app that submits, waits the fence, then + * submits again — per frame, or worse per TILE — leaves the GPU idle while + * the CPU waits, so the governor reads ~50 % busy and parks the GPU at about a + * third of its clock, which makes every frame slower, which keeps it idle. + * displayxr-demo-modelviewer went 13 → 30 fps on an Adreno 740 (PR #170) by + * keeping 3 frames in flight with one submit per frame; this header is the + * generic half of that fix. + * + * ## The rule + * + * 1. Keep >= 2 frames in flight (default 3). + * 2. ONE vkQueueSubmit per frame — all tiles/views of the atlas go into one + * command buffer, ordered by a barrier between tiles, not by a CPU wait. + * 3. Never wait the fence of the frame you just submitted. The only wait on + * the hot path is beginFrame() waiting the fence of the frame that used + * the same slot N frames ago — normally long signaled. + * 4. Never allocate command buffers per frame — the ring allocates N once and + * vkResetCommandBuffer()s them (per-frame vkAllocateCommandBuffers costs + * the Adreno driver a fresh zeroed allocation every time; runtime #1936). + * + * ## Usage skeleton (OpenXR projection layer, one atlas swapchain) + * @code + * dxr::VkFrameRing ring; + * dxr::VkFrameRing::Config cfg; + * cfg.perViewUboBytes = sizeof(MyViewUbo); // 0 = no UBO + * cfg.maxViewsPerFrame = kMaxViews; + * ring.init(device, physDevice, queue, queueFamily, cfg); + * // one descriptor set, binding = UNIFORM_BUFFER_DYNAMIC on ring.uboBuffer(), + * // range = ring.uboSliceBytes() + * + * // per frame: + * xrWaitFrame / xrBeginFrame / xrLocateViews + * xrAcquireSwapchainImage + xrWaitSwapchainImage + * VkCommandBuffer cmd = ring.beginFrame(); + * for (uint32_t i = 0; i < viewCount; ++i) { + * if (i > 0) dxr::VkFrameRing::tileBarrier(cmd); // shared targets + * uint32_t dynOff; + * auto* ubo = static_cast(ring.viewUbo(i, &dynOff)); + * *ubo = ...; // slot-private + * vkCmdBindDescriptorSets(cmd, ..., 1, &dynOff); + * ...record tile i into its atlas sub-rect... + * } + * VkResult r = ring.endFrame(); // ONE submit, no wait + * if (r != VK_SUCCESS) log(r); + * xrReleaseSwapchainImage // AFTER endFrame + * xrEndFrame + * @endcode + * + * ## Do NOT + * + * - vkQueueWaitIdle / vkDeviceWaitIdle / vkWaitForFences on the frame you + * just submitted, anywhere on the per-frame path. + * - Submit once per tile or per eye (with or without a wait in between). + * - vkAllocateCommandBuffers / vkFreeCommandBuffers per frame or per upload. + * - Write a host-visible buffer that an in-flight frame may still read — + * give each slot its own copy (viewUbo()), or waitAll() first. + * - Destroy a resource the GPU may still be reading — deferDestroy() it. + * - Release the swapchain image before endFrame(): the runtime composes on + * the same VkQueue, so the app's work must be queued ahead of it. + * - Block anywhere except submitImmediate() (the one named escape hatch for + * genuine one-off uploads) and waitAll() (resize / teardown). + * + * The frames-in-flight count can be overridden with the env var + * DXR_VK_FRAMES_IN_FLIGHT (clamped 1..8) for A/B tests — 1 reproduces the + * submit→wait anti-pattern. Android processes have no usable environment; + * Android callers pass Config::framesInFlight explicitly. + * + * Threading: single-threaded per instance (call from the render thread). The + * queue is the caller's; external synchronisation of it is the caller's too. + * + * Compile note: defining DXR_VK_FRAME_RING_PURE_ONLY before including this + * header compiles ONLY the Vulkan-free helpers in dxr::detail (used by the + * unit test on runners without the Vulkan SDK). + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace dxr { +namespace detail { + +//! Default frames in flight (modelviewer #170's measured value). +constexpr uint32_t kDefaultFramesInFlight = 3; +//! Upper clamp for frames in flight. +constexpr uint32_t kMaxFramesInFlight = 8; + +//! Round @p v up to a multiple of @p align (0 or 1 = no alignment; any other +//! value need not be a power of two, though Vulkan's always are). +inline uint64_t +alignUp(uint64_t v, uint64_t align) +{ + if (align <= 1) return v; + return ((v + align - 1) / align) * align; +} + +//! Clamp a frames-in-flight count to [1, kMaxFramesInFlight]; 0 = default. +inline uint32_t +clampFramesInFlight(uint32_t n) +{ + if (n == 0) return kDefaultFramesInFlight; + return n > kMaxFramesInFlight ? kMaxFramesInFlight : n; +} + +/*! + * Resolve the frames-in-flight count. @p env is the DXR_VK_FRAMES_IN_FLIGHT + * value (may be null): a decimal integer wins over @p configured, clamped to + * [1, 8]. Null / empty / non-numeric / 0 falls back to @p configured (itself + * 0 = default 3, else clamped). + */ +inline uint32_t +parseFramesInFlight(const char* env, uint32_t configured) +{ + if (env != nullptr && env[0] != 0) { + char* end = nullptr; + const long v = std::strtol(env, &end, 10); + if (end != env && *end == 0 && v > 0) { + return v > long(kMaxFramesInFlight) ? kMaxFramesInFlight : uint32_t(v); + } + } + return clampFramesInFlight(configured); +} + +//! Next slot index in an @p n-slot ring. +inline uint32_t +ringAdvance(uint32_t idx, uint32_t n) +{ + return n == 0 ? 0 : (idx + 1) % n; +} + +//! Byte offset of view @p view's slice in slot @p slot, in a UBO laid out as +//! slots × maxViews slices of @p stride bytes (stride already aligned). +inline uint64_t +uboSliceOffset(uint32_t slot, uint32_t view, uint32_t maxViews, uint64_t stride) +{ + return (uint64_t(slot) * maxViews + view) * stride; +} + +//! FIFO of deferred actions; run() drains in registration order. Actions +//! queued from inside an action run on the NEXT run(), never re-entrantly. +class DeferredQueue { +public: + void push(std::function fn) + { + if (fn) q_.push_back(std::move(fn)); + } + void run() + { + std::vector> todo; + todo.swap(q_); + for (auto& fn : todo) fn(); + } + size_t size() const { return q_.size(); } + bool empty() const { return q_.empty(); } + +private: + std::vector> q_; +}; + +} // namespace detail +} // namespace dxr + +#ifndef DXR_VK_FRAME_RING_PURE_ONLY + +#include + +namespace dxr { + +/*! + * N-slot command-buffer / fence / UBO ring. See the file comment for the rule + * and the usage skeleton. + */ +class VkFrameRing { +public: + struct Config { + //! Frames in flight; 0 = default (3). DXR_VK_FRAMES_IN_FLIGHT, when + //! set, overrides this (A/B knob). Clamped to 1..8. + uint32_t framesInFlight = 0; + //! Bytes of one view's UBO slice; 0 = the ring owns no UBO. + VkDeviceSize perViewUboBytes = 0; + //! UBO slices per slot (views/tiles per frame). Ignored without a UBO. + uint32_t maxViewsPerFrame = 16; + }; + + VkFrameRing() = default; + VkFrameRing(const VkFrameRing&) = delete; + VkFrameRing& operator=(const VkFrameRing&) = delete; + ~VkFrameRing() { destroy(); } + + //! Create the pool, N command buffers, N fences and the optional UBO, all + //! ONCE. Returns false (and frees anything created) on failure. + bool + init(VkDevice device, VkPhysicalDevice phys, VkQueue queue, uint32_t queueFamily, + const Config& cfg) + { + destroy(); + dev_ = device; + queue_ = queue; + n_ = detail::parseFramesInFlight(std::getenv("DXR_VK_FRAMES_IN_FLIGHT"), + cfg.framesInFlight); + slots_.assign(n_, Slot{}); + idx_ = n_ - 1; // first beginFrame() lands on slot 0 + + VkCommandPoolCreateInfo pi = {VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO}; + pi.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; + pi.queueFamilyIndex = queueFamily; + if (vkCreateCommandPool(dev_, &pi, nullptr, &pool_) != VK_SUCCESS) return fail(); + + // N frame command buffers + the submitImmediate() one, in one call. + std::vector cmds(n_ + 1, VK_NULL_HANDLE); + VkCommandBufferAllocateInfo ai = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO}; + ai.commandPool = pool_; + ai.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; + ai.commandBufferCount = n_ + 1; + if (vkAllocateCommandBuffers(dev_, &ai, cmds.data()) != VK_SUCCESS) return fail(); + for (uint32_t i = 0; i < n_; ++i) slots_[i].cmd = cmds[i]; + immCmd_ = cmds[n_]; + + VkFenceCreateInfo fi = {VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; + for (Slot& s : slots_) { + if (vkCreateFence(dev_, &fi, nullptr, &s.fence) != VK_SUCCESS) return fail(); + } + if (vkCreateFence(dev_, &fi, nullptr, &immFence_) != VK_SUCCESS) return fail(); + + if (cfg.perViewUboBytes > 0 && cfg.maxViewsPerFrame > 0) { + VkPhysicalDeviceProperties props; + vkGetPhysicalDeviceProperties(phys, &props); + uboSlice_ = cfg.perViewUboBytes; + uboStride_ = detail::alignUp(cfg.perViewUboBytes, + props.limits.minUniformBufferOffsetAlignment); + maxViews_ = cfg.maxViewsPerFrame; + if (!createUbo(phys, uboStride_ * maxViews_ * n_)) return fail(); + } + inited_ = true; + return true; + } + + /*! + * Advance to the next slot, wait ONLY that slot's fence (the frame that + * used it N frames ago) if still pending, run its deferred deletions, + * reset and begin its command buffer (ONE_TIME_SUBMIT). Returns the + * recording command buffer, or VK_NULL_HANDLE on failure. Calling it while + * a frame is already open returns the open frame's buffer unchanged. + */ + VkCommandBuffer + beginFrame() + { + if (!inited_) return VK_NULL_HANDLE; + if (open_) return slots_[idx_].cmd; + idx_ = detail::ringAdvance(idx_, n_); + Slot& s = slots_[idx_]; + waitSlot(s); + s.deferred.run(); + vkResetCommandBuffer(s.cmd, 0); + VkCommandBufferBeginInfo bi = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; + bi.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; + if (vkBeginCommandBuffer(s.cmd, &bi) != VK_SUCCESS) return VK_NULL_HANDLE; + open_ = true; + return s.cmd; + } + + //! The open frame's command buffer (VK_NULL_HANDLE outside begin/end). + VkCommandBuffer current() const { return open_ ? slots_[idx_].cmd : VK_NULL_HANDLE; } + + /*! + * This slot's private UBO slice for @p view (host-coherent, persistently + * mapped — write it, no flush needed). @p outDynamicOffset receives the + * dynamic offset to pass to vkCmdBindDescriptorSets for a + * UNIFORM_BUFFER_DYNAMIC binding on uboBuffer(). Returns null outside an + * open frame, without a UBO, or when @p view >= maxViewsPerFrame. + */ + void* + viewUbo(uint32_t view, uint32_t* outDynamicOffset) + { + if (!open_ || uboMapped_ == nullptr || view >= maxViews_) return nullptr; + const uint64_t off = detail::uboSliceOffset(idx_, view, maxViews_, uboStride_); + if (outDynamicOffset != nullptr) *outDynamicOffset = uint32_t(off); + return static_cast(uboMapped_) + off; + } + + //! The ring's single UBO buffer (all slots × views; slots are disjoint + //! ranges, so ONE descriptor set with a dynamic offset serves every slot). + VkBuffer uboBuffer() const { return ubo_; } + //! Descriptor range for the UBO binding (= Config::perViewUboBytes). + VkDeviceSize uboSliceBytes() const { return uboSlice_; } + //! Aligned distance between consecutive slices. + VkDeviceSize uboStride() const { return uboStride_; } + + uint32_t framesInFlight() const { return n_; } + uint32_t slotIndex() const { return idx_; } + bool frameOpen() const { return open_; } + bool initialized() const { return inited_; } + + /*! + * Full memory barrier, ALL_COMMANDS → ALL_COMMANDS (MEMORY_WRITE → + * MEMORY_READ|MEMORY_WRITE). Record it between tiles that share any + * intermediate target (colour / depth / MSAA / copy) — it is exactly the + * ordering the old per-tile CPU wait gave, without the CPU in the loop. + */ + static void + tileBarrier(VkCommandBuffer cmd) + { + VkMemoryBarrier mb = {VK_STRUCTURE_TYPE_MEMORY_BARRIER}; + mb.srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT; + mb.dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT; + vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, + VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 1, &mb, 0, nullptr, 0, + nullptr); + } + + /*! + * Narrower variant: colour-attachment writes → fragment-shader reads, + * transfer reads/writes, or the next tile's colour-attachment writes. Use + * it only when the sole shared hazard is a colour target; shared DEPTH + * targets or compute need tileBarrier(). + */ + static void + colorAttachmentBarrier(VkCommandBuffer cmd) + { + VkMemoryBarrier mb = {VK_STRUCTURE_TYPE_MEMORY_BARRIER}; + mb.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; + mb.dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_TRANSFER_READ_BIT | + VK_ACCESS_TRANSFER_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | + VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; + vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, + VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | + VK_PIPELINE_STAGE_TRANSFER_BIT | + VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, + 0, 1, &mb, 0, nullptr, 0, nullptr); + } + + /*! + * vkEndCommandBuffer + EXACTLY ONE vkQueueSubmit with the slot fence, and + * NO wait. Call BEFORE xrReleaseSwapchainImage. A begun frame is always + * submitted, even if nothing was recorded (an empty submit is cheap and + * keeps the fence bookkeeping uniform). Returns the submit's VkResult for + * the caller to log; on failure the slot is not marked pending (waiting a + * fence that was never submitted would hang). VK_NOT_READY if no frame is + * open. + */ + VkResult + endFrame(const VkSemaphore* waitSems = nullptr, const VkPipelineStageFlags* waitStages = nullptr, + uint32_t waitCount = 0, const VkSemaphore* signalSems = nullptr, + uint32_t signalCount = 0) + { + if (!open_) return VK_NOT_READY; + Slot& s = slots_[idx_]; + open_ = false; + VkResult r = vkEndCommandBuffer(s.cmd); + if (r != VK_SUCCESS) return r; + vkResetFences(dev_, 1, &s.fence); + VkSubmitInfo si = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + si.waitSemaphoreCount = waitCount; + si.pWaitSemaphores = waitSems; + si.pWaitDstStageMask = waitStages; + si.commandBufferCount = 1; + si.pCommandBuffers = &s.cmd; + si.signalSemaphoreCount = signalCount; + si.pSignalSemaphores = signalSems; + r = vkQueueSubmit(queue_, 1, &si, s.fence); + s.pending = (r == VK_SUCCESS); + return r; + } + + /*! + * Run @p fn once the GPU has finished every frame submitted so far — i.e. + * when the current slot's fence is next waited (N frames from now, or at + * waitAll()/destroy()). Use for buffers/images/descriptor sets an + * in-flight frame may still read. Valid inside or outside a frame. + */ + void + deferDestroy(std::function fn) + { + if (!inited_) { + if (fn) fn(); + return; + } + slots_[idx_].deferred.push(std::move(fn)); + } + + /*! + * Wait every pending fence and run all deferred deletions. For resize, + * teardown, or a host rewrite of a buffer shared by all frames. Blocks — + * never call it per frame. If a frame is open, its deferred list is left + * alone (the open command buffer may still reference those resources). + */ + void + waitAll() + { + if (!inited_) return; + for (uint32_t i = 0; i < n_; ++i) { + Slot& s = slots_[i]; + waitSlot(s); + if (!(open_ && i == idx_)) s.deferred.run(); + } + } + + /*! + * The ONLY sanctioned blocking submit: record a genuine one-off (an asset + * upload, a layout init) into the ring's dedicated transient command + * buffer (allocated once, reset per use), submit it alone, and wait its + * own fence. Never call it per frame. Independent of the open frame. + */ + VkResult + submitImmediate(const std::function& record) + { + if (!inited_) return VK_ERROR_INITIALIZATION_FAILED; + vkResetCommandBuffer(immCmd_, 0); + VkCommandBufferBeginInfo bi = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; + bi.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; + VkResult r = vkBeginCommandBuffer(immCmd_, &bi); + if (r != VK_SUCCESS) return r; + if (record) record(immCmd_); + r = vkEndCommandBuffer(immCmd_); + if (r != VK_SUCCESS) return r; + vkResetFences(dev_, 1, &immFence_); + VkSubmitInfo si = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; + si.commandBufferCount = 1; + si.pCommandBuffers = &immCmd_; + r = vkQueueSubmit(queue_, 1, &si, immFence_); + if (r != VK_SUCCESS) return r; + return vkWaitForFences(dev_, 1, &immFence_, VK_TRUE, UINT64_MAX); + } + + //! waitAll(), then free everything. Safe to call twice / uninitialised. + void + destroy() + { + if (dev_ == VK_NULL_HANDLE) return; + if (open_) { + // Close the recording so the pool can be destroyed cleanly; it is + // never submitted. + vkEndCommandBuffer(slots_[idx_].cmd); + open_ = false; + } + waitAll(); + for (Slot& s : slots_) { + s.deferred.run(); + if (s.fence != VK_NULL_HANDLE) vkDestroyFence(dev_, s.fence, nullptr); + } + if (immFence_ != VK_NULL_HANDLE) vkDestroyFence(dev_, immFence_, nullptr); + if (pool_ != VK_NULL_HANDLE) vkDestroyCommandPool(dev_, pool_, nullptr); // frees cmds + if (uboMem_ != VK_NULL_HANDLE) { + if (uboMapped_ != nullptr) vkUnmapMemory(dev_, uboMem_); + vkFreeMemory(dev_, uboMem_, nullptr); + } + if (ubo_ != VK_NULL_HANDLE) vkDestroyBuffer(dev_, ubo_, nullptr); + slots_.clear(); + dev_ = VK_NULL_HANDLE; + queue_ = VK_NULL_HANDLE; + pool_ = VK_NULL_HANDLE; + immCmd_ = VK_NULL_HANDLE; + immFence_ = VK_NULL_HANDLE; + ubo_ = VK_NULL_HANDLE; + uboMem_ = VK_NULL_HANDLE; + uboMapped_ = nullptr; + uboSlice_ = uboStride_ = 0; + maxViews_ = n_ = idx_ = 0; + open_ = inited_ = false; + } + +private: + struct Slot { + VkCommandBuffer cmd = VK_NULL_HANDLE; + VkFence fence = VK_NULL_HANDLE; + bool pending = false; // submitted, fence not yet observed + detail::DeferredQueue deferred; + }; + + void + waitSlot(Slot& s) + { + if (s.pending) { + vkWaitForFences(dev_, 1, &s.fence, VK_TRUE, UINT64_MAX); + s.pending = false; + } + } + + bool + fail() + { + inited_ = true; // let destroy() walk what was created + destroy(); + return false; + } + + bool + createUbo(VkPhysicalDevice phys, VkDeviceSize bytes) + { + VkBufferCreateInfo bi = {VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO}; + bi.size = bytes; + bi.usage = VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT; + bi.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + if (vkCreateBuffer(dev_, &bi, nullptr, &ubo_) != VK_SUCCESS) return false; + VkMemoryRequirements mr; + vkGetBufferMemoryRequirements(dev_, ubo_, &mr); + VkPhysicalDeviceMemoryProperties mp; + vkGetPhysicalDeviceMemoryProperties(phys, &mp); + const VkMemoryPropertyFlags host = + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT; + // Prefer device-local host-visible (unified memory on mobile, BAR on + // desktop), else any host-visible coherent type. + uint32_t type = UINT32_MAX; + const VkMemoryPropertyFlags wants[2] = {host | VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, host}; + for (VkMemoryPropertyFlags want : wants) { + for (uint32_t i = 0; i < mp.memoryTypeCount && type == UINT32_MAX; ++i) { + if ((mr.memoryTypeBits & (1u << i)) && + (mp.memoryTypes[i].propertyFlags & want) == want) { + type = i; + } + } + if (type != UINT32_MAX) break; + } + if (type == UINT32_MAX) return false; + VkMemoryAllocateInfo ai = {VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO}; + ai.allocationSize = mr.size; + ai.memoryTypeIndex = type; + return vkAllocateMemory(dev_, &ai, nullptr, &uboMem_) == VK_SUCCESS && + vkBindBufferMemory(dev_, ubo_, uboMem_, 0) == VK_SUCCESS && + vkMapMemory(dev_, uboMem_, 0, VK_WHOLE_SIZE, 0, &uboMapped_) == VK_SUCCESS; + } + + VkDevice dev_ = VK_NULL_HANDLE; + VkQueue queue_ = VK_NULL_HANDLE; + VkCommandPool pool_ = VK_NULL_HANDLE; + std::vector slots_; + uint32_t n_ = 0; + uint32_t idx_ = 0; + bool open_ = false; + bool inited_ = false; + + VkCommandBuffer immCmd_ = VK_NULL_HANDLE; + VkFence immFence_ = VK_NULL_HANDLE; + + VkBuffer ubo_ = VK_NULL_HANDLE; + VkDeviceMemory uboMem_ = VK_NULL_HANDLE; + void* uboMapped_ = nullptr; + VkDeviceSize uboSlice_ = 0; + VkDeviceSize uboStride_ = 0; + uint32_t maxViews_ = 0; +}; + +} // namespace dxr + +#endif // DXR_VK_FRAME_RING_PURE_ONLY diff --git a/common/vk_overlay_kit.h b/common/vk_overlay_kit.h index 4d61cbe..e82c01b 100644 --- a/common/vk_overlay_kit.h +++ b/common/vk_overlay_kit.h @@ -15,8 +15,14 @@ * queued GPU work into the app thread. * * The shared discipline: submit with a fence, never wait on the hot path — - * consume/reuse on the NEXT invocation, by which time the fence is long - * signaled (the runtime's per-frame commit already drains the queue). + * consume/reuse on the NEXT invocation. Do NOT assume the runtime drains the + * queue for you: it used to vkQueueWaitIdle at every commit, but no longer + * does (runtime #1923 / #1936), so the fence of the previous use may really + * still be pending — the wait is ~free only because a frame or more has + * passed. Each stage allocates its command buffer ONCE and resets it per use + * (per-use allocate/free is a measurable per-frame cost on Adreno — the cost + * runtime #1936 removed from the compositor). For the app's main render + * submit, see vk_frame_ring.h. * * dxr::CachedLayerUploader — window-space / Local2D layer image uploads, * gated on a caller-provided content hash: unchanged hash = complete @@ -128,8 +134,8 @@ FindHostVisibleType(VkPhysicalDevice phys, uint32_t typeBits, bool forCpuReads = return UINT32_MAX; } -//! Shared plumbing: a mapped host-visible buffer + own cmd pool + one fence + -//! the lazily-retired previous command buffer. +//! Shared plumbing: a mapped host-visible buffer + own cmd pool + ONE command +//! buffer (allocated once, reset per use) + one fence. struct FencedStage { VkDevice dev = VK_NULL_HANDLE; VkBuffer buf = VK_NULL_HANDLE; @@ -138,7 +144,7 @@ struct FencedStage { VkDeviceSize bytes = 0; VkCommandPool pool = VK_NULL_HANDLE; VkFence fence = VK_NULL_HANDLE; - VkCommandBuffer prevCmd = VK_NULL_HANDLE; + VkCommandBuffer cmd = VK_NULL_HANDLE; // allocated once in init() bool pending = false; //! Did the allocation actually land on a HOST_CACHED type? Read it back and //! LOG it — "we asked for cached" is not the same claim as "the driver had @@ -181,13 +187,18 @@ struct FencedStage { pi.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; pi.queueFamilyIndex = queueFamily; if (vkCreateCommandPool(dev, &pi, nullptr, &pool) != VK_SUCCESS) return false; + VkCommandBufferAllocateInfo cai = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO}; + cai.commandPool = pool; + cai.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; + cai.commandBufferCount = 1; + if (vkAllocateCommandBuffers(dev, &cai, &cmd) != VK_SUCCESS) return false; VkFenceCreateInfo fi = {VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; return vkCreateFence(dev, &fi, nullptr, &fence) == VK_SUCCESS; } - //! Wait the in-flight submission (if any) and retire its command buffer. - //! Called at the START of the next use — by then the fence is long - //! signaled, so the wait is ~free. + //! Wait the in-flight submission (if any) so the staging buffer and the + //! command buffer may be reused. Called at the START of the next use — + //! normally a frame or more later, so the wait is ~free. void retire() { @@ -195,45 +206,33 @@ struct FencedStage { vkWaitForFences(dev, 1, &fence, VK_TRUE, UINT64_MAX); pending = false; } - if (prevCmd != VK_NULL_HANDLE) { - vkFreeCommandBuffers(dev, pool, 1, &prevCmd); - prevCmd = VK_NULL_HANDLE; - } } - //! Submit @p cmd on @p queue with the fence; the cmd retires on the next - //! retire() call. Returns false (and frees the cmd) on submit failure. + //! Submit the recorded command buffer (@p recorded must be the one + //! beginCmd() returned) on @p queue with the fence; no wait. Returns false + //! on submit failure (nothing is pending then). bool - submit(VkQueue queue, VkCommandBuffer cmd) + submit(VkQueue queue, VkCommandBuffer recorded) { VkSubmitInfo si = {VK_STRUCTURE_TYPE_SUBMIT_INFO}; si.commandBufferCount = 1; - si.pCommandBuffers = &cmd; + si.pCommandBuffers = &recorded; vkResetFences(dev, 1, &fence); - if (vkQueueSubmit(queue, 1, &si, fence) != VK_SUCCESS) { - vkFreeCommandBuffers(dev, pool, 1, &cmd); - return false; - } + if (vkQueueSubmit(queue, 1, &si, fence) != VK_SUCCESS) return false; pending = true; - prevCmd = cmd; return true; } + //! Reset + begin the stage's one command buffer. Call retire() first (the + //! buffer must not be pending). VK_NULL_HANDLE on failure. VkCommandBuffer beginCmd() { - VkCommandBufferAllocateInfo ai = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO}; - ai.commandPool = pool; - ai.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; - ai.commandBufferCount = 1; - VkCommandBuffer cmd = VK_NULL_HANDLE; - if (vkAllocateCommandBuffers(dev, &ai, &cmd) != VK_SUCCESS) return VK_NULL_HANDLE; + if (cmd == VK_NULL_HANDLE || pending) return VK_NULL_HANDLE; + if (vkResetCommandBuffer(cmd, 0) != VK_SUCCESS) return VK_NULL_HANDLE; VkCommandBufferBeginInfo bi = {VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; bi.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; - if (vkBeginCommandBuffer(cmd, &bi) != VK_SUCCESS) { - vkFreeCommandBuffers(dev, pool, 1, &cmd); - return VK_NULL_HANDLE; - } + if (vkBeginCommandBuffer(cmd, &bi) != VK_SUCCESS) return VK_NULL_HANDLE; return cmd; } @@ -243,7 +242,7 @@ struct FencedStage { if (dev == VK_NULL_HANDLE) return; retire(); if (fence != VK_NULL_HANDLE) vkDestroyFence(dev, fence, nullptr); - if (pool != VK_NULL_HANDLE) vkDestroyCommandPool(dev, pool, nullptr); + if (pool != VK_NULL_HANDLE) vkDestroyCommandPool(dev, pool, nullptr); // frees cmd if (mem != VK_NULL_HANDLE) { if (mapped != nullptr) vkUnmapMemory(dev, mem); vkFreeMemory(dev, mem, nullptr); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e62e161..ab00fd7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -113,3 +113,26 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux") CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON) add_test(NAME displayxr_linux_window_input_test COMMAND displayxr_linux_window_input_test) endif() + +# vk_frame_ring.h (frame-ring discipline, Adreno DVFS). The bare CI runners have +# no Vulkan SDK, so the test always runs the pure helpers (ring arithmetic, +# env clamp, UBO slice alignment, deferred-deletion order) via +# DXR_VK_FRAME_RING_PURE_ONLY, and additionally compile-checks the full +# VkFrameRing class (plus vk_overlay_kit.h) when Vulkan headers are found. +# GPU-free by design: nothing here creates a VkDevice. +add_executable(displayxr_vk_frame_ring_test vk_frame_ring_test.cpp) +target_link_libraries(displayxr_vk_frame_ring_test PRIVATE displayxr::rules) +find_package(Vulkan QUIET) +if(TARGET Vulkan::Headers) + target_link_libraries(displayxr_vk_frame_ring_test PRIVATE Vulkan::Headers) +else() + find_path(DXR_VULKAN_INCLUDE_DIR vulkan/vulkan.h + HINTS "$ENV{VULKAN_SDK}/include" /opt/homebrew/include /usr/local/include) + if(DXR_VULKAN_INCLUDE_DIR) + target_include_directories(displayxr_vk_frame_ring_test PRIVATE "${DXR_VULKAN_INCLUDE_DIR}") + endif() +endif() +set_target_properties(displayxr_vk_frame_ring_test PROPERTIES + CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON) + +add_test(NAME displayxr_vk_frame_ring_test COMMAND displayxr_vk_frame_ring_test) diff --git a/tests/vk_frame_ring_test.cpp b/tests/vk_frame_ring_test.cpp new file mode 100644 index 0000000..1b52910 --- /dev/null +++ b/tests/vk_frame_ring_test.cpp @@ -0,0 +1,211 @@ +// Copyright 2026, DisplayXR +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief GPU-free test for vk_frame_ring.h. + * + * Exercises the pure helpers the ring is built from (slot advance + which + * fence a frame waits, env clamp, UBO slice alignment, deferred-deletion + * order) on every runner. When the Vulkan headers are available it also + * compile-checks the full VkFrameRing class and vk_overlay_kit.h — no device + * is created, so nothing needs the loader at link time. + */ + +#include +#include +#include +#include + +#if defined(__has_include) +#if __has_include() +#define DXR_TEST_HAS_VULKAN 1 +#endif +#endif + +#ifndef DXR_TEST_HAS_VULKAN +#define DXR_VK_FRAME_RING_PURE_ONLY 1 +#endif +#include "vk_frame_ring.h" +#ifdef DXR_TEST_HAS_VULKAN +#include "vk_overlay_kit.h" +#endif + +static int g_failures = 0; + +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + std::fprintf(stderr, "FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + ++g_failures; \ + } \ + } while (0) + +using namespace dxr::detail; + +static void +testAlignUp() +{ + CHECK(alignUp(0, 256) == 0); + CHECK(alignUp(1, 256) == 256); + CHECK(alignUp(256, 256) == 256); + CHECK(alignUp(257, 256) == 512); + CHECK(alignUp(208, 64) == 256); + CHECK(alignUp(208, 16) == 208); + CHECK(alignUp(13, 0) == 13); // 0 / 1 = unaligned + CHECK(alignUp(13, 1) == 13); + CHECK(alignUp(10, 12) == 12); // non-power-of-two still rounds up +} + +static void +testParseFramesInFlight() +{ + CHECK(parseFramesInFlight(nullptr, 0) == 3); // default + CHECK(parseFramesInFlight("", 0) == 3); + CHECK(parseFramesInFlight(nullptr, 2) == 2); // explicit config (Android path) + CHECK(parseFramesInFlight(nullptr, 99) == 8); // config clamped + CHECK(parseFramesInFlight("1", 3) == 1); // env wins: A/B the anti-pattern + CHECK(parseFramesInFlight("4", 2) == 4); + CHECK(parseFramesInFlight("8", 0) == 8); + CHECK(parseFramesInFlight("9", 0) == 8); // clamp high + CHECK(parseFramesInFlight("1000000", 0) == 8); + CHECK(parseFramesInFlight("0", 2) == 2); // 0 = not set + CHECK(parseFramesInFlight("-3", 0) == 3); // negative = not set + CHECK(parseFramesInFlight("abc", 5) == 5); // garbage = not set + CHECK(parseFramesInFlight("3x", 0) == 3); // trailing junk = not set + CHECK(parseFramesInFlight("3x", 6) == 6); + CHECK(clampFramesInFlight(0) == 3); + CHECK(clampFramesInFlight(1) == 1); + CHECK(clampFramesInFlight(9) == 8); +} + +//! Model the ring exactly as VkFrameRing drives it: idx starts at n-1, each +//! beginFrame() advances then waits that slot's fence if pending, endFrame() +//! marks it pending. The fence waited at frame k must be frame k-n's — never +//! the frame just submitted (k-1) unless n == 1. +static void +testRingSchedule() +{ + for (uint32_t n = 1; n <= kMaxFramesInFlight; ++n) { + uint32_t idx = n - 1; + std::vector submittedFrame(n, -1); // -1 = never submitted (not pending) + for (int k = 0; k < 50; ++k) { + idx = ringAdvance(idx, n); + CHECK(idx == uint32_t(k) % n); + const int waited = submittedFrame[idx]; // beginFrame waits this + if (k < int(n)) { + CHECK(waited == -1); // warm-up: nothing to wait + } else { + CHECK(waited == k - int(n)); + if (n >= 2) CHECK(waited != k - 1); // never the frame just submitted + } + submittedFrame[idx] = k; // endFrame + } + } + CHECK(ringAdvance(0, 0) == 0); // degenerate, no division by zero + CHECK(ringAdvance(2, 3) == 0); +} + +static void +testUboSlices() +{ + // 3 slots x 4 views, 208-byte UBO at 256-byte alignment. + const uint64_t stride = alignUp(208, 256); + CHECK(stride == 256); + std::vector seen; + for (uint32_t s = 0; s < 3; ++s) { + for (uint32_t v = 0; v < 4; ++v) { + const uint64_t off = uboSliceOffset(s, v, 4, stride); + CHECK(off % 256 == 0); // every dynamic offset is legal + CHECK(off + 208 <= 3 * 4 * stride); // inside the buffer + for (uint64_t o : seen) CHECK(o != off); // slots/views disjoint + seen.push_back(off); + } + } + CHECK(uboSliceOffset(0, 0, 4, stride) == 0); + CHECK(uboSliceOffset(1, 0, 4, stride) == 4 * stride); + CHECK(uboSliceOffset(2, 3, 4, stride) == 11 * stride); +} + +static void +testDeferredQueue() +{ + DeferredQueue q; + std::string log; + CHECK(q.empty()); + q.push([&] { log += "a"; }); + q.push([&] { log += "b"; }); + q.push(nullptr); // ignored + q.push([&] { + log += "c"; + q.push([&] { log += "d"; }); // queued from inside: next run, not now + }); + CHECK(q.size() == 3); + q.run(); + CHECK(log == "abc"); // FIFO + CHECK(q.size() == 1); + q.run(); + CHECK(log == "abcd"); + CHECK(q.empty()); + q.run(); // empty run is a no-op + CHECK(log == "abcd"); +} + +#ifdef DXR_TEST_HAS_VULKAN +static void +testFullHeaderCompiles() +{ + static_assert(!std::is_copy_constructible::value, "ring owns handles"); + static_assert(!std::is_copy_assignable::value, "ring owns handles"); + dxr::VkFrameRing::Config cfg; + CHECK(cfg.framesInFlight == 0); + CHECK(cfg.perViewUboBytes == 0); + CHECK(cfg.maxViewsPerFrame == 16); + // Signatures downstream code is written against. decltype is unevaluated, + // so this checks the API without ODR-using (and so linking) any vk* call. + using R = dxr::VkFrameRing; + static_assert(std::is_same::value, ""); + static_assert(std::is_same::value, ""); + static_assert(std::is_same::value, + ""); + static_assert(std::is_same::value, + ""); + static_assert(std::is_same::value, ""); + static_assert(std::is_same::value, ""); + static_assert( + std::is_same::value, ""); + static_assert( + std::is_same)>::value, ""); + static_assert(std::is_same::value, ""); + static_assert(std::is_same&)>::value, + ""); + static_assert(std::is_same::value, + ""); + std::printf("vk_frame_ring_test: full header (Vulkan) compiled\n"); +} +#endif + +int +main() +{ + testAlignUp(); + testParseFramesInFlight(); + testRingSchedule(); + testUboSlices(); + testDeferredQueue(); +#ifdef DXR_TEST_HAS_VULKAN + testFullHeaderCompiles(); +#else + std::printf("vk_frame_ring_test: no Vulkan headers — pure helpers only\n"); +#endif + if (g_failures != 0) { + std::fprintf(stderr, "vk_frame_ring_test: %d failure(s)\n", g_failures); + return 1; + } + std::printf("vk_frame_ring_test: OK\n"); + return 0; +}