surface: improve role precommit hook, add commit_request signal

include/types/wlr_xdg_shell.h

@@ -20,7 +20,8 @@ void unmap_xdg_surface(struct wlr_xdg_surface *surface);
 void reset_xdg_surface(struct wlr_xdg_surface *xdg_surface);
 void destroy_xdg_surface(struct wlr_xdg_surface *surface);
 void handle_xdg_surface_commit(struct wlr_surface *wlr_surface);
-void handle_xdg_surface_precommit(struct wlr_surface *wlr_surface);
+void handle_xdg_surface_precommit(struct wlr_surface *wlr_surface,
+		struct wlr_surface_state *state);
 
 void create_xdg_positioner(struct wlr_xdg_client *client, uint32_t id);
 struct wlr_xdg_positioner_resource *get_xdg_positioner_from_resource(

include/wlr/types/wlr_surface.h

@@ -73,7 +73,8 @@ struct wlr_surface_state {
 struct wlr_surface_role {
 	const char *name;
 	void (*commit)(struct wlr_surface *surface);
-	void (*precommit)(struct wlr_surface *surface);
+	void (*precommit)(struct wlr_surface *surface,
+		struct wlr_surface_state *state);
 };
 
 struct wlr_surface_output {
@@ -135,7 +136,10 @@ struct wlr_surface {
 	void *role_data; // role-specific data
 
 	struct {
+		// Fired on the current state update
 		struct wl_signal commit;
+		// Fired on wl_surface.commit request
+		struct wl_signal commit_request;
 		struct wl_signal new_subsurface;
 		struct wl_signal destroy;
 	} events;
@@ -182,6 +186,7 @@ struct wlr_subsurface {
 	bool mapped;
 
 	struct wl_listener surface_destroy;
+	struct wl_listener surface_commit_request;
 	struct wl_listener parent_destroy;
 
 	struct {

types/wlr_surface.c

@@ -408,6 +408,10 @@ static void surface_commit_state(struct wlr_surface *surface,
 		struct wlr_surface_state *next) {
 	assert(next->cached_state_locks == 0);
 
+	if (surface->role && surface->role->precommit) {
+		surface->role->precommit(surface, next);
+	}
+
 	bool invalid_buffer = next->committed & WLR_SURFACE_STATE_BUFFER;
 
 	surface->sx += next->dx;
@@ -511,42 +515,21 @@ static void subsurface_parent_commit(struct wlr_subsurface *subsurface,
 	}
 }
 
-static void subsurface_commit(struct wlr_subsurface *subsurface) {
-	struct wlr_surface *surface = subsurface->surface;
-
-	if (subsurface_is_synchronized(subsurface)) {
-		if (subsurface->has_cache) {
-			// We already lock a previous commit. The prevents any future
-			// commit to be applied before we release the previous commit.
-			return;
-		}
-		subsurface->has_cache = true;
-		subsurface->cached_seq = wlr_surface_lock_pending(surface);
-	}
-}
-
 static void surface_handle_commit(struct wl_client *client,
 		struct wl_resource *resource) {
 	struct wlr_surface *surface = wlr_surface_from_resource(resource);
 
-	struct wlr_subsurface *subsurface = wlr_surface_is_subsurface(surface) ?
-		wlr_subsurface_from_wlr_surface(surface) : NULL;
-	if (subsurface != NULL) {
-		subsurface_commit(subsurface);
-	}
-
 	surface_finalize_pending(surface);
 
-	if (surface->role && surface->role->precommit) {
-		surface->role->precommit(surface);
-	}
+	wlr_signal_emit_safe(&surface->events.commit_request, surface);
 
 	if (surface->pending.cached_state_locks > 0 || !wl_list_empty(&surface->cached)) {
 		surface_cache_pending(surface);
 	} else {
 		surface_commit_state(surface, &surface->pending);
 	}
 
+	struct wlr_subsurface *subsurface;
 	wl_list_for_each(subsurface, &surface->current.subsurfaces_below, current.link) {
 		subsurface_parent_commit(subsurface, false);
 	}
@@ -745,6 +728,7 @@ struct wlr_surface *surface_create(struct wl_client *client,
 	surface->pending.seq = 1;
 
 	wl_signal_init(&surface->events.commit);
+	wl_signal_init(&surface->events.commit_request);
 	wl_signal_init(&surface->events.destroy);
 	wl_signal_init(&surface->events.new_subsurface);
 	wl_list_init(&surface->current_outputs);
@@ -1094,15 +1078,15 @@ static void subsurface_role_commit(struct wlr_surface *surface) {
 	subsurface_consider_map(subsurface, true);
 }
 
-static void subsurface_role_precommit(struct wlr_surface *surface) {
+static void subsurface_role_precommit(struct wlr_surface *surface,
+		struct wlr_surface_state *state) {
 	struct wlr_subsurface *subsurface =
 		wlr_subsurface_from_wlr_surface(surface);
 	if (subsurface == NULL) {
 		return;
 	}
 
-	if (surface->pending.committed & WLR_SURFACE_STATE_BUFFER &&
-			surface->pending.buffer == NULL) {
+	if (state->committed & WLR_SURFACE_STATE_BUFFER && state->buffer == NULL) {
 		// This is a NULL commit
 		subsurface_unmap(subsurface);
 	}
@@ -1132,6 +1116,23 @@ static void subsurface_handle_surface_destroy(struct wl_listener *listener,
 	subsurface_destroy(subsurface);
 }
 
+static void subsurface_handle_surface_commit_request(
+		struct wl_listener *listener, void *data) {
+	struct wlr_subsurface *subsurface =
+		wl_container_of(listener, subsurface, surface_commit_request);
+	struct wlr_surface *surface = data;
+
+	if (subsurface_is_synchronized(subsurface)) {
+		if (subsurface->has_cache) {
+			// We already lock a previous commit. The prevents any future
+			// commit to be applied before we release the previous commit.
+			return;
+		}
+		subsurface->has_cache = true;
+		subsurface->cached_seq = wlr_surface_lock_pending(surface);
+	}
+}
+
 struct wlr_subsurface *subsurface_create(struct wlr_surface *surface,
 		struct wlr_surface *parent, uint32_t version, uint32_t id) {
 	struct wl_client *client = wl_resource_get_client(surface->resource);
@@ -1160,6 +1161,10 @@ struct wlr_subsurface *subsurface_create(struct wlr_surface *surface,
 
 	wl_signal_add(&surface->events.destroy, &subsurface->surface_destroy);
 	subsurface->surface_destroy.notify = subsurface_handle_surface_destroy;
+	wl_signal_add(&surface->events.commit_request,
+		&subsurface->surface_commit_request);
+	subsurface->surface_commit_request.notify =
+		subsurface_handle_surface_commit_request;
 
 	// link parent
 	subsurface->parent = parent;

types/xdg_shell/wlr_xdg_surface.c

@@ -342,15 +342,15 @@ void handle_xdg_surface_commit(struct wlr_surface *wlr_surface) {
 	}
 }
 
-void handle_xdg_surface_precommit(struct wlr_surface *wlr_surface) {
+void handle_xdg_surface_precommit(struct wlr_surface *wlr_surface,
+		struct wlr_surface_state *state) {
 	struct wlr_xdg_surface *surface =
 		wlr_xdg_surface_from_wlr_surface(wlr_surface);
 	if (surface == NULL) {
 		return;
 	}
 
-	if (wlr_surface->pending.committed & WLR_SURFACE_STATE_BUFFER &&
-			wlr_surface->pending.buffer == NULL) {
+	if (state->committed & WLR_SURFACE_STATE_BUFFER && state->buffer == NULL) {
 		// This is a NULL commit
 		if (surface->configured && surface->mapped) {
 			unmap_xdg_surface(surface);

xwayland/xwm.c

@@ -852,15 +852,15 @@ static void xwayland_surface_role_commit(struct wlr_surface *wlr_surface) {
 	}
 }
 
-static void xwayland_surface_role_precommit(struct wlr_surface *wlr_surface) {
+static void xwayland_surface_role_precommit(struct wlr_surface *wlr_surface,
+		struct wlr_surface_state *state) {
 	assert(wlr_surface->role == &xwayland_surface_role);
 	struct wlr_xwayland_surface *surface = wlr_surface->role_data;
 	if (surface == NULL) {
 		return;
 	}
 
-	if (wlr_surface->pending.committed & WLR_SURFACE_STATE_BUFFER &&
-			wlr_surface->pending.buffer == NULL) {
+	if (state->committed & WLR_SURFACE_STATE_BUFFER && state->buffer == NULL) {
 		// This is a NULL commit
 		if (surface->mapped) {
 			wlr_signal_emit_safe(&surface->events.unmap, surface);