From 1a00163d8369ce116c71876eee905b9dc1eaec63 Mon Sep 17 00:00:00 2001 From: dfattal Date: Fri, 5 Jun 2026 02:00:56 -0700 Subject: [PATCH] fix(#425): opaque atlas capture + descriptive PNG naming + in-process Unity capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xrCaptureAtlasEXT hardening (XR_EXT_atlas_capture SPEC_VERSION 1->2): - Opaque alpha at all 6 encode sites via new aux helper u_image_force_opaque_rgba8 (u_image_capture.h) — forces A=255 so captures are no longer fully-transparent/black. Applied in d3d11/d3d12/gl/metal/ vk_native compositors + comp_d3d11_service.cpp. - Runtime-owned PNG suffix _atlas__x built only at the EXT-contract layer (oxr_capture.c in-process resolves the active mode tile layout via GET_XDEV_BY_ROLE(head); comp_d3d11_service.cpp IPC). Callers pass a bare - prefix; the *_capture_atlas_to_png cores write verbatim. test_apps/common/atlas_capture.* + the macOS cube apps updated to pass the bare prefix. - In-process D3D11 capture: dispatch via d3d11_compositor_dispatch_capture_ zerocopy() reading the zero-copy swapchain SRV directly, so single-shared- swapchain projection layers (Unity in-process, zero-copy eligible) no longer silently drop the capture. Shared core refactored to d3d11_capture_texture_to_png(tex, w, h, path). - Diagnostics: stop the misleading "Legacy app" WARN at xrCreateInstance for engine probe instances (Unity/Unreal create a zero-extension throwaway instance that can never start a session). oxr_system_fill_in downgrades the provisional-scale log to INFO; the authoritative LEGACY-session WARN now fires in oxr_session_create only when the compromise scale actually drives rendering. Verified on native test apps, demo apps, and the Unity/Unreal plugins. --- docs/roadmap/unified-atlas-capture.md | 21 ++- .../openxr/XR_EXT_atlas_capture.h | 29 ++-- src/xrt/auxiliary/util/u_image_capture.h | 50 +++++++ .../d3d11/comp_d3d11_compositor.cpp | 134 +++++++++++++++--- .../d3d11_service/comp_d3d11_service.cpp | 10 +- .../d3d12/comp_d3d12_compositor.cpp | 4 + src/xrt/compositor/gl/comp_gl_compositor.cpp | 5 + .../compositor/metal/comp_metal_compositor.m | 5 +- .../vk_native/comp_vk_native_compositor.c | 5 + src/xrt/state_trackers/oxr/oxr_capture.c | 41 ++++-- src/xrt/state_trackers/oxr/oxr_session.c | 16 +++ src/xrt/state_trackers/oxr/oxr_system.c | 49 ++++++- test_apps/common/atlas_capture.cpp | 16 ++- test_apps/common/atlas_capture.h | 10 +- test_apps/common/atlas_capture_macos.mm | 13 +- test_apps/cube_handle_gl_macos/main.mm | 7 +- test_apps/cube_handle_metal_macos/main.mm | 7 +- test_apps/cube_handle_vk_macos/main.mm | 6 +- 18 files changed, 358 insertions(+), 70 deletions(-) create mode 100644 src/xrt/auxiliary/util/u_image_capture.h diff --git a/docs/roadmap/unified-atlas-capture.md b/docs/roadmap/unified-atlas-capture.md index c8dbc6aa3..30e753eb8 100644 --- a/docs/roadmap/unified-atlas-capture.md +++ b/docs/roadmap/unified-atlas-capture.md @@ -112,7 +112,7 @@ New header `src/external/openxr_includes/openxr/XR_EXT_atlas_capture.h` ```c #define XR_EXT_atlas_capture 1 -#define XR_EXT_atlas_capture_SPEC_VERSION 1 +#define XR_EXT_atlas_capture_SPEC_VERSION 2 #define XR_EXT_ATLAS_CAPTURE_EXTENSION_NAME "XR_EXT_atlas_capture" // Reuse the reserved 1000999xxx range (next free slot after the workspace block). @@ -132,7 +132,7 @@ typedef struct XrAtlasCaptureInfoEXT { XrStructureType type; // XR_TYPE_ATLAS_CAPTURE_INFO_EXT const void* XR_MAY_ALIAS next; XrAtlasCaptureStageEXT stage; - char pathPrefix[XR_ATLAS_CAPTURE_PATH_MAX_EXT]; // runtime appends "_atlas.png" + char pathPrefix[XR_ATLAS_CAPTURE_PATH_MAX_EXT]; // runtime appends "_atlas__x.png" } XrAtlasCaptureInfoEXT; // Identical metadata block to XrWorkspaceCaptureResultEXT (see rationale below). @@ -153,6 +153,23 @@ typedef XrResult (XRAPI_PTR *PFN_xrCaptureAtlasEXT)( XrAtlasCaptureResultEXT *result); // result may be NULL if the caller wants only the PNG ``` +### Filename + alpha contract (SPEC_VERSION 2, issue #425) + +- **Suffix.** The runtime appends `_atlas__x.png` to + `pathPrefix` (e.g. a 2-view 2×1 capture → `_atlas_2_2x1.png`), so + consumers don't re-derive the atlas geometry. `viewCount` is the tile count + (`cols*rows` for all current layouts). Callers pass a bare `-` prefix + and must **not** pre-bake the layout (avoids `..._2x1_atlas_2_2x1.png`). The + suffix is built at the EXT-contract layer — `oxr_capture.c` (in-process, + resolving the active rendering mode's tile layout) and + `comp_d3d11_service.cpp` (IPC) — **not** inside the per-API + `*_capture_atlas_to_png`, because the MCP `capture_frame` tool and the dev + trigger files write/report a verbatim full path. +- **Opaque alpha.** Every encoder forces `A=255` before PNG write + (`u_image_force_opaque_rgba8`). The swapchain alpha is undefined for display + output (the DP/weaver ignores it); left verbatim it reads back as 0 → fully + transparent → renders black. + Edit to `XR_EXT_spatial_workspace.h` (spec_version bump): add the second capture flag so the privileged path also supports stage selection. diff --git a/src/external/openxr_includes/openxr/XR_EXT_atlas_capture.h b/src/external/openxr_includes/openxr/XR_EXT_atlas_capture.h index 66fc67cf5..7548a0589 100644 --- a/src/external/openxr_includes/openxr/XR_EXT_atlas_capture.h +++ b/src/external/openxr_includes/openxr/XR_EXT_atlas_capture.h @@ -27,7 +27,10 @@ extern "C" { #endif #define XR_EXT_atlas_capture 1 -#define XR_EXT_atlas_capture_SPEC_VERSION 1 +// SPEC_VERSION 2: the runtime appends "_atlas__x.png" +// (was a flat "_atlas.png" in v1), and the encoded PNG is always opaque +// (alpha forced to 255). See issue #425. +#define XR_EXT_atlas_capture_SPEC_VERSION 2 #define XR_EXT_ATLAS_CAPTURE_EXTENSION_NAME "XR_EXT_atlas_capture" // Reserved 1000999xxx range, next free slot after the workspace block @@ -57,9 +60,13 @@ typedef enum XrAtlasCaptureStageEXT { /*! * @brief Request struct for xrCaptureAtlasEXT. * - * The runtime appends a format-specific suffix (e.g. "_atlas.png") to - * @c pathPrefix. The prefix is an in-struct char array (not a separately - * allocated string) so the same struct can cross the IPC schema unchanged. + * The runtime appends a layout-encoded suffix + * "_atlas__x.png" to @c pathPrefix (e.g. a 2-view 2x1 + * capture → "_atlas_2_2x1.png"), so consumers don't re-derive the + * multi-view atlas geometry. Callers should pass a bare prefix (e.g. + * "-") and not pre-bake the layout, to avoid duplicating it. The + * prefix is an in-struct char array (not a separately allocated string) so the + * same struct can cross the IPC schema unchanged. */ typedef struct XrAtlasCaptureInfoEXT { XrStructureType type; //!< Must be XR_TYPE_ATLAS_CAPTURE_INFO_EXT @@ -72,9 +79,10 @@ typedef struct XrAtlasCaptureInfoEXT { * @brief Result returned by xrCaptureAtlasEXT. * * Same metadata block as XrWorkspaceCaptureResultEXT minus @c viewsWritten. - * For in-process sessions @c eyeLeftM / @c eyeRightM (and @c tileColumns / - * @c tileRows) may be zero — eye-pose plumbing currently stops at the display - * processor and is only surfaced on the IPC/workspace path. + * @c tileColumns / @c tileRows are populated on both paths (from the active + * rendering mode in-process). For in-process sessions @c eyeLeftM / + * @c eyeRightM may still be zero — eye-pose plumbing currently stops at the + * display processor and is only surfaced on the IPC/workspace path. */ typedef struct XrAtlasCaptureResultEXT { XrStructureType type; //!< Must be XR_TYPE_ATLAS_CAPTURE_RESULT_EXT @@ -96,8 +104,11 @@ typedef struct XrAtlasCaptureResultEXT { * @brief Capture the multi-view atlas the runtime composes for this session. * * The runtime reads the compositor's own atlas at @c info->stage and writes it - * to a PNG named after @c info->pathPrefix (the runtime appends "_atlas.png"). - * If @c result is non-NULL it is filled with metadata describing the capture. + * to a PNG named after @c info->pathPrefix (the runtime appends + * "_atlas__x.png"). The encoded PNG is always opaque + * (alpha forced to 255 — the swapchain's alpha is undefined for display + * output). If @c result is non-NULL it is filled with metadata describing the + * capture. * * Timing: the call is non-blocking and latches the request; the readback runs * at the next composed frame (the caller's next xrEndFrame), so the PNG exists diff --git a/src/xrt/auxiliary/util/u_image_capture.h b/src/xrt/auxiliary/util/u_image_capture.h new file mode 100644 index 000000000..d507bf4d6 --- /dev/null +++ b/src/xrt/auxiliary/util/u_image_capture.h @@ -0,0 +1,50 @@ +// Copyright 2026, DisplayXR +// SPDX-License-Identifier: BSL-1.0 +/*! + * @file + * @brief Small pixel-buffer helpers shared by the atlas-capture PNG encoders. + * @ingroup aux_util + */ + +#pragma once + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/*! + * Force every pixel's alpha to 255 (opaque) in a tightly-or-strided RGBA8 + * buffer, in place. + * + * The atlas-capture readback copies the swapchain's alpha channel verbatim, + * but that alpha is *undefined for display output* — the compositor / display + * processor (weaver) never reads it, so it is typically 0. Left as-is the + * encoded PNG is fully transparent and renders black in normal image viewers + * (issue #425). The captured atlas is opaque display content, so the encoder + * forces A=255 before @c stbi_write_png. + * + * @param pixels Base of an RGBA8 buffer (byte order R,G,B,A per pixel). + * @param width Pixels per row. + * @param height Number of rows. + * @param stride_bytes Bytes per row (>= width*4; equals width*4 when tight). + */ +static inline void +u_image_force_opaque_rgba8(uint8_t *pixels, uint32_t width, uint32_t height, size_t stride_bytes) +{ + if (pixels == NULL) { + return; + } + for (uint32_t y = 0; y < height; y++) { + uint8_t *row = pixels + (size_t)y * stride_bytes; + for (uint32_t x = 0; x < width; x++) { + row[(size_t)x * 4 + 3] = 255; + } + } +} + +#ifdef __cplusplus +} +#endif diff --git a/src/xrt/compositor/d3d11/comp_d3d11_compositor.cpp b/src/xrt/compositor/d3d11/comp_d3d11_compositor.cpp index 8604689f4..d84533392 100644 --- a/src/xrt/compositor/d3d11/comp_d3d11_compositor.cpp +++ b/src/xrt/compositor/d3d11/comp_d3d11_compositor.cpp @@ -52,6 +52,7 @@ #include "util/u_tiling.h" #include "util/u_canvas.h" #include "util/u_capture_intent.h" +#include "util/u_image_capture.h" #ifdef XRT_BUILD_DRIVER_QWERTY #include "qwerty_interface.h" @@ -900,26 +901,21 @@ d3d11_crop_atlas_for_dp(struct comp_d3d11_compositor *c, // compositor crops and sends to the DP) into a staging texture, then write // @p path as PNG. D3D11 renderer uses DXGI_FORMAT_R8G8B8A8_UNORM so no // channel swap is needed. +// Read back the @p content_w × @p content_h top-left region of @p atlas_tex +// (clamped to its actual size) and write @p path as an opaque RGBA8 PNG. The +// source may be the renderer's composited atlas OR, in zero-copy present, the +// app's own swapchain image — both already hold the laid-out multi-view atlas. static bool -d3d11_compositor_capture_atlas_to_png(struct comp_d3d11_compositor *c, const char *path) +d3d11_capture_texture_to_png(struct comp_d3d11_compositor *c, + ID3D11Texture2D *atlas_tex, + uint32_t content_w, + uint32_t content_h, + const char *path) { - ID3D11Texture2D *atlas_tex = static_cast( - comp_d3d11_renderer_get_atlas_texture(c->renderer)); - if (atlas_tex == nullptr || c->renderer == nullptr) { - return false; - } - - uint32_t tile_columns = 1, tile_rows = 1; - comp_d3d11_renderer_get_tile_layout(c->renderer, &tile_columns, &tile_rows); - uint32_t view_w = 0, view_h = 0; - comp_d3d11_renderer_get_view_dimensions(c->renderer, &view_w, &view_h); - if (tile_columns == 0 || tile_rows == 0 || view_w == 0 || view_h == 0) { + if (atlas_tex == nullptr || content_w == 0 || content_h == 0) { return false; } - uint32_t content_w = tile_columns * view_w; - uint32_t content_h = tile_rows * view_h; - D3D11_TEXTURE2D_DESC adesc; atlas_tex->GetDesc(&adesc); if (content_w > adesc.Width) content_w = adesc.Width; @@ -947,14 +943,61 @@ d3d11_compositor_capture_atlas_to_png(struct comp_d3d11_compositor *c, const cha return false; } - bool ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, - m.pData, (int)m.RowPitch) != 0; + // Repack into a tightly-packed RGBA8 buffer (the mapped staging is + // READ-only, so we can't fix alpha in place) and force opaque: swapchain + // alpha is undefined for display output, and left as-is the PNG renders + // fully transparent/black (issue #425). + bool ok = false; + size_t tight_pitch = (size_t)content_w * 4; + uint8_t *tight = (uint8_t *)malloc(tight_pitch * content_h); + if (tight != nullptr) { + const uint8_t *src = (const uint8_t *)m.pData; + for (uint32_t y = 0; y < content_h; y++) { + memcpy(tight + (size_t)y * tight_pitch, src + (size_t)y * m.RowPitch, tight_pitch); + } + u_image_force_opaque_rgba8(tight, content_w, content_h, tight_pitch); + ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, tight, (int)tight_pitch) != 0; + free(tight); + } c->context->Unmap(staging, 0); staging->Release(); return ok; } +// Resolve the active content region (tile_columns·view_w × tile_rows·view_h) +// from the renderer. Returns false if the renderer hasn't been sized yet. +static bool +d3d11_compositor_content_dims(struct comp_d3d11_compositor *c, uint32_t *out_w, uint32_t *out_h) +{ + if (c->renderer == nullptr) { + return false; + } + uint32_t tile_columns = 1, tile_rows = 1; + comp_d3d11_renderer_get_tile_layout(c->renderer, &tile_columns, &tile_rows); + uint32_t view_w = 0, view_h = 0; + comp_d3d11_renderer_get_view_dimensions(c->renderer, &view_w, &view_h); + if (tile_columns == 0 || tile_rows == 0 || view_w == 0 || view_h == 0) { + return false; + } + *out_w = tile_columns * view_w; + *out_h = tile_rows * view_h; + return true; +} + +// Capture from the renderer's composited atlas (non-zero-copy frames). +static bool +d3d11_compositor_capture_atlas_to_png(struct comp_d3d11_compositor *c, const char *path) +{ + ID3D11Texture2D *atlas_tex = static_cast( + comp_d3d11_renderer_get_atlas_texture(c->renderer)); + uint32_t content_w = 0, content_h = 0; + if (atlas_tex == nullptr || !d3d11_compositor_content_dims(c, &content_w, &content_h)) { + return false; + } + return d3d11_capture_texture_to_png(c, atlas_tex, content_w, content_h, path); +} + // Service a pending MCP capture_frame request — thin wrapper around // Run the capture readback if the per-frame intent matches @p mode_filter. // Mirrors vk_native_dispatch_capture / gl_compositor_dispatch_capture. @@ -975,6 +1018,54 @@ d3d11_compositor_dispatch_capture(struct comp_d3d11_compositor *c, uint32_t mode u_capture_intent_complete(&c->capture_intent, &c->mcp_capture, ok); } +// Zero-copy capture: in zero-copy present the app's own swapchain image IS the +// laid-out multi-view atlas (that's why it qualifies), and the normal composite +// passes — and their PROJECTION_ONLY/POST_COMPOSE dispatch points — are skipped, +// so a pending capture would otherwise silently produce no PNG (issue #425: the +// Unity path submits a single double-wide projection layer that hits zero-copy, +// unlike the native/Unreal apps). Read directly from the same swapchain SRV the +// DP receives — NOT by forcing a re-composite, which re-projects the already- +// tiled texture and corrupts it. Stage selector is irrelevant here: zero-copy +// has a single projection layer and no window-space layers, so PROJECTION_ONLY +// and POST_COMPOSE are identical. +static void +d3d11_compositor_dispatch_capture_zerocopy(struct comp_d3d11_compositor *c, void *zc_srv) +{ + if (!c->capture_intent.pending || zc_srv == nullptr) { + return; + } + ID3D11ShaderResourceView *srv = static_cast(zc_srv); + ID3D11Resource *resource = nullptr; + srv->GetResource(&resource); + ID3D11Texture2D *zc_tex = static_cast(resource); + + // Capture the whole swapchain texture: zero-copy is only eligible when the + // app's swapchain matches the active mode's atlas dimensions exactly + // (u_tiling_can_zero_copy), so the entire texture IS the multi-view atlas. + // Do NOT use the renderer's view/tile dims here — for a legacy app those are + // the 2D/3D compromise size (e.g. 1200×1080), which is smaller than the + // app's real per-view render (e.g. 1920×1080) and would crop the atlas + // (issue #425, the Unity stretched/cropped-PNG case). + bool ok = false; + if (zc_tex != nullptr) { + D3D11_TEXTURE2D_DESC zdesc; + zc_tex->GetDesc(&zdesc); + ok = d3d11_capture_texture_to_png(c, zc_tex, zdesc.Width, zdesc.Height, + c->capture_intent.path); + } + if (resource != nullptr) { + resource->Release(); + } + if (ok) { + U_LOG_I("Atlas captured (zero-copy, mode=%u) to %s", + c->capture_intent.mode, c->capture_intent.path); + } else { + U_LOG_W("Atlas capture failed (zero-copy, mode=%u path=%s)", + c->capture_intent.mode, c->capture_intent.path); + } + u_capture_intent_complete(&c->capture_intent, &c->mcp_capture, ok); +} + static xrt_result_t d3d11_compositor_layer_commit(struct xrt_compositor *xc, xrt_graphics_sync_handle_t sync_handle) { @@ -1198,6 +1289,7 @@ d3d11_compositor_layer_commit(struct xrt_compositor *xc, xrt_graphics_sync_handl } } + // Wait for GPU completion of all projection swapchain textures before reading. // // barrier_image(TO_COMP) at xrReleaseSwapchainImage inserts a Flush() then @@ -1277,6 +1369,14 @@ d3d11_compositor_layer_commit(struct xrt_compositor *xc, xrt_graphics_sync_handl } } + // Zero-copy capture: the app's swapchain (synced by the GPU-completion + // wait above) already holds the atlas the DP will present, and the normal + // dispatch points below are gated behind !zero_copy — so service a pending + // capture here, reading the swapchain directly (issue #425). + if (zero_copy) { + d3d11_compositor_dispatch_capture_zerocopy(c, zc_srv); + } + // Render layers to atlas texture (skip if zero-copy). Split into a // projection pass + window-space pass so a projection-only capture // can read the atlas in between. diff --git a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp index d66b72921..e5d944e1e 100644 --- a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp +++ b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp @@ -42,6 +42,7 @@ #include "util/u_hud.h" #include "util/u_tiling.h" +#include "util/u_image_capture.h" #include #ifdef XRT_BUILD_DRIVER_QWERTY @@ -10945,8 +10946,15 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, src + (size_t)y * m.RowPitch, (size_t)used_w * 4u); } + // Swapchain alpha is undefined for display output — force opaque so the + // PNG doesn't render fully transparent/black (issue #425). + u_image_force_opaque_rgba8(buf.data(), used_w, used_h, (size_t)used_w * 4u); + // Encode the atlas geometry into the suffix so consumers don't re-derive + // it: "_atlas__x.png" (issue #425). viewCount is + // the tile count (cols*rows for all current DisplayXR layouts). char path[MAX_PATH]; - snprintf(path, sizeof(path), "%s_atlas.png", path_prefix); + snprintf(path, sizeof(path), "%s_atlas_%u_%ux%u.png", path_prefix, + tile_columns * tile_rows, tile_columns, tile_rows); if (stbi_write_png(path, (int)used_w, (int)used_h, 4, buf.data(), (int)(used_w * 4u)) != 0) { views_written |= want_flag; diff --git a/src/xrt/compositor/d3d12/comp_d3d12_compositor.cpp b/src/xrt/compositor/d3d12/comp_d3d12_compositor.cpp index ea1d026f5..d1f08989c 100644 --- a/src/xrt/compositor/d3d12/comp_d3d12_compositor.cpp +++ b/src/xrt/compositor/d3d12/comp_d3d12_compositor.cpp @@ -33,6 +33,7 @@ #include "util/u_tiling.h" #include "util/u_canvas.h" #include "util/u_capture_intent.h" +#include "util/u_image_capture.h" #include "util/u_hud.h" #include @@ -1060,6 +1061,9 @@ d3d12_compositor_capture_atlas_to_png(struct comp_d3d12_compositor *c, const cha rb_pixels + (size_t)y * row_pitch, tight_pitch); } + // Swapchain alpha is undefined for display output — force opaque + // so the PNG doesn't render fully transparent/black (issue #425). + u_image_force_opaque_rgba8(tight, content_w, content_h, tight_pitch); ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, tight, (int)tight_pitch) != 0; free(tight); diff --git a/src/xrt/compositor/gl/comp_gl_compositor.cpp b/src/xrt/compositor/gl/comp_gl_compositor.cpp index 0f0d4307d..edb9d6995 100644 --- a/src/xrt/compositor/gl/comp_gl_compositor.cpp +++ b/src/xrt/compositor/gl/comp_gl_compositor.cpp @@ -32,6 +32,7 @@ #include "util/u_tiling.h" #include "util/u_canvas.h" #include "util/u_capture_intent.h" +#include "util/u_image_capture.h" #include "util/u_time.h" #include "util/u_hud.h" #include @@ -1133,6 +1134,10 @@ gl_compositor_capture_atlas_to_png(struct comp_gl_compositor *c, const char *pat } free(bottom_up); + // Swapchain alpha is undefined for display output — force opaque so the + // PNG doesn't render fully transparent/black (issue #425). + u_image_force_opaque_rgba8(top_down, content_w, content_h, row_pitch); + bool ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, top_down, (int)row_pitch) != 0; free(top_down); return ok; diff --git a/src/xrt/compositor/metal/comp_metal_compositor.m b/src/xrt/compositor/metal/comp_metal_compositor.m index e5cfff06e..921caf6a5 100644 --- a/src/xrt/compositor/metal/comp_metal_compositor.m +++ b/src/xrt/compositor/metal/comp_metal_compositor.m @@ -1399,7 +1399,10 @@ - (BOOL)wantsUpdateLayer rgba[i + 0] = bgra[i + 2]; rgba[i + 1] = bgra[i + 1]; rgba[i + 2] = bgra[i + 0]; - rgba[i + 3] = bgra[i + 3]; + // Force opaque: swapchain alpha is undefined for display + // output, and left as-is the PNG renders transparent/black + // (issue #425). + rgba[i + 3] = 255; } ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, rgba, (int)row_pitch) != 0; free(rgba); diff --git a/src/xrt/compositor/vk_native/comp_vk_native_compositor.c b/src/xrt/compositor/vk_native/comp_vk_native_compositor.c index ec8b0fd03..0b5051c04 100644 --- a/src/xrt/compositor/vk_native/comp_vk_native_compositor.c +++ b/src/xrt/compositor/vk_native/comp_vk_native_compositor.c @@ -37,6 +37,7 @@ #include "util/u_tiling.h" #include "util/u_canvas.h" #include "util/u_capture_intent.h" +#include "util/u_image_capture.h" #include // STB_IMAGE_WRITE_STATIC scopes all stbi_write_* to this TU so linking @@ -1837,6 +1838,10 @@ vk_native_capture_atlas_to_png(struct comp_vk_native_compositor *c, const char * pixels = swapped; } } + // Force opaque: swapchain alpha is undefined for display output, and + // left as-is the PNG renders transparent/black (issue #425). Covers + // both the BGRA-swapped copy and the direct (mapped) path. + u_image_force_opaque_rgba8(pixels, content_w, content_h, (size_t)content_w * 4); ok = stbi_write_png(path, (int)content_w, (int)content_h, 4, pixels, (int)content_w * 4) != 0; free(swapped); diff --git a/src/xrt/state_trackers/oxr/oxr_capture.c b/src/xrt/state_trackers/oxr/oxr_capture.c index 3100dcf67..ae2317c80 100644 --- a/src/xrt/state_trackers/oxr/oxr_capture.c +++ b/src/xrt/state_trackers/oxr/oxr_capture.c @@ -108,11 +108,32 @@ capture_inprocess(struct oxr_logger *log, const XrAtlasCaptureInfoEXT *info, XrAtlasCaptureResultEXT *result) { - // The runtime appends "_atlas.png" — same suffix convention as the - // workspace capture path. - char path[XR_ATLAS_CAPTURE_PATH_MAX_EXT + 16]; - int n = snprintf(path, sizeof(path), "%.*s_atlas.png", (int)(XR_ATLAS_CAPTURE_PATH_MAX_EXT - 1), - info->pathPrefix); + // Resolve the active rendering mode's tile layout so the suffix can encode + // the atlas geometry (issue #425). This also surfaces tileColumns/tileRows + // on the in-process path, which previously reported zero. + struct xrt_device *head = GET_XDEV_BY_ROLE(sess->sys, head); + uint32_t cols = 0, rows = 0; + if (head != NULL && head->hmd != NULL) { + uint32_t idx = head->hmd->active_rendering_mode_index; + if (idx < head->rendering_mode_count) { + cols = head->rendering_modes[idx].tile_columns; + rows = head->rendering_modes[idx].tile_rows; + } + } + if (cols == 0) { + cols = 1; + } + if (rows == 0) { + rows = 1; + } + uint32_t view_count = cols * rows; + + // The runtime appends "_atlas__x.png" so consumers + // don't re-derive the multi-view atlas geometry — same suffix the workspace + // (IPC) capture path emits. + char path[XR_ATLAS_CAPTURE_PATH_MAX_EXT + 32]; + int n = snprintf(path, sizeof(path), "%.*s_atlas_%u_%ux%u.png", + (int)(XR_ATLAS_CAPTURE_PATH_MAX_EXT - 1), info->pathPrefix, view_count, cols, rows); if (n <= 0 || (size_t)n >= sizeof(path)) { return oxr_error(log, XR_ERROR_VALIDATION_FAILURE, "xrCaptureAtlasEXT: path prefix too long"); } @@ -137,11 +158,11 @@ capture_inprocess(struct oxr_logger *log, // Per-view recommended dims approximate the captured eye-tile size. result->eyeWidth = sci->views[0].recommended.width_pixels; result->eyeHeight = sci->views[0].recommended.height_pixels; - // Tile layout and eye poses are not surfaced to oxr on the in-process - // path (no accessor; eye-pose plumbing stops at the display processor). - // Left zero; the IPC path below returns them. See the W6 design doc. - result->tileColumns = 0; - result->tileRows = 0; + // Tile layout from the active rendering mode (resolved above). Eye + // poses are still not surfaced on the in-process path — eye-pose + // plumbing stops at the display processor; the IPC path returns them. + result->tileColumns = cols; + result->tileRows = rows; result->displayWidthM = sci->display_width_m; result->displayHeightM = sci->display_height_m; result->eyeLeftM[0] = result->eyeLeftM[1] = result->eyeLeftM[2] = 0.0f; diff --git a/src/xrt/state_trackers/oxr/oxr_session.c b/src/xrt/state_trackers/oxr/oxr_session.c index eeb753834..78ef916af 100644 --- a/src/xrt/state_trackers/oxr/oxr_session.c +++ b/src/xrt/state_trackers/oxr/oxr_session.c @@ -2764,6 +2764,22 @@ oxr_session_create(struct oxr_logger *log, (xsi.external_window_handle != NULL || xsi.readback_callback != NULL || xsi.shared_texture_handle != NULL); sess->is_bridge_relay = xsi.is_bridge_relay; +#ifdef OXR_HAVE_EXT_display_info + // Authoritative legacy WARN: fires only when the compromise view scale + // computed in oxr_system_fill_in() will actually drive rendering — i.e. + // a real session exists on an instance without XR_EXT_display_info. + // Engine probe instances (created with zero extensions, then destroyed) + // never reach this point, so they no longer emit a misleading "Legacy + // app" WARN at xrCreateInstance. + if (!sys->inst->extensions.EXT_display_info && sys->xsysc != NULL && + sys->xsysc->info.legacy_app_tile_scaling) { + U_LOG_W("LEGACY session: app did not enable XR_EXT_display_info - " + "compromise view scale %.2fx%.2f (%ux%u per view) is in effect", + sys->xsysc->info.legacy_view_scale_x, sys->xsysc->info.legacy_view_scale_y, + sys->xsysc->info.legacy_view_width_pixels, sys->xsysc->info.legacy_view_height_pixels); + } +#endif + #ifdef XRT_OS_WINDOWS // GH #227 Tier 0: when workspace mode hid the app HWND above (the // ShowWindow(SW_HIDE) site at line ~2669), install the CBT hook that diff --git a/src/xrt/state_trackers/oxr/oxr_system.c b/src/xrt/state_trackers/oxr/oxr_system.c index ada97fef6..4f743277e 100644 --- a/src/xrt/state_trackers/oxr/oxr_system.c +++ b/src/xrt/state_trackers/oxr/oxr_system.c @@ -153,6 +153,37 @@ oxr_system_fill_in( // use 0.5x1.0 so the app renders full-height tiles. The compositor will // downscale Y for 3D and stretch X for 2D. if (!inst->extensions.EXT_display_info) { + // Engine OpenXR plug-ins (Unity, Unreal) create a throwaway probe + // instance with ZERO extensions before the real one. Such an + // instance can never create a session (no graphics binding / + // headless extension), so the compromise scale computed below is + // dead on arrival. Log it as a clearly-labeled INFO instead of a + // misleading "Legacy app" WARN — the authoritative legacy WARN + // fires in oxr_session_create() when the scale actually takes + // effect (probe instances never get there). + bool probe_only = true; +#ifdef OXR_HAVE_KHR_D3D11_enable + probe_only = probe_only && !inst->extensions.KHR_D3D11_enable; +#endif +#ifdef OXR_HAVE_KHR_D3D12_enable + probe_only = probe_only && !inst->extensions.KHR_D3D12_enable; +#endif +#ifdef OXR_HAVE_KHR_opengl_enable + probe_only = probe_only && !inst->extensions.KHR_opengl_enable; +#endif +#ifdef OXR_HAVE_KHR_vulkan_enable + probe_only = probe_only && !inst->extensions.KHR_vulkan_enable; +#endif +#ifdef OXR_HAVE_KHR_vulkan_enable2 + probe_only = probe_only && !inst->extensions.KHR_vulkan_enable2; +#endif +#ifdef OXR_HAVE_KHR_metal_enable + probe_only = probe_only && !inst->extensions.KHR_metal_enable; +#endif +#ifdef OXR_HAVE_MND_headless + probe_only = probe_only && !inst->extensions.MND_headless; +#endif + struct xrt_device *head = GET_XDEV_BY_ROLE(sys, head); if (head != NULL && head->rendering_mode_count > 1) { uint32_t default_3d_idx = head->hmd->active_rendering_mode_index; @@ -165,9 +196,6 @@ oxr_system_fill_in( info->legacy_app_tile_scaling = true; info->legacy_view_scale_x = 0.5f; info->legacy_view_scale_y = 1.0f; - U_LOG_W("Legacy app (no XR_EXT_display_info): using compromise " - "view scale 0.5x1.0 (3D mode '%s' is %.1fx%.1f)", - mode3d->mode_name, mode3d->view_scale_x, mode3d->view_scale_y); } else { // Case B: use 3D mode's actual scale, stretch for 2D view_scale_x = mode3d->view_scale_x; @@ -175,8 +203,17 @@ oxr_system_fill_in( info->legacy_app_tile_scaling = true; info->legacy_view_scale_x = mode3d->view_scale_x; info->legacy_view_scale_y = mode3d->view_scale_y; - U_LOG_W("Legacy app (no XR_EXT_display_info): using 3D mode '%s' " - "scale %.2fx%.2f", mode3d->mode_name, + } + if (probe_only) { + U_LOG_I("Probe instance (no extensions enabled): provisional " + "compromise view scale %.2fx%.2f computed - NOT used for " + "rendering, this instance cannot create a session", + view_scale_x, view_scale_y); + } else { + U_LOG_I("Instance without XR_EXT_display_info: compromise view " + "scale %.2fx%.2f provisioned (3D mode '%s' is %.2fx%.2f); " + "a LEGACY-session WARN fires at xrCreateSession if used", + view_scale_x, view_scale_y, mode3d->mode_name, mode3d->view_scale_x, mode3d->view_scale_y); } } @@ -210,7 +247,7 @@ oxr_system_fill_in( h = imin(h, h_max); if (i == 0 && info->legacy_app_tile_scaling) { - U_LOG_W("Legacy view[0]: raw w=%u h=%u (disp=%ux%u scale=%.2fx%.2f dbg_scale=%.2f) " + U_LOG_I("Compromise view[0] (provisional): raw w=%u h=%u (disp=%ux%u scale=%.2fx%.2f dbg_scale=%.2f) " "w_max=%u h_max=%u -> clamped w=%u h=%u", (uint32_t)(info->display_pixel_width * view_scale_x * scale), (uint32_t)(info->display_pixel_height * view_scale_y * scale), diff --git a/test_apps/common/atlas_capture.cpp b/test_apps/common/atlas_capture.cpp index f21868415..830ec489f 100644 --- a/test_apps/common/atlas_capture.cpp +++ b/test_apps/common/atlas_capture.cpp @@ -104,14 +104,17 @@ std::string MakeCapturePath(const std::string& stem, std::string MakeCaptureAtlasPrefix(const std::string& stem, uint32_t cols, uint32_t rows) { std::string dir = PicturesDirectory(); - // xrCaptureAtlasEXT appends "_atlas.png"; number against that final name so - // repeats accumulate rather than overwrite (the legacy "_CxR.png" name space - // is scanned separately, so the two never clobber each other's count). + // The runtime owns the layout tokens: it appends + // "_atlas__x.png" (viewCount == cols*rows). We pass + // just "-" so the final name isn't duplicated. Number against that + // final name so repeats accumulate rather than overwrite (the legacy + // "_CxR.png" name space is scanned separately, so the two never clobber + // each other's count). char atlasSuffix[80]; - snprintf(atlasSuffix, sizeof(atlasSuffix), "_%ux%u_atlas.png", cols, rows); + snprintf(atlasSuffix, sizeof(atlasSuffix), "_atlas_%u_%ux%u.png", cols * rows, cols, rows); int n = NextCaptureNumSuffix(dir, stem, atlasSuffix); char tail[256]; - snprintf(tail, sizeof(tail), "%s-%d_%ux%u", stem.c_str(), n, cols, rows); + snprintf(tail, sizeof(tail), "%s-%d", stem.c_str(), n); return dir.empty() ? std::string(tail) : (dir + "\\" + tail); } @@ -188,7 +191,8 @@ bool RequestRuntimeAtlasCapture(const ::XrSessionManager& xr, XrResult cr = xr.pfnCaptureAtlasEXT(xr.session, &info, nullptr); if (XR_SUCCEEDED(cr)) { - LOG_INFO("Atlas capture requested -> %s_atlas.png", prefix.c_str()); + LOG_INFO("Atlas capture requested -> %s_atlas_%u_%ux%u.png", prefix.c_str(), + tileColumns * tileRows, tileColumns, tileRows); PostFlashRequest(flashHwnd); return true; } diff --git a/test_apps/common/atlas_capture.h b/test_apps/common/atlas_capture.h index 1632b8a25..bd58d65ad 100644 --- a/test_apps/common/atlas_capture.h +++ b/test_apps/common/atlas_capture.h @@ -99,11 +99,13 @@ std::string MakeCapturePath(const std::string& stem, uint32_t cols, uint32_t rows); -// Path PREFIX (no extension) for xrCaptureAtlasEXT, which appends "_atlas.png". -// Numbers against existing "-_x_atlas.png" files (the +// Path PREFIX (no extension) for xrCaptureAtlasEXT, which appends +// "_atlas__x.png" (viewCount == cols*rows). Numbers +// against existing "-_atlas__x.png" files (the // runtime-produced names) so repeat captures accumulate instead of overwriting. -// Returns "/-_x" (no "_atlas", no ".png"). The legacy -// readback path keeps MakeCapturePath; the two name spaces don't collide. +// Returns "/-" (no "_atlas", no ".png") — the runtime owns the +// layout tokens. The legacy readback path keeps MakeCapturePath; the two name +// spaces don't collide. std::string MakeCaptureAtlasPrefix(const std::string& stem, uint32_t cols, uint32_t rows); diff --git a/test_apps/common/atlas_capture_macos.mm b/test_apps/common/atlas_capture_macos.mm index 990070503..021da5b36 100644 --- a/test_apps/common/atlas_capture_macos.mm +++ b/test_apps/common/atlas_capture_macos.mm @@ -100,14 +100,17 @@ int NextCaptureNum(const std::string& dir, std::string MakeCaptureAtlasPrefix(const std::string& stem, uint32_t cols, uint32_t rows) { std::string dir = PicturesDirectory(); - // xrCaptureAtlasEXT appends "_atlas.png"; number against that final name so - // repeats accumulate rather than overwrite (the legacy "_CxR.png" name space - // is scanned separately, so the two never clobber each other's count). + // The runtime owns the layout tokens: it appends + // "_atlas__x.png" (viewCount == cols*rows). We pass + // just "-" so the final name isn't duplicated. Number against that + // final name so repeats accumulate rather than overwrite (the legacy + // "_CxR.png" name space is scanned separately, so the two never clobber + // each other's count). char atlasSuffix[80]; - snprintf(atlasSuffix, sizeof(atlasSuffix), "_%ux%u_atlas.png", cols, rows); + snprintf(atlasSuffix, sizeof(atlasSuffix), "_atlas_%u_%ux%u.png", cols * rows, cols, rows); int n = NextCaptureNumSuffix(dir, stem, atlasSuffix); char tail[256]; - snprintf(tail, sizeof(tail), "%s-%d_%ux%u", stem.c_str(), n, cols, rows); + snprintf(tail, sizeof(tail), "%s-%d", stem.c_str(), n); return dir.empty() ? std::string(tail) : (dir + "/" + tail); } diff --git a/test_apps/cube_handle_gl_macos/main.mm b/test_apps/cube_handle_gl_macos/main.mm index 0aabf874f..fb27ca0d4 100644 --- a/test_apps/cube_handle_gl_macos/main.mm +++ b/test_apps/cube_handle_gl_macos/main.mm @@ -1858,7 +1858,8 @@ int main(int argc, char **argv) // XR_EXT_atlas_capture (W6 of #396) — the runtime does the readback // from its own atlas image, so the app keeps no staging texture. // PROJECTION_ONLY = the app's own projection atlas, pre-compose. - // Skipped for mono (1×1) layouts; the runtime appends "_atlas.png". + // Skipped for mono (1×1) layouts; the runtime appends + // "_atlas__x.png". if (g_input.captureAtlasRequested) { g_input.captureAtlasRequested = false; if (app.pfnCaptureAtlasEXT && app.session != XR_NULL_HANDLE) { @@ -1873,8 +1874,8 @@ int main(int argc, char **argv) info.pathPrefix[XR_ATLAS_CAPTURE_PATH_MAX_EXT - 1] = '\0'; XrResult cr = app.pfnCaptureAtlasEXT(app.session, &info, nullptr); if (XR_SUCCEEDED(cr)) { - LOG_INFO("Atlas capture requested -> %s_atlas.png", - prefix.c_str()); + LOG_INFO("Atlas capture requested -> %s_atlas_%u_%ux%u.png", + prefix.c_str(), tileColumns * tileRows, tileColumns, tileRows); dxr_capture::TriggerCaptureFlash((__bridge void*)g_glView); } else { LOG_WARN("xrCaptureAtlasEXT failed: 0x%x", (unsigned)cr); diff --git a/test_apps/cube_handle_metal_macos/main.mm b/test_apps/cube_handle_metal_macos/main.mm index beac0562a..28c633a86 100644 --- a/test_apps/cube_handle_metal_macos/main.mm +++ b/test_apps/cube_handle_metal_macos/main.mm @@ -1870,7 +1870,8 @@ int main(int argc, char **argv) // XR_EXT_atlas_capture (W6 of #396) — the runtime does the readback // from its own atlas image, so the app keeps no staging texture. // PROJECTION_ONLY = the app's own projection atlas, pre-compose. - // Skipped for mono (1×1) layouts; the runtime appends "_atlas.png". + // Skipped for mono (1×1) layouts; the runtime appends + // "_atlas__x.png". if (g_input.captureAtlasRequested) { g_input.captureAtlasRequested = false; if (app.pfnCaptureAtlasEXT && app.session != XR_NULL_HANDLE) { @@ -1885,8 +1886,8 @@ int main(int argc, char **argv) info.pathPrefix[XR_ATLAS_CAPTURE_PATH_MAX_EXT - 1] = '\0'; XrResult cr = app.pfnCaptureAtlasEXT(app.session, &info, nullptr); if (XR_SUCCEEDED(cr)) { - LOG_INFO("Atlas capture requested -> %s_atlas.png", - prefix.c_str()); + LOG_INFO("Atlas capture requested -> %s_atlas_%u_%ux%u.png", + prefix.c_str(), tileColumns * tileRows, tileColumns, tileRows); dxr_capture::TriggerCaptureFlash((__bridge void*)g_metalView); } else { LOG_WARN("xrCaptureAtlasEXT failed: 0x%x", (unsigned)cr); diff --git a/test_apps/cube_handle_vk_macos/main.mm b/test_apps/cube_handle_vk_macos/main.mm index da84380ac..0e783d804 100644 --- a/test_apps/cube_handle_vk_macos/main.mm +++ b/test_apps/cube_handle_vk_macos/main.mm @@ -3163,7 +3163,7 @@ int main() { // image, so the app keeps no staging texture. // PROJECTION_ONLY = the app's own projection atlas, // pre-compose. Skipped for mono (1×1) layouts; the - // runtime appends "_atlas.png". + // runtime appends "_atlas__x.png". if (g_input.captureAtlasRequested) { g_input.captureAtlasRequested = false; if (xr.pfnCaptureAtlasEXT && xr.session != XR_NULL_HANDLE) { @@ -3178,8 +3178,8 @@ int main() { info.pathPrefix[XR_ATLAS_CAPTURE_PATH_MAX_EXT - 1] = '\0'; XrResult cr = xr.pfnCaptureAtlasEXT(xr.session, &info, nullptr); if (XR_SUCCEEDED(cr)) { - LOG_INFO("Atlas capture requested -> %s_atlas.png", - prefix.c_str()); + LOG_INFO("Atlas capture requested -> %s_atlas_%u_%ux%u.png", + prefix.c_str(), tileColumns * tileRows, tileColumns, tileRows); dxr_capture::TriggerCaptureFlash((__bridge void*)g_metalView); } else { LOG_WARN("xrCaptureAtlasEXT failed: 0x%x", (unsigned)cr);