output: avoid use of wlr_scene_output.WLR_PRIVATE.index

We were only using it to allow quick bitset comparisons of sets of
outputs (such as view->outputs). We can maintain our own bit IDs for
this purpose and avoid using the private wlroots field.

Note: from my reading of wlr_scene_output_create(), it appears to
always take the lowest unused index, resulting in aggressive re-use of
index values when outputs are disconnected and reconnected. I've tried
to make re-use as infrequent as possible. This could theoretically
reduce the chance of a mix-up in view_update_outputs(), although I'm
not aware of any practical scenario where it matters.

v2: prevent adding more than 64 outputs

include/labwc.h

@@ -257,6 +257,7 @@ struct server {
 	struct wl_list outputs;
 	struct wl_listener new_output;
 	struct wlr_output_layout *output_layout;
+	uint64_t next_output_id_bit;
 
 	struct wl_listener output_layout_change;
 	struct wlr_output_manager_v1 *output_manager;

include/output.h

@@ -33,6 +33,18 @@ struct output {
 	struct wl_listener frame;
 	struct wl_listener request_state;
 
+	/*
+	 * Unique power-of-two ID used in bitsets such as view->outputs.
+	 * (This assumes there are never more than 64 outputs connected
+	 * at once; wlr_scene_output has a similar limitation.)
+	 *
+	 * There's currently no attempt to maintain the same ID if the
+	 * same physical output is disconnected and reconnected.
+	 * However, IDs do get reused eventually if enough outputs are
+	 * disconnected and connected again.
+	 */
+	uint64_t id_bit;
+
 	bool gamma_lut_changed;
 };
 

include/view.h

@@ -158,7 +158,7 @@ struct view {
 	 * This is used to notify the foreign toplevel
 	 * implementation and to update the SSD invisible
 	 * resize area.
-	 * It is a bitset of output->scene_output->index.
+	 * It is a bitset of output->id_bit.
 	 */
 	uint64_t outputs;
 

src/output.c

@@ -414,6 +414,41 @@ configure_new_output(struct server *server, struct output *output)
 	server->pending_output_layout_change--;
 }
 
+static uint64_t
+get_unused_output_id_bit(struct server *server)
+{
+	uint64_t used_id_bits = 0;
+	struct output *output;
+	wl_list_for_each(output, &server->outputs, link) {
+		used_id_bits |= output->id_bit;
+	}
+
+	if (used_id_bits == UINT64_MAX) {
+		return 0;
+	}
+
+	uint64_t id_bit = server->next_output_id_bit;
+	/*
+	 * __builtin_popcountll() should be supported by GCC & clang.
+	 * If it causes portability issues, just remove the assert.
+	 */
+	assert(__builtin_popcountll(id_bit) == 1);
+
+	while ((id_bit & used_id_bits) != 0) {
+		id_bit = (id_bit << 1) | (id_bit >> 63); /* rotate left */
+	}
+
+	/*
+	 * The current implementation of view_update_outputs() isn't
+	 * robust against ID bit re-use. Save the next bit here so we
+	 * can cycle through all 64 available bits, making re-use less
+	 * frequent (on a best-effort basis).
+	 */
+	server->next_output_id_bit = (id_bit << 1) | (id_bit >> 63);
+
+	return id_bit;
+}
+
 static void
 handle_new_output(struct wl_listener *listener, void *data)
 {
@@ -437,6 +472,12 @@ handle_new_output(struct wl_listener *listener, void *data)
 		}
 	}
 
+	uint64_t id_bit = get_unused_output_id_bit(server);
+	if (!id_bit) {
+		wlr_log(WLR_ERROR, "Cannot add more than 64 outputs");
+		return;
+	}
+
 	if (wlr_output_is_wl(wlr_output)) {
 		char title[64];
 		snprintf(title, sizeof(title), "%s - %s", "labwc", wlr_output->name);
@@ -487,6 +528,7 @@ handle_new_output(struct wl_listener *listener, void *data)
 	output->wlr_output = wlr_output;
 	wlr_output->data = output;
 	output->server = server;
+	output->id_bit = id_bit;
 	output_state_init(output);
 
 	wl_list_insert(&server->outputs, &output->link);
@@ -583,6 +625,7 @@ output_init(struct server *server)
 		server->output_layout);
 
 	wl_list_init(&server->outputs);
+	server->next_output_id_bit = (1 << 0);
 
 	output_manager_init(server);
 }

src/view.c

@@ -570,7 +570,7 @@ view_update_outputs(struct view *view)
 	wl_list_for_each(output, &view->server->outputs, link) {
 		if (output_is_usable(output) && wlr_output_layout_intersects(
 				layout, output->wlr_output, &view->current)) {
-			new_outputs |= (1ull << output->scene_output->WLR_PRIVATE.index);
+			new_outputs |= output->id_bit;
 		}
 	}
 
@@ -586,8 +586,7 @@ view_on_output(struct view *view, struct output *output)
 {
 	assert(view);
 	assert(output);
-	return output->scene_output
-			&& (view->outputs & (1ull << output->scene_output->WLR_PRIVATE.index));
+	return (view->outputs & output->id_bit) != 0;
 }
 
 void