diff options
author | Kenny Levinsen <kl@kl.wtf> | 2021-02-12 21:27:28 +0100 |
---|---|---|
committer | Simon Ser <contact@emersion.fr> | 2021-02-16 00:18:26 +0100 |
commit | c8bf84c82d82e332c1bb83ff27c3df87576c892e (patch) | |
tree | cff03b4c167b30b9725e56654582f220dfc80c8f | |
parent | Align ordering of core node properties with i3 (diff) | |
download | sway-c8bf84c82d82e332c1bb83ff27c3df87576c892e.tar.gz sway-c8bf84c82d82e332c1bb83ff27c3df87576c892e.tar.zst sway-c8bf84c82d82e332c1bb83ff27c3df87576c892e.zip |
transactions: Amend pending transactions
The transaction system contains a necessary optimization where a popped
transaction is combined with later, similar transactions. This breaks
the chronological order of states, and can lead to desynchronized
geometries.
To fix this, we replace the queue with only 2 transactions: current and
pending. If a pending transaction exists, it is updated with new state
instead of creating additional transactions.
As we never have more than a single waiting transaction, we no longer
need the queue optimization that is causing problems.
Closes: https://github.com/swaywm/sway/issues/6012
-rw-r--r-- | include/sway/server.h | 18 | ||||
-rw-r--r-- | sway/desktop/transaction.c | 158 | ||||
-rw-r--r-- | sway/server.c | 2 |
3 files changed, 93 insertions, 85 deletions
diff --git a/include/sway/server.h b/include/sway/server.h index 0f5e3ab2..5a2562b3 100644 --- a/include/sway/server.h +++ b/include/sway/server.h | |||
@@ -23,6 +23,8 @@ | |||
23 | #include "sway/xwayland.h" | 23 | #include "sway/xwayland.h" |
24 | #endif | 24 | #endif |
25 | 25 | ||
26 | struct sway_transaction; | ||
27 | |||
26 | struct sway_server { | 28 | struct sway_server { |
27 | struct wl_display *wl_display; | 29 | struct wl_display *wl_display; |
28 | struct wl_event_loop *wl_event_loop; | 30 | struct wl_event_loop *wl_event_loop; |
@@ -85,8 +87,22 @@ struct sway_server { | |||
85 | struct wlr_text_input_manager_v3 *text_input; | 87 | struct wlr_text_input_manager_v3 *text_input; |
86 | struct wlr_foreign_toplevel_manager_v1 *foreign_toplevel_manager; | 88 | struct wlr_foreign_toplevel_manager_v1 *foreign_toplevel_manager; |
87 | 89 | ||
90 | // The timeout for transactions, after which a transaction is applied | ||
91 | // regardless of readiness. | ||
88 | size_t txn_timeout_ms; | 92 | size_t txn_timeout_ms; |
89 | list_t *transactions; | 93 | |
94 | // Stores a transaction after it has been committed, but is waiting for | ||
95 | // views to ack the new dimensions before being applied. A queued | ||
96 | // transaction is frozen and must not have new instructions added to it. | ||
97 | struct sway_transaction *queued_transaction; | ||
98 | |||
99 | // Stores a pending transaction that will be committed once the existing | ||
100 | // queued transaction is applied and freed. The pending transaction can be | ||
101 | // updated with new instructions as needed. | ||
102 | struct sway_transaction *pending_transaction; | ||
103 | |||
104 | // Stores the nodes that have been marked as "dirty" and will be put into | ||
105 | // the pending transaction. | ||
90 | list_t *dirty_nodes; | 106 | list_t *dirty_nodes; |
91 | }; | 107 | }; |
92 | 108 | ||
diff --git a/sway/desktop/transaction.c b/sway/desktop/transaction.c index bea9d539..9f488963 100644 --- a/sway/desktop/transaction.c +++ b/sway/desktop/transaction.c | |||
@@ -87,7 +87,11 @@ static void transaction_destroy(struct sway_transaction *transaction) { | |||
87 | static void copy_output_state(struct sway_output *output, | 87 | static void copy_output_state(struct sway_output *output, |
88 | struct sway_transaction_instruction *instruction) { | 88 | struct sway_transaction_instruction *instruction) { |
89 | struct sway_output_state *state = &instruction->output_state; | 89 | struct sway_output_state *state = &instruction->output_state; |
90 | state->workspaces = create_list(); | 90 | if (state->workspaces) { |
91 | state->workspaces->length = 0; | ||
92 | } else { | ||
93 | state->workspaces = create_list(); | ||
94 | } | ||
91 | list_cat(state->workspaces, output->workspaces); | 95 | list_cat(state->workspaces, output->workspaces); |
92 | 96 | ||
93 | state->active_workspace = output_get_active_workspace(output); | 97 | state->active_workspace = output_get_active_workspace(output); |
@@ -105,8 +109,16 @@ static void copy_workspace_state(struct sway_workspace *ws, | |||
105 | state->layout = ws->layout; | 109 | state->layout = ws->layout; |
106 | 110 | ||
107 | state->output = ws->output; | 111 | state->output = ws->output; |
108 | state->floating = create_list(); | 112 | if (state->floating) { |
109 | state->tiling = create_list(); | 113 | state->floating->length = 0; |
114 | } else { | ||
115 | state->floating = create_list(); | ||
116 | } | ||
117 | if (state->tiling) { | ||
118 | state->tiling->length = 0; | ||
119 | } else { | ||
120 | state->tiling = create_list(); | ||
121 | } | ||
110 | list_cat(state->floating, ws->floating); | 122 | list_cat(state->floating, ws->floating); |
111 | list_cat(state->tiling, ws->tiling); | 123 | list_cat(state->tiling, ws->tiling); |
112 | 124 | ||
@@ -147,7 +159,11 @@ static void copy_container_state(struct sway_container *container, | |||
147 | state->content_height = container->content_height; | 159 | state->content_height = container->content_height; |
148 | 160 | ||
149 | if (!container->view) { | 161 | if (!container->view) { |
150 | state->children = create_list(); | 162 | if (state->children) { |
163 | state->children->length = 0; | ||
164 | } else { | ||
165 | state->children = create_list(); | ||
166 | } | ||
151 | list_cat(state->children, container->children); | 167 | list_cat(state->children, container->children); |
152 | } | 168 | } |
153 | 169 | ||
@@ -163,13 +179,32 @@ static void copy_container_state(struct sway_container *container, | |||
163 | 179 | ||
164 | static void transaction_add_node(struct sway_transaction *transaction, | 180 | static void transaction_add_node(struct sway_transaction *transaction, |
165 | struct sway_node *node) { | 181 | struct sway_node *node) { |
166 | struct sway_transaction_instruction *instruction = | 182 | struct sway_transaction_instruction *instruction = NULL; |
167 | calloc(1, sizeof(struct sway_transaction_instruction)); | 183 | |
168 | if (!sway_assert(instruction, "Unable to allocate instruction")) { | 184 | // Check if we have an instruction for this node already, in which case we |
169 | return; | 185 | // update that instead of creating a new one. |
186 | if (node->ntxnrefs > 0) { | ||
187 | for (int idx = 0; idx < transaction->instructions->length; idx++) { | ||
188 | struct sway_transaction_instruction *other = | ||
189 | transaction->instructions->items[idx]; | ||
190 | if (other->node == node) { | ||
191 | instruction = other; | ||
192 | break; | ||
193 | } | ||
194 | } | ||
195 | } | ||
196 | |||
197 | if (!instruction) { | ||
198 | instruction = calloc(1, sizeof(struct sway_transaction_instruction)); | ||
199 | if (!sway_assert(instruction, "Unable to allocate instruction")) { | ||
200 | return; | ||
201 | } | ||
202 | instruction->transaction = transaction; | ||
203 | instruction->node = node; | ||
204 | |||
205 | list_add(transaction->instructions, instruction); | ||
206 | node->ntxnrefs++; | ||
170 | } | 207 | } |
171 | instruction->transaction = transaction; | ||
172 | instruction->node = node; | ||
173 | 208 | ||
174 | switch (node->type) { | 209 | switch (node->type) { |
175 | case N_ROOT: | 210 | case N_ROOT: |
@@ -184,9 +219,6 @@ static void transaction_add_node(struct sway_transaction *transaction, | |||
184 | copy_container_state(node->sway_container, instruction); | 219 | copy_container_state(node->sway_container, instruction); |
185 | break; | 220 | break; |
186 | } | 221 | } |
187 | |||
188 | list_add(transaction->instructions, instruction); | ||
189 | node->ntxnrefs++; | ||
190 | } | 222 | } |
191 | 223 | ||
192 | static void apply_output_state(struct sway_output *output, | 224 | static void apply_output_state(struct sway_output *output, |
@@ -307,70 +339,25 @@ static void transaction_apply(struct sway_transaction *transaction) { | |||
307 | cursor_rebase_all(); | 339 | cursor_rebase_all(); |
308 | } | 340 | } |
309 | 341 | ||
310 | static void transaction_commit(struct sway_transaction *transaction); | 342 | static void transaction_commit_pending(void); |
311 | 343 | ||
312 | // Return true if both transactions operate on the same nodes | 344 | static void transaction_progress(void) { |
313 | static bool transaction_same_nodes(struct sway_transaction *a, | 345 | if (!server.queued_transaction) { |
314 | struct sway_transaction *b) { | ||
315 | if (a->instructions->length != b->instructions->length) { | ||
316 | return false; | ||
317 | } | ||
318 | for (int i = 0; i < a->instructions->length; ++i) { | ||
319 | struct sway_transaction_instruction *a_inst = a->instructions->items[i]; | ||
320 | struct sway_transaction_instruction *b_inst = b->instructions->items[i]; | ||
321 | if (a_inst->node != b_inst->node) { | ||
322 | return false; | ||
323 | } | ||
324 | } | ||
325 | return true; | ||
326 | } | ||
327 | |||
328 | static void transaction_progress_queue(void) { | ||
329 | if (!server.transactions->length) { | ||
330 | return; | 346 | return; |
331 | } | 347 | } |
332 | // Only the first transaction in the queue is committed, so that's the one | 348 | if (server.queued_transaction->num_waiting > 0) { |
333 | // we try to process. | ||
334 | struct sway_transaction *transaction = server.transactions->items[0]; | ||
335 | if (transaction->num_waiting) { | ||
336 | return; | 349 | return; |
337 | } | 350 | } |
338 | transaction_apply(transaction); | 351 | transaction_apply(server.queued_transaction); |
339 | transaction_destroy(transaction); | 352 | transaction_destroy(server.queued_transaction); |
340 | list_del(server.transactions, 0); | 353 | server.queued_transaction = NULL; |
341 | 354 | ||
342 | if (server.transactions->length == 0) { | 355 | if (!server.pending_transaction) { |
343 | // The transaction queue is empty, so we're done. | ||
344 | sway_idle_inhibit_v1_check_active(server.idle_inhibit_manager_v1); | 356 | sway_idle_inhibit_v1_check_active(server.idle_inhibit_manager_v1); |
345 | return; | 357 | return; |
346 | } | 358 | } |
347 | 359 | ||
348 | // If there's a bunch of consecutive transactions which all apply to the | 360 | transaction_commit_pending(); |
349 | // same views, skip all except the last one. | ||
350 | while (server.transactions->length >= 2) { | ||
351 | struct sway_transaction *txn = server.transactions->items[0]; | ||
352 | struct sway_transaction *dup = NULL; | ||
353 | |||
354 | for (int i = 1; i < server.transactions->length; i++) { | ||
355 | struct sway_transaction *maybe_dup = server.transactions->items[i]; | ||
356 | if (transaction_same_nodes(txn, maybe_dup)) { | ||
357 | dup = maybe_dup; | ||
358 | break; | ||
359 | } | ||
360 | } | ||
361 | |||
362 | if (dup) { | ||
363 | list_del(server.transactions, 0); | ||
364 | transaction_destroy(txn); | ||
365 | } else { | ||
366 | break; | ||
367 | } | ||
368 | } | ||
369 | |||
370 | // We again commit the first transaction in the queue to process it. | ||
371 | transaction = server.transactions->items[0]; | ||
372 | transaction_commit(transaction); | ||
373 | transaction_progress_queue(); | ||
374 | } | 361 | } |
375 | 362 | ||
376 | static int handle_timeout(void *data) { | 363 | static int handle_timeout(void *data) { |
@@ -378,7 +365,7 @@ static int handle_timeout(void *data) { | |||
378 | sway_log(SWAY_DEBUG, "Transaction %p timed out (%zi waiting)", | 365 | sway_log(SWAY_DEBUG, "Transaction %p timed out (%zi waiting)", |
379 | transaction, transaction->num_waiting); | 366 | transaction, transaction->num_waiting); |
380 | transaction->num_waiting = 0; | 367 | transaction->num_waiting = 0; |
381 | transaction_progress_queue(); | 368 | transaction_progress(); |
382 | return 0; | 369 | return 0; |
383 | } | 370 | } |
384 | 371 | ||
@@ -479,6 +466,17 @@ static void transaction_commit(struct sway_transaction *transaction) { | |||
479 | } | 466 | } |
480 | } | 467 | } |
481 | 468 | ||
469 | static void transaction_commit_pending(void) { | ||
470 | if (server.queued_transaction) { | ||
471 | return; | ||
472 | } | ||
473 | struct sway_transaction *transaction = server.pending_transaction; | ||
474 | server.pending_transaction = NULL; | ||
475 | server.queued_transaction = transaction; | ||
476 | transaction_commit(transaction); | ||
477 | transaction_progress(); | ||
478 | } | ||
479 | |||
482 | static void set_instruction_ready( | 480 | static void set_instruction_ready( |
483 | struct sway_transaction_instruction *instruction) { | 481 | struct sway_transaction_instruction *instruction) { |
484 | struct sway_transaction *transaction = instruction->transaction; | 482 | struct sway_transaction *transaction = instruction->transaction; |
@@ -504,7 +502,7 @@ static void set_instruction_ready( | |||
504 | } | 502 | } |
505 | 503 | ||
506 | instruction->node->instruction = NULL; | 504 | instruction->node->instruction = NULL; |
507 | transaction_progress_queue(); | 505 | transaction_progress(); |
508 | } | 506 | } |
509 | 507 | ||
510 | void transaction_notify_view_ready_by_serial(struct sway_view *view, | 508 | void transaction_notify_view_ready_by_serial(struct sway_view *view, |
@@ -541,24 +539,20 @@ void transaction_commit_dirty(void) { | |||
541 | if (!server.dirty_nodes->length) { | 539 | if (!server.dirty_nodes->length) { |
542 | return; | 540 | return; |
543 | } | 541 | } |
544 | struct sway_transaction *transaction = transaction_create(); | 542 | |
545 | if (!transaction) { | 543 | if (!server.pending_transaction) { |
546 | return; | 544 | server.pending_transaction = transaction_create(); |
545 | if (!server.pending_transaction) { | ||
546 | return; | ||
547 | } | ||
547 | } | 548 | } |
549 | |||
548 | for (int i = 0; i < server.dirty_nodes->length; ++i) { | 550 | for (int i = 0; i < server.dirty_nodes->length; ++i) { |
549 | struct sway_node *node = server.dirty_nodes->items[i]; | 551 | struct sway_node *node = server.dirty_nodes->items[i]; |
550 | transaction_add_node(transaction, node); | 552 | transaction_add_node(server.pending_transaction, node); |
551 | node->dirty = false; | 553 | node->dirty = false; |
552 | } | 554 | } |
553 | server.dirty_nodes->length = 0; | 555 | server.dirty_nodes->length = 0; |
554 | 556 | ||
555 | list_add(server.transactions, transaction); | 557 | transaction_commit_pending(); |
556 | |||
557 | // We only commit the first transaction added to the queue. | ||
558 | if (server.transactions->length == 1) { | ||
559 | transaction_commit(transaction); | ||
560 | // Attempting to progress the queue here is useful | ||
561 | // if the transaction has nothing to wait for. | ||
562 | transaction_progress_queue(); | ||
563 | } | ||
564 | } | 558 | } |
diff --git a/sway/server.c b/sway/server.c index f180da9a..278afd19 100644 --- a/sway/server.c +++ b/sway/server.c | |||
@@ -199,7 +199,6 @@ bool server_init(struct sway_server *server) { | |||
199 | } | 199 | } |
200 | 200 | ||
201 | server->dirty_nodes = create_list(); | 201 | server->dirty_nodes = create_list(); |
202 | server->transactions = create_list(); | ||
203 | 202 | ||
204 | server->input = input_manager_create(server); | 203 | server->input = input_manager_create(server); |
205 | input_manager_get_default_seat(); // create seat0 | 204 | input_manager_get_default_seat(); // create seat0 |
@@ -215,7 +214,6 @@ void server_fini(struct sway_server *server) { | |||
215 | wl_display_destroy_clients(server->wl_display); | 214 | wl_display_destroy_clients(server->wl_display); |
216 | wl_display_destroy(server->wl_display); | 215 | wl_display_destroy(server->wl_display); |
217 | list_free(server->dirty_nodes); | 216 | list_free(server->dirty_nodes); |
218 | list_free(server->transactions); | ||
219 | } | 217 | } |
220 | 218 | ||
221 | bool server_start(struct sway_server *server) { | 219 | bool server_start(struct sway_server *server) { |