From bfb965a2ea09f23cc3592169baed60cf355aebf7 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Fri, 1 Nov 2019 16:34:12 +0100 Subject: [PATCH] fix(xworkspaces): Assign desktops to correct viewport Before the module would just try to evenly distribute desktops (workspaces) among the viewports. But since `_NET_DESKTOP_VIEWPORT` actually maps desktops to viewports, we can use that information to assign workspaces to the right viewport. Fixes #1849 Fixes #1764 --- include/x11/extensions/randr.hpp | 14 +++-- src/modules/xworkspaces.cpp | 90 ++++++++++++++++++++++---------- src/x11/extensions/randr.cpp | 23 +++++--- 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/include/x11/extensions/randr.hpp b/include/x11/extensions/randr.hpp index 29954159e6..5bfd800bc7 100644 --- a/include/x11/extensions/randr.hpp +++ b/include/x11/extensions/randr.hpp @@ -7,6 +7,7 @@ #endif #include + #include #include "common.hpp" @@ -19,7 +20,7 @@ struct position; namespace evt { using randr_notify = xpp::randr::event::notify; using randr_screen_change_notify = xpp::randr::event::screen_change_notify; -} +} // namespace evt struct backlight_values { unsigned int atom{0}; @@ -40,6 +41,8 @@ struct randr_output { bool match(const string& o, bool exact = true) const; bool match(const position& p) const; + + bool contains(const position& p) const; }; using monitor_t = shared_ptr; @@ -49,13 +52,14 @@ namespace randr_util { bool check_monitor_support(); - monitor_t make_monitor(xcb_randr_output_t randr, string name, unsigned short int w, unsigned short int h, short int x, short int y, - bool primary); - vector get_monitors(connection& conn, xcb_window_t root, bool connected_only = false, bool realloc = false); + monitor_t make_monitor(xcb_randr_output_t randr, string name, unsigned short int w, unsigned short int h, short int x, + short int y, bool primary); + vector get_monitors( + connection& conn, xcb_window_t root, bool connected_only = false, bool realloc = false); monitor_t match_monitor(vector monitors, const string& name, bool exact_match); void get_backlight_range(connection& conn, const monitor_t& mon, backlight_values& dst); void get_backlight_value(connection& conn, const monitor_t& mon, backlight_values& dst); -} +} // namespace randr_util POLYBAR_NS_END diff --git a/src/modules/xworkspaces.cpp b/src/modules/xworkspaces.cpp index 1515ca4e77..e9008e9302 100644 --- a/src/modules/xworkspaces.cpp +++ b/src/modules/xworkspaces.cpp @@ -1,17 +1,17 @@ #include "modules/xworkspaces.hpp" #include +#include #include #include "drawtypes/iconset.hpp" #include "drawtypes/label.hpp" +#include "modules/meta/base.inl" #include "utils/factory.hpp" #include "utils/math.hpp" #include "x11/atoms.hpp" #include "x11/connection.hpp" -#include "modules/meta/base.inl" - POLYBAR_NS namespace { @@ -20,6 +20,15 @@ namespace { } } // namespace +/** + * Defines a lexicographical order on position: + * + * (x1, y1) < (x2, y2) <==> (x1 == x2 && y1 < y2) || (x1 < x2) + */ +bool operator<(const position& a, const position& b) { + return a.x < b.x || (a.x == b.x && a.y < b.y); +} + namespace modules { template class module; @@ -147,37 +156,62 @@ namespace modules { /** * Rebuild the desktop tree + * + * This requires m_desktop_names to have an up-to-date value */ void xworkspaces_module::rebuild_desktops() { m_viewports.clear(); - auto bounds = [&] { - if (m_monitorsupport) { - return ewmh_util::get_desktop_viewports(); - } else { - vector b; - std::fill_n(std::back_inserter(b), m_desktop_names.size(), position{m_bar.monitor->x, m_bar.monitor->y}); - return b; - } - }(); - - bounds.erase(std::unique(bounds.begin(), bounds.end(), [](auto& a, auto& b) { return a == b; }), bounds.end()); + /* + * Stores the _NET_DESKTOP_VIEWPORT hint + * + * For WMs that don't support that hint, we store an empty vector + * + * If the length of the vector is less than _NET_NUMBER_OF_DESKTOPS + * all desktops which aren't explicitly assigned a postion will be + * assigned (0, 0) + * + * We use this to map workspaces to viewports, desktop i is at position + * ws_positions[i]. + */ + vector ws_positions; + if (m_monitorsupport) { + ws_positions = ewmh_util::get_desktop_viewports(); + } - unsigned int step; - if (bounds.size() > 0) { - step = m_desktop_names.size() / bounds.size(); - } else { - step = 1; + /* + * Not all desktops were assigned a viewport, add (0, 0) for all missing + * desktops. + */ + if (ws_positions.size() < m_desktop_names.size()) { + auto num_insert = m_desktop_names.size() - ws_positions.size(); + ws_positions.reserve(num_insert); + std::fill_n(std::back_inserter(ws_positions), num_insert, position{0, 0}); } - unsigned int offset = 0; - for (unsigned int i = 0; i < bounds.size(); i++) { - if (!m_pinworkspaces || m_bar.monitor->match(bounds[i])) { + + /* + * The list of viewports is the set of unique positions in ws_positions + * Using a set automatically removes duplicates. + * + * TODO is it possible to have two monitors but only a single workspace? In + * that case, this would only give us a single viewport. + */ + std::set viewports(ws_positions.begin(), ws_positions.end()); + + for (auto&& viewport_pos : viewports) { + /* + * If pin-workspaces is set, we only add the viewport if it's in the + * monitor the bar is on. + * Generally viewport_pos is the same as the top-left coordinate of the + * monitor but we use `contains` here as a safety in case it isn't exactly. + */ + if (!m_pinworkspaces || m_bar.monitor->contains(viewport_pos)) { auto viewport = make_unique(); viewport->state = viewport_state::UNFOCUSED; - viewport->pos = bounds[i]; + viewport->pos = viewport_pos; for (auto&& m : m_monitors) { - if (m->match(viewport->pos)) { + if (m->contains(viewport->pos)) { viewport->name = m->name; viewport->state = viewport_state::FOCUSED; } @@ -193,13 +227,15 @@ namespace modules { return label; }(); - for (unsigned int index = offset; index < offset + step; index++) { - viewport->desktops.emplace_back(make_unique(index, offset, desktop_state::EMPTY, label_t{})); + for (size_t i = 0; i < ws_positions.size(); i++) { + auto&& ws_pos = ws_positions[i]; + if (ws_pos == viewport_pos) { + viewport->desktops.emplace_back(make_unique(i, 0, desktop_state::EMPTY, label_t{})); + } } m_viewports.emplace_back(move(viewport)); } - offset += step; } } @@ -214,7 +250,7 @@ namespace modules { for (auto&& v : m_viewports) { for (auto&& d : v->desktops) { - if (m_desktop_names[d->index] == m_current_desktop_name) { + if (d->index == m_current_desktop) { d->state = desktop_state::ACTIVE; } else if (occupied_desks.count(d->index) > 0) { d->state = desktop_state::OCCUPIED; diff --git a/src/x11/extensions/randr.cpp b/src/x11/extensions/randr.cpp index 36baedb534..c0166e0c87 100644 --- a/src/x11/extensions/randr.cpp +++ b/src/x11/extensions/randr.cpp @@ -1,3 +1,5 @@ +#include "x11/extensions/randr.hpp" + #include #include @@ -7,7 +9,6 @@ #include "utils/string.hpp" #include "x11/atoms.hpp" #include "x11/connection.hpp" -#include "x11/extensions/randr.hpp" POLYBAR_NS @@ -32,6 +33,13 @@ bool randr_output::match(const position& p) const { return p.x == x && p.y == y; } +/** + * Checks if `this` contains the position p + */ +bool randr_output::contains(const position& p) const { + return x <= p.x && y <= p.y && (x + w) > p.x && (y + h) > p.y; +} + namespace randr_util { /** * XRandR version @@ -63,9 +71,8 @@ namespace randr_util { /** * Define monitor */ - monitor_t make_monitor( - xcb_randr_output_t randr, string name, unsigned short int w, unsigned short int h, short int x, short int y, - bool primary) { + monitor_t make_monitor(xcb_randr_output_t randr, string name, unsigned short int w, unsigned short int h, short int x, + short int y, bool primary) { monitor_t mon{new monitor_t::element_type{}}; mon->output = randr; mon->name = move(name); @@ -192,9 +199,9 @@ namespace randr_util { */ monitor_t match_monitor(vector monitors, const string& name, bool exact_match) { monitor_t result{}; - for(auto&& monitor : monitors) { + for (auto&& monitor : monitors) { // If we can do an exact match, we have found our result - if(monitor->match(name, true)) { + if (monitor->match(name, true)) { result = move(monitor); break; } @@ -205,7 +212,7 @@ namespace randr_util { * Note: If exact_match == true, we don't need to run this because it * would be the exact same check as above */ - if(!exact_match && monitor->match(name, false)) { + if (!exact_match && monitor->match(name, false)) { result = move(monitor); } } @@ -254,6 +261,6 @@ namespace randr_util { dst.val = static_cast(value); } } -} +} // namespace randr_util POLYBAR_NS_END