wayland/surface: Use transactions for synchronized sub-surface state

Instead of cached_state.

surface_commit for a synchronized sub-surface either commits the
transaction or merges it into the parent surface's transaction (if
the parent is a synchronized sub-surface itself).

This should fix or at least improve the behaviour of nested synchronized
sub-surfaces.

Also change wl_subsurface_set_desync:

* Commit sub-surface transactions separately. This may allow some of
  them to be applied earlier in some cases.
* Commit transaction only for descendant sub-surfaces which become
  newly de-synchronized themselves.

v2:
* Drop unused function prototypes
v3:
* Use g_clear_pointer for surface->sub.transaction.
v4:
* Use g_steal_pointer instead of g_clear_pointer. (Sebastian Wick, Jonas
  Ådahl)
v5: (Carlos Garnacho)
* Add spaces between type casts and values.
* Use (gpointer *) instead of (void**).
v6: (Jonas Ådahl)
* Use g_clear_object in meta_wayland_transaction_entry_merge_into.
* Use meta_wayland_transaction_entry_free in
  meta_wayland_transaction_merge_into.
* Fix alignment of meta_wayland_transaction_merge_pending_state
  parameters.
* Remove unused meta_wayland_transaction_add_state declaration.
v7:
* Use meta_wayland_surface_state_new in
  meta_wayland_transaction_merge_pending_state.

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1880>

src/wayland/meta-wayland-subsurface.c

@@ -290,10 +290,10 @@ wl_subsurface_set_position (struct wl_client   *client,
                             int32_t             y)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
+  MetaWaylandTransaction *transaction;
 
-  surface->sub.pending_x = x;
+  transaction = meta_wayland_surface_ensure_transaction (surface);
-  surface->sub.pending_y = y;
+  meta_wayland_transaction_add_subsurface_position (transaction, surface, x, y);
-  surface->sub.pending_pos = TRUE;
 }
 
 static gboolean
@@ -417,29 +417,34 @@ wl_subsurface_set_sync (struct wl_client   *client,
   surface->sub.synchronous = TRUE;
 }
 
+static void
+meta_wayland_subsurface_parent_desynced (MetaWaylandSurface *surface)
+{
+  MetaWaylandSurface *subsurface_surface;
+
+  if (surface->sub.synchronous)
+    return;
+
+  if (surface->sub.transaction)
+    meta_wayland_transaction_commit (g_steal_pointer (&surface->sub.transaction));
+
+  META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (surface, subsurface_surface)
+    meta_wayland_subsurface_parent_desynced (subsurface_surface);
+}
+
 static void
 wl_subsurface_set_desync (struct wl_client   *client,
                           struct wl_resource *resource)
 {
   MetaWaylandSurface *surface = wl_resource_get_user_data (resource);
-  gboolean is_parent_effectively_synchronized;
 
   if (!surface->sub.synchronous)
     return;
 
-  is_parent_effectively_synchronized =
-    meta_wayland_surface_is_synchronized (surface->sub.parent);
-
-  if (!is_parent_effectively_synchronized)
-    {
-      MetaWaylandTransaction *transaction;
-
-      transaction = meta_wayland_transaction_new (surface->compositor);
-      meta_wayland_transaction_add_cached_states (transaction, surface);
-      meta_wayland_transaction_commit (transaction);
-    }
-
   surface->sub.synchronous = FALSE;
+
+  if (!meta_wayland_surface_is_synchronized (surface))
+    meta_wayland_subsurface_parent_desynced (surface);
 }
 
 static const struct wl_subsurface_interface meta_wayland_wl_subsurface_interface = {

src/wayland/meta-wayland-surface.c

@@ -535,14 +535,14 @@ meta_wayland_surface_state_clear (MetaWaylandSurfaceState *state)
   meta_wayland_surface_state_discard_presentation_feedback (state);
 }
 
-static void
+void
 meta_wayland_surface_state_reset (MetaWaylandSurfaceState *state)
 {
   meta_wayland_surface_state_clear (state);
   meta_wayland_surface_state_set_default (state);
 }
 
-static void
+void
 meta_wayland_surface_state_merge_into (MetaWaylandSurfaceState *from,
                                        MetaWaylandSurfaceState *to)
 {
@@ -982,10 +982,21 @@ meta_wayland_surface_get_pending_state (MetaWaylandSurface *surface)
   return surface->pending_state;
 }
 
+MetaWaylandTransaction *
+meta_wayland_surface_ensure_transaction (MetaWaylandSurface *surface)
+{
+  if (!surface->sub.transaction)
+      surface->sub.transaction = meta_wayland_transaction_new (surface->compositor);
+
+  return surface->sub.transaction;
+}
+
 static void
 meta_wayland_surface_commit (MetaWaylandSurface *surface)
 {
   MetaWaylandSurfaceState *pending = surface->pending_state;
+  MetaWaylandTransaction *transaction;
+  MetaWaylandSurface *subsurface_surface;
 
   COGL_TRACE_BEGIN_SCOPED (MetaWaylandSurfaceCommit,
                            "WaylandSurface (commit)");
@@ -994,6 +1005,23 @@ meta_wayland_surface_commit (MetaWaylandSurface *surface)
       !meta_wayland_buffer_is_realized (pending->buffer))
     meta_wayland_buffer_realize (pending->buffer);
 
+  if (meta_wayland_surface_is_synchronized (surface))
+    transaction = meta_wayland_surface_ensure_transaction (surface);
+  else
+    transaction = meta_wayland_transaction_new (surface->compositor);
+
+  meta_wayland_transaction_merge_pending_state (transaction, surface);
+
+  META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (surface, subsurface_surface)
+    {
+      if (!subsurface_surface->sub.transaction)
+        continue;
+
+      meta_wayland_transaction_merge_into (subsurface_surface->sub.transaction,
+                                           transaction);
+      subsurface_surface->sub.transaction = NULL;
+    }
+
   /*
    * If this is a sub-surface and it is in effective synchronous mode, only
    * cache the pending surface state until either one of the following two
@@ -1002,32 +1030,8 @@ meta_wayland_surface_commit (MetaWaylandSurface *surface)
    *  2) Its mode changes from synchronized to desynchronized and its parent
    *     surface is in effective desynchronized mode.
    */
-  if (meta_wayland_surface_is_synchronized (surface))
+  if (!meta_wayland_surface_is_synchronized (surface))
-    {
+    meta_wayland_transaction_commit (transaction);
-      if (surface->cached_state)
-        {
-          meta_wayland_surface_state_merge_into (pending, surface->cached_state);
-          meta_wayland_surface_state_reset (pending);
-        }
-      else
-        {
-          surface->cached_state = pending;
-          surface->pending_state = meta_wayland_surface_state_new ();
-        }
-    }
-  else
-    {
-      MetaWaylandTransaction *transaction;
-
-      transaction = meta_wayland_transaction_new (surface->compositor);
-      meta_wayland_transaction_add_state (transaction, surface, pending);
-      if (surface->sub.pending_pos)
-          meta_wayland_transaction_add_subsurface_position (transaction, surface);
-      meta_wayland_transaction_add_cached_child_states (transaction, surface);
-      meta_wayland_transaction_commit (transaction);
-
-      surface->pending_state = meta_wayland_surface_state_new ();
-    }
 }
 
 static void
@@ -1478,8 +1482,8 @@ wl_surface_destructor (struct wl_resource *resource)
   g_clear_pointer (&surface->texture, cogl_object_unref);
   g_clear_pointer (&surface->buffer_ref, meta_wayland_buffer_ref_unref);
 
-  g_clear_object (&surface->cached_state);
   g_clear_object (&surface->pending_state);
+  g_clear_pointer (&surface->sub.transaction, meta_wayland_transaction_free);
 
   if (surface->opaque_region)
     cairo_region_destroy (surface->opaque_region);

src/wayland/meta-wayland-surface.h

@@ -191,8 +191,6 @@ struct _MetaWaylandSurface
 
   /* All the pending state that wl_surface.commit will apply. */
   MetaWaylandSurfaceState *pending_state;
-  /* State cached due to inter-surface synchronization such. */
-  MetaWaylandSurfaceState *cached_state;
 
   /* Extension resources. */
   struct wl_resource *wl_subsurface;
@@ -215,9 +213,10 @@ struct _MetaWaylandSurface
      */
     gboolean synchronous;
 
-    int32_t pending_x;
+    /* Transaction which contains all synchronized state for this sub-surface.
-    int32_t pending_y;
+     * This can include state for nested sub-surfaces.
-    gboolean pending_pos;
+     */
+    MetaWaylandTransaction *transaction;
   } sub;
 
   /* wp_viewport */
@@ -270,14 +269,19 @@ MetaWaylandSurface *meta_wayland_surface_create (MetaWaylandCompositor *composit
                                                  struct wl_resource    *compositor_resource,
                                                  guint32                id);
 
+void                meta_wayland_surface_state_reset (MetaWaylandSurfaceState *state);
+
+void                meta_wayland_surface_state_merge_into (MetaWaylandSurfaceState *from,
+                                                           MetaWaylandSurfaceState *to);
+
 void                meta_wayland_surface_apply_state (MetaWaylandSurface      *surface,
                                                       MetaWaylandSurfaceState *state);
 
 MetaWaylandSurfaceState *
                     meta_wayland_surface_get_pending_state (MetaWaylandSurface *surface);
 
-MetaWaylandSurfaceState *
+MetaWaylandTransaction *
-                    meta_wayland_surface_ensure_cached_state (MetaWaylandSurface *surface);
+                    meta_wayland_surface_ensure_transaction (MetaWaylandSurface *surface);
 
 gboolean            meta_wayland_surface_assign_role (MetaWaylandSurface *surface,
                                                       GType               role_type,

src/wayland/meta-wayland-transaction.c

@@ -308,70 +308,6 @@ meta_wayland_transaction_ensure_entry (MetaWaylandTransaction *transaction,
   return entry;
 }
 
-void
-meta_wayland_transaction_add_state (MetaWaylandTransaction  *transaction,
-                                    MetaWaylandSurface      *surface,
-                                    MetaWaylandSurfaceState *state)
-{
-  MetaWaylandTransactionEntry *entry;
-
-  entry = meta_wayland_transaction_ensure_entry (transaction, surface);
-  g_assert (!entry->state);
-  entry->state = state;
-}
-
-void
-meta_wayland_transaction_add_subsurface_position (MetaWaylandTransaction *transaction,
-                                                  MetaWaylandSurface     *surface)
-{
-  MetaWaylandTransactionEntry *entry;
-
-  entry = meta_wayland_transaction_ensure_entry (transaction, surface);
-  entry->x = surface->sub.pending_x;
-  entry->y = surface->sub.pending_y;
-  entry->has_sub_pos = TRUE;
-  surface->sub.pending_pos = FALSE;
-}
-
-static gboolean
-meta_wayland_transaction_add_cached_state (MetaWaylandTransaction *transaction,
-                                           MetaWaylandSurface     *surface)
-{
-  MetaWaylandSurfaceState *cached = surface->cached_state;
-  gboolean is_synchronized;
-
-  is_synchronized = meta_wayland_surface_is_synchronized (surface);
-
-  if (is_synchronized && cached)
-    {
-      meta_wayland_transaction_add_state (transaction, surface, cached);
-      surface->cached_state = NULL;
-    }
-
-  if (surface->sub.pending_pos)
-    meta_wayland_transaction_add_subsurface_position (transaction, surface);
-
-  return is_synchronized;
-}
-
-void
-meta_wayland_transaction_add_cached_child_states (MetaWaylandTransaction *transaction,
-                                                  MetaWaylandSurface     *surface)
-{
-  MetaWaylandSurface *subsurface_surface;
-
-  META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (surface, subsurface_surface)
-    meta_wayland_transaction_add_cached_states (transaction, subsurface_surface);
-}
-
-void
-meta_wayland_transaction_add_cached_states (MetaWaylandTransaction *transaction,
-                                            MetaWaylandSurface     *surface)
-{
-  if (meta_wayland_transaction_add_cached_state (transaction, surface))
-    meta_wayland_transaction_add_cached_child_states (transaction, surface);
-}
-
 static void
 meta_wayland_transaction_entry_free (MetaWaylandTransactionEntry *entry)
 {
@@ -379,6 +315,88 @@ meta_wayland_transaction_entry_free (MetaWaylandTransactionEntry *entry)
   g_free (entry);
 }
 
+void
+meta_wayland_transaction_add_subsurface_position (MetaWaylandTransaction *transaction,
+                                                  MetaWaylandSurface     *surface,
+                                                  int                     x,
+                                                  int                     y)
+{
+  MetaWaylandTransactionEntry *entry;
+
+  entry = meta_wayland_transaction_ensure_entry (transaction, surface);
+  entry->x = x;
+  entry->y = y;
+  entry->has_sub_pos = TRUE;
+}
+
+static void
+meta_wayland_transaction_entry_merge_into (MetaWaylandTransactionEntry *from,
+                                           MetaWaylandTransactionEntry *to)
+{
+  if (from->has_sub_pos)
+    {
+      to->x = from->x;
+      to->y = from->y;
+      to->has_sub_pos = TRUE;
+    }
+
+  if (to->state)
+    {
+      meta_wayland_surface_state_merge_into (from->state, to->state);
+      g_clear_object (&from->state);
+      return;
+    }
+
+  to->state = from->state;
+}
+
+void
+meta_wayland_transaction_merge_into (MetaWaylandTransaction *from,
+                                     MetaWaylandTransaction *to)
+{
+  GHashTableIter iter;
+  MetaWaylandSurface *surface;
+  MetaWaylandTransactionEntry *from_entry, *to_entry;
+
+  g_hash_table_iter_init (&iter, from->entries);
+  while (g_hash_table_iter_next (&iter, (gpointer *) &surface,
+                                 (gpointer *) &from_entry))
+    {
+      g_hash_table_iter_steal (&iter);
+      to_entry = meta_wayland_transaction_get_entry (to, surface);
+      if (!to_entry)
+        {
+          g_hash_table_insert (to->entries, surface, from_entry);
+          continue;
+        }
+
+      meta_wayland_transaction_entry_merge_into (from_entry, to_entry);
+      meta_wayland_transaction_entry_free (from_entry);
+    }
+
+  meta_wayland_transaction_free (from);
+}
+
+void
+meta_wayland_transaction_merge_pending_state (MetaWaylandTransaction *transaction,
+                                              MetaWaylandSurface     *surface)
+{
+  MetaWaylandSurfaceState *pending = surface->pending_state;
+  MetaWaylandTransactionEntry *entry;
+
+  entry = meta_wayland_transaction_ensure_entry (transaction, surface);
+
+  if (!entry->state)
+    {
+      entry->state = pending;
+      surface->pending_state = meta_wayland_surface_state_new ();
+      return;
+    }
+
+  meta_wayland_surface_state_merge_into (pending, entry->state);
+  meta_wayland_surface_state_reset (pending);
+}
+
 MetaWaylandTransaction *
 meta_wayland_transaction_new (MetaWaylandCompositor *compositor)
 {

src/wayland/meta-wayland-transaction.h

@@ -24,18 +24,16 @@
 
 void meta_wayland_transaction_commit (MetaWaylandTransaction *transaction);
 
-void meta_wayland_transaction_add_state (MetaWaylandTransaction  *transaction,
-                                         MetaWaylandSurface      *surface,
-                                         MetaWaylandSurfaceState *state);
-
 void meta_wayland_transaction_add_subsurface_position (MetaWaylandTransaction *transaction,
-                                                       MetaWaylandSurface     *surface);
+                                                       MetaWaylandSurface     *surface,
+                                                       int                     x,
+                                                       int                     y);
 
-void meta_wayland_transaction_add_cached_states (MetaWaylandTransaction *transaction,
+void meta_wayland_transaction_merge_into (MetaWaylandTransaction *from,
-                                                 MetaWaylandSurface     *surface);
+                                          MetaWaylandTransaction *to);
 
-void meta_wayland_transaction_add_cached_child_states (MetaWaylandTransaction *transaction,
+void meta_wayland_transaction_merge_pending_state (MetaWaylandTransaction *transaction,
-                                                       MetaWaylandSurface     *surface);
+                                                   MetaWaylandSurface *surface);
 
 MetaWaylandTransaction *meta_wayland_transaction_new (MetaWaylandCompositor *compositor);