Merge pull request #2458 from RyanDwyer/cleanup-transactions

Simplify transactions

include/sway/desktop/transaction.h

@@ -20,6 +20,8 @@
  * create and commits a transaction from the dirty containers.
  */
 
+struct sway_transaction_instruction;
+
 /**
  * Find all dirty containers, create and commit a transaction containing them,
  * and unmark them as dirty.
@@ -31,7 +33,8 @@ void transaction_commit_dirty(void);
  *
  * When all views in the transaction are ready, the layout will be applied.
  */
-void transaction_notify_view_ready(struct sway_view *view, uint32_t serial);
+void transaction_notify_view_ready_by_serial(struct sway_view *view,
+		uint32_t serial);
 
 /**
  * Notify the transaction system that a view is ready for the new layout, but

include/sway/tree/container.h

@@ -151,7 +151,12 @@ struct sway_container {
 	struct wlr_texture *title_urgent;
 	size_t title_height;
 
-	list_t *instructions; // struct sway_transaction_instruction *
+	// The number of transactions which reference this container.
+	size_t ntxnrefs;
+
+	// If this container is a view and is waiting for the client to respond to a
+	// configure then this will be populated, otherwise NULL.
+	struct sway_transaction_instruction *instruction;
 
 	bool destroying;
 

sway/desktop/transaction.c

@@ -43,12 +43,14 @@ struct sway_transaction_instruction {
 	struct sway_container *container;
 	struct sway_container_state state;
 	uint32_t serial;
-	bool ready;
 };
 
 static struct sway_transaction *transaction_create() {
 	struct sway_transaction *transaction =
 		calloc(1, sizeof(struct sway_transaction));
+	if (!sway_assert(transaction, "Unable to allocate transaction")) {
+		return NULL;
+	}
 	transaction->instructions = create_list();
 	if (server.debug_txn_timings) {
 		clock_gettime(CLOCK_MONOTONIC, &transaction->create_time);
@@ -62,13 +64,11 @@ static void transaction_destroy(struct sway_transaction *transaction) {
 		struct sway_transaction_instruction *instruction =
 			transaction->instructions->items[i];
 		struct sway_container *con = instruction->container;
-		for (int j = 0; j < con->instructions->length; ++j) {
+		con->ntxnrefs--;
-			if (con->instructions->items[j] == instruction) {
+		if (con->instruction == instruction) {
-				list_del(con->instructions, j);
+			con->instruction = NULL;
-				break;
-			}
 		}
-		if (con->destroying && !con->instructions->length) {
+		if (con->destroying && con->ntxnrefs == 0) {
 			container_free(con);
 		}
 		free(instruction);
@@ -130,12 +130,16 @@ static void transaction_add_container(struct sway_transaction *transaction,
 		struct sway_container *container) {
 	struct sway_transaction_instruction *instruction =
 		calloc(1, sizeof(struct sway_transaction_instruction));
+	if (!sway_assert(instruction, "Unable to allocate instruction")) {
+		return;
+	}
 	instruction->transaction = transaction;
 	instruction->container = container;
 
 	copy_pending_state(container, &instruction->state);
 
 	list_add(transaction->instructions, instruction);
+	container->ntxnrefs++;
 }
 
 /**
@@ -195,21 +199,11 @@ static void transaction_apply(struct sway_transaction *transaction) {
 		memcpy(&container->current, &instruction->state,
 				sizeof(struct sway_container_state));
 
-		if (container->type == C_VIEW) {
+		if (container->type == C_VIEW && container->sway_view->saved_buffer) {
-			if (container->destroying) {
+			view_remove_saved_buffer(container->sway_view);
-				if (container->instructions->length == 1 &&
-						container->sway_view->saved_buffer) {
-					view_remove_saved_buffer(container->sway_view);
-				}
-			} else {
-				if (container->sway_view->saved_buffer) {
-					view_remove_saved_buffer(container->sway_view);
-				}
-				if (container->instructions->length > 1) {
-					view_save_buffer(container->sway_view);
-				}
-			}
 		}
+
+		container->instruction = NULL;
 	}
 }
 
@@ -269,20 +263,8 @@ static bool should_configure(struct sway_container *con,
 	if (con->destroying) {
 		return false;
 	}
-	// The settled dimensions are what size the view will be once any pending
+	if (con->current.view_width == instruction->state.view_width &&
-	// configures have applied (excluding the one we might be configuring now).
+			con->current.view_height == instruction->state.view_height) {
-	// If these match the dimensions that this transaction wants then we don't
-	// need to configure it.
-	int settled_width = con->current.view_width;
-	int settled_height = con->current.view_height;
-	if (con->instructions->length) {
-		struct sway_transaction_instruction *last_instruction =
-			con->instructions->items[con->instructions->length - 1];
-		settled_width = last_instruction->state.view_width;
-		settled_height = last_instruction->state.view_height;
-	}
-	if (settled_width == instruction->state.view_width &&
-			settled_height == instruction->state.view_height) {
 		return false;
 	}
 	return true;
@@ -312,10 +294,10 @@ static void transaction_commit(struct sway_transaction *transaction) {
 			struct timespec when;
 			wlr_surface_send_frame_done(con->sway_view->surface, &when);
 		}
-		if (con->type == C_VIEW && !con->sway_view->saved_buffer) {
+		if (con->type == C_VIEW) {
 			view_save_buffer(con->sway_view);
 		}
-		list_add(con->instructions, instruction);
+		con->instruction = instruction;
 	}
 	transaction->num_configures = transaction->num_waiting;
 	if (server.debug_txn_timings) {
@@ -347,7 +329,6 @@ static void transaction_commit(struct sway_transaction *transaction) {
 
 static void set_instruction_ready(
 		struct sway_transaction_instruction *instruction) {
-	instruction->ready = true;
 	struct sway_transaction *transaction = instruction->transaction;
 
 	if (server.debug_txn_timings) {
@@ -371,44 +352,25 @@ static void set_instruction_ready(
 			wl_event_source_timer_update(transaction->timer, 0);
 		}
 	}
-}
 
-/**
+	instruction->container->instruction = NULL;
- * Mark all of the view's instructions as ready up to and including the
- * instruction at the given index. This allows the view to skip a configure.
- */
-static void set_instructions_ready(struct sway_view *view, int index) {
-	for (int i = 0; i <= index; ++i) {
-		struct sway_transaction_instruction *instruction =
-			view->swayc->instructions->items[i];
-		if (!instruction->ready) {
-			set_instruction_ready(instruction);
-		}
-	}
 	transaction_progress_queue();
 }
 
-void transaction_notify_view_ready(struct sway_view *view, uint32_t serial) {
+void transaction_notify_view_ready_by_serial(struct sway_view *view,
-	for (int i = 0; i < view->swayc->instructions->length; ++i) {
+		uint32_t serial) {
-		struct sway_transaction_instruction *instruction =
+	struct sway_transaction_instruction *instruction = view->swayc->instruction;
-			view->swayc->instructions->items[i];
+	if (view->swayc->instruction->serial == serial) {
-		if (instruction->serial == serial && !instruction->ready) {
+		set_instruction_ready(instruction);
-			set_instructions_ready(view, i);
-			return;
-		}
 	}
 }
 
 void transaction_notify_view_ready_by_size(struct sway_view *view,
 		int width, int height) {
-	for (int i = 0; i < view->swayc->instructions->length; ++i) {
+	struct sway_transaction_instruction *instruction = view->swayc->instruction;
-		struct sway_transaction_instruction *instruction =
+	if (instruction->state.view_width == width &&
-			view->swayc->instructions->items[i];
+			instruction->state.view_height == height) {
-		if (!instruction->ready && instruction->state.view_width == width &&
+		set_instruction_ready(instruction);
-				instruction->state.view_height == height) {
-			set_instructions_ready(view, i);
-			return;
-		}
 	}
 }
 
@@ -417,6 +379,9 @@ void transaction_commit_dirty(void) {
 		return;
 	}
 	struct sway_transaction *transaction = transaction_create();
+	if (!transaction) {
+		return;
+	}
 	for (int i = 0; i < server.dirty_containers->length; ++i) {
 		struct sway_container *container = server.dirty_containers->items[i];
 		transaction_add_container(transaction, container);

sway/desktop/xdg_shell.c

@@ -254,8 +254,9 @@ static void handle_commit(struct wl_listener *listener, void *data) {
 		return;
 	}
 
-	if (view->swayc->instructions->length) {
+	if (view->swayc->instruction) {
-		transaction_notify_view_ready(view, xdg_surface->configure_serial);
+		transaction_notify_view_ready_by_serial(view,
+				xdg_surface->configure_serial);
 	}
 
 	view_damage_from(view);

sway/desktop/xdg_shell_v6.c

@@ -250,8 +250,9 @@ static void handle_commit(struct wl_listener *listener, void *data) {
 	if (!view->swayc) {
 		return;
 	}
-	if (view->swayc->instructions->length) {
+	if (view->swayc->instruction) {
-		transaction_notify_view_ready(view, xdg_surface_v6->configure_serial);
+		transaction_notify_view_ready_by_serial(view,
+				xdg_surface_v6->configure_serial);
 	}
 
 	view_damage_from(view);

sway/desktop/xwayland.c

@@ -284,7 +284,7 @@ static void handle_commit(struct wl_listener *listener, void *data) {
 	struct wlr_xwayland_surface *xsurface = view->wlr_xwayland_surface;
 	struct wlr_surface_state *surface_state = &xsurface->surface->current;
 
-	if (view->swayc->instructions->length) {
+	if (view->swayc->instruction) {
 		transaction_notify_view_ready_by_size(view,
 				surface_state->width, surface_state->height);
 	} else if (container_is_floating(view->swayc)) {

sway/tree/container.c

@@ -108,7 +108,6 @@ struct sway_container *container_create(enum sway_container_type type) {
 	c->layout = L_NONE;
 	c->type = type;
 	c->alpha = 1.0f;
-	c->instructions = create_list();
 
 	if (type != C_VIEW) {
 		c->children = create_list();
@@ -140,8 +139,8 @@ void container_free(struct sway_container *cont) {
 				"Tried to free container which wasn't marked as destroying")) {
 		return;
 	}
-	if (!sway_assert(cont->instructions->length == 0,
+	if (!sway_assert(cont->ntxnrefs == 0, "Tried to free container "
-				"Tried to free container with pending instructions")) {
+				"which is still referenced by transactions")) {
 		return;
 	}
 	free(cont->name);
@@ -150,7 +149,6 @@ void container_free(struct sway_container *cont) {
 	wlr_texture_destroy(cont->title_focused_inactive);
 	wlr_texture_destroy(cont->title_unfocused);
 	wlr_texture_destroy(cont->title_urgent);
-	list_free(cont->instructions);
 	list_free(cont->children);
 	list_free(cont->current.children);
 

sway/tree/root.c

@@ -26,7 +26,6 @@ void root_create(void) {
 	root_container.type = C_ROOT;
 	root_container.layout = L_NONE;
 	root_container.name = strdup("root");
-	root_container.instructions = create_list();
 	root_container.children = create_list();
 	root_container.current.children = create_list();
 	wl_signal_init(&root_container.events.destroy);
@@ -55,7 +54,6 @@ void root_destroy(void) {
 	free(root_container.sway_root);
 
 	// root_container
-	list_free(root_container.instructions);
 	list_free(root_container.children);
 	list_free(root_container.current.children);
 	free(root_container.name);