On Thu, Feb 15, 2018 at 7:57 AM, Daniel Stone <dani...@collabora.com> wrote:

> From: Louis-Francis Ratté-Boulianne <l...@collabora.com>
>
> When it is detected that a window could have been flipped
> but has been copied because of suboptimal format/modifier.
> The Vulkan client should then re-create the swapchain.
>
> Signed-off-by: Louis-Francis Ratté-Boulianne <l...@collabora.com>
> Reviewed-by: Daniel Stone <dani...@collabora.com>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
>  src/vulkan/wsi/wsi_common_x11.c | 64 ++++++++++++++++++++++++++++++
> +++++++----
>  1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_
> x11.c
> index c569aa17187..a9929af338c 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -130,6 +130,8 @@ wsi_x11_connection_create(const VkAllocationCallbacks
> *alloc,
>  {
>     xcb_query_extension_cookie_t dri3_cookie, pres_cookie, amd_cookie,
> nv_cookie;
>     xcb_query_extension_reply_t *dri3_reply, *pres_reply, *amd_reply,
> *nv_reply;
> +   bool has_dri3_v1_1 = false;
> +   bool has_present_v1_1 = false;
>
>     struct wsi_x11_connection *wsi_conn =
>        vk_alloc(alloc, sizeof(*wsi_conn), 8,
> @@ -138,7 +140,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks
> *alloc,
>        return NULL;
>
>     dri3_cookie = xcb_query_extension(conn, 4, "DRI3");
> -   pres_cookie = xcb_query_extension(conn, 7, "PRESENT");
> +   pres_cookie = xcb_query_extension(conn, 7, "Present");
>

This seems a bit odd.  Did we just not use it before?  Looking through
things, it appears we didn't.


>     /* We try to be nice to users and emit a warning if they try to use a
>      * Vulkan application on a system without DRI3 enabled.  However, this
> ends
> @@ -173,13 +175,27 @@ wsi_x11_connection_create(const
> VkAllocationCallbacks *alloc,
>
>        ver_cookie = xcb_dri3_query_version(conn, 1, 1);
>        ver_reply = xcb_dri3_query_version_reply(conn, ver_cookie, NULL);
> -      wsi_conn->has_dri3_modifiers =
> +      has_dri3_v1_1 =
>           (ver_reply->major_version > 1 || ver_reply->minor_version >= 1);
>        free(ver_reply);
>     }
>  #endif
>
>     wsi_conn->has_present = pres_reply->present != 0;
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +   if (wsi_conn->has_present) {
> +      xcb_present_query_version_cookie_t ver_cookie;
> +      xcb_present_query_version_reply_t *ver_reply;
> +
> +      ver_cookie = xcb_present_query_version(conn, 1, 1);
> +      ver_reply = xcb_present_query_version_reply(conn, ver_cookie,
> NULL);
> +      has_present_v1_1 =
> +        (ver_reply->major_version > 1 || ver_reply->minor_version >= 1);
> +      free(ver_reply);
> +   }
> +#endif
> +
> +   wsi_conn->has_dri3_modifiers = has_dri3_v1_1 && has_present_v1_1;
>     wsi_conn->is_proprietary_x11 = false;
>     if (amd_reply && amd_reply->present)
>        wsi_conn->is_proprietary_x11 = true;
> @@ -651,6 +667,8 @@ struct x11_swapchain {
>
>     bool                                         threaded;
>     VkResult                                     status;
> +   bool                                         suboptimal;
> +   bool                                         realloc_suboptimal;
>     struct wsi_queue                             present_queue;
>     struct wsi_queue                             acquire_queue;
>     pthread_t                                    queue_manager;
> @@ -699,6 +717,10 @@ x11_handle_dri3_present_event(struct x11_swapchain
> *chain,
>        xcb_present_complete_notify_event_t *complete = (void *) event;
>        if (complete->kind == XCB_PRESENT_COMPLETE_KIND_PIXMAP)
>           chain->last_present_msc = complete->msc;
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +      if (complete->mode == XCB_PRESENT_COMPLETE_MODE_SUBOPTIMAL_COPY)
> +         chain->suboptimal = true;
>

I think I like the approach taken in GLX better.  Here, we'll properly
reallocate when we go from not flipping to flipping but, what happens if we
stop flipping?  In that case, we can do better if we reallocate again.

Also, I find "chain->suboptimal" and "chain->realloc_suboptimal" to be very
confusing.  chain->suboptimal has an obvious meaning but the other
doesn't.  At the very least we need better documentation as to what they
mean.


> +#endif
>        break;
>     }
>
> @@ -828,6 +850,11 @@ x11_present_to_x11(struct x11_swapchain *chain,
> uint32_t image_index,
>     if (chain->base.present_mode == VK_PRESENT_MODE_IMMEDIATE_KHR)
>        options |= XCB_PRESENT_OPTION_ASYNC;
>
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +   if (chain->has_dri3_modifiers)
> +      options |= XCB_PRESENT_OPTION_SUBOPTIMAL;
> +#endif
> +
>     xshmfence_reset(image->shm_fence);
>
>     ++chain->send_sbc;
> @@ -862,11 +889,19 @@ x11_acquire_next_image(struct wsi_swapchain
> *anv_chain,
>                         uint32_t *image_index)
>  {
>     struct x11_swapchain *chain = (struct x11_swapchain *)anv_chain;
> +   VkResult result;
>
>     if (chain->threaded) {
> -      return x11_acquire_next_image_from_queue(chain, image_index,
> timeout);
> +      result = x11_acquire_next_image_from_queue(chain, image_index,
> timeout);
>     } else {
> -      return x11_acquire_next_image_poll_x11(chain, image_index,
> timeout);
> +      result = x11_acquire_next_image_poll_x11(chain, image_index,
> timeout);
> +   }
> +
> +   if (result != VK_SUCCESS) {
> +      return result;
> +   } else {
> +      return (chain->realloc_suboptimal && chain->suboptimal) ?
> VK_SUBOPTIMAL_KHR
> +                                                              :
> VK_SUCCESS;
>     }
>  }
>
> @@ -876,12 +911,20 @@ x11_queue_present(struct wsi_swapchain *anv_chain,
>                    const VkPresentRegionKHR *damage)
>  {
>     struct x11_swapchain *chain = (struct x11_swapchain *)anv_chain;
> +   VkResult result;
>
>     if (chain->threaded) {
>        wsi_queue_push(&chain->present_queue, image_index);
> -      return chain->status;
> +      result = chain->status;
>     } else {
> -      return x11_present_to_x11(chain, image_index, 0);
> +      result = x11_present_to_x11(chain, image_index, 0);
> +   }
> +
> +   if (result != VK_SUCCESS) {
> +      return result;
> +   } else {
> +      return (chain->realloc_suboptimal && chain->suboptimal) ?
> VK_SUBOPTIMAL_KHR
> +                                                              :
> VK_SUCCESS;
>     }
>  }
>
> @@ -1220,6 +1263,15 @@ x11_surface_create_swapchain(VkIcdSurfaceBase
> *icd_surface,
>     chain->status = VK_SUCCESS;
>     chain->has_dri3_modifiers = wsi_conn->has_dri3_modifiers;
>
> +   chain->suboptimal = false;
> +   chain->realloc_suboptimal = true;
> +   if (pCreateInfo->oldSwapchain) {
> +      struct x11_swapchain *old_chain = (void *)pCreateInfo->oldSwapchain;
> +
> +      if (old_chain->suboptimal)
> +         chain->realloc_suboptimal = false;
> +   }
> +
>     if (!wsi_x11_check_dri3_compatible(conn, local_fd))
>         chain->base.use_prime_blit = true;
>
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to