diff --git a/doc/changes/state_trackers/mr.359.5.md b/doc/changes/state_trackers/mr.359.5.md new file mode 100644 index 000000000..a1a5ddf2a --- /dev/null +++ b/doc/changes/state_trackers/mr.359.5.md @@ -0,0 +1,4 @@ +OpenXR: A major overhaul of the swapchain acquire, wait and release code. This +makes it almost completely conformant with the spec. Tricky parts include that +multiple images can be acquired, but only one can be waited on before being +released. diff --git a/src/xrt/state_trackers/oxr/oxr_objects.h b/src/xrt/state_trackers/oxr/oxr_objects.h index e0953a483..47e86d578 100644 --- a/src/xrt/state_trackers/oxr/oxr_objects.h +++ b/src/xrt/state_trackers/oxr/oxr_objects.h @@ -17,6 +17,7 @@ #include "os/os_threading.h" +#include "util/u_index_fifo.h" #include "util/u_hashset.h" #include "util/u_hashmap.h" @@ -112,6 +113,7 @@ struct oxr_binding; struct oxr_interaction_profile; #define XRT_MAX_HANDLE_CHILDREN 256 +#define OXR_MAX_SWAPCHAIN_IMAGES 8 struct time_state; @@ -146,6 +148,16 @@ enum oxr_sub_action_path OXR_SUB_ACTION_PATH_GAMEPAD, }; +/*! + * Tracks the state of a image that belongs to a @ref oxr_swapchain. + */ +enum oxr_image_state +{ + OXR_IMAGE_STATE_READY, + OXR_IMAGE_STATE_ACQUIRED, + OXR_IMAGE_STATE_WAITED, +}; + /* * @@ -1297,9 +1309,30 @@ struct oxr_swapchain //! Compositor swapchain. struct xrt_swapchain *swapchain; - //! Actual state tracked! :D - int acquired_index; - int released_index; + struct + { + enum oxr_image_state state; + } images[OXR_MAX_SWAPCHAIN_IMAGES]; + + struct + { + size_t num; + struct u_index_fifo fifo; + } acquired; + + struct + { + bool yes; + int index; + } waited; + + struct + { + bool yes; + int index; + } released; + + XrResult (*destroy)(struct oxr_logger *, struct oxr_swapchain *); diff --git a/src/xrt/state_trackers/oxr/oxr_session.c b/src/xrt/state_trackers/oxr/oxr_session.c index a1d5b7c25..a892e34a8 100644 --- a/src/xrt/state_trackers/oxr/oxr_session.c +++ b/src/xrt/state_trackers/oxr/oxr_session.c @@ -518,14 +518,14 @@ verify_quad_layer(struct xrt_compositor *xc, } #endif - if (sc->released_index == -1) { + if (!sc->released.yes) { return oxr_error(log, XR_ERROR_LAYER_INVALID, "(frameEndInfo->layers[%u]->subImage." "swapchain) swapchain has not been released!", layer_index); } - if (sc->released_index >= (int)sc->swapchain->num_images) { + if (sc->released.index >= (int)sc->swapchain->num_images) { return oxr_error( log, XR_ERROR_RUNTIME_FAILURE, "(frameEndInfo->layers[%u]->subImage.swapchain) internal " @@ -597,7 +597,7 @@ verify_projection_layer(struct xrt_compositor *xc, struct oxr_swapchain *sc = XRT_CAST_OXR_HANDLE_TO_PTR( struct oxr_swapchain *, proj->views[i].subImage.swapchain); - if (sc->released_index == -1) { + if (!sc->released.yes) { return oxr_error( log, XR_ERROR_LAYER_INVALID, "(frameEndInfo->layers[%u]->views[%i].subImage." @@ -605,7 +605,7 @@ verify_projection_layer(struct xrt_compositor *xc, layer_index, i); } - if (sc->released_index >= (int)sc->swapchain->num_images) { + if (sc->released.index >= (int)sc->swapchain->num_images) { return oxr_error( log, XR_ERROR_RUNTIME_FAILURE, "(frameEndInfo->layers[%u]->views[%i].subImage." @@ -634,7 +634,7 @@ submit_quad_layer(struct xrt_compositor *xc, xrt_comp_layer_quad( xc, timestamp, head, XRT_INPUT_GENERIC_HEAD_POSE, quad->layerFlags, (enum xrt_layer_eye_visibility)quad->eyeVisibility, sc->swapchain, - sc->released_index, (struct xrt_rect *)&quad->subImage.imageRect, + sc->released.index, (struct xrt_rect *)&quad->subImage.imageRect, quad->subImage.imageArrayIndex, &pose, (struct xrt_vec2 *)&quad->size, false); } @@ -666,12 +666,12 @@ submit_projection_layer(struct xrt_compositor *xc, xrt_comp_layer_stereo_projection( xc, timestamp, head, XRT_INPUT_GENERIC_HEAD_POSE, flags, scs[0]->swapchain, // Left - scs[0]->released_index, + scs[0]->released.index, (struct xrt_rect *)&proj->views[0].subImage.imageRect, proj->views[0].subImage.imageArrayIndex, (struct xrt_fov *)&proj->views[0].fov, &pose[0], scs[1]->swapchain, // Right - scs[1]->released_index, + scs[1]->released.index, (struct xrt_rect *)&proj->views[1].subImage.imageRect, proj->views[1].subImage.imageArrayIndex, (struct xrt_fov *)&proj->views[1].fov, &pose[1], false); diff --git a/src/xrt/state_trackers/oxr/oxr_swapchain.c b/src/xrt/state_trackers/oxr/oxr_swapchain.c index 5648ef751..2acf4e6c3 100644 --- a/src/xrt/state_trackers/oxr/oxr_swapchain.c +++ b/src/xrt/state_trackers/oxr/oxr_swapchain.c @@ -26,9 +26,9 @@ oxr_swapchain_acquire_image(struct oxr_logger *log, uint32_t *out_index) { uint32_t index; - if (sc->acquired_index >= 0) { + if (sc->acquired.num >= sc->swapchain->num_images) { return oxr_error(log, XR_ERROR_CALL_ORDER_INVALID, - " image already acquired"); + " all images have been acquired"); } struct xrt_swapchain *xsc = (struct xrt_swapchain *)sc->swapchain; @@ -37,12 +37,21 @@ oxr_swapchain_acquire_image(struct oxr_logger *log, " call to xsc->acquire_image failed"); } - sc->acquired_index = (int)index; + if (sc->images[index].state != OXR_IMAGE_STATE_READY) { + return oxr_error( + log, XR_ERROR_RUNTIME_FAILURE, + "Internal acquire call returned non-ready image."); + } + + sc->acquired.num++; + u_index_fifo_push(&sc->acquired.fifo, index); + sc->images[index].state = OXR_IMAGE_STATE_ACQUIRED; // If the compositor is resuing the image, // mark it as invalid to use in xrEndFrame. - if (sc->released_index == (int)index) { - sc->released_index = -1; + if (sc->released.index == (int)index) { + sc->released.yes = false; + sc->released.index = -1; } *out_index = index; @@ -55,18 +64,31 @@ oxr_swapchain_wait_image(struct oxr_logger *log, struct oxr_swapchain *sc, const XrSwapchainImageWaitInfo *waitInfo) { - if (sc->acquired_index < 0) { + if (sc->waited.yes) { + return oxr_error( + log, XR_ERROR_CALL_ORDER_INVALID, + " swapchain has already been waited, call release"); + } + + if (u_index_fifo_is_empty(&sc->acquired.fifo)) { return oxr_error(log, XR_ERROR_CALL_ORDER_INVALID, " no image acquired"); } + uint32_t index; + u_index_fifo_pop(&sc->acquired.fifo, &index); + struct xrt_swapchain *xsc = (struct xrt_swapchain *)sc->swapchain; - if (!xsc->wait_image(xsc, waitInfo->timeout, - (uint32_t)sc->acquired_index)) { + if (!xsc->wait_image(xsc, waitInfo->timeout, index)) { return oxr_error(log, XR_ERROR_RUNTIME_FAILURE, " call to xsc->wait_image failed"); } + // The app can only wait on one image. + sc->waited.yes = true; + sc->waited.index = index; + sc->images[index].state = OXR_IMAGE_STATE_WAITED; + return oxr_session_success_result(sc->sess); } @@ -75,20 +97,27 @@ oxr_swapchain_release_image(struct oxr_logger *log, struct oxr_swapchain *sc, const XrSwapchainImageReleaseInfo *releaseInfo) { - if (sc->acquired_index < 0) { + if (!sc->waited.yes) { return oxr_error(log, XR_ERROR_CALL_ORDER_INVALID, - " no image acquired"); + " no swapchain images waited on"); } + sc->waited.yes = false; + uint32_t index = sc->waited.index; + struct xrt_swapchain *xsc = (struct xrt_swapchain *)sc->swapchain; - if (!xsc->release_image(xsc, (uint32_t)sc->acquired_index)) { + if (!xsc->release_image(xsc, index)) { return oxr_error(log, XR_ERROR_RUNTIME_FAILURE, " call to xsc->release_image failed"); } + // Only decerement here. + sc->acquired.num--; + // Overwrite the old released image, with new. - sc->released_index = sc->acquired_index; - sc->acquired_index = -1; + sc->released.yes = true; + sc->released.index = index; + sc->images[index].state = OXR_IMAGE_STATE_READY; return oxr_session_success_result(sc->sess); } @@ -169,8 +198,6 @@ oxr_create_swapchain(struct oxr_logger *log, sc->acquire_image = oxr_swapchain_acquire_image; sc->wait_image = oxr_swapchain_wait_image; sc->release_image = oxr_swapchain_release_image; - sc->acquired_index = -1; - sc->released_index = -1; *out_swapchain = sc; diff --git a/src/xrt/state_trackers/oxr/oxr_swapchain_gl.c b/src/xrt/state_trackers/oxr/oxr_swapchain_gl.c index 59f731723..8a97f52dc 100644 --- a/src/xrt/state_trackers/oxr/oxr_swapchain_gl.c +++ b/src/xrt/state_trackers/oxr/oxr_swapchain_gl.c @@ -23,7 +23,15 @@ static XrResult oxr_swapchain_gl_destroy(struct oxr_logger *log, struct oxr_swapchain *sc) { - if (sc->acquired_index >= 0) { + // Release any waited image. + if (sc->waited.yes) { + sc->release_image(log, sc, NULL); + } + + // Release any acquired images. + XrSwapchainImageWaitInfo waitInfo = {0}; + while (!u_index_fifo_is_empty(&sc->acquired.fifo)) { + sc->wait_image(log, sc, &waitInfo); sc->release_image(log, sc, NULL); } diff --git a/src/xrt/state_trackers/oxr/oxr_swapchain_vk.c b/src/xrt/state_trackers/oxr/oxr_swapchain_vk.c index 55daae670..c56f0558a 100644 --- a/src/xrt/state_trackers/oxr/oxr_swapchain_vk.c +++ b/src/xrt/state_trackers/oxr/oxr_swapchain_vk.c @@ -20,11 +20,17 @@ static XrResult oxr_swapchain_vk_destroy(struct oxr_logger *log, struct oxr_swapchain *sc) { - if (sc->acquired_index >= 0) { + // Release any waited image. + if (sc->waited.yes) { sc->release_image(log, sc, NULL); } - sc->acquired_index = 0; + // Release any acquired images. + XrSwapchainImageWaitInfo waitInfo = {0}; + while (!u_index_fifo_is_empty(&sc->acquired.fifo)) { + sc->wait_image(log, sc, &waitInfo); + sc->release_image(log, sc, NULL); + } if (sc->swapchain != NULL) { sc->swapchain->destroy(sc->swapchain);