From 574f41a0e43e84a3b2f03a12a0167856139f6c6e Mon Sep 17 00:00:00 2001 From: Daniel Lynch Date: Fri, 7 Aug 2026 22:41:25 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20deep-review=20round=20=E2=80=94=20legacy?= =?UTF-8?q?-ABI=20export,=20threading,=20and=20Vulkan=20robustness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ovrp_GetTimeInSeconds: real export returning double (legacy direct-return ABI); the autogen ovrpResult stub left d0 uninitialized and the game reads it - seqlock writer: fence the odd-mark before the data stores (torn-pose window) - drain the frameState ring on STOPPING/shutdown (stale displayTime + permanent one-frame skew after every session pause) - begin_frame FIFO bail: bound the wait (surface-teardown deadlock) and measure the deadline on CLOCK_MONOTONIC, immune to wall-clock steps - passthru: pthread_once takeover + acquire/release PT_FWD pointer cache (first-use race could run the shim path mid-takeover) - luma/dump/barcode readback disabled for non-4-byte swapchain formats (staging buffers assumed 4 B/texel; wider fallback formats would overflow) - vkWaitForFences: honor timeouts everywhere; never reset a pending cmd buffer - detect_ue_queue: keep the caller's queue family/index when detection fails - xrr_get_node_pose: g_spaceLock vs the sitting/standing appSpace recreate (locate on a freed XrSpace) Co-Authored-By: Claude Fable 5 --- shim/README.md | 7 +++++ shim/gen_stubs.sh | 2 +- shim/include/passthru.h | 16 +++++++++-- shim/src/core.c | 25 ++++++++++++++-- shim/src/passthru.c | 11 ++++--- shim/src/stubs.c | 3 +- shim/src/vk_session.c | 58 +++++++++++++++++++++++++++++-------- shim/src/xr_runtime.c | 64 +++++++++++++++++++++++++++++++++++++---- shim/src/xr_runtime.h | 4 +++ 9 files changed, 160 insertions(+), 30 deletions(-) diff --git a/shim/README.md b/shim/README.md index 79f9f20..b56e3f9 100644 --- a/shim/README.md +++ b/shim/README.md @@ -47,6 +47,13 @@ Session lifecycle, frame loop (xrWaitFrame/Begin/EndFrame), poses (xrLocateViews Space), swapchains (xrCreateSwapchain from ovrpLayerDesc, acquire/wait/release, XrCompositionLayerProjection submit). Vulkan binding from Initialize5 args [VERIFIED]. +## Debug: passthru forwarding (author-only diagnostic) +`debug.re4vr.passthru=1` makes the shim hand the whole OVRPlugin session to a real, +SONAME-patched `libOVRPlugin_real.so` if one is present in the APK — an A/B harness for +diffing native runtime behavior against the shim. **No such binary is included or +distributed by this repo**; you would have to place one from your own device dump, and +without it the flag is inert. All other `debug.re4vr.*` props are likewise diagnostics. + ## Next (see ../TESTING.md for the test plan) 1. NDK arm64 build (host build is validation only). 2. [ANDROID-TODO] JavaVM/activity -> XrInstanceCreateInfoAndroidKHR + xrInitializeLoaderKHR. diff --git a/shim/gen_stubs.sh b/shim/gen_stubs.sh index 4b707fd..443c538 100755 --- a/shim/gen_stubs.sh +++ b/shim/gen_stubs.sh @@ -18,7 +18,7 @@ ovrp_BeginFrame4 ovrp_EndFrame4 ovrp_GetPredictedDisplayTime ovrp_GetSystemHeads ovrp_GetTrackingOriginType2 ovrp_SetTrackingOriginType2 ovrp_RecenterTrackingOrigin2 \ ovrp_SetupLayer ovrp_GetLayerTextureStageCount ovrp_GetLayerTexture2 \ ovrp_GetInstanceExtensionsVk ovrp_GetDeviceExtensionsVk \ -ovrp_GetInitialized ovrp_GetSystemDisplayFrequency2 \ +ovrp_GetInitialized ovrp_GetSystemDisplayFrequency2 ovrp_GetTimeInSeconds \ ovrp_GetAppHasVrFocus2 ovrp_GetAppShouldQuit2 ovrp_GetUserPresent2 \ ovrp_GetAppShouldRecenter2 ovrp_GetAppShouldRecreateDistortionWindow2 \ ovrp_GetSystemMultiViewSupported2 ovrp_GetAppHasInputFocus ovrp_SetupDistortionWindow3 \ diff --git a/shim/include/passthru.h b/shim/include/passthru.h index ab8b1b7..2ae39d5 100644 --- a/shim/include/passthru.h +++ b/shim/include/passthru.h @@ -18,12 +18,22 @@ void pt_log_call(const char *name, long ret); /* rate-limited native call cens * resolved pointer per call site. Drop as the FIRST statement of each game-called export * in core.c / layers.c. Compiles to nothing reachable when passthru is off. Logs the * native return value (rate-limited) so we can diff the full call surface vs our shim. */ +/* The per-site cache is written by whichever thread gets here first while another may + * be mid-call on the same export: publish the pointer BEFORE the got-flag (release) and + * read the flag with acquire, or a second thread could see got=1 with a still-NULL + * pointer and fall through to the shim path for one call — the partial-forwarding + * mixed state that crashes mid-frame. Both threads resolving is fine (same value). */ #define PT_FWD(fn, ...) do { \ if (pt_active()) { \ static __typeof__(&fn) _pt_p; static int _pt_got; \ - if (!_pt_got) { _pt_p = (__typeof__(&fn))pt_real(#fn); _pt_got = 1; } \ - if (_pt_p) { \ - __typeof__(_pt_p(__VA_ARGS__)) _pt_r = _pt_p(__VA_ARGS__); \ + if (!__atomic_load_n(&_pt_got, __ATOMIC_ACQUIRE)) { \ + __atomic_store_n(&_pt_p, (__typeof__(&fn))pt_real(#fn), \ + __ATOMIC_RELAXED); \ + __atomic_store_n(&_pt_got, 1, __ATOMIC_RELEASE); \ + } \ + __typeof__(&fn) _pt_f = _pt_p; \ + if (_pt_f) { \ + __typeof__(_pt_f(__VA_ARGS__)) _pt_r = _pt_f(__VA_ARGS__); \ pt_log_call(#fn, (long)_pt_r); \ return _pt_r; \ } \ diff --git a/shim/src/core.c b/shim/src/core.c index 828afdd..20a1406 100644 --- a/shim/src/core.c +++ b/shim/src/core.c @@ -66,8 +66,12 @@ OVRP_EXPORT ovrpResult ovrp_EndFrame4(int frameIndex, const ovrpLayerSubmit *const *layerSubmitPtr, int layerSubmitCount, void *commandQueue) { if (pt_active()) { + /* open-coded PT_FWD (extra logging below) — same acquire/release cache as the macro */ static __typeof__(&ovrp_EndFrame4) _r; static int _g; - if (!_g) { _r = (__typeof__(&ovrp_EndFrame4))pt_real("ovrp_EndFrame4"); _g = 1; } + if (!__atomic_load_n(&_g, __ATOMIC_ACQUIRE)) { + __atomic_store_n(&_r, (__typeof__(&ovrp_EndFrame4))pt_real("ovrp_EndFrame4"), __ATOMIC_RELAXED); + __atomic_store_n(&_g, 1, __ATOMIC_RELEASE); + } if (_r) { /* native per-layer submit: id, head-lock flag, world pose — diff vs ours */ static int po = 0; @@ -102,7 +106,10 @@ OVRP_EXPORT ovrpResult ovrp_GetNodePoseState3(ovrpStep step, int frameIndex, ovrpNode nodeId, ovrpPoseStatef *outState) { if (pt_active()) { static __typeof__(&ovrp_GetNodePoseState3) _r; static int _g; - if (!_g) { _r = (__typeof__(&ovrp_GetNodePoseState3))pt_real("ovrp_GetNodePoseState3"); _g = 1; } + if (!__atomic_load_n(&_g, __ATOMIC_ACQUIRE)) { + __atomic_store_n(&_r, (__typeof__(&ovrp_GetNodePoseState3))pt_real("ovrp_GetNodePoseState3"), __ATOMIC_RELAXED); + __atomic_store_n(&_g, 1, __ATOMIC_RELEASE); + } if (_r) { ovrpResult rr = _r(step, frameIndex, nodeId, outState); /* native eye poses — the ghost diff target (cf. our STEREO/HEADvsEYE logs) */ @@ -186,6 +193,15 @@ OVRP_EXPORT ovrpBool ovrp_GetInitialized(void) { /* called every frame, many times — don't log (it rolls the logcat buffer) */ return (g_xr.session != XR_NULL_HANDLE) ? ovrpBool_True : ovrpBool_False; } +/* [legacy direct-return ABI — same class as GetInitialized] returns the current time + * as a double (seconds) in d0. The autogen stub returned ovrpResult in w0 and left d0 + * holding garbage — and the game DOES import this (shim_surface.txt), so it was reading + * an uninitialized FP register for OVRPlugin time. Same time base as XrTime / + * predictedDisplayTime (CLOCK_MONOTONIC ns on Meta + Monado Android). */ +OVRP_EXPORT double ovrp_GetTimeInSeconds(void) { + PT_FWD(ovrp_GetTimeInSeconds); + return (double)xrr_now_ns() * 1e-9; +} OVRP_EXPORT ovrpResult ovrp_GetSystemDisplayFrequency2(float *outFreq) { PT_FWD(ovrp_GetSystemDisplayFrequency2, outFreq); if (outFreq) *outFreq = 72.0f; /* Quest 2 default refresh */ @@ -237,7 +253,10 @@ OVRP_EXPORT ovrpBool ovrp_GetMixedRealityInitialized(void) { OVRP_EXPORT ovrpResult ovrp_GetNodeFrustum2(ovrpNode node, ovrpFrustum2f *out) { if (pt_active()) { static __typeof__(&ovrp_GetNodeFrustum2) _r; static int _g; - if (!_g) { _r = (__typeof__(&ovrp_GetNodeFrustum2))pt_real("ovrp_GetNodeFrustum2"); _g = 1; } + if (!__atomic_load_n(&_g, __ATOMIC_ACQUIRE)) { + __atomic_store_n(&_r, (__typeof__(&ovrp_GetNodeFrustum2))pt_real("ovrp_GetNodeFrustum2"), __ATOMIC_RELAXED); + __atomic_store_n(&_g, 1, __ATOMIC_RELEASE); + } if (_r) { ovrpResult rr = _r(node, out); if (out) { static int po = 0; diff --git a/shim/src/passthru.c b/shim/src/passthru.c index e1ba101..e083504 100644 --- a/shim/src/passthru.c +++ b/shim/src/passthru.c @@ -7,6 +7,7 @@ #include "xr_runtime.h" #include "log.h" #include +#include #ifdef __ANDROID__ #include #include @@ -62,9 +63,6 @@ PT_STUB(GetLayerTextureFoveation, -1005) PT_STUB(GetControllerHapticsDesc2, -1005) static void pt_first_use(void) { - static int done = 0; - if (done) return; - done = 1; #ifdef __ANDROID__ char s[PROP_VALUE_MAX] = {0}; if (__system_property_get("debug.re4vr.passthru", s) <= 0 || s[0] != '1') return; @@ -104,7 +102,12 @@ static void pt_first_use(void) { #endif } -int pt_active(void) { pt_first_use(); return g_pt_active; } +/* pthread_once, not a bare flag: UE calls ovrp exports from the game AND RHI threads at + * startup, and the takeover (dlopen + JNI_OnLoad + dlsym table) is multi-ms. A second + * thread racing a bare done-flag would run the SHIM path mid-takeover — exactly the + * partial-forwarding mixed state the module invariant forbids. once() blocks it instead. */ +static pthread_once_t g_pt_once = PTHREAD_ONCE_INIT; +int pt_active(void) { pthread_once(&g_pt_once, pt_first_use); return g_pt_active; } void *pt_real(const char *name) { return g_pt_handle ? dlsym(g_pt_handle, name) : 0; } /* Rate-limited native call census. Keyed by the string-literal pointer (each PT_FWD call diff --git a/shim/src/stubs.c b/shim/src/stubs.c index c86f4a1..ec36746 100644 --- a/shim/src/stubs.c +++ b/shim/src/stubs.c @@ -201,7 +201,6 @@ OVRP_EXPORT ovrpResult ovrp_GetSystemVolume() { static int o; if(!o){o=1;XRRLOG( OVRP_EXPORT ovrpResult ovrp_GetSystemVolume2() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetSystemVolume2 -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } OVRP_EXPORT ovrpResult ovrp_GetSystemVSyncCount() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetSystemVSyncCount -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } OVRP_EXPORT ovrpResult ovrp_GetSystemVSyncCount2() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetSystemVSyncCount2 -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } -OVRP_EXPORT ovrpResult ovrp_GetTimeInSeconds() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetTimeInSeconds -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } OVRP_EXPORT ovrpResult ovrp_GetTrackerFrustum() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetTrackerFrustum -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } OVRP_EXPORT ovrpResult ovrp_GetTrackerPose() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetTrackerPose -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } OVRP_EXPORT ovrpResult ovrp_GetTrackingCalibratedOrigin() { static int o; if(!o){o=1;XRRLOG("stub ovrp_GetTrackingCalibratedOrigin -> ovrpFailure_NotYetImplemented");} return ovrpFailure_NotYetImplemented; } @@ -381,4 +380,4 @@ OVRP_EXPORT ovrpResult ovrp_Update2() { static int o; if(!o){o=1;XRRLOG("stub ov OVRP_EXPORT ovrpResult ovrp_UpdateCameraDevices() { static int o; if(!o){o=1;XRRLOG("stub ovrp_UpdateCameraDevices -> ovrpFailure_Unsupported");} return ovrpFailure_Unsupported; } OVRP_EXPORT ovrpResult ovrp_UpdateExternalCamera() { static int o; if(!o){o=1;XRRLOG("stub ovrp_UpdateExternalCamera -> ovrpFailure_Unsupported");} return ovrpFailure_Unsupported; } -/* generated: unsup=103 noop=67 todo=202 skipped(core)=66 */ +/* generated: unsup=103 noop=67 todo=201 skipped(core)=67 */ diff --git a/shim/src/vk_session.c b/shim/src/vk_session.c index 18052db..29e4f89 100644 --- a/shim/src/vk_session.c +++ b/shim/src/vk_session.c @@ -61,11 +61,12 @@ static PFN_vkGetDeviceQueue load_get_device_queue(VkDevice dev) { PFN_vkGetDeviceProcAddr gdpa; vk_loaders(&gdpa, NULL); return gdpa ? (PFN_vkGetDeviceQueue)gdpa(dev, "vkGetDeviceQueue") : NULL; } +/* On failure the caller's (family,index) are left untouched — the game-provided + * queueFamilyIndex is a better default than a hardcoded 0. */ static void detect_ue_queue(VkDevice dev, uint32_t *family, uint32_t *index) { - *family = 0; *index = 0; - if (!s_queue) return; + if (!s_queue) { XRRLOG("queue detect: no UE queue handle yet, keeping fam=%u idx=%u", *family, *index); return; } PFN_vkGetDeviceQueue gdq = load_get_device_queue(dev); - if (!gdq) { XRRLOG("queue detect: no vkGetDeviceQueue"); return; } + if (!gdq) { XRRLOG("queue detect: no vkGetDeviceQueue, keeping fam=%u idx=%u", *family, *index); return; } for (uint32_t f = 0; f < 4; f++) for (uint32_t i = 0; i < 4; i++) { VkQueue q = VK_NULL_HANDLE; @@ -76,7 +77,7 @@ static void detect_ue_queue(VkDevice dev, uint32_t *family, uint32_t *index) { return; } } - XRRLOG("queue detect: UE queue not matched, defaulting 0/0"); + XRRLOG("queue detect: UE queue not matched, keeping fam=%u idx=%u", *family, *index); } /* ----------------------------------------------- app-side extension queries -- */ @@ -295,8 +296,13 @@ int xrr_vk_flush_submit_ex(uint64_t image, unsigned int arrayLayers, int isDepth if (!vk_lazy_init()) return -1; int idx = s_ring++ % XRR_FLUSH_RING; /* this slot's previous submit must be done before we re-record it (its fence - * was already waited at present time a frame ago, so this is ~instant) */ - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000 /*100ms*/); + * was already waited at present time a frame ago, so this is ~instant). On a + * TIMEOUT the buffer is still pending — resetting it then is UB, so put the + * slot back and skip this flush (a possible black frame beats corruption). */ + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000 /*100ms*/) != VK_SUCCESS) { + s_ring--; XRRERR("flush: slot %d still pending after 100ms (GPU stalled), skipping", idx); + return -1; + } p_ResetFences(s_dev, 1, &s_fence[idx]); p_ResetCmd(s_cmd[idx], 0); VkCommandBufferBeginInfo bi = { VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; @@ -432,8 +438,18 @@ static int dump_lazy(PFN_vkGetDeviceProcAddr gdpa) { return 1; } +/* The luma/dump/barcode readbacks size staging buffers at 4 bytes/texel and read byte + * channels; setup_layer clears this when the negotiated swapchain format is wider + * (else the GPU copy writes past the staging allocation = device-memory corruption). */ +static int s_rb4 = 1; +void xrr_vk_set_readback_4byte(int ok) { + if (!ok) XRRLOG("readback: swapchain format not 4-byte RGBA/BGRA — luma/dump/barcode disabled"); + s_rb4 = ok; +} + void xrr_vk_dump_image(uint64_t image, unsigned int w, unsigned int h, unsigned int arrayLayer, const char *path) { + if (!s_rb4) { XRRERR("dump: non-4-byte swapchain format, refusing"); return; } if (!vk_lazy_init() || !s_phys) { XRRERR("dump: not ready"); return; } PFN_vkGetDeviceProcAddr gdpa; PFN_vkGetInstanceProcAddr gipa; vk_loaders(&gdpa, &gipa); if (!gdpa || !dump_lazy(gdpa)) return; @@ -462,7 +478,10 @@ void xrr_vk_dump_image(uint64_t image, unsigned int w, unsigned int h, p_BindBuf(s_dev, buf, mem, 0); int idx = s_ring++ % XRR_FLUSH_RING; - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000) != VK_SUCCESS) { + s_ring--; XRRERR("dump: slot %d still pending (GPU stalled), skipping", idx); + goto cleanup; /* reset-while-pending is UB */ + } p_ResetFences(s_dev, 1, &s_fence[idx]); p_ResetCmd(s_cmd[idx], 0); VkCommandBufferBeginInfo cbi = { VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; @@ -496,7 +515,9 @@ void xrr_vk_dump_image(uint64_t image, unsigned int w, unsigned int h, VkSubmitInfo si = { VK_STRUCTURE_TYPE_SUBMIT_INFO }; si.commandBufferCount = 1; si.pCommandBuffers = &s_cmd[idx]; if (p_Submit(s_queue, 1, &si, s_fence[idx]) != VK_SUCCESS) { XRRERR("dump: submit"); goto cleanup; } - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 1000000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 1000000000) != VK_SUCCESS) { + XRRERR("dump: copy fence timed out, not reading"); goto cleanup; + } { /* downsample to <=256 wide PPM (P6). Format is RGBA8; take RGB. */ uint8_t *px = NULL; @@ -586,6 +607,7 @@ static uint8_t *s_luPx; static unsigned s_luCap; /* bytes allocated */ int xrr_vk_frame_luma(uint64_t image, unsigned int w, unsigned int h, unsigned int arrayLayer) { + if (!s_rb4) return -1; if (!vk_lazy_init() || !s_phys) return -1; PFN_vkGetDeviceProcAddr gdpa; PFN_vkGetInstanceProcAddr gipa; vk_loaders(&gdpa, &gipa); if (!gdpa || !dump_lazy(gdpa)) return -1; /* dump_lazy loads CreateBuf/Copy2Buf/Map/etc */ @@ -616,7 +638,10 @@ int xrr_vk_frame_luma(uint64_t image, unsigned int w, unsigned int h, unsigned i } int idx = s_ring++ % XRR_FLUSH_RING; - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000) != VK_SUCCESS) { + s_ring--; XRRERR("luma: slot %d still pending (GPU stalled), skipping", idx); + return -1; /* reset-while-pending is UB */ + } p_ResetFences(s_dev, 1, &s_fence[idx]); p_ResetCmd(s_cmd[idx], 0); VkCommandBufferBeginInfo cbi = { VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; @@ -648,7 +673,9 @@ int xrr_vk_frame_luma(uint64_t image, unsigned int w, unsigned int h, unsigned i VkSubmitInfo si = { VK_STRUCTURE_TYPE_SUBMIT_INFO }; si.commandBufferCount = 1; si.pCommandBuffers = &s_cmd[idx]; if (p_Submit(s_queue, 1, &si, s_fence[idx]) != VK_SUCCESS) { XRRERR("luma: submit"); return -1; } - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000) != VK_SUCCESS) { + XRRERR("luma: copy fence timed out, not reading"); return -1; + } /* max luminance over the sampled pixels (RGBA8; max channel is enough for "any light"). */ int mx = 0; unsigned npx = LUMA_ROWS * w; for (unsigned i = 0; i < npx; i++) { @@ -661,6 +688,7 @@ int xrr_vk_frame_luma(uint64_t image, unsigned int w, unsigned int h, unsigned i void xrr_vk_stamp_barcode(uint64_t image, unsigned int w, unsigned int h, unsigned int arrayLayer, unsigned int value, int flagged) { + if (!s_rb4) return; /* strip staging is 4-byte texels */ if (!vk_lazy_init() || !s_phys) return; PFN_vkGetDeviceProcAddr gdpa; PFN_vkGetInstanceProcAddr gipa; vk_loaders(&gdpa, &gipa); if (!gdpa) return; @@ -691,7 +719,10 @@ void xrr_vk_stamp_barcode(uint64_t image, unsigned int w, unsigned int h, } int idx = s_ring++ % XRR_FLUSH_RING; - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000) != VK_SUCCESS) { + s_ring--; XRRERR("barcode: slot %d still pending (GPU stalled), skipping", idx); + return; /* reset-while-pending is UB */ + } p_ResetFences(s_dev, 1, &s_fence[idx]); p_ResetCmd(s_cmd[idx], 0); VkCommandBufferBeginInfo cbi = { VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; @@ -840,7 +871,10 @@ int xrr_vk_copy_submit_ex(uint64_t srcShim, uint64_t dstXr, unsigned int w, unsigned int h, unsigned int arrayLayers, int isDepth) { if (!vk_lazy_init() || !p_CopyImg) return -1; int idx = s_ring++ % XRR_FLUSH_RING; - p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000); + if (p_WaitFences(s_dev, 1, &s_fence[idx], VK_TRUE, 100000000) != VK_SUCCESS) { + s_ring--; XRRERR("copy: slot %d still pending (GPU stalled), skipping", idx); + return -1; /* reset-while-pending is UB */ + } p_ResetFences(s_dev, 1, &s_fence[idx]); p_ResetCmd(s_cmd[idx], 0); VkCommandBufferBeginInfo bi = { VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO }; diff --git a/shim/src/xr_runtime.c b/shim/src/xr_runtime.c index 0fbc871..623198c 100644 --- a/shim/src/xr_runtime.c +++ b/shim/src/xr_runtime.c @@ -23,6 +23,7 @@ static int64_t now_ns(void) { clock_gettime(CLOCK_MONOTONIC, &ts); return (int64_t)ts.tv_sec * 1000000000LL + ts.tv_nsec; } +int64_t xrr_now_ns(void) { return now_ns(); } /* for ovrp_GetTimeInSeconds (core.c) */ XrRuntime g_xr; @@ -426,7 +427,15 @@ static int g_havePerfExt = 0; /* XR_EXT_performance_settings advertised */ static int g_haveFoveation = 0; /* XR_FB_foveation (+config+swapchain_update_state) advertised */ static int g_haveEyeTrackedFov = 0;/* XR_META_foveation_eye_tracked advertised (Steam Frame/Quest Pro) */ +/* Guards appSpace destroy/recreate against the UNLOCKED xrLocateSpace in + * xrr_get_node_pose (hot game-thread getter that must not contend on g_xrlock — + * the render thread holds that across GPU waits). The in-game sitting/standing + * toggle recreates appSpace mid-play; locating a freed XrSpace crashes runtimes. + * Order: g_xrlock -> g_spaceLock only (never the reverse). */ +static pthread_mutex_t g_spaceLock = PTHREAD_MUTEX_INITIALIZER; + static void make_app_space(int floor) { + pthread_mutex_lock(&g_spaceLock); if (g_xr.appSpace != XR_NULL_HANDLE) { xrDestroySpace(g_xr.appSpace); g_xr.appSpace = XR_NULL_HANDLE; } XrReferenceSpaceCreateInfo ci = { XR_TYPE_REFERENCE_SPACE_CREATE_INFO }; ci.poseInReferenceSpace.orientation.w = 1.0f; @@ -438,7 +447,7 @@ static void make_app_space(int floor) { ci.referenceSpaceType = XR_REFERENCE_SPACE_TYPE_LOCAL_FLOOR; if (xrCreateReferenceSpace(g_xr.session, &ci, &g_xr.appSpace) == XR_SUCCESS) { XRRLOG("app space: LOCAL_FLOOR (floor, player-centred)"); - g_floorOrigin = floor; return; + g_floorOrigin = floor; pthread_mutex_unlock(&g_spaceLock); return; } XRRLOG("app space: LOCAL_FLOOR create failed, falling back to LOCAL"); } @@ -446,6 +455,7 @@ static void make_app_space(int floor) { xrCreateReferenceSpace(g_xr.session, &ci, &g_xr.appSpace); XRRLOG("app space: LOCAL (eye level) [requested floor=%d]", floor); g_floorOrigin = floor; + pthread_mutex_unlock(&g_spaceLock); } void xrr_set_tracking_origin(int floor) { @@ -648,6 +658,7 @@ void xrr_shutdown(void) { pthread_mutex_lock(&g_xrlock); g_xr.running = 0; + g_fsTail = g_fsHead; /* the ring outlives the memset below — drain it */ pthread_cond_broadcast(&g_fsCond); /* poll_events already issued xrEndSession on STOPPING; destroy is valid from any state. */ if (g_xr.session) xrDestroySession(g_xr.session); @@ -689,6 +700,11 @@ void xrr_poll_events(void) { g_xr.inFrame = 0; } pipeline_reset(); /* release held images, drop stale composition */ + /* drop unconsumed frameStates: the game's ~1-frame lead means one is + * normally queued here; carrying it across the restart would make the + * first resumed frame present with a minutes-old displayTime AND leave + * the ring permanently one entry ahead (a frame of extra pose latency). */ + g_fsTail = g_fsHead; xrEndSession(g_xr.session); g_xr.running = 0; } @@ -746,13 +762,33 @@ ovrpResult xrr_begin_frame(int frameIndex) { * game leads by ~1 frame so it's normally already queued; wait briefly otherwise. xrWaitFrame is * NO LONGER called here — pacing now blocks the game thread (vrapi model), freeing this render * thread from pacing back-pressure that was stalling UE's render -> dropped frames -> judder. */ + /* BOUND this wait. The game thread feeds the ring, but during an Android surface teardown + * (e.g. a DOFF / guardian interstitial fired mid-load) the game thread blocks in + * onSurfaceDestroyed waiting for THIS render thread to reach a safe point. If we spin here + * indefinitely the two deadlock: game thread never feeds the ring, render thread never returns + * to release the surface -> ANR -> infinite black. Cap the wait at a few frames and bail with + * the already-handled soft failure so the render thread returns and UE can drain its command + * queue (including the surface-release). Normal operation never hits this (game leads by ~1 + * frame, ring is non-empty); only a stalled/blocked game thread does. */ + /* MONOTONIC for the deadline: Android steps CLOCK_REALTIME on time-sync (e.g. Wi-Fi + * reconnect right after a doff — the very scenario this bound targets), which would + * stretch or falsify a wall-clock bound. The 20ms timedwait slices below must stay + * REALTIME (default-attr condvar), but they only bound each nap, not the total. */ + int64_t deadlineNs = now_ns() + 100LL * 1000000LL; /* ~7 frames @72Hz; << the 5s ANR threshold */ while (g_fsHead == g_fsTail && g_xr.running) { struct timespec to; clock_gettime(CLOCK_REALTIME, &to); to.tv_nsec += 20L * 1000000L; if (to.tv_nsec >= 1000000000L) { to.tv_sec++; to.tv_nsec -= 1000000000L; } pthread_cond_timedwait(&g_fsCond, &g_xrlock, &to); + if (now_ns() >= deadlineNs) { + static long b = 0; + if (b++ < 30 || (b % 240) == 0) + XRRLOG("begin_frame: FIFO starved >100ms, bailing to avoid " + "surface-teardown deadlock (#%ld)", b); + break; + } } - if (g_fsHead == g_fsTail) { /* shutting down or game produced no wait */ + if (g_fsHead == g_fsTail) { /* shutting down, game thread stalled/blocked, or no wait produced */ pthread_mutex_unlock(&g_xrlock); return ovrpFailure_NotYetImplemented; } g_xr.frameState = g_fsRing[g_fsTail % FS_RING]; @@ -768,10 +804,14 @@ ovrpResult xrr_begin_frame(int frameIndex) { /* publish the view snapshot for the game-thread getters (seqlock writer; single writer = us) */ { unsigned s = g_poseSeq; - __atomic_store_n(&g_poseSeq, s + 1, __ATOMIC_RELEASE); /* odd: write in progress */ + __atomic_store_n(&g_poseSeq, s + 1, __ATOMIC_RELAXED); /* odd: write in progress */ + /* store-store fence: the odd-mark must become visible BEFORE the data stores. A + * release store on the mark alone doesn't give that (it only orders EARLIER accesses), + * so a reader could see even+torn data. */ + __atomic_thread_fence(__ATOMIC_RELEASE); g_pubViews[0] = g_xr.views[0]; g_pubViews[1] = g_xr.views[1]; g_pubViewCount = g_xr.viewCount; g_pubDisplayTime = g_xr.frameState.predictedDisplayTime; - __atomic_store_n(&g_poseSeq, s + 2, __ATOMIC_RELEASE); /* even: stable */ + __atomic_store_n(&g_poseSeq, s + 2, __ATOMIC_RELEASE); /* even: stable (data ordered before) */ } { /* DIAG: is the head pose UE renders from (xrLocateSpace VIEW, node=Head) * consistent with the eye poses we submit (xrLocateViews)? UE builds its @@ -1971,6 +2011,14 @@ ovrpResult xrr_setup_layer(const ovrpLayerDesc *desc, int *outLayerId) { (long long)L->colorFormat, (long long)fallback, nf); L->colorFormat = fallback; } + /* the luma/dump/barcode readback paths size their staging buffers at 4 bytes/texel; + * a wider fallback format (e.g. RGBA16F on some runtimes) would overflow them, so + * tell vk_session whether readback is safe for this format. */ + if (L->isEyeFov) + xrr_vk_set_readback_4byte(L->colorFormat == 37 /*R8G8B8A8_UNORM*/ || + L->colorFormat == 43 /*R8G8B8A8_SRGB*/ || + L->colorFormat == 44 /*B8G8R8A8_UNORM*/ || + L->colorFormat == 50 /*B8G8R8A8_SRGB*/); int useCopyRing = L->isEyeFov && copyring_wanted(); XrSwapchainCreateInfo ci = { XR_TYPE_SWAPCHAIN_CREATE_INFO }; @@ -2136,7 +2184,13 @@ ovrpResult xrr_get_node_pose(ovrpNode node, ovrpPoseStatef *out) { if (node == ovrpNode_Head || node == ovrpNode_EyeCenter) { XrTime dt; pose_snapshot(NULL, NULL, &dt); XrSpaceLocation loc = { XR_TYPE_SPACE_LOCATION }; - if (xrLocateSpace(g_xr.viewSpace, g_xr.appSpace, dt, &loc) == XR_SUCCESS) + /* g_spaceLock: the sitting/standing toggle destroys+recreates appSpace + * concurrently (make_app_space); locating a freed handle is a crash. */ + pthread_mutex_lock(&g_spaceLock); + int ok = g_xr.viewSpace != XR_NULL_HANDLE && g_xr.appSpace != XR_NULL_HANDLE && + xrLocateSpace(g_xr.viewSpace, g_xr.appSpace, dt, &loc) == XR_SUCCESS; + pthread_mutex_unlock(&g_spaceLock); + if (ok) ovrp_pose_from_xr(&loc.pose, &out->Pose); return ovrpSuccess; } diff --git a/shim/src/xr_runtime.h b/shim/src/xr_runtime.h index f9f5b2f..3d8cd12 100644 --- a/shim/src/xr_runtime.h +++ b/shim/src/xr_runtime.h @@ -165,6 +165,10 @@ void xrr_vk_stamp_barcode(uint64_t image, unsigned int w, unsigned int h, /* per-frame black detector: max luminance (0..255) over a few rows of the resolved eye * image; ~0 => truncated/black frame. Covers the whole black tail (debug.re4vr.lumagate). */ int xrr_vk_frame_luma(uint64_t image, unsigned int w, unsigned int h, unsigned int arrayLayer); +/* luma/dump/barcode staging assumes 4-byte texels; setup_layer clears this for wider formats */ +void xrr_vk_set_readback_4byte(int ok); +/* CLOCK_MONOTONIC ns (the XrTime base) — for ovrp_GetTimeInSeconds */ +int64_t xrr_now_ns(void); /* input (xr_input.c) — OpenXR action sets -> ovrpControllerState4 + hand poses */ int xrr_input_init(void);