[mutter] wayland/surface: Put buffer reference on heap

[Jonas Ådahl <jadahl src gnome org>]

commit 47002bf0cd9983cb5e45121d566d203baef71103
Author: Jonas Ådahl <jadahl gmail com>
Date:   Tue Aug 13 18:46:36 2019 +0200

    wayland/surface: Put buffer reference on heap
    
    Currently a buffer use count always reaches zero before it is replaced.
    This is due to the fact that at the point a new buffer is attached, the
    last potential user releases it (the stage) since the currently
    displayed frame has a composited copy of the buffer.
    
    This may however change, if a buffer is scanned out directly, meaning it
    should not be released until the page flip callback is invoked.
    
    Prepare for this by making the buffer reference a heap allocated struct,
    enabling us to keep a pointer to it longer than the buffer is attached.
    
    https://gitlab.gnome.org/GNOME/mutter/merge_requests/798

 src/wayland/meta-wayland-actor-surface.c    |  2 +-
 src/wayland/meta-wayland-legacy-xdg-shell.c | 12 ++++----
 src/wayland/meta-wayland-shell-surface.c    |  4 +--
 src/wayland/meta-wayland-subsurface.c       |  2 +-
 src/wayland/meta-wayland-surface.c          | 47 +++++++++++++++++++++--------
 src/wayland/meta-wayland-surface.h          | 11 ++++---
 src/wayland/meta-wayland-wl-shell.c         |  4 +--
 src/wayland/meta-wayland-xdg-shell.c        | 14 ++++-----
 src/wayland/meta-xwayland-surface.c         |  2 +-
 9 files changed, 61 insertions(+), 37 deletions(-)
---
diff --git a/src/wayland/meta-wayland-actor-surface.c b/src/wayland/meta-wayland-actor-surface.c
index 6b030fa7b..faf170067 100644
--- a/src/wayland/meta-wayland-actor-surface.c
+++ b/src/wayland/meta-wayland-actor-surface.c
@@ -156,7 +156,7 @@ meta_wayland_actor_surface_real_sync_actor_state (MetaWaylandActorSurface *actor
   surface_actor = priv->actor;
   stex = meta_surface_actor_get_texture (surface_actor);
 
-  buffer = surface->buffer_ref.buffer;
+  buffer = surface->buffer_ref->buffer;
   if (buffer)
     {
       CoglSnippet *snippet;
diff --git a/src/wayland/meta-wayland-legacy-xdg-shell.c b/src/wayland/meta-wayland-legacy-xdg-shell.c
index eba802e0c..9c5b02e1a 100644
--- a/src/wayland/meta-wayland-legacy-xdg-shell.c
+++ b/src/wayland/meta-wayland-legacy-xdg-shell.c
@@ -986,7 +986,7 @@ meta_wayland_zxdg_popup_v6_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (!pending->newly_attached)
     return;
 
-  if (!surface->buffer_ref.buffer)
+  if (!surface->buffer_ref->buffer)
     return;
 
   if (pending->has_acked_configure_serial)
@@ -1348,7 +1348,7 @@ meta_wayland_zxdg_surface_v6_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (!priv->resource)
     return;
 
-  if (surface->buffer_ref.buffer == NULL && priv->first_buffer_attached)
+  if (!surface->buffer_ref->buffer && priv->first_buffer_attached)
     {
       /* XDG surfaces can't commit NULL buffers */
       wl_resource_post_error (surface->resource,
@@ -1357,7 +1357,7 @@ meta_wayland_zxdg_surface_v6_apply_state (MetaWaylandSurfaceRole  *surface_role,
       return;
     }
 
-  if (surface->buffer_ref.buffer && !priv->configure_sent)
+  if (surface->buffer_ref->buffer && !priv->configure_sent)
     {
       wl_resource_post_error (surface->resource,
                               ZXDG_SURFACE_V6_ERROR_UNCONFIGURED_BUFFER,
@@ -1368,7 +1368,7 @@ meta_wayland_zxdg_surface_v6_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (!window)
     return;
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     priv->first_buffer_attached = TRUE;
   else
     return;
@@ -1413,7 +1413,7 @@ meta_wayland_zxdg_surface_v6_assigned (MetaWaylandSurfaceRole *surface_role)
   priv->configure_sent = FALSE;
   priv->first_buffer_attached = FALSE;
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     {
       wl_resource_post_error (xdg_shell_resource,
                               ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE,
@@ -1957,7 +1957,7 @@ zxdg_shell_v6_get_xdg_surface (struct wl_client   *client,
       return;
     }
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     {
       wl_resource_post_error (resource,
                               ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE,
diff --git a/src/wayland/meta-wayland-shell-surface.c b/src/wayland/meta-wayland-shell-surface.c
index a1f5f36af..12d291043 100644
--- a/src/wayland/meta-wayland-shell-surface.c
+++ b/src/wayland/meta-wayland-shell-surface.c
@@ -245,7 +245,7 @@ meta_wayland_shell_surface_surface_pre_apply_state (MetaWaylandSurfaceRole  *sur
     meta_wayland_surface_role_get_surface (surface_role);
 
   if (pending->newly_attached &&
-      !surface->buffer_ref.buffer &&
+      !surface->buffer_ref->buffer &&
       priv->window)
     meta_window_queue (priv->window, META_QUEUE_CALC_SHOWING);
 }
@@ -271,7 +271,7 @@ meta_wayland_shell_surface_surface_apply_state (MetaWaylandSurfaceRole  *surface
     META_WAYLAND_SURFACE_ROLE_CLASS (meta_wayland_shell_surface_parent_class);
   surface_role_class->apply_state (surface_role, pending);
 
-  buffer = surface->buffer_ref.buffer;
+  buffer = surface->buffer_ref->buffer;
   if (!buffer)
     return;
 
diff --git a/src/wayland/meta-wayland-subsurface.c b/src/wayland/meta-wayland-subsurface.c
index efa1da1b6..1ec5b230b 100644
--- a/src/wayland/meta-wayland-subsurface.c
+++ b/src/wayland/meta-wayland-subsurface.c
@@ -87,7 +87,7 @@ sync_actor_subsurface_state (MetaWaylandSurface *surface)
   clutter_actor_set_position (actor, x, y);
   clutter_actor_set_reactive (actor, TRUE);
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     clutter_actor_show (actor);
   else
     clutter_actor_hide (actor);
diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c
index 7e5e8eb71..e80c722fa 100644
--- a/src/wayland/meta-wayland-surface.c
+++ b/src/wayland/meta-wayland-surface.c
@@ -120,6 +120,22 @@ meta_wayland_surface_role_is_on_logical_monitor (MetaWaylandSurfaceRole *surface
 static MetaWaylandSurface *
 meta_wayland_surface_role_get_toplevel (MetaWaylandSurfaceRole *surface_role);
 
+static MetaWaylandBufferRef *
+meta_wayland_buffer_ref_new (void)
+{
+  MetaWaylandBufferRef *buffer_ref;
+
+  buffer_ref = g_new0 (MetaWaylandBufferRef, 1);
+
+  return buffer_ref;
+}
+
+static void
+meta_wayland_buffer_ref_free (MetaWaylandBufferRef *buffer_ref)
+{
+  g_free (buffer_ref);
+}
+
 static void
 role_assignment_valist_to_properties (GType       role_type,
                                       const char *first_property_name,
@@ -364,30 +380,30 @@ meta_wayland_surface_queue_pending_state_frame_callbacks (MetaWaylandSurface
 MetaWaylandBuffer *
 meta_wayland_surface_get_buffer (MetaWaylandSurface *surface)
 {
-  return surface->buffer_ref.buffer;
+  return surface->buffer_ref->buffer;
 }
 
 void
 meta_wayland_surface_ref_buffer_use_count (MetaWaylandSurface *surface)
 {
-  g_return_if_fail (surface->buffer_ref.buffer);
-  g_warn_if_fail (surface->buffer_ref.buffer->resource);
+  g_return_if_fail (surface->buffer_ref->buffer);
+  g_warn_if_fail (surface->buffer_ref->buffer->resource);
 
-  surface->buffer_ref.use_count++;
+  surface->buffer_ref->use_count++;
 }
 
 void
 meta_wayland_surface_unref_buffer_use_count (MetaWaylandSurface *surface)
 {
-  MetaWaylandBuffer *buffer = surface->buffer_ref.buffer;
+  MetaWaylandBuffer *buffer = surface->buffer_ref->buffer;
 
-  g_return_if_fail (surface->buffer_ref.use_count != 0);
+  g_return_if_fail (surface->buffer_ref->use_count != 0);
 
-  surface->buffer_ref.use_count--;
+  surface->buffer_ref->use_count--;
 
   g_return_if_fail (buffer);
 
-  if (surface->buffer_ref.use_count == 0 && buffer->resource)
+  if (surface->buffer_ref->use_count == 0 && buffer->resource)
     wl_buffer_send_release (buffer->resource);
 }
 
@@ -634,7 +650,7 @@ meta_wayland_surface_apply_state (MetaWaylandSurface      *surface,
       if (surface->buffer_held)
         meta_wayland_surface_unref_buffer_use_count (surface);
 
-      g_set_object (&surface->buffer_ref.buffer, state->buffer);
+      g_set_object (&surface->buffer_ref->buffer, state->buffer);
 
       if (state->buffer)
         meta_wayland_surface_ref_buffer_use_count (surface);
@@ -739,7 +755,8 @@ meta_wayland_surface_apply_state (MetaWaylandSurface      *surface,
            * role the surface is given. That means we need to also keep a use
            * count for wl_buffer's that are used by unassigned wl_surface's.
            */
-          g_set_object (&surface->unassigned.buffer, surface->buffer_ref.buffer);
+          g_set_object (&surface->unassigned.buffer,
+                        surface->buffer_ref->buffer);
           if (surface->unassigned.buffer)
             meta_wayland_surface_ref_buffer_use_count (surface);
         }
@@ -750,7 +767,7 @@ cleanup:
    * be released if no-one else has a use-reference to it.
    */
   if (state->newly_attached &&
-      !surface->buffer_held && surface->buffer_ref.buffer)
+      !surface->buffer_held && surface->buffer_ref->buffer)
     meta_wayland_surface_unref_buffer_use_count (surface);
 
   g_signal_emit (state,
@@ -1278,7 +1295,8 @@ wl_surface_destructor (struct wl_resource *resource)
   if (surface->buffer_held)
     meta_wayland_surface_unref_buffer_use_count (surface);
   g_clear_pointer (&surface->texture, cogl_object_unref);
-  g_clear_object (&surface->buffer_ref.buffer);
+  g_clear_object (&surface->buffer_ref->buffer);
+  g_clear_pointer (&surface->buffer_ref, meta_wayland_buffer_ref_free);
 
   g_clear_object (&surface->cached_state);
   g_clear_object (&surface->pending_state);
@@ -1544,6 +1562,9 @@ static void
 meta_wayland_surface_init (MetaWaylandSurface *surface)
 {
   surface->pending_state = g_object_new (META_TYPE_WAYLAND_SURFACE_STATE, NULL);
+
+  surface->buffer_ref = meta_wayland_buffer_ref_new ();
+
   surface->subsurface_branch_node = g_node_new (surface);
   surface->subsurface_leaf_node =
     g_node_prepend_data (surface->subsurface_branch_node, surface);
@@ -1813,7 +1834,7 @@ meta_wayland_surface_calculate_input_region (MetaWaylandSurface *surface)
   cairo_region_t *region;
   cairo_rectangle_int_t buffer_rect;
 
-  if (!surface->buffer_ref.buffer)
+  if (!surface->buffer_ref->buffer)
     return NULL;
 
   buffer_rect = (cairo_rectangle_int_t) {
diff --git a/src/wayland/meta-wayland-surface.h b/src/wayland/meta-wayland-surface.h
index 00b61c615..b4ffeb338 100644
--- a/src/wayland/meta-wayland-surface.h
+++ b/src/wayland/meta-wayland-surface.h
@@ -136,6 +136,12 @@ struct _MetaWaylandDragDestFuncs
                       MetaWaylandSurface    *surface);
 };
 
+typedef struct _MetaWaylandBufferRef
+{
+  MetaWaylandBuffer *buffer;
+  unsigned int use_count;
+} MetaWaylandBufferRef;
+
 struct _MetaWaylandSurface
 {
   GObject parent;
@@ -156,10 +162,7 @@ struct _MetaWaylandSurface
   CoglTexture *texture;
 
   /* Buffer reference state. */
-  struct {
-    MetaWaylandBuffer *buffer;
-    unsigned int use_count;
-  } buffer_ref;
+  MetaWaylandBufferRef *buffer_ref;
 
   /* Buffer renderer state. */
   gboolean buffer_held;
diff --git a/src/wayland/meta-wayland-wl-shell.c b/src/wayland/meta-wayland-wl-shell.c
index 3215f4394..7add5667d 100644
--- a/src/wayland/meta-wayland-wl-shell.c
+++ b/src/wayland/meta-wayland-wl-shell.c
@@ -598,11 +598,11 @@ wl_shell_surface_role_apply_state (MetaWaylandSurfaceRole  *surface_role,
   /* For wl_shell, it's equivalent to an unmap. Semantics
    * are poorly defined, so we can choose some that are
    * convenient for us. */
-  if (surface->buffer_ref.buffer && !window)
+  if (surface->buffer_ref->buffer && !window)
     {
       create_wl_shell_surface_window (surface);
     }
-  else if (!surface->buffer_ref.buffer && window)
+  else if (!surface->buffer_ref->buffer && window)
     {
       if (wl_shell_surface->popup)
         meta_wayland_popup_dismiss (wl_shell_surface->popup);
diff --git a/src/wayland/meta-wayland-xdg-shell.c b/src/wayland/meta-wayland-xdg-shell.c
index d8f639757..ecad04e12 100644
--- a/src/wayland/meta-wayland-xdg-shell.c
+++ b/src/wayland/meta-wayland-xdg-shell.c
@@ -754,7 +754,7 @@ meta_wayland_xdg_toplevel_apply_state (MetaWaylandSurfaceRole  *surface_role,
       return;
     }
 
-  if (!surface->buffer_ref.buffer && xdg_surface_priv->first_buffer_attached)
+  if (!surface->buffer_ref->buffer && xdg_surface_priv->first_buffer_attached)
     {
       MetaWaylandActorSurface *actor_surface =
         META_WAYLAND_ACTOR_SURFACE (xdg_toplevel);
@@ -1100,7 +1100,7 @@ meta_wayland_xdg_popup_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (xdg_popup->setup.parent_surface)
     finish_popup_setup (xdg_popup);
 
-  if (!surface->buffer_ref.buffer && xdg_surface_priv->first_buffer_attached)
+  if (!surface->buffer_ref->buffer && xdg_surface_priv->first_buffer_attached)
     {
       meta_wayland_xdg_surface_reset (xdg_surface);
       meta_wayland_surface_cache_pending_frame_callbacks (surface, pending);
@@ -1111,7 +1111,7 @@ meta_wayland_xdg_popup_apply_state (MetaWaylandSurfaceRole  *surface_role,
     META_WAYLAND_SURFACE_ROLE_CLASS (meta_wayland_xdg_popup_parent_class);
   surface_role_class->apply_state (surface_role, pending);
 
-  if (xdg_popup->dismissed_by_client && surface->buffer_ref.buffer)
+  if (xdg_popup->dismissed_by_client && surface->buffer_ref->buffer)
     {
       wl_resource_post_error (xdg_popup->resource,
                               XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,
@@ -1127,7 +1127,7 @@ meta_wayland_xdg_popup_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (!pending->newly_attached)
     return;
 
-  if (!surface->buffer_ref.buffer)
+  if (!surface->buffer_ref->buffer)
     return;
 
   if (pending->has_acked_configure_serial)
@@ -1524,7 +1524,7 @@ meta_wayland_xdg_surface_apply_state (MetaWaylandSurfaceRole  *surface_role,
   if (!window)
     return;
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     priv->first_buffer_attached = TRUE;
   else
     return;
@@ -1568,7 +1568,7 @@ meta_wayland_xdg_surface_assigned (MetaWaylandSurfaceRole *surface_role)
   priv->configure_sent = FALSE;
   priv->first_buffer_attached = FALSE;
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     {
       wl_resource_post_error (xdg_wm_base_resource,
                               XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,
@@ -2254,7 +2254,7 @@ xdg_wm_base_get_xdg_surface (struct wl_client   *client,
       return;
     }
 
-  if (surface->buffer_ref.buffer)
+  if (surface->buffer_ref->buffer)
     {
       wl_resource_post_error (resource,
                               XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,
diff --git a/src/wayland/meta-xwayland-surface.c b/src/wayland/meta-xwayland-surface.c
index 4a4615a2c..c8625f414 100644
--- a/src/wayland/meta-xwayland-surface.c
+++ b/src/wayland/meta-xwayland-surface.c
@@ -182,7 +182,7 @@ meta_xwayland_surface_pre_apply_state (MetaWaylandSurfaceRole  *surface_role,
   MetaXwaylandSurface *xwayland_surface = META_XWAYLAND_SURFACE (surface_role);
 
   if (pending->newly_attached &&
-      surface->buffer_ref.buffer &&
+      surface->buffer_ref->buffer &&
       xwayland_surface->window)
     meta_window_queue (xwayland_surface->window, META_QUEUE_CALC_SHOWING);
 }