Skip to content

Commit

Permalink
Fixing issue with swapchains not releasing properly
Browse files Browse the repository at this point in the history
  • Loading branch information
@BastiaanOlij
BastiaanOlij committed Jun 13, 2022
1 parent 2340c01 commit 947b1e5
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 40 deletions.
108 changes: 68 additions & 40 deletions src/openxr/OpenXRApi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,12 @@ bool OpenXRApi::initialiseSwapChains() {
return false;
}

swapchain_acquired = (bool *)malloc(sizeof(bool) * view_count);
if (swapchain_acquired == nullptr) {
Godot::print_error("OpenXR Couldn't allocate memory for swap chains", __FUNCTION__, __FILE__, __LINE__);
return false;
}

// Damn you microsoft for not supporting this!!
// uint32_t swapchainLength[view_count];
uint32_t *swapchainLength = (uint32_t *)malloc(sizeof(uint32_t) * view_count);
Expand All @@ -2024,6 +2030,8 @@ bool OpenXRApi::initialiseSwapChains() {
}

for (uint32_t i = 0; i < view_count; i++) {
swapchain_acquired[i] = false;

// again Microsoft wants these in order!
XrSwapchainCreateInfo swapchainCreateInfo = {
.type = XR_TYPE_SWAPCHAIN_CREATE_INFO,
Expand Down Expand Up @@ -2160,6 +2168,10 @@ bool OpenXRApi::initialiseSwapChains() {
}

void OpenXRApi::cleanupSwapChains() {
if (swapchain_acquired != nullptr) {
free(swapchain_acquired);
swapchain_acquired = nullptr;
}
if (swapchains != NULL) {
for (uint32_t i = 0; i < view_count; i++) {
if (swapchains[i] != XR_NULL_HANDLE) {
Expand Down Expand Up @@ -2940,9 +2952,42 @@ XrResult OpenXRApi::acquire_image(int eye) {
if (!xr_result(result, "failed to wait for swapchain image!")) {
return result;
}

swapchain_acquired[eye] = true;

return XR_SUCCESS;
}

bool OpenXRApi::release_swapchain(int eye) {
if (swapchain_acquired[eye]) {
swapchain_acquired[eye] = false; // mark as false whether we succeed or not...

XrSwapchainImageReleaseInfo swapchainImageReleaseInfo = {
.type = XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO,
.next = nullptr
};
XrResult result = xrReleaseSwapchainImage(swapchains[eye], &swapchainImageReleaseInfo);
return xr_result(result, "failed to release swapchain image!");
} else {
return XR_SUCCESS;
}
}

void OpenXRApi::end_frame(uint32_t p_layer_count, const XrCompositionLayerBaseHeader *const *p_layers) {
// MS wants these in order..
// submit 0 layers when we shouldn't render
XrFrameEndInfo frameEndInfo = {
.type = XR_TYPE_FRAME_END_INFO,
.next = nullptr,
.displayTime = frameState.predictedDisplayTime,
.environmentBlendMode = XR_ENVIRONMENT_BLEND_MODE_OPAQUE,
.layerCount = p_layer_count,
.layers = p_layers,
};
XrResult result = xrEndFrame(session, &frameEndInfo);
xr_result(result, "failed to end frame!"); // just report the error
}

void OpenXRApi::render_openxr(int eye, uint32_t texid, bool has_external_texture_support) {
if (!initialised) {
return;
Expand All @@ -2962,30 +3007,11 @@ void OpenXRApi::render_openxr(int eye, uint32_t texid, bool has_external_texture
* TODO: Tell godot not to call get_external_texture_for_eye() when
* frameState.shouldRender is false, then remove the image release here
*/
if (has_external_texture_support) {
XrSwapchainImageReleaseInfo swapchainImageReleaseInfo = {
.type = XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO,
.next = nullptr
};
result = xrReleaseSwapchainImage(swapchains[eye], &swapchainImageReleaseInfo);
if (!xr_result(result, "failed to release swapchain image!")) {
return;
}
}
release_swapchain(eye); // just report the error and ignore

if (eye == 1) {
// MS wants these in order..
// submit 0 layers when we shouldn't render
XrFrameEndInfo frameEndInfo = {
.type = XR_TYPE_FRAME_END_INFO,
.next = nullptr,
.displayTime = frameState.predictedDisplayTime,
.environmentBlendMode = XR_ENVIRONMENT_BLEND_MODE_OPAQUE,
.layerCount = 0,
.layers = nullptr,
};
result = xrEndFrame(session, &frameEndInfo);
xr_result(result, "failed to end frame!");
// we must always end our frame, even if we don't have an image to submit...
end_frame(0, nullptr);
}

// neither eye is rendered
Expand All @@ -2995,6 +3021,11 @@ void OpenXRApi::render_openxr(int eye, uint32_t texid, bool has_external_texture
if (!has_external_texture_support) {
result = acquire_image(eye);
if (!xr_result(result, "failed to acquire swapchain image!")) {
if (eye == 1) {
// we must always end our frame, even if we don't have an image to submit...
end_frame(0, nullptr);
}

return;
}

Expand All @@ -3017,12 +3048,12 @@ void OpenXRApi::render_openxr(int eye, uint32_t texid, bool has_external_texture
// printf("Godot already rendered into our textures\n");
}

XrSwapchainImageReleaseInfo swapchainImageReleaseInfo = {
.type = XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO,
.next = nullptr
};
result = xrReleaseSwapchainImage(swapchains[eye], &swapchainImageReleaseInfo);
if (!xr_result(result, "failed to release swapchain image!")) {
if (!release_swapchain(eye)) {
if (eye == 1) {
// we must always end our frame, even if we don't have an image to submit...
end_frame(0, nullptr);
}

return;
}

Expand All @@ -3046,18 +3077,7 @@ void OpenXRApi::render_openxr(int eye, uint32_t texid, bool has_external_texture

projectionLayer->layerFlags = layers_list.size() > 1 ? XR_COMPOSITION_LAYER_BLEND_TEXTURE_SOURCE_ALPHA_BIT | XR_COMPOSITION_LAYER_CORRECT_CHROMATIC_ABERRATION_BIT : XR_COMPOSITION_LAYER_CORRECT_CHROMATIC_ABERRATION_BIT;

XrFrameEndInfo frameEndInfo = {
.type = XR_TYPE_FRAME_END_INFO,
.next = nullptr,
.displayTime = frameState.predictedDisplayTime,
.environmentBlendMode = XR_ENVIRONMENT_BLEND_MODE_OPAQUE,
.layerCount = static_cast<uint32_t>(layers_list.size()),
.layers = layers_list.data(),
};
result = xrEndFrame(session, &frameEndInfo);
if (!xr_result(result, "failed to end frame!")) {
return;
}
end_frame(static_cast<uint32_t>(layers_list.size()), layers_list.data());
}

#ifdef WIN32
Expand Down Expand Up @@ -3423,6 +3443,14 @@ int OpenXRApi::get_external_texture_for_eye(int eye, bool *has_support) {
if (!running || state >= XR_SESSION_STATE_STOPPING)
return 0;

if (!frameState.shouldRender) {
// We shouldn't be rendering at all but this prevents acquiring and rendering to our swap chain (Quest doesn't seem to like this)
// instead we render to Godots internal buffers. Also good for desktop as we still get our preview.

// We really should make it possible to return say -1 and have Godot skip rendering this frame. That would need to be a change upstream.
return 0;
}

// this only gets called from Godot 3.2 and newer, allows us to use
// OpenXR swapchain directly.

Expand Down
4 changes: 4 additions & 0 deletions src/openxr/OpenXRApi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ class OpenXRApi {
uint32_t swapchain_sample_count = 1;

XrSwapchain *swapchains = NULL;
bool *swapchain_acquired = NULL;
uint32_t view_count;

XrCompositionLayerProjection *projectionLayer = NULL;
Expand Down Expand Up @@ -270,6 +271,9 @@ class OpenXRApi {
void update_actions();
void transform_from_matrix(godot_transform *p_dest, XrMatrix4x4f *matrix, float p_world_scale);

bool release_swapchain(int eye);
void end_frame(uint32_t p_layer_count, const XrCompositionLayerBaseHeader *const *p_layers);

bool parse_action_sets(const godot::String &p_json);
bool parse_interaction_profiles(const godot::String &p_json);

Expand Down

0 comments on commit 947b1e5

Please sign in to comment.