Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
35 changes: 22 additions & 13 deletions docs/guides/displayxr-app-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**:
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
105 changes: 91 additions & 14 deletions scripts/check_displayxr_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -375,18 +395,13 @@ 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:
continue
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
Expand All @@ -411,14 +426,76 @@ 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)
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.
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. 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
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
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", 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.",
))


Expand Down
Loading
Loading