From eca870774fda9d69fa504740ba9f45de267efb88 Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Wed, 14 Dec 2016 05:42:46 +0100 Subject: [PATCH] fix: Handle single input events --- include/components/controller.hpp | 2 +- include/components/eventloop.hpp | 44 ++++++----- include/events/signal.hpp | 4 +- src/components/controller.cpp | 67 +++++++--------- src/components/eventloop.cpp | 125 +++++++++++++----------------- src/components/logger.cpp | 10 +-- 6 files changed, 119 insertions(+), 133 deletions(-) diff --git a/include/components/controller.hpp b/include/components/controller.hpp index 5668a2d5..582d4c2b 100644 --- a/include/components/controller.hpp +++ b/include/components/controller.hpp @@ -48,7 +48,7 @@ class controller : public signal_receiver eventloop, unique_ptr bar, unique_ptr ipc, watch_t confwatch, bool writeback); + unique_ptr&& eventloop, unique_ptr&& bar, unique_ptr&& ipc, watch_t&& confwatch, bool writeback); ~controller(); void setup(); diff --git a/include/components/eventloop.hpp b/include/components/eventloop.hpp index ab892e1a..ee15f717 100644 --- a/include/components/eventloop.hpp +++ b/include/components/eventloop.hpp @@ -27,6 +27,7 @@ using namespace signals::eventloop; using module_t = unique_ptr; using modulemap_t = std::map>; +namespace chrono = std::chrono; using namespace std::chrono_literals; class eventloop : public signal_receiver using queue_t = moodycamel::BlockingConcurrentQueue; - using duration_t = std::chrono::duration; public: using make_type = unique_ptr; @@ -64,7 +60,7 @@ class eventloop : public signal_receiver m_queue; - /** - * @brief Event queue - */ - queue_t m_inputqueue; - /** * @brief Loaded modules */ @@ -123,12 +109,32 @@ class eventloop : public signal_receiver m_lastinput; + + /** + * @brief Input data + */ + string m_inputdata; }; POLYBAR_NS_END diff --git a/include/events/signal.hpp b/include/events/signal.hpp index fac50bc2..e0494127 100644 --- a/include/events/signal.hpp +++ b/include/events/signal.hpp @@ -92,13 +92,13 @@ namespace signals { DEFINE_VALUE_SIGNAL(1, process_quit, eventloop_t::event); DEFINE_VALUE_SIGNAL(2, process_update, eventloop_t::event); - DEFINE_VALUE_SIGNAL(3, process_input, eventloop_t::input_data); + DEFINE_VALUE_SIGNAL(3, process_input, string); DEFINE_VALUE_SIGNAL(4, process_check, eventloop_t::event); DEFINE_VALUE_SIGNAL(5, enqueue_event, eventloop_t::event); DEFINE_VALUE_SIGNAL(6, enqueue_quit, eventloop_t::event); DEFINE_VALUE_SIGNAL(7, enqueue_update, eventloop_t::event); - DEFINE_VALUE_SIGNAL(8, enqueue_input, eventloop_t::input_data); + DEFINE_VALUE_SIGNAL(8, enqueue_input, string); DEFINE_VALUE_SIGNAL(9, enqueue_check, eventloop_t::event); } diff --git a/src/components/controller.cpp b/src/components/controller.cpp index 8d3401cc..27df9a4b 100644 --- a/src/components/controller.cpp +++ b/src/components/controller.cpp @@ -32,7 +32,7 @@ controller::make_type controller::make(string&& path_confwatch, bool enable_ipc, eventloop::make(), bar::make(), enable_ipc ? ipc::make() : ipc::make_type{}, - !path_confwatch.empty() ? inotify_util::make_watch(forward(path_confwatch)) : watch_t{}, + !path_confwatch.empty() ? inotify_util::make_watch(forward(move(path_confwatch))) : watch_t{}, writeback); // clang-format on } @@ -41,37 +41,25 @@ controller::make_type controller::make(string&& path_confwatch, bool enable_ipc, * Construct controller object */ controller::controller(connection& conn, signal_emitter& emitter, const logger& logger, const config& config, - unique_ptr eventloop, unique_ptr bar, unique_ptr ipc, watch_t confwatch, bool writeback) + unique_ptr&& eventloop, unique_ptr&& bar, unique_ptr&& ipc, watch_t&& confwatch, bool writeback) : m_connection(conn) , m_sig(emitter) , m_log(logger) , m_conf(config) - , m_eventloop(move(eventloop)) - , m_bar(move(bar)) - , m_ipc(move(ipc)) - , m_confwatch(move(confwatch)) + , m_eventloop(forward(eventloop)) + , m_bar(forward(bar)) + , m_ipc(forward(ipc)) + , m_confwatch(forward(confwatch)) , m_writeback(writeback) {} /** * Deconstruct controller object */ controller::~controller() { - if (m_eventloop) { - m_log.info("Deconstructing eventloop"); - m_eventloop.reset(); - } if (m_command) { m_log.info("Terminating running shell command"); m_command.reset(); } - if (m_bar) { - m_log.info("Deconstructing bar"); - m_bar.reset(); - } - if (m_ipc) { - m_log.info("Deconstructing ipc"); - m_ipc.reset(); - } if (!m_writeback) { m_log.info("Interrupting X event loop"); m_connection.send_dummy_event(m_connection.root()); @@ -89,12 +77,13 @@ controller::~controller() { ; } - m_sig.detach(this); m_connection.flush(); } void controller::setup() { - string bs{m_conf.bar_section()}; + if (!m_writeback) { + m_connection.ensure_event_mask(m_connection.root(), XCB_EVENT_MASK_STRUCTURE_NOTIFY); + } string bs{m_conf.section()}; m_log.trace("controller: Setup user-defined modules"); @@ -150,11 +139,12 @@ void controller::setup() { bool controller::run() { assert(!m_connection.connection_has_error()); - m_sig.attach(this); m_log.info("Starting application"); m_running = true; + m_sig.attach(this); + if (m_confwatch && !m_writeback) { m_threads.emplace_back(thread(&controller::wait_for_configwatch, this)); } @@ -191,13 +181,20 @@ bool controller::run() { m_log.warn("Termination signal received, shutting down..."); m_log.trace("controller: Caught signal %d", caught_signal); - // Signal the eventloop, in case it's still running - m_eventloop->enqueue(eventloop::make_quit_evt(false)); + m_sig.detach(this); if (m_eventloop) { + // Signal the eventloop, in case it's still running + m_eventloop->enqueue(eventloop::make_quit_evt(false)); + m_log.trace("controller: Stopping event loop"); m_eventloop->stop(); } + + if (m_ipc) { + m_ipc.reset(); + } + if (!m_writeback && m_confwatch) { m_log.trace("controller: Removing config watch"); m_confwatch->remove(true); @@ -351,7 +348,7 @@ bool controller::on(const sig_ev::process_update& evt) { bool controller::on(const sig_ev::process_input& evt) { try { - string input{(*evt()).data}; + string input{*evt()}; if (m_command) { m_log.warn("Terminating previous shell command"); @@ -382,13 +379,12 @@ bool controller::on(const sig_ui::button_press& evt) { string input{*evt()}; - if (input.length() >= sizeof(eventloop::input_data)) { - m_log.warn("Ignoring input event (size)"); - } else if (!m_sig.emit(enqueue_input{eventloop::make_input_data(move(input))})) { - m_log.trace_x("controller: Dispatcher busy"); + if (input.empty()) { + m_log.err("Cannot enqueue empty input"); + return false; } - return true; + return m_sig.emit(enqueue_input{move(input)}); } bool controller::on(const sig_ipc::process_action& evt) { @@ -396,16 +392,13 @@ bool controller::on(const sig_ipc::process_action& evt) { string action{a.payload}; action.erase(0, strlen(ipc_action::prefix)); - if (action.size() >= sizeof(eventloop::input_data)) { - m_log.warn("Ignoring input event (size)"); - } else if (action.empty()) { + if (action.empty()) { m_log.err("Cannot enqueue empty ipc action"); - } else { - m_log.info("Enqueuing ipc action: %s", action); - m_eventloop->enqueue(eventloop::make_input_data(move(action))); - m_eventloop->enqueue(eventloop::make_input_evt()); + return false; } - return true; + + m_log.info("Enqueuing ipc action: %s", action); + return m_sig.emit(enqueue_input{move(action)}); } bool controller::on(const sig_ipc::process_command& evt) { diff --git a/src/components/eventloop.cpp b/src/components/eventloop.cpp index 239dbf36..1012a713 100644 --- a/src/components/eventloop.cpp +++ b/src/components/eventloop.cpp @@ -1,11 +1,11 @@ #include #include +#include "components/config.hpp" #include "components/eventloop.hpp" +#include "components/logger.hpp" #include "components/types.hpp" #include "events/signal.hpp" -#include "components/logger.hpp" -#include "components/config.hpp" #include "modules/meta/base.hpp" #include "utils/factory.hpp" #include "utils/string.hpp" @@ -21,31 +21,16 @@ eventloop::make_type eventloop::make() { return factory_util::unique(signal_emitter::make(), logger::make(), config::make()); } -eventloop::event eventloop::make_quit_evt(bool reload) { - return event{static_cast(event_type::QUIT), reload}; -} -eventloop::event eventloop::make_update_evt(bool force) { - return event{static_cast(event_type::UPDATE), force}; -} -eventloop::event eventloop::make_input_evt() { - return event{static_cast(event_type::INPUT)}; -} -eventloop::event eventloop::make_check_evt() { - return event{static_cast(event_type::CHECK)}; -} -eventloop::input_data eventloop::make_input_data(string&& data) { - input_data d{}; - memcpy(d.data, &data[0], sizeof(d.data)); - return d; -} - /** * Construct eventloop instance */ eventloop::eventloop(signal_emitter& emitter, const logger& logger, const config& config) : m_sig(emitter), m_log(logger), m_conf(config) { - m_swallow_time = duration_t{m_conf.get("settings", "eventloop-swallow-time", 10)}; - m_swallow_limit = m_conf.get("settings", "eventloop-swallow", 5U); + m_swallow_limit = m_conf.deprecated("settings", "eventloop-swallow", "throttle-output", m_swallow_limit); + m_swallow_update = m_conf.deprecated( + "settings", "eventloop-swallow-time", "throttle-output-for", m_swallow_update); + m_swallow_input = + m_conf.get("settings", "throttle-input-for", m_swallow_update * m_swallow_limit); m_sig.attach(this); } @@ -77,7 +62,6 @@ void eventloop::start() { while (m_running) { event evt, next; - input_data data; m_queue.wait_dequeue(evt); @@ -86,12 +70,10 @@ void eventloop::start() { } if (evt.type == static_cast(event_type::INPUT)) { - while (m_running && m_inputqueue.try_dequeue(data)) { - m_sig.emit(process_input{move(data)}); - } + handle_inputdata(); } else { size_t swallowed{0}; - while (swallowed++ < m_swallow_limit && m_queue.wait_dequeue_timed(next, m_swallow_time)) { + while (swallowed++ < m_swallow_limit && m_queue.wait_dequeue_timed(next, m_swallow_update)) { if (next.type == static_cast(event_type::QUIT)) { evt = next; break; @@ -100,8 +82,8 @@ void eventloop::start() { evt = next; break; - } else if (!compare_events(evt, next)) { - enqueue(move(next)); + } else if (evt.type != next.type) { + m_queue.try_enqueue(move(next)); break; } else { @@ -111,11 +93,15 @@ void eventloop::start() { } if (evt.type == static_cast(event_type::INPUT)) { - while (m_inputqueue.try_dequeue(data)) { - m_sig.emit(process_input{move(data)}); - } + handle_inputdata(); + } else if (evt.type == static_cast(event_type::QUIT)) { + m_sig.emit(process_quit{reinterpret_cast(evt)}); + } else if (evt.type == static_cast(event_type::UPDATE)) { + m_sig.emit(process_update{reinterpret_cast(evt)}); + } else if (evt.type == static_cast(event_type::CHECK)) { + m_sig.emit(process_check{reinterpret_cast(evt)}); } else { - forward_event(evt); + m_log.warn("Unknown event type for enqueued event (%d)", evt.type); } } } @@ -123,14 +109,6 @@ void eventloop::start() { m_log.info("Queue worker done"); } -void eventloop::process_inputqueue() { - input_data data{}; - - while (m_inputqueue.try_dequeue(data)) { - m_sig.emit(process_input{move(data)}); - } -} - /** * Stop main loop by enqueuing a QUIT event */ @@ -155,15 +133,25 @@ bool eventloop::enqueue(event&& evt) { } /** - * Enqueue input event + * Enqueue input data */ -bool eventloop::enqueue(input_data&& evt) { - if (!m_inputqueue.enqueue(move(evt))) { - m_log.warn("Failed to enqueue input_data"); +bool eventloop::enqueue(string&& input_data) { + if (m_inputlock.try_lock()) { return false; } - return true; + std::lock_guard guard(m_inputlock, std::adopt_lock); + + if (!m_inputdata.empty()) { + m_log.trace("eventloop: Swallowing input event (pending data)"); + return false; + } else if (chrono::system_clock::now() - m_swallow_input < m_lastinput) { + m_log.trace("eventloop: Swallowing input event (throttled)"); + return false; + } + + m_inputdata = forward(input_data); + return enqueue(make_input_evt()); } /** @@ -216,33 +204,19 @@ void eventloop::dispatch_modules() { } /** - * Compare given events + * Process pending input data */ -inline bool eventloop::compare_events(event evt, event evt2) { - return evt.type != evt2.type; -} - -inline bool eventloop::compare_events(input_data data, input_data data2) { - return data.data[0] == data2.data[0] && strncmp(data.data, data2.data, strlen(data.data)) == 0; -} - -/** - * Forward event to handler based on type - */ -void eventloop::forward_event(event evt) { - if (evt.type == static_cast(event_type::QUIT)) { - m_sig.emit(process_quit{reinterpret_cast(evt)}); - } else if (evt.type == static_cast(event_type::UPDATE)) { - m_sig.emit(process_update{reinterpret_cast(evt)}); - } else if (evt.type == static_cast(event_type::CHECK)) { - m_sig.emit(process_check{reinterpret_cast(evt)}); - } else { - m_log.warn("Unknown event type for enqueued event (%d)", evt.type); +void eventloop::handle_inputdata() { + std::lock_guard guard(m_inputlock); + if (!m_inputdata.empty()) { + m_sig.emit(process_input{move(m_inputdata)}); + m_lastinput = chrono::time_point_cast(chrono::system_clock::now()); + m_inputdata.clear(); } } bool eventloop::on(const process_input& evt) { - string input{(*evt()).data}; + string input{*evt()}; for (auto&& block : m_modules) { for (auto&& module : block.second) { @@ -302,7 +276,7 @@ bool eventloop::on(const enqueue_update& evt) { bool eventloop::on(const enqueue_input& evt) { m_log.trace("eventloop: enqueuing INPUT event"); - return enqueue(input_data{(*evt())}) && enqueue(make_input_evt()); + return enqueue(string{move(*evt())}); } bool eventloop::on(const enqueue_check& evt) { @@ -312,4 +286,17 @@ bool eventloop::on(const enqueue_check& evt) { return enqueue(move(check)); } +eventloop::event eventloop::make_quit_evt(bool reload) { + return event{static_cast(event_type::QUIT), reload}; +} +eventloop::event eventloop::make_update_evt(bool force) { + return event{static_cast(event_type::UPDATE), force}; +} +eventloop::event eventloop::make_input_evt() { + return event{static_cast(event_type::INPUT)}; +} +eventloop::event eventloop::make_check_evt() { + return event{static_cast(event_type::CHECK)}; +} + POLYBAR_NS_END diff --git a/src/components/logger.cpp b/src/components/logger.cpp index 1d2a2b0d..cf31144f 100644 --- a/src/components/logger.cpp +++ b/src/components/logger.cpp @@ -11,7 +11,7 @@ POLYBAR_NS * Create instance */ logger::make_type logger::make(loglevel level) { - return static_cast(*factory_util::singleton(level)); + return static_cast(*factory_util::singleton>(level)); } /** @@ -29,10 +29,10 @@ logger::logger(loglevel level) : m_level(level) { m_suffixes[loglevel::WARNING] = "\033[0m"; m_suffixes[loglevel::ERROR] = "\033[0m"; } else { - m_prefixes.emplace(make_pair(loglevel::TRACE, "polybar|trace ")); - m_prefixes.emplace(make_pair(loglevel::INFO, "polybar|infoe ")); - m_prefixes.emplace(make_pair(loglevel::WARNING, "polybar|warne ")); - m_prefixes.emplace(make_pair(loglevel::ERROR, "polybar|error ")); + m_prefixes.emplace(make_pair(loglevel::TRACE, "polybar|trace: ")); + m_prefixes.emplace(make_pair(loglevel::INFO, "polybar|info: ")); + m_prefixes.emplace(make_pair(loglevel::WARNING, "polybar|warn: ")); + m_prefixes.emplace(make_pair(loglevel::ERROR, "polybar|error: ")); m_suffixes.emplace(make_pair(loglevel::TRACE, "")); m_suffixes.emplace(make_pair(loglevel::INFO, "")); m_suffixes.emplace(make_pair(loglevel::WARNING, ""));