From cd1d4fa183504d17302f788fae6fe4035ecb7333 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Tue, 2 Feb 2021 23:03:44 +0100 Subject: [PATCH] Stop tracking action blocks in renderer Dispatch can now directly get the current position from the renderer and create action blocks with the proper coordinates. --- include/components/renderer.hpp | 5 +- include/components/renderer_interface.hpp | 29 ++++++--- include/tags/context.hpp | 52 ++++++++++++++--- include/tags/dispatch.hpp | 1 - src/components/renderer.cpp | 21 ++----- src/tags/context.cpp | 17 ++++-- src/tags/dispatch.cpp | 11 ++-- tests/unit_tests/tags/action_context.cpp | 71 +++++++++-------------- tests/unit_tests/tags/dispatch.cpp | 6 +- 9 files changed, 121 insertions(+), 92 deletions(-) diff --git a/include/components/renderer.hpp b/include/components/renderer.hpp index 5b70ca0e..a6fd0b7a 100644 --- a/include/components/renderer.hpp +++ b/include/components/renderer.hpp @@ -53,8 +53,9 @@ class renderer : public renderer_interface, void change_alignment(const tags::context& ctxt) override; - void action_open(const tags::context& ctxt, tags::action_t id) override; - void action_close(const tags::context& ctxt, tags::action_t id) override; + double get_x(const tags::context& ctxt) const override; + + double get_alignment_start(const alignment align) const override; protected: void fill_background(); diff --git a/include/components/renderer_interface.hpp b/include/components/renderer_interface.hpp index d0daa203..dd43bb3f 100644 --- a/include/components/renderer_interface.hpp +++ b/include/components/renderer_interface.hpp @@ -7,23 +7,36 @@ POLYBAR_NS class renderer_interface { public: - renderer_interface(tags::action_context& action_ctxt) : m_action_ctxt(action_ctxt){}; + renderer_interface(const tags::action_context& action_ctxt) : m_action_ctxt(action_ctxt){}; virtual void render_offset(const tags::context& ctxt, int pixels) = 0; virtual void render_text(const tags::context& ctxt, const string&& str) = 0; virtual void change_alignment(const tags::context& ctxt) = 0; - virtual void action_open(const tags::context& ctxt, tags::action_t id) = 0; - virtual void action_close(const tags::context& ctxt, tags::action_t id) = 0; + /** + * Get the current x-coordinate of the renderer. + * + * This position denotes the coordinate where the next thing will be rendered. + * It is relative to the start of the current alignment because the absolute + * positions may not be known until after the renderer has finished. + */ + virtual double get_x(const tags::context& ctxt) const = 0; + + /** + * Get the absolute x-position of the start of an alignment block. + * + * The position is absolute in terms of the bar window. + * + * Only call this after all the rendering is finished as these values change + * when new things are rendered. + */ + virtual double get_alignment_start(const alignment align) const = 0; protected: /** - * Stores information about actions in the current action cycle. - * - * The renderer is only responsible for updating the start and end positions - * of each action block. + * Stores information about actions in the current render cycle. */ - tags::action_context& m_action_ctxt; + const tags::action_context& m_action_ctxt; }; POLYBAR_NS_END diff --git a/include/tags/context.hpp b/include/tags/context.hpp index d202e950..424ee9d0 100644 --- a/include/tags/context.hpp +++ b/include/tags/context.hpp @@ -98,23 +98,47 @@ namespace tags { static constexpr action_t NO_ACTION = -1; + /** + * Defines a clickable (or scrollable) action block. + * + * An action block is an area on the bar that executes some command when clicked. + */ struct action_block { action_block(const string&& cmd, mousebtn button, alignment align, bool is_open) : cmd(std::move(cmd)), button(button), align(align), is_open(is_open){}; string cmd; + /** + * Start position of the action block (inclusive), relative to the alignment. + */ double start_x{0}; + + /** + * End position of the action block (exclusive), relative to the alignment. + */ double end_x{0}; mousebtn button; alignment align; + /** + * Tracks whether this block is still open or whether it already has a + * corresponding closing tag. + * + * After rendering, all action blocks should be closed. + */ bool is_open; unsigned int width() const { return static_cast(end_x - start_x + 0.5); } - bool test(int point) const { - return static_cast(start_x) <= point && static_cast(end_x) > point; + /** + * Tests whether a given point is inside this block. + * + * This additionally needs the position of the start of the alignment + * because the given point is relative to the bar window. + */ + bool test(double align_start, int point) const { + return static_cast(start_x + align_start) <= point && static_cast(end_x + align_start) > point; } }; @@ -122,11 +146,10 @@ namespace tags { public: void reset(); - action_t action_open(mousebtn btn, const string&& cmd, alignment align); - std::pair action_close(mousebtn btn, alignment align); + action_t action_open(mousebtn btn, const string&& cmd, alignment align, double x); + std::pair action_close(mousebtn btn, alignment align, double x); - void set_start(action_t id, double x); - void set_end(action_t id, double x); + void set_alignmnent_start(const alignment a, const double x); std::map get_actions(int x) const; action_t has_action(mousebtn btn, int x) const; @@ -137,15 +160,28 @@ namespace tags { size_t num_actions() const; // TODO provide better interface for adjusting the start positions of actions - std::vector& get_blocks(); + const std::vector& get_blocks() const; protected: + void set_start(action_t id, double x); + void set_end(action_t id, double x); + /** * Stores all currently known action blocks. * - * The action_t type is meant as an index into this vector. + * The action_t type is an index into this vector. */ std::vector m_action_blocks; + + /** + * Stores the x-coordinate for the start of all the alignment blocks. + * + * This is needed because the action block coordinates are relative to the + * alignment blocks and thus need the alignment block coordinates for + * intersection tests. + */ + std::map m_align_start{ + {alignment::NONE, 0}, {alignment::LEFT, 0}, {alignment::CENTER, 0}, {alignment::RIGHT, 0}}; }; } // namespace tags diff --git a/include/tags/dispatch.hpp b/include/tags/dispatch.hpp index 2fb44877..2ceccbaf 100644 --- a/include/tags/dispatch.hpp +++ b/include/tags/dispatch.hpp @@ -34,7 +34,6 @@ namespace tags { void handle_control(controltag ctrl); private: - vector m_actions; const logger& m_log; unique_ptr m_ctxt; diff --git a/src/components/renderer.cpp b/src/components/renderer.cpp index 5eea227f..83feacf2 100644 --- a/src/components/renderer.cpp +++ b/src/components/renderer.cpp @@ -251,19 +251,6 @@ void renderer::begin(xcb_rectangle_t rect) { void renderer::end() { m_log.trace_x("renderer: end"); - /* - * Finalize the positions of the action blocks. - * Up until this point, the positions were relative to the start of the - * alignment block the action was located in. - * Here we add the start position of the block as well as the start position - * of the bar itself (without borders or tray) to create positions relative to - * the bar window. - */ - for (auto& a : m_action_ctxt.get_blocks()) { - a.start_x += block_x(a.align) + m_rect.x; - a.end_x += block_x(a.align) + m_rect.x; - } - if (m_align != alignment::NONE) { m_log.trace_x("renderer: pop(%i)", static_cast(m_align)); m_context->pop(&m_blocks[m_align].pattern); @@ -706,12 +693,12 @@ void renderer::change_alignment(const tags::context& ctxt) { } } -void renderer::action_open(const tags::context&, tags::action_t id) { - m_action_ctxt.set_start(id, m_blocks.at(m_align).x); +double renderer::get_x(const tags::context& ctxt) const { + return m_blocks.at(ctxt.get_alignment()).x; } -void renderer::action_close(const tags::context&, tags::action_t id) { - m_action_ctxt.set_end(id, m_blocks.at(m_align).x); +double renderer::get_alignment_start(const alignment align) const { + return block_x(align) + m_rect.x; } /** diff --git a/src/tags/context.cpp b/src/tags/context.cpp index 5aeb704f..ec0873e3 100644 --- a/src/tags/context.cpp +++ b/src/tags/context.cpp @@ -118,19 +118,22 @@ namespace tags { m_action_blocks.clear(); } - action_t action_context::action_open(mousebtn btn, const string&& cmd, alignment align) { + action_t action_context::action_open(mousebtn btn, const string&& cmd, alignment align, double x) { action_t id = m_action_blocks.size(); m_action_blocks.emplace_back(std::move(cmd), btn, align, true); + set_start(id, x); return id; } - std::pair action_context::action_close(mousebtn btn, alignment align) { + std::pair action_context::action_close(mousebtn btn, alignment align, double x) { for (auto it = m_action_blocks.rbegin(); it != m_action_blocks.rend(); it++) { if (it->is_open && it->align == align && (btn == mousebtn::NONE || it->button == btn)) { it->is_open = false; // Converts a reverse iterator into an index - return {std::distance(m_action_blocks.begin(), it.base()) - 1, it->button}; + action_t id = std::distance(m_action_blocks.begin(), it.base()) - 1; + set_end(id, x); + return {id, it->button}; } } @@ -145,6 +148,10 @@ namespace tags { m_action_blocks[id].end_x = x; } + void action_context::set_alignmnent_start(const alignment a, const double x) { + m_align_start[a] = x; + } + std::map action_context::get_actions(int x) const { std::map buttons; @@ -157,7 +164,7 @@ namespace tags { mousebtn btn = action.button; // Higher IDs are higher in the action stack. - if (id > buttons[btn] && action.test(x)) { + if (id > buttons[btn] && action.test(m_align_start.at(action.align), x)) { buttons[action.button] = id; } } @@ -191,7 +198,7 @@ namespace tags { return m_action_blocks.size(); } - std::vector& action_context::get_blocks() { + const std::vector& action_context::get_blocks() const { return m_action_blocks; } diff --git a/src/tags/dispatch.cpp b/src/tags/dispatch.cpp index 66c7a96d..222df632 100644 --- a/src/tags/dispatch.cpp +++ b/src/tags/dispatch.cpp @@ -100,6 +100,10 @@ namespace tags { } } + for (auto a : {alignment::LEFT, alignment::CENTER, alignment::RIGHT}) { + m_action_ctxt.set_alignmnent_start(a, renderer.get_alignment_start(a)); + } + // TODO handle unclosed action tags in ctxt /* if (!m_actions.empty()) { */ /* throw runtime_error(to_string(m_actions.size()) + " unclosed action block(s)"); */ @@ -122,12 +126,9 @@ namespace tags { void dispatch::handle_action(renderer_interface& renderer, mousebtn btn, bool closing, const string&& cmd) { if (closing) { - action_t id; - std::tie(id, btn) = m_action_ctxt.action_close(btn, m_ctxt->get_alignment()); - renderer.action_close(*m_ctxt, id); + m_action_ctxt.action_close(btn, m_ctxt->get_alignment(), renderer.get_x(*m_ctxt)); } else { - action_t id = m_action_ctxt.action_open(btn, std::move(cmd), m_ctxt->get_alignment()); - renderer.action_open(*m_ctxt, id); + m_action_ctxt.action_open(btn, std::move(cmd), m_ctxt->get_alignment(), renderer.get_x(*m_ctxt)); } } diff --git a/tests/unit_tests/tags/action_context.cpp b/tests/unit_tests/tags/action_context.cpp index 8f61c27f..a5be8414 100644 --- a/tests/unit_tests/tags/action_context.cpp +++ b/tests/unit_tests/tags/action_context.cpp @@ -1,7 +1,6 @@ -#include "tags/context.hpp" - #include "common/test.hpp" #include "components/logger.hpp" +#include "tags/context.hpp" using namespace polybar; using namespace std; @@ -10,20 +9,20 @@ using namespace tags; TEST(ActionCtxtTest, dblClick) { action_context ctxt; - ctxt.action_open(mousebtn::DOUBLE_LEFT, "", alignment::LEFT); - ctxt.action_close(mousebtn::DOUBLE_LEFT, alignment::LEFT); + ctxt.action_open(mousebtn::DOUBLE_LEFT, "", alignment::LEFT, 0); + ctxt.action_close(mousebtn::DOUBLE_LEFT, alignment::LEFT, 1); ASSERT_TRUE(ctxt.has_double_click()); ctxt.reset(); - ctxt.action_open(mousebtn::DOUBLE_MIDDLE, "", alignment::LEFT); - ctxt.action_close(mousebtn::DOUBLE_MIDDLE, alignment::LEFT); + ctxt.action_open(mousebtn::DOUBLE_MIDDLE, "", alignment::LEFT, 0); + ctxt.action_close(mousebtn::DOUBLE_MIDDLE, alignment::LEFT, 1); ASSERT_TRUE(ctxt.has_double_click()); - ctxt.action_open(mousebtn::DOUBLE_RIGHT, "", alignment::LEFT); - ctxt.action_close(mousebtn::DOUBLE_RIGHT, alignment::LEFT); + ctxt.action_open(mousebtn::DOUBLE_RIGHT, "", alignment::LEFT, 0); + ctxt.action_close(mousebtn::DOUBLE_RIGHT, alignment::LEFT, 1); ASSERT_TRUE(ctxt.has_double_click()); } @@ -31,20 +30,20 @@ TEST(ActionCtxtTest, dblClick) { TEST(ActionCtxtTest, closing) { action_context ctxt; - auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT); - auto id2 = ctxt.action_open(mousebtn::RIGHT, "", alignment::CENTER); - auto id3 = ctxt.action_open(mousebtn::RIGHT, "", alignment::LEFT); - auto id4 = ctxt.action_open(mousebtn::MIDDLE, "", alignment::LEFT); + auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT, 0); + auto id2 = ctxt.action_open(mousebtn::RIGHT, "", alignment::CENTER, 0); + auto id3 = ctxt.action_open(mousebtn::RIGHT, "", alignment::LEFT, 0); + auto id4 = ctxt.action_open(mousebtn::MIDDLE, "", alignment::LEFT, 0); EXPECT_NE(NO_ACTION, id1); EXPECT_NE(NO_ACTION, id2); EXPECT_NE(NO_ACTION, id3); EXPECT_NE(NO_ACTION, id4); - EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::LEFT, alignment::LEFT)); - EXPECT_EQ(make_pair(id4, mousebtn::MIDDLE), ctxt.action_close(mousebtn::NONE, alignment::LEFT)); - EXPECT_EQ(make_pair(id3, mousebtn::RIGHT), ctxt.action_close(mousebtn::NONE, alignment::LEFT)); - EXPECT_EQ(make_pair(id2, mousebtn::RIGHT), ctxt.action_close(mousebtn::NONE, alignment::CENTER)); + EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::LEFT, alignment::LEFT, 1)); + EXPECT_EQ(make_pair(id4, mousebtn::MIDDLE), ctxt.action_close(mousebtn::NONE, alignment::LEFT, 1)); + EXPECT_EQ(make_pair(id3, mousebtn::RIGHT), ctxt.action_close(mousebtn::NONE, alignment::LEFT, 1)); + EXPECT_EQ(make_pair(id2, mousebtn::RIGHT), ctxt.action_close(mousebtn::NONE, alignment::CENTER, 1)); EXPECT_EQ(4, ctxt.num_actions()); } @@ -63,19 +62,12 @@ TEST(ActionCtxtTest, overlapping) { * clang-format on */ - auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT); - auto id2 = ctxt.action_open(mousebtn::MIDDLE, "", alignment::LEFT); - auto id3 = ctxt.action_open(mousebtn::RIGHT, "", alignment::LEFT); - EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::LEFT, alignment::LEFT)); - EXPECT_EQ(make_pair(id3, mousebtn::RIGHT), ctxt.action_close(mousebtn::RIGHT, alignment::LEFT)); - EXPECT_EQ(make_pair(id2, mousebtn::MIDDLE), ctxt.action_close(mousebtn::MIDDLE, alignment::LEFT)); - - ctxt.set_start(id1, 0); - ctxt.set_end(id1, 3); - ctxt.set_start(id2, 1); - ctxt.set_end(id2, 6); - ctxt.set_start(id3, 2); - ctxt.set_end(id3, 5); + auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT, 0); + auto id2 = ctxt.action_open(mousebtn::MIDDLE, "", alignment::LEFT, 1); + auto id3 = ctxt.action_open(mousebtn::RIGHT, "", alignment::LEFT, 2); + EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::LEFT, alignment::LEFT, 3)); + EXPECT_EQ(make_pair(id3, mousebtn::RIGHT), ctxt.action_close(mousebtn::RIGHT, alignment::LEFT, 6)); + EXPECT_EQ(make_pair(id2, mousebtn::MIDDLE), ctxt.action_close(mousebtn::MIDDLE, alignment::LEFT, 5)); auto actions = ctxt.get_actions(2); @@ -100,19 +92,12 @@ TEST(ActionCtxtTest, stacking) { * clang-format on */ - auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT); - auto id2 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT); - auto id3 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT); - EXPECT_EQ(make_pair(id3, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT)); - EXPECT_EQ(make_pair(id2, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT)); - EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT)); - - ctxt.set_start(id1, 0); - ctxt.set_end(id1, 8); - ctxt.set_start(id2, 1); - ctxt.set_end(id2, 7); - ctxt.set_start(id3, 3); - ctxt.set_end(id3, 6); + auto id1 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT, 0); + auto id2 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT, 1); + auto id3 = ctxt.action_open(mousebtn::LEFT, "", alignment::LEFT, 3); + EXPECT_EQ(make_pair(id3, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT, 6)); + EXPECT_EQ(make_pair(id2, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT, 7)); + EXPECT_EQ(make_pair(id1, mousebtn::LEFT), ctxt.action_close(mousebtn::NONE, alignment::LEFT, 8)); EXPECT_EQ(id1, ctxt.has_action(mousebtn::LEFT, 0)); EXPECT_EQ(id2, ctxt.has_action(mousebtn::LEFT, 1)); @@ -131,7 +116,7 @@ TEST(ActionCtxtTest, cmd) { string cmd = "foobar"; - auto id = ctxt.action_open(mousebtn::DOUBLE_RIGHT, cmd.substr(), alignment::RIGHT); + auto id = ctxt.action_open(mousebtn::DOUBLE_RIGHT, cmd.substr(), alignment::RIGHT, 0); ASSERT_EQ(cmd, ctxt.get_action(id)); } diff --git a/tests/unit_tests/tags/dispatch.cpp b/tests/unit_tests/tags/dispatch.cpp index a3c5c35f..5e455a7d 100644 --- a/tests/unit_tests/tags/dispatch.cpp +++ b/tests/unit_tests/tags/dispatch.cpp @@ -8,8 +8,8 @@ using namespace polybar; using namespace std; using namespace tags; -using ::testing::InSequence; using ::testing::_; +using ::testing::InSequence; class MockRenderer : public renderer_interface { public: @@ -18,8 +18,8 @@ class MockRenderer : public renderer_interface { MOCK_METHOD(void, render_offset, (const context& ctxt, int pixels), (override)); MOCK_METHOD(void, render_text, (const context& ctxt, const string&& str), (override)); MOCK_METHOD(void, change_alignment, (const context& ctxt), (override)); - MOCK_METHOD(void, action_open, (const context& ctxt, action_t id), (override)); - MOCK_METHOD(void, action_close, (const context& ctxt, action_t id), (override)); + MOCK_METHOD(double, get_x, (const context& ctxt), (const, override)); + MOCK_METHOD(double, get_alignment_start, (const alignment align), (const, override)); }; class TestableDispatch : public dispatch {};