Mesa (main): vulkan/wsi/x11: do not inherit last_present_mode

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 206fe780d509b3e0814c52bf15765a0de33743bf
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=206fe780d509b3e0814c52bf15765a0de33743bf

Author: Chia-I Wu <olvaffe at gmail.com>
Date:   Thu Jul 22 16:52:47 2021 -0700

vulkan/wsi/x11: do not inherit last_present_mode

Under XWayland, the first present after a window resize is sometimes
completed with COPY (seems to happen when the previous present with the
old size is pending; not really sure).  The following presents are
completed with FLIP.

When a swapchain is created with an old swapchain, and
old_chain->last_present_mode is FLIP, chain->last_present_mode is set to
FLIP as well.  This causes the new swapchain to be marked
VK_SUBOPTIMAL_KHR, which is sticky, if the first present is completed
with COPY.

Instead of inheriting, treat each swapchain as independent.  We will
miss the case where an old swapchain is flipping but a new swapchain is
copying.  But swapchain reallocation normally happens in response to
present engine state change.  If the newly allocated swapchain is
copying, another reallocation is unlikely to fix that.

Fixes: 61309c2a727 ("vulkan/wsi/x11: Return VK_SUBOPTIMAL_KHR for X11")
Signed-off-by: Chia-I Wu <olvaffe at gmail.com>
Reviewed-by: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12030>

---

 src/vulkan/wsi/wsi_common_x11.c | 55 ++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 3c2ad386807..9dce78eddeb 100644
--- a/src/vulkan/wsi/wsi_common_x11.c
+++ b/src/vulkan/wsi/wsi_common_x11.c
@@ -816,7 +816,7 @@ struct x11_swapchain {
    bool                                         has_present_queue;
    bool                                         has_acquire_queue;
    VkResult                                     status;
-   xcb_present_complete_mode_t                  last_present_mode;
+   bool                                         copy_is_suboptimal;
    struct wsi_queue                             present_queue;
    struct wsi_queue                             acquire_queue;
    pthread_t                                    queue_manager;
@@ -932,25 +932,30 @@ x11_handle_dri3_present_event(struct x11_swapchain *chain,
       }
 
       VkResult result = VK_SUCCESS;
-
-      /* The winsys is now trying to flip directly and cannot due to our
-       * configuration. Request the user reallocate.
-       */
+      switch (complete->mode) {
+      case XCB_PRESENT_COMPLETE_MODE_COPY:
+         if (chain->copy_is_suboptimal)
+            result = VK_SUBOPTIMAL_KHR;
+         break;
+      case XCB_PRESENT_COMPLETE_MODE_FLIP:
+         /* If we ever go from flipping to copying, the odds are very likely
+          * that we could reallocate in a more optimal way if we didn't have
+          * to care about scanout, so we always do this.
+          */
+         chain->copy_is_suboptimal = true;
+         break;
 #ifdef HAVE_DRI3_MODIFIERS
-      if (complete->mode == XCB_PRESENT_COMPLETE_MODE_SUBOPTIMAL_COPY &&
-          chain->last_present_mode != XCB_PRESENT_COMPLETE_MODE_SUBOPTIMAL_COPY)
+      case XCB_PRESENT_COMPLETE_MODE_SUBOPTIMAL_COPY:
+         /* The winsys is now trying to flip directly and cannot due to our
+          * configuration. Request the user reallocate.
+          */
          result = VK_SUBOPTIMAL_KHR;
+         break;
 #endif
+      default:
+         break;
+      }
 
-      /* When we go from flipping to copying, the odds are very likely that
-       * we could reallocate in a more optimal way if we didn't have to care
-       * about scanout, so we always do this.
-       */
-      if (complete->mode == XCB_PRESENT_COMPLETE_MODE_COPY &&
-          chain->last_present_mode == XCB_PRESENT_COMPLETE_MODE_FLIP)
-         result = VK_SUBOPTIMAL_KHR;
-
-      chain->last_present_mode = complete->mode;
       return result;
    }
 
@@ -1631,17 +1636,15 @@ x11_surface_create_swapchain(VkIcdSurfaceBase *icd_surface,
    if (chain->extent.width != cur_width || chain->extent.height != cur_height)
        chain->status = VK_SUBOPTIMAL_KHR;
 
-   /* If we are reallocating from an old swapchain, then we inherit its
-    * last completion mode, to ensure we don't get into reallocation
-    * cycles. If we are starting anew, we set 'COPY', as that is the only
-    * mode which provokes reallocation when anything changes, to make
-    * sure we have the most optimal allocation.
+   /* We used to inherit copy_is_suboptimal from pCreateInfo->oldSwapchain.
+    * When it was true, and when the next present was completed with copying,
+    * we would return VK_SUBOPTIMAL_KHR and hint the app to reallocate again
+    * for no good reason.  If all following presents on the surface were
+    * completed with copying because of some surface state change, we would
+    * always return VK_SUBOPTIMAL_KHR no matter how many times the app had
+    * reallocated.
     */
-   VK_FROM_HANDLE(x11_swapchain, old_chain, pCreateInfo->oldSwapchain);
-   if (old_chain)
-      chain->last_present_mode = old_chain->last_present_mode;
-   else
-      chain->last_present_mode = XCB_PRESENT_COMPLETE_MODE_COPY;
+   chain->copy_is_suboptimal = false;
 
    if (!wsi_device->sw)
       if (!wsi_x11_check_dri3_compatible(wsi_device, conn))