fix(monitor): do not include outputs when monitors are supported (#2485)

* fix(monitor): do not include outputs when monitors are supported

Previously, when splitting an output into two monitors, `polybar -m`
would report both the splitted monitors and the output. This was not
caught by the the clone detection as the detection works by removing
monitors contained into another monitors (and monitors were excluded
from that logic) and we want the other way around: outputs covered by
monitors should be ignored.

Instead of trying to detect covered outputs, the solution is quite
simple: when monitors are supported, do not consider outputs, unless
we request all outputs. A monitor can be set primary (and RandR
reports primary outputs as primary monitors). The only information we
would miss from monitors are things like refresh rate and EDID. We
don't need that, so we are fine.

As monitors are only created for connected output (and they are in
this case "active") or disconnected output if they are mapped (and
they are in this case "inactive"), I am a bit unsure if we have
exactly the same behaviour as previously when `connected_only` is set
to `false`.

As some modules require an output, we keep the output in the
`monitor_t` structure and we ensure it is correctly set when using
monitors. A monitor can have 0 or several outputs. We only handle the
0 and 1 cases. When a monitor has more outputs, only the first one is
used. AFAIK, only the xbacklight module needs that and I think we are
fine waiting for a user needing this module and merging monitors.

The C++ binding fail to expose the `outputs()` method to iterate over
the outputs of a monitor. This seems to be a bug in XPP. The field is
correctly defined in the RandR XML file and it works with the Python
binding.

```xml
	<struct name="MonitorInfo">
		<field type="ATOM" name="name" />
		<field type="BOOL" name="primary" />
		<field type="BOOL" name="automatic" />
		<field type="CARD16" name="nOutput" />
		<field type="INT16" name="x" />
		<field type="INT16" name="y" />
		<field type="CARD16" name="width" /> <!-- pixels -->
		<field type="CARD16" name="height" /> <!-- pixels -->
		<field type="CARD32" name="width_in_millimeters" />
		<field type="CARD32" name="height_in_millimeters" />
		<list type="OUTPUT" name="outputs">
		    <fieldref>nOutput</fieldref>
		</list>
	</struct>
```

Falling back to C only to access the list of outputs is not enough
because the list is appended to the structure and not visible through
the public API. When copied, the structure loses the list of monitors.

Also, change the mention "XRandR monitor" to "no output" when there is
no output attached. People using monitors know what it means and it is
useful to catch a future regression where we don't have an output at
all (which would break the brightness plugin).

Fix #2481

* Update CHANGELOG.md

Co-authored-by: Patrick Ziegler <p.ziegler96@gmail.com>

CHANGELOG.md

@@ -126,6 +126,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([`#2367`](https://github.com/polybar/polybar/issues/2367))
 - Warning message regarding T@ in bspwm module
   ([`#2371`](https://github.com/polybar/polybar/issues/2371))
+- `polybar -m` used to show both physical outputs and randr monitors, even if the outputs were covered by monitors.
+  ([`#2481`](https://github.com/polybar/polybar/issues/2481))
 
 ## [3.5.6] - 2021-05-24
 ### Build

src/main.cpp

@@ -81,8 +81,8 @@ int main(int argc, char** argv) {
       bool purge_clones = !cli->has("list-all-monitors");
       auto monitors = randr_util::get_monitors(conn, conn.root(), true, purge_clones);
       for (auto&& mon : monitors) {
-        if (WITH_XRANDR_MONITORS && mon->output == XCB_NONE) {
+        if (mon->output == XCB_NONE) {
-          printf("%s: %ix%i+%i+%i (XRandR monitor%s)\n", mon->name.c_str(), mon->w, mon->h, mon->x, mon->y,
+          printf("%s: %ix%i+%i+%i (no output%s)\n", mon->name.c_str(), mon->w, mon->h, mon->x, mon->y,
               mon->primary ? ", primary" : "");
         } else {
           printf("%s: %ix%i+%i+%i%s\n", mon->name.c_str(), mon->w, mon->h, mon->x, mon->y,

src/x11/extensions/randr.cpp

@@ -107,19 +107,36 @@ namespace randr_util {
    */
   vector<monitor_t> get_monitors(connection& conn, xcb_window_t root, bool connected_only, bool purge_clones) {
     vector<monitor_t> monitors;
+    bool found = false;
 
-#if WITH_XRANDR_MONITORS
     if (check_monitor_support()) {
-      for (auto&& mon : conn.get_monitors(root, true).monitors()) {
+#if WITH_XRANDR_MONITORS
-        try {
+      /* Use C, because XPP does not provide access to output info from monitors. */
-          auto name = conn.get_atom_name(mon.name).name();
+      xcb_generic_error_t *err;
-          monitors.emplace_back(make_monitor(XCB_NONE, move(name), mon.width, mon.height, mon.x, mon.y, mon.primary));
+      auto rrmonitors =
-        } catch (const exception&) {
+        xcb_randr_get_monitors_reply(
-          // silently ignore output
+          conn, xcb_randr_get_monitors(conn, root, true), &err);
-        }
+      if (err != NULL) {
+        free(err);
+      } else {
+        for (auto iter = xcb_randr_get_monitors_monitors_iterator(rrmonitors);
+             iter.rem;
+             xcb_randr_monitor_info_next(&iter)) {
+          auto mon = iter.data;
+          auto name = conn.get_atom_name(mon->name).name();
+
+          /* Output is only useful for some plugins. We take the first one. */
+          int randr_output_len = xcb_randr_monitor_info_outputs_length(mon);
+          auto randr_outputs = xcb_randr_monitor_info_outputs(mon);
+          auto output = (randr_output_len >= 1) ? randr_outputs[0] : XCB_NONE;
+          monitors.emplace_back(make_monitor(output, move(name), mon->width, mon->height, mon->x, mon->y, mon->primary));
+          found = true;
         }
+        free(rrmonitors);
       }
 #endif
+    }
+    if (!found || !purge_clones) {
       auto primary_output = conn.get_output_primary(root).output();
       string primary_name{};
 
@@ -141,24 +158,16 @@ namespace randr_util {
           auto name_iter = info.name();
           string name{name_iter.begin(), name_iter.end()};
 
-#if WITH_XRANDR_MONITORS
-        if (check_monitor_support()) {
-          auto mon = std::find_if(
-              monitors.begin(), monitors.end(), [&name](const monitor_t& mon) { return mon->name == name; });
-          if (mon != monitors.end()) {
-            (*mon)->output = output;
-            continue;
-          }
-        }
-#endif
-
           auto crtc = conn.get_crtc_info(info->crtc);
           auto primary = (primary_name == name);
+          if (!std::any_of(monitors.begin(), monitors.end(), [name](const auto &monitor) { return monitor->name == name; })) {
             monitors.emplace_back(make_monitor(output, move(name), crtc->width, crtc->height, crtc->x, crtc->y, primary));
+          }
         } catch (const exception&) {
           // silently ignore output
         }
       }
+    }
 
     if (purge_clones) {
       for (auto& outer : monitors) {
@@ -169,8 +178,6 @@ namespace randr_util {
         for (auto& inner : monitors) {
           if (outer == inner || inner->w == 0) {
             continue;
-          } else if (check_monitor_support() && (inner->output == XCB_NONE || outer->output == XCB_NONE)) {
-            continue;
           }
 
           // If inner is contained in outer, inner is removed