From f606a4424a5afc71672566b15f56971bfb7fa4db Mon Sep 17 00:00:00 2001
From: Daniel van Vugt <daniel.van.vugt@canonical.com>
Date: Fri, 16 Feb 2024 16:57:55 +0800
Subject: [PATCH] compositor/x11: Sync again at the end of before_paint

The existing comment tells us this is necessary:

> there may be drawing between the last damage event and the
> XDamageSubtract() that needs to be flushed as well.

But the commit message for 551101c65cda also tells us that
synchronization is necessary before-update. Assuming both are correct
then it needs to be done in both places.

I did try optimizing out the second sync to only do it if damage
arrived during the update, but that doesn't seem to be the issue.
The damage event is arriving before the update starts and it's some
secondary changes within the damage region running late that need
flushing. So this means the client is reporting damage more frequently
than the frame rate and we're ignoring the secondary damage reports
for efficiency (XDamageReportBoundingBox), which is still a good thing.

Fixes: 551101c65cda ("compositor-x11: Move synchronization to before-update")
Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/2880
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3590>
---
 src/compositor/meta-compositor-x11.c | 30 ++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/compositor/meta-compositor-x11.c b/src/compositor/meta-compositor-x11.c
index 7694abc19..87ccff5fb 100644
--- a/src/compositor/meta-compositor-x11.c
+++ b/src/compositor/meta-compositor-x11.c
@@ -325,10 +325,7 @@ out:
 }
 
 static void
-on_before_update (ClutterStage     *stage,
-                  ClutterStageView *stage_view,
-                  ClutterFrame     *frame,
-                  MetaCompositor   *compositor)
+maybe_do_sync (MetaCompositor *compositor)
 {
   MetaCompositorX11 *compositor_x11 = META_COMPOSITOR_X11 (compositor);
 
@@ -363,6 +360,15 @@ on_before_update (ClutterStage     *stage,
     }
 }
 
+static void
+on_before_update (ClutterStage     *stage,
+                  ClutterStageView *stage_view,
+                  ClutterFrame     *frame,
+                  MetaCompositor   *compositor)
+{
+  maybe_do_sync (compositor);
+}
+
 static void
 on_after_update (ClutterStage     *stage,
                  ClutterStageView *stage_view,
@@ -391,6 +397,22 @@ meta_compositor_x11_before_paint (MetaCompositor     *compositor,
 
   parent_class = META_COMPOSITOR_CLASS (meta_compositor_x11_parent_class);
   parent_class->before_paint (compositor, compositor_view);
+
+  /* We must sync after MetaCompositor's before_paint because that's the final
+   * time XDamageSubtract may happen before painting (when it calls
+   * meta_window_actor_x11_before_paint -> handle_updates ->
+   * meta_surface_actor_x11_handle_updates). If a client was to redraw between
+   * the last damage event and XDamageSubtract, and the bounding box of the
+   * region didn't grow, then we will not receive a new damage report for it
+   * (because XDamageReportBoundingBox). Then if we haven't synchronized again
+   * and the same region doesn't change on subsequent frames, we have lost some
+   * part of the update from the client. So to ensure the correct pixels get
+   * composited we must sync at least once between XDamageSubtract and
+   * compositing, which is here. More related documentation can be found in
+   * maybe_do_sync.
+   */
+
+  maybe_do_sync (compositor);
 }
 
 static void