output-layers: change semantics of wlr_output_state.layers

Previously, we were requiring all layers to be included in
wlr_output_state.layers and wlr_output_layer_state.buffer to be
set to NULL to disable a layer.

This commit changes these semantics: disabled layers are left out
of wlr_output_state.layers, and wlr_output_layer_state.buffer is
required to be non-NULL.

This new API should make it easier for callers to populate the
layers, at the cost of some additional complexity in backends,
mostly addressed by introducing a new
wlr_output_state_is_layer_enabled() helper.

backend/drm/drm.c

@@ -804,11 +804,7 @@ static bool drm_connector_set_pending_layer_fbs(struct wlr_drm_connector *conn,
 			return false;
 		}
 
-		if (layer_state->buffer != NULL) {
-			drm_fb_import(&layer->pending_fb, drm, layer_state->buffer, NULL);
-		} else {
-			drm_fb_clear(&layer->pending_fb);
-		}
+		drm_fb_import(&layer->pending_fb, drm, layer_state->buffer, NULL);
 	}
 
 	return true;

backend/drm/libliftoff.c

@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <wlr/interfaces/wlr_output.h>
 #include <wlr/util/box.h>
 #include <wlr/util/log.h>
 
@@ -184,23 +185,17 @@ static bool set_layer_props(struct wlr_drm_backend *drm,
 		struct wl_array *fb_damage_clips_arr) {
 	struct wlr_drm_layer *layer = get_drm_layer(drm, state->layer);
 
-	uint32_t width = 0, height = 0;
-	if (state->buffer != NULL) {
-		width = state->buffer->width;
-		height = state->buffer->height;
-	}
+	uint32_t width = state->buffer->width;
+	uint32_t height = state->buffer->height;
 
 	struct wlr_drm_fb *fb = layer->pending_fb;
-	int ret = 0;
-	if (state->buffer == NULL) {
-		ret = liftoff_layer_set_property(layer->liftoff, "FB_ID", 0);
-	} else if (fb == NULL) {
+	if (fb == NULL) {
 		liftoff_layer_set_fb_composited(layer->liftoff);
 	} else {
-		ret = liftoff_layer_set_property(layer->liftoff, "FB_ID", fb->id);
-	}
-	if (ret != 0) {
-		return false;
+		int ret = liftoff_layer_set_property(layer->liftoff, "FB_ID", fb->id);
+		if (ret != 0) {
+			return false;
+		}
 	}
 
 	uint64_t crtc_x = (uint64_t)state->dst_box.x;
@@ -364,6 +359,18 @@ static bool add_connector(drmModeAtomicReq *req,
 				ok = ok && set_layer_props(drm, layer_state, i + 1,
 					fb_damage_clips_arr);
 			}
+
+			struct wlr_output_layer *wlr_layer;
+			wl_list_for_each(wlr_layer, &conn->output.layers, link) {
+				if (wlr_output_state_is_layer_enabled(state->base, wlr_layer)) {
+					continue;
+				}
+				struct wlr_drm_layer *layer = get_drm_layer(drm, wlr_layer);
+				if (layer == NULL) {
+					continue;
+				}
+				liftoff_layer_set_property(layer->liftoff, "FB_ID", 0);
+			}
 		}
 
 		if (crtc->cursor) {

backend/wayland/output.c

@@ -498,29 +498,29 @@ static bool output_test(struct wlr_output *wlr_output,
 		bool supported = output->backend->subcompositor != NULL;
 		for (ssize_t i = state->layers_len - 1; i >= 0; i--) {
 			struct wlr_output_layer_state *layer_state = &state->layers[i];
-			if (layer_state->buffer != NULL) {
-				int x = layer_state->dst_box.x;
-				int y = layer_state->dst_box.y;
-				int width = layer_state->dst_box.width;
-				int height = layer_state->dst_box.height;
-				bool needs_viewport = width != layer_state->buffer->width ||
-					height != layer_state->buffer->height;
-				if (!wlr_fbox_empty(&layer_state->src_box)) {
-					needs_viewport = needs_viewport ||
-						layer_state->src_box.x != 0 ||
-						layer_state->src_box.y != 0 ||
-						layer_state->src_box.width != width ||
-						layer_state->src_box.height != height;
-				}
-				if (x < 0 || y < 0 ||
-						x + width > wlr_output->width ||
-						y + height > wlr_output->height ||
-						(output->backend->viewporter == NULL && needs_viewport)) {
-					supported = false;
-				}
-				supported = supported &&
-					test_buffer(output->backend, layer_state->buffer);
+
+			int x = layer_state->dst_box.x;
+			int y = layer_state->dst_box.y;
+			int width = layer_state->dst_box.width;
+			int height = layer_state->dst_box.height;
+			bool needs_viewport = width != layer_state->buffer->width ||
+				height != layer_state->buffer->height;
+			if (!wlr_fbox_empty(&layer_state->src_box)) {
+				needs_viewport = needs_viewport ||
+					layer_state->src_box.x != 0 ||
+					layer_state->src_box.y != 0 ||
+					layer_state->src_box.width != width ||
+					layer_state->src_box.height != height;
 			}
+			if (x < 0 || y < 0 ||
+					x + width > wlr_output->width ||
+					y + height > wlr_output->height ||
+					(output->backend->viewporter == NULL && needs_viewport)) {
+				supported = false;
+			}
+			supported = supported &&
+				test_buffer(output->backend, layer_state->buffer);
+
 			layer_state->accepted = supported;
 		}
 	}
@@ -584,19 +584,23 @@ static struct wlr_wl_output_layer *get_or_create_output_layer(
 
 static bool has_layers_order_changed(struct wlr_wl_output *output,
 		struct wlr_output_layer_state *layers, size_t layers_len) {
-	// output_basic_check() ensures that layers_len equals the number of
-	// registered output layers
-	size_t i = 0;
-	struct wlr_output_layer *layer;
-	wl_list_for_each(layer, &output->wlr_output.layers, link) {
-		assert(i < layers_len);
-		const struct wlr_output_layer_state *layer_state = &layers[i];
-		if (layer_state->layer != layer) {
+	// Iterate over all enabled layers, check whether the list of all existing
+	// layers has the same ordering
+	struct wl_list *cur = output->wlr_output.layers.next;
+	for (size_t i = 0; i < layers_len; i++) {
+		bool found = false;
+		while (cur != &output->wlr_output.layers) {
+			struct wlr_output_layer *layer = wl_container_of(cur, layer, link);
+			if (layer == layers[i].layer) {
+				found = true;
+				break;
+			}
+			cur = cur->next;
+		}
+		if (!found) {
 			return true;
 		}
-		i++;
 	}
-	assert(i == layers_len);
 	return false;
 }
 
@@ -635,11 +639,6 @@ static bool output_layer_commit(struct wlr_wl_output *output,
 		wl_subsurface_set_position(layer->subsurface, state->dst_box.x, state->dst_box.y);
 	}
 
-	if (state->buffer == NULL) {
-		output_layer_unmap(layer);
-		return true;
-	}
-
 	struct wlr_wl_buffer *buffer =
 		get_or_create_wl_buffer(output->backend, state->buffer);
 	if (buffer == NULL) {
@@ -677,22 +676,23 @@ static bool output_layer_commit(struct wlr_wl_output *output,
 }
 
 static bool commit_layers(struct wlr_wl_output *output,
-		struct wlr_output_layer_state *layers, size_t layers_len) {
-	if (output->backend->subcompositor == NULL) {
+		const struct wlr_output_state *state) {
+	if (!(state->committed & WLR_OUTPUT_STATE_LAYERS) ||
+			output->backend->subcompositor == NULL) {
 		return true;
 	}
 
-	bool reordered = has_layers_order_changed(output, layers, layers_len);
+	bool reordered = has_layers_order_changed(output, state->layers, state->layers_len);
 
 	struct wlr_wl_output_layer *prev_layer = NULL;
-	for (size_t i = 0; i < layers_len; i++) {
+	for (size_t i = 0; i < state->layers_len; i++) {
 		struct wlr_wl_output_layer *layer =
-			get_or_create_output_layer(output, layers[i].layer);
+			get_or_create_output_layer(output, state->layers[i].layer);
 		if (layer == NULL) {
 			return false;
 		}
 
-		if (!layers[i].accepted) {
+		if (!state->layers[i].accepted) {
 			output_layer_unmap(layer);
 			continue;
 		}
@@ -702,13 +702,25 @@ static bool commit_layers(struct wlr_wl_output *output,
 				prev_layer->surface);
 		}
 
-		if (!output_layer_commit(output, layer, &layers[i])) {
+		if (!output_layer_commit(output, layer, &state->layers[i])) {
 			return false;
 		}
 
 		prev_layer = layer;
 	}
 
+	struct wlr_output_layer *wlr_layer;
+	wl_list_for_each(wlr_layer, &output->wlr_output.layers, link) {
+		if (wlr_output_state_is_layer_enabled(state, wlr_layer)) {
+			continue;
+		}
+		struct wlr_wl_output_layer *layer =
+			get_or_create_output_layer(output, wlr_layer);
+		if (layer != NULL) {
+			output_layer_unmap(layer);
+		}
+	}
+
 	return true;
 }
 
@@ -851,8 +863,7 @@ static bool output_commit(struct wlr_output *wlr_output, const struct wlr_output
 		}
 	}
 
-	if ((state->committed & WLR_OUTPUT_STATE_LAYERS) &&
-			!commit_layers(output, state->layers, state->layers_len)) {
+	if (!commit_layers(output, state)) {
 		return false;
 	}
 

examples/output-layers.c

@@ -67,6 +67,10 @@ static void output_handle_frame(struct wl_listener *listener, void *data) {
 	struct wl_array layers_arr = {0};
 	struct output_surface *output_surface;
 	wl_list_for_each(output_surface, &output->surfaces, link) {
+		if (output_surface->buffer == NULL) {
+			continue;
+		}
+
 		struct wlr_output_layer_state *layer_state =
 			wl_array_add(&layers_arr, sizeof(*layer_state));
 		*layer_state = (struct wlr_output_layer_state){
@@ -103,6 +107,10 @@ static void output_handle_frame(struct wl_listener *listener, void *data) {
 	size_t i = 0;
 	struct wlr_output_layer_state *layers = layers_arr.data;
 	wl_list_for_each(output_surface, &output->surfaces, link) {
+		if (output_surface->buffer == NULL) {
+			continue;
+		}
+
 		struct wlr_surface *wlr_surface = output_surface->wlr_surface;
 
 		output_surface->layer_accepted = layers[i].accepted;

include/wlr/interfaces/wlr_output.h

@@ -13,6 +13,8 @@
 #include <wlr/backend.h>
 #include <wlr/types/wlr_output.h>
 
+struct wlr_output_layer;
+
 /**
  * Output state fields that don't require backend support. Backends can ignore
  * them without breaking the API contract.
@@ -136,4 +138,12 @@ void wlr_output_send_present(struct wlr_output *output,
 void wlr_output_send_request_state(struct wlr_output *output,
 	const struct wlr_output_state *state);
 
+/**
+ * Check whether a layer is enabled in an output state.
+ *
+ * The output state must have the layers field populated.
+ */
+bool wlr_output_state_is_layer_enabled(const struct wlr_output_state *state,
+	struct wlr_output_layer *layer);
+
 #endif

include/wlr/types/wlr_output_layer.h

@@ -38,7 +38,8 @@
  * output contents to be composited onto a single buffer, e.g. during screen
  * capture.
  *
- * Callers must always include the state for all layers on output test/commit.
+ * To disable an output layer, callers can leave it out of the array supplied
+ * on output test/commit.
  */
 struct wlr_output_layer {
 	struct wl_list link; // wlr_output.layers
@@ -62,7 +63,7 @@ struct wlr_output_layer {
 struct wlr_output_layer_state {
 	struct wlr_output_layer *layer;
 
-	// Buffer to display, or NULL to disable the layer
+	// Buffer to display, must not be NULL
 	struct wlr_buffer *buffer;
 	// Source box, leave empty to use the whole buffer
 	struct wlr_fbox src_box;

types/output/output.c

@@ -664,13 +664,14 @@ static bool output_basic_test(struct wlr_output *output,
 	}
 
 	if (state->committed & WLR_OUTPUT_STATE_LAYERS) {
-		if (state->layers_len != (size_t)wl_list_length(&output->layers)) {
-			wlr_log(WLR_DEBUG, "All output layers must be specified in wlr_output_state.layers");
-			return false;
-		}
-
 		for (size_t i = 0; i < state->layers_len; i++) {
 			assert(state->layers[i].layer != NULL);
+
+			if (state->layers[i].buffer == NULL) {
+				wlr_log(WLR_DEBUG, "All output layer states must have a buffer");
+				return false;
+			}
+
 			state->layers[i].accepted = false;
 		}
 	}

types/wlr_output_layer.c

@@ -1,6 +1,8 @@
 #include <assert.h>
 #include <stdlib.h>
+#include <wlr/interfaces/wlr_output.h>
 #include <wlr/types/wlr_output_layer.h>
+#include <wlr/util/log.h>
 
 struct wlr_output_layer *wlr_output_layer_create(struct wlr_output *output) {
 	struct wlr_output_layer *layer = calloc(1, sizeof(*layer));
@@ -28,3 +30,16 @@ void wlr_output_layer_destroy(struct wlr_output_layer *layer) {
 	wl_list_remove(&layer->link);
 	free(layer);
 }
+
+bool wlr_output_state_is_layer_enabled(const struct wlr_output_state *state,
+		struct wlr_output_layer *layer) {
+	assert(state->committed & WLR_OUTPUT_STATE_LAYERS);
+
+	for (size_t i = 0; i < state->layers_len; i++) {
+		if (state->layers[i].layer == layer) {
+			return true;
+		}
+	}
+
+	return false;
+}