subcompositor: sync subsurface state flow with parent

include/wlr/types/wlr_subcompositor.h

@@ -12,6 +12,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <wayland-server-core.h>
+#include <wlr/types/wlr_compositor.h>
 
 struct wlr_surface;
 
@@ -23,6 +24,8 @@ struct wlr_surface;
 struct wlr_subsurface_parent_state {
 	int32_t x, y;
 	struct wl_list link;
+
+	struct wlr_surface_synced_state synced_state;
 };
 
 struct wlr_subsurface {
@@ -40,9 +43,10 @@ struct wlr_subsurface {
 	bool mapped;
 	bool added;
 
+	struct wlr_surface_synced parent_synced;
+
 	struct wl_listener surface_destroy;
 	struct wl_listener surface_client_commit;
-	struct wl_listener parent_destroy;
 
 	struct {
 		struct wl_signal destroy;
@@ -51,6 +55,12 @@ struct wlr_subsurface {
 	} events;
 
 	void *data;
+
+	// private state
+
+	struct {
+		int32_t x, y;
+	} previous;
 };
 
 struct wlr_subcompositor {

types/wlr_compositor.c

@@ -335,7 +335,19 @@ static void surface_update_input_region(struct wlr_surface *surface) {
 		0, 0, surface->current.width, surface->current.height);
 }
 
-static void subsurface_parent_commit(struct wlr_subsurface *subsurface);
+static void subsurface_parent_commit_notify(struct wlr_subsurface *subsurface) {
+	if (subsurface->synchronized && subsurface->has_cache) {
+		wlr_surface_unlock_cached(subsurface->surface,
+			subsurface->cached_seq);
+		subsurface->has_cache = false;
+	}
+
+	if (!subsurface->added) {
+		subsurface->added = true;
+		wlr_signal_emit_safe(&subsurface->parent->events.new_subsurface,
+			subsurface);
+	}
+}
 
 static void surface_precommit(struct wlr_surface *surface,
 		struct wlr_surface_state *next) {
@@ -390,23 +402,14 @@ static void surface_commit(struct wlr_surface *surface) {
 	surface_update_opaque_region(surface);
 	surface_update_input_region(surface);
 
-	// commit subsurface order
 	struct wlr_subsurface *subsurface;
 	wl_list_for_each_reverse(subsurface, &surface->pending.subsurfaces_above,
 			pending.link) {
-		wl_list_remove(&subsurface->current.link);
+		subsurface_parent_commit_notify(subsurface);
-		wl_list_insert(&surface->current.subsurfaces_above,
-			&subsurface->current.link);
-
-		subsurface_parent_commit(subsurface);
 	}
 	wl_list_for_each_reverse(subsurface, &surface->pending.subsurfaces_below,
 			pending.link) {
-		wl_list_remove(&subsurface->current.link);
+		subsurface_parent_commit_notify(subsurface);
-		wl_list_insert(&surface->current.subsurfaces_below,
-			&subsurface->current.link);
-
-		subsurface_parent_commit(subsurface);
 	}
 
 	if (surface->role && surface->role->commit) {
@@ -416,47 +419,6 @@ static void surface_commit(struct wlr_surface *surface) {
 	wlr_signal_emit_safe(&surface->events.commit, surface);
 }
 
-static void collect_subsurface_damage_iter(struct wlr_surface *surface,
-		int sx, int sy, void *data) {
-	struct wlr_subsurface *subsurface = data;
-	pixman_region32_t *damage = &subsurface->parent->external_damage;
-	pixman_region32_union_rect(damage, damage,
-		subsurface->current.x + sx,
-		subsurface->current.y + sy,
-		surface->current.width, surface->current.height);
-}
-
-// TODO: untangle from wlr_surface
-static void subsurface_parent_commit(struct wlr_subsurface *subsurface) {
-	struct wlr_surface *surface = subsurface->surface;
-
-	bool moved = subsurface->current.x != subsurface->pending.x ||
-		subsurface->current.y != subsurface->pending.y;
-	if (subsurface->mapped && moved) {
-		wlr_surface_for_each_surface(surface,
-			collect_subsurface_damage_iter, subsurface);
-	}
-
-	if (subsurface->synchronized && subsurface->has_cache) {
-		wlr_surface_unlock_cached(surface, subsurface->cached_seq);
-		subsurface->has_cache = false;
-	}
-
-	subsurface->current.x = subsurface->pending.x;
-	subsurface->current.y = subsurface->pending.y;
-	if (subsurface->mapped && (moved || subsurface->reordered)) {
-		subsurface->reordered = false;
-		wlr_surface_for_each_surface(surface,
-			collect_subsurface_damage_iter, subsurface);
-	}
-
-	if (!subsurface->added) {
-		subsurface->added = true;
-		wlr_signal_emit_safe(&subsurface->parent->events.new_subsurface,
-			subsurface);
-	}
-}
-
 /**
  * "Squash" a state by merging it into the state right before it in the queue
  * (wlr_surface_state.link.prev) and clearing the given state. The previous
@@ -525,6 +487,25 @@ static void surface_squash_state(struct wlr_surface *surface,
 		wl_list_init(&src->frame_callback_list);
 	}
 
+	// Squash subsurface order
+	struct wlr_subsurface_parent_state *sub_state_dst, *sub_state_src;
+	wl_list_for_each_reverse(sub_state_src, &src->subsurfaces_above, link) {
+		sub_state_dst = wl_container_of(
+			sub_state_src->synced_state.state_link.prev,
+			sub_state_dst, synced_state.state_link);
+		wl_list_remove(&sub_state_dst->link);
+		wl_list_insert(&dst->subsurfaces_above,
+			&sub_state_dst->link);
+	}
+	wl_list_for_each_reverse(sub_state_src, &src->subsurfaces_below, link) {
+		sub_state_dst = wl_container_of(
+			sub_state_src->synced_state.state_link.prev,
+			sub_state_dst, synced_state.state_link);
+		wl_list_remove(&sub_state_dst->link);
+		wl_list_insert(&dst->subsurfaces_below,
+			&sub_state_dst->link);
+	}
+
 	dst->committed |= src->committed;
 	src->committed = 0;
 

types/wlr_subcompositor.c

@@ -8,6 +8,16 @@
 
 #define SUBCOMPOSITOR_VERSION 1
 
+static void collect_subsurface_damage_iter(struct wlr_surface *surface,
+		int sx, int sy, void *data) {
+	struct wlr_subsurface *subsurface = data;
+	pixman_region32_t *damage = &subsurface->parent->external_damage;
+	pixman_region32_union_rect(damage, damage,
+		subsurface->current.x + sx,
+		subsurface->current.y + sy,
+		surface->current.width, surface->current.height);
+}
+
 static bool subsurface_is_synchronized(struct wlr_subsurface *subsurface) {
 	while (subsurface != NULL) {
 		if (subsurface->synchronized) {
@@ -43,7 +53,8 @@ static void subsurface_destroy(struct wlr_subsurface *subsurface) {
 	wl_list_remove(&subsurface->surface_client_commit.link);
 	wl_list_remove(&subsurface->current.link);
 	wl_list_remove(&subsurface->pending.link);
-	wl_list_remove(&subsurface->parent_destroy.link);
+
+	wlr_surface_synced_finish(&subsurface->parent_synced);
 
 	wl_resource_set_user_data(subsurface->resource, NULL);
 	if (subsurface->surface) {
@@ -52,6 +63,72 @@ static void subsurface_destroy(struct wlr_subsurface *subsurface) {
 	free(subsurface);
 }
 
+static void subsurface_parent_synced_destroy(struct wlr_surface_synced *synced) {
+	struct wlr_subsurface *subsurface =
+		wl_container_of(synced, subsurface, parent_synced);
+	// Once the parent is destroyed, the client has no way to use the
+	// wl_subsurface object anymore, so we can destroy it.
+	subsurface_destroy(subsurface);
+}
+
+static void subsurface_parent_synced_squash_state(
+		struct wlr_surface_synced_state *synced_dst,
+		struct wlr_surface_synced_state *synced_src) {
+	struct wlr_subsurface_parent_state *dst =
+		wl_container_of(synced_dst, dst, synced_state);
+	struct wlr_subsurface_parent_state *src =
+		wl_container_of(synced_src, src, synced_state);
+
+	dst->x = src->x;
+	dst->y = src->y;
+
+	// For the sake of simplicity, copying the position in list is done
+	// by the parent itself
+}
+
+static struct wlr_surface_synced_state *subsurface_parent_synced_create_state(void) {
+	struct wlr_subsurface_parent_state *state = calloc(1, sizeof(*state));
+	if (!state) {
+		return NULL;
+	}
+	wl_list_init(&state->link);
+	return &state->synced_state;
+}
+
+static void subsurface_parent_synced_destroy_state(
+		struct wlr_surface_synced_state *synced_state) {
+	struct wlr_subsurface_parent_state *state =
+		wl_container_of(synced_state, state, synced_state);
+	wl_list_remove(&state->link);
+	free(state);
+}
+
+static void subsurface_parent_synced_precommit(struct wlr_surface_synced *synced,
+		struct wlr_surface_synced_state *synced_state) {
+	struct wlr_subsurface *subsurface =
+		wl_container_of(synced, subsurface, parent_synced);
+	struct wlr_subsurface_parent_state *state =
+		wl_container_of(synced_state, state, synced_state);
+
+	subsurface->previous.x = subsurface->current.x;
+	subsurface->previous.y = subsurface->current.y;
+
+	if (subsurface->mapped && (subsurface->current.x != state->x ||
+			subsurface->current.y != state->y)) {
+		wlr_surface_for_each_surface(subsurface->surface,
+			collect_subsurface_damage_iter, subsurface);
+	}
+}
+
+static const struct wlr_surface_synced_interface subsurface_parent_synced_impl = {
+	.name = "wlr_subsurface parent",
+	.destroy = subsurface_parent_synced_destroy,
+	.squash_state = subsurface_parent_synced_squash_state,
+	.create_state = subsurface_parent_synced_create_state,
+	.destroy_state = subsurface_parent_synced_destroy_state,
+	.precommit = subsurface_parent_synced_precommit,
+};
+
 static const struct wl_subsurface_interface subsurface_implementation;
 
 /**
@@ -272,6 +349,13 @@ static void subsurface_role_commit(struct wlr_surface *surface) {
 	}
 
 	subsurface_consider_map(subsurface, true);
+
+	if (subsurface->mapped &&
+			(subsurface->previous.x != subsurface->current.x ||
+			subsurface->previous.y != subsurface->current.y)) {
+		wlr_surface_for_each_surface(surface,
+			collect_subsurface_damage_iter, subsurface);
+	}
 }
 
 static void subsurface_role_precommit(struct wlr_surface *surface,
@@ -294,15 +378,6 @@ const struct wlr_surface_role subsurface_role = {
 	.precommit = subsurface_role_precommit,
 };
 
-static void subsurface_handle_parent_destroy(struct wl_listener *listener,
-		void *data) {
-	struct wlr_subsurface *subsurface =
-		wl_container_of(listener, subsurface, parent_destroy);
-	// Once the parent is destroyed, the client has no way to use the
-	// wl_subsurface object anymore, so we can destroy it.
-	subsurface_destroy(subsurface);
-}
-
 static void subsurface_handle_surface_destroy(struct wl_listener *listener,
 		void *data) {
 	struct wlr_subsurface *subsurface =
@@ -340,11 +415,21 @@ static struct wlr_subsurface *subsurface_create(struct wlr_surface *surface,
 		wl_client_post_no_memory(client);
 		return NULL;
 	}
+	if (!wlr_surface_synced_init(&subsurface->parent_synced,
+			&subsurface_parent_synced_impl, parent,
+			&subsurface->current.synced_state,
+			&subsurface->pending.synced_state)) {
+		free(subsurface);
+		wl_client_post_no_memory(client);
+		return NULL;
+	}
+
 	subsurface->synchronized = true;
 	subsurface->surface = surface;
 	subsurface->resource =
 		wl_resource_create(client, &wl_subsurface_interface, version, id);
 	if (subsurface->resource == NULL) {
+		wlr_surface_synced_finish(&subsurface->parent_synced);
 		free(subsurface);
 		wl_client_post_no_memory(client);
 		return NULL;
@@ -363,10 +448,7 @@ static struct wlr_subsurface *subsurface_create(struct wlr_surface *surface,
 	subsurface->surface_client_commit.notify =
 		subsurface_handle_surface_client_commit;
 
-	// link parent
 	subsurface->parent = parent;
-	wl_signal_add(&parent->events.destroy, &subsurface->parent_destroy);
-	subsurface->parent_destroy.notify = subsurface_handle_parent_destroy;
 
 	wl_list_init(&subsurface->current.link);
 	wl_list_insert(parent->pending.subsurfaces_above.prev,