diff --git a/docs/adr/ADR-011-d3d11-typeless-swapchain-textures.md b/docs/adr/ADR-011-d3d11-typeless-swapchain-textures.md index 4ee09859f..f355d6f61 100644 --- a/docs/adr/ADR-011-d3d11-typeless-swapchain-textures.md +++ b/docs/adr/ADR-011-d3d11-typeless-swapchain-textures.md @@ -34,5 +34,5 @@ Depth textures already used TYPELESS prior to this change and are unaffected. ## Consequences - **Unity D3D11 works** — the original black screen issue is resolved. - **All D3D11 apps must create typed views** — applications that previously relied on `nullptr` view descriptors (inferring format from the texture) must now pass explicit `D3D11_RENDER_TARGET_VIEW_DESC` / `D3D11_SHADER_RESOURCE_VIEW_DESC` with the concrete format. Our test apps were updated accordingly. -- **Internal compositor textures are mostly unaffected** — the renderer's atlas, HUD, and display processor input textures remain concrete format. *Exception:* the **shell-mode per-client atlas** (`d3d11_client_render_resources::atlas_texture` when `sys->shell_mode == true`) was later switched to `R8G8B8A8_TYPELESS` for the same cross-format-view reason as app swapchains: it needs both a UNORM SRV and a UNORM_SRGB SRV onto the same bytes, which D3D11 only allows on TYPELESS storage. Non-shell mode keeps the atlas at concrete UNORM. See [Compositor Pipeline §Color-space handling](../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor-shell-mode) and `webxr-bridge-color-shift-plan.md` for the motivation (DP-side reinterpretation of SRGB-source vs UNORM-source per-client atlases). +- **Internal compositor textures are mostly unaffected** — the renderer's atlas, HUD, and display processor input textures remain concrete format. *Exception:* the **shell-mode per-client atlas** (`d3d11_client_render_resources::atlas_texture` when `sys->shell_mode == true`) was later switched to `R8G8B8A8_TYPELESS` for the same cross-format-view reason as app swapchains: it needs both a UNORM SRV and a UNORM_SRGB SRV onto the same bytes, which D3D11 only allows on TYPELESS storage. Non-shell mode keeps the atlas at concrete UNORM. See [Compositor Pipeline §Color-space handling](../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor) and `webxr-bridge-color-shift-plan.md` for the motivation (DP-side reinterpretation of SRGB-source vs UNORM-source per-client atlases). - **Matches Khronos reference behavior** — aligns with `hello_xr` and other conformant OpenXR runtimes. diff --git a/docs/adr/ADR-021-color-management-encoding-state-invariant.md b/docs/adr/ADR-021-color-management-encoding-state-invariant.md new file mode 100644 index 000000000..710b195d6 --- /dev/null +++ b/docs/adr/ADR-021-color-management-encoding-state-invariant.md @@ -0,0 +1,195 @@ +--- +status: Accepted +date: 2026-06-04 +source: "#409" +--- +# ADR-021: Color Management & the Encoding-State Invariant + +## Context + +The runtime's color handling grew ad-hoc — each compositor path adopted whatever +colorspace strategy fit the apps it happened to meet. The recent sRGB/gamma fixes +([#407](https://github.com/DisplayXR/displayxr-runtime/pull/407) GL, +[#408](https://github.com/DisplayXR/displayxr-runtime/pull/408) D3D11/D3D12/VK/Metal) +corrected a real ~2.2×-too-dark bug in the in-process path, but the investigation +([#409](https://github.com/DisplayXR/displayxr-runtime/issues/409)) showed the three +paths each encode a *different* convention, and that one of them carries the same +half-conversion #407 fixed — latent only because no shipping app exercises it. + +There was no written contract for what colorspace state a texture holds at each hop, +so every fix was reactive — and worse, the reactive fixes were tuned to what one +vendor's display processor happened to want. That inverts DisplayXR's core +dependency direction: **the runtime defines vendor-neutral conventions; the display +processor adapts to them** (ADR-003, ADR-007, ADR-019). This ADR establishes the +color contract on those terms. + +### The bug class, generalized + +A texture holds pixels in one of two **encoding states**: + +- **Linear** (scene-referred / radiance) — proportional to light. Blending, filtering, + MSAA resolve, and mipmap generation are *only correct here*. +- **Encoded** (display-referred, sRGB/gamma) — in the standard display transfer + function; what a display physically wants. + +A texture's **format declares which**: a `*_SRGB` format ⟹ encoded (the GPU +auto-decodes on sample, auto-encodes on render-target write); a UNORM / float format +⟹ linear. Every bug we chased was the same shape — a **half-conversion**: a decode +(format said sRGB) with no matching encode, or vice versa. + +## Decision + +### 1. The matched-pair invariant (the central rule) + +> **Colorspace conversions come in matched pairs (decode … encode) or not at all +> (passthrough) — never half.** + +This is path-independent and model-independent. Any place that decodes (samples an sRGB +SRV, calls a `srgb_to_linear`) must have a corresponding encode before the bytes leave +for the next canonical space, or it must not decode at all. + +### 2. Standard transfer function; no vendor curves in the runtime + +The canonical encoded state is **standard sRGB**. The runtime's compositor/shader code +must never use a vendor-specific transfer function. The existing +`linear_to_srgb()` in `d3d11_service_shaders.h` bakes a vendor-specific power-law curve +into shared runtime code — a vendor-isolation violation; it is to be removed. If a +vendor's panel needs a non-sRGB curve, that mapping lives **inside that vendor's DP**, +applied after the runtime hands off standard bytes. + +When a path does decode/encode, **both sides of the pair use the same (sRGB) curve.** +Pairing an sRGB decode with a different-curve encode is a half-broken round-trip +(persistent tint), not a clean one. + +### 3. Two canonical spaces; the DP-handoff space is *declared by the DP* + +``` +app render → [swapchain] → sample → [compose / atlas] → process_atlas() → panel + ^^^^^^^ ^^^^^^^^^^^^^^^^ + COMPOSE space DP-HANDOFF space +``` + +- **Compose space** is DisplayXR-internal and vendor-neutral (model choice in §5). +- **DP-handoff space is a negotiated contract, not a fixed value.** Two declarations + define it: + 1. **DP capability (static):** the DP declares which encoding state(s) it can accept — + `LINEAR`, `ENCODED`, or `EITHER`. + 2. **Runtime intent (per-frame):** the runtime declares, through `process_atlas()`, the + encoding state of the atlas it is actually sending. (Today the `format` argument is + hardcoded to `R8G8B8A8_UNORM` and carries no encoding meaning — this declaration + replaces that dead parameter with real intent.) + + The runtime guarantees it sends an encoding the DP can accept; if its compose space + doesn't match a capability-restricted DP, the runtime converts compose→accepted as a + matched-pair step before handoff. The DP then guarantees panel-correct (encoded) output + to the display, converting internally as needed. + + This is the key correction over a naive "handoff = encoded" rule, which was + reverse-engineered from one vendor's weaver. A future vendor whose weaver consumes only + linear input declares `LINEAR`; one that consumes only encoded declares `ENCODED`; the + runtime serves both with no compositor change. + +### 4. DPs that accept `EITHER` make the contract cheap + +A display processor whose weaver exposes an explicit input/output sRGB-conversion control +(decode-before-weave and/or encode-after-weave) can accept **`EITHER`** encoding *and* +perform the final output encode itself — the encoding is selected by an API knob, not +inferred from the texture format. This is the common case for production weavers, and it +collapses the boundary problem: + +- The runtime sends its **native compose space** to such a DP and never converts at the + boundary; the DP configures its weaver to match the runtime's per-frame declaration: + - atlas **encoded** → weave as-is, output encoded (today's behavior), or decode→weave + in linear→re-encode. + - atlas **linear** → weave in linear (correct blend/sampling math) and **encode on + output**. The matched encode lives in the DP's weaver, so the runtime needs **no + encode shader** for Model B. +- This is why deleting `linear_to_srgb()` from the runtime (§2) is not just legal but + correct: the encode belongs in the DP. + +For an `EITHER`-capable DP, the runtime's A-vs-B choice (§5) is purely an internal +correctness decision with **zero** boundary-conversion cost. The concrete control +exposed by a given vendor's weaver is documented under that vendor's docs +(`docs/vendors//`), never here. + +### 5. Compose-space model: Model A baseline, Model B sanctioned future + +The compose-space encoding is an **internal** DisplayXR decision about compositing +correctness, now decoupled from whatever any DP wants: + +- **Model A (baseline, today):** compose in **encoded** space (passthrough). Apps in + this ecosystem write display-referred bytes into both sRGB and UNORM swapchains, so + passthrough is the coherent baseline. The in-process path is already here (post + #407/#408); the service/workspace path must drop the unmatched decode on its + sRGB-client branch (use the UNORM SRV / raw bytes). Until then that branch is guarded + with a one-shot warning in `multi_compositor_render`. +- **Model B (sanctioned future):** compose in **linear** space — decode iff sRGB → + compose linear → convert to the DP's declared handoff state. This is the *correct* + model for alpha compositing, premultiplied-alpha math, and the compose-under-background + transparency feature, all wrong in non-linear space. + + Key property: **B degenerates to A (passthrough) wherever there is nothing to blend** + (decode-then-immediately-reconvert is the identity). So B only costs more on the + **workspace/service compose path** where surfaces overlap — exactly where the + transparency feature lives. It is gated on that feature needing correct blending. + +### 6. Format is the source of truth for a swapchain's encoding state + +`*_SRGB` ⟺ encoded; UNORM / float ⟺ linear. This requires apps to declare honestly +(request an sRGB swapchain and store correctly-encoded pixels). Some test apps violate +this today (encoded bytes in UNORM swapchains); see *Consequences*. + +## Consequences + +- **Vendor isolation preserved.** Color conventions are DisplayXR's; the DP adapts via + a declaration. Adding a vendor with a different input-encoding need requires zero + compositor changes — only the DP declares its preference. +- **New DP-vtable capability required:** the DP declares its accepted handoff encoding + (`LINEAR` / `ENCODED` / `EITHER`). Per ADR-020 (append-only, `struct_size`-gated), + this is a new field; **absent ⟹ `ENCODED`**, which preserves the current de-facto + passthrough behavior for older plug-ins. Production weavers with a configurable + conversion control declare **`EITHER`**. +- **`process_atlas()` gains a per-frame atlas-encoding argument** (replacing the dead + hardcoded `R8G8B8A8_UNORM` `format` parameter, which DPs ignore for colorspace anyway). + The runtime states whether the atlas is linear or encoded; the DP configures itself + accordingly. +- **Vendor DP work (per-vendor, in each plug-in repo):** a vendor whose weaver supports + configurable conversion should (1) wire that control to the runtime's declared atlas + encoding (encode on output whenever input is linear), (2) expose it for **every + graphics-API variant** it ships, and (3) declare its capability. Concrete per-vendor + steps live in that vendor's docs, not in this ADR. +- **One rule for all paths.** "Is this path balanced?" becomes a mechanical check + against the per-hop encoding table — and a regression test, not a code review. +- **The latent service-path bug is documented and guarded**, with a defined fix: under + Model A, passthrough the sRGB-SRV branch; the conversion-to-handoff machinery (§4) is + what ultimately replaces the ad-hoc branch. +- **Vendor curve removed.** `linear_to_srgb()` (a vendor power-law) is deleted from + runtime shaders; any panel curve is DP-internal. +- **App colorspace contract becomes load-bearing.** Honest declaration is required for + Model B; a transitional per-app `treat-as-encoded` override is the escape hatch for + legacy/test apps that put encoded bytes in UNORM. Our own test apps should migrate to + honest sRGB swapchains. +- **Verification gap must be closed.** No current test app renders *true linear* into a + UNORM swapchain, so the linear path has never been exercised — which is why the + half-conversion sat hidden. Required regression matrix: + `{sRGB swapchain, UNORM-encoded, true-linear} × {single-layer opaque, multi-layer + blend} × {in-process, IPC, workspace}`, and — once the declaration lands — a DP test + double that declares `LINEAR` to exercise the encode-at-handoff direction. +- **HDR / wide-gamut**, if it lands on the roadmap, forces Model B with a float16 linear + compose space and would move B from "gated future" to "now." + +## Encoding state at each hop (current baseline = Model A; DP configured for encoded passthrough) + +| Hop | In-process | IPC / service | Workspace / shell | +|---|---|---|---| +| App swapchain | encoded (in sRGB or UNORM) | encoded | encoded | +| Sample into compose | passthrough (UNORM SRV / skip-decode) | passthrough (raw copy) | passthrough (raw copy → TYPELESS atlas) | +| Compose / atlas | encoded | encoded | encoded (UNORM SRV branch) / **linear (sRGB SRV branch — unmatched decode, guarded, to be removed)** | +| Compose → handoff (per DP decl.) | passthrough (DP accepts encoded) | passthrough | passthrough except the sRGB-SRV branch | +| DP handoff | **encoded** ✓ | **encoded** ✓ | **encoded** ✓ except the sRGB-SRV branch | + +Under Model B (future) the "Compose / atlas" row becomes **linear** on the +workspace/service path, and the "Compose → handoff" row carries the matched conversion +to whatever the DP declared (encode-to-sRGB for an `ENCODED` DP; passthrough for a +`LINEAR` DP). The in-process row is unchanged (passthrough == A == B with nothing to +blend). diff --git a/docs/architecture/compositor-pipeline.md b/docs/architecture/compositor-pipeline.md index 8dc94f8cf..98e68358a 100644 --- a/docs/architecture/compositor-pipeline.md +++ b/docs/architecture/compositor-pipeline.md @@ -27,24 +27,21 @@ 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. -- **Color-space delivery is path-specific** — in the **D3D11 service compositor (shell mode)** the DP is fed a *linearized* atlas (the compositor linearizes SRGB source swapchains on the way in; see *Color-space handling* below). On the **in-process native compositors** the DP instead expects **display-referred (sRGB-encoded)** bytes — the compositor passes the app's swapchain bytes through unchanged (GL as of PR #407; D3D11/D3D12/Vulkan/Metal being aligned). Either way the app's obligation is the same: request an sRGB swapchain and store a correctly-encoded image. +- **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 apps store display-referred bytes, so the in-process path passes through encoded (GL #407, D3D11/D3D12/Vulkan/Metal #408). The app's obligation: request an sRGB swapchain and store a correctly-encoded image. 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.) - **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, shell mode) +## Color-space handling (D3D11 service compositor) -The DP weaver expects linear input. App swapchains can be either UNORM (linear bytes) or SRGB (gamma-encoded bytes), and the compositor must reconcile both into a linear stream by the time the DP samples. +The DP declares its accepted handoff encoding and the runtime declares (per frame) the encoding it is sending; production DPs typically declare **`EITHER`** (their weaver has an input/output sRGB-conversion control that handles both directions, including the output encode) — see [ADR-021](../adr/ADR-021-color-management-encoding-state-invariant.md). Apps write encoded bytes into both UNORM and SRGB swapchains, so today the runtime sends **encoded** and the correct path is **passthrough** — hand the app's bytes to the DP unchanged. The multi-compositor stage runs the blit shader at `convert_srgb=0.0` (passthrough) into the combined atlas; the crop step preserves bytes; the DP receives the app's encoded bytes verbatim. (A `LINEAR`-only DP would instead get a matched decode at the handoff step.) -**Non-shell mode** does this at the swapchain → per-client atlas blit: SRGB swapchains are sampled through an SRGB SRV (GPU auto-linearizes) and written to the UNORM atlas as linear bytes via the projection-blit shader. UNORM swapchains take a raw `CopySubresourceRegion` (already linear). The DP then reads the per-client atlas directly. +The swapchain → per-client atlas blit is a raw `CopySubresourceRegion` regardless of source format — a shader-blit at this boundary races with the keyed-mutex release back to the app and can leave per-eye tiles stale on subsequent frames. Per-client atlas storage is `R8G8B8A8_TYPELESS` (workspace mode), and two parallel SRVs view the same bytes, selected at the **multi-comp read** boundary by a per-client `atlas_holds_srgb_bytes` flag (set in `compositor_layer_commit` from `view_is_srgb[0]`): -**Shell mode** uses a multi-compositor stage (per-client atlas → combined atlas → crop → DP). The swapchain → per-client atlas blit must be a raw `CopySubresourceRegion` regardless of source format — a shader-blit at this boundary races with the keyed-mutex release back to the app and can leave per-eye tiles stale on subsequent frames. The reinterpretation therefore happens at the **multi-comp read** boundary instead: +- `atlas_srv` (UNORM-typed) — raw bytes, no conversion. For a client that stored encoded bytes in a UNORM swapchain this is correct passthrough → encoded reaches the DP. +- `atlas_srv_srgb` (UNORM_SRGB-typed) — the GPU **decodes** sRGB→linear on sample. -- Per-client atlas storage is `R8G8B8A8_TYPELESS` (shell mode only). Two parallel SRVs view the same bytes: - - `atlas_srv` (UNORM-typed) — used when the source swapchain bytes are already linear. - - `atlas_srv_srgb` (UNORM_SRGB-typed) — used when the source bytes are gamma-encoded; the GPU auto-linearizes on sample. -- A per-client `atlas_holds_srgb_bytes` flag is set in `compositor_layer_commit` from `view_is_srgb[0]` after the blit. `multi_compositor_render` selects the SRV at sample time. -- The multi-comp blit shader runs at `convert_srgb=0.0` (passthrough) and writes linear values to the combined atlas (UNORM). The crop step preserves bytes, and the DP receives linear input. +> ⚠️ **Known latent half-conversion (ADR-021 / [#409](https://github.com/DisplayXR/displayxr-runtime/issues/409)).** The `atlas_srv_srgb` branch decodes to linear but **nothing re-encodes** before `process_atlas()` (the blit shader is passthrough, and `linear_to_srgb()` in `d3d11_service_shaders.h` is defined but never called). So an sRGB-swapchain client would reach the weaver ~2.2× too dark — the same unmatched decode #407/#408 fixed in-process. It is **dead in practice today** (every workspace app stores encoded-into-UNORM → flag false → UNORM SRV), and the branch is guarded with a one-shot warning in `multi_compositor_render`. The fix is the Model-A passthrough baseline (always use the UNORM SRV here) or, if/when the compose path moves to Model B, the matched re-encode at the DP boundary. Do not "fix" this by linearizing more — that deepens the half-conversion. -This keeps the shell vs non-shell pipelines distinct downstream — a load-bearing invariant; do not refactor the swapchain → atlas blit into a shared shader path. The non-shell atlas remains UNORM-typed; only the shell-mode atlas is TYPELESS-with-dual-SRV. +The non-workspace atlas remains UNORM-typed (single `atlas_srv`); only the workspace-mode atlas is TYPELESS-with-dual-SRV. Do not refactor the swapchain → atlas blit into a shared shader path. ## Per-tile alpha (workspace mode) diff --git a/docs/guides/displayxr-app-rules.md b/docs/guides/displayxr-app-rules.md index 5954b8549..0aa03b23c 100644 --- a/docs/guides/displayxr-app-rules.md +++ b/docs/guides/displayxr-app-rules.md @@ -259,7 +259,7 @@ re-implementing — see [INV-8.1](#8-app-folder-layout--what-to-include)). - 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 - `docs/architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor-shell-mode`. + `docs/architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor`. As of #408 **all five in-process native compositors pass sRGB swapchains through correctly**; the broader color-management design is tracked in [#409](https://github.com/DisplayXR/displayxr-runtime/issues/409). diff --git a/docs/roadmap/surround-2d-rollout.md b/docs/roadmap/surround-2d-rollout.md index 75bb90cf0..f980d903b 100644 --- a/docs/roadmap/surround-2d-rollout.md +++ b/docs/roadmap/surround-2d-rollout.md @@ -37,7 +37,7 @@ End goal: ship the capability through Unity's plugin so Unity editors and runtim - [ ] In `src/xrt/compositor/d3d11/comp_d3d11_compositor.cpp` (or wherever the per-frame `process_atlas` is dispatched), add a surround-blit pass after the DP's weave finishes: 1. `AcquireSync(0)` on the surround texture's KeyedMutex. - 2. Bind two SRVs on the surround texture (UNORM + UNORM_SRGB, parallel to the shell-mode `atlas_holds_srgb_bytes` pattern in [`compositor-pipeline.md`](../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor-shell-mode)). + 2. Bind two SRVs on the surround texture (UNORM + UNORM_SRGB, parallel to the workspace-mode `atlas_holds_srgb_bytes` pattern in [`compositor-pipeline.md`](../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor)). 3. Run a fullscreen blit shader against the target swapchain with a scissor-rect strategy: - Path A: four scissor-rects (top/bottom/left/right strips around the canvas) → four `CopySubresourceRegion` or quad draws. - Path B: one fullscreen pass with a "skip if inside canvas rect" branch in the pixel shader. diff --git a/docs/specs/extensions/XR_EXT_win32_window_binding.md b/docs/specs/extensions/XR_EXT_win32_window_binding.md index ef213ab4a..c3a6ffccb 100644 --- a/docs/specs/extensions/XR_EXT_win32_window_binding.md +++ b/docs/specs/extensions/XR_EXT_win32_window_binding.md @@ -418,7 +418,7 @@ typedef XrResult (XRAPI_PTR *PFN_xrSetSharedTextureSurround2DEXT)( **Valid Usage:** - Session must have been created with `XrWin32WindowBindingCreateInfoEXT` (or `XrCocoaWindowBindingCreateInfoEXT` on macOS). - The handle must be a D3D11 or D3D12 NT handle (`CreateSharedHandle` flow). DXGI legacy shared handles are not supported. -- The texture's pixel format must be `DXGI_FORMAT_R8G8B8A8_UNORM` or `DXGI_FORMAT_R8G8B8A8_UNORM_SRGB`. The runtime opens both UNORM and UNORM_SRGB SRVs on the underlying resource and selects at sample time based on the texture's actual format, matching the color-space handling pattern documented in [`compositor-pipeline.md`](../../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor-shell-mode). +- The texture's pixel format must be `DXGI_FORMAT_R8G8B8A8_UNORM` or `DXGI_FORMAT_R8G8B8A8_UNORM_SRGB`. The runtime opens both UNORM and UNORM_SRGB SRVs on the underlying resource and selects at sample time based on the texture's actual format, matching the color-space handling pattern documented in [`compositor-pipeline.md`](../../architecture/compositor-pipeline.md#color-space-handling-d3d11-service-compositor). - The texture must include `D3D11_RESOURCE_MISC_SHARED_NTHANDLE | D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX` (D3D12: `D3D12_HEAP_FLAG_SHARED | D3D12_HEAP_FLAG_SHARED_CROSS_ADAPTER` as appropriate) so the runtime can synchronize via `IDXGIKeyedMutex`. - The synchronization key is **0**: app `AcquireSync(0)` before writing, `ReleaseSync(0)` when done; runtime `AcquireSync(0)` before sampling, `ReleaseSync(0)` after submitting the frame. Symmetric to the multiview shared texture. - Call with `sharedTextureHandle = NULL` to disable the surround blit. Non-canvas pixels of the target swapchain then fall back to undefined (the spec v5 behavior). diff --git a/docs/vendors/leia/weaver.md b/docs/vendors/leia/weaver.md index f5f3b9ca3..ecd6bd98e 100644 --- a/docs/vendors/leia/weaver.md +++ b/docs/vendors/leia/weaver.md @@ -474,6 +474,32 @@ void setShaderSRGBConversion(bool read, bool write); - `write = true`: Shader converts output from linear to sRGB before writing to RT. - Set to `false` when hardware handles conversion (e.g., `_SRGB` format views) or when the pipeline is already in the desired color space. +### Mapping to the DisplayXR encoding-state contract (ADR-021) + +This knob is exactly what lets this weaver satisfy the runtime's color contract +([ADR-021](../../adr/ADR-021-color-management-encoding-state-invariant.md)): because the +encoding is selected by API call (not inferred from the input format), this DP's +capability is **`EITHER`**, and the weaver itself owns the matched output encode +(`write`). The DP drives the knob from the atlas encoding the runtime declares per frame: + +| Runtime-declared atlas encoding | `setShaderSRGBConversion` | Result | +|---|---|---| +| Encoded (today's default) | `(false, false)` | weave as-is, output encoded | +| Encoded, weave in linear | `(true, true)` | decode → weave linear → re-encode | +| Linear (ADR-021 Model B) | `(false, true)` | weave linear → encode on output | + +Because the weaver does the output encode, the **runtime needs no encode shader** — the +matched encode partner lives here, in the DP, which is the vendor-isolation-correct place +for it. + +**Current plug-in gaps (tracked for `displayxr-leia-plugin`):** +- The D3D11 wrapper exposes the setter (`leiasr_d3d11_set_srgb_conversion`) but the + display processor **never calls it**, so the weaver currently does no conversion. That + is fine while the runtime sends encoded (passthrough), but means feeding it linear + bytes would render too dark — wire the call to the declared atlas encoding. +- The **D3D12 and GL** wrappers do not expose the setter at all, though the SDK provides + it for every API variant — expose it for parity before enabling a linear compose path. + --- ## Phase Snapping diff --git a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp index 0fbcca7c4..310b2824c 100644 --- a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp +++ b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp @@ -6515,14 +6515,35 @@ multi_compositor_render(struct d3d11_service_system *sys) // based on whether the client's most-recent swapchain was // SRGB-encoded. Atlas storage is TYPELESS in workspace mode (see // init_client_render_resources), so both views were created - // up-front. The SRGB-SRV path makes the GPU auto-linearize - // on sample — the multi-comp shader (passthrough at - // convert_srgb=0) then writes linear values to the combined - // atlas, which is what the DP weaver expects. Falls back to - // the UNORM SRV if the SRGB SRV isn't available (non-workspace - // atlas storage is UNORM and only atlas_srv exists; not - // expected to reach here in non-workspace mode but stays robust). + // up-front. The UNORM SRV is raw passthrough: a client that + // stored encoded bytes in a UNORM swapchain reaches the DP + // correctly. DisplayXR's handoff encoding is whatever the active + // DP accepts (not a fixed value); the runtime sends encoded today + // and the active DP accepts it, so passthrough is correct + // (see ADR-021). + // + // KNOWN LATENT HALF-CONVERSION (ADR-021 / #409): the SRGB-SRV + // branch makes the GPU decode sRGB->linear on sample, but + // NOTHING re-encodes before process_atlas() (the multi-comp + // shader is passthrough at convert_srgb=0; linear_to_srgb() in + // d3d11_service_shaders.h is defined but never called). So an + // sRGB-swapchain client would reach an ENCODED-declaring DP + // ~2.2x too dark -- the same unmatched decode #407/#408 fixed + // in-process. Dead in practice today: every workspace app stores + // encoded-into-UNORM, so atlas_holds_srgb_bytes is false and we + // take the UNORM SRV. Fix is the Model-A passthrough baseline + // (always use the UNORM SRV here); the eventual general fix is the + // declared compose->handoff conversion in ADR-021 §4. Do NOT + // "fix" by linearizing more. if (cc->atlas_holds_srgb_bytes && cc->render.atlas_srv_srgb) { + static bool logged_srgb_halfconv = false; + if (!logged_srgb_halfconv) { + logged_srgb_halfconv = true; + U_LOG_W("Color: sRGB-SRV decode on per-client atlas with no " + "re-encode before process_atlas() -- known latent " + "half-conversion (ADR-021/#409); an ENCODED-declaring " + "DP receives too-dark values for this sRGB client."); + } slot_srv = cc->render.atlas_srv_srgb.get(); } else { slot_srv = cc->render.atlas_srv.get();