From 682b525490d74e03fc06159089165a6883b1d2e9 Mon Sep 17 00:00:00 2001 From: dfattal Date: Fri, 2 Oct 2026 09:43:38 -0700 Subject: [PATCH] =?UTF-8?q?feat(color):=20window-space=20swapchains=20defa?= =?UTF-8?q?ult=20to=20=5FSRGB=20(ADR-044=20=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Window-space (HUD / toast / button / Local2D) layers carry DISPLAY-REFERRED R8G8B8A8 bytes: HudRenderer rasterizes with D2D/DirectWrite into a private R8G8B8A8_UNORM texture, HudRendererMacOS into a kCGColorSpaceSRGB bitmap, and the app moves them into the swapchain image. CreateWindowSpaceSwapchain and both CreateHudSwapchain overloads still preferred R8G8B8A8_UNORM, so on a format-honest runtime (D3D11 >= v2.21.0, D3D12 >= v2.21.1, GL >= v2.21.2, vk_native >= v2.21.7) those bytes were read as LINEAR and encoded a second time: the translucent black backdrop lifted to grey and the text washed out. This is the open item ADR-044 §7 names; INV-4.6 is the rule. - color_policy.h: dxr::ChooseWindowSpaceSwapchainFormat(), header-inline so displayxr::rules consumers (the Android legs) can choose through the same rule. Default / =srgb: the first R8G8B8A8 _SRGB code advertised together with its UNORM sibling (DXGI 29/28, VK 43/37, GL_SRGB8_ALPHA8/GL_RGBA8, Metal 71/70); else the R8G8B8A8 UNORM code (fellBack, warned); else formats[0]. =unorm: the R8G8B8A8 UNORM code, exactly the old choice, so DXR_SWAPCHAIN_ENCODING (#49) now A/Bs projection and window-space together. The family never follows formats[0]'s channel order: the copy is family-bound, and 28 -> 29 is the same R8G8B8A8_TYPELESS family the runtime allocates, so CopyTextureRegion / CopyResource / UpdateSubresource / vkCmdCopyBufferToImage / glTexSubImage2D / replaceRegion: stay legal and byte-exact. The pair requirement keeps a colliding code from another API (29 is also VK_FORMAT_R8G8B8_SRGB) from being taken. Pure; never notes the projection format, so RenderSceneLinear() is unaffected. - xr_session_common.cpp CreateWindowSpaceSwapchain and xr_window_space_hud.cpp CreateHudSwapchain use it; the misleading "deliberately NOT migrated" comments are rewritten. - Consumer audit (in-library): HudRenderer clears/draws only its private UNORM texture (transparent clears, D2D never binds the swapchain image); CachedLayerUploader is a raw vkCmdCopyBufferToImage. No library code renders, blits or clears into a window-space image. Header notes on hud_renderer.h, hud_renderer_macos.h, vk_overlay_kit.h, xr_session_common.h and xr_window_space_hud.h state the raw-copy rule. - README: "Window-space swapchains: _SRGB by default too" with the selection table and the per-method verdict table (raw copy OK; _SRGB RTV / D2D on the image / vkCmdBlitImage / GL draw with GL_FRAMEBUFFER_SRGB / non-zero clears are not, with the fix for each). - tests/common_smoke.cpp: test_window_space_format_policy() pins the choice on the four lists the runtime advertises, =unorm/=srgb, the fallbacks, the collision guard, and that the projection note is untouched. CMakeLists VERSION -> 2.27.0 (behaviour change; it had drifted to 2.20.0). Co-Authored-By: Claude Opus 5.5 --- CMakeLists.txt | 2 +- README.md | 59 +++++++++++++++++--- common/color_policy.h | 99 +++++++++++++++++++++++++++++++++- common/hud_renderer.h | 8 +++ common/hud_renderer_macos.h | 7 +++ common/vk_overlay_kit.h | 6 +++ common/xr_session_common.cpp | 55 ++++++++++--------- common/xr_session_common.h | 9 ++++ common/xr_window_space_hud.cpp | 35 ++++++------ common/xr_window_space_hud.h | 13 +++-- tests/common_smoke.cpp | 67 +++++++++++++++++++++++ 11 files changed, 306 insertions(+), 54 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c26e775..277f3da 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.20.0 + VERSION 2.27.0 DESCRIPTION "Shared off-axis Kooima projection math + C++ app scaffolding for DisplayXR 3D displays" LANGUAGES C CXX) diff --git a/README.md b/README.md index fd4e310..f4bc8d9 100644 --- a/README.md +++ b/README.md @@ -341,12 +341,59 @@ siblings, then make the storage view in the UNORM sibling and the sampled / atta the `_SRGB` one — the same recipe the runtime's Vulkan compositor uses for its own swapchain images. The bytes are unchanged; only the view decides whether a read decodes. -**Not migrated (deliberate):** the window-space HUD swapchain -(`CreateWindowSpaceSwapchain`, `CreateHudSwapchain`) stays `R8G8B8A8_UNORM`. It is a CPU-upload -path — the HUD is rasterized on the CPU into display-referred RGBA8 and copied in, so nothing -in it can encode, and the format also pins the copy family on four graphics APIs -(`CopyTextureRegion` / `vkCmdCopy*`). Declaring those bytes `_SRGB` is the correct end state and -costs no quality, but it needs its own verified change. +#### Window-space swapchains: `_SRGB` by default too (`v2.27.0`, ADR-044 §7) + +> **Behaviour change in `v2.27.0`.** `CreateWindowSpaceSwapchain` / `CreateHudSwapchain` (both +> overloads) now ask for the **R8G8B8A8 `_SRGB` sibling** — DXGI `29`, VK `43`, +> `GL_SRGB8_ALPHA8`, Metal `71` — instead of R8G8B8A8 UNORM. **An app that moves the HUD bytes in +> with a raw copy needs no change**, and its HUD/toast stops washing out on a format-honest +> runtime. `DXR_SWAPCHAIN_ENCODING=unorm` restores the old format (same switch as above, so one +> variable A/Bs projection and window-space together). + +Window-space (HUD, toast, button, Local2D) layers carry **display-referred** R8G8B8A8 bytes: the +CPU rasterizers (`HudRenderer` = D2D/DirectWrite into a private `R8G8B8A8_UNORM` texture, +`HudRendererMacOS` = a `kCGColorSpaceSRGB` bitmap) write the authored sRGB values verbatim. The +runtime is format-honest on D3D11 ≥ v2.21.0, D3D12 ≥ v2.21.1, GL ≥ v2.21.2 and vk_native ≥ +v2.21.7 ([ADR-044](https://github.com/DisplayXR/displayxr-runtime/blob/main/docs/adr/ADR-044-colour-contract-per-backend.md), +[INV-4.6](https://github.com/DisplayXR/displayxr-runtime/blob/main/docs/guides/displayxr-app-rules.md)): +a UNORM swapchain holds **linear** values and is sRGB-encoded on the way to the panel, so those +bytes in the old UNORM swapchain were **encoded twice** — the translucent black backdrop lifted to +grey, the text washed out. Declaring them `_SRGB` is correct on every backend, Metal included +(which is not yet format-honest and passes bytes through either way). + +`dxr::ChooseWindowSpaceSwapchainFormat()` (`color_policy.h`, header-inline, so `displayxr::rules` +— i.e. the Android legs — can call it too) is the rule: + +| `DXR_SWAPCHAIN_ENCODING` | Chosen format | +|---|---| +| *unset* (**default**) / `srgb` | the first R8G8B8A8 `_SRGB` code advertised **together with its UNORM sibling** (`29`/`28`, `43`/`37`, `0x8C43`/`0x8058`, `71`/`70`); else the R8G8B8A8 UNORM code with one `WARN`; else `formats[0]` | +| `unorm` | the R8G8B8A8 UNORM code — exactly the pre-`v2.27.0` choice | + +Unlike the projection rule it **never follows `formats[0]`'s channel order** (a Vulkan runtime +lists BGRA first): the copy is family-bound. `28 → 29` is the same `R8G8B8A8_TYPELESS` family the +runtime allocates, so the copy stays legal and byte-exact. The pair requirement keeps a colliding +code from another API (`29` is also `VK_FORMAT_R8G8B8_SRGB`) from being taken. + +**What the change means for the code that fills the image** — the bytes are already encoded, so +they must land **without conversion**: + +| Method into the window-space image | On `_SRGB` | | +|---|---|---| +| D3D11 `UpdateSubresource` / `CopyResource` / `CopySubresourceRegion`; D3D12 `CopyTextureRegion` (upload buffer → image, footprint format = the swapchain format or its UNORM sibling) | raw copy | ✅ byte-exact | +| Vulkan `vkCmdCopyBufferToImage` (`dxr::CachedLayerUploader`) / `vkCmdCopyImage` | raw copy | ✅ byte-exact | +| GL `glTexSubImage2D` | raw upload (`GL_FRAMEBUFFER_SRGB` does not apply to uploads) | ✅ byte-exact | +| Metal `replaceRegion:` / blit-encoder `copyFromTexture:` | raw copy | ✅ byte-exact | +| D3D draw through an RTV created with the swapchain format (`29`), or D2D `CreateDxgiSurfaceRenderTarget` on the swapchain texture | encodes on write; **D2D cannot bind an `_SRGB` surface at all** | ❌ — draw into a private UNORM texture and copy, or create the RTV with the **UNORM** sibling (`28`) over the runtime's `R8G8B8A8_TYPELESS` texture | +| `vkCmdBlitImage` from a UNORM source | encodes on write | ❌ — blit into a UNORM-sibling scratch, then `vkCmdCopyImage` | +| GL draw / `glBlitFramebuffer` into the image with `GL_FRAMEBUFFER_SRGB` enabled | encodes on write | ❌ — disable `GL_FRAMEBUFFER_SRGB` for that pass and restore the app's state | +| Clear to transparent `(0,0,0,0)` | 0 encodes to 0 | ✅ | +| Clear to an authored, non-zero colour | the value is linear on an `_SRGB` target | ❌ — `ClearRenderTargetViewDisplayReferred()` / `dxr::VkDisplayReferredClearColor()` / `d3d12_clear.h` / `gl_clear.h` | + +Inside this library every write into a window-space image is in the first rows: `HudRenderer` +clears and draws only its **private** UNORM texture (whose bytes are then read back), and the only +library code that touches a swapchain image is `CachedLayerUploader`'s `vkCmdCopyBufferToImage`. +The writes into the swapchain image live in the apps — audit them with the table above. **Metal** +is compile-checked by CI only; a macOS atlas check is owed before relying on the Metal row. ### The undock launch contract (`launch_args.h`, `url_fetch.h`, `view_protocol.h`) diff --git a/common/color_policy.h b/common/color_policy.h index d45a4d6..3a423fa 100644 --- a/common/color_policy.h +++ b/common/color_policy.h @@ -37,7 +37,9 @@ namespace dxr { -//! What `DXR_SWAPCHAIN_ENCODING` asked for. +//! What `DXR_SWAPCHAIN_ENCODING` asked for. Governs BOTH the projection/quad +//! swapchain (ChooseColorSwapchainFormat) and the window-space ones +//! (ChooseWindowSpaceSwapchainFormat), so one switch A/Bs the whole app. enum class ColorEncodingPreference { HonestSrgb, //!< default (env unset or unrecognized) — prefer an `_SRGB` format ForceSrgb, //!< `DXR_SWAPCHAIN_ENCODING=srgb` @@ -184,4 +186,99 @@ SceneLinearToDisplayReferred(float c) return 1.055f * std::pow(c, 1.0f / 2.4f) - 0.055f; } +/*! + * THE selection rule for a **window-space / HUD / toast / Local2D** swapchain — + * the layers whose pixels are rasterized on the CPU (D2D/DirectWrite, + * Core Graphics) or by a private device into **display-referred R8G8B8A8** + * bytes and then *moved* into the swapchain image, never rendered into it. + * + * ADR-044 §7 / INV-4.6. On a format-honest runtime (D3D11 ≥ v2.21.0, D3D12 ≥ + * v2.21.1, GL ≥ v2.21.2, vk_native ≥ v2.21.7) a UNORM swapchain holds LINEAR + * values and the runtime sRGB-encodes them, so display-referred bytes in one + * are encoded twice (washed out: lifted backdrop, grey text). The bytes ARE + * encoded, so the honest declaration is the `_SRGB` format — which costs no + * quality and needs no conversion anywhere, provided the bytes land by a RAW + * copy (see README § *Window-space swapchains*). + * + * Unlike ChooseColorSwapchainFormat() this deliberately does NOT follow the + * channel order of `formats[0]`: the CPU rasterizers always emit R8G8B8A8, and + * the copy into the image must stay inside that one family (D3D12 + * `CopyTextureRegion` requires it; a buffer→image copy reinterprets the bytes + * in the image's order). R8G8B8A8_UNORM → its `_SRGB` sibling is the same + * typeless family (DXGI 28/29 are both R8G8B8A8_TYPELESS, which is what the + * runtime allocates), so a raw copy stays legal and byte-exact. + * + * - HonestSrgb (default) / ForceSrgb → the first R8G8B8A8 `_SRGB` code + * advertised **together with its UNORM sibling** — DXGI 29, VK 43, + * GL_SRGB8_ALPHA8 0x8C43, Metal 71 (the pair requirement is what keeps a + * colliding code from another API, e.g. VK_FORMAT_R8G8B8_SRGB = 29, from + * being taken). Else the R8G8B8A8 UNORM code (fellBack: the runtime offers + * no sibling, so the bytes stay as before and the caller should say so + * once). Else `formats[0]` (fellBack). + * - ForceUnorm (`DXR_SWAPCHAIN_ENCODING=unorm`) → the R8G8B8A8 UNORM code — + * exactly the pre-v2.27.0 behaviour, so ONE switch A/Bs the projection and + * the window-space swapchains together. Else `formats[0]` (fellBack). + * + * Header-inline so `displayxr::rules` consumers (the Android legs, which cannot + * link `displayxr::common`) choose through the same rule. Pure: reads no + * environment, touches no global state, and never calls + * NoteColorSwapchainFormat() — the projection swapchain alone decides + * RenderSceneLinear(). + */ +inline ColorFormatChoice +ChooseWindowSpaceSwapchainFormat(const std::vector &formats, ColorEncodingPreference pref) +{ + struct Rgba8Pair { + int64_t unorm; + int64_t srgb; + }; + // Same order the pre-v2.27.0 UNORM list was scanned in. + static const Rgba8Pair kRgba8[] = { + {28, 29}, // DXGI R8G8B8A8_UNORM / R8G8B8A8_UNORM_SRGB + {37, 43}, // VK R8G8B8A8_UNORM / R8G8B8A8_SRGB + {0x8058, 0x8C43}, // GL RGBA8 / SRGB8_ALPHA8 + {70, 71}, // Metal RGBA8Unorm / RGBA8Unorm_sRGB + }; + auto advertised = [&formats](int64_t f) { + for (int64_t x : formats) { + if (x == f) { + return true; + } + } + return false; + }; + + ColorFormatChoice out; + out.preference = pref; + if (formats.empty()) { + return out; + } + + if (pref != ColorEncodingPreference::ForceUnorm) { + for (const Rgba8Pair &p : kRgba8) { + if (advertised(p.srgb) && advertised(p.unorm)) { + out.format = p.srgb; + out.isSrgb = true; + return out; + } + } + } + for (const Rgba8Pair &p : kRgba8) { + if (advertised(p.unorm)) { + out.format = p.unorm; + out.fellBack = (pref != ColorEncodingPreference::ForceUnorm); + return out; + } + } + out.format = formats[0]; + out.isSrgb = false; + for (const Rgba8Pair &p : kRgba8) { + if (p.srgb == formats[0]) { + out.isSrgb = true; + } + } + out.fellBack = true; + return out; +} + } // namespace dxr diff --git a/common/hud_renderer.h b/common/hud_renderer.h index cebe80b..b49e399 100644 --- a/common/hud_renderer.h +++ b/common/hud_renderer.h @@ -55,6 +55,14 @@ bool InitializeHudRenderer(HudRenderer& hud, uint32_t w, uint32_t h, uint32_t fo // Returns pixel pointer (R8G8B8A8_UNORM) and row pitch in bytes. // Caller must call UnmapHud() after consuming the pixels. // +// The bytes are DISPLAY-REFERRED (D2D writes the authored sRGB values verbatim +// into a private UNORM texture). Move them into the window-space swapchain +// image — `_SRGB` by default since v2.27.0 (ADR-044 §7) — with a RAW copy: +// UpdateSubresource / CopyResource / CopyTextureRegion / a staging buffer + +// vkCmdCopyBufferToImage / glTexSubImage2D. Never draw them through an `_SRGB` +// RTV, vkCmdBlitImage them, or point D2D at the swapchain image itself (D2D +// cannot bind an `_SRGB` surface): each of those encodes a second time. +// // `drawBody`: when false, skip the text sections and emit only the buttons. // Lets a HUD-toggle key hide the info panel without losing always-visible // chrome buttons. When buttons are present, body text is laid out below the diff --git a/common/hud_renderer_macos.h b/common/hud_renderer_macos.h index 8d41110..949253f 100644 --- a/common/hud_renderer_macos.h +++ b/common/hud_renderer_macos.h @@ -8,6 +8,13 @@ * version): produces an R8G8B8A8 CPU buffer that the caller uploads into its * graphics-API-specific HUD swapchain image (Metal `replaceRegion:`, * GL `glTexSubImage2D`, Vulkan staging-buffer copy). + * + * The bitmap is a kCGColorSpaceSRGB context, i.e. DISPLAY-REFERRED bytes. All + * three uploads above are RAW copies, so they stay byte-exact into the `_SRGB` + * HUD swapchain CreateHudSwapchain() picks by default since v2.27.0 (ADR-044 + * §7: MTLPixelFormatRGBA8Unorm_sRGB / GL_SRGB8_ALPHA8 / VK_FORMAT_R8G8B8A8_SRGB). + * GL: keep GL_FRAMEBUFFER_SRGB irrelevant by uploading with glTexSubImage2D, + * never by drawing or glBlitFramebuffer-ing into the image. */ #pragma once diff --git a/common/vk_overlay_kit.h b/common/vk_overlay_kit.h index 52f747f..4d61cbe 100644 --- a/common/vk_overlay_kit.h +++ b/common/vk_overlay_kit.h @@ -262,6 +262,12 @@ struct FencedStage { * submitted layer. Changed hash → one staging copy submitted with a fence and * NO wait. Barriers match the demos' hand-rolled pattern (UNDEFINED → * TRANSFER_DST → COLOR_ATTACHMENT_OPTIMAL). + * + * Colour: vkCmdCopyBufferToImage is a RAW copy — it never converts — so + * display-referred RGBA8 bytes land byte-exact in the `_SRGB` window-space + * swapchain (VK_FORMAT_R8G8B8A8_SRGB) that CreateHudSwapchain / + * CreateWindowSpaceSwapchain choose by default since v2.27.0 (ADR-044 §7). + * Do not replace it with vkCmdBlitImage: a blit into an `_SRGB` image encodes. */ class CachedLayerUploader { public: diff --git a/common/xr_session_common.cpp b/common/xr_session_common.cpp index c88625e..640978f 100644 --- a/common/xr_session_common.cpp +++ b/common/xr_session_common.cpp @@ -854,33 +854,38 @@ bool CreateWindowSpaceSwapchain(XrSessionManager& xr, SwapchainInfo& out, std::vector formats(formatCount); XR_CHECK(xrEnumerateSwapchainFormats(xr.session, formatCount, &formatCount, formats.data())); - // Prefer R8G8B8A8_UNORM for window-space swapchains because HudRenderer - // (standalone D3D11 device) always outputs R8G8B8A8_UNORM pixels. Using the - // runtime's default format (often B8G8R8A8 on Vulkan-backed compositors) - // would cause a format family mismatch in D3D12 CopyTextureRegion, silently - // failing the copy. - // Well-known R8G8B8A8_UNORM codes: DXGI=28, VK=37, GL_RGBA8=0x8058. + // ADR-044 §7 / INV-4.6: the R8G8B8A8 `_SRGB` sibling by default. + // Window-space layers carry DISPLAY-REFERRED (sRGB-encoded) bytes — + // HudRenderer rasterizes them with D2D/DirectWrite into a private + // R8G8B8A8_UNORM texture and the app moves them into this image. On a + // format-honest runtime a UNORM swapchain is read as LINEAR and encoded + // again (washed out), so the honest declaration is `_SRGB`. // - // #1589: this one deliberately does NOT follow the honest-sRGB default. It - // is a CPU-upload path — the HUD is rasterized on the CPU into - // display-referred RGBA8 and copied in, so nothing here can encode. Making - // it `_SRGB` is the right end state (the bytes ARE encoded, so an `_SRGB` - // texture would simply be declaring them honestly, at no quality cost), but - // it changes the copy-format family on four graphics APIs and so needs its - // own verified change; it is NOT part of the chooser migration. Until then - // the HUD is a known display-referred-into-UNORM source. - int64_t selectedFormat = formats[0]; - const int64_t preferredFormats[] = { 28, 37, 0x8058 }; - for (int64_t pref : preferredFormats) { - for (uint32_t i = 0; i < formatCount; i++) { - if (formats[i] == pref) { - selectedFormat = pref; - goto found; - } - } + // The family stays R8G8B8A8 (never formats[0]'s channel order): DXGI + // 28 -> 29 is the same R8G8B8A8_TYPELESS family the runtime allocates, so + // D3D12 CopyTextureRegion / D3D11 CopyResource / UpdateSubresource, + // vkCmdCopyBufferToImage and glTexSubImage2D all stay legal and BYTE-EXACT. + // What changes is anything that RENDERS, BLITS or CLEARS into the image — + // that now encodes on write. Move the bytes with a raw copy only; see + // README "Window-space swapchains". DXR_SWAPCHAIN_ENCODING=unorm restores + // the R8G8B8A8_UNORM choice (same switch as the projection swapchain). + const dxr::ColorEncodingPreference pref = dxr::ColorEncodingPreferenceFromEnvironment(); + const dxr::ColorFormatChoice choice = dxr::ChooseWindowSpaceSwapchainFormat(formats, pref); + if (choice.format == 0) { + LOG_ERROR("Window-space swapchain: the runtime advertised no swapchain formats"); + return false; + } + const int64_t selectedFormat = choice.format; + if (choice.fellBack) { + LOG_WARN("Window-space swapchain: no R8G8B8A8 %s pair advertised — using %lld (0x%llX); " + "display-referred bytes in a non-sRGB swapchain wash out on a format-honest runtime", + pref == dxr::ColorEncodingPreference::ForceUnorm ? "UNORM" : "_SRGB/UNORM", + selectedFormat, selectedFormat); + } else { + LOG_INFO("Selected window-space swapchain format: %s %lld (0x%llX)%s", + choice.isSrgb ? "honest _SRGB" : "UNORM", selectedFormat, selectedFormat, + pref == dxr::ColorEncodingPreference::HonestSrgb ? " (default)" : " (DXR_SWAPCHAIN_ENCODING)"); } - found: - LOG_INFO("Selected window-space swapchain format: %lld (0x%llX)", selectedFormat, selectedFormat); XrSwapchainCreateInfo swapchainInfo = {XR_TYPE_SWAPCHAIN_CREATE_INFO}; swapchainInfo.usageFlags = XR_SWAPCHAIN_USAGE_COLOR_ATTACHMENT_BIT | XR_SWAPCHAIN_USAGE_SAMPLED_BIT | diff --git a/common/xr_session_common.h b/common/xr_session_common.h index f3981aa..de3cc2c 100644 --- a/common/xr_session_common.h +++ b/common/xr_session_common.h @@ -362,6 +362,15 @@ bool ReleaseHudSwapchainImage(XrSessionManager& xr); // Generic window-space swapchain helpers (the Hud* ones above are thin wrappers // over these). Used for additional independent UI layers (e.g. a standalone // button bar). Acquire/Release operate on the passed SwapchainInfo directly. +// +// Format (ADR-044 §7, v2.27.0+): the R8G8B8A8 `_SRGB` sibling by default — +// DXGI 29 / VK 43 / GL_SRGB8_ALPHA8 — falling back to R8G8B8A8_UNORM only when +// the sibling is not advertised; DXR_SWAPCHAIN_ENCODING=unorm forces UNORM. +// `out.format` says which. The image holds DISPLAY-REFERRED bytes, so fill it +// with a RAW copy (CopyResource / CopyTextureRegion / UpdateSubresource / +// vkCmdCopyBufferToImage / vkCmdCopyImage / glTexSubImage2D). A draw through an +// `_SRGB` RTV, a vkCmdBlitImage, or a clear to a non-transparent colour now +// encodes on write — see README "Window-space swapchains". bool CreateWindowSpaceSwapchain(XrSessionManager& xr, SwapchainInfo& out, uint32_t width, uint32_t height); bool AcquireWindowSpaceImage(SwapchainInfo& sc, uint32_t& imageIndex); diff --git a/common/xr_window_space_hud.cpp b/common/xr_window_space_hud.cpp index 874889e..8bd54c1 100644 --- a/common/xr_window_space_hud.cpp +++ b/common/xr_window_space_hud.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: BSL-1.0 #include "xr_window_space_hud.h" +#include "color_policy.h" // ChooseWindowSpaceSwapchainFormat (ADR-044 §7) #include #include @@ -17,24 +18,24 @@ bool CreateHudSwapchain(XrSession session, uint32_t width, uint32_t height, XrHu return false; } - // Prefer R8G8B8A8_UNORM in any of the well-known per-API codes; the CPU - // rasterizer always emits RGBA8 pixels and a family mismatch silently - // corrupts the upload (CopyTextureRegion on D3D12 / vkCmdCopy on Vulkan). - // DXGI_FORMAT_R8G8B8A8_UNORM=28, VK_FORMAT_R8G8B8A8_UNORM=37, - // GL_RGBA8=0x8058, MTLPixelFormatRGBA8Unorm=70. - // - // #1589: deliberately NOT migrated to the honest-sRGB default (see the twin - // note in xr_session_common.cpp::CreateWindowSpaceSwapchain). CPU-uploaded - // display-referred pixels; moving it to the `_SRGB` sibling is correct but - // must be validated on all four copy paths first. - const int64_t preferred[] = { 28, 37, 0x8058, 70 }; - int64_t selected = formats[0]; - for (int64_t pref : preferred) { - for (int64_t f : formats) { - if (f == pref) { selected = pref; goto found; } - } + // ADR-044 §7 / INV-4.6: the R8G8B8A8 `_SRGB` sibling by default — the + // same rule as xr_session_common.cpp::CreateWindowSpaceSwapchain, via the + // one shared chooser. The CPU rasterizers (HudRenderer / HudRendererMacOS) + // emit DISPLAY-REFERRED R8G8B8A8 bytes; on a format-honest runtime a UNORM + // swapchain is read as LINEAR and encoded a second time (washed out), so + // `_SRGB` is the honest declaration: DXGI 29, VK 43, GL_SRGB8_ALPHA8, + // MTLPixelFormatRGBA8Unorm_sRGB (71). The family stays R8G8B8A8 so the + // upload (CopyTextureRegion / vkCmdCopyBufferToImage / glTexSubImage2D / + // replaceRegion:) stays a legal, byte-exact RAW copy. Anything that + // renders, blits or clears into the image now encodes on write — move the + // bytes with a raw copy only (README "Window-space swapchains"). + // DXR_SWAPCHAIN_ENCODING=unorm restores the R8G8B8A8 UNORM choice. + const dxr::ColorFormatChoice choice = dxr::ChooseWindowSpaceSwapchainFormat( + formats, dxr::ColorEncodingPreferenceFromEnvironment()); + const int64_t selected = choice.format; + if (selected == 0) { + return false; } -found: XrSwapchainCreateInfo sci = { XR_TYPE_SWAPCHAIN_CREATE_INFO }; sci.usageFlags = XR_SWAPCHAIN_USAGE_COLOR_ATTACHMENT_BIT | diff --git a/common/xr_window_space_hud.h b/common/xr_window_space_hud.h index a4d498c..59574b0 100644 --- a/common/xr_window_space_hud.h +++ b/common/xr_window_space_hud.h @@ -32,10 +32,15 @@ struct XrHudSwapchain { }; /*! - * Create a HUD swapchain that prefers an RGBA8_UNORM format. The CPU-side - * rasterizer (HudRenderer on Windows / HudRendererMacOS on macOS) emits - * R8G8B8A8 pixels, so a format-family mismatch would silently corrupt the - * upload on Vulkan/D3D12 backends. + * Create a HUD swapchain in the R8G8B8A8 family, preferring the `_SRGB` sibling + * (DXGI 29 / VK 43 / GL_SRGB8_ALPHA8 / MTLPixelFormatRGBA8Unorm_sRGB 71) and + * falling back to R8G8B8A8 UNORM only when the sibling is not advertised + * (ADR-044 §7, v2.27.0+; `DXR_SWAPCHAIN_ENCODING=unorm` forces UNORM). The + * CPU-side rasterizer (HudRenderer on Windows / HudRendererMacOS on macOS) + * emits DISPLAY-REFERRED R8G8B8A8 pixels: the family keeps the upload legal on + * Vulkan/D3D12, and the `_SRGB` declaration keeps a format-honest runtime from + * encoding them a second time. Upload with a RAW copy only (`out.format` says + * which format was chosen); see README "Window-space swapchains". */ bool CreateHudSwapchain(XrSession session, uint32_t width, uint32_t height, XrHudSwapchain& out); diff --git a/tests/common_smoke.cpp b/tests/common_smoke.cpp index fcc7402..3ffed17 100644 --- a/tests/common_smoke.cpp +++ b/tests/common_smoke.cpp @@ -1618,6 +1618,72 @@ static void test_color_policy() } } +// ADR-044 §7 / INV-4.6: window-space (HUD / toast / Local2D) swapchains carry +// display-referred R8G8B8A8 bytes moved in by a raw copy, so the honest format +// is the R8G8B8A8 `_SRGB` sibling — and the family must never follow +// formats[0]'s channel order (the copy is family-bound). +static void test_window_space_format_policy() +{ + using dxr::ColorEncodingPreference; + const ColorEncodingPreference kDefault = ColorEncodingPreference::HonestSrgb; + + // The four lists the runtime actually advertises (ADR-044 §1), in order. + const std::vector d3d = {28, 29, 87, 91, 10, 11}; // D3D11 / D3D12 + const std::vector vk = {44, 50, 37, 43, 97, 91, 64}; // vk_native + const std::vector gl = {0x8058, 0x8C43, 0x881A, 0x8814}; // GL + const std::vector mtl = {70, 71, 80, 81, 115, 90}; // Metal + + dxr::ColorFormatChoice c = dxr::ChooseWindowSpaceSwapchainFormat(d3d, kDefault); + CHECK(c.format == 29 && c.isSrgb && !c.fellBack, "D3D window-space -> R8G8B8A8_UNORM_SRGB (29)"); + c = dxr::ChooseWindowSpaceSwapchainFormat(vk, kDefault); + CHECK(c.format == 43 && c.isSrgb && !c.fellBack, + "VK window-space -> R8G8B8A8_SRGB (43), NOT formats[0]'s BGRA sibling (50)"); + c = dxr::ChooseWindowSpaceSwapchainFormat(gl, kDefault); + CHECK(c.format == 0x8C43 && c.isSrgb, "GL window-space -> GL_SRGB8_ALPHA8"); + c = dxr::ChooseWindowSpaceSwapchainFormat(mtl, kDefault); + CHECK(c.format == 71 && c.isSrgb, "Metal window-space -> RGBA8Unorm_sRGB (71)"); + + // Same typeless family both ways: the raw copy the HUD path relies on + // stays legal (28 <-> 29, 37 <-> 43, RGBA8 <-> SRGB8_ALPHA8, 70 <-> 71). + CHECK(dxr::UnormSiblingOf(29) == 28 && dxr::UnormSiblingOf(43) == 37 && + dxr::UnormSiblingOf(0x8C43) == 0x8058 && dxr::UnormSiblingOf(71) == 70, + "every window-space sRGB pick is the R8G8B8A8 UNORM code's own sibling"); + + // =srgb agrees with the default; =unorm is exactly the pre-v2.27.0 choice. + c = dxr::ChooseWindowSpaceSwapchainFormat(vk, ColorEncodingPreference::ForceSrgb); + CHECK(c.format == 43, "=srgb agrees with the default"); + c = dxr::ChooseWindowSpaceSwapchainFormat(d3d, ColorEncodingPreference::ForceUnorm); + CHECK(c.format == 28 && !c.isSrgb && !c.fellBack, "=unorm -> DXGI R8G8B8A8_UNORM (old behaviour)"); + c = dxr::ChooseWindowSpaceSwapchainFormat(vk, ColorEncodingPreference::ForceUnorm); + CHECK(c.format == 37 && !c.isSrgb, "=unorm -> VK R8G8B8A8_UNORM, not formats[0] (44)"); + c = dxr::ChooseWindowSpaceSwapchainFormat(gl, ColorEncodingPreference::ForceUnorm); + CHECK(c.format == 0x8058, "=unorm -> GL_RGBA8"); + c = dxr::ChooseWindowSpaceSwapchainFormat(mtl, ColorEncodingPreference::ForceUnorm); + CHECK(c.format == 70, "=unorm -> Metal RGBA8Unorm"); + + // A runtime without the sibling: the UNORM code, flagged so the caller warns. + c = dxr::ChooseWindowSpaceSwapchainFormat({87, 28}, kDefault); + CHECK(c.format == 28 && !c.isSrgb && c.fellBack, "no 29 advertised -> 28, reported as a fallback"); + + // The sRGB code alone is not enough: it must come WITH its UNORM sibling, + // so a colliding code from another API (VK_FORMAT_R8G8B8_SRGB = 29) is + // never mistaken for DXGI R8G8B8A8_UNORM_SRGB. + c = dxr::ChooseWindowSpaceSwapchainFormat({44, 50, 37, 29}, kDefault); + CHECK(c.format == 37 && c.fellBack, "a lone colliding 29 on a VK list is not taken"); + + // Nothing in the family: formats[0], flagged. Empty: 0. + c = dxr::ChooseWindowSpaceSwapchainFormat({87, 91}, kDefault); + CHECK(c.format == 87 && c.fellBack, "no R8G8B8A8 at all -> formats[0], flagged"); + c = dxr::ChooseWindowSpaceSwapchainFormat({}, kDefault); + CHECK(c.format == 0, "an empty list yields 0"); + + // Choosing a window-space format must not touch the projection note that + // drives RenderSceneLinear() (the chooser is pure). + const bool before = dxr::ColorSwapchainIsSrgb(); + (void)dxr::ChooseWindowSpaceSwapchainFormat(d3d, kDefault); + CHECK(dxr::ColorSwapchainIsSrgb() == before, "window-space choice leaves the projection note alone"); +} + int main() { test_capture_numbering(); @@ -1634,6 +1700,7 @@ int main() test_content_bounds(); test_content_mask(); test_color_policy(); + test_window_space_format_policy(); test_clear_policy(); #ifdef _WIN32 test_input_state_defaults();