From c7e22483febd2f21452f8dff8f987c998f9a75f5 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Tue, 20 Feb 2018 20:56:02 +0000 Subject: [PATCH] vulkan/wsi/x11: Consistently update and return swapchain status Use a helper function for updating the swapchain status. This will be used later to handle VK_SUBOPTIMAL_KHR, where we need to make a non-error status stick to the swapchain until recreation. Instead of direct comparisons to VK_SUCCESS to check for error, test for negative numbers meaning an error status, and positive numbers indicating non-error statuses. v2 (Jason Ekstrand): - Use a pattern of "return x11_swapchain_result(chain, VK_WHATEVER)" - Handle wsi_queue_pull returning VK_TIMEOUT - Call x11_swapchain_result in x11_present_to_x11 Signed-off-by: Daniel Stone Reviewed-by: Jason Ekstrand --- src/vulkan/wsi/wsi_common_x11.c | 82 +++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c index 15d291463e9..e5e719e1d50 100644 --- a/src/vulkan/wsi/wsi_common_x11.c +++ b/src/vulkan/wsi/wsi_common_x11.c @@ -641,6 +641,36 @@ struct x11_swapchain { struct x11_image images[0]; }; +/** + * Update the swapchain status with the result of an operation, and return + * the combined status. The chain status will eventually be returned from + * AcquireNextImage and QueuePresent. + * + * We make sure to 'stick' more pessimistic statuses: an out-of-date error + * is permanent once seen, and every subsequent call will return this. If + * this has not been seen, success will be returned. + */ +static VkResult +x11_swapchain_result(struct x11_swapchain *chain, VkResult result) +{ + /* Prioritise returning existing errors for consistency. */ + if (chain->status < 0) + return chain->status; + + /* If we have a new error, mark it as permanent on the chain and return. */ + if (result < 0) { + chain->status = result; + return result; + } + + /* Return temporary errors, but don't persist them. */ + if (result == VK_TIMEOUT || result == VK_NOT_READY) + return result; + + /* No changes, so return the last status. */ + return chain->status; +} + static struct wsi_image * x11_get_wsi_image(struct wsi_swapchain *wsi_chain, uint32_t image_index) { @@ -648,6 +678,9 @@ x11_get_wsi_image(struct wsi_swapchain *wsi_chain, uint32_t image_index) return &chain->images[image_index].base; } +/** + * Process an X11 Present event. Does not update chain->status. + */ static VkResult x11_handle_dri3_present_event(struct x11_swapchain *chain, xcb_present_generic_event_t *event) @@ -726,7 +759,7 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain *chain, xshmfence_await(chain->images[i].shm_fence); *image_index = i; chain->images[i].busy = true; - return VK_SUCCESS; + return x11_swapchain_result(chain, VK_SUCCESS); } } @@ -735,13 +768,13 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain *chain, if (timeout == UINT64_MAX) { event = xcb_wait_for_special_event(chain->conn, chain->special_event); if (!event) - return VK_ERROR_OUT_OF_DATE_KHR; + return x11_swapchain_result(chain, VK_ERROR_OUT_OF_DATE_KHR); } else { event = xcb_poll_for_special_event(chain->conn, chain->special_event); if (!event) { int ret; if (timeout == 0) - return VK_NOT_READY; + return x11_swapchain_result(chain, VK_NOT_READY); atimeout = wsi_get_absolute_timeout(timeout); @@ -749,9 +782,9 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain *chain, pfds.events = POLLIN; ret = poll(&pfds, 1, timeout / 1000 / 1000); if (ret == 0) - return VK_TIMEOUT; + return x11_swapchain_result(chain, VK_TIMEOUT); if (ret == -1) - return VK_ERROR_OUT_OF_DATE_KHR; + return x11_swapchain_result(chain, VK_ERROR_OUT_OF_DATE_KHR); /* If a non-special event happens, the fd will still * poll. So recalculate the timeout now just in case. @@ -765,10 +798,13 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain *chain, } } + /* Update the swapchain status here. We may catch non-fatal errors here, + * in which case we need to update the status and continue. + */ VkResult result = x11_handle_dri3_present_event(chain, (void *)event); free(event); - if (result != VK_SUCCESS) - return result; + if (result < 0) + return x11_swapchain_result(chain, result); } } @@ -781,9 +817,13 @@ x11_acquire_next_image_from_queue(struct x11_swapchain *chain, uint32_t image_index; VkResult result = wsi_queue_pull(&chain->acquire_queue, &image_index, timeout); - if (result != VK_SUCCESS) { - return result; - } else if (chain->status != VK_SUCCESS) { + if (result < 0 || result == VK_TIMEOUT) { + /* On error, the thread has shut down, so safe to update chain->status. + * Calling x11_swapchain_result with VK_TIMEOUT won't modify + * chain->status so that is also safe. + */ + return x11_swapchain_result(chain, result); + } else if (chain->status < 0) { return chain->status; } @@ -792,7 +832,7 @@ x11_acquire_next_image_from_queue(struct x11_swapchain *chain, *image_index_out = image_index; - return VK_SUCCESS; + return chain->status; } static VkResult @@ -835,7 +875,7 @@ x11_present_to_x11(struct x11_swapchain *chain, uint32_t image_index, xcb_flush(chain->conn); - return VK_SUCCESS; + return x11_swapchain_result(chain, VK_SUCCESS); } static VkResult @@ -876,7 +916,7 @@ x11_manage_fifo_queues(void *state) assert(chain->base.present_mode == VK_PRESENT_MODE_FIFO_KHR); - while (chain->status == VK_SUCCESS) { + while (chain->status >= 0) { /* It should be safe to unconditionally block here. Later in the loop * we blocks until the previous present has landed on-screen. At that * point, we should have received IDLE_NOTIFY on all images presented @@ -885,15 +925,19 @@ x11_manage_fifo_queues(void *state) */ uint32_t image_index; result = wsi_queue_pull(&chain->present_queue, &image_index, INT64_MAX); - if (result != VK_SUCCESS) { + assert(result != VK_TIMEOUT); + if (result < 0) { goto fail; - } else if (chain->status != VK_SUCCESS) { + } else if (chain->status < 0) { + /* The status can change underneath us if the swapchain is destroyed + * from another thread. + */ return NULL; } uint64_t target_msc = chain->last_present_msc + 1; result = x11_present_to_x11(chain, image_index, target_msc); - if (result != VK_SUCCESS) + if (result < 0) goto fail; while (chain->last_present_msc < target_msc) { @@ -906,13 +950,13 @@ x11_manage_fifo_queues(void *state) result = x11_handle_dri3_present_event(chain, (void *)event); free(event); - if (result != VK_SUCCESS) + if (result < 0) goto fail; } } fail: - chain->status = result; + result = x11_swapchain_result(chain, result); wsi_queue_push(&chain->acquire_queue, UINT32_MAX); return NULL; @@ -934,7 +978,7 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain, result = wsi_create_native_image(&chain->base, pCreateInfo, 0, NULL, NULL, &image->base); } - if (result != VK_SUCCESS) + if (result < 0) return result; image->pixmap = xcb_generate_id(chain->conn); -- 2.30.2