Skip to content

fix(test-apps): D3D12 cubes decode authored colour for an _SRGB RTV; GL macOS cubes prefer GL_SRGB8_ALPHA8 (ADR-044, #1882) #1885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dfattal merged 1 commit into from
Oct 9, 2026
Merged

fix(test-apps): D3D12 cubes decode authored colour for an _SRGB RTV; GL macOS cubes prefer GL_SRGB8_ALPHA8 (ADR-044, #1882) #1885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 51 additions & 7 deletions test_apps/handle/cube_handle_d3d12_win/d3d12_renderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "d3d12_renderer.h"
#include "d3d12_clear.h" // dxr::ClearRenderTargetViewDisplayReferred (#1647)
#include "color_policy.h" // dxr::IsSrgbColorFormat / RenderSceneLinear (ADR-044)
#include "logging.h"
#include "mip_chain.h"
#include <d3d12sdklayers.h>
Expand Down Expand Up @@ -85,7 +86,18 @@ float4 PSMain(PSInput input) : SV_TARGET {
float3 lightDir = normalize(float3(0.3, 0.8, 0.5));
float diffuse = max(dot(mappedNormal, lightDir), 0.0) * 0.7 + 0.3;

return float4(basecolor * ao * diffuse, 1.0);
float3 col = basecolor * ao * diffuse;
#ifdef DXR_LINEARIZE
// ADR-044 / INV-4.6 (#1882). The cube's authored colours (and its
// basecolor/AO textures, uploaded as plain UNORM) are DISPLAY-REFERRED.
// When the RTV this PSO targets is `_SRGB` the render target encodes on
// write, so emitting those numbers raw encodes them twice -- the washed-out
// cube David saw on the rig. Decoding here makes the round-trip an
// identity: the bytes stored are exactly the authored ones, honestly
// declared as encoded. Same rule as the D3D11 reference renderer.
col = (col <= 0.04045) ? (col / 12.92) : pow((col + 0.055) / 1.055, 2.4);
#endif
return float4(col, 1.0);
}
)";

Expand All @@ -111,13 +123,22 @@ PSInput VSMain(VSInput input) {
}

float4 PSMain(PSInput input) : SV_TARGET {
return color;
float3 col = color.rgb;
#ifdef DXR_LINEARIZE
// Same rule as the cube PS: the grid colour is authored display-referred.
col = (col <= 0.04045) ? (col / 12.92) : pow((col + 0.055) / 1.055, 2.4);
#endif
return float4(col, color.a);
}
)";

static bool CompileShader(const char* source, const char* entryPoint, const char* target, ID3DBlob** blob) {
// ADR-044 / INV-4.6: `linearize` defines DXR_LINEARIZE so the pixel shaders
// emit SCENE-LINEAR for an `_SRGB` render target (which encodes on write).
static bool CompileShader(const char* source, const char* entryPoint, const char* target, ID3DBlob** blob,
bool linearize = false) {
ComPtr<ID3DBlob> errorBlob;
HRESULT hr = D3DCompile(source, strlen(source), nullptr, nullptr, nullptr,
const D3D_SHADER_MACRO macros[] = { { "DXR_LINEARIZE", "1" }, { nullptr, nullptr } };
HRESULT hr = D3DCompile(source, strlen(source), nullptr, linearize ? macros : nullptr, nullptr,
entryPoint, target, D3DCOMPILE_OPTIMIZATION_LEVEL3, 0, blob, &errorBlob);
if (FAILED(hr)) {
if (errorBlob) {
Expand All @@ -128,6 +149,22 @@ static bool CompileShader(const char* source, const char* entryPoint, const char
return true;
}

// Must the pixel shaders decode their display-referred colours for THIS render
// target? Keyed on the format the RTV/PSO is created with, not on the
// process-wide dxr::RenderSceneLinear() flag, for the same reason the clear
// helper is (#1647): the runtime hands out TYPELESS swapchain textures, so the
// view is what arms the hardware encode, and a caller that resolves the view to
// the UNORM sibling (cube_hosted_legacy_d3d12_win writes raw bytes) must get
// the raw variant even though the swapchain itself was noted `_SRGB`.
// DXR_TRUE_LINEAR still overrides both ways (the ADR-021 matrix cells).
static bool LinearizeForTarget(DXGI_FORMAT rtvFormat) {
const char* e = getenv("DXR_TRUE_LINEAR");
if (e != nullptr && *e != '\0') {
return dxr::RenderSceneLinear();
}
return dxr::IsSrgbColorFormat((int64_t)rtvFormat);
}

static ComPtr<ID3D12Resource> CreateUploadBuffer(ID3D12Device* device, const void* data, UINT64 size) {
ComPtr<ID3D12Resource> buffer;
D3D12_HEAP_PROPERTIES heapProps = {};
Expand Down Expand Up @@ -682,12 +719,19 @@ bool CreateSwapchainRTVs(D3D12Renderer& renderer,
psoDesc.DSVFormat = DXGI_FORMAT_D24_UNORM_S8_UINT;
psoDesc.SampleDesc.Count = 1;

// Recompile shaders for PSO recreation
// Recompile shaders for PSO recreation. This is the first point where
// the real RTV format is known, so it is where the ADR-044 decision is
// made: an `_SRGB` target gets the scene-linear PS variant (#1882).
const bool linearize = LinearizeForTarget(format);
LOG_WARN("[color] rtvFormat=%u sceneLinear=%s (ADR-044: %s)", (unsigned)format,
linearize ? "yes" : "no",
linearize ? "the _SRGB RTV encodes on write, so the PS decodes its authored colours"
: "raw target, authored display-referred bytes are written verbatim");
ComPtr<ID3DBlob> cubeVS, cubePS, gridVS, gridPS;
if (!CompileShader(g_cubeShaderSource, "VSMain", "vs_5_0", &cubeVS) ||
!CompileShader(g_cubeShaderSource, "PSMain", "ps_5_0", &cubePS) ||
!CompileShader(g_cubeShaderSource, "PSMain", "ps_5_0", &cubePS, linearize) ||
!CompileShader(g_gridShaderSource, "VSMain", "vs_5_0", &gridVS) ||
!CompileShader(g_gridShaderSource, "PSMain", "ps_5_0", &gridPS)) {
!CompileShader(g_gridShaderSource, "PSMain", "ps_5_0", &gridPS, linearize)) {
LOG_ERROR("Failed to recompile shaders for format 0x%X", format);
return false;
}
Expand Down
15 changes: 11 additions & 4 deletions test_apps/handle/cube_handle_gl_macos/main.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1503,13 +1503,20 @@ static bool CreateSwapchain(AppXrSession &app)
XR_CHECK(xrEnumerateSwapchainFormats(app.session, formatCount, &formatCount, formats.data()));

LOG_INFO("Supported swapchain formats:");
// ADR-044 / INV-4.6: prefer GL_SRGB8_ALPHA8, fall back to GL_RGBA8.
// The shaders write display-referred bytes with GL_FRAMEBUFFER_SRGB
// off, so an `_SRGB` swapchain stores exactly the authored bytes, honestly
// declared; the format-honest GL compositor (>= v2.21.2) reads a UNORM
// swapchain as LINEAR and encodes it -- the old GL_RGBA8 preference came
// out washed out there.
int64_t selectedFormat = formats[0];
bool haveSrgb = false, haveRgba8 = false;
for (auto f : formats) {
// Prefer GL_RGBA8 (0x8058) or GL_SRGB8_ALPHA8 (0x8C43)
if (f == 0x8058) { // GL_RGBA8
selectedFormat = f;
}
if (f == GL_SRGB8_ALPHA8) haveSrgb = true;
if (f == GL_RGBA8) haveRgba8 = true;
}
if (haveSrgb) selectedFormat = GL_SRGB8_ALPHA8;
else if (haveRgba8) selectedFormat = GL_RGBA8;
for (auto f : formats) {
LOG_INFO(" format 0x%llx%s", (long long)f, f == selectedFormat ? " (selected)" : "");
}
Expand Down
10 changes: 7 additions & 3 deletions test_apps/handle/cube_handle_gl_win/gl_renderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,13 @@ void RenderScene(
: imageIndex;
glBindFramebuffer_(GL_FRAMEBUFFER, renderer.fbos[fboIdx]);

// TODO: If the swapchain format is GL_SRGB8_ALPHA8, we may need
// glEnable(GL_FRAMEBUFFER_SRGB) here for correct linear-to-sRGB conversion.
// Without it, output may appear too dark compared to the D3D11 version.
// ADR-044 / INV-4.6: the swapchain is GL_SRGB8_ALPHA8 and GL_FRAMEBUFFER_SRGB
// stays DISABLED on purpose. The shaders emit the authored DISPLAY-REFERRED
// colours (the textures are plain GL_RGBA, never decoded), so a raw write
// stores exactly those bytes in an honestly-declared encoded swapchain and
// the format-honest GL compositor (>= v2.21.2) decodes them once on sample.
// Enabling GL_FRAMEBUFFER_SRGB here would encode them a second time
// (washed out) -- that is the #1882 class of bug, not a fix for a dark image.

glViewport(viewportX, viewportY, width, height);
glScissor(viewportX, viewportY, width, height);
Expand Down
5 changes: 4 additions & 1 deletion test_apps/handle/cube_handle_gl_win/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,10 @@ static uint32_t g_quadTexSize = 256;
static bool CreateAndFillQuadTexture(XrSessionManager& xr, uint32_t size) {
XrSwapchainCreateInfo sci = {XR_TYPE_SWAPCHAIN_CREATE_INFO};
sci.usageFlags = XR_SWAPCHAIN_USAGE_COLOR_ATTACHMENT_BIT | XR_SWAPCHAIN_USAGE_SAMPLED_BIT;
sci.format = 0x8058; // GL_RGBA8
// ADR-044 / INV-4.6: the bytes below are display-referred and land by a raw
// glTexSubImage2D, so declare them GL_SRGB8_ALPHA8 (UNORM would be encoded
// a second time by the format-honest compositor).
sci.format = PickPanelFormat(xr.session, 0x8C43 /* GL_SRGB8_ALPHA8 */, 0x8058 /* GL_RGBA8 */);
sci.sampleCount = 1;
sci.width = size;
sci.height = size;
Expand Down
58 changes: 51 additions & 7 deletions test_apps/handle/cube_zones_d3d12_win/d3d12_renderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "d3d12_renderer.h"
#include "d3d12_clear.h" // dxr::ClearRenderTargetViewDisplayReferred (#1647)
#include "color_policy.h" // dxr::IsSrgbColorFormat / RenderSceneLinear (ADR-044)
#include "logging.h"
#include "mip_chain.h"
#include <d3d12sdklayers.h>
Expand Down Expand Up @@ -85,7 +86,18 @@ float4 PSMain(PSInput input) : SV_TARGET {
float3 lightDir = normalize(float3(0.3, 0.8, 0.5));
float diffuse = max(dot(mappedNormal, lightDir), 0.0) * 0.7 + 0.3;

return float4(basecolor * ao * diffuse, 1.0);
float3 col = basecolor * ao * diffuse;
#ifdef DXR_LINEARIZE
// ADR-044 / INV-4.6 (#1882). The cube's authored colours (and its
// basecolor/AO textures, uploaded as plain UNORM) are DISPLAY-REFERRED.
// When the RTV this PSO targets is `_SRGB` the render target encodes on
// write, so emitting those numbers raw encodes them twice -- the washed-out
// cube David saw on the rig. Decoding here makes the round-trip an
// identity: the bytes stored are exactly the authored ones, honestly
// declared as encoded. Same rule as the D3D11 reference renderer.
col = (col <= 0.04045) ? (col / 12.92) : pow((col + 0.055) / 1.055, 2.4);
#endif
return float4(col, 1.0);
}
)";

Expand All @@ -111,13 +123,22 @@ PSInput VSMain(VSInput input) {
}

float4 PSMain(PSInput input) : SV_TARGET {
return color;
float3 col = color.rgb;
#ifdef DXR_LINEARIZE
// Same rule as the cube PS: the grid colour is authored display-referred.
col = (col <= 0.04045) ? (col / 12.92) : pow((col + 0.055) / 1.055, 2.4);
#endif
return float4(col, color.a);
}
)";

static bool CompileShader(const char* source, const char* entryPoint, const char* target, ID3DBlob** blob) {
// ADR-044 / INV-4.6: `linearize` defines DXR_LINEARIZE so the pixel shaders
// emit SCENE-LINEAR for an `_SRGB` render target (which encodes on write).
static bool CompileShader(const char* source, const char* entryPoint, const char* target, ID3DBlob** blob,
bool linearize = false) {
ComPtr<ID3DBlob> errorBlob;
HRESULT hr = D3DCompile(source, strlen(source), nullptr, nullptr, nullptr,
const D3D_SHADER_MACRO macros[] = { { "DXR_LINEARIZE", "1" }, { nullptr, nullptr } };
HRESULT hr = D3DCompile(source, strlen(source), nullptr, linearize ? macros : nullptr, nullptr,
entryPoint, target, D3DCOMPILE_OPTIMIZATION_LEVEL3, 0, blob, &errorBlob);
if (FAILED(hr)) {
if (errorBlob) {
Expand All @@ -128,6 +149,22 @@ static bool CompileShader(const char* source, const char* entryPoint, const char
return true;
}

// Must the pixel shaders decode their display-referred colours for THIS render
// target? Keyed on the format the RTV/PSO is created with, not on the
// process-wide dxr::RenderSceneLinear() flag, for the same reason the clear
// helper is (#1647): the runtime hands out TYPELESS swapchain textures, so the
// view is what arms the hardware encode, and a caller that resolves the view to
// the UNORM sibling (cube_hosted_legacy_d3d12_win writes raw bytes) must get
// the raw variant even though the swapchain itself was noted `_SRGB`.
// DXR_TRUE_LINEAR still overrides both ways (the ADR-021 matrix cells).
static bool LinearizeForTarget(DXGI_FORMAT rtvFormat) {
const char* e = getenv("DXR_TRUE_LINEAR");
if (e != nullptr && *e != '\0') {
return dxr::RenderSceneLinear();
}
return dxr::IsSrgbColorFormat((int64_t)rtvFormat);
}

static ComPtr<ID3D12Resource> CreateUploadBuffer(ID3D12Device* device, const void* data, UINT64 size) {
ComPtr<ID3D12Resource> buffer;
D3D12_HEAP_PROPERTIES heapProps = {};
Expand Down Expand Up @@ -682,12 +719,19 @@ bool CreateSwapchainRTVs(D3D12Renderer& renderer,
psoDesc.DSVFormat = DXGI_FORMAT_D24_UNORM_S8_UINT;
psoDesc.SampleDesc.Count = 1;

// Recompile shaders for PSO recreation
// Recompile shaders for PSO recreation. This is the first point where
// the real RTV format is known, so it is where the ADR-044 decision is
// made: an `_SRGB` target gets the scene-linear PS variant (#1882).
const bool linearize = LinearizeForTarget(format);
LOG_WARN("[color] rtvFormat=%u sceneLinear=%s (ADR-044: %s)", (unsigned)format,
linearize ? "yes" : "no",
linearize ? "the _SRGB RTV encodes on write, so the PS decodes its authored colours"
: "raw target, authored display-referred bytes are written verbatim");
ComPtr<ID3DBlob> cubeVS, cubePS, gridVS, gridPS;
if (!CompileShader(g_cubeShaderSource, "VSMain", "vs_5_0", &cubeVS) ||
!CompileShader(g_cubeShaderSource, "PSMain", "ps_5_0", &cubePS) ||
!CompileShader(g_cubeShaderSource, "PSMain", "ps_5_0", &cubePS, linearize) ||
!CompileShader(g_gridShaderSource, "VSMain", "vs_5_0", &gridVS) ||
!CompileShader(g_gridShaderSource, "PSMain", "ps_5_0", &gridPS)) {
!CompileShader(g_gridShaderSource, "PSMain", "ps_5_0", &gridPS, linearize)) {
LOG_ERROR("Failed to recompile shaders for format 0x%X", format);
return false;
}
Expand Down
22 changes: 11 additions & 11 deletions test_apps/legacy/cube_hosted_legacy_d3d12_win/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,18 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
LOG_INFO("Enumerated %u D3D12 swapchain images", count);
}

// Determine swapchain format for RTV creation.
// Determine the RTV format (ADR-044 / INV-4.6).
//
// The runtime deliberately creates 8-bit colour swapchain resources as
// TYPELESS so it can pick the view interpretation itself
// (comp_d3d12_swapchain.cpp), and it then samples the atlas through an
// *_UNORM SRV -- no sRGB decode (comp_d3d12_compositor.cpp, "mirrors the
// GL skip-decode fix"). So the app must write raw UNORM to match.
//
// Resolving TYPELESS to *_UNORM_SRGB instead made the hardware sRGB-encode
// on write while the runtime still read the bytes as UNORM, double-encoding
// every colour: the 0.05/0.05/0.25 clear landed at #3F3F89 instead of
// #0D0D40, i.e. a washed-out purple rather than dark navy.
// CreateSwapchain() asked for an `_SRGB` swapchain (displayxr-common's
// default), and the runtime hands out TYPELESS resources. This app takes
// the "display-referred bytes, GPU sRGB-write OFF" row of ADR-044 section 2:
// the RTV is resolved to the plain *_UNORM sibling, so the shaders' authored
// colours and the 0.05/0.05/0.25 clear land verbatim, and the format-honest
// D3D12 compositor (>= v2.21.1) decodes the `_SRGB`-declared swapchain
// exactly once on sample. The shared d3d12_renderer keys its scene-linear
// PS variant on the RTV format it is given (#1882), so a UNORM RTV gets the
// raw variant here while cube_handle_d3d12_win's `_SRGB` RTV gets the
// decoding one -- both store the same bytes.
DXGI_FORMAT rtvFormat = DXGI_FORMAT_R8G8B8A8_UNORM;
if (swapchainImages.size() > 0 && swapchainImages[0].texture != nullptr) {
D3D12_RESOURCE_DESC desc = swapchainImages[0].texture->GetDesc();
Expand Down
14 changes: 10 additions & 4 deletions test_apps/legacy/cube_hosted_legacy_gl_macos/main.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,13 +822,19 @@ static bool CreateSwapchain(AppXrSession &app)
XR_CHECK(xrEnumerateSwapchainFormats(app.session, formatCount, &formatCount, formats.data()));

LOG_INFO("Supported swapchain formats:");
// ADR-044 / INV-4.6: prefer GL_SRGB8_ALPHA8, fall back to GL_RGBA8. The
// shaders write display-referred bytes with GL_FRAMEBUFFER_SRGB off, so an
// `_SRGB` swapchain stores exactly the authored bytes, honestly declared;
// the format-honest GL compositor (>= v2.21.2) reads a UNORM swapchain as
// LINEAR and encodes it -- the old GL_RGBA8 preference came out washed out.
int64_t selectedFormat = formats[0];
bool haveSrgb = false, haveRgba8 = false;
for (auto f : formats) {
// Prefer GL_RGBA8
if (f == GL_RGBA8) {
selectedFormat = f;
}
if (f == GL_SRGB8_ALPHA8) haveSrgb = true;
if (f == GL_RGBA8) haveRgba8 = true;
}
if (haveSrgb) selectedFormat = GL_SRGB8_ALPHA8;
else if (haveRgba8) selectedFormat = GL_RGBA8;
for (auto f : formats) {
LOG_INFO(" format 0x%llx%s", (long long)f, f == selectedFormat ? " (selected)" : "");
}
Expand Down
Loading
Loading