Repository navigation
feat(color): window-space swapchains default to _SRGB (ADR-044 §7) - #72
Merged
Merged
Conversation
Window-space (HUD / toast / button / Local2D) layers carry DISPLAY-REFERRED R8G8B8A8 bytes: HudRenderer rasterizes with D2D/DirectWrite into a private R8G8B8A8_UNORM texture, HudRendererMacOS into a kCGColorSpaceSRGB bitmap, and the app moves them into the swapchain image. CreateWindowSpaceSwapchain and both CreateHudSwapchain overloads still preferred R8G8B8A8_UNORM, so on a format-honest runtime (D3D11 >= v2.21.0, D3D12 >= v2.21.1, GL >= v2.21.2, vk_native >= v2.21.7) those bytes were read as LINEAR and encoded a second time: the translucent black backdrop lifted to grey and the text washed out. This is the open item ADR-044 §7 names; INV-4.6 is the rule. - color_policy.h: dxr::ChooseWindowSpaceSwapchainFormat(), header-inline so displayxr::rules consumers (the Android legs) can choose through the same rule. Default / =srgb: the first R8G8B8A8 _SRGB code advertised together with its UNORM sibling (DXGI 29/28, VK 43/37, GL_SRGB8_ALPHA8/GL_RGBA8, Metal 71/70); else the R8G8B8A8 UNORM code (fellBack, warned); else formats[0]. =unorm: the R8G8B8A8 UNORM code, exactly the old choice, so DXR_SWAPCHAIN_ENCODING (#49) now A/Bs projection and window-space together. The family never follows formats[0]'s channel order: the copy is family-bound, and 28 -> 29 is the same R8G8B8A8_TYPELESS family the runtime allocates, so CopyTextureRegion / CopyResource / UpdateSubresource / vkCmdCopyBufferToImage / glTexSubImage2D / replaceRegion: stay legal and byte-exact. The pair requirement keeps a colliding code from another API (29 is also VK_FORMAT_R8G8B8_SRGB) from being taken. Pure; never notes the projection format, so RenderSceneLinear() is unaffected. - xr_session_common.cpp CreateWindowSpaceSwapchain and xr_window_space_hud.cpp CreateHudSwapchain use it; the misleading "deliberately NOT migrated" comments are rewritten. - Consumer audit (in-library): HudRenderer clears/draws only its private UNORM texture (transparent clears, D2D never binds the swapchain image); CachedLayerUploader is a raw vkCmdCopyBufferToImage. No library code renders, blits or clears into a window-space image. Header notes on hud_renderer.h, hud_renderer_macos.h, vk_overlay_kit.h, xr_session_common.h and xr_window_space_hud.h state the raw-copy rule. - README: "Window-space swapchains: _SRGB by default too" with the selection table and the per-method verdict table (raw copy OK; _SRGB RTV / D2D on the image / vkCmdBlitImage / GL draw with GL_FRAMEBUFFER_SRGB / non-zero clears are not, with the fix for each). - tests/common_smoke.cpp: test_window_space_format_policy() pins the choice on the four lists the runtime advertises, =unorm/=srgb, the fallbacks, the collision guard, and that the projection note is untouched. CMakeLists VERSION -> 2.27.0 (behaviour change; it had drifted to 2.20.0). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Addendum to the hardware section, so the verification claims are complete:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Window-space (HUD / toast / button / Local2D) swapchains now default to the R8G8B8A8
_SRGBsibling: DXGI 29, VK 43,GL_SRGB8_ALPHA8, or Metal 71. Before this they used R8G8B8A8 UNORM. This closes the open item named in runtime ADR-044 §7:The rule is INV-4.6. An
_SRGBswapchain holds encoded colour. A UNORM swapchain holds linear values, and the runtime sRGB-encodes them on D3D11 ≥ v2.21.0, D3D12 ≥ v2.21.1, GL ≥ v2.21.2 and vk_native ≥ v2.21.7. The HUD bytes are display-referred:HudRendererdraws them with D2D/DirectWrite into a private UNORM texture, andHudRendererMacOSdraws them into akCGColorSpaceSRGBbitmap. So the honest declaration for them is_SRGB.It mirrors #49, which did the same for projection swapchains: same
ColorFormatChoice, sameDXR_SWAPCHAIN_ENCODINGsemantics, so one switch now A/Bs both kinds of swapchain.Format-selection change
dxr::ChooseWindowSpaceSwapchainFormat(formats, pref)is new incolor_policy.h. It is header-inline, sodisplayxr::rulesconsumers (the Android legs) can choose through the same rule. It is used by:CreateWindowSpaceSwapchain/CreateHudSwapchain(XrSessionManager&)inxr_session_common.cpp;CreateHudSwapchain(XrSession, …)inxr_window_space_hud.cpp, the overload the macOS GL/Metal/VK test apps use. That covers Metal 70 → 71.DXR_SWAPCHAIN_ENCODINGsrgb_SRGBcode advertised together with its UNORM sibling:29/28,43/37,0x8C43/0x8058or71/70. If there is none, the R8G8B8A8 UNORM code, with oneWARN. If that is missing too,formats[0].unormformats[0]'s channel order, because vk_native lists BGRA first. The copy into the image is bound to the format family, and28 → 29stays inside the sameR8G8B8A8_TYPELESSfamily. I confirmed that the runtime allocates TYPELESS resources for both D3D11 and D3D12:d3d_dxgi_format_to_typeless_dxgimapsR8G8B8A8_UNORM_SRGBandR8G8B8A8_UNORMtoR8G8B8A8_TYPELESS, ind3d_d3d11_allocator.cpp:88andd3d_d3d12_allocator.cpp:95. SoCopyTextureRegion/CopyResource/UpdateSubresourcestay legal and byte-exact.29is alsoVK_FORMAT_R8G8B8_SRGB.NoteColorSwapchainFormat(), soRenderSceneLinear()is still decided by the projection swapchain alone.Consumer audit
Inside displayxr-common: every path that touches a window-space image
_SRGBHudRendererRenderHudAndMap/RenderButtonStandalone/RenderToastStandalone(hud_renderer.cpp)ClearRenderTargetView+ D2D draw into a privateR8G8B8A8_UNORMtexture →CopyResource→ staging →Map(CPU bytes)(0,0,0,0)or(0,0,0,0.5)on a UNORM target, where 0 is 0 in any space.hud_renderer.h(raw copy only)text_overlay.cppRenderText/RenderButton/RenderToast/RenderFilledRectCreateDxgiSurfaceRenderTargeton the texture it is givenHudRenderer's private UNORM texture. D2D cannot bind an_SRGBsurface, so an app must not point it at the swapchain image.hud_renderer.hnoteHudRendererMacOS(hud_renderer_macos.mm)kCGColorSpaceSRGB(CPU)replaceRegion:/glTexSubImage2D/ VK staging copy)dxr::CachedLayerUploader::upload(vk_overlay_kit.h)vkCmdCopyBufferToImageinto the swapchain imagevkCmdBlitImagedxr::PipelinedReadback(vk_overlay_kit.h)vkCmdCopyImageToBuffer(read)toast.cpp(ToastState,ComputeToastLayerRect)xr_window_space_hud.cppSubmitWindowSpace*Frame,EndFrameWithWindowSpaceLayersNo library code renders, blits or clears into a window-space swapchain image. All the writes happen in apps.
The coordinator's constraints, one by one
_SRGBDXGI surface. Satisfied.HudRendererdraws D2D into a privateR8G8B8A8_UNORMtexture and reads it back. No D2D draw reaches the swapchain image, in the library or in any audited app. The runtime's D3D11/D3D12 swapchain textures areR8G8B8A8_TYPELESS(see above), so an app that does want to draw could still create a UNORM (28) RTV over them. The README table says so.vkCmdCopyBufferToImageorvkCmdCopyImage:CachedLayerUploader, earthview's inline copies, avatar, mediaplayer's staging→vkCmdCopyImage, and the runtime test apps. None of them usesvkCmdBlitImage.GL_FRAMEBUFFER_SRGBis never enabled in any audited repo. The only mention is a TODO comment in runtimecube_handle_gl_win/gl_renderer.cpp:388. Every GL HUD write isglTexSubImage2D, an upload that the flag does not affect. The README tells drawing apps to turn the flag off for that pass and restore it afterwards.cube_zones_vk_{linux,macos}(Local2D strips). It already goes throughdxr::VkDisplayReferredClearColor, and those strips use the projection format, not this chooser.RGBA8Unorm_sRGB(71) is correct but unverified. Metal is not format-honest yet and passes bytes through, so this should be a no-op there. CI only compiles the macOS leg, so a macOS atlas check is still needed before relying on the Metal row.Downstream: what each consumer does, so the follow-up demo audit has a list
v2.25.0; Androiddisplayxr_common_rulesv2.15.0hud_bar.cpp:209is its own swapchain, uses the projection format, and is a debug pathCachedLayerUploader(raw); AndroidvkCmdCopyBufferToImage(raw)v2.25.0; Android rulesv2.16.0CachedLayerUploader(raw)v2.25.0; Androiddisplayxr_common_dxrv2.0.0vkCmdCopyBufferToImage(raw)v2.25.0; Android rulesv2.15.0linux/main.cpp:2091), macOS (macos/main.mm:2526) and Android (hud_bar.cpp:223), already_SRGB-firstvkCmdCopyBufferToImage(raw) everywherev2.26.0; Windows/macOSv2.15.0headers only; Androidv2.15.0headers onlysrc/xr/XrSession.cpp:817, Androidmain.cpp:1556), already 43 → 37 →formats[0]vkCmdCopyImage(src/ui/ImGuiLayer.cpp:661). The CPU HUD usesvkCmdCopyBufferToImage. UNORM-fallback path: the render pass draws directly on the swapchain image (:202)test_appsv2.24.0; macOS GL/Metalv2.0.0probes/windowspace_handle_*. Own Local2D panels and strips (87 / 44 / 0x8058 / Metal 80 /PickStripFormat91→87 / GL 0x8C43→0x8058)UpdateSubresource,CopyTextureRegionwith footprint = HUD format,glTexSubImage2D,replaceRegion,vkCmdCopyBufferToImage), exceptcube_zones_vk_{linux,macos}strips, which usevkCmdClearColorImageviaVkDisplayReferredClearColor_SRGBonce re-pinned. The app-owned UNORM Local2D panels (87 / 44 / 0x8058 / 80) are still display-referred in UNORM and need their own fixCopyResource/UpdateSubresource. macOS uses hard-coded VK 37 everywhere (rawvkCmdCopyBufferToImage)v2.7.0xrWeaveSubmitDXRplus a DXGI HWND swapchain)Hardware check (Leia panel, runtime v2.22.2, vk_native)
displayxr-demo-modelviewerat origin/main was built twice from an out-of-treegit archive, so nothing in the model-viewer repo was modified:v2.25.0;FETCHCONTENT_SOURCE_DIR_DISPLAYXR_COMMONpointing at this branch.Both builds are otherwise identical. For each one I opened the HUD with Shift+Tab, raised the "View reset" toast with Space, and took a runtime atlas capture.
Format really changed. The app log reads
Selected window-space swapchain format: 37 (0x25)before (×3: HUD, button bar, toast) andhonest _SRGB 43 (0x2B) (default)after (×3).Pixels (atlas 1280×360, left view; the right view is identical). The visible window-space layer is the common button bar ("Open…", "Mode: LeiaSR"):
RenderHudAndMapbuttons, D2D fill black at α 0.55, white text, uploaded byCachedLayerUploader. The scene background behind it is the same(155,169,196)in both runs._SRGB)(70,76,88)(70,76,88)(172,174,178)/(253,253,253)(95,100,109),(101,108,122)(155,169,196)(155,169,196)So the result on this box is "no change", not "lifted → fixed". The authored button fill (black at α 0.55 over
(155,169,196)) composites to exactly0.45 × bgin encoded bytes ((70,76,88)) with either format. A linear-light blend would give about(107,117,137). On runtime v2.22.2 vk_native, then, the window-space layer is not read as linear when it is UNORM and was not double-encoded before this change. That doesn't match ADR-044 §1's vk_native row (a UNORM swapchain is linear, layers blend in linear), so it is worth a runtime-side look (cts session). This PR is still the contract-correct side (INV-4.6:_SRGBfor encoded bytes) and is byte-identical here. The washed-out "before" the ADR describes should show on the backends that do read UNORM window-space layers as linear. The HUD body and toast could not be raised by synthetic key input in this run (no key events reached the app log), so the button bar is the only measured window-space layer. Also measured: the D3D11/D3D12/GL paths were not exercised on hardware, and Metal was not run at all (see constraint 5).Build / test
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release && cmake --build build --config Release && ctest -C Release): all targets build and 5/5 tests pass. The newtest_window_space_format_policy()passes with and withoutDXR_SWAPCHAIN_ENCODING=unormin the environment.v2.25.0) builds unchanged against this branch.xr_window_space_hud.cpp, which now includes the platform-neutralcolor_policy.h. No#ifdefcontext was changed.CMakeListsVERSION →2.27.0: this is a behaviour change, and the field had drifted to 2.20.0 against thev2.26.0tag. The repo has no CHANGELOG file; by repo convention the behaviour-change note is in the README colour section. Not tagged.Refs: runtime ADR-044 §7, INV-4.6, #49 (projection
_SRGBdefault), #51 (TRANSFER_DST, blit pairing), #56 / runtime #1647 (clear helpers).🤖 Generated with Claude Code