[gtk/wip/damaged-but-no-frost-bite: 2/5] x11: defer _NET_WM_FRAME_DRAWN update until frame usable by compositor

[Ray Strode <halfline src gnome org>]

commit e60414960cd2b80bfaca66947be0eea77d7f9bcc
Author: Ray Strode <rstrode redhat com>
Date:   Wed May 27 15:12:35 2020 -0400

    x11: defer _NET_WM_FRAME_DRAWN update until frame usable by compositor
    
    With the vendor provided Nvidia driver there is a small window of time
    after drawing to a GL surface before the updates to that surface
    can be used by the compositor.
    
    Drawing is already coordinated with the compositor through the frame
    synchronization protocol detailed here:
    
    https://fishsoup.net/misc/wm-spec-synchronization.html
    
    Unfortunately, at the moment, GdkX11Surface tells the compositor the
    frame is ready imediately after drawing to the surface, not later,
    when it's consumable by the compositor.
    
    This commit defers announcing the frame as ready until it's consumable
    by the compositor. It does this by listening for the X server to announce
    damage events associated with the frame drawing.  It finds the right
    damage event by matching it up with a fence created at buffer swap time.

 gdk/x11/gdkdisplay-x11.h   |  1 +
 gdk/x11/gdkglcontext-x11.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++
 gdk/x11/gdkglcontext-x11.h |  9 ++++++
 gdk/x11/gdkprivate-x11.h   |  3 ++
 gdk/x11/gdksurface-x11.c   | 24 ++++++++++++--
 gdk/x11/gdksurface-x11.h   |  3 ++
 6 files changed, 118 insertions(+), 3 deletions(-)
---
diff --git a/gdk/x11/gdkdisplay-x11.h b/gdk/x11/gdkdisplay-x11.h
index 3e4fb24d63..bc1066286c 100644
--- a/gdk/x11/gdkdisplay-x11.h
+++ b/gdk/x11/gdkdisplay-x11.h
@@ -149,6 +149,7 @@ struct _GdkX11Display
   guint has_glx_multisample : 1;
   guint has_glx_visual_rating : 1;
   guint has_glx_create_es2_context : 1;
+  guint has_async_glx_swap_buffers : 1;
 
   gint damage_event_base;
   gint damage_error_base;
diff --git a/gdk/x11/gdkglcontext-x11.c b/gdk/x11/gdkglcontext-x11.c
index 95d8e66c66..f2a752d690 100644
--- a/gdk/x11/gdkglcontext-x11.c
+++ b/gdk/x11/gdkglcontext-x11.c
@@ -183,6 +183,16 @@ gdk_x11_gl_context_end_frame (GdkDrawContext *draw_context,
 
   gdk_x11_surface_pre_damage (surface);
 
+#ifdef HAVE_XDAMAGE
+  if (context_x11->xdamage != 0)
+    {
+      g_assert (context_x11->frame_fence == 0);
+
+      context_x11->frame_fence = glFenceSync (GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+      _gdk_x11_surface_set_still_painting_frame (surface, TRUE);
+    }
+#endif
+
   glXSwapBuffers (dpy, drawable);
 
   if (context_x11->do_frame_sync && info != NULL && display_x11->has_glx_video_sync)
@@ -565,6 +575,45 @@ create_legacy_context (GdkDisplay   *display,
   return res;
 }
 
+#ifdef HAVE_XDAMAGE
+static gboolean
+on_gl_surface_xevent (GdkGLContext   *context,
+                      XEvent         *xevent,
+                      GdkX11Display  *display_x11)
+{
+  GdkX11GLContext *context_x11 = GDK_X11_GL_CONTEXT (context);
+  GdkSurface *surface = gdk_gl_context_get_surface (context);
+  XDamageNotifyEvent *damage_xevent;
+
+  if (!context_x11->is_attached)
+    return FALSE;
+
+  if (xevent->type != (display_x11->damage_event_base + XDamageNotify))
+    return FALSE;
+
+  damage_xevent = (XDamageNotifyEvent *) xevent;
+
+  if (damage_xevent->damage != context_x11->xdamage)
+    return FALSE;
+
+  if (context_x11->frame_fence)
+    {
+      GLenum wait_result;
+
+      wait_result = glClientWaitSync (context_x11->frame_fence, 0, 0);
+
+      if (wait_result == GL_ALREADY_SIGNALED)
+        {
+          glDeleteSync (context_x11->frame_fence);
+          context_x11->frame_fence = 0;
+          _gdk_x11_surface_set_still_painting_frame (surface, FALSE);
+        }
+    }
+
+  return FALSE;
+}
+#endif
+
 static gboolean
 gdk_x11_gl_context_realize (GdkGLContext  *context,
                             GError       **error)
@@ -737,6 +786,24 @@ gdk_x11_gl_context_realize (GdkGLContext  *context,
   context_x11->attached_drawable = info->glx_drawable ? info->glx_drawable : gdk_x11_surface_get_xid 
(surface);
   context_x11->unattached_drawable = info->dummy_glx ? info->dummy_glx : info->dummy_xwin;
 
+#ifdef HAVE_XDAMAGE
+  if (display_x11->have_damage && display_x11->has_async_glx_swap_buffers)
+    {
+      gdk_x11_display_error_trap_push (display);
+      context_x11->xdamage = XDamageCreate (dpy,
+                                            gdk_x11_surface_get_xid (surface),
+                                            XDamageReportRawRectangles);
+      if (gdk_x11_display_error_trap_pop (display))
+        context_x11->xdamage = 0;
+      else
+        g_signal_connect_object (G_OBJECT (display),
+                                 "xevent",
+                                 G_CALLBACK (on_gl_surface_xevent),
+                                 context,
+                                 G_CONNECT_SWAPPED);
+    }
+#endif
+
   context_x11->is_direct = glXIsDirect (dpy, context_x11->glx_context);
 
   GDK_DISPLAY_NOTE (display, OPENGL,
@@ -766,6 +833,10 @@ gdk_x11_gl_context_dispose (GObject *gobject)
       context_x11->glx_context = NULL;
     }
 
+#ifdef HAVE_XDAMAGE
+  context_x11->xdamage = 0;
+#endif
+
   G_OBJECT_CLASS (gdk_x11_gl_context_parent_class)->dispose (gobject);
 }
 
@@ -841,6 +912,16 @@ gdk_x11_screen_init_gl (GdkX11Screen *screen)
   display_x11->has_glx_visual_rating =
     epoxy_has_glx_extension (dpy, screen_num, "GLX_EXT_visual_rating");
 
+  if (g_strcmp0 (glXGetClientString (dpy, GLX_VENDOR), "NVIDIA Corporation") == 0)
+    {
+      /* There is a window of time after glXSwapBuffers before other processes can
+       * see the updated drawing with the Nvidia vendor driver.  We need to take
+       * special care, in this case, to defer telling the compositor our latest
+       * frame is ready until after the X server says the frame has been drawn.
+       */
+      display_x11->has_async_glx_swap_buffers = TRUE;
+    }
+
   GDK_DISPLAY_NOTE (display, OPENGL,
             g_message ("GLX version %d.%d found\n"
                        " - Vendor: %s\n"
diff --git a/gdk/x11/gdkglcontext-x11.h b/gdk/x11/gdkglcontext-x11.h
index 89d1131fcc..b17ed2956e 100644
--- a/gdk/x11/gdkglcontext-x11.h
+++ b/gdk/x11/gdkglcontext-x11.h
@@ -24,6 +24,10 @@
 #include <X11/X.h>
 #include <X11/Xlib.h>
 
+#ifdef HAVE_XDAMAGE
+#include <X11/extensions/Xdamage.h>
+#endif
+
 #include <epoxy/gl.h>
 #include <epoxy/glx.h>
 
@@ -44,6 +48,11 @@ struct _GdkX11GLContext
   GLXDrawable attached_drawable;
   GLXDrawable unattached_drawable;
 
+#ifdef HAVE_XDAMAGE
+  GLsync frame_fence;
+  Damage xdamage;
+#endif
+
   guint is_attached : 1;
   guint is_direct : 1;
   guint do_frame_sync : 1;
diff --git a/gdk/x11/gdkprivate-x11.h b/gdk/x11/gdkprivate-x11.h
index 7b8fbc7711..46017b17da 100644
--- a/gdk/x11/gdkprivate-x11.h
+++ b/gdk/x11/gdkprivate-x11.h
@@ -104,6 +104,9 @@ void _gdk_x11_surface_grab_check_unmap   (GdkSurface *window,
                                           gulong     serial);
 void _gdk_x11_surface_grab_check_destroy (GdkSurface *window);
 
+void _gdk_x11_surface_set_still_painting_frame (GdkSurface *surface,
+                                                gboolean    painting);
+
 gboolean _gdk_x11_display_is_root_window (GdkDisplay *display,
                                           Window      xroot_window);
 
diff --git a/gdk/x11/gdksurface-x11.c b/gdk/x11/gdksurface-x11.c
index 30eb411d62..e3f06d4746 100644
--- a/gdk/x11/gdksurface-x11.c
+++ b/gdk/x11/gdksurface-x11.c
@@ -360,6 +360,23 @@ gdk_x11_surface_begin_frame (GdkSurface *surface,
     }
 }
 
+void
+_gdk_x11_surface_set_still_painting_frame (GdkSurface *surface,
+                                           gboolean    painting)
+{
+  GdkX11Surface *impl = GDK_X11_SURFACE (surface);
+
+  if (impl->toplevel->frame_still_painting == painting)
+    return;
+
+  impl->toplevel->frame_still_painting = painting;
+
+  if (!impl->toplevel->frame_still_painting && impl->toplevel->frame_pending)
+    set_sync_counter (GDK_SURFACE_XDISPLAY (surface),
+                      impl->toplevel->extended_update_counter,
+                      impl->toplevel->current_counter_value);
+}
+
 static void
 gdk_x11_surface_end_frame (GdkSurface *surface)
 {
@@ -405,9 +422,10 @@ gdk_x11_surface_end_frame (GdkSurface *surface)
       else
         impl->toplevel->current_counter_value += 1;
 
-      set_sync_counter(GDK_SURFACE_XDISPLAY (surface),
-                      impl->toplevel->extended_update_counter,
-                      impl->toplevel->current_counter_value);
+      if (!impl->toplevel->frame_still_painting)
+        set_sync_counter (GDK_SURFACE_XDISPLAY (surface),
+                          impl->toplevel->extended_update_counter,
+                          impl->toplevel->current_counter_value);
 
       if (impl->frame_sync_enabled &&
           gdk_x11_screen_supports_net_wm_hint (GDK_SURFACE_SCREEN (surface),
diff --git a/gdk/x11/gdksurface-x11.h b/gdk/x11/gdksurface-x11.h
index 8d8e255a36..5715c2f21d 100644
--- a/gdk/x11/gdksurface-x11.h
+++ b/gdk/x11/gdksurface-x11.h
@@ -124,6 +124,9 @@ struct _GdkToplevelX11
 
   guint in_frame : 1;
 
+  /* If we're waiting for damage from the X server after painting a frame */
+  guint frame_still_painting : 1;
+
   /* If we're expecting a response from the compositor after painting a frame */
   guint frame_pending : 1;