From 0ffa7103baaac24f227c243cb078281b23bb3411 Mon Sep 17 00:00:00 2001 From: Beyley Cardellio Date: Tue, 1 Jul 2025 17:52:03 -0700 Subject: [PATCH] c/main: Wait for presentation using KHR_present_wait Part-of: --- src/xrt/compositor/main/comp_renderer.c | 83 ++++++++++++------- src/xrt/compositor/main/comp_target.h | 29 ++++++- .../compositor/main/comp_target_swapchain.c | 62 ++++++++++++-- .../compositor/main/comp_window_debug_image.c | 9 ++ 4 files changed, 145 insertions(+), 38 deletions(-) diff --git a/src/xrt/compositor/main/comp_renderer.c b/src/xrt/compositor/main/comp_renderer.c index 06d1cb72a..c28e4b05f 100644 --- a/src/xrt/compositor/main/comp_renderer.c +++ b/src/xrt/compositor/main/comp_renderer.c @@ -830,6 +830,59 @@ renderer_present_swapchain_image(struct comp_renderer *r, uint64_t desired_prese } } +static void +renderer_wait_for_present(struct comp_renderer *r, uint64_t desired_present_time_ns) +{ + struct comp_compositor *c = r->c; + + if (!comp_target_check_ready(c->target)) { + return; + } + + // For estimating frame misses. + uint64_t before_ns = os_monotonic_get_ns(); + + if (c->target->wait_for_present_supported) { + // reasonable timeout + time_duration_ns timeout_ns = c->frame_interval_ns * 2.5f; + + // @note we don't actually care about the return value, just swallow errors, anything *critical* that + // may be returned will be handled quite soon by later calls + VkResult result = comp_target_wait_for_present(c->target, timeout_ns); + (void)result; + + assert(result != VK_ERROR_EXTENSION_NOT_PRESENT); + } else { + /* + * For direct mode this makes us wait until the last frame has been + * actually shown to the user, this avoids us missing that we have + * missed a frame and miss-predicting the next frame. + * + * Not all drivers follow this behaviour, so KHR_present_wait + * should be preferred in all circumstances. + * + * Only do this if we are ready. + */ + + // Do the acquire + renderer_acquire_swapchain_image(r); + } + + // How long did it take? + uint64_t after_ns = os_monotonic_get_ns(); + + /* + * Make sure we at least waited 1ms before warning. Then check + * if we are more then 1ms behind when we wanted to present. + */ + if (before_ns + U_TIME_1MS_IN_NS < after_ns && // + desired_present_time_ns + U_TIME_1MS_IN_NS < after_ns) { + uint64_t diff_ns = after_ns - desired_present_time_ns; + double diff_ms_f = time_ns_to_ms_f(diff_ns); + COMP_WARN(c, "Compositor probably missed frame by %.2fms", diff_ms_f); + } +} + static void renderer_fini(struct comp_renderer *r) { @@ -1273,35 +1326,7 @@ comp_renderer_draw(struct comp_renderer *r) render_gfx_fini(&render_g); } - - /* - * For direct mode this makes us wait until the last frame has been - * actually shown to the user, this avoids us missing that we have - * missed a frame and miss-predicting the next frame. - * - * Only do this if we are ready. - */ - if (comp_target_check_ready(r->c->target)) { - // For estimating frame misses. - uint64_t then_ns = os_monotonic_get_ns(); - - // Do the acquire - renderer_acquire_swapchain_image(r); - - // How long did it take? - uint64_t now_ns = os_monotonic_get_ns(); - - /* - * Make sure we at least waited 1ms before warning. Then check - * if we are more then 1ms behind when we wanted to present. - */ - if (then_ns + U_TIME_1MS_IN_NS < now_ns && // - desired_present_time_ns + U_TIME_1MS_IN_NS < now_ns) { - uint64_t diff_ns = now_ns - desired_present_time_ns; - double diff_ms_f = time_ns_to_ms_f(diff_ns); - COMP_WARN(c, "Compositor probably missed frame by %.2fms", diff_ms_f); - } - } + renderer_wait_for_present(r, desired_present_time_ns); comp_target_update_timings(ct); diff --git a/src/xrt/compositor/main/comp_target.h b/src/xrt/compositor/main/comp_target.h index 143d41639..2122f46e7 100644 --- a/src/xrt/compositor/main/comp_target.h +++ b/src/xrt/compositor/main/comp_target.h @@ -154,9 +154,12 @@ struct comp_target //! Transformation of the current surface, required for pre-rotation VkSurfaceTransformFlagBitsKHR surface_transform; - // Holds semaphore information. + //! Holds semaphore information. struct comp_target_semaphores semaphores; + //! Whether wait_for_present is supported by this comp_target. + bool wait_for_present_supported; + /* * * Vulkan functions. @@ -228,6 +231,14 @@ struct comp_target int64_t desired_present_time_ns, int64_t present_slop_ns); + /*! + * Wait for the latest presented image to be displayed to the user. + * + * @param ct self + * @param timeout_ns The amount of time to wait for presentation to succeed. + */ + VkResult (*wait_for_present)(struct comp_target *ct, time_duration_ns timeout_ns); + /*! * Flush any WSI state before rendering. */ @@ -441,6 +452,22 @@ comp_target_present(struct comp_target *ct, present_slop_ns); // } +/*! + * @copydoc comp_target::wait_for_present + * + * @public @memberof comp_target + * @ingroup comp_main + */ +static inline VkResult +comp_target_wait_for_present(struct comp_target *ct, time_duration_ns timeout) +{ + COMP_TRACE_MARKER(); + + return ct->wait_for_present( // + ct, // + timeout); // +} + /*! * @copydoc comp_target::flush * diff --git a/src/xrt/compositor/main/comp_target_swapchain.c b/src/xrt/compositor/main/comp_target_swapchain.c index 08e602c8b..8ee4470fa 100644 --- a/src/xrt/compositor/main/comp_target_swapchain.c +++ b/src/xrt/compositor/main/comp_target_swapchain.c @@ -46,6 +46,7 @@ * to 3 anyways so we get what we want. */ DEBUG_GET_ONCE_NUM_OPTION(preferred_at_least_image_count, "XRT_COMPOSITOR_PREFERRED_IMAGE_COUNT", 2) +DEBUG_GET_ONCE_BOOL_OPTION(use_present_wait, "XRT_COMPOSITOR_USE_PRESENT_WAIT", false) static inline struct vk_bundle * get_vk(struct comp_target_swapchain *cts) @@ -649,6 +650,9 @@ comp_target_swapchain_create_images(struct comp_target *ct, const struct comp_ta u_pc_fake_create(ct->c->frame_interval_ns, now_ns, &cts->upc); } + // if we have the present wait extension, mark it as supported now + ct->wait_for_present_supported = vk->has_KHR_present_wait && debug_get_bool_option_use_present_wait(); + // Free old image views. destroy_image_views(cts); @@ -876,6 +880,17 @@ comp_target_swapchain_present(struct comp_target *ct, assert(cts->current_frame_id >= 0); assert(cts->current_frame_id <= UINT32_MAX); + VkPresentInfoKHR present_info = { + .sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR, + .pNext = NULL, + .waitSemaphoreCount = 1, + .pWaitSemaphores = &cts->base.semaphores.render_complete, + .swapchainCount = 1, + .pSwapchains = &cts->swapchain.handle, + .pImageIndices = &index, + }; + +#ifdef VK_GOOGLE_display_timing VkPresentTimeGOOGLE times = { .presentID = (uint32_t)cts->current_frame_id, .desiredPresentTime = desired_present_time_ns - present_slop_ns, @@ -887,20 +902,30 @@ comp_target_swapchain_present(struct comp_target *ct, .pTimes = ×, }; - VkPresentInfoKHR presentInfo = { - .sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR, - .pNext = vk->has_GOOGLE_display_timing ? &timings : NULL, - .waitSemaphoreCount = 1, - .pWaitSemaphores = &cts->base.semaphores.render_complete, + if (vk->has_GOOGLE_display_timing) { + vk_append_to_pnext_chain((VkBaseInStructure *)&present_info, (VkBaseInStructure *)&timings); + } +#endif + +#ifdef VK_KHR_present_id + // @note first present should be 1, not 0, as the swapchain starts at ID 0 and increments from that + uint64_t present_id = (uint64_t)cts->current_frame_id + 1; + + VkPresentIdKHR vk_present_id = { + .sType = VK_STRUCTURE_TYPE_PRESENT_ID_KHR, .swapchainCount = 1, - .pSwapchains = &cts->swapchain.handle, - .pImageIndices = &index, + .pPresentIds = &present_id, }; + if (vk->features.present_wait) { + vk_append_to_pnext_chain((VkBaseInStructure *)&present_info, (VkBaseInStructure *)&vk_present_id); + } +#endif + // Need to take the queue lock for present. os_mutex_lock(&vk->queue_mutex); - VkResult ret = vk->vkQueuePresentKHR(queue, &presentInfo); + VkResult ret = vk->vkQueuePresentKHR(queue, &present_info); os_mutex_unlock(&vk->queue_mutex); @@ -918,6 +943,25 @@ comp_target_swapchain_present(struct comp_target *ct, return ret; } +static VkResult +comp_target_swapchain_wait_for_present(struct comp_target *ct, time_duration_ns timeout_ns) +{ + struct comp_target_swapchain *cts = (struct comp_target_swapchain *)ct; + struct vk_bundle *vk = get_vk(cts); + +#ifdef VK_KHR_present_wait + if (!vk->features.present_wait) { + return VK_ERROR_EXTENSION_NOT_PRESENT; + } + + // @note current frame ID is incremented by 1 to match the ID given to Vulkan, see comp_target_swapchain_present + return vk->vkWaitForPresentKHR(vk->device, cts->swapchain.handle, (uint64_t)cts->current_frame_id + 1, + timeout_ns); +#else + return VK_ERROR_EXTENSION_NOT_PRESENT; +#endif +} + static bool comp_target_swapchain_check_ready(struct comp_target *ct) { @@ -1091,9 +1135,11 @@ comp_target_swapchain_init_and_set_fnptrs(struct comp_target_swapchain *cts, cts->base.has_images = comp_target_swapchain_has_images; cts->base.acquire = comp_target_swapchain_acquire_next_image; cts->base.present = comp_target_swapchain_present; + cts->base.wait_for_present = comp_target_swapchain_wait_for_present; cts->base.calc_frame_pacing = comp_target_swapchain_calc_frame_pacing; cts->base.mark_timing_point = comp_target_swapchain_mark_timing_point; cts->base.update_timings = comp_target_swapchain_update_timings; cts->base.info_gpu = comp_target_swapchain_info_gpu; + os_thread_helper_init(&cts->vblank.event_thread); } diff --git a/src/xrt/compositor/main/comp_window_debug_image.c b/src/xrt/compositor/main/comp_window_debug_image.c index 4f5ac32f0..893e9d582 100644 --- a/src/xrt/compositor/main/comp_window_debug_image.c +++ b/src/xrt/compositor/main/comp_window_debug_image.c @@ -212,6 +212,12 @@ target_present(struct comp_target *ct, return VK_SUCCESS; } +static VkResult +target_wait_for_present(struct comp_target *ct, time_duration_ns timeout_ns) +{ + return VK_ERROR_EXTENSION_NOT_PRESENT; +} + static void target_flush(struct comp_target *ct) { @@ -340,6 +346,7 @@ target_create(struct comp_compositor *c) dit->base.has_images = target_has_images; dit->base.acquire = target_acquire; dit->base.present = target_present; + dit->base.wait_for_present = target_wait_for_present; dit->base.flush = target_flush; dit->base.calc_frame_pacing = target_calc_frame_pacing; dit->base.mark_timing_point = target_mark_timing_point; @@ -349,6 +356,8 @@ target_create(struct comp_compositor *c) dit->base.destroy = target_destroy; dit->base.c = c; + dit->base.wait_for_present_supported = false; + // Create the pacer. uint64_t now_ns = os_monotonic_get_ns(); u_pc_fake_create(c->settings.nominal_frame_interval_ns, now_ns, &dit->upc); -- 2.51.2