From 9708cdca7ffc69a1241eb9ed5dda85723d596a7d Mon Sep 17 00:00:00 2001 From: David Date: Mon, 28 Sep 2026 14:30:51 -0700 Subject: [PATCH] =?UTF-8?q?docs(adr):=20ADR-044=20=E2=80=94=20the=20colour?= =?UTF-8?q?=20contract,=20per=20backend=20and=20swapchain=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One table for what #1589/#1610 shipped across D3D11, the D3D11 service, D3D12, GL, vk_native (Windows / Linux / macOS-MoltenVK / Android) and Metal: - what each backend enumerates, in order; - what it does with an _SRGB vs a UNORM/float swapchain; - where layers blend; - what the DP receives; - which CTS test pins each row. It also records the rows that are NOT CTS-covered (vk_native on macOS and Android, Metal) and what guards them instead. The app rule fits in one line: write encoded colour into _SRGB, or linear into UNORM; prefer _SRGB, the same way on every leg. Amends ADR-021 (principles unchanged). Its "Model A" now reads as "the atlas is encoded", not "UNORM passes through". Pointers added from ADR-021, INV-4.6 and compositor-pipeline.md. Open rows are recorded, not fixed: - Metal is still pre-#1589 passthrough; - the IPC comp_multi paths are unaudited; - the displayxr-common Windows HUD swapchain is UNORM with display-referred bytes. Motivated by the 2026-09-28 washed-out demos: every demo's Android leg, plus avatar's macOS/Linux legs, picked UNORM and wrote display-referred bytes. Fixed app-side in gauss#138, avatar#113, earthview#76, modelviewer#154 and mediaplayer#89. The per-leg lint is in #1760. Co-Authored-By: Claude Opus 5.5 --- docs/README.md | 1 + ...lor-management-encoding-state-invariant.md | 5 + .../ADR-044-colour-contract-per-backend.md | 150 ++++++++++++++++++ docs/adr/README.md | 1 + docs/architecture/compositor-pipeline.md | 2 +- docs/guides/displayxr-app-rules.md | 3 + 6 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 docs/adr/ADR-044-colour-contract-per-backend.md diff --git a/docs/README.md b/docs/README.md index 0eb0ef16b..78a44d4a2 100644 --- a/docs/README.md +++ b/docs/README.md @@ -148,6 +148,7 @@ Integrate your 3D display hardware into DisplayXR. - [ADR-040](adr/ADR-040-rear-depth-budget.md) — Rear depth budget — the runtime owns the policy, the plug-in owns pixels, the app owns geometry - [ADR-041](adr/ADR-041-fixed-view-count-with-per-frame-activity.md) — Fixed view count with per-frame activity — inactive views alias, they do not disappear - [ADR-043](adr/ADR-043-stereo-camera-source.md) — A display's stereo camera is a plug-in-provided source, owned by the service and privacy-gated by the runtime +- [ADR-044](adr/ADR-044-colour-contract-per-backend.md) — The colour contract, per backend and swapchain format --- diff --git a/docs/adr/ADR-021-color-management-encoding-state-invariant.md b/docs/adr/ADR-021-color-management-encoding-state-invariant.md index 484098e04..a906305db 100644 --- a/docs/adr/ADR-021-color-management-encoding-state-invariant.md +++ b/docs/adr/ADR-021-color-management-encoding-state-invariant.md @@ -5,6 +5,11 @@ source: "#409" --- # ADR-021: Color Management & the Encoding-State Invariant +> **Amended by [ADR-044](ADR-044-colour-contract-per-backend.md)** — the shipped contract per +> backend × swapchain format (what the runtime enumerates, what an app must write, what the +> compositor and DP do, which CTS test pins each row). Read §5 "Model A" as *the atlas is +> encoded*, not *a UNORM swapchain passes through*: since #1589 a UNORM swapchain is linear. + ## Context The runtime's color handling grew ad-hoc — each compositor path adopted whatever diff --git a/docs/adr/ADR-044-colour-contract-per-backend.md b/docs/adr/ADR-044-colour-contract-per-backend.md new file mode 100644 index 000000000..785f334a6 --- /dev/null +++ b/docs/adr/ADR-044-colour-contract-per-backend.md @@ -0,0 +1,150 @@ +# ADR-044: The colour contract, per backend and swapchain format + +**Status:** Proposed (2026-09-28) · amends [ADR-021](ADR-021-color-management-encoding-state-invariant.md) +(principles unchanged; this records what shipped and replaces its "Model A = passthrough" reading +for app authors) · driven by [#1589](https://github.com/DisplayXR/displayxr-runtime/issues/1589) / +[#1610](https://github.com/DisplayXR/displayxr-runtime/issues/1610) (CTS) and the 2026-09-28 +washed-out-demos regression · related: [INV-4.6](../guides/displayxr-app-rules.md), +[F-7](../guides/displayxr-app-rules.md), [compositor-pipeline](../architecture/compositor-pipeline.md), +[`test_apps/COLOR_REGRESSION_MATRIX.md`](../../test_apps/COLOR_REGRESSION_MATRIX.md) + +## In one paragraph + +**The swapchain's format says what its bytes mean. The runtime believes it.** +- An `_SRGB` swapchain holds **encoded** (display-referred) colour. +- Any other colour format (UNORM, float) holds **linear** values, and the runtime sRGB-encodes them on the way to the panel. + +That is OpenXR's rule ("all other formats will be treated as linear values"), and the conformance tests judge it: `GradientFormatsLinearVsNonLinear` and the source-alpha blending tests. Every native compositor except Metal has followed it since v2.21.0–v2.21.7. + +An app that picks UNORM and stores already-encoded bytes gets them encoded twice, which looks **washed out**. The runtime is not wrong there. The app is, and the fix is the app's. + +**The app rule:** + +> **Write encoded colour into an `_SRGB` swapchain, or linear colour into a UNORM one. Prefer `_SRGB`, and make that choice the same way on every platform leg.** + +## Context + +ADR-021 set the principles: +- conversions come in matched pairs; +- the format is the source of truth; +- the display processor (DP) declares its handoff encoding. + +ADR-021 also allowed a "Model A" in which the atlas is encoded and a UNORM swapchain was, *in practice*, a byte passthrough. App authors read that as "UNORM means display-referred", and the app guide said so explicitly ("a linear/UNORM swapchain is NOT color-managed"). Most apps therefore picked UNORM and wrote encoded bytes. + +The Khronos CTS disagreed: + +- **#1589.** `GradientFormatsLinearVsNonLinear` failed: a UNORM projection gradient came out dark because it was never encoded. +- **#1610.** `SourceAlphaBlending` and `SourceAlphaBlendingWithEnvironment` failed: layers blended in encoded space, not linear. + +Fixing both made the runtime format-honest. Each backend shipped in its own release: + +| Backend | Release | +|---|---| +| D3D11 in-process + service | v2.21.0 | +| D3D12 | v2.21.1 | +| GL | v2.21.2 | +| vk_native (748b540f3) | v2.21.7 | + +The in-tree Vulkan cube apps were migrated with the vk_native change (#1623, 0a6217a8d). The standalone demos were not. Their macOS/Linux legs had already moved to `_SRGB` for other reasons, but every Android leg and the avatar desktop legs still preferred UNORM. They went washed out on the first runtime ≥ v2.21.7 they met (Leia tablet, runtime v2.21.11, 2026-09-28). + +The rule was right. What was missing was a single statement of it per backend, and a check that sees each leg of an app separately. + +## Decision + +### 1. The contract table + +"Encoded" means standard sRGB (IEC 61966-2-1). "Atlas" is the tiled image handed to the DP's `process_atlas`. + +| Backend (path) | Enumerated colour formats, in order | `_SRGB` swapchain | UNORM / float swapchain | Layers blend in | Atlas the DP receives | Since | +|---|---|---|---|---|---|---| +| **D3D11** in-process | `R8G8B8A8_UNORM`, `R8G8B8A8_UNORM_SRGB`, `B8G8R8A8_UNORM`, `B8G8R8A8_UNORM_SRGB`, `R16G16B16A16_FLOAT`, `R16G16B16A16_UNORM` | sampled through an `_SRGB` view (decode) | read as linear | linear, in a private `_SRGB`-RTV target (encodes on write) | **encoded** | v2.21.0 | +| **D3D11 service** (IPC clients of every API, workspace/shell) | same list, mapped per client API | decode on sample | linear | linear, private `_SRGB`-view target | **encoded**; **linear** under Model B (shell, ≥ 2 honest `_SRGB` clients, DP accepts linear) | v2.21.0 (+ #1591) | +| **D3D12** | same list as D3D11 | decode on sample | linear | linear, private `_SRGB`-view target | **encoded** | v2.21.1 | +| **OpenGL** (Windows, macOS) | `GL_RGBA8`, `GL_SRGB8_ALPHA8`, `GL_RGBA16F`, `GL_RGBA32F` | decode on sample | linear | linear, private `GL_SRGB8_ALPHA8` target | **encoded** | v2.21.2 | +| **Vulkan `vk_native`**: Windows, Linux, macOS (MoltenVK), **Android** | `B8G8R8A8_UNORM`, `B8G8R8A8_SRGB`, `R8G8B8A8_UNORM`, `R8G8B8A8_SRGB`, `R16G16B16A16_SFLOAT`, `R16G16B16A16_UNORM`, `A2B10G10R10_UNORM_PACK32` | the image **is** `_SRGB` (#1559); an `_SRGB` view decodes | linear | linear, private `MUTABLE_FORMAT` image through its `_SRGB` view; handoff by `vkCmdCopyImage`, never a blit | **encoded**, declared via `set_atlas_encoding` (#1484) | v2.21.7 | +| **Metal** (macOS) | `RGBA8Unorm`, `RGBA8Unorm_sRGB`, `BGRA8Unorm`, `BGRA8Unorm_sRGB`, `RGBA16Float`, `RGB10A2Unorm` | sampled through a **UNORM** view (raw bytes) | **raw bytes (pre-#1589 passthrough)** | encoded space | encoded | **not migrated**: see §4 | + +What the table implies: + +- **Order is not a preference.** Every backend lists a UNORM format before its `_SRGB` sibling. That order is unchanged between v2.20.1 and v2.21.11. An app that takes `formats[0]` gets UNORM. +- **Float formats are linear.** They follow the UNORM column. The atlas is 8-bit encoded, so values above 1.0 clip. There is no HDR path (ADR-021 *Consequences*). +- **Where the rows do not differ:** + - **Fast path.** A single-layer `_SRGB` frame skips the private target and is byte-identical to the old path. The predicate is `u_color_compose_fast_path()` in `auxiliary/util/u_color_encoding.h`. + - **Zero-copy.** A UNORM source never takes zero-copy, because it still owes the encode. +- **Escape hatch.** `DXR_COLOR_LEGACY_UNORM_ENCODED=1` restores the passthrough reading process-wide. On Android set it with `adb shell setprop debug.xrt.DXR_COLOR_LEGACY_UNORM_ENCODED 1`. It is a diagnostic, not a setting. + +### 2. What the app writes, per format + +| App's swapchain | App writes | Result | +|---|---|---| +| `_SRGB` | linear shader output, GPU encodes on write (`_SRGB` RTV/attachment, `GL_FRAMEBUFFER_SRGB`) | ✓ | +| `_SRGB` | display-referred bytes moved **without** conversion. Vulkan: blit into an UNORM-sibling scratch, then `vkCmdCopyImage`. D3D: `CopyResource` within the typeless family. | ✓ | +| `_SRGB` | display-referred bytes through a blit, clear or `_SRGB` render target | ✗ encoded twice (washed out) | +| UNORM / float | linear values | ✓ | +| UNORM / float | display-referred bytes | ✗ **encoded twice (washed out)** on every row of §1 except Metal | + +**Clears.** Clear values (render-pass `loadOp`, `vkCmdClearColorImage`, `ClearRenderTargetView`) are *linear* on an `_SRGB` target. A display-referred clear colour must be linearized first. displayxr-common provides this: +- `dxr::DisplayReferredToSceneLinear()`; +- `dxr::VkDisplayReferredClearColor()`; +- `ClearRenderTargetViewDisplayReferred()`, and its D3D12 twin (#1647). + +The helper keys on the *target's* format. Pinning a format and calling the helper are one step (#1647). + +**Alpha is never converted.** sRGB formats encode colour channels only. + +### 3. Layers, alpha and blending (the same on every format-honest backend) + +- **Blend modes (OpenXR §10.6.2), via `comp_layer_blend_mode()` in `compositor/util/comp_layer_view_camera.h`:** + - no `SOURCE_ALPHA_BIT` → `OPAQUE_COVER`: colour replaces, and **alpha is written as 1** (folded into the shader); + - `SOURCE_ALPHA_BIT` → premultiplied; + - `+ UNPREMULTIPLIED_ALPHA_BIT` → straight. + + The **first full-tile projection layer** into a tile is `REPLACE`: its alpha reaches the atlas verbatim. The DP's compose-under-desktop gate depends on it (#225), and it is what a transparent-window app relies on. +- **Blending happens in linear light, in the private `_SRGB`-view target (#1610).** No compose shader contains gamma arithmetic; the encode is a property of the render target. The test `colour: the encode is a render target, never shader arithmetic` pins this. +- **Metal** implements the §10.6.2 blend rule (#1621) but blends in encoded space. + +### 4. The DP side + +- **Handoff.** The atlas reaches the DP **encoded** on every in-process backend. The only exception is D3D11-service Model B, where the atlas is **linear** and the DP performs the one matched encode. +- **DP declarations (ADR-021 §3).** The DP declares `get_handoff_color_capability`. The runtime states each frame's encoding with `set_atlas_encoding` (D3D11 service; vk_native since #1484). If the slot is absent, the DP assumes `ENCODED`. +- **What the DP emits.** It emits encoded pixels for the panel. Any vendor panel curve lives inside the DP, never in the runtime (ADR-021 §2). +- **Leia Android CNSDK DP** (plug-in v2.7.x) implements neither slot. That is correct, because vk_native always hands it encoded bytes. Its v2.7.0→v2.7.6 changes are alpha-gate texel reads only; nothing colour-related. + +### 5. Capture + +- **What the PNG contains.** Atlas captures (`xrCaptureAtlasDXR`, the file triggers, MCP) write the atlas bytes, i.e. **encoded** colour. That is what the DP sees, so pixel values compare directly with an app's authored display-referred colour. +- **Alpha.** Alpha is stamped to 255 unless `DXR_ATLAS_CAPTURE_RAW_ALPHA=1` (#425). With it set, the capture becomes an oracle for §10.6.2: 0 means the opaque-cover fix is missing, 255 means it is present. +- **Where the files go:** + +| Backend | Trigger | Output | +|---|---|---| +| vk_native | `$TMPDIR/displayxr_atlas_trigger` | `$TMPDIR/displayxr_atlas.png` | +| Metal, GL | `/tmp/dxr_atlas_trigger` | `/tmp/dxr_atlas.png` | +| D3D11 service | `%TEMP%\workspace_screenshot_trigger` | pre-weave `…_atlas_*.png` plus the post-weave file | + +### 6. What guards each row + +| Row | Pinned by | +|---|---| +| D3D11, D3D12, GL, vk_native on **Windows** | CTS 1.1.63.0: automated on the hosted lane (WARP / llvmpipe / lavapipe, all 5 arms gate). **Interactive composition, human-judged:**
- `GradientFormatsLinearVsNonLinear`, `SourceAlphaBlending`, `SourceAlphaBlendingWithEnvironment`, `QuadOcclusion`
- d3d11 is the reference lane; d3d12 and opengl match it case for case
- opengl needs the CTS GL-plugin `GL_FRAMEBUFFER_SRGB` patch for the gradients ([procedure §10](../reference/cts-interactive-procedure.md))
- a Windows `vulkan` hardware run passed the gradients 13/13 | +| vk_native on **Linux** | CTS `vulkan`/`vulkan2` automated (lavapipe, gating). Interactive Linux `vulkan` pass 2026-09-25: 16/0/11, gradients 13/13. | +| Every backend, structurally | `tests_comp_color_policy.cpp`: each backend asks the shared predicate, the encode is a render target, zero-copy refuses UNORM. `tests_aux_color_encoding.cpp`: the oracle curve, the fast-path truth table, legacy hatch off by default. | +| vk_native on **macOS (MoltenVK)** | **Not covered by CTS** (macOS is out of scope for #1523). Same source as the Linux row, so the structural tests cover it. `tools/vk_srgb_blend_probe.c` checked the `_SRGB`-view-over-`MUTABLE_FORMAT` blend on M1 Pro. | +| vk_native on **Android** | **Not covered by CTS** (the #1523 Android lane does not exist yet). Same source as Linux: `vk_native` has no `XRT_OS_ANDROID` branch in its swapchain or compose code. **The Adreno/Mali driver behaviour of the `_SRGB`-view blend target is unverified**; re-run `vk_srgb_blend_probe` on device. | +| **Metal** | Not covered, and not migrated. Only the structural tests cover it. | +| **Apps** (every platform) | `scripts/check_displayxr_app.py` INV-4.6, **per leg** (#1760): it flags a UNORM-only format preference list and a scan that stops at the first UNORM, in any leg that enumerates formats. Demo repos add a CI guard that every leg chooses through one shared helper (e.g. displayxr-demo-gaussiansplat#138 `lint.yml`). | + +### 7. Rows still open + +- **Metal is not format-honest.** A UNORM swapchain is passed through and layers blend encoded. So the same app can look right on Metal and washed out on vk_native, or the reverse for a true-linear UNORM app. + + Migrating Metal is the same three-part change D3D11 got: true-format views, a private `_SRGB` target, a raw copy. It needs its own issue and a macOS-only verification. macOS understates transfer-function errors, so judge numerically from atlas captures, not by eye. +- **The IPC `comp_multi` paths** (macOS service, Android service flavour, the Linux headless service) have **not been audited** against §1. The shared-atlas path documents a `vkCmdBlitImage` from each client's swapchain format into one target-format atlas (`compositor/multi/comp_multi_system.c`). A blit converts through both formats, so any format-honesty there is incidental, not contracted. +- **The displayxr-common Windows window-space HUD swapchain** stays `R8G8B8A8_UNORM` with display-referred bytes. On any format-honest backend it is encoded twice; its move to `_SRGB` is pending (displayxr-common). + +## Consequences + +- **One sentence for app authors** (the rule at the top), one table for runtime authors (§1), and a per-leg lint. +- **The runtime does not change to accommodate apps.** Every correct row is CTS- or test-pinned, and reverting a row to passthrough would fail `GradientFormatsLinearVsNonLinear` again. An app that looks washed out on v2.21.7+ is fixed app-side: request `_SRGB` and keep the bytes unconverted. +- **ADR-021 stands for the principles** (matched pairs, format as source of truth, DP-declared handoff). Its §5 "Model A baseline" now means "the atlas is encoded". It no longer means "a UNORM swapchain passes through", and the hop table in ADR-021 already says so. +- **Metal is the known exception** until it is migrated (§7). App guidance stays the same regardless: `_SRGB` is correct on every row, Metal included. diff --git a/docs/adr/README.md b/docs/adr/README.md index 0f67bfe70..fd2db195c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -46,3 +46,4 @@ - [ADR-040](ADR-040-rear-depth-budget.md) — Rear depth budget — the runtime owns the policy, the plug-in owns pixels, the app owns geometry - [ADR-041](ADR-041-fixed-view-count-with-per-frame-activity.md) — Fixed view count with per-frame activity — inactive views alias, they do not disappear - [ADR-043](ADR-043-stereo-camera-source.md) — A display's stereo camera is a plug-in-provided source, owned by the service and privacy-gated by the runtime +- [ADR-044](ADR-044-colour-contract-per-backend.md) — The colour contract, per backend and swapchain format diff --git a/docs/architecture/compositor-pipeline.md b/docs/architecture/compositor-pipeline.md index 9f16fc281..dcc84fa2d 100644 --- a/docs/architecture/compositor-pipeline.md +++ b/docs/architecture/compositor-pipeline.md @@ -27,7 +27,7 @@ Compositor Display Processor - **Compositor never weaves** — no vendor-specific display format logic in compositor code. All 3D output processing is delegated to the display processor via `process_atlas()`. - **Tile-layout-aware** — the display processor receives `tile_columns` and `tile_rows` rather than assuming any particular view arrangement (e.g., side-by-side). This supports arbitrary multiview layouts. - **Canvas sub-rect flows to DP** — for `_texture` apps, the canvas may be a sub-rect of the window. The compositor passes `canvas_offset_x`, `canvas_offset_y`, `canvas_width`, and `canvas_height` through to `process_atlas()` so the display processor can compute correct phase alignment. The app's real window handle (HWND / NSView) is passed directly to the display processor — no hidden windows are involved. -- **DP-handoff encoding is declared by the DP, not fixed** — DisplayXR owns a vendor-neutral compose space; the display processor *declares* whether it accepts `LINEAR`, `ENCODED`, or `EITHER` input, and the runtime converts compose→handoff to match (a matched-pair step, no-op when they already agree). The runtime never derives the convention from — or bakes a curve of — any particular vendor's weaver (ADR-003/007). Production DPs typically declare `EITHER` (their weaver has an explicit input/output sRGB-conversion control and can do the output encode itself), and the atlas always reaches the DP **encoded**. Since #1589 the runtime is format-honest about how it gets there: an `_SRGB` swapchain already holds encoded bytes and passes through (GL #407, D3D11/D3D12/Vulkan/Metal #408), while a UNORM swapchain holds **linear** values and is encoded by the `_SRGB`-view compose target on the way in. The app's obligation is unchanged and now load-bearing: request an sRGB swapchain and store a correctly-encoded image, or take UNORM and store scene-linear. Full contract incl. the matched-pair invariant and both-direction conversion: [ADR-021](../adr/ADR-021-color-management-encoding-state-invariant.md). (This supersedes an earlier "DP expects linear input" note — that described a service-path *intermediate*, not the handoff, and is the latent half-conversion documented below.) +- **DP-handoff encoding is declared by the DP, not fixed** — DisplayXR owns a vendor-neutral compose space; the display processor *declares* whether it accepts `LINEAR`, `ENCODED`, or `EITHER` input, and the runtime converts compose→handoff to match (a matched-pair step, no-op when they already agree). The runtime never derives the convention from — or bakes a curve of — any particular vendor's weaver (ADR-003/007). Production DPs typically declare `EITHER` (their weaver has an explicit input/output sRGB-conversion control and can do the output encode itself), and the atlas always reaches the DP **encoded**. Since #1589 the runtime is format-honest about how it gets there: an `_SRGB` swapchain already holds encoded bytes and passes through (GL #407, D3D11/D3D12/Vulkan/Metal #408), while a UNORM swapchain holds **linear** values and is encoded by the `_SRGB`-view compose target on the way in. The app's obligation is unchanged and now load-bearing: request an sRGB swapchain and store a correctly-encoded image, or take UNORM and store scene-linear. Full contract incl. the matched-pair invariant and both-direction conversion: [ADR-021](../adr/ADR-021-color-management-encoding-state-invariant.md). Per-backend table (enumeration order, `_SRGB` vs UNORM vs float, DP handoff, CTS coverage): [ADR-044](../adr/ADR-044-colour-contract-per-backend.md). (This supersedes an earlier "DP expects linear input" note — that described a service-path *intermediate*, not the handoff, and is the latent half-conversion documented below.) - **Vendor isolation** — adding a new display vendor requires zero changes to compositor code. The vendor implements the display processor vtable under `src/xrt/drivers//`. ## Color-space handling (D3D11 service compositor) diff --git a/docs/guides/displayxr-app-rules.md b/docs/guides/displayxr-app-rules.md index 7cad58d05..016ea09eb 100644 --- a/docs/guides/displayxr-app-rules.md +++ b/docs/guides/displayxr-app-rules.md @@ -516,6 +516,9 @@ re-implementing — see [INV-8.1](#8-app-folder-layout--what-to-include)). linear atlas — see the canonical reference). As an app author you don't depend on which path runs you: request an sRGB swapchain and write a correctly-encoded image, and both paths are correct. + - **The per-backend contract** (enumerated order, what each backend does with `_SRGB` vs UNORM + vs float, what the DP receives, which CTS test pins it, which rows are not CTS-covered): + [ADR-044](../adr/ADR-044-colour-contract-per-backend.md). - Reference: PRs [#407](https://github.com/DisplayXR/displayxr-runtime/pull/407) (GL) and [#408](https://github.com/DisplayXR/displayxr-runtime/pull/408) (D3D11/D3D12/Vulkan/Metal), which established the cross-API sRGB-passthrough contract, plus