From adf61dbd9385206da5b28d60f0d9800396640317 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 28 Sep 2026 13:51:54 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(app-rules):=20INV-4.6=20says=20what=20#?= =?UTF-8?q?1589=20made=20true=20=E2=80=94=20UNORM=20is=20linear=20?= =?UTF-8?q?=E2=80=94=20and=20the=20linter=20checks=20it=20per=20leg?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The INV-4.6 guide text still described the pre-#1589 model: "a linear / UNORM swapchain is NOT color-managed ... the compositor passes the bytes straight through". Since the format-honest colour model shipped (D3D11 v2.21.0, D3D12 v2.21.1, GL v2.21.2, vk_native v2.21.7, 748b540f3) an UNORM swapchain is read as LINEAR, as OpenXR specifies, and sRGB-encoded on the way to the panel. An app that stores display-referred bytes in UNORM is encoded twice and looks washed out. The runtime is right; the guide told app authors the opposite. That is what reached the Leia tablet: the Gaussian-splat demo's Android leg prefers {R8G8B8A8_UNORM, B8G8R8A8_UNORM} (unchanged since the leg was written) while its macOS/Linux legs had moved to _SRGB, so the demo was correct on macOS and washed out on Android once the tablet took runtime v2.21.11. Reproduced on macOS vk_native with the Android leg's choice forced: authored (229,182,127) reaches the atlas as (243,220,187) — the exact sRGB encode of the authored value — and v2.20.1 with the same UNORM choice is byte-identical to v2.21.11 with _SRGB. The linter did not catch it because INV-4.6 was app-wide: "an sRGB token appears anywhere" was satisfied by the desktop legs. It is now per leg (shared code counts toward every leg), and in any file that enumerates swapchain formats it flags the two UNORM-first shapes seen in the demos: a preference list naming only UNORM formats, and a scan loop that breaks on the first UNORM. Across the in-tree test_apps the INV-4.6 findings are unchanged; across the five demo repos it flags every Android leg plus the avatar desktop legs. .claude/ (agent worktrees) is now excluded from the scan. scripts/tests/test_check_displayxr_app_color.py pins it (8 hermetic cases, wired into lint.yml); 3 of them fail against the previous linter. The guide also records the on-device A/B for this class: `adb shell setprop debug.xrt.DXR_COLOR_LEGACY_UNORM_ENCODED 1`. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/lint.yml | 6 + docs/guides/displayxr-app-rules.md | 35 +++-- scripts/check_displayxr_app.py | 97 +++++++++++-- .../tests/test_check_displayxr_app_color.py | 129 ++++++++++++++++++ 4 files changed, 240 insertions(+), 27 deletions(-) create mode 100644 scripts/tests/test_check_displayxr_app_color.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c05d988cd2..1800fed72b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -224,3 +224,9 @@ jobs: - name: Pin-bump gate unit tests run: python3 scripts/tests/test_downstream_pin_bump.py + + - name: App-linter INV-4.6 colour-swapchain unit tests + # Hermetic (synthetic app trees). Pins the per-leg _SRGB-first check that + # would have caught the gauss demo's Android leg preferring UNORM after + # #1589 made UNORM swapchains linear. + run: python3 scripts/tests/test_check_displayxr_app_color.py diff --git a/docs/guides/displayxr-app-rules.md b/docs/guides/displayxr-app-rules.md index 7cad58d057..4e3485bb00 100644 --- a/docs/guides/displayxr-app-rules.md +++ b/docs/guides/displayxr-app-rules.md @@ -480,9 +480,11 @@ re-implementing — see [INV-8.1](#8-app-folder-layout--what-to-include)). `eyeCount = 1`, render at full window resolution, center-eye = average of the located eyes. The runtime accepts `viewCount == 1` only in a 2D/non-3D mode. Ref: `main.cpp:415,632-644,707-711`. -- **INV-4.6 — Color space: request an sRGB swapchain.** The display panel is sRGB, and an - in-process native compositor is a **byte passthrough to the panel** — it does *not* color-manage - a linear-format swapchain. So the encoding of the bytes you store is what reaches the display. +- **INV-4.6 — Color space: request an sRGB swapchain.** The display panel is sRGB, and the + runtime reads a swapchain the way OpenXR says to: an `*_SRGB` format holds **encoded** colour and + reaches the panel as stored; every other format "will be treated as linear values" and is + **sRGB-encoded** on the way to the panel (#1589). So what you store must match the format you + asked for. - **Do this — request an sRGB swapchain format:** `..._UNORM_SRGB` (D3D11/D3D12), `GL_SRGB8_ALPHA8` (GL), `..._SRGB` (Vulkan), `MTLPixelFormat*sRGB` (Metal). Then you are correct **either way you render into it**: @@ -499,15 +501,22 @@ re-implementing — see [INV-8.1](#8-app-folder-layout--what-to-include)). scratch image of the swapchain's UNORM sibling, then `vkCmdCopyImage` into the swapchain (a copy never converts). This is byte-exact on any runtime; - clear with the linearized colour (sRGB EOTF), not the display-referred one. - - **A linear / UNORM swapchain (`R8G8B8A8_UNORM`, `GL_RGBA8`, …) is NOT color-managed** on the - in-process path: the compositor passes the bytes straight through with no linear→sRGB encode. - It's only correct if you write display-referred bytes into it; writing genuinely-linear values - and expecting the runtime to encode → **too bright**. ("Submit a linear-format swapchain and - the runtime color-manages it" is a future runtime follow-up, not current behavior.) **Subject to - change — see [#409](https://github.com/DisplayXR/displayxr-runtime/issues/409):** if the color - model lands on linear-compose ("Model B"), a linear-format swapchain *would* be honored as - linear (the runtime would encode at the DP boundary). This bullet describes current ("Model A") - behavior; the recommendation above (request an sRGB swapchain) stays correct either way. + - **A linear / UNORM swapchain (`R8G8B8A8_UNORM`, `GL_RGBA8`, …) holds LINEAR values** and the + runtime encodes it. Display-referred (already gamma-encoded) bytes stored in one are encoded a + **second** time → **washed out**: lifted blacks, desaturated colour (a splat demo's authored + `(229,182,127)` reaches the atlas as `(243,220,187)`). This is the #1589/#1610 format-honest + colour model, shipped per backend — D3D11 in v2.21.0, D3D12 v2.21.1, GL v2.21.2, **Vulkan + (`vk_native`, incl. Android) v2.21.7**; Metal still passes UNORM bytes through. Before those + releases a UNORM swapchain *was* a byte passthrough, which is why an app that picked UNORM and + wrote display-referred bytes looked right until it met a newer runtime — including a leg that + lagged its siblings (an Android leg that still preferred `{R8G8B8A8_UNORM, B8G8R8A8_UNORM}` + after the desktop legs had moved to `_SRGB`). **Every leg of a multi-platform app must make the + same `_SRGB`-first choice** — route it through one shared helper rather than a per-leg + preference list; `scripts/check_displayxr_app.py` flags a UNORM-first preference per leg. + - **A/B on a runtime:** `DXR_COLOR_LEGACY_UNORM_ENCODED=1` restores the old passthrough reading + of UNORM (on Android: `adb shell setprop debug.xrt.DXR_COLOR_LEGACY_UNORM_ENCODED 1`, then + relaunch the app). If that makes a washed-out app look right, the app is storing + display-referred bytes in UNORM — fix the app, the knob is a diagnostic, not a setting. - **Data textures (normal, AO, roughness, metalness) are ALWAYS linear — never sRGB.** Only albedo/color is sRGB; sample those through an `_SRGB` texture view (or decode in-shader) so lighting runs in linear, then write your result into the sRGB swapchain per above. @@ -1047,7 +1056,7 @@ macOS app (no manifest → no Android findings). - [ ] View configuration chosen at startup: N-view app begins `PRIMARY_MULTIVIEW_DXR` (when enumerated), stereo-fixed app stays on `PRIMARY_STEREO`; `xrLocateViews` into an 8-wide buffer; **render** `eyeCount` from the active mode, not 2 (INV-3.1) - [ ] Projection layer **submits the LOCATED count**, with the inactive tail aliased via `DxrAliasInactiveViews()` — including every 3D zone layer (INV-3.4) - [ ] App swapchain sized once to worst-case atlas (INV-4.2); per-tile = window/canvas × scaleXY, never display (INV-4.3) -- [ ] Color space: request an **sRGB swapchain** and write a correctly-encoded image (linear render + GPU sRGB-write, or display-referred bytes — not both); linear/UNORM swapchain is not color-managed; data textures always linear (INV-4.6) +- [ ] Color space: request an **sRGB swapchain** and write a correctly-encoded image (linear render + GPU sRGB-write, or display-referred bytes — not both); a linear/UNORM swapchain is read as LINEAR and encoded by the runtime (display-referred bytes in it wash out); same `_SRGB`-first choice on every leg; data textures always linear (INV-4.6) - [ ] Whole declared `imageRect` is written — partial-tile renders clear the full tile to `(0,0,0,0)` first (or shrink the rect); no undefined pixels reach the atlas, esp. transparent-bg (INV-4.7) - [ ] (if `XR_EXT_view_configuration_views_change` is enabled) the handler re-enumerates and **moves `subImage.imageRect`** — no `xrCreateSwapchain` from the event; filters on `systemId` **and** `viewConfigurationType` (INV-4.9) - [ ] (texture) regions declared via display-zones — 3D zones + Local2D zones, not output-rect/surround (INV-5.1/5.3) diff --git a/scripts/check_displayxr_app.py b/scripts/check_displayxr_app.py index 1b41de6411..d521c21c18 100755 --- a/scripts/check_displayxr_app.py +++ b/scripts/check_displayxr_app.py @@ -42,7 +42,7 @@ # Directories never linted: shared reference code, vendored headers, build output. EXCLUDE_DIRS = { "build", ".git", "third_party", "openxr_includes", "common", - "_package", "__pycache__", "node_modules", ".vs", "out", + "_package", "__pycache__", "node_modules", ".vs", "out", ".claude", } SOURCE_EXTS = {".c", ".cc", ".cpp", ".cxx", ".h", ".hh", ".hpp", ".m", ".mm"} # JVM sources are linted for the Android rules only (§11) — the C/C++ checks @@ -56,7 +56,7 @@ "INV-3.1": "An N-view app BEGINS XR_VIEW_CONFIGURATION_TYPE_PRIMARY_MULTIVIEW_DXR (when enumerated; needs XR_DXR_display_info), locates into an XRT_MAX_VIEWS (8)-wide buffer and submits the active mode's viewCount. A stereo-fixed app stays on PRIMARY_STEREO and receives exactly 2. Deriving eyeCount from the rendering mode's viewCount without the opt-in is an error: PRIMARY_STEREO reports 2 and rejects viewCount>2.", "INV-3.4": "A projection layer carries the LOCATED view count (what xrLocateViews returned), for every view configuration type. Render only the active mode's views, then alias the inactive tail [active, located) onto view 0's subImage keeping each view's own located pose/fov (DxrAliasInactiveViews in dxr_view_config.h: test_apps/common in-tree, displayxr-common outside). Checked per leg: every platform directory of a multi-leg app must call it itself. Submitting the ACTIVE count is under-submit and xrEndFrame now refuses it (ADR-041). A 3D zone layer is a projection layer and obeys the same rule.", "INV-4.3": "Per-tile render size = window/canvas x scaleXY, never display size.", - "INV-4.6": "Request an sRGB swapchain (and store a correctly-encoded image); don't double-encode.", + "INV-4.6": "Request an sRGB swapchain (and store a correctly-encoded image); don't double-encode. A UNORM swapchain is read as LINEAR and encoded by the runtime (#1589), so display-referred bytes in it wash out. Checked per leg: every platform directory must choose _SRGB first.", "INV-4.7": "Write every pixel of the imageRect you declare — clear partial-tile renders to (0,0,0,0) first (or shrink the rect); undefined pixels read as opaque magenta on MoltenVK and break transparent-bg.", "INV-4.9": "An app that enables XR_EXT_view_configuration_views_change must not call xrCreateSwapchain from its event handler — move subImage.imageRect instead (an app sized at maxImageRect* per ADR-010 never needs to reallocate).", "INV-5.9": "VK apps MUST use XR_KHR_vulkan_enable2 (the runtime creates the VkDevice via xrCreateVulkanDeviceKHR); an app-side vkCreateDevice = enable1, which forfeits the #868 weave-rate decoupling and the late-weave pacing.", @@ -156,6 +156,27 @@ def __init__(self, level, rule, path, line, msg, fix): re.IGNORECASE, ) CREATES_SWAPCHAIN = re.compile(r"\bxrCreateSwapchain\b") +ENUMERATES_SWAPCHAIN_FORMATS = re.compile(r"\bxrEnumerateSwapchainFormats\b") +# INV-4.6, the per-leg shapes of "this leg prefers UNORM" (#1589). Since the +# format-honest colour model (D3D11 v2.21.0 ... vk_native v2.21.7) an UNORM +# swapchain holds LINEAR values and is sRGB-encoded on the way to the panel, so +# an app that picks UNORM and stores display-referred bytes is encoded twice — +# washed out. Every leg used to get away with it because UNORM was a byte +# passthrough; the one that did not get migrated (an Android leg preferring +# {R8G8B8A8_UNORM, B8G8R8A8_UNORM} after its desktop siblings moved to _SRGB) +# is what these catch. Only checked in files that enumerate swapchain formats. +_UNORM8 = r"(?:VK_FORMAT|DXGI_FORMAT)_[RB]8G8[RB]8A8_UNORM\b" +# (a) a preference list naming only 8-bit UNORM colour formats (2+ entries): +# const int64_t preferred[] = {VK_FORMAT_R8G8B8A8_UNORM, VK_FORMAT_B8G8R8A8_UNORM}; +UNORM_ONLY_PREFERENCE_LIST = re.compile( + r"\{\s*(?:\(\s*\w+\s*\)\s*)?" + _UNORM8 + + r"(?:\s*,\s*(?:\(\s*\w+\s*\)\s*)?" + _UNORM8 + r")+\s*,?\s*\}" +) +# (b) a scan loop that STOPS at the first UNORM it meets: +# if (f == VK_FORMAT_B8G8R8A8_UNORM || ...) { selected = f; break; } +UNORM_FIRST_BREAK = re.compile( + r"==\s*" + _UNORM8 + r"[^;{}]*\)\s*\{[^{}]*\bbreak\b" +) # INV-3.1 (#1486). Two different questions, two different markers: # # MULTIVIEW_OPT_IN — does the app BEGIN the N-view view configuration? Only a @@ -291,7 +312,6 @@ def scan_sources(root: Path, findings: list): stereo_fixed_marked = True stereo_fixed_legs.add(leg_of(path, root)) is_multiview_app = any(MULTIVIEW_OPT_IN.search(t) for _, t in files) - any_srgb = any(SRGB_TOKENS.search(t) for _, t in files) # INV-3.1 (#1486): mode-derived view counts WITHOUT the PRIMARY_MULTIVIEW_DXR # opt-in. The app reads the active mode's viewCount (so it can reach 4) but @@ -375,7 +395,6 @@ def scan_sources(root: Path, findings: list): "3D zone layers are projection layers and need the same treatment, per zone.", )) - swapchain_loc = None for path, text in files: for regex, level, rule, msg, fix, multiview_only in SRC_PATTERNS: if multiview_only and not is_multiview_app: @@ -383,10 +402,6 @@ def scan_sources(root: Path, findings: list): for m in regex.finditer(text): line_no = text.count("\n", 0, m.start()) + 1 findings.append(Finding(level, rule, rel(path, root), line_no, msg, fix)) - if swapchain_loc is None: - m = CREATES_SWAPCHAIN.search(text) - if m: - swapchain_loc = (rel(path, root), text.count("\n", 0, m.start()) + 1) # INV-5.9 (enforced): a VK app that creates its own VkDevice is using # XR_KHR_vulkan_enable (enable1). enable1 forfeits the runtime-owned VkQueue @@ -411,14 +426,68 @@ def scan_sources(root: Path, findings: list): "app-side vkCreateInstance / vkCreateDevice). Reference: test_apps/handle/cube_handle_vk_win.", )) - # INV-4.6 advisory: creates a swapchain but no sRGB format appears anywhere. - if swapchain_loc and not any_srgb: - p, ln = swapchain_loc + check_color_swapchain_per_leg(files, root, findings) + + +def check_color_swapchain_per_leg(files, root: Path, findings: list): + """INV-4.6, per leg. A multi-leg app is only as right as its worst leg: the + shared/single group ("") counts toward every leg, but one leg's _SRGB choice + does not cover another's (the Android leg of a splat demo kept preferring + UNORM for months after its desktop legs moved to _SRGB, and an app-wide + "any sRGB token anywhere" check was satisfied by the desktop legs).""" + fix_unorm = ( + "Choose the first _SRGB format the runtime enumerates (fall back to UNORM only if none), " + "the same way on every leg — ideally through one shared helper. Store display-referred " + "bytes in it by a raw copy (blit into an UNORM-sibling scratch, then vkCmdCopyImage) or " + "render linear and let the GPU encode; linearize clear colours. Since #1589 an UNORM " + "swapchain is read as LINEAR and encoded by the runtime, so display-referred bytes in it " + "come out washed out. A/B: DXR_COLOR_LEGACY_UNORM_ENCODED=1 (Android: setprop " + "debug.xrt.DXR_COLOR_LEGACY_UNORM_ENCODED 1) restores the old passthrough." + ) + by_leg = {} + for path, text in files: + by_leg.setdefault(leg_of(path, root), []).append((path, text)) + shared = by_leg.get("", []) + shared_srgb = any(SRGB_TOKENS.search(t) for _, t in shared) + + for leg, leg_files in sorted(by_leg.items()): + # (a)/(b): a UNORM-first choice, wherever the leg enumerates formats. + for path, text in leg_files: + if not ENUMERATES_SWAPCHAIN_FORMATS.search(text): + continue + for regex, what in ( + (UNORM_ONLY_PREFERENCE_LIST, "a swapchain-format preference list naming only UNORM formats"), + (UNORM_FIRST_BREAK, "a swapchain-format scan that stops at the first UNORM format"), + ): + for m in regex.finditer(text): + findings.append(Finding( + WARN, "INV-4.6", rel(path, root), text.count("\n", 0, m.start()) + 1, + f"UNORM-first colour swapchain choice ({what}). An UNORM swapchain is " + "read as LINEAR and encoded by the runtime (#1589): display-referred " + "bytes stored in it are encoded twice and look washed out.", + fix_unorm, + )) + # The leg creates a swapchain but neither it nor the shared code ever + # names an sRGB format. + if leg == "" and len(by_leg) > 1: + continue # shared code alone is not a leg + loc = None + for path, text in leg_files: + m = CREATES_SWAPCHAIN.search(text) + if m: + loc = (rel(path, root), text.count("\n", 0, m.start()) + 1) + break + if loc is None: + continue + if any(SRGB_TOKENS.search(t) for _, t in leg_files) or shared_srgb: + continue findings.append(Finding( - WARN, "INV-4.6", p, ln, - "No sRGB swapchain format detected — INV-4.6 recommends an sRGB swapchain.", + WARN, "INV-4.6", loc[0], loc[1], + "No sRGB swapchain format detected" + (f" in the {leg}/ leg" if leg else "") + + " — INV-4.6 recommends an sRGB swapchain.", "Request an sRGB swapchain (_UNORM_SRGB / GL_SRGB8_ALPHA8 / _SRGB / MTLPixelFormat*sRGB). " - "A UNORM swapchain is valid ONLY if you store display-referred (already-encoded) bytes.", + "A UNORM swapchain is read as LINEAR and encoded by the runtime (#1589); it is valid " + "only if you store scene-linear values in it.", )) diff --git a/scripts/tests/test_check_displayxr_app_color.py b/scripts/tests/test_check_displayxr_app_color.py new file mode 100644 index 0000000000..de4ac14ca1 --- /dev/null +++ b/scripts/tests/test_check_displayxr_app_color.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +"""Unit tests for the INV-4.6 colour-swapchain checks in scripts/check_displayxr_app.py. + +Hermetic: each case writes a tiny synthetic app tree to a temp dir and lints it. + +The shapes are taken from real demo legs. Since the #1589 format-honest colour +model (vk_native in v2.21.7) an UNORM swapchain is read as LINEAR and encoded by +the runtime, so a leg that picks UNORM and stores display-referred bytes is +encoded twice and looks washed out. The Android leg of the Gaussian-splat demo +did exactly that through v1.29.0 while its desktop legs had moved to _SRGB, and +the old app-wide "any sRGB token anywhere" check was satisfied by the desktop +legs, so nothing flagged it. + + python3 scripts/tests/test_check_displayxr_app_color.py +""" +from __future__ import annotations + +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +import check_displayxr_app as lint # noqa: E402 + +ENUM = "xrEnumerateSwapchainFormats(s, n, &n, formats);\nxrCreateSwapchain(s, &ci, &sc);\n" + +# The Android leg of displayxr-demo-gaussiansplat v1.29.0 (android/src/main/cpp/main.cpp:923). +ANDROID_UNORM_LIST = ENUM + """ +const int64_t preferred[] = {VK_FORMAT_R8G8B8A8_UNORM, VK_FORMAT_B8G8R8A8_UNORM}; +for (int64_t pref : preferred) { for (uint32_t i = 0; i < n; ++i) if (formats[i] == pref) fmt = pref; } +""" + +# A desktop leg that stops at the first UNORM (displayxr-demo-avatar v0.14.0 macos/main.mm). +UNORM_FIRST_BREAK = ENUM + """ +for (auto f : fmts) { + if (f == VK_FORMAT_B8G8R8A8_UNORM || f == VK_FORMAT_R8G8B8A8_UNORM) { selectedFmt = f; break; } + if (f == VK_FORMAT_B8G8R8A8_SRGB || f == VK_FORMAT_R8G8B8A8_SRGB) selectedFmt = f; +} +""" + +# The correct shape: stop at the first _SRGB, remember a UNORM only as a fallback. +SRGB_FIRST_BREAK = ENUM + """ +for (auto f : fmts) { + if (f == VK_FORMAT_B8G8R8A8_SRGB || f == VK_FORMAT_R8G8B8A8_SRGB) { selectedFmt = f; break; } + if (f == VK_FORMAT_B8G8R8A8_UNORM || f == VK_FORMAT_R8G8B8A8_UNORM) selectedFmt = f; +} +""" + +# Choosing through a shared helper (no format token in the leg itself). +VIA_HELPER = ENUM + "fmt = gsChooseSwapchainFormat(formats, n);\n" + +# A mutable-format list on an intermediate image, not a swapchain preference. +MUTABLE_LIST = ENUM + """ +const VkFormat view_formats[] = {VK_FORMAT_R8G8B8A8_UNORM, VK_FORMAT_R8G8B8A8_SRGB}; +""" + +SHARED_HELPER = """ +bool gsIsSrgbFormat(VkFormat f) { return f == VK_FORMAT_R8G8B8A8_SRGB || f == VK_FORMAT_B8G8R8A8_SRGB; } +""" + + +def lint_tree(files: dict[str, str]) -> list: + with tempfile.TemporaryDirectory() as d: + root = Path(d) + for rel, text in files.items(): + p = root / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(text) + findings: list = [] + lint.scan_sources(root, findings) + return [f for f in findings if f.rule == "INV-4.6"] + + +class Inv46PerLeg(unittest.TestCase): + def test_android_unorm_list_flagged_although_desktop_legs_are_srgb(self): + got = lint_tree({ + "android/src/main/cpp/main.cpp": ANDROID_UNORM_LIST, + "macos/main.mm": SRGB_FIRST_BREAK, + "linux/main.cpp": SRGB_FIRST_BREAK, + "3dgs_common/gs_vulkan_utils.cpp": SHARED_HELPER, + }) + self.assertEqual([(f.path, f.line) for f in got], [("android/src/main/cpp/main.cpp", 4)]) + self.assertIn("preference list naming only UNORM", got[0].msg) + + def test_unorm_first_break_flagged(self): + got = lint_tree({"macos/main.mm": UNORM_FIRST_BREAK}) + self.assertEqual(len(got), 1) + self.assertIn("stops at the first UNORM", got[0].msg) + + def test_fixed_tree_is_clean(self): + got = lint_tree({ + "android/src/main/cpp/main.cpp": VIA_HELPER, + "macos/main.mm": VIA_HELPER, + "linux/main.cpp": VIA_HELPER, + "3dgs_common/gs_vulkan_utils.cpp": SHARED_HELPER, + }) + self.assertEqual(got, []) + + def test_srgb_first_scan_is_clean(self): + self.assertEqual(lint_tree({"linux/main.cpp": SRGB_FIRST_BREAK}), []) + + def test_mutable_format_list_is_not_a_preference(self): + self.assertEqual(lint_tree({"linux/main.cpp": MUTABLE_LIST}), []) + + def test_leg_with_no_srgb_anywhere_is_named(self): + got = lint_tree({ + "android/src/main/cpp/main.cpp": ENUM, + "macos/main.mm": SRGB_FIRST_BREAK, + }) + self.assertEqual(len(got), 1) + self.assertEqual(got[0].path, "android/src/main/cpp/main.cpp") + self.assertIn("android/ leg", got[0].msg) + + def test_single_leg_app_without_srgb(self): + got = lint_tree({"main.cpp": ENUM}) + self.assertEqual(len(got), 1) + self.assertIn("No sRGB swapchain format detected", got[0].msg) + + def test_commented_out_unorm_list_is_ignored(self): + got = lint_tree({ + "linux/main.cpp": VIA_HELPER + "// was: {VK_FORMAT_R8G8B8A8_UNORM, VK_FORMAT_B8G8R8A8_UNORM}\n", + "shared/util.cpp": SHARED_HELPER, + }) + self.assertEqual(got, []) + + +if __name__ == "__main__": + unittest.main(verbosity=2) From a127cd3df3574b4dfabca85f3f878b24ef6ac6d7 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 28 Sep 2026 14:16:36 -0700 Subject: [PATCH 2/2] fix(check_displayxr_app): judge 'no sRGB' per leg only where the leg chooses the format A leg that creates swapchains in a format negotiated elsewhere (displayxr- common's session code; avatar's windows zone swapchain reuses the main format) was newly warned. It falls back to the app-wide test the check always had; a leg that enumerates formats is still judged on its own. Linted all five demos: every current main is flagged on the UNORM-first legs, and all five fix branches (gauss #138, avatar #113, earthview #76, modelviewer #154, mediaplayer #89) are INV-4.6-clean. test_apps unchanged. Co-Authored-By: Claude Opus 5.5 --- scripts/check_displayxr_app.py | 12 ++++++++++-- scripts/tests/test_check_displayxr_app_color.py | 9 +++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/scripts/check_displayxr_app.py b/scripts/check_displayxr_app.py index d521c21c18..923c8f8c68 100755 --- a/scripts/check_displayxr_app.py +++ b/scripts/check_displayxr_app.py @@ -449,6 +449,7 @@ def check_color_swapchain_per_leg(files, root: Path, findings: list): by_leg.setdefault(leg_of(path, root), []).append((path, text)) shared = by_leg.get("", []) shared_srgb = any(SRGB_TOKENS.search(t) for _, t in shared) + app_srgb = any(SRGB_TOKENS.search(t) for _, t in files) for leg, leg_files in sorted(by_leg.items()): # (a)/(b): a UNORM-first choice, wherever the leg enumerates formats. @@ -468,7 +469,10 @@ def check_color_swapchain_per_leg(files, root: Path, findings: list): fix_unorm, )) # The leg creates a swapchain but neither it nor the shared code ever - # names an sRGB format. + # names an sRGB format. Judged per leg only where the leg CHOOSES the + # format (enumerates it); a leg that creates swapchains in a format + # negotiated elsewhere (displayxr-common's session code, "reuse the + # main format") falls back to the app-wide test the check always had. if leg == "" and len(by_leg) > 1: continue # shared code alone is not a leg loc = None @@ -479,7 +483,11 @@ def check_color_swapchain_per_leg(files, root: Path, findings: list): break if loc is None: continue - if any(SRGB_TOKENS.search(t) for _, t in leg_files) or shared_srgb: + leg_chooses = any(ENUMERATES_SWAPCHAIN_FORMATS.search(t) for _, t in leg_files) + if leg_chooses: + if any(SRGB_TOKENS.search(t) for _, t in leg_files) or shared_srgb: + continue + elif app_srgb: continue findings.append(Finding( WARN, "INV-4.6", loc[0], loc[1], diff --git a/scripts/tests/test_check_displayxr_app_color.py b/scripts/tests/test_check_displayxr_app_color.py index de4ac14ca1..cd6254512d 100644 --- a/scripts/tests/test_check_displayxr_app_color.py +++ b/scripts/tests/test_check_displayxr_app_color.py @@ -112,6 +112,15 @@ def test_leg_with_no_srgb_anywhere_is_named(self): self.assertEqual(got[0].path, "android/src/main/cpp/main.cpp") self.assertIn("android/ leg", got[0].msg) + def test_leg_reusing_a_negotiated_format_is_not_flagged(self): + # displayxr-demo-avatar windows/main.cpp: a zone swapchain created with + # `ci.format = xr->swapchain.format` (chosen by displayxr-common). + got = lint_tree({ + "windows/main.cpp": "ci.format = xr->swapchain.format;\nxrCreateSwapchain(s, &ci, &sc);\n", + "macos/main.mm": SRGB_FIRST_BREAK, + }) + self.assertEqual(got, []) + def test_single_leg_app_without_srgb(self): got = lint_tree({"main.cpp": ENUM}) self.assertEqual(len(got), 1)