From 151a263654f9a5c7dc2b2f866eee40d06e98e30e Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Thu, 2 Sep 2021 18:07:21 +0200 Subject: [PATCH] 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 nOutput ``` 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 --- CHANGELOG.md | 2 + src/main.cpp | 4 +- src/x11/extensions/randr.cpp | 99 +++++++++++++++++++----------------- 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ec25b8c..580b8c64 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/main.cpp b/src/main.cpp index 4f61876c..9092cd53 100644 --- a/src/main.cpp +++ b/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) { - printf("%s: %ix%i+%i+%i (XRandR monitor%s)\n", mon->name.c_str(), mon->w, mon->h, mon->x, mon->y, + if (mon->output == XCB_NONE) { + 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, diff --git a/src/x11/extensions/randr.cpp b/src/x11/extensions/randr.cpp index 1baac9b4..82f112f8 100644 --- a/src/x11/extensions/randr.cpp +++ b/src/x11/extensions/randr.cpp @@ -107,58 +107,67 @@ namespace randr_util { */ vector get_monitors(connection& conn, xcb_window_t root, bool connected_only, bool purge_clones) { vector 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 + /* Use C, because XPP does not provide access to output info from monitors. */ + xcb_generic_error_t *err; + auto rrmonitors = + xcb_randr_get_monitors_reply( + 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{}; + + if (primary_output != XCB_NONE) { + auto primary_info = conn.get_output_info(primary_output); + auto name_iter = primary_info.name(); + primary_name = {name_iter.begin(), name_iter.end()}; + } + + for (auto&& output : conn.get_screen_resources(root).outputs()) { try { - auto name = conn.get_atom_name(mon.name).name(); - monitors.emplace_back(make_monitor(XCB_NONE, move(name), mon.width, mon.height, mon.x, mon.y, mon.primary)); + auto info = conn.get_output_info(output); + if (info->crtc == XCB_NONE) { + continue; + } else if (connected_only && info->connection != XCB_RANDR_CONNECTION_CONNECTED) { + continue; + } + + auto name_iter = info.name(); + string name{name_iter.begin(), name_iter.end()}; + + 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 } } } -#endif - auto primary_output = conn.get_output_primary(root).output(); - string primary_name{}; - - if (primary_output != XCB_NONE) { - auto primary_info = conn.get_output_info(primary_output); - auto name_iter = primary_info.name(); - primary_name = {name_iter.begin(), name_iter.end()}; - } - - for (auto&& output : conn.get_screen_resources(root).outputs()) { - try { - auto info = conn.get_output_info(output); - if (info->crtc == XCB_NONE) { - continue; - } else if (connected_only && info->connection != XCB_RANDR_CONNECTION_CONNECTED) { - continue; - } - - 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); - 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