From 65279883c3d37521f23fa40b621d8dc0d4db09d7 Mon Sep 17 00:00:00 2001
From: patrick96
Date: Wed, 30 Nov 2022 15:05:34 +0100
Subject: [PATCH] Replace wait_for_response with handle callback
The poll-loop could in theory swallow events which would not be seen by
the main eventloop
---
include/components/screen.hpp | 9 +-
include/x11/connection.hpp | 151 +++++++++++++++-------------------
src/components/screen.cpp | 25 +++---
3 files changed, 86 insertions(+), 99 deletions(-)
diff --git a/include/components/screen.hpp b/include/components/screen.hpp
index 474e5d9b..aa4ad55c 100644
--- a/include/components/screen.hpp
+++ b/include/components/screen.hpp
@@ -16,7 +16,7 @@ class logger;
class connection;
class signal_emitter;
-class screen : public xpp::event::sink {
+class screen : public xpp::event::sink {
public:
using make_type = unique_ptr;
static make_type make();
@@ -25,6 +25,7 @@ class screen : public xpp::event::sink {
~screen();
protected:
+ void handle(const evt::map_notify& evt) override;
void handle(const evt::randr_screen_change_notify& evt) override;
private:
@@ -42,6 +43,12 @@ class screen : public xpp::event::sink {
};
bool m_sigraised{false};
+ /**
+ * Original event mask on the root window.
+ * Used to restore event mask after the proxy window is mapped.
+ */
+ uint32_t m_root_mask{0};
+
bool have_monitors_changed() const;
};
diff --git a/include/x11/connection.hpp b/include/x11/connection.hpp
index 9991041e..3d9ecda3 100644
--- a/include/x11/connection.hpp
+++ b/include/x11/connection.hpp
@@ -16,80 +16,80 @@
POLYBAR_NS
namespace detail {
- template
- class interfaces : public xpp::x::extension::interface, Connection>,
- public Extensions::template interface, Connection>... {
- public:
- const Connection& connection() const {
- return static_cast(*this);
- }
- };
+template
+class interfaces : public xpp::x::extension::interface, Connection>,
+ public Extensions::template interface, Connection>... {
+ public:
+ const Connection& connection() const {
+ return static_cast(*this);
+ }
+};
- template
- class connection_base : public xpp::core,
- public xpp::generic::error_dispatcher,
- public detail::interfaces, Extensions...>,
- private xpp::x::extension,
- private xpp::x::extension::error_dispatcher,
- private Extensions...,
- private Extensions::error_dispatcher... {
- public:
- explicit connection_base(xcb_connection_t* c, int s)
- : xpp::core(c)
- , interfaces, Extensions...>(*this)
- , Extensions(m_c.get())...
- , Extensions::error_dispatcher(static_cast(*this).get())... {
- core::m_screen = s;
- m_root_window = screen_of_display(s)->root;
- }
+template
+class connection_base : public xpp::core,
+ public xpp::generic::error_dispatcher,
+ public detail::interfaces, Extensions...>,
+ private xpp::x::extension,
+ private xpp::x::extension::error_dispatcher,
+ private Extensions...,
+ private Extensions::error_dispatcher... {
+ public:
+ explicit connection_base(xcb_connection_t* c, int s)
+ : xpp::core(c)
+ , interfaces, Extensions...>(*this)
+ , Extensions(m_c.get())...
+ , Extensions::error_dispatcher(static_cast(*this).get())... {
+ core::m_screen = s;
+ m_root_window = screen_of_display(s)->root;
+ }
- void operator()(const shared_ptr& error) const override {
+ void operator()(const shared_ptr& error) const override {
+ check(error);
+ }
+
+ template
+ const Extension& extension() const {
+ return static_cast(*this);
+ }
+
+ xcb_window_t root() const {
+ return m_root_window;
+ }
+
+ shared_ptr wait_for_event() const override {
+ try {
+ return core::wait_for_event();
+ } catch (const shared_ptr& error) {
check(error);
}
+ throw; // re-throw exception
+ }
- template
- const Extension& extension() const {
- return static_cast(*this);
+ shared_ptr wait_for_special_event(xcb_special_event_t* se) const override {
+ try {
+ return core::wait_for_special_event(se);
+ } catch (const shared_ptr& error) {
+ check(error);
}
+ throw; // re-throw exception
+ }
- xcb_window_t root() const {
- return m_root_window;
- }
+ private:
+ xcb_window_t m_root_window;
- shared_ptr wait_for_event() const override {
- try {
- return core::wait_for_event();
- } catch (const shared_ptr& error) {
- check(error);
- }
- throw; // re-throw exception
- }
+ template
+ void check(const shared_ptr& error) const {
+ check(error);
+ check(error);
+ }
- shared_ptr wait_for_special_event(xcb_special_event_t* se) const override {
- try {
- return core::wait_for_special_event(se);
- } catch (const shared_ptr& error) {
- check(error);
- }
- throw; // re-throw exception
- }
-
- private:
- xcb_window_t m_root_window;
-
- template
- void check(const shared_ptr& error) const {
- check(error);
- check(error);
- }
-
- template
- void check(const shared_ptr& error) const {
- using error_dispatcher = typename Extension::error_dispatcher;
- auto& dispatcher = static_cast(*this);
- dispatcher(error);
- }
- };
+ template
+ void check(const shared_ptr& error) const {
+ using error_dispatcher = typename Extension::error_dispatcher;
+ auto& dispatcher = static_cast(*this);
+ dispatcher(error);
+ }
+};
} // namespace detail
class connection : public detail::connection_base {
@@ -129,27 +129,6 @@ class connection : public detail::connection_base& evt) const;
- template
- void wait_for_response(function check_event) {
- int fd = get_file_descriptor();
- shared_ptr evt{};
- while (!connection_has_error()) {
- fd_set fds;
- FD_ZERO(&fds);
- FD_SET(fd, &fds);
-
- if (!select(fd + 1, &fds, nullptr, nullptr, nullptr)) {
- continue;
- } else if ((evt = shared_ptr(xcb_poll_for_event(*this), free)) == nullptr) {
- continue;
- } else if (evt->response_type != ResponseType) {
- continue;
- } else if (check_event(reinterpret_cast(&*evt))) {
- break;
- }
- }
- }
-
template
void attach_sink(Sink&& sink, registry::priority prio = 0) {
m_registry.attach(prio, forward(sink));
diff --git a/src/components/screen.cpp b/src/components/screen.cpp
index 1f11da30..756afba9 100644
--- a/src/components/screen.cpp
+++ b/src/components/screen.cpp
@@ -54,27 +54,19 @@ screen::screen(connection& conn, signal_emitter& emitter, const logger& logger,
// Update the root windows event mask
auto attributes = m_connection.get_window_attributes(m_root);
- auto root_mask = attributes->your_event_mask;
+ m_root_mask = attributes->your_event_mask;
attributes->your_event_mask = attributes->your_event_mask | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY;
m_connection.change_window_attributes(m_root, XCB_CW_EVENT_MASK, &attributes->your_event_mask);
// Receive randr events
m_connection.randr().select_input(m_proxy, XCB_RANDR_NOTIFY_MASK_SCREEN_CHANGE);
+ // Attach the sink to process randr events
+ m_connection.attach_sink(this, SINK_PRIORITY_SCREEN);
+
// Create window used as event proxy
m_connection.map_window(m_proxy);
m_connection.flush();
-
- // Wait until the proxy window has been mapped
- // TODO replace wait with a `handle` method
- using evt = xcb_map_notify_event_t;
- m_connection.wait_for_response([&](const evt* evt) -> bool { return evt->window == m_proxy; });
-
- // Restore the root windows event mask
- m_connection.change_window_attributes(m_root, XCB_CW_EVENT_MASK, &root_mask);
-
- // Finally attach the sink the process randr events
- m_connection.attach_sink(this, SINK_PRIORITY_SCREEN);
}
/**
@@ -88,6 +80,15 @@ screen::~screen() {
}
}
+void screen::handle(const evt::map_notify& evt) {
+ if (evt->window != m_proxy) {
+ return;
+ }
+
+ // Once the proxy window has been mapped, restore the original root window event mask.
+ m_connection.change_window_attributes(m_root, XCB_CW_EVENT_MASK, &m_root_mask);
+}
+
/**
* Handle XCB_RANDR_SCREEN_CHANGE_NOTIFY events
*