From 0556aa0c5918b90118da8abdf572b27a44e1b51d Mon Sep 17 00:00:00 2001
From: Simon Ser <contact@emersion.fr>
Date: Sun, 18 Sep 2022 16:29:36 +0200
Subject: [PATCH] output: rejigger attach/clear for back buffer

In wlr_output_attach_render(), stop setting
wlr_output.pending.buffer. This removes one footgun: using the
wlr_buffer at that stage is invalid, because rendering operations
haven't been flushed to the GPU yet. We need to wait until
output_clear_back_buffer() for the wlr_buffer to be used safely.

Instead, set wlr_output.pending.buffer in wlr_output_test() and
wlr_output_commit().

Additionally, move the output_clear_back_buffer() from
wlr_output_commit_state() to wlr_output_commit(). This reduces the
number of calls in the failure path.
---
 types/output/output.c | 37 +++++++++++++++++++++----------------
 types/output/render.c |  7 +------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/types/output/output.c b/types/output/output.c
index 26122f93..14c7844a 100644
--- a/types/output/output.c
+++ b/types/output/output.c
@@ -700,7 +700,15 @@ bool wlr_output_test_state(struct wlr_output *output,
 }
 
 bool wlr_output_test(struct wlr_output *output) {
-	return wlr_output_test_state(output, &output->pending);
+	struct wlr_output_state state = output->pending;
+
+	if (output->back_buffer != NULL) {
+		assert((state.committed & WLR_OUTPUT_STATE_BUFFER) == 0);
+		state.committed |= WLR_OUTPUT_STATE_BUFFER;
+		state.buffer = output->back_buffer;
+	}
+
+	return wlr_output_test_state(output, &state);
 }
 
 bool wlr_output_commit_state(struct wlr_output *output,
@@ -714,21 +722,17 @@ bool wlr_output_commit_state(struct wlr_output *output,
 
 	if (!output_basic_test(output, &pending)) {
 		wlr_log(WLR_ERROR, "Basic output test failed for %s", output->name);
-		output_clear_back_buffer(output);
 		return false;
 	}
 
 	bool new_back_buffer = false;
 	if (!output_ensure_buffer(output, &pending, &new_back_buffer)) {
-		output_clear_back_buffer(output);
 		return false;
 	}
 	if (new_back_buffer) {
 		assert((pending.committed & WLR_OUTPUT_STATE_BUFFER) == 0);
-		pending.committed |= WLR_OUTPUT_STATE_BUFFER;
-		// Lock the buffer to ensure it stays valid past the
-		// output_clear_back_buffer() call below.
-		pending.buffer = wlr_buffer_lock(output->back_buffer);
+		output_state_attach_buffer(&pending, output->back_buffer);
+		output_clear_back_buffer(output);
 	}
 
 	if ((pending.committed & WLR_OUTPUT_STATE_BUFFER) &&
@@ -747,15 +751,6 @@ bool wlr_output_commit_state(struct wlr_output *output,
 	};
 	wl_signal_emit_mutable(&output->events.precommit, &pre_event);
 
-	// output_clear_back_buffer detaches the buffer from the renderer. This is
-	// important to do before calling impl->commit(), because this marks an
-	// implicit rendering synchronization point. The backend needs it to avoid
-	// displaying a buffer when asynchronous GPU work isn't finished.
-	if ((pending.committed & WLR_OUTPUT_STATE_BUFFER) &&
-			output->back_buffer != NULL) {
-		output_clear_back_buffer(output);
-	}
-
 	if (!output->impl->commit(output, &pending)) {
 		if (new_back_buffer) {
 			wlr_buffer_unlock(pending.buffer);
@@ -845,6 +840,16 @@ bool wlr_output_commit(struct wlr_output *output) {
 	// Make sure the pending state is cleared before the output is committed
 	struct wlr_output_state state = {0};
 	output_state_move(&state, &output->pending);
+
+	// output_clear_back_buffer detaches the buffer from the renderer. This is
+	// important to do before calling impl->commit(), because this marks an
+	// implicit rendering synchronization point. The backend needs it to avoid
+	// displaying a buffer when asynchronous GPU work isn't finished.
+	if (output->back_buffer != NULL) {
+		output_state_attach_buffer(&state, output->back_buffer);
+		output_clear_back_buffer(output);
+	}
+
 	bool ok = wlr_output_commit_state(output, &state);
 	output_state_finish(&state);
 	return ok;
diff --git a/types/output/render.c b/types/output/render.c
index 0926a94b..b36c145e 100644
--- a/types/output/render.c
+++ b/types/output/render.c
@@ -142,12 +142,7 @@ void output_clear_back_buffer(struct wlr_output *output) {
 }
 
 bool wlr_output_attach_render(struct wlr_output *output, int *buffer_age) {
-	struct wlr_output_state *state = &output->pending;
-	if (!output_attach_back_buffer(output, state, buffer_age)) {
-		return false;
-	}
-	output_state_attach_buffer(state, output->back_buffer);
-	return true;
+	return output_attach_back_buffer(output, &output->pending, buffer_age);
 }
 
 static bool output_attach_empty_back_buffer(struct wlr_output *output,