output: re-introduce atomic mode, enabled, scale and transform

This reverts commit 01f903874b and re-applies
commit ee5f98ad49.

Updates: https://github.com/swaywm/wlroots/issues/1640 (Atomic output updates issue)
See also: https://github.com/swaywm/wlroots/pull/1762 (Atomic output updates original PR)
See also: https://github.com/swaywm/wlroots/issues/1780 (Issue caused by atomic output updates)
See also: https://github.com/swaywm/sway/issues/4419 (Issue caused by atomic output updates)
See also: https://github.com/swaywm/wlroots/pull/1781 (Revert PR)

backend/drm/drm.c

@@ -334,12 +334,9 @@ static bool drm_connector_attach_render(struct wlr_output *output,
 	return make_drm_surface_current(&conn->crtc->primary->surf, buffer_age);
 }
 
-static bool drm_connector_commit(struct wlr_output *output) {
+static bool drm_connector_commit_buffer(struct wlr_output *output) {
 	struct wlr_drm_connector *conn = get_drm_connector_from_output(output);
 	struct wlr_drm_backend *drm = get_drm_backend_from_backend(output->backend);
-	if (!drm->session->active) {
-		return false;
-	}
 
 	struct wlr_drm_crtc *crtc = conn->crtc;
 	if (!crtc) {
@@ -415,6 +412,49 @@ static bool drm_connector_commit(struct wlr_output *output) {
 	return true;
 }
 
+static bool drm_connector_set_custom_mode(struct wlr_output *output,
+	int32_t width, int32_t height, int32_t refresh);
+
+static bool drm_connector_commit(struct wlr_output *output) {
+	struct wlr_drm_backend *drm = get_drm_backend_from_backend(output->backend);
+
+	if (!drm->session->active) {
+		return false;
+	}
+
+	if (output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
+		if (!enable_drm_connector(output, output->pending.enabled)) {
+			return false;
+		}
+	}
+
+	if (output->pending.committed & WLR_OUTPUT_STATE_MODE) {
+		switch (output->pending.mode_type) {
+		case WLR_OUTPUT_STATE_MODE_FIXED:
+			if (!drm_connector_set_mode(output, output->pending.mode)) {
+				return false;
+			}
+			break;
+		case WLR_OUTPUT_STATE_MODE_CUSTOM:
+			if (!drm_connector_set_custom_mode(output,
+					output->pending.custom_mode.width,
+					output->pending.custom_mode.height,
+					output->pending.custom_mode.refresh)) {
+				return false;
+			}
+			break;
+		}
+	}
+
+	if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
+		if (!drm_connector_commit_buffer(output)) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 static void fill_empty_gamma_table(size_t size,
 		uint16_t *r, uint16_t *g, uint16_t *b) {
 	for (uint32_t i = 0; i < size; ++i) {
@@ -1002,9 +1042,6 @@ static void drm_connector_destroy(struct wlr_output *output) {
 }
 
 static const struct wlr_output_impl output_impl = {
-	.enable = enable_drm_connector,
-	.set_mode = drm_connector_set_mode,
-	.set_custom_mode = drm_connector_set_custom_mode,
 	.set_cursor = drm_connector_set_cursor,
 	.move_cursor = drm_connector_move_cursor,
 	.destroy = drm_connector_destroy,

backend/headless/output.c

@@ -58,8 +58,26 @@ static bool output_attach_render(struct wlr_output *wlr_output,
 }
 
 static bool output_commit(struct wlr_output *wlr_output) {
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
+		wlr_log(WLR_DEBUG, "Cannot disable a headless output");
+		return false;
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_MODE) {
+		assert(wlr_output->pending.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM);
+		if (!output_set_custom_mode(wlr_output,
+				wlr_output->pending.custom_mode.width,
+				wlr_output->pending.custom_mode.height,
+				wlr_output->pending.custom_mode.refresh)) {
+			return false;
+		}
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		// Nothing needs to be done for pbuffers
 		wlr_output_send_present(wlr_output, NULL);
+	}
+
 	return true;
 }
 
@@ -76,7 +94,6 @@ static void output_destroy(struct wlr_output *wlr_output) {
 }
 
 static const struct wlr_output_impl output_impl = {
-	.set_custom_mode = output_set_custom_mode,
 	.destroy = output_destroy,
 	.attach_render = output_attach_render,
 	.commit = output_commit,

backend/noop/output.c

@@ -12,21 +12,32 @@ static struct wlr_noop_output *noop_output_from_output(
 	return (struct wlr_noop_output *)wlr_output;
 }
 
-static bool output_set_custom_mode(struct wlr_output *wlr_output,
-		int32_t width, int32_t height, int32_t refresh) {
-	wlr_output_update_custom_mode(wlr_output, width, height, refresh);
-	return true;
-}
-
 static bool output_attach_render(struct wlr_output *wlr_output,
 		int *buffer_age) {
 	return false;
 }
 
 static bool output_commit(struct wlr_output *wlr_output) {
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
+		wlr_log(WLR_DEBUG, "Cannot disable a noop output");
 		return false;
 	}
 
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_MODE) {
+		assert(wlr_output->pending.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM);
+		wlr_output_update_custom_mode(wlr_output,
+			wlr_output->pending.custom_mode.width,
+			wlr_output->pending.custom_mode.height,
+			wlr_output->pending.custom_mode.refresh);
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
+		return false;
+	}
+
+	return true;
+}
+
 static void output_destroy(struct wlr_output *wlr_output) {
 	struct wlr_noop_output *output =
 		noop_output_from_output(wlr_output);
@@ -37,7 +48,6 @@ static void output_destroy(struct wlr_output *wlr_output) {
 }
 
 static const struct wlr_output_impl output_impl = {
-	.set_custom_mode = output_set_custom_mode,
 	.destroy = output_destroy,
 	.attach_render = output_attach_render,
 	.commit = output_commit,

backend/rdp/output.c

@@ -166,9 +166,9 @@ static bool nsc_swap_buffers(
 	return true;
 }
 
-static bool output_commit(struct wlr_output *wlr_output) {
+static bool output_commit_buffer(struct wlr_rdp_output *output) {
-	struct wlr_rdp_output *output =
+	struct wlr_output *wlr_output = &output->wlr_output;
-		rdp_output_from_output(wlr_output);
+
 	bool ret = false;
 
 	pixman_region32_t output_region;
@@ -220,6 +220,33 @@ out:
 	return ret;
 }
 
+static bool output_commit(struct wlr_output *wlr_output) {
+	struct wlr_rdp_output *output = rdp_output_from_output(wlr_output);
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
+		wlr_log(WLR_DEBUG, "Cannot disable an RDP output");
+		return false;
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_MODE) {
+		assert(wlr_output->pending.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM);
+		if (!output_set_custom_mode(wlr_output,
+				wlr_output->pending.custom_mode.width,
+				wlr_output->pending.custom_mode.height,
+				wlr_output->pending.custom_mode.refresh)) {
+			return false;
+		}
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
+		if (!output_commit_buffer(output)) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 static void output_destroy(struct wlr_output *wlr_output) {
 	struct wlr_rdp_output *output =
 		rdp_output_from_output(wlr_output);
@@ -234,7 +261,6 @@ static void output_destroy(struct wlr_output *wlr_output) {
 }
 
 static const struct wlr_output_impl output_impl = {
-	.set_custom_mode = output_set_custom_mode,
 	.destroy = output_destroy,
 	.attach_render = output_attach_render,
 	.commit = output_commit,

backend/wayland/output.c

@@ -166,19 +166,22 @@ static bool output_commit(struct wlr_output *wlr_output) {
 	struct wlr_wl_output *output =
 		get_wl_output_from_output(wlr_output);
 
-	if (output->frame_callback != NULL) {
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
-		wlr_log(WLR_ERROR, "Skipping buffer swap");
+		wlr_log(WLR_DEBUG, "Cannot disable a Wayland output");
 		return false;
 	}
 
-	output->frame_callback = wl_surface_frame(output->surface);
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_MODE) {
-	wl_callback_add_listener(output->frame_callback, &frame_listener, output);
+		assert(wlr_output->pending.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM);
-
+		if (!output_set_custom_mode(wlr_output,
-	pixman_region32_t *damage = NULL;
+				wlr_output->pending.custom_mode.width,
-	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_DAMAGE) {
+				wlr_output->pending.custom_mode.height,
-		damage = &wlr_output->pending.damage;
+				wlr_output->pending.custom_mode.refresh)) {
+			return false;
+		}
 	}
 
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		wlr_buffer_unref(output->current_buffer);
 		output->current_buffer = NULL;
 		if (output->current_wl_buffer != NULL) {
@@ -192,7 +195,11 @@ static bool output_commit(struct wlr_output *wlr_output) {
 				output->surface);
 		}
 
-	assert(wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER);
+		pixman_region32_t *damage = NULL;
+		if (wlr_output->pending.committed & WLR_OUTPUT_STATE_DAMAGE) {
+			damage = &wlr_output->pending.damage;
+		}
+
 		switch (wlr_output->pending.buffer_type) {
 		case WLR_OUTPUT_STATE_BUFFER_RENDER:
 			if (!wlr_egl_swap_buffers(&output->backend->egl,
@@ -239,6 +246,7 @@ static bool output_commit(struct wlr_output *wlr_output) {
 		} else {
 			wlr_output_send_present(wlr_output, NULL);
 		}
+	}
 
 	return true;
 }
@@ -382,7 +390,6 @@ static bool output_schedule_frame(struct wlr_output *wlr_output) {
 }
 
 static const struct wlr_output_impl output_impl = {
-	.set_custom_mode = output_set_custom_mode,
 	.destroy = output_destroy,
 	.attach_render = output_attach_render,
 	.attach_buffer  = output_attach_buffer,

backend/x11/output.c

@@ -103,6 +103,22 @@ static bool output_commit(struct wlr_output *wlr_output) {
 	struct wlr_x11_output *output = get_x11_output_from_output(wlr_output);
 	struct wlr_x11_backend *x11 = output->x11;
 
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) {
+		wlr_log(WLR_DEBUG, "Cannot disable an X11 output");
+		return false;
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_MODE) {
+		assert(wlr_output->pending.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM);
+		if (!output_set_custom_mode(wlr_output,
+				wlr_output->pending.custom_mode.width,
+				wlr_output->pending.custom_mode.height,
+				wlr_output->pending.custom_mode.refresh)) {
+			return false;
+		}
+	}
+
+	if (wlr_output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		pixman_region32_t *damage = NULL;
 		if (wlr_output->pending.committed & WLR_OUTPUT_STATE_DAMAGE) {
 			damage = &wlr_output->pending.damage;
@@ -113,11 +129,12 @@ static bool output_commit(struct wlr_output *wlr_output) {
 		}
 
 		wlr_output_send_present(wlr_output, NULL);
+	}
+
 	return true;
 }
 
 static const struct wlr_output_impl output_impl = {
-	.set_custom_mode = output_set_custom_mode,
 	.destroy = output_destroy,
 	.attach_render = output_attach_render,
 	.commit = output_commit,

include/wlr/interfaces/wlr_output.h

@@ -15,10 +15,6 @@
 #include <wlr/types/wlr_output.h>
 
 struct wlr_output_impl {
-	bool (*enable)(struct wlr_output *output, bool enable);
-	bool (*set_mode)(struct wlr_output *output, struct wlr_output_mode *mode);
-	bool (*set_custom_mode)(struct wlr_output *output, int32_t width,
-		int32_t height, int32_t refresh);
 	bool (*set_cursor)(struct wlr_output *output, struct wlr_texture *texture,
 		int32_t scale, enum wl_output_transform transform,
 		int32_t hotspot_x, int32_t hotspot_y, bool update_texture);

include/wlr/types/wlr_output.h

@@ -49,6 +49,10 @@ struct wlr_output_cursor {
 enum wlr_output_state_field {
 	WLR_OUTPUT_STATE_BUFFER = 1 << 0,
 	WLR_OUTPUT_STATE_DAMAGE = 1 << 1,
+	WLR_OUTPUT_STATE_MODE = 1 << 2,
+	WLR_OUTPUT_STATE_ENABLED = 1 << 3,
+	WLR_OUTPUT_STATE_SCALE = 1 << 4,
+	WLR_OUTPUT_STATE_TRANSFORM = 1 << 5,
 };
 
 enum wlr_output_state_buffer_type {
@@ -56,16 +60,32 @@ enum wlr_output_state_buffer_type {
 	WLR_OUTPUT_STATE_BUFFER_SCANOUT,
 };
 
+enum wlr_output_state_mode_type {
+	WLR_OUTPUT_STATE_MODE_FIXED,
+	WLR_OUTPUT_STATE_MODE_CUSTOM,
+};
+
 /**
  * Holds the double-buffered output state.
  */
 struct wlr_output_state {
 	uint32_t committed; // enum wlr_output_state_field
 	pixman_region32_t damage; // output-buffer-local coordinates
+	bool enabled;
+	float scale;
+	enum wl_output_transform transform;
 
 	// only valid if WLR_OUTPUT_STATE_BUFFER
 	enum wlr_output_state_buffer_type buffer_type;
 	struct wlr_buffer *buffer; // if WLR_OUTPUT_STATE_BUFFER_SCANOUT
+
+	// only valid if WLR_OUTPUT_STATE_MODE
+	enum wlr_output_state_mode_type mode_type;
+	struct wlr_output_mode *mode;
+	struct {
+		int32_t width, height;
+		int32_t refresh; // mHz, may be zero
+	} custom_mode;
 };
 
 struct wlr_output_impl;
@@ -191,8 +211,11 @@ struct wlr_surface;
 /**
  * Enables or disables the output. A disabled output is turned off and doesn't
  * emit `frame` events.
+ *
+ * Whether an output is enabled is double-buffered state, see
+ * `wlr_output_commit`.
  */
-bool wlr_output_enable(struct wlr_output *output, bool enable);
+void wlr_output_enable(struct wlr_output *output, bool enable);
 void wlr_output_create_global(struct wlr_output *output);
 void wlr_output_destroy_global(struct wlr_output *output);
 /**
@@ -202,17 +225,31 @@ void wlr_output_destroy_global(struct wlr_output *output);
 struct wlr_output_mode *wlr_output_preferred_mode(struct wlr_output *output);
 /**
  * Sets the output mode. Enables the output if it's currently disabled.
+ *
+ * Mode is double-buffered state, see `wlr_output_commit`.
  */
-bool wlr_output_set_mode(struct wlr_output *output,
+void wlr_output_set_mode(struct wlr_output *output,
 	struct wlr_output_mode *mode);
 /**
  * Sets a custom mode on the output. If modes are available, they are preferred.
  * Setting `refresh` to zero lets the backend pick a preferred value.
+ *
+ * Custom mode is double-buffered state, see `wlr_output_commit`.
  */
-bool wlr_output_set_custom_mode(struct wlr_output *output, int32_t width,
+void wlr_output_set_custom_mode(struct wlr_output *output, int32_t width,
 	int32_t height, int32_t refresh);
+/**
+ * Sets a transform for the output.
+ *
+ * Transform is double-buffered state, see `wlr_output_commit`.
+ */
 void wlr_output_set_transform(struct wlr_output *output,
 	enum wl_output_transform transform);
+/**
+ * Sets a scale for the output.
+ *
+ * Scale is double-buffered state, see `wlr_output_commit`.
+ */
 void wlr_output_set_scale(struct wlr_output *output, float scale);
 void wlr_output_set_subpixel(struct wlr_output *output,
 	enum wl_output_subpixel subpixel);
@@ -271,9 +308,10 @@ void wlr_output_set_damage(struct wlr_output *output,
 	pixman_region32_t *damage);
 /**
  * Commit the pending output state. If `wlr_output_attach_render` has been
- * called, the pending frame will be submitted for display.
+ * called, the pending frame will be submitted for display and a `frame` event
+ * will be scheduled.
  *
- * This function schedules a `frame` event.
+ * On failure, the pending changes are rolled back.
  */
 bool wlr_output_commit(struct wlr_output *output);
 /**

types/wlr_output.c

@@ -154,38 +154,50 @@ static void output_update_matrix(struct wlr_output *output) {
 		output->height, output->transform);
 }
 
-bool wlr_output_enable(struct wlr_output *output, bool enable) {
+void wlr_output_enable(struct wlr_output *output, bool enable) {
 	if (output->enabled == enable) {
-		return true;
+		return;
 	}
 
-	if (output->impl->enable) {
+	output->pending.committed |= WLR_OUTPUT_STATE_ENABLED;
-		return output->impl->enable(output, enable);
+	output->pending.enabled = enable;
-	}
-	return false;
 }
 
-bool wlr_output_set_mode(struct wlr_output *output,
+static void output_state_clear_mode(struct wlr_output_state *state) {
+	if (!(state->committed & WLR_OUTPUT_STATE_MODE)) {
+		return;
+	}
+
+	state->mode = NULL;
+
+	state->committed &= ~WLR_OUTPUT_STATE_MODE;
+}
+
+void wlr_output_set_mode(struct wlr_output *output,
 		struct wlr_output_mode *mode) {
-	if (!output->impl || !output->impl->set_mode) {
-		return false;
-	}
 	if (output->current_mode == mode) {
-		return true;
+		return;
-	}
-	return output->impl->set_mode(output, mode);
 	}
 
-bool wlr_output_set_custom_mode(struct wlr_output *output, int32_t width,
+	output_state_clear_mode(&output->pending);
-		int32_t height, int32_t refresh) {
+	output->pending.committed |= WLR_OUTPUT_STATE_MODE;
-	if (!output->impl || !output->impl->set_custom_mode) {
+	output->pending.mode_type = WLR_OUTPUT_STATE_MODE_FIXED;
-		return false;
+	output->pending.mode = mode;
 }
+
+void wlr_output_set_custom_mode(struct wlr_output *output, int32_t width,
+		int32_t height, int32_t refresh) {
 	if (output->width == width && output->height == height &&
 			output->refresh == refresh) {
-		return true;
+		return;
 	}
-	return output->impl->set_custom_mode(output, width, height, refresh);
+
+	output_state_clear_mode(&output->pending);
+	output->pending.committed |= WLR_OUTPUT_STATE_MODE;
+	output->pending.mode_type = WLR_OUTPUT_STATE_MODE_CUSTOM;
+	output->pending.custom_mode.width = width;
+	output->pending.custom_mode.height = height;
+	output->pending.custom_mode.refresh = refresh;
 }
 
 void wlr_output_update_mode(struct wlr_output *output,
@@ -227,16 +239,8 @@ void wlr_output_set_transform(struct wlr_output *output,
 		return;
 	}
 
-	output->transform = transform;
+	output->pending.committed |= WLR_OUTPUT_STATE_TRANSFORM;
-	output_update_matrix(output);
+	output->pending.transform = transform;
-
-	struct wl_resource *resource;
-	wl_resource_for_each(resource, &output->resources) {
-		send_geometry(resource);
-	}
-	wlr_output_schedule_done(output);
-
-	wlr_signal_emit_safe(&output->events.transform, output);
 }
 
 void wlr_output_set_scale(struct wlr_output *output, float scale) {
@@ -244,15 +248,8 @@ void wlr_output_set_scale(struct wlr_output *output, float scale) {
 		return;
 	}
 
-	output->scale = scale;
+	output->pending.committed |= WLR_OUTPUT_STATE_SCALE;
-
+	output->pending.scale = scale;
-	struct wl_resource *resource;
-	wl_resource_for_each(resource, &output->resources) {
-		send_scale(resource);
-	}
-	wlr_output_schedule_done(output);
-
-	wlr_signal_emit_safe(&output->events.scale, output);
 }
 
 void wlr_output_set_subpixel(struct wlr_output *output,
@@ -478,18 +475,15 @@ static void output_state_clear(struct wlr_output_state *state) {
 }
 
 bool wlr_output_commit(struct wlr_output *output) {
+	if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		if (output->frame_pending) {
-		wlr_log(WLR_ERROR, "Tried to commit when a frame is pending");
+			wlr_log(WLR_ERROR, "Tried to commit a buffer while a frame is pending");
 			return false;
 		}
 		if (output->idle_frame != NULL) {
 			wl_event_source_remove(output->idle_frame);
 			output->idle_frame = NULL;
 		}
-
-	if (!(output->pending.committed & WLR_OUTPUT_STATE_BUFFER)) {
-		wlr_log(WLR_ERROR, "Tried to commit without attaching a buffer");
-		return false;
 	}
 
 	struct timespec now;
@@ -506,6 +500,7 @@ bool wlr_output_commit(struct wlr_output *output) {
 		return false;
 	}
 
+	if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		struct wlr_output_cursor *cursor;
 		wl_list_for_each(cursor, &output->cursors, link) {
 			if (!cursor->enabled || !cursor->visible || cursor->surface == NULL) {
@@ -513,15 +508,46 @@ bool wlr_output_commit(struct wlr_output *output) {
 			}
 			wlr_surface_send_frame_done(cursor->surface, &now);
 		}
+	}
 
 	output->commit_seq++;
 
 	wlr_signal_emit_safe(&output->events.commit, output);
 
+	bool scale_updated = output->pending.committed & WLR_OUTPUT_STATE_SCALE;
+	if (scale_updated) {
+		output->scale = output->pending.scale;
+		wlr_signal_emit_safe(&output->events.scale, output);
+	}
+
+	if (output->pending.committed & WLR_OUTPUT_STATE_TRANSFORM) {
+		output->transform = output->pending.transform;
+		output_update_matrix(output);
+		wlr_signal_emit_safe(&output->events.transform, output);
+	}
+
+	bool geometry_updated = output->pending.committed &
+		(WLR_OUTPUT_STATE_MODE | WLR_OUTPUT_STATE_TRANSFORM);
+	if (geometry_updated || scale_updated) {
+		struct wl_resource *resource;
+		wl_resource_for_each(resource, &output->resources) {
+			if (geometry_updated) {
+				send_geometry(resource);
+			}
+			if (scale_updated) {
+				send_scale(resource);
+			}
+		}
+		wlr_output_schedule_done(output);
+	}
+
+	if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) {
 		output->frame_pending = true;
 		output->needs_frame = false;
-	output_state_clear(&output->pending);
 		pixman_region32_clear(&output->damage);
+	}
+
+	output_state_clear(&output->pending);
 	return true;
 }