wayland/surface: Overhaul handling of buffer use count

Move the use count from a separate MetaWaylandBufferRef struct to the
MetaWaylandBuffer class, and remove the former.

The buffer use count is now incremented already in
meta_wayland_surface_commit, since the Wayland protocol defines the
buffer to be in use by the compositor at that point. If the buffer
attachment ends up being dropped again before it is applied to the
surface state (e.g. because another buffer is committed to a
synchronized sub-surface before the parent surface is committed),
the use count is now decremented, and a buffer release event is sent if
the use count drops to 0.

Buffer release events were previously incorrectly not sent under these
circumstances. Test case: Run the weston-subsurfaces demo with the -r1
and/or -t1 command line parameter. Resize the window. Before this
change, weston-subsurfaces would freeze or abort after a few resize
operations, because mutter failed to send release events and the
client ran out of usable buffers.

v2:
* Handle NULL priv->buffer_ref in
  meta_wayland_cursor_surface_apply_state.
v3:
* Remove MetaWaylandBufferRef altogether, move the use count tracking
  to MetaWaylandBuffer itself. Much simpler, and doesn't run into
  lifetime issues when mutter shuts down.
v4:
* Warn if use count isn't 0 in meta_wayland_buffer_finalize.
* Keep pending_buffer_resource_destroyed for attached but not yet
  committed buffers. If the client attaches a buffer and then destroys
  it before commit, we ignore the buffer attachement, same as before
  this MR.
v5:
* Rebase on top of new commit which splits up surface->texture.
* MetaWaylandSurfaceState::buffer can only be non-NULL if
  ::newly_attached is TRUE, simplify accordingly.

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1880>
This commit is contained in:
Michel Dänzer 2022-06-22 18:43:11 +02:00 committed by Michel Dänzer
parent 6e6a38342e
commit 05efcb4b21
11 changed files with 102 additions and 157 deletions

View File

@ -184,7 +184,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 = meta_wayland_surface_get_buffer (surface);
if (buffer)
{
CoglSnippet *snippet;

View File

@ -621,6 +621,25 @@ meta_wayland_buffer_create_snippet (MetaWaylandBuffer *buffer)
#endif /* HAVE_WAYLAND_EGLSTREAM */
}
void
meta_wayland_buffer_inc_use_count (MetaWaylandBuffer *buffer)
{
g_warn_if_fail (buffer->resource);
buffer->use_count++;
}
void
meta_wayland_buffer_dec_use_count (MetaWaylandBuffer *buffer)
{
g_return_if_fail (buffer->use_count > 0);
buffer->use_count--;
if (buffer->use_count == 0 && buffer->resource)
wl_buffer_send_release (buffer->resource);
}
gboolean
meta_wayland_buffer_is_y_inverted (MetaWaylandBuffer *buffer)
{
@ -804,6 +823,8 @@ meta_wayland_buffer_finalize (GObject *object)
{
MetaWaylandBuffer *buffer = META_WAYLAND_BUFFER (object);
g_warn_if_fail (buffer->use_count == 0);
g_clear_pointer (&buffer->egl_image.texture, cogl_object_unref);
#ifdef HAVE_WAYLAND_EGLSTREAM
g_clear_pointer (&buffer->egl_stream.texture, cogl_object_unref);

View File

@ -54,6 +54,8 @@ struct _MetaWaylandBuffer
struct wl_resource *resource;
struct wl_listener destroy_listener;
unsigned int use_count;
gboolean is_y_inverted;
MetaWaylandBufferType type;
@ -93,6 +95,8 @@ gboolean meta_wayland_buffer_attach (MetaWaylandBuff
CoglTexture **texture,
GError **error);
CoglSnippet * meta_wayland_buffer_create_snippet (MetaWaylandBuffer *buffer);
void meta_wayland_buffer_inc_use_count (MetaWaylandBuffer *buffer);
void meta_wayland_buffer_dec_use_count (MetaWaylandBuffer *buffer);
gboolean meta_wayland_buffer_is_y_inverted (MetaWaylandBuffer *buffer);
void meta_wayland_buffer_process_damage (MetaWaylandBuffer *buffer,
CoglTexture *texture,

View File

@ -139,12 +139,10 @@ meta_wayland_cursor_surface_pre_apply_state (MetaWaylandSurfaceRole *surface_ro
META_WAYLAND_CURSOR_SURFACE (surface_role);
MetaWaylandCursorSurfacePrivate *priv =
meta_wayland_cursor_surface_get_instance_private (cursor_surface);
MetaWaylandSurface *surface =
meta_wayland_surface_role_get_surface (surface_role);
if (pending->newly_attached && priv->buffer)
{
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (priv->buffer);
g_clear_object (&priv->buffer);
}
}
@ -157,15 +155,11 @@ meta_wayland_cursor_surface_apply_state (MetaWaylandSurfaceRole *surface_role,
META_WAYLAND_CURSOR_SURFACE (surface_role);
MetaWaylandCursorSurfacePrivate *priv =
meta_wayland_cursor_surface_get_instance_private (cursor_surface);
MetaWaylandSurface *surface =
meta_wayland_surface_role_get_surface (surface_role);
MetaWaylandBuffer *buffer = meta_wayland_surface_get_buffer (surface);
if (pending->newly_attached)
if (pending->buffer)
{
g_set_object (&priv->buffer, buffer);
if (priv->buffer)
meta_wayland_surface_ref_buffer_use_count (surface);
priv->buffer = g_object_ref (pending->buffer);
meta_wayland_buffer_inc_use_count (priv->buffer);
}
wl_list_insert_list (&priv->frame_callbacks,
@ -213,8 +207,6 @@ meta_wayland_cursor_surface_dispose (GObject *object)
META_WAYLAND_CURSOR_SURFACE (object);
MetaWaylandCursorSurfacePrivate *priv =
meta_wayland_cursor_surface_get_instance_private (cursor_surface);
MetaWaylandSurface *surface =
meta_wayland_surface_role_get_surface (META_WAYLAND_SURFACE_ROLE (object));
MetaWaylandFrameCallback *cb, *next;
wl_list_for_each_safe (cb, next, &priv->frame_callbacks, link)
@ -234,7 +226,7 @@ meta_wayland_cursor_surface_dispose (GObject *object)
if (priv->buffer)
{
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (priv->buffer);
g_clear_object (&priv->buffer);
}
@ -263,8 +255,8 @@ meta_wayland_cursor_surface_constructed (GObject *object)
if (buffer && buffer->resource)
{
g_set_object (&priv->buffer, buffer);
meta_wayland_surface_ref_buffer_use_count (surface);
priv->buffer = g_object_ref (surface->buffer);
meta_wayland_buffer_inc_use_count (priv->buffer);
}
priv->cursor_sprite = meta_cursor_sprite_wayland_new (surface,

View File

@ -215,7 +215,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 &&
priv->window)
meta_window_queue (priv->window, META_QUEUE_CALC_SHOWING);
}

View File

@ -58,7 +58,7 @@ transform_subsurface_position (MetaWaylandSurface *surface,
static gboolean
should_show (MetaWaylandSurface *surface)
{
if (!surface->buffer_ref->buffer)
if (!surface->buffer)
return FALSE;
else if (surface->output_state.parent)
return should_show (surface->output_state.parent);
@ -126,7 +126,7 @@ meta_wayland_subsurface_union_geometry (MetaWaylandSubsurface *subsurface,
.height = meta_wayland_surface_get_height (surface),
};
if (surface->buffer_ref->buffer)
if (surface->buffer)
meta_rectangle_union (out_geometry, &geometry, out_geometry);
META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (&surface->output_state,

View File

@ -143,58 +143,6 @@ set_surface_is_on_output (MetaWaylandSurface *surface,
MetaWaylandOutput *wayland_output,
gboolean is_on_output);
static MetaWaylandBufferRef *
meta_wayland_buffer_ref_new (void)
{
MetaWaylandBufferRef *buffer_ref;
buffer_ref = g_new0 (MetaWaylandBufferRef, 1);
g_ref_count_init (&buffer_ref->ref_count);
return buffer_ref;
}
static MetaWaylandBufferRef *
meta_wayland_buffer_ref_ref (MetaWaylandBufferRef *buffer_ref)
{
g_ref_count_inc (&buffer_ref->ref_count);
return buffer_ref;
}
static void
meta_wayland_buffer_ref_unref (MetaWaylandBufferRef *buffer_ref)
{
if (g_ref_count_dec (&buffer_ref->ref_count))
{
g_warn_if_fail (buffer_ref->use_count == 0);
g_clear_object (&buffer_ref->buffer);
g_free (buffer_ref);
}
}
static void
meta_wayland_buffer_ref_inc_use_count (MetaWaylandBufferRef *buffer_ref)
{
g_return_if_fail (buffer_ref->buffer);
g_warn_if_fail (buffer_ref->buffer->resource);
buffer_ref->use_count++;
}
static void
meta_wayland_buffer_ref_dec_use_count (MetaWaylandBufferRef *buffer_ref)
{
MetaWaylandBuffer *buffer = buffer_ref->buffer;
g_return_if_fail (buffer_ref->use_count > 0);
g_return_if_fail (buffer);
buffer_ref->use_count--;
if (buffer_ref->use_count == 0 && buffer->resource)
wl_buffer_send_release (buffer->resource);
}
static void
role_assignment_valist_to_properties (GType role_type,
const char *first_property_name,
@ -287,7 +235,7 @@ meta_wayland_surface_assign_role (MetaWaylandSurface *surface,
/* Release the use count held on behalf of the just assigned role. */
if (surface->unassigned.buffer)
{
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (surface->unassigned.buffer);
g_clear_object (&surface->unassigned.buffer);
}
@ -447,19 +395,7 @@ surface_process_damage (MetaWaylandSurface *surface,
MetaWaylandBuffer *
meta_wayland_surface_get_buffer (MetaWaylandSurface *surface)
{
return surface->buffer_ref->buffer;
}
void
meta_wayland_surface_ref_buffer_use_count (MetaWaylandSurface *surface)
{
meta_wayland_buffer_ref_inc_use_count (surface->buffer_ref);
}
void
meta_wayland_surface_unref_buffer_use_count (MetaWaylandSurface *surface)
{
meta_wayland_buffer_ref_dec_use_count (surface->buffer_ref);
return surface->buffer;
}
static void
@ -531,8 +467,15 @@ meta_wayland_surface_state_clear (MetaWaylandSurfaceState *state)
g_clear_pointer (&state->opaque_region, cairo_region_destroy);
g_clear_pointer (&state->xdg_positioner, g_free);
if (state->buffer)
g_clear_signal_handler (&state->buffer_destroy_handler_id, state->buffer);
if (state->buffer_destroy_handler_id)
{
g_clear_signal_handler (&state->buffer_destroy_handler_id, state->buffer);
state->buffer = NULL;
}
else
{
g_clear_object (&state->buffer);
}
wl_list_for_each_safe (cb, next, &state->frame_callback_list, link)
wl_resource_destroy (cb->resource);
@ -557,10 +500,14 @@ meta_wayland_surface_state_merge_into (MetaWaylandSurfaceState *from,
if (from->newly_attached)
{
if (to->buffer)
g_clear_signal_handler (&to->buffer_destroy_handler_id, to->buffer);
{
g_warn_if_fail (to->buffer_destroy_handler_id == 0);
meta_wayland_buffer_dec_use_count (to->buffer);
g_object_unref (to->buffer);
}
to->newly_attached = TRUE;
to->buffer = from->buffer;
to->buffer = g_steal_pointer (&from->buffer);
cogl_clear_object (&to->texture);
to->texture = g_steal_pointer (&from->texture);
@ -646,14 +593,6 @@ meta_wayland_surface_state_merge_into (MetaWaylandSurfaceState *from,
to->has_new_viewport_dst_size = TRUE;
}
if (to->buffer && to->buffer_destroy_handler_id == 0)
{
to->buffer_destroy_handler_id =
g_signal_connect (to->buffer, "resource-destroyed",
G_CALLBACK (pending_buffer_resource_destroyed),
to);
}
if (from->subsurface_placement_ops != NULL)
{
if (to->subsurface_placement_ops != NULL)
@ -795,7 +734,7 @@ meta_wayland_surface_apply_state (MetaWaylandSurface *surface,
{
if (state->newly_attached && surface->unassigned.buffer)
{
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (surface->unassigned.buffer);
g_clear_object (&surface->unassigned.buffer);
}
}
@ -808,19 +747,9 @@ meta_wayland_surface_apply_state (MetaWaylandSurface *surface,
* is symmetric.
*/
if (surface->buffer_held)
meta_wayland_surface_unref_buffer_use_count (surface);
if (surface->buffer_ref->use_count > 0)
{
meta_wayland_buffer_ref_unref (surface->buffer_ref);
surface->buffer_ref = meta_wayland_buffer_ref_new ();
}
g_set_object (&surface->buffer_ref->buffer, state->buffer);
if (state->buffer)
meta_wayland_surface_ref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (surface->buffer);
g_set_object (&surface->buffer, state->buffer);
cogl_clear_object (&surface->output_state.texture);
surface->output_state.texture = g_steal_pointer (&state->texture);
@ -933,28 +862,26 @@ meta_wayland_surface_apply_state (MetaWaylandSurface *surface,
&state->frame_callback_list);
wl_list_init (&state->frame_callback_list);
if (state->newly_attached)
if (state->buffer)
{
/* The need to keep the wl_buffer from being released depends on what
* 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);
if (surface->unassigned.buffer)
meta_wayland_surface_ref_buffer_use_count (surface);
surface->unassigned.buffer = g_object_ref (state->buffer);
meta_wayland_buffer_inc_use_count (surface->unassigned.buffer);
}
}
if (state->subsurface_placement_ops)
meta_wayland_surface_notify_subsurface_state_changed (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 we need to hold the newly attached buffer, drop its reference from the
* state, to prevent meta_wayland_transaction_entry_destroy from decreasing
* the use count.
*/
if (state->newly_attached &&
!surface->buffer_held && surface->buffer_ref->buffer)
meta_wayland_surface_unref_buffer_use_count (surface);
if (state->newly_attached && surface->buffer_held)
g_clear_object (&state->buffer);
g_signal_emit (state,
surface_state_signals[SURFACE_STATE_SIGNAL_APPLIED],
@ -1010,6 +937,9 @@ meta_wayland_surface_commit (MetaWaylandSurface *surface)
{
g_autoptr (GError) error = NULL;
g_clear_signal_handler (&pending->buffer_destroy_handler_id,
buffer);
if (!meta_wayland_buffer_is_realized (buffer))
meta_wayland_buffer_realize (buffer);
@ -1027,6 +957,9 @@ meta_wayland_surface_commit (MetaWaylandSurface *surface)
}
pending->texture = cogl_object_ref (surface->protocol_state.texture);
g_object_ref (buffer);
meta_wayland_buffer_inc_use_count (buffer);
}
else if (pending->newly_attached)
{
@ -1503,14 +1436,14 @@ meta_wayland_surface_finalize (GObject *object)
if (surface->unassigned.buffer)
{
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (surface->unassigned.buffer);
g_clear_object (&surface->unassigned.buffer);
}
if (surface->buffer_held)
meta_wayland_surface_unref_buffer_use_count (surface);
meta_wayland_buffer_dec_use_count (surface->buffer);
g_clear_pointer (&surface->output_state.texture, cogl_object_unref);
g_clear_pointer (&surface->buffer_ref, meta_wayland_buffer_ref_unref);
g_clear_object (&surface->buffer);
if (surface->opaque_region)
cairo_region_destroy (surface->opaque_region);
@ -1796,8 +1729,6 @@ meta_wayland_surface_init (MetaWaylandSurface *surface)
{
surface->pending_state = meta_wayland_surface_state_new ();
surface->buffer_ref = meta_wayland_buffer_ref_new ();
surface->output_state.subsurface_branch_node = g_node_new (surface);
surface->output_state.subsurface_leaf_node =
g_node_prepend_data (surface->output_state.subsurface_branch_node, surface);
@ -2113,7 +2044,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)
return NULL;
buffer_rect = (cairo_rectangle_int_t) {
@ -2227,10 +2158,10 @@ static void
scanout_destroyed (gpointer data,
GObject *where_the_object_was)
{
MetaWaylandBufferRef *buffer_ref = data;
MetaWaylandBuffer *buffer = data;
meta_wayland_buffer_ref_dec_use_count (buffer_ref);
meta_wayland_buffer_ref_unref (buffer_ref);
meta_wayland_buffer_dec_use_count (buffer);
g_object_unref (buffer);
}
CoglScanout *
@ -2238,22 +2169,22 @@ meta_wayland_surface_try_acquire_scanout (MetaWaylandSurface *surface,
CoglOnscreen *onscreen)
{
CoglScanout *scanout;
MetaWaylandBufferRef *buffer_ref;
MetaWaylandBuffer *buffer;
if (!surface->buffer_ref->buffer)
if (!surface->buffer)
return NULL;
if (surface->buffer_ref->use_count == 0)
if (surface->buffer->use_count == 0)
return NULL;
scanout = meta_wayland_buffer_try_acquire_scanout (surface->buffer_ref->buffer,
scanout = meta_wayland_buffer_try_acquire_scanout (surface->buffer,
onscreen);
if (!scanout)
return NULL;
buffer_ref = meta_wayland_buffer_ref_ref (surface->buffer_ref);
meta_wayland_buffer_ref_inc_use_count (buffer_ref);
g_object_weak_ref (G_OBJECT (scanout), scanout_destroyed, buffer_ref);
buffer = g_object_ref (surface->buffer);
meta_wayland_buffer_inc_use_count (buffer);
g_object_weak_ref (G_OBJECT (scanout), scanout_destroyed, buffer);
return scanout;
}

View File

@ -155,13 +155,6 @@ struct _MetaWaylandDragDestFuncs
MetaWaylandSurface *surface);
};
typedef struct _MetaWaylandBufferRef
{
grefcount ref_count;
MetaWaylandBuffer *buffer;
unsigned int use_count;
} MetaWaylandBufferRef;
struct _MetaWaylandSurface
{
GObject parent;
@ -178,7 +171,7 @@ struct _MetaWaylandSurface
MetaMonitorTransform buffer_transform;
/* Buffer reference state. */
MetaWaylandBufferRef *buffer_ref;
MetaWaylandBuffer *buffer;
/* Buffer renderer state. */
gboolean buffer_held;
@ -301,10 +294,6 @@ gboolean meta_wayland_surface_assign_role (MetaWaylandSurface *surfac
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);

View File

@ -26,6 +26,7 @@
#include <glib-unix.h>
#include "wayland/meta-wayland.h"
#include "wayland/meta-wayland-buffer.h"
#include "wayland/meta-wayland-dma-buf.h"
#define META_WAYLAND_TRANSACTION_NONE ((void *)(uintptr_t) G_MAXSIZE)
@ -392,7 +393,14 @@ meta_wayland_transaction_ensure_entry (MetaWaylandTransaction *transaction,
static void
meta_wayland_transaction_entry_free (MetaWaylandTransactionEntry *entry)
{
g_clear_object (&entry->state);
if (entry->state)
{
if (entry->state->buffer)
meta_wayland_buffer_dec_use_count (entry->state->buffer);
g_clear_object (&entry->state);
}
g_free (entry);
}

View File

@ -783,7 +783,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 && xdg_surface_priv->first_buffer_attached)
{
meta_wayland_xdg_surface_reset (xdg_surface);
meta_wayland_actor_surface_queue_frame_callbacks (actor_surface,
@ -1230,7 +1230,7 @@ meta_wayland_xdg_popup_apply_state (MetaWaylandSurfaceRole *surface_role,
meta_window_update_placement_rule (window, &placement_rule);
}
if (!surface->buffer_ref->buffer && xdg_surface_priv->first_buffer_attached)
if (!surface->buffer && xdg_surface_priv->first_buffer_attached)
{
meta_wayland_xdg_surface_reset (xdg_surface);
meta_wayland_actor_surface_queue_frame_callbacks (actor_surface, pending);
@ -1241,7 +1241,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)
{
wl_resource_post_error (xdg_popup->resource,
XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,
@ -1273,7 +1273,7 @@ meta_wayland_xdg_popup_post_apply_state (MetaWaylandSurfaceRole *surface_role,
if (!window)
return;
if (!surface->buffer_ref->buffer)
if (!surface->buffer)
return;
surface_role_class->post_apply_state (surface_role, pending);
@ -1693,7 +1693,7 @@ meta_wayland_xdg_surface_apply_state (MetaWaylandSurfaceRole *surface_role,
if (!window)
return;
if (surface->buffer_ref->buffer)
if (surface->buffer)
priv->first_buffer_attached = TRUE;
}
@ -1755,7 +1755,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)
{
wl_resource_post_error (xdg_wm_base_resource,
XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,
@ -2460,7 +2460,7 @@ xdg_wm_base_get_xdg_surface (struct wl_client *client,
return;
}
if (surface->buffer_ref->buffer)
if (surface->buffer)
{
wl_resource_post_error (resource,
XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE,

View File

@ -152,7 +152,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 &&
xwayland_surface->window)
meta_window_queue (xwayland_surface->window, META_QUEUE_CALC_SHOWING);
}