st/oxr: Overhaul the acquire, wait, release handling

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.

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 *);
 

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);

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;
 

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);
 	}
 

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);