diff --git a/docs/adr/ADR-044-colour-contract-per-backend.md b/docs/adr/ADR-044-colour-contract-per-backend.md index 01c9a4a01..519345ae5 100644 --- a/docs/adr/ADR-044-colour-contract-per-backend.md +++ b/docs/adr/ADR-044-colour-contract-per-backend.md @@ -139,7 +139,7 @@ The helper keys on the *target's* format. Pinning a format and calling the helpe - **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 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. **Exception: the macOS shared surface's content pass is honest since #1801.** It sampled each client as its declared format but drew into the encoded atlas through its UNORM view, so every `_SRGB` app was stored one decode too dark (measured: luminance 61 against 131 for the same scene declared UNORM). It now draws through the same `_SRGB` atlas view as the #1795 decorations. The focus-glow style colour is display-referred and is decoded on the CPU first, like a clear colour. A UNORM client there is read as linear and encoded, like vk_native. The Android and Linux service flavours remain unaudited. - **Local2D layers are not format-honest.** The Local2D flatten samples the app's swapchain through a *non-decoding* view and writes a UNORM scratch, on every in-process backend: - D3D11 `comp_d3d11_swapchain_get_srv`, not `get_compose_srv`; - D3D12 `comp_d3d12_swapchain_sample_format`; diff --git a/src/xrt/auxiliary/util/u_color_encoding.h b/src/xrt/auxiliary/util/u_color_encoding.h index aa73ce716..a01e92ba9 100644 --- a/src/xrt/auxiliary/util/u_color_encoding.h +++ b/src/xrt/auxiliary/util/u_color_encoding.h @@ -69,6 +69,28 @@ u_color_srgb_encode(float linear) return 1.055f * powf(linear, 1.0f / 2.4f) - 0.055f; } +/*! + * Inverse of u_color_srgb_encode (display-referred → linear), IEC 61966-2-1. + * + * For CPU-side CONSTANTS only (a display-referred colour handed to a shader + * whose target encodes on write, like a clear colour). Never shader arithmetic + * on sampled pixels: decoding is a property of the `_SRGB` view. + */ +static inline float +u_color_srgb_decode(float encoded) +{ + if (encoded <= 0.0f) { + return 0.0f; + } + if (encoded >= 1.0f) { + return 1.0f; + } + if (encoded <= 0.04045f) { + return encoded / 12.92f; + } + return powf((encoded + 0.055f) / 1.055f, 2.4f); +} + /*! * The 8-bit atlas byte a linear value must land on: `0.0 → 0`, `0.2 → 124`, * `0.5 → 188`, `1.0 → 255`. These four are the #1589 acceptance numbers. diff --git a/src/xrt/compositor/multi/comp_multi_system.c b/src/xrt/compositor/multi/comp_multi_system.c index 1f7bf0ea7..edf6bcd25 100644 --- a/src/xrt/compositor/multi/comp_multi_system.c +++ b/src/xrt/compositor/multi/comp_multi_system.c @@ -5235,7 +5235,16 @@ render_shared_surface_locked(struct multi_system_compositor *msc, int64_t displa // corners + feathered edges. ONE render pass over the whole atlas so overlapping // windows blend in painter (far→near, already-sorted) submission order; every // content source image must therefore be SHADER_READ before the pass begins. - if (shared_ensure_content_blend(msc, vk)) { + // + // ADR-044 §1: the content pass draws into the same target as the decorations + // (#1795). Sources are sampled as their DECLARED format, so the target must + // encode on write: through the atlas's `_SRGB` view (or an `_SRGB` atlas). + // Rendering into the encoded atlas through its UNORM view stored the decoded + // (linear) values of an `_SRGB` client raw, one decode too dark. Under the + // legacy hatch the target is the UNORM atlas view, as before. + struct shared_deco_target content_target = {0}; + if (shared_resolve_deco_target(msc, vk, &content_target) && content_target.content != NULL) { + struct comp_multi_content_blend *content_blend = content_target.content; // Corner radius matches the shell focus ring (fh*0.045) so the content // corners are concentric with it; feather is ~2 px of the window height, // capped at the radius (Windows caps so feather_band = feather/ry ≤ 1). @@ -5313,8 +5322,8 @@ render_shared_surface_locked(struct multi_system_compositor *msc, int64_t displa uniq_n, pre); } - comp_multi_content_blend_begin(&msc->shared_content_blend, vk, cmd, msc->shared_atlas_fb, - (uint32_t)msc->shared_atlas_w, (uint32_t)msc->shared_atlas_h); + comp_multi_content_blend_begin(content_blend, vk, cmd, content_target.fb, (uint32_t)msc->shared_atlas_w, + (uint32_t)msc->shared_atlas_h); // Per client (far→near), per eye: project the window rect, clip it to the // eye tile, remap the source sub-rect, and draw with rounded-rect coverage. // M2: a placed window projects its center pose through each eye onto the @@ -5351,6 +5360,14 @@ render_shared_surface_locked(struct multi_system_compositor *msc, int64_t displa glow_color[1] = st.focus_glow_color[1]; glow_color[2] = st.focus_glow_color[2]; glow_color[3] = st.focus_glow_color[3]; + // The style colour is display-referred. An encoding target + // would encode it again, so hand the shader its linear value + // (a constant, like a clear colour; ADR-044 §2). + if (content_target.honest) { + for (int k = 0; k < 3; k++) { + glow_color[k] = u_color_srgb_decode(glow_color[k]); + } + } if (st.edge_feather_meters > 0.0f && e->win_h_m > 0.0f) { float sf = st.edge_feather_meters / e->win_h_m; if (sf > edge_feather) { @@ -5458,8 +5475,8 @@ render_shared_surface_locked(struct multi_system_compositor *msc, int64_t displa memcpy(pcq.corners, corners, sizeof(corners)); SHARED_SET_HUD_PC(pcq); comp_multi_content_blend_draw_quad( - &msc->shared_content_blend, vk, cmd, im, ai, fmt, hud_img, hud_fmt, &pcq, - tile_x0_e, 0, (uint32_t)eye_w, (uint32_t)eye_h, (uint32_t)msc->shared_atlas_w, + content_blend, vk, cmd, im, ai, fmt, hud_img, hud_fmt, &pcq, tile_x0_e, 0, + (uint32_t)eye_w, (uint32_t)eye_h, (uint32_t)msc->shared_atlas_w, (uint32_t)msc->shared_atlas_h); continue; } @@ -5515,13 +5532,13 @@ render_shared_surface_locked(struct multi_system_compositor *msc, int64_t displa .glow_color = {glow_color[0], glow_color[1], glow_color[2], glow_color[3]}, }; SHARED_SET_HUD_PC(pc); - comp_multi_content_blend_draw(&msc->shared_content_blend, vk, cmd, im, ai, fmt, - hud_img, hud_fmt, &pc, (int32_t)cdx0, (int32_t)cdy0, + comp_multi_content_blend_draw(content_blend, vk, cmd, im, ai, fmt, hud_img, hud_fmt, + &pc, (int32_t)cdx0, (int32_t)cdy0, (uint32_t)(cdx1 - cdx0), (uint32_t)(cdy1 - cdy0)); } #undef SHARED_SET_HUD_PC } - comp_multi_content_blend_end(&msc->shared_content_blend, vk, cmd); + comp_multi_content_blend_end(content_blend, vk, cmd); // Restore the cross-process sources to GENERAL (their rest layout). if (uniq_n > 0) { diff --git a/tests/tests_aux_color_encoding.cpp b/tests/tests_aux_color_encoding.cpp index a81eac94a..fdc58f3e2 100644 --- a/tests/tests_aux_color_encoding.cpp +++ b/tests/tests_aux_color_encoding.cpp @@ -197,3 +197,18 @@ TEST_CASE("DXR_COLOR_LEGACY_UNORM_ENCODED is OFF by default") // `legacy_hatch` argument above is for.) CHECK_FALSE(u_color_legacy_unorm_encoded()); } + +TEST_CASE("u_color_srgb_decode — inverse of the encode, for CPU-side constants") +{ + CHECK(u_color_srgb_decode(0.0f) == 0.0f); + CHECK(u_color_srgb_decode(1.0f) == 1.0f); + // 188/255 is the encode of linear 0.5. + CHECK(u_color_srgb_decode(188.0f / 255.0f) == Catch::Approx(0.5f).margin(0.005f)); + + // Every 8-bit display-referred value survives decode → encode, so a + // decoded constant drawn through an `_SRGB` target lands on its byte. + for (int b = 0; b <= 255; b++) { + INFO("byte " << b); + CHECK(u_color_srgb_encode_u8(u_color_srgb_decode((float)b / 255.0f)) == b); + } +} diff --git a/tests/tests_comp_color_policy.cpp b/tests/tests_comp_color_policy.cpp index ae2b50c86..ad58327a0 100644 --- a/tests/tests_comp_color_policy.cpp +++ b/tests/tests_comp_color_policy.cpp @@ -508,3 +508,23 @@ TEST_CASE("colour: the D3D11 service's passthrough draws sample raw (#1769)") CHECK(direct_proj == 0); CHECK(count_code_lines_with(src, "sys, &c->render, proj_src_srv, src_x,") == 2); } + +TEST_CASE("colour: the macOS shared-surface content pass draws into the encoding target") +{ + /* + * comp_multi samples each client's content as its DECLARED format, so an + * `_SRGB` client arrives decoded. Drawing that into the encoded atlas + * through its plain UNORM framebuffer stored linear values raw: every + * `_SRGB` app one decode too dark (measured: lum 61 vs 131). The pass must + * take the same resolved target as the decorations (#1795), whose + * framebuffer is the atlas's `_SRGB` view unless the legacy hatch is on. + */ + const std::string path = std::string(DXR_COMP_SRC_DIR) + "/multi/comp_multi_system.c"; + const std::string src = read_whole_file(path); + + INFO("the content pass must resolve its target through shared_resolve_deco_target()"); + CHECK(contains_in_code(src, "shared_resolve_deco_target(msc, vk, &content_target)")); + + INFO("no pass may begin on the bare UNORM atlas framebuffer with the UNORM content pipeline"); + CHECK_FALSE(contains_in_code(src, "comp_multi_content_blend_begin(&msc->shared_content_blend")); +}