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

diff --git a/include/labwc.h b/include/labwc.h
index d853c01d0b45e0d63cf4f25c2dc91707256cf66d..160c42f43b53a99fdeb10379b4a89db2a6e9760f 100644 (file)
--- a/include/labwc.h
+++ b/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;

diff --git a/include/output.h b/include/output.h
index fe11b0910d1e649f825665b6659684d7b3d37116..3226c9642f1b678a6c03f4aa0b892f3b0b863d70 100644 (file)
--- a/include/output.h
+++ b/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;
 };
 

diff --git a/include/view.h b/include/view.h
index 46a8e29f4033382be2ab0fd6e8b053f2df0eb978..2d89426565aefb2c2cf01be51465bf899d5682c7 100644 (file)
--- a/include/view.h
+++ b/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;
 

diff --git a/src/output.c b/src/output.c
index f74a56971d125aeaba89654584a835c48ca80f6c..3859ee15ef56e77055e15a1957c2cc96bb206024 100644 (file)
--- a/src/output.c
+++ b/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);
 }

diff --git a/src/view.c b/src/view.c
index 8a0d3cafb7a094cfc818cf46f0c82e44441ae12b..f04722890bb53d15ab1b4a2961b6ba86165386e6 100644 (file)
--- a/src/view.c
+++ b/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