[mutter] wayland: Move buffer use count to MetaWaylandSurface

[Jonas Ådahl <jadahl src gnome org>]

commit 10a01148561879f398f2b0a02e3416e6ca4f5fa9
Author: Jonas Ådahl <jadahl gmail com>
Date:   Wed Mar 16 14:47:53 2016 +0800

    wayland: Move buffer use count to MetaWaylandSurface
    
    Each wl_surface.commit with a newly attached buffer should result in
    one wl_buffer.release for the attached buffer. For example attaching
    the same buffer to two different surfaces must always result in two
    wl_buffer.release events being emitted by the server. The client is
    responsible for counting the wl_buffer.release events and be sure to
    have received as many release events as it has attached and committed
    the buffer, before reusing it.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=762828

 src/compositor/meta-surface-actor-wayland.c |    3 +-
 src/core/window.c                           |    4 +-
 src/wayland/meta-wayland-buffer.c           |   22 ----
 src/wayland/meta-wayland-buffer.h           |    3 -
 src/wayland/meta-wayland-pointer.c          |   15 ++--
 src/wayland/meta-wayland-surface.c          |  152 ++++++++++++++++-----------
 src/wayland/meta-wayland-surface.h          |   17 +++-
 7 files changed, 117 insertions(+), 99 deletions(-)
---
diff --git a/src/compositor/meta-surface-actor-wayland.c b/src/compositor/meta-surface-actor-wayland.c
index bb8bed2..6b6655e 100644
--- a/src/compositor/meta-surface-actor-wayland.c
+++ b/src/compositor/meta-surface-actor-wayland.c
@@ -136,7 +136,8 @@ meta_surface_actor_wayland_get_subsurface_rect (MetaSurfaceActorWayland *self,
                                                 MetaRectangle           *rect)
 {
   MetaWaylandSurface *surface = meta_surface_actor_wayland_get_surface (self);
-  CoglTexture *texture = surface->buffer->texture;
+  MetaWaylandBuffer *buffer = meta_wayland_surface_get_buffer (surface);
+  CoglTexture *texture = buffer->texture;
   MetaWindow *toplevel_window;
   int monitor_scale;
   float x, y;
diff --git a/src/core/window.c b/src/core/window.c
index 45cfcf4..bc481dc 100644
--- a/src/core/window.c
+++ b/src/core/window.c
@@ -726,7 +726,7 @@ client_window_should_be_mapped (MetaWindow *window)
 {
 #ifdef HAVE_WAYLAND
   if (window->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND &&
-      !window->surface->buffer)
+      !meta_wayland_surface_get_buffer (window->surface))
     return FALSE;
 #endif
 
@@ -1554,7 +1554,7 @@ meta_window_should_be_showing (MetaWindow  *window)
 {
 #ifdef HAVE_WAYLAND
   if (window->client_type == META_WINDOW_CLIENT_TYPE_WAYLAND &&
-      !window->surface->buffer)
+      !meta_wayland_surface_get_buffer (window->surface))
     return FALSE;
 #endif
 
diff --git a/src/wayland/meta-wayland-buffer.c b/src/wayland/meta-wayland-buffer.c
index b59fd30..775bd67 100644
--- a/src/wayland/meta-wayland-buffer.c
+++ b/src/wayland/meta-wayland-buffer.c
@@ -53,25 +53,6 @@ meta_wayland_buffer_destroy_handler (struct wl_listener *listener,
   g_object_unref (buffer);
 }
 
-void
-meta_wayland_buffer_ref_use_count (MetaWaylandBuffer *buffer)
-{
-  g_warn_if_fail (buffer->resource);
-
-  buffer->use_count++;
-}
-
-void
-meta_wayland_buffer_unref_use_count (MetaWaylandBuffer *buffer)
-{
-  g_return_if_fail (buffer->use_count != 0);
-
-  buffer->use_count--;
-
-  if (buffer->use_count == 0 && buffer->resource)
-    wl_resource_queue_event (buffer->resource, WL_BUFFER_RELEASE);
-}
-
 MetaWaylandBuffer *
 meta_wayland_buffer_from_resource (struct wl_resource *resource)
 {
@@ -106,7 +87,6 @@ meta_wayland_buffer_ensure_texture (MetaWaylandBuffer *buffer)
   CoglTexture *texture;
   struct wl_shm_buffer *shm_buffer;
 
-  g_return_val_if_fail (buffer->use_count != 0, NULL);
   g_return_val_if_fail (buffer->resource, NULL);
 
   if (buffer->texture)
@@ -142,8 +122,6 @@ meta_wayland_buffer_process_damage (MetaWaylandBuffer *buffer,
 {
   struct wl_shm_buffer *shm_buffer;
 
-  g_return_if_fail (buffer->use_count != 0);
-
   shm_buffer = wl_shm_buffer_get (buffer->resource);
 
   if (shm_buffer)
diff --git a/src/wayland/meta-wayland-buffer.h b/src/wayland/meta-wayland-buffer.h
index dcfef2d..977892b 100644
--- a/src/wayland/meta-wayland-buffer.h
+++ b/src/wayland/meta-wayland-buffer.h
@@ -39,7 +39,6 @@ struct _MetaWaylandBuffer
   struct wl_listener destroy_listener;
 
   CoglTexture *texture;
-  uint32_t use_count;
 };
 
 #define META_TYPE_WAYLAND_BUFFER (meta_wayland_buffer_get_type ())
@@ -47,8 +46,6 @@ G_DECLARE_FINAL_TYPE (MetaWaylandBuffer, meta_wayland_buffer,
                       META, WAYLAND_BUFFER, GObject);
 
 MetaWaylandBuffer *     meta_wayland_buffer_from_resource       (struct wl_resource    *resource);
-void                    meta_wayland_buffer_ref_use_count       (MetaWaylandBuffer     *buffer);
-void                    meta_wayland_buffer_unref_use_count     (MetaWaylandBuffer     *buffer);
 CoglTexture *           meta_wayland_buffer_ensure_texture      (MetaWaylandBuffer     *buffer);
 void                    meta_wayland_buffer_process_damage      (MetaWaylandBuffer     *buffer,
                                                                  cairo_region_t        *region);
diff --git a/src/wayland/meta-wayland-pointer.c b/src/wayland/meta-wayland-pointer.c
index f7106c6..173d7d0 100644
--- a/src/wayland/meta-wayland-pointer.c
+++ b/src/wayland/meta-wayland-pointer.c
@@ -966,24 +966,25 @@ update_cursor_sprite_texture (MetaWaylandSurface *surface)
   MetaWaylandSurfaceRoleCursor *cursor_role =
     META_WAYLAND_SURFACE_ROLE_CURSOR (surface->role);
   MetaCursorSprite *cursor_sprite = cursor_role->cursor_sprite;
+  MetaWaylandBuffer *buffer = meta_wayland_surface_get_buffer (surface);
 
-  g_return_if_fail (!surface->buffer || surface->buffer->texture);
+  g_return_if_fail (!buffer || buffer->texture);
 
-  if (surface->buffer)
+  if (buffer)
     {
       meta_cursor_sprite_set_texture (cursor_sprite,
-                                      surface->buffer->texture,
+                                      buffer->texture,
                                       cursor_role->hot_x * surface->scale,
                                       cursor_role->hot_y * surface->scale);
 
-      if (surface->using_buffer)
+      if (surface->buffer_ref.use_count > 0)
         {
-          struct wl_resource *buffer;
+          struct wl_resource *buffer_resource;
 
-          buffer = surface->buffer->resource;
+          buffer_resource = buffer->resource;
           meta_cursor_renderer_realize_cursor_from_wl_buffer (cursor_renderer,
                                                               cursor_sprite,
-                                                              buffer);
+                                                              buffer_resource);
         }
     }
   else
diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c
index 5e92e13..7e1fb01 100644
--- a/src/wayland/meta-wayland-surface.c
+++ b/src/wayland/meta-wayland-surface.c
@@ -176,55 +176,28 @@ meta_wayland_surface_assign_role (MetaWaylandSurface *surface,
 }
 
 static void
-surface_use_buffer (MetaWaylandSurface *surface)
-{
-  g_return_if_fail (!surface->using_buffer);
-
-  meta_wayland_buffer_ref_use_count (surface->buffer);
-  surface->using_buffer = TRUE;
-}
-
-static void
-surface_stop_using_buffer (MetaWaylandSurface *surface)
-{
-  if (!surface->using_buffer)
-    return;
-
-  meta_wayland_buffer_unref_use_count (surface->buffer);
-  surface->using_buffer = FALSE;
-}
-
-static void
-surface_set_buffer (MetaWaylandSurface *surface,
-                    MetaWaylandBuffer  *buffer)
-{
-  if (surface->buffer == buffer)
-    return;
-
-  if (surface->buffer)
-    surface_stop_using_buffer (surface);
-
-  g_set_object (&surface->buffer, buffer);
-}
-
-static void
 surface_process_damage (MetaWaylandSurface *surface,
                         cairo_region_t *region)
 {
+  MetaWaylandBuffer *buffer = surface->buffer_ref.buffer;
   unsigned int buffer_width;
   unsigned int buffer_height;
   cairo_rectangle_int_t surface_rect;
   cairo_region_t *scaled_region;
   int i, n_rectangles;
 
-  if (!surface->buffer)
+  /* If the client destroyed the buffer it attached before committing, but
+   * still posted damage, or posted damage without any buffer, don't try to
+   * process it on the non-existing buffer.
+   */
+  if (!buffer)
     return;
 
   /* Intersect the damage region with the surface region before scaling in
    * order to avoid integer overflow when scaling a damage region is too large
    * (for example INT32_MAX which mesa passes). */
-  buffer_width = cogl_texture_get_width (surface->buffer->texture);
-  buffer_height = cogl_texture_get_height (surface->buffer->texture);
+  buffer_width = cogl_texture_get_width (buffer->texture);
+  buffer_height = cogl_texture_get_height (buffer->texture);
   surface_rect = (cairo_rectangle_int_t) {
     .width = buffer_width / surface->scale,
     .height = buffer_height / surface->scale,
@@ -236,7 +209,7 @@ surface_process_damage (MetaWaylandSurface *surface,
   scaled_region = meta_region_scale (region, surface->scale);
 
   /* First update the buffer. */
-  meta_wayland_buffer_process_damage (surface->buffer, scaled_region);
+  meta_wayland_buffer_process_damage (buffer, scaled_region);
 
   /* Now damage the actor. The actor expects damage in the unscaled texture
    * coordinate space, i.e. same as the buffer. */
@@ -290,7 +263,7 @@ calculate_surface_window_geometry (MetaWaylandSurface *surface,
   if (!CLUTTER_ACTOR_IS_VISIBLE (CLUTTER_ACTOR (surface_actor)))
     return;
 
-  if (!surface->buffer)
+  if (!surface->buffer_ref.buffer)
     return;
 
   meta_surface_actor_wayland_get_subsurface_rect (surface_actor,
@@ -326,6 +299,36 @@ destroy_window (MetaWaylandSurface *surface)
   g_assert (surface->window == NULL);
 }
 
+MetaWaylandBuffer *
+meta_wayland_surface_get_buffer (MetaWaylandSurface *surface)
+{
+  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);
+
+  surface->buffer_ref.use_count++;
+}
+
+void
+meta_wayland_surface_unref_buffer_use_count (MetaWaylandSurface *surface)
+{
+  MetaWaylandBuffer *buffer = surface->buffer_ref.buffer;
+
+  g_return_if_fail (surface->buffer_ref.use_count != 0);
+
+  surface->buffer_ref.use_count--;
+
+  g_return_if_fail (buffer);
+
+  if (surface->buffer_ref.use_count == 0 && buffer->resource)
+    wl_resource_queue_event (buffer->resource, WL_BUFFER_RELEASE);
+}
+
 static void
 queue_surface_actor_frame_callbacks (MetaWaylandSurface      *surface,
                                      MetaWaylandPendingState *pending)
@@ -344,6 +347,7 @@ toplevel_surface_commit (MetaWaylandSurfaceRole  *surface_role,
 {
   MetaWaylandSurface *surface =
     meta_wayland_surface_role_get_surface (surface_role);
+  MetaWaylandBuffer *buffer = surface->buffer_ref.buffer;
   MetaWindow *window = surface->window;
 
   queue_surface_actor_frame_callbacks (surface, pending);
@@ -359,12 +363,12 @@ toplevel_surface_commit (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 && !window)
+      if (buffer && !window)
         {
           window = meta_window_wayland_new (meta_get_display (), surface);
           meta_wayland_surface_set_window (surface, window);
         }
-      else if (surface->buffer == NULL && window)
+      else if (buffer == NULL && window)
         {
           destroy_window (surface);
           return;
@@ -372,7 +376,7 @@ toplevel_surface_commit (MetaWaylandSurfaceRole  *surface_role,
     }
   else
     {
-      if (surface->buffer == NULL)
+      if (buffer == NULL)
         {
           /* XDG surfaces can't commit NULL buffers */
           wl_resource_post_error (surface->resource,
@@ -390,7 +394,7 @@ toplevel_surface_commit (MetaWaylandSurfaceRole  *surface_role,
     {
       MetaRectangle geom = { 0 };
 
-      CoglTexture *texture = surface->buffer->texture;
+      CoglTexture *texture = buffer->texture;
       /* Update the buffer rect immediately. */
       window->buffer_rect.width = cogl_texture_get_width (texture);
       window->buffer_rect.height = cogl_texture_get_height (texture);
@@ -559,7 +563,7 @@ subsurface_surface_commit (MetaWaylandSurfaceRole  *surface_role,
 
   queue_surface_actor_frame_callbacks (surface, pending);
 
-  if (surface->buffer != NULL)
+  if (surface->buffer_ref.buffer != NULL)
     clutter_actor_show (CLUTTER_ACTOR (surface_actor));
   else
     clutter_actor_hide (CLUTTER_ACTOR (surface_actor));
@@ -656,28 +660,46 @@ static void
 apply_pending_state (MetaWaylandSurface      *surface,
                      MetaWaylandPendingState *pending)
 {
-  gboolean release_new_buffer = FALSE;
+  MetaSurfaceActorWayland *surface_actor_wayland =
+    META_SURFACE_ACTOR_WAYLAND (surface->surface_actor);
 
   if (pending->newly_attached)
     {
-      if (!surface->buffer && surface->window)
+      gboolean switched_buffer;
+
+      if (!surface->buffer_ref.buffer && surface->window)
         meta_window_queue (surface->window, META_QUEUE_CALC_SHOWING);
 
-      surface_set_buffer (surface, pending->buffer);
+      /* Always release any previously held buffer. If the buffer held is same
+       * as the newly attached buffer, we still need to release it here, because
+       * wl_surface.attach+commit and wl_buffer.release on the attached buffer
+       * is symmetric.
+       */
+      if (surface->buffer_held)
+        meta_wayland_surface_unref_buffer_use_count (surface);
+
+      switched_buffer = g_set_object (&surface->buffer_ref.buffer,
+                                      pending->buffer);
 
-      if (pending->buffer && !surface->using_buffer)
-        {
-          struct wl_shm_buffer *shm_buffer = wl_shm_buffer_get (pending->buffer->resource);
+      if (pending->buffer)
+        meta_wayland_surface_ref_buffer_use_count (surface);
 
-          surface_use_buffer (surface);
-          CoglTexture *texture = meta_wayland_buffer_ensure_texture (pending->buffer);
-          meta_surface_actor_wayland_set_texture (META_SURFACE_ACTOR_WAYLAND (surface->surface_actor), 
texture);
+      if (switched_buffer && pending->buffer)
+        {
+          CoglTexture *texture;
 
-          /* Release the buffer as soon as possible, so the client can reuse it
-          */
-          if (shm_buffer)
-            release_new_buffer = TRUE;
+          texture = meta_wayland_buffer_ensure_texture (pending->buffer);
+          meta_surface_actor_wayland_set_texture (surface_actor_wayland,
+                                                  texture);
         }
+
+      /* If the newly attached buffer is going to be accessed directly without
+       * making a copy, such as an EGL buffer, mark it as in-use don't release
+       * it until is replaced by a subsequent wl_surface.commit or when the
+       * wl_surface is destroyed.
+       */
+      surface->buffer_held = (pending->buffer &&
+                              !wl_shm_buffer_get (pending->buffer->resource));
     }
 
   if (pending->scale > 0)
@@ -686,8 +708,12 @@ apply_pending_state (MetaWaylandSurface      *surface,
   if (!cairo_region_is_empty (pending->damage))
     surface_process_damage (surface, pending->damage);
 
-  if (release_new_buffer)
-    surface_stop_using_buffer (surface);
+  /* If we have a buffer that we are not using, decrease the use count so it may
+   * be released if no-one else has a use-reference to it.
+   */
+  if (pending->newly_attached &&
+      !surface->buffer_held && surface->buffer_ref.buffer)
+    meta_wayland_surface_unref_buffer_use_count (surface);
 
   surface->offset_x += pending->dx;
   surface->offset_y += pending->dy;
@@ -732,8 +758,7 @@ apply_pending_state (MetaWaylandSurface      *surface,
                  pending_state_signals[PENDING_STATE_SIGNAL_APPLIED],
                  0);
 
-  meta_surface_actor_wayland_sync_state (
-    META_SURFACE_ACTOR_WAYLAND (surface->surface_actor));
+  meta_surface_actor_wayland_sync_state (surface_actor_wayland);
 
   pending_state_reset (pending);
 
@@ -1122,7 +1147,10 @@ wl_surface_destructor (struct wl_resource *resource)
   if (surface->window)
     destroy_window (surface);
 
-  surface_set_buffer (surface, NULL);
+  if (surface->buffer_held)
+    meta_wayland_surface_unref_buffer_use_count (surface);
+  g_clear_object (&surface->buffer_ref.buffer);
+
   g_clear_object (&surface->pending);
 
   if (surface->opaque_region)
@@ -2771,10 +2799,10 @@ meta_wayland_surface_calculate_input_region (MetaWaylandSurface *surface)
   cairo_rectangle_int_t buffer_rect;
   CoglTexture *texture;
 
-  if (!surface->buffer)
+  if (!surface->buffer_ref.buffer)
     return NULL;
 
-  texture = surface->buffer->texture;
+  texture = surface->buffer_ref.buffer->texture;
   buffer_rect = (cairo_rectangle_int_t) {
     .width = cogl_texture_get_width (texture) / surface->scale,
     .height = cogl_texture_get_height (texture) / surface->scale,
diff --git a/src/wayland/meta-wayland-surface.h b/src/wayland/meta-wayland-surface.h
index 806903e..e7523d8 100644
--- a/src/wayland/meta-wayland-surface.h
+++ b/src/wayland/meta-wayland-surface.h
@@ -151,8 +151,6 @@ struct _MetaWaylandSurface
   MetaSurfaceActor *surface_actor;
   MetaWaylandSurfaceRole *role;
   MetaWindow *window;
-  MetaWaylandBuffer *buffer;
-  gboolean using_buffer;
   cairo_region_t *input_region;
   cairo_region_t *opaque_region;
   int scale;
@@ -160,6 +158,15 @@ struct _MetaWaylandSurface
   GList *subsurfaces;
   GHashTable *outputs_to_destroy_notify_id;
 
+  /* Buffer reference state. */
+  struct {
+    MetaWaylandBuffer *buffer;
+    unsigned int use_count;
+  } buffer_ref;
+
+  /* Buffer renderer state. */
+  gboolean buffer_held;
+
   /* List of pending frame callbacks that needs to stay queued longer than one
    * commit sequence, such as when it has not yet been assigned a role.
    */
@@ -230,6 +237,12 @@ MetaWaylandSurface *meta_wayland_surface_create (MetaWaylandCompositor *composit
 gboolean            meta_wayland_surface_assign_role (MetaWaylandSurface *surface,
                                                       GType               role_type);
 
+MetaWaylandBuffer  *meta_wayland_surface_get_buffer (MetaWaylandSurface *surface);
+
+void                meta_wayland_surface_ref_buffer_use_count (MetaWaylandSurface *surface);
+
+void                meta_wayland_surface_unref_buffer_use_count (MetaWaylandSurface *surface);
+
 void                meta_wayland_surface_set_window (MetaWaylandSurface *surface,
                                                      MetaWindow         *window);