From a68bc85b0dbf0d72a82bbd043933bab6a64a1684 Mon Sep 17 00:00:00 2001 From: dfattal Date: Sat, 3 Oct 2026 11:05:12 -0700 Subject: [PATCH] fix(capture): IPC clients can capture their own atlas (#1810) An IPC client (forced IPC or shell-launched) had no working capture route. - Per-app MCP capture_frame + %TEMP%\displayxr_atlas_trigger[.][.projection]: the IPC client compositor now installs the same mcp_capture hand-off the in-process compositors do and polls u_capture_intent after each layer commit. A request asks the SERVICE for this client's per-client atlas (pre-weave, DP-input crop, encoded bytes as stored) over the existing workspace_capture_frame call, then moves the PNG to the exact path the in-process compositors write. Windows-only (only the D3D11 service answers). - Service: an APP caller now captures its OWN compositor's atlas (comp_d3d11_service_capture_frame_for_client) instead of whichever client rendered last; CONTROLLER/DIAG and the debug triggers are unchanged. - IPC_CAPTURE_FLAG_RAW_ALPHA carries the client's DXR_ATLAS_CAPTURE_RAW_ALPHA to the service (which reads its own env). Request-only modifier bit; the wire struct and proto.json are unchanged; an old service ignores it. xrCaptureAtlasDXR's IPC branch sends it too. - workspace_screenshot_trigger is now consumed for an ADR-029 CLIENT_TEXTURE presenter (pipeline_client_texture_weave): the render thread's poll sits past the PRESENTER_SELF/CLIENT_TEXTURE early return, so it was dead there. Stat before render_mutex at 4 Hz; dump only after a frame was woven. - workspace_capture_frame with flags == 0 (what the shell's MCP capture_frame sends) was rejected before any log; the stage now defaults to IPC_CAPTURE_FLAG_ALL, and a failed capture logs a WARN. In-process capture paths are untouched. Co-Authored-By: Claude Opus 5.5 --- .../d3d11_service/comp_d3d11_service.cpp | 77 ++++++++++- .../d3d11_service/comp_d3d11_service.h | 22 +++ src/xrt/ipc/client/ipc_client_compositor.c | 127 ++++++++++++++++++ src/xrt/ipc/server/ipc_server_handler.c | 30 ++++- src/xrt/ipc/shared/ipc_protocol.h | 7 + src/xrt/state_trackers/oxr/oxr_capture.c | 6 +- 6 files changed, 260 insertions(+), 9 deletions(-) diff --git a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp index a4b822b871..3256163873 100644 --- a/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp +++ b/src/xrt/compositor/d3d11_service/comp_d3d11_service.cpp @@ -19683,6 +19683,41 @@ pipeline_client_texture_weave(struct d3d11_service_system *sys, struct d3d11_ser // unbounded, and this thread must never hold `render_mutex` across it (#925). svc_ensure_client_weave_dp(sys, c); + // #1810: %TEMP%\workspace_screenshot_trigger for a CLIENT_TEXTURE presenter. + // The render thread's direct-path poll sits past the PRESENTER_SELF / + // CLIENT_TEXTURE early return in pipeline_default_policy_render, and this + // client's weave never reaches multi_compositor_render's compose poll, so the + // trigger was dead here — no PNG and no log. Stat it BEFORE taking + // render_mutex (4 Hz, the #925 audit rank 5 budget); consume it under the lock + // only once a frame was actually woven below. + bool ss_armed = false; + struct ss_paths + { + char trigger[MAX_PATH]; + char prefix[MAX_PATH]; + }; + // Every CLIENT_TEXTURE client's IPC thread runs this: a magic static, not + // the render thread's lazily-filled char arrays. + static const ss_paths s_ss = [] { + ss_paths p = {}; + const char *tmp = getenv("TEMP"); + if (!tmp) { + tmp = "C:\\Temp"; + } + snprintf(p.trigger, sizeof(p.trigger), "%s\\workspace_screenshot_trigger", tmp); + snprintf(p.prefix, sizeof(p.prefix), "%s\\workspace_screenshot", tmp); + return p; + }(); + { + static std::atomic ss_last_poll_ns{0}; + const int64_t ss_now_ns = (int64_t)os_monotonic_get_ns(); + int64_t ss_prev_ns = ss_last_poll_ns.load(std::memory_order_relaxed); + if (ss_now_ns - ss_prev_ns >= 250LL * 1000000LL && + ss_last_poll_ns.compare_exchange_strong(ss_prev_ns, ss_now_ns)) { + ss_armed = GetFileAttributesA(s_ss.trigger) != INVALID_FILE_ATTRIBUTES; + } + } + render_mutex_fair_lock lock(sys); // Re-check under the lock: focus can move between the load and here. #1208: we @@ -19808,6 +19843,21 @@ pipeline_client_texture_weave(struct d3d11_service_system *sys, struct d3d11_ser } } + // #1810: the screenshot trigger, armed above. Same two files as the render + // thread's sites: (1) workspace_screenshot.png = the woven frame this client + // is about to present (the scratch the DP wrote, on sys->device — this + // presenter kind is split-ineligible), (2) workspace_screenshot_atlas_*.png = + // THIS client's pre-weave atlas. Left on disk when nothing was woven, so the + // next woven frame takes it rather than writing nothing. Blocking Map — + // debug only, explicit trigger only, on this client's own IPC thread. + if (ss_armed && wove && DeleteFileA(s_ss.trigger)) { + dxr_diag_dump_tex(sys, c->render.transparent_weave_scratch.get(), "workspace_screenshot", + /*from_out*/ false); + struct ipc_capture_result dummy = {}; + comp_d3d11_service_capture_frame_for_client(&sys->base, &c->base.base, s_ss.prefix, + IPC_CAPTURE_FLAG_PROJECTION_ONLY, &dummy); + } + // ADR-029: signal the service->client fence. The client polls it with // GetCompletedValue() and copies the matching ring slice out - THE call that // wedged the render thread in #1017, bounded here to this client's own thread. @@ -27606,6 +27656,16 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, const char *path_prefix, uint32_t flags, struct ipc_capture_result *out_result) +{ + return comp_d3d11_service_capture_frame_for_client(xsysc, /*xc=*/nullptr, path_prefix, flags, out_result); +} + +bool +comp_d3d11_service_capture_frame_for_client(struct xrt_system_compositor *xsysc, + struct xrt_compositor *xc, + const char *path_prefix, + uint32_t flags, + struct ipc_capture_result *out_result) { if (xsysc == nullptr || path_prefix == nullptr || out_result == nullptr || flags == 0) { return false; @@ -27644,7 +27704,10 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, uint32_t dp_content_h = 0; if (flags & IPC_CAPTURE_FLAG_PROJECTION_ONLY) { std::lock_guard alock(sys->active_compositor_mutex); - struct d3d11_service_compositor *ac = sys->active_compositor; + // #1810: a named client captures ITSELF; NULL keeps the historical + // "whichever compositor rendered last" source (debug trigger, controller). + struct d3d11_service_compositor *ac = + xc != nullptr ? d3d11_service_compositor_from_xrt(xc) : sys->active_compositor; if (ac != nullptr) { // Prefer the content-sized crop atlas — the actual DP input, tightly // packed to (tile_columns·view_w)×(tile_rows·view_h) — so the PNG @@ -27756,8 +27819,10 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, // PNG doesn't render fully transparent/black (issue #425). // DXR_ATLAS_CAPTURE_RAW_ALPHA=1 opts out: the atlas's true alpha is the // only way to test compose-under / opaque-cover alpha (see - // u_image_capture_raw_alpha()). - if (!u_image_capture_raw_alpha()) { + // u_image_capture_raw_alpha()). #1810: an IPC client's own + // DXR_ATLAS_CAPTURE_RAW_ALPHA arrives as IPC_CAPTURE_FLAG_RAW_ALPHA, + // because this process reads the SERVICE's environment, not the client's. + if (!u_image_capture_raw_alpha() && (flags & IPC_CAPTURE_FLAG_RAW_ALPHA) == 0) { 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 @@ -27790,7 +27855,7 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, out_result->display_height_m = sys->base.info.display_height_m; struct xrt_vec3 le = {0, 0, 0}, re = {0, 0, 0}; - if (comp_d3d11_service_get_predicted_eye_positions(xsysc, &le, &re)) { + if (comp_d3d11_service_get_predicted_eye_positions_for_client(xsysc, xc, &le, &re)) { out_result->eye_left_m[0] = le.x; out_result->eye_left_m[1] = le.y; out_result->eye_left_m[2] = le.z; @@ -27799,8 +27864,8 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, out_result->eye_right_m[2] = re.z; } - U_LOG_W("capture_frame: prefix=%s flags=0x%x written=0x%x used=%ux%u (atlas=%ux%u) eye=%ux%u", - path_prefix, flags, views_written, used_w, used_h, atlas_w, atlas_h, eye_w, eye_h); + U_LOG_W("capture_frame: prefix=%s flags=0x%x written=0x%x used=%ux%u (atlas=%ux%u) eye=%ux%u client=%p", + path_prefix, flags, views_written, used_w, used_h, atlas_w, atlas_h, eye_w, eye_h, (void *)xc); return views_written != 0; } diff --git a/src/xrt/compositor/d3d11_service/comp_d3d11_service.h b/src/xrt/compositor/d3d11_service/comp_d3d11_service.h index de1214c9ad..40367a5a18 100644 --- a/src/xrt/compositor/d3d11_service/comp_d3d11_service.h +++ b/src/xrt/compositor/d3d11_service/comp_d3d11_service.h @@ -502,6 +502,28 @@ comp_d3d11_service_capture_frame(struct xrt_system_compositor *xsysc, uint32_t flags, struct ipc_capture_result *out_result); +/*! + * #1810: @ref comp_d3d11_service_capture_frame for ONE client's compositor. + * + * With @p xc non-NULL, @c IPC_CAPTURE_FLAG_PROJECTION_ONLY reads THAT client's + * per-client atlas (its content-sized DP-input crop, else its full atlas) instead + * of whichever compositor last rendered (`sys->active_compositor`), so a + * standalone IPC app capturing itself gets its own pixels even with other clients + * running. NULL @p xc is exactly @ref comp_d3d11_service_capture_frame. + * + * @p xc must stay alive for the call: pass the CALLING client's own compositor + * from its own IPC thread (compositor teardown runs on that same thread), or a + * compositor the caller otherwise holds under render_mutex. + * + * @ingroup comp_d3d11_service + */ +bool +comp_d3d11_service_capture_frame_for_client(struct xrt_system_compositor *xsysc, + struct xrt_compositor *xc, + const char *path_prefix, + uint32_t flags, + struct ipc_capture_result *out_result); + /*! * Service a pending MCP capture_frame request. Delegates to * comp_d3d11_service_capture_frame for the atlas, then writes diff --git a/src/xrt/ipc/client/ipc_client_compositor.c b/src/xrt/ipc/client/ipc_client_compositor.c index fc5e26094d..4c2555be04 100644 --- a/src/xrt/ipc/client/ipc_client_compositor.c +++ b/src/xrt/ipc/client/ipc_client_compositor.c @@ -25,6 +25,13 @@ #include "util/u_trace_marker.h" #include "util/u_limited_unique_id.h" #include "util/u_snap_grid.h" +#include "util/u_logging.h" + +#ifdef XRT_OS_WINDOWS +#include "util/u_capture_intent.h" +#include "util/u_image_capture.h" +#include +#endif #include "shared/ipc_protocol.h" #include "client/ipc_client.h" @@ -107,6 +114,17 @@ struct ipc_client_compositor //! sentinel and the legacy KeyedMutex path stays in effect. uint64_t workspace_sync_fence_value; +#ifdef XRT_OS_WINDOWS + //! #1810: MCP capture_frame request box — the same hand-off the in-process + //! compositors install, serviced at the end of layer_commit by asking the + //! SERVICE for this client's per-client atlas (see ipc_client_capture_poll). + struct mcp_capture_request mcp_capture; + //! #1810: per-commit capture intent (MCP request or %TEMP% trigger file). + struct u_capture_intent capture_intent; + //! True while @ref mcp_capture is installed as the process's handler. + bool mcp_capture_installed; +#endif + #ifdef IPC_USE_LOOPBACK_IMAGE_ALLOCATOR //! To test image allocator. struct xrt_image_native_allocator loopback_xina; @@ -2555,6 +2573,88 @@ ipc_compositor_layer_zone_3d(struct xrt_compositor *xc, return XRT_SUCCESS; } +#ifdef XRT_OS_WINDOWS +/*! + * #1810 — service a capture request for an IPC client. + * + * The in-process compositors read their own atlas back; an IPC client owns no + * atlas, so before this the MCP @c capture_frame tool answered "not wired" and + * the per-exe trigger file sat on disk forever. Both now ask the SERVICE for this + * client's per-client atlas — the pre-weave, content-sized buffer its display + * processor consumes, encoded bytes as stored (ADR-044 §5), alpha forced opaque + * unless DXR_ATLAS_CAPTURE_RAW_ALPHA=1 here — over the existing + * workspace_capture_frame call (the xrCaptureAtlasDXR IPC branch), then move the + * file to the exact path the in-process compositors would have written, so every + * recipe keeps its filename. + * + * Both MCP modes map to the service's PROJECTION_ONLY stage: that is the only + * per-client buffer the service has, and it is everything this client sends the + * DP (Local2D / window-space included on the standalone path). Under a + * workspace an app may not capture (#960) and this reports failure. + * + * Runs on the app's thread right after the commit's layer-sync call returned, + * once per commit — the same cadence as the in-process u_capture_intent_poll. + * The service call is bounded (copy + Map under render_mutex, on an explicit + * request only); nothing here waits on another process otherwise. + */ +static void +ipc_client_capture_poll(struct ipc_client_compositor *icc) +{ + if (!icc->mcp_capture_installed) { + return; + } + + u_capture_intent_poll(&icc->capture_intent, &icc->mcp_capture); + if (!icc->capture_intent.pending) { + return; + } + + const char *path = icc->capture_intent.path; + bool ok = false; + + // Prefix = the target path minus ".png"; the service appends + // "_atlas__x.png" and we rename it onto the target. + struct ipc_capture_request req = {0}; + size_t plen = strlen(path); + if (plen > 4 && _stricmp(path + plen - 4, ".png") == 0) { + plen -= 4; + } + if (plen > 0 && plen < sizeof(req.path_prefix)) { + memcpy(req.path_prefix, path, plen); + req.path_prefix[plen] = '\0'; + req.flags = IPC_CAPTURE_FLAG_PROJECTION_ONLY | + (u_image_capture_raw_alpha() ? IPC_CAPTURE_FLAG_RAW_ALPHA : 0u); + + struct ipc_capture_result result = {0}; + xrt_result_t xret = ipc_call_workspace_capture_frame(icc->ipc_c, &req, &result); + if (xret == XRT_SUCCESS && (result.views_written & IPC_CAPTURE_FLAG_PROJECTION_ONLY) != 0) { + uint32_t cols = result.tile_columns > 0 ? result.tile_columns : 1; + uint32_t rows = result.tile_rows > 0 ? result.tile_rows : 1; + char written[IPC_CAPTURE_PATH_MAX + 48]; + snprintf(written, sizeof(written), "%s_atlas_%u_%ux%u.png", req.path_prefix, cols * rows, + cols, rows); + ok = MoveFileExA(written, path, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) != 0; + if (ok) { + U_LOG_W("[capture] IPC client atlas -> %s (%ux%u, grid %ux%u, mode=%u, raw_alpha=%d)", path, + result.atlas_width, result.atlas_height, cols, rows, icc->capture_intent.mode, + (int)((req.flags & IPC_CAPTURE_FLAG_RAW_ALPHA) != 0)); + } else { + U_LOG_W("[capture] IPC client atlas: service wrote %s but the move to %s failed (%lu)", + written, path, (unsigned long)GetLastError()); + } + } else { + U_LOG_W("[capture] IPC client atlas: service wrote nothing for %s (xret=%d views_written=0x%x) " + "-- an app cannot capture under a workspace controller (#960); see the service log", + path, (int)xret, result.views_written); + } + } else { + U_LOG_W("[capture] IPC client atlas: path too long for the capture request: %s", path); + } + + u_capture_intent_complete(&icc->capture_intent, &icc->mcp_capture, ok); +} +#endif + static xrt_result_t ipc_compositor_layer_commit(struct xrt_compositor *xc, xrt_graphics_sync_handle_t sync_handle) { @@ -2591,6 +2691,10 @@ ipc_compositor_layer_commit(struct xrt_compositor *xc, xrt_graphics_sync_handle_ u_graphics_sync_unref(&sync_handle); } +#ifdef XRT_OS_WINDOWS + ipc_client_capture_poll(icc); // #1810 +#endif + return xret; } @@ -2626,6 +2730,10 @@ ipc_compositor_layer_commit_with_semaphore(struct xrt_compositor *xc, // Reset. icc->layers.layer_count = 0; +#ifdef XRT_OS_WINDOWS + ipc_client_capture_poll(icc); // #1810 +#endif + return xret; } @@ -2701,6 +2809,15 @@ ipc_compositor_destroy(struct xrt_compositor *xc) os_precise_sleeper_deinit(&icc->sleeper); +#ifdef XRT_OS_WINDOWS + // #1810: drop the MCP capture hand-off before its storage can be reused. + if (icc->mcp_capture_installed) { + mcp_capture_uninstall(); + mcp_capture_fini(&icc->mcp_capture); + icc->mcp_capture_installed = false; + } +#endif + icc->compositor_created = false; } @@ -2743,6 +2860,16 @@ ipc_compositor_init(struct ipc_client_compositor *icc, struct xrt_compositor_nat // Using in wait frame. os_precise_sleeper_init(&icc->sleeper); +#ifdef XRT_OS_WINDOWS + // #1810: wire the per-app MCP capture_frame tool + the %TEMP% atlas trigger + // files for this IPC session, exactly as the in-process compositors do at + // create. Windows-only: only the D3D11 service answers the capture call. + memset(&icc->capture_intent, 0, sizeof(icc->capture_intent)); + mcp_capture_init(&icc->mcp_capture); + mcp_capture_install(&icc->mcp_capture); + icc->mcp_capture_installed = true; +#endif + // Fetch info from the compositor, among it the format format list. get_info(&(icc->base.base), &icc->base.base.info); diff --git a/src/xrt/ipc/server/ipc_server_handler.c b/src/xrt/ipc/server/ipc_server_handler.c index f8e1ccec35..744bcc4c9b 100644 --- a/src/xrt/ipc/server/ipc_server_handler.c +++ b/src/xrt/ipc/server/ipc_server_handler.c @@ -5214,8 +5214,34 @@ ipc_handle_workspace_capture_frame(volatile struct ipc_client_state *_ics, memcpy(prefix, request->path_prefix, sizeof(prefix)); prefix[sizeof(prefix) - 1] = '\0'; - bool ok = comp_d3d11_service_capture_frame(s->xsysc, prefix, request->flags, - out_capture_result); + // #1810: flags == 0 means "runtime picks" — that is what the shell's MCP + // capture_frame sends (`req.flags = 0`), and comp_d3d11_service_capture_frame + // rejects a zero stage mask outright, so every shell capture came back + // {"ok":false} with nothing written and nothing logged. Default the STAGE + // bits only, keeping any modifier bit (RAW_ALPHA) the caller set. + uint32_t flags = request->flags; + if ((flags & (IPC_CAPTURE_FLAG_ATLAS | IPC_CAPTURE_FLAG_PROJECTION_ONLY)) == 0) { + flags |= IPC_CAPTURE_FLAG_ALL; + } + + // #1810: an APP (allowed only outside workspace mode, above) captures its + // OWN per-client atlas, not whichever client rendered last. ics->xc is this + // thread's own compositor, and its teardown runs on this same thread, so it + // outlives the call. CONTROLLER/DIAG keep the historical source. + struct xrt_compositor *self_xc = NULL; + { + uint32_t cls = _ics->client_state.client_class; + bool privileged = cls == XRT_CLIENT_CLASS_CONTROLLER || cls == XRT_CLIENT_CLASS_DIAG; + if (!privileged) { + self_xc = (struct xrt_compositor *)_ics->xc; + } + } + + bool ok = comp_d3d11_service_capture_frame_for_client(s->xsysc, self_xc, prefix, flags, out_capture_result); + if (!ok) { + IPC_WARN(s, "workspace_capture_frame: nothing written for pid %ld (prefix=%s flags=0x%x)", + _ics->peer_pid, prefix, flags); + } return ok ? XRT_SUCCESS : XRT_ERROR_IPC_FAILURE; #else (void)s; diff --git a/src/xrt/ipc/shared/ipc_protocol.h b/src/xrt/ipc/shared/ipc_protocol.h index 69e7b6c300..df30ffff49 100644 --- a/src/xrt/ipc/shared/ipc_protocol.h +++ b/src/xrt/ipc/shared/ipc_protocol.h @@ -566,6 +566,13 @@ struct ipc_workspace_input_event_batch // readback lands in a follow-up (Phase 2b); until then the service writes nothing // for this bit and the caller sees it absent from views_written. #define IPC_CAPTURE_FLAG_PROJECTION_ONLY (1u << 1) +// #1810: request-only MODIFIER, never a stage and never reported back in +// views_written. Asks the service to write the atlas's TRUE alpha instead of +// forcing it opaque — the client's DXR_ATLAS_CAPTURE_RAW_ALPHA, carried over the +// wire because the service reads its OWN environment, not the client's. A service +// that predates the bit ignores it (it only tests the two stage bits), so an old +// service + new client degrades to "service env decides", exactly as before. +#define IPC_CAPTURE_FLAG_RAW_ALPHA (1u << 2) #define IPC_CAPTURE_FLAG_ALL (IPC_CAPTURE_FLAG_ATLAS) #define IPC_CAPTURE_PATH_MAX 256 diff --git a/src/xrt/state_trackers/oxr/oxr_capture.c b/src/xrt/state_trackers/oxr/oxr_capture.c index 0dffc22f35..a41a2ec5c8 100644 --- a/src/xrt/state_trackers/oxr/oxr_capture.c +++ b/src/xrt/state_trackers/oxr/oxr_capture.c @@ -36,6 +36,7 @@ #include "util/u_logging.h" #include "util/u_trace_marker.h" #include "util/u_capture_dims.h" +#include "util/u_image_capture.h" #include "os/os_time.h" @@ -206,13 +207,16 @@ capture_ipc(struct oxr_logger *log, uint32_t want_flag = (info->stage == XR_ATLAS_CAPTURE_STAGE_PROJECTION_ONLY_DXR) ? IPC_CAPTURE_FLAG_PROJECTION_ONLY : IPC_CAPTURE_FLAG_ATLAS; + // #1810: the service reads its OWN environment, so this client's + // DXR_ATLAS_CAPTURE_RAW_ALPHA travels as a request modifier bit. + uint32_t req_flags = want_flag | (u_image_capture_raw_alpha() ? IPC_CAPTURE_FLAG_RAW_ALPHA : 0u); uint64_t ts_ns = 0; uint32_t aw = 0, ah = 0, ew = 0, eh = 0, vw = 0, tc = 0, tr = 0; float dw_m = 0.0f, dh_m = 0.0f; float eye_l[3] = {0}, eye_r[3] = {0}; xrt_result_t xret = comp_ipc_client_compositor_workspace_capture_frame( - &sess->xcn->base, info->pathPrefix, want_flag, &ts_ns, &aw, &ah, &ew, &eh, &vw, &tc, &tr, &dw_m, + &sess->xcn->base, info->pathPrefix, req_flags, &ts_ns, &aw, &ah, &ew, &eh, &vw, &tc, &tr, &dw_m, &dh_m, eye_l, eye_r); if (xret != XRT_SUCCESS) { return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,