mirror of
https://github.com/daniel-lynch/ovrplugin-openxr-shim.git
synced 2026-10-06 03:00:05 +02:00
fix(xr): keep a timed-out swapchain image acquired and retry its wait
On XR_TIMEOUT_EXPIRED from xrWaitSwapchainImage, begin_frame released the image. Per spec that release fails (CALL_ORDER_INVALID: the image was never waited), so the acquire leaked and acquiredIndex desynced for good — the next wait returns the oldest un-waited image, not the newly acquired one. Track acquired-but-unwaited per swapchain (waitPending / depthWaitPending): on timeout the image stays acquired, the layer (or its depth chain) is left out of this frame's composition, and the next begin_frame retries the wait without a new acquire. Teardown paths never release an unwaited image: pipeline_reset leaves it for the post-restart retry (and now also releases held depth/deferred images it used to leak), destroy_layer just destroys the swapchain. These are the only two xrWaitSwapchainImage call sites. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
1f3dc40c07
commit
1d64595266
2 files changed
+50
-11
No files matched your search
+45
-11
@@ -398,14 +398,21 @@ static int dump_pending(void) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Drop any in-flight pipelined state and release held images (session teardown).
|
/* Drop any in-flight pipelined state and release held images (session teardown).
|
||||||
* Caller must hold g_xrlock and the session must still be valid. */
|
* Caller must hold g_xrlock and the session must still be valid. Only WAITED images
|
||||||
|
* are released; an acquired-but-unwaited one (waitPending/depthWaitPending) can't be,
|
||||||
|
* so it stays acquired and the next begin_frame (after a session restart) retries its
|
||||||
|
* wait — the swapchains outlive the session stop. */
|
||||||
static void pipeline_reset(void) {
|
static void pipeline_reset(void) {
|
||||||
for (int i = 0; i < g_xr.layerCount; i++) {
|
for (int i = 0; i < g_xr.layerCount; i++) {
|
||||||
XrLayer *L = &g_xr.layers[i];
|
XrLayer *L = &g_xr.layers[i];
|
||||||
XrSwapchainImageReleaseInfo ri = { XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO };
|
XrSwapchainImageReleaseInfo ri = { XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO };
|
||||||
if (L->presentPending) { xrReleaseSwapchainImage(L->swapchain, &ri); L->presentPending = 0; }
|
if (L->presentPending) { xrReleaseSwapchainImage(L->swapchain, &ri); L->presentPending = 0; }
|
||||||
|
if (L->deferColor) { xrReleaseSwapchainImage(L->swapchain, &ri); L->deferColor = 0; }
|
||||||
if (L->imageAcquired) { xrReleaseSwapchainImage(L->swapchain, &ri); L->imageAcquired = 0; }
|
if (L->imageAcquired) { xrReleaseSwapchainImage(L->swapchain, &ri); L->imageAcquired = 0; }
|
||||||
|
if (L->deferDepth) { xrReleaseSwapchainImage(L->depthSwapchain, &ri); L->deferDepth = 0; }
|
||||||
|
if (L->depthAcquired) { xrReleaseSwapchainImage(L->depthSwapchain, &ri); L->depthAcquired = 0; }
|
||||||
}
|
}
|
||||||
|
g_deferPresent = 0;
|
||||||
g_pending.valid = 0;
|
g_pending.valid = 0;
|
||||||
g_pipelineActive = 0;
|
g_pipelineActive = 0;
|
||||||
}
|
}
|
||||||
@@ -862,6 +869,10 @@ ovrpResult xrr_begin_frame(int frameIndex) {
|
|||||||
for (int i = 0; i < g_xr.layerCount; i++) {
|
for (int i = 0; i < g_xr.layerCount; i++) {
|
||||||
XrLayer *L = &g_xr.layers[i];
|
XrLayer *L = &g_xr.layers[i];
|
||||||
if (!L->active || L->swapchain == XR_NULL_HANDLE || L->imageAcquired) continue;
|
if (!L->active || L->swapchain == XR_NULL_HANDLE || L->imageAcquired) continue;
|
||||||
|
/* A previous frame's wait timed out: that image is still acquired (and is the
|
||||||
|
* oldest un-waited one), so retry the wait on it — a fresh acquire would leak
|
||||||
|
* it and desync acquiredIndex from the image the wait actually returns. */
|
||||||
|
if (!L->waitPending) {
|
||||||
XrSwapchainImageAcquireInfo ai = { XR_TYPE_SWAPCHAIN_IMAGE_ACQUIRE_INFO };
|
XrSwapchainImageAcquireInfo ai = { XR_TYPE_SWAPCHAIN_IMAGE_ACQUIRE_INFO };
|
||||||
XrResult ar = xrAcquireSwapchainImage(L->swapchain, &ai, &L->acquiredIndex);
|
XrResult ar = xrAcquireSwapchainImage(L->swapchain, &ai, &L->acquiredIndex);
|
||||||
{ /* DIAG: index sequence + any acquire failure (e.g. second acquire while
|
{ /* DIAG: index sequence + any acquire failure (e.g. second acquire while
|
||||||
@@ -872,33 +883,50 @@ ovrpResult xrr_begin_frame(int frameIndex) {
|
|||||||
}
|
}
|
||||||
if (ar != XR_SUCCESS)
|
if (ar != XR_SUCCESS)
|
||||||
continue;
|
continue;
|
||||||
|
}
|
||||||
XrSwapchainImageWaitInfo wi = { XR_TYPE_SWAPCHAIN_IMAGE_WAIT_INFO };
|
XrSwapchainImageWaitInfo wi = { XR_TYPE_SWAPCHAIN_IMAGE_WAIT_INFO };
|
||||||
wi.timeout = 100000000; /* 100 ms — never block the render thread forever */
|
wi.timeout = 100000000; /* 100 ms — never block the render thread forever */
|
||||||
XrResult wr = xrWaitSwapchainImage(L->swapchain, &wi);
|
XrResult wr = xrWaitSwapchainImage(L->swapchain, &wi);
|
||||||
if (wr == XR_TIMEOUT_EXPIRED) {
|
if (wr == XR_TIMEOUT_EXPIRED) {
|
||||||
/* couldn't get the image in time: release the acquire so we stay
|
/* couldn't get the image in time: it stays ACQUIRED (an un-waited image
|
||||||
* balanced and skip this layer this frame rather than deadlock. */
|
* can't be released), the layer is skipped this frame, and the next
|
||||||
|
* begin_frame retries the wait rather than deadlocking here. */
|
||||||
static int tw = 0;
|
static int tw = 0;
|
||||||
if (tw++ < 40) XRRLOG("WaitSwapchainImage TIMEOUT layer=%d idx=%u", i, L->acquiredIndex);
|
if (tw++ < 40) XRRLOG("WaitSwapchainImage TIMEOUT layer=%d idx=%u (retry=%d)",
|
||||||
XrSwapchainImageReleaseInfo ri = { XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO };
|
i, L->acquiredIndex, L->waitPending);
|
||||||
xrReleaseSwapchainImage(L->swapchain, &ri);
|
L->waitPending = 1;
|
||||||
L->imageAcquired = 0;
|
L->imageAcquired = 0;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
if (L->waitPending) {
|
||||||
|
static int wrc = 0;
|
||||||
|
if (wrc++ < 40) XRRLOG("WaitSwapchainImage recovered layer=%d idx=%u", i, L->acquiredIndex);
|
||||||
|
}
|
||||||
|
L->waitPending = 0;
|
||||||
L->imageAcquired = 1;
|
L->imageAcquired = 1;
|
||||||
/* acquire+wait the paired depth image (lockstep with color) so UE renders
|
/* acquire+wait the paired depth image (lockstep with color) so UE renders
|
||||||
* depth into it; released together in end_frame. Depth only runs in the
|
* depth into it; released together in end_frame. Depth only runs in the
|
||||||
* synchronous path (the pipeline holds images and doesn't manage depth). */
|
* synchronous path (the pipeline holds images and doesn't manage depth).
|
||||||
|
* Same timeout rule as color: an un-waited depth image stays acquired and
|
||||||
|
* its wait is retried next frame. */
|
||||||
L->depthAcquired = 0;
|
L->depthAcquired = 0;
|
||||||
if (L->depthSwapchain != XR_NULL_HANDLE && !g_pipelineActive) {
|
if (L->depthSwapchain != XR_NULL_HANDLE && !g_pipelineActive) {
|
||||||
|
int haveDepth = L->depthWaitPending;
|
||||||
|
if (!haveDepth) {
|
||||||
XrSwapchainImageAcquireInfo dai = { XR_TYPE_SWAPCHAIN_IMAGE_ACQUIRE_INFO };
|
XrSwapchainImageAcquireInfo dai = { XR_TYPE_SWAPCHAIN_IMAGE_ACQUIRE_INFO };
|
||||||
if (xrAcquireSwapchainImage(L->depthSwapchain, &dai, &L->depthAcquiredIndex) == XR_SUCCESS) {
|
haveDepth = (xrAcquireSwapchainImage(L->depthSwapchain, &dai,
|
||||||
|
&L->depthAcquiredIndex) == XR_SUCCESS);
|
||||||
|
}
|
||||||
|
if (haveDepth) {
|
||||||
XrSwapchainImageWaitInfo dwi = { XR_TYPE_SWAPCHAIN_IMAGE_WAIT_INFO };
|
XrSwapchainImageWaitInfo dwi = { XR_TYPE_SWAPCHAIN_IMAGE_WAIT_INFO };
|
||||||
dwi.timeout = 100000000;
|
dwi.timeout = 100000000;
|
||||||
if (xrWaitSwapchainImage(L->depthSwapchain, &dwi) == XR_TIMEOUT_EXPIRED) {
|
if (xrWaitSwapchainImage(L->depthSwapchain, &dwi) == XR_TIMEOUT_EXPIRED) {
|
||||||
XrSwapchainImageReleaseInfo dri = { XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO };
|
static int dtw = 0;
|
||||||
xrReleaseSwapchainImage(L->depthSwapchain, &dri); /* stay balanced */
|
if (dtw++ < 40) XRRLOG("WaitSwapchainImage(depth) TIMEOUT layer=%d idx=%u",
|
||||||
|
i, L->depthAcquiredIndex);
|
||||||
|
L->depthWaitPending = 1; /* keep it acquired; retry next frame */
|
||||||
} else {
|
} else {
|
||||||
|
L->depthWaitPending = 0;
|
||||||
L->depthAcquired = 1;
|
L->depthAcquired = 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -939,6 +967,9 @@ static void build_composition(PendingFrame *pf,
|
|||||||
if (!s || s->LayerId < 0 || s->LayerId >= g_xr.layerCount) continue;
|
if (!s || s->LayerId < 0 || s->LayerId >= g_xr.layerCount) continue;
|
||||||
XrLayer *L = &g_xr.layers[s->LayerId];
|
XrLayer *L = &g_xr.layers[s->LayerId];
|
||||||
if (!L->active || L->swapchain == XR_NULL_HANDLE) continue;
|
if (!L->active || L->swapchain == XR_NULL_HANDLE) continue;
|
||||||
|
/* its image wait timed out in begin_frame: nothing was rendered/released for
|
||||||
|
* it this frame, so skip the layer (retried next frame). */
|
||||||
|
if (L->waitPending) continue;
|
||||||
/* projection only: drop quads (diag=1, or copy-ring mode which is eye-only) */
|
/* projection only: drop quads (diag=1, or copy-ring mode which is eye-only) */
|
||||||
if ((diag == 1 || g_copyRingEngaged) && !L->isEyeFov) continue;
|
if ((diag == 1 || g_copyRingEngaged) && !L->isEyeFov) continue;
|
||||||
|
|
||||||
@@ -999,7 +1030,8 @@ static void build_composition(PendingFrame *pf,
|
|||||||
/* Chain per-eye depth so the compositor can positionally reproject
|
/* Chain per-eye depth so the compositor can positionally reproject
|
||||||
* (artifact B). Reverse-Z/infinite-far is UE's Quest convention;
|
* (artifact B). Reverse-Z/infinite-far is UE's Quest convention;
|
||||||
* params tunable via debug.re4vr.depth_* . Only in normal mode. */
|
* params tunable via debug.re4vr.depth_* . Only in normal mode. */
|
||||||
if (diag == 0 && !g_pipelineActive && L->depthSwapchain != XR_NULL_HANDLE) {
|
if (diag == 0 && !g_pipelineActive && L->depthSwapchain != XR_NULL_HANDLE
|
||||||
|
&& !L->depthWaitPending) {
|
||||||
XrCompositionLayerDepthInfoKHR *d = &pf->pdepth[eye];
|
XrCompositionLayerDepthInfoKHR *d = &pf->pdepth[eye];
|
||||||
*d = (XrCompositionLayerDepthInfoKHR){ XR_TYPE_COMPOSITION_LAYER_DEPTH_INFO_KHR };
|
*d = (XrCompositionLayerDepthInfoKHR){ XR_TYPE_COMPOSITION_LAYER_DEPTH_INFO_KHR };
|
||||||
d->subImage.swapchain = L->depthSwapchain;
|
d->subImage.swapchain = L->depthSwapchain;
|
||||||
@@ -1682,6 +1714,8 @@ void xrr_destroy_layer(int layerId) {
|
|||||||
xrReleaseSwapchainImage(L->swapchain, &ri);
|
xrReleaseSwapchainImage(L->swapchain, &ri);
|
||||||
L->imageAcquired = 0;
|
L->imageAcquired = 0;
|
||||||
}
|
}
|
||||||
|
/* waitPending/depthWaitPending images are acquired but un-waited and can't be
|
||||||
|
* released; destroying the swapchain with them held is valid, so just drop them. */
|
||||||
if (L->swapchain != XR_NULL_HANDLE) xrDestroySwapchain(L->swapchain);
|
if (L->swapchain != XR_NULL_HANDLE) xrDestroySwapchain(L->swapchain);
|
||||||
if (L->depthSwapchain != XR_NULL_HANDLE) xrDestroySwapchain(L->depthSwapchain);
|
if (L->depthSwapchain != XR_NULL_HANDLE) xrDestroySwapchain(L->depthSwapchain);
|
||||||
if (L->shimCount > 0) xrr_vk_free_images(L->shimImages, L->shimMem, L->shimCount);
|
if (L->shimCount > 0) xrr_vk_free_images(L->shimImages, L->shimMem, L->shimCount);
|
||||||
|
|||||||
@@ -28,6 +28,11 @@ typedef struct {
|
|||||||
uint32_t depthAcquiredIndex;
|
uint32_t depthAcquiredIndex;
|
||||||
int depthAcquired; /* paired depth image held this frame */
|
int depthAcquired; /* paired depth image held this frame */
|
||||||
int imageAcquired; /* this frame's render image is held */
|
int imageAcquired; /* this frame's render image is held */
|
||||||
|
/* xrWaitSwapchainImage timed out: the image is ACQUIRED but not yet WAITED. It
|
||||||
|
* cannot be released (spec: CALL_ORDER_INVALID), so it stays acquired and the
|
||||||
|
* next begin_frame retries the wait on it instead of acquiring another. */
|
||||||
|
int waitPending; /* color: acquiredIndex awaiting wait */
|
||||||
|
int depthWaitPending; /* depth: depthAcquiredIndex awaiting wait */
|
||||||
/* render-ahead pipeline: the image rendered LAST frame is held one extra
|
/* render-ahead pipeline: the image rendered LAST frame is held one extra
|
||||||
* frame so its tile-memory flush completes off the critical path. */
|
* frame so its tile-memory flush completes off the critical path. */
|
||||||
int presentPending; /* holding last frame's image to present */
|
int presentPending; /* holding last frame's image to present */
|
||||||
|
|||||||
Reference in new issue
Block a user