Skip mirrored outputs

[insom]

I think that https://github.com/LemonBoy/bar/issues/241 reports an issue that I'm also seeing: if I start lemonbar after I have a second monitor connected as a mirror, I get two bars, but they are both docked to the bottom of the same screen, and the blank one covers the real one.

I've tested this commit and it seems to fix this behavior.

(I'm a big fan of lemonbar, thanks for writing it!)

[LemonBoy]

Thanks for the patch. One small question, is the output with num_clones > 0 the "real" one or the cloned one? In other words, if the user has the same output cloned two times will this patch skip the two clones?

[insom]

I found it pretty much impossible to find documentation for this, so I attached two mirrors to my laptop and patched lemonbar to let me know. I confirmed that the monitor with num_clones=0 is the primary.

eDP-1 is my primary, DP-1 and DP2 are set up as --same-as eDP-1 with xrandr.

OUTPUT 0 CLONES: 0 NAME: eDP-1
OUTPUT 1 CLONES: 1 NAME: DP-1Q
OUTPUT 3 CLONES: 1 NAME: DP-2C

(The hacky patch, just in case it matters)

diff --git a/lemonbar.c b/lemonbar.c
index 528c2f0..840cc6f 100644
--- a/lemonbar.c
+++ b/lemonbar.c
@@ -995,7 +995,7 @@ get_randr_monitors (void)

         // Output disconnected, not attached to any CRTC or mirrored ?
         if (!oi_reply || oi_reply->crtc == XCB_NONE || oi_reply->connection != XCB_RANDR_CONNECTION_CONNECTED
-                || oi_reply->num_clones > 0) {
+                || oi_reply->num_clones > 2) {
             free(oi_reply);
             continue;
         }
@@ -1011,6 +1011,7 @@ get_randr_monitors (void)

         int name_len = xcb_randr_get_output_info_name_length(oi_reply);
         uint8_t *name_ptr = xcb_randr_get_output_info_name(oi_reply);
+        printf("OUTPUT %d CLONES: %d NAME: %s\n", i, oi_reply->num_clones, name_ptr);

         bool is_valid = true;

[insom]

:( Okay so please hold off on reviewing this. It looks like my understanding of clones in RANDR-extension speak is wrong. This still shows > 0 when displays are not mirrored. In my case, the HDMI and DP outputs are "cloned" internally, and that's not the same thing. Sorry to have wasted your time, I'll work on a better patch.

[insom]

I looked at what other software does and realized we'd need to check for overlapping rectangles in the outputs, and I thought I'd seen similar code in lemonbar and ... yeah ... a few pages down there it is.

There was a check which doesn't run this when mons[j].name is non-NULL, but I think that since 0bfd555265c9ea4acbd81bc27320e61486c85e49 nothing sets this to NULL. I've removed this check and can confirm that it only puts one bar on a mirrored screen while it still puts a second bar on my extended desktop.

With this verbose patch

diff --git a/lemonbar.c b/lemonbar.c
index 83d9677..28b598f 100644
--- a/lemonbar.c
+++ b/lemonbar.c
@@ -1055,9 +1055,11 @@ get_randr_monitors (void)
             // Does I contain J ?

             if (i != j && mons[j].width) {
+                printf("Checking %s for overlapping.\n", mons[j].name);
                 if (mons[j].x >= mons[i].x && mons[j].x + mons[j].width <= mons[i].x + mons[i].width &&
                     mons[j].y >= mons[i].y && mons[j].y + mons[j].height <= mons[i].y + mons[i].height) {
                     mons[j].width = 0;
+                    printf("Match! Removing %s for overlapping.\n", mons[j].name);
                     valid--;
                 }
             }

lemonbar will now remove the second bar from DP-1 (mirror) while adding one on DP-2 (right-of eDP-1)

% ./lemonbar 
Checking DP-1 for overlapping.
Match! Removing DP-1 for overlapping.
Checking DP-2 for overlapping.
Checking eDP-1 for overlapping.

Thanks for your patience with this, sorry the original version of this PR was spurious.

[LemonBoy]

I found it pretty much impossible to find documentation for this

That's valid for everything about XCB heh

There was a check which doesn't run this when mons[j].name is non-NULL, but I think that since https://github.com/LemonBoy/bar/commit/0bfd555265c9ea4acbd81bc27320e61486c85e49 nothing sets this to NULL.

Oh well, I guess (that code was written quite a while ago) the old logic was meant to skip the deduplication logic if the user specifies a set of monitors to cover. But I don't think that's desirable, I cannot come up with a reason to keep that logic.

Thanks for the investigation, let's merge this and see if anybody complains :D