diff --git a/doc/changes/state_trackers/mr.359.11.md b/doc/changes/state_trackers/mr.359.11.md new file mode 100644 index 000000000..7b6c681fe --- /dev/null +++ b/doc/changes/state_trackers/mr.359.11.md @@ -0,0 +1,3 @@ +OpenXR: Validate the subImage data for both projection and quad layers layers, +refactor code out so it can be shared with the different types of layers. Need +to track some state on the `oxr_swapchain` in order to do the checking. diff --git a/src/xrt/state_trackers/oxr/oxr_objects.h b/src/xrt/state_trackers/oxr/oxr_objects.h index 4d0160fb8..446b93835 100644 --- a/src/xrt/state_trackers/oxr/oxr_objects.h +++ b/src/xrt/state_trackers/oxr/oxr_objects.h @@ -1315,6 +1315,14 @@ struct oxr_swapchain //! Compositor swapchain. struct xrt_swapchain *swapchain; + struct + { + //! Swapchain size. + uint32_t width, height; + //! For 1 is 2D texture, greater then 1 2D array texture. + uint32_t num_array_layers; + }; + struct { enum oxr_image_state state; diff --git a/src/xrt/state_trackers/oxr/oxr_session.c b/src/xrt/state_trackers/oxr/oxr_session.c index b8af489c3..f81c237b3 100644 --- a/src/xrt/state_trackers/oxr/oxr_session.c +++ b/src/xrt/state_trackers/oxr/oxr_session.c @@ -501,6 +501,31 @@ verify_space(struct oxr_logger *log, uint32_t layer_index, XrSpace space) return XR_SUCCESS; } +static XrResult +is_rect_neg(const XrRect2Di *imageRect) +{ + if (imageRect->offset.x < 0 || imageRect->offset.y < 0) { + return true; + } + + return false; +} + +static XrResult +is_rect_out_of_bounds(const XrRect2Di *imageRect, struct oxr_swapchain *sc) +{ + uint32_t total_width = imageRect->offset.x + imageRect->extent.width; + if (total_width > sc->width) { + return true; + } + uint32_t total_height = imageRect->offset.y + imageRect->extent.height; + if (total_height > sc->height) { + return true; + } + + return false; +} + static XrResult verify_quad_layer(struct xrt_compositor *xc, struct oxr_logger *log, @@ -540,14 +565,15 @@ verify_quad_layer(struct xrt_compositor *xc, layer_index, p->x, p->y, p->z); } -#if 0 - if (quad->subImage.imageArrayIndex > 0 && - sc->swapchain->array_size <= quad->subImage.imageArrayIndex) { - return oxr_error(log, XR_ERROR_VALIDATION_FAILURE, - "Invalid swapchain array index for layer %u.", - layer_index); + if (sc->num_array_layers <= quad->subImage.imageArrayIndex) { + return oxr_error( + log, XR_ERROR_VALIDATION_FAILURE, + "(frameEndInfo->layers[%u]->subImage.imageArrayIndex == " + "%u) Invalid swapchain array " + "index for quad layer (%u).", + layer_index, quad->subImage.imageArrayIndex, + sc->num_array_layers); } -#endif if (!sc->released.yes) { return oxr_error(log, XR_ERROR_LAYER_INVALID, @@ -564,18 +590,25 @@ verify_quad_layer(struct xrt_compositor *xc, layer_index); } - if (quad->subImage.imageRect.offset.x < 0 || - quad->subImage.imageRect.offset.y < 0) { - return oxr_error(log, XR_ERROR_SWAPCHAIN_RECT_INVALID, - "imageRect offset is negative for layer %u.", - layer_index); + if (is_rect_neg(&quad->subImage.imageRect)) { + return oxr_error( + log, XR_ERROR_SWAPCHAIN_RECT_INVALID, + "(frameEndInfo->layers[%u]->subImage.imageRect.offset == " + "{%i, %i}) has negative component(s)", + layer_index, quad->subImage.imageRect.offset.x, + quad->subImage.imageRect.offset.y); } - if (quad->subImage.imageRect.offset.x >= 1 || - quad->subImage.imageRect.offset.y >= 1) { - return oxr_error(log, XR_ERROR_SWAPCHAIN_RECT_INVALID, - "imageRect offset out of bounds for layer %u.", - layer_index); + if (is_rect_out_of_bounds(&quad->subImage.imageRect, sc)) { + return oxr_error( + log, XR_ERROR_SWAPCHAIN_RECT_INVALID, + "(frameEndInfo->layers[%u]->subImage.imageRect == {{%i, " + "%i}, {%u, %u}}) imageRect out of image bounds (%u, %u)", + layer_index, quad->subImage.imageRect.offset.x, + quad->subImage.imageRect.offset.y, + quad->subImage.imageRect.extent.width, + quad->subImage.imageRect.extent.height, sc->width, + sc->height); } return XR_SUCCESS; @@ -603,11 +636,12 @@ verify_projection_layer(struct xrt_compositor *xc, // Check for valid swapchain states. for (uint32_t i = 0; i < proj->viewCount; i++) { + const XrCompositionLayerProjectionView *view = &proj->views[i]; + //! @todo More validation? if (!math_quat_validate( - (struct xrt_quat *)&proj->views[i].pose.orientation)) { - const XrQuaternionf *q = - &proj->views[i].pose.orientation; + (struct xrt_quat *)&view->pose.orientation)) { + const XrQuaternionf *q = &view->pose.orientation; return oxr_error( log, XR_ERROR_POSE_INVALID, "(frameEndInfo->layers[%u]->views[%i]->pose." @@ -616,8 +650,8 @@ verify_projection_layer(struct xrt_compositor *xc, } if (!math_vec3_validate( - (struct xrt_vec3 *)&proj->views[i].pose.position)) { - const XrVector3f *p = &proj->views[i].pose.position; + (struct xrt_vec3 *)&view->pose.position)) { + const XrVector3f *p = &view->pose.position; return oxr_error( log, XR_ERROR_POSE_INVALID, "(frameEndInfo->layers[%u]->views[%i]->pose." @@ -626,7 +660,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); + struct oxr_swapchain *, view->subImage.swapchain); if (!sc->released.yes) { return oxr_error( @@ -643,6 +677,39 @@ verify_projection_layer(struct xrt_compositor *xc, "swapchain) internal image index out of bounds", layer_index, i); } + + if (sc->num_array_layers <= view->subImage.imageArrayIndex) { + return oxr_error( + log, XR_ERROR_VALIDATION_FAILURE, + "(frameEndInfo->layers[%u]->views[%i]->subImage." + "imageArrayIndex == %u) Invalid swapchain array " + "index for projection layer (%u).", + layer_index, i, view->subImage.imageArrayIndex, + sc->num_array_layers); + } + + if (is_rect_neg(&view->subImage.imageRect)) { + return oxr_error(log, XR_ERROR_SWAPCHAIN_RECT_INVALID, + "(frameEndInfo->layers[%u]->views[%i]-" + ">subImage.imageRect.offset == {%i, " + "%i}) has negative component(s)", + layer_index, i, + view->subImage.imageRect.offset.x, + view->subImage.imageRect.offset.y); + } + + if (is_rect_out_of_bounds(&view->subImage.imageRect, sc)) { + return oxr_error( + log, XR_ERROR_SWAPCHAIN_RECT_INVALID, + "(frameEndInfo->layers[%u]->views[%i]->subImage." + "imageRect == {{%i, %i}, {%u, %u}}) imageRect out " + "of image bounds (%u, %u)", + layer_index, i, view->subImage.imageRect.offset.x, + view->subImage.imageRect.offset.y, + view->subImage.imageRect.extent.width, + view->subImage.imageRect.extent.height, sc->width, + sc->height); + } } return XR_SUCCESS; diff --git a/src/xrt/state_trackers/oxr/oxr_swapchain.c b/src/xrt/state_trackers/oxr/oxr_swapchain.c index 5b482c0d1..7277f0634 100644 --- a/src/xrt/state_trackers/oxr/oxr_swapchain.c +++ b/src/xrt/state_trackers/oxr/oxr_swapchain.c @@ -200,6 +200,9 @@ oxr_create_swapchain(struct oxr_logger *log, oxr_swapchain_destroy, &sess->handle); sc->sess = sess; sc->swapchain = xsc; + sc->width = createInfo->width; + sc->height = createInfo->height; + sc->num_array_layers = createInfo->arraySize; sc->acquire_image = oxr_swapchain_acquire_image; sc->wait_image = oxr_swapchain_wait_image; sc->release_image = oxr_swapchain_release_image;