From 5fd62edfca7ad1c60df1e44d20db0d97f22ee028 Mon Sep 17 00:00:00 2001 From: patrick96 Date: Wed, 14 Sep 2022 21:53:02 +0200 Subject: [PATCH] tray: Cleanup logging --- include/components/types.hpp | 4 +- include/x11/tray_client.hpp | 7 ++++ include/x11/tray_manager.hpp | 2 +- include/x11/xembed.hpp | 2 + src/components/screen.cpp | 1 + src/x11/tray_client.cpp | 43 ++++++++++++++------ src/x11/tray_manager.cpp | 76 ++++++++++++++++-------------------- src/x11/xembed.cpp | 6 +++ 8 files changed, 83 insertions(+), 58 deletions(-) diff --git a/include/components/types.hpp b/include/components/types.hpp index 3f9f36fd..9c152654 100644 --- a/include/components/types.hpp +++ b/include/components/types.hpp @@ -85,8 +85,8 @@ struct position { }; struct size { - unsigned int w{1U}; - unsigned int h{1U}; + unsigned w{1U}; + unsigned h{1U}; }; enum class spacing_type { SPACE, POINT, PIXEL }; diff --git a/include/x11/tray_client.hpp b/include/x11/tray_client.hpp index dad6e39b..84fa44f8 100644 --- a/include/x11/tray_client.hpp +++ b/include/x11/tray_client.hpp @@ -25,6 +25,8 @@ class tray_client : public non_copyable_mixin { tray_client(tray_client&&); tray_client& operator=(tray_client&&); + string name() const; + unsigned int width() const; unsigned int height() const; void clear_window() const; @@ -58,6 +60,11 @@ class tray_client : public non_copyable_mixin { connection& m_connection; + /** + * Name of the client window for debugging. + */ + string m_name{}; + /** * Embedder window. * diff --git a/include/x11/tray_manager.hpp b/include/x11/tray_manager.hpp index df67a62b..9b7c1cff 100644 --- a/include/x11/tray_manager.hpp +++ b/include/x11/tray_manager.hpp @@ -192,7 +192,7 @@ class tray_manager : public xpp::event::sink m_hidden{false}; + bool m_hidden{false}; thread m_delaythread; diff --git a/include/x11/xembed.hpp b/include/x11/xembed.hpp index b663d74c..fa48c486 100644 --- a/include/x11/xembed.hpp +++ b/include/x11/xembed.hpp @@ -42,6 +42,8 @@ namespace xembed { bool is_mapped() const; + string to_string() const; + protected: uint32_t version; uint32_t flags; diff --git a/src/components/screen.cpp b/src/components/screen.cpp index c79fe90a..fd5815d8 100644 --- a/src/components/screen.cpp +++ b/src/components/screen.cpp @@ -66,6 +66,7 @@ screen::screen(connection& conn, signal_emitter& emitter, const logger& logger, 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; }); diff --git a/src/x11/tray_client.cpp b/src/x11/tray_client.cpp index 9c489b28..f13fcd51 100644 --- a/src/x11/tray_client.cpp +++ b/src/x11/tray_client.cpp @@ -5,6 +5,7 @@ #include "utils/memory.hpp" #include "x11/connection.hpp" +#include "x11/ewmh.hpp" #include "x11/winspec.hpp" POLYBAR_NS @@ -19,14 +20,14 @@ POLYBAR_NS * 3. Use background color */ tray_client::tray_client(const logger& log, connection& conn, xcb_window_t tray, xcb_window_t win, size s) - : m_log(log), m_connection(conn), m_client(win), m_size(s) { + : m_log(log), m_connection(conn), m_name(ewmh_util::get_wm_name(win)), m_client(win), m_size(s) { auto geom = conn.get_geometry(win); auto attrs = conn.get_window_attributes(win); int client_depth = geom->depth; auto client_visual = attrs->visual; auto client_colormap = attrs->colormap; - m_log.trace("tray(%s): depth: %u, width: %u, height: %u", conn.id(win), client_depth, geom->width, geom->height); + m_log.trace("%s: depth: %u, width: %u, height: %u", name(), client_depth, geom->width, geom->height); /* * Create embedder window for tray icon @@ -69,6 +70,7 @@ tray_client::~tray_client() { } tray_client::tray_client(tray_client&& c) : m_log(c.m_log), m_connection(c.m_connection) { + std::swap(m_name, c.m_name); std::swap(m_wrapper, c.m_wrapper); std::swap(m_client, c.m_client); std::swap(m_xembed_supported, c.m_xembed_supported); @@ -81,6 +83,7 @@ tray_client::tray_client(tray_client&& c) : m_log(c.m_log), m_connection(c.m_con tray_client& tray_client::operator=(tray_client&& c) { m_log = c.m_log; m_connection = c.m_connection; + std::swap(m_name, c.m_name); std::swap(m_wrapper, c.m_wrapper); std::swap(m_client, c.m_client); std::swap(m_xembed_supported, c.m_xembed_supported); @@ -91,6 +94,10 @@ tray_client& tray_client::operator=(tray_client&& c) { return *this; } +string tray_client::name() const { + return "tray_client(" + m_connection.id(m_client) + ", " + m_name + ")"; +} + unsigned int tray_client::width() const { return m_size.w; } @@ -118,12 +125,12 @@ void tray_client::update_client_attributes() const { connection::pack_values(configure_mask, &configure_params, configure_values); - m_log.trace("tray(%s): Update client window", m_connection.id(client())); + m_log.trace("%s: Update client window", name()); m_connection.change_window_attributes_checked(client(), configure_mask, configure_values.data()); } void tray_client::reparent() const { - m_log.trace("tray(%s): Reparent client", m_connection.id(client())); + m_log.trace("%s: Reparent client", name()); m_connection.reparent_window_checked(client(), embedder(), 0, 0); } @@ -145,8 +152,10 @@ bool tray_client::mapped() const { * Set client window mapped state */ void tray_client::mapped(bool state) { - m_log.trace("tray(%s): set mapped: %i", m_connection.id(client()), state); - m_mapped = state; + if (m_mapped != state) { + m_log.trace("%s: set mapped: %i", name(), state); + m_mapped = state; + } } /** @@ -168,6 +177,12 @@ xcb_window_t tray_client::client() const { void tray_client::query_xembed() { m_xembed_supported = xembed::query(m_connection, m_client, m_xembed); + + if (is_xembed_supported()) { + m_log.trace("%s: %s", name(), get_xembed().to_string()); + } else { + m_log.trace("%s: no xembed"); + } } bool tray_client::is_xembed_supported() const { @@ -180,13 +195,13 @@ const xembed::info& tray_client::get_xembed() const { void tray_client::notify_xembed() const { if (is_xembed_supported()) { - m_log.trace("tray(%s): Send embbeded notification to client", m_connection.id(client())); + m_log.trace("%s: Send embbeded notification to client", name()); xembed::notify_embedded(m_connection, client(), embedder(), m_xembed.get_version()); } } void tray_client::add_to_save_set() const { - m_log.trace("tray(%s): Add client window to the save set", m_connection.id(client())); + m_log.trace("%s: Add client window to the save set", name()); m_connection.change_save_set_checked(XCB_SET_MODE_INSERT, client()); } @@ -204,15 +219,17 @@ void tray_client::ensure_state() const { should_be_mapped = false; } - m_log.trace("tray(%s): ensure_state (hidden=%i, mapped=%i, should_be_mapped=%i)", m_connection.id(client()), m_hidden, - m_mapped, should_be_mapped); + // TODO can we stop here if should_be_mapped == m_mapped? + + m_log.trace( + "%s: ensure_state (hidden=%i, mapped=%i, should_be_mapped=%i)", name(), m_hidden, m_mapped, should_be_mapped); if (should_be_mapped) { - m_log.trace("tray(%s): Map client", m_connection.id(client())); + m_log.trace("%s: Map client", name()); m_connection.map_window_checked(embedder()); m_connection.map_window_checked(client()); } else { - m_log.trace("tray(%s): Unmap client", m_connection.id(client())); + m_log.trace("%s: Unmap client", name()); m_connection.unmap_window_checked(client()); m_connection.unmap_window_checked(embedder()); } @@ -222,7 +239,7 @@ void tray_client::ensure_state() const { * Configure window size */ void tray_client::reconfigure(int x, int y) const { - m_log.trace("tray(%s): moving to (%d, %d)", m_connection.id(client()), x, y); + m_log.trace("%s: moving to (%d, %d)", name(), x, y); uint32_t configure_mask = 0; std::array configure_values{}; diff --git a/src/x11/tray_manager.cpp b/src/x11/tray_manager.cpp index c38e3fda..e58fe819 100644 --- a/src/x11/tray_manager.cpp +++ b/src/x11/tray_manager.cpp @@ -123,7 +123,7 @@ void tray_manager::activate() { return; } - m_log.info("Activating tray manager"); + m_log.info("tray: Activating tray manager"); try { set_tray_colors(); @@ -134,9 +134,8 @@ void tray_manager::activate() { return; } - xcb_window_t other_owner = XCB_NONE; - // Attempt to get control of the systray selection + xcb_window_t other_owner = XCB_NONE; if (!acquire_selection(other_owner)) { // Transition to WAITING state wait_for_selection(other_owner); @@ -150,6 +149,7 @@ void tray_manager::activate() { m_state = state::ACTIVE; // Send delayed notification + // TODO try to remove this? if (!m_firstactivation) { notify_clients(); } else { @@ -194,7 +194,7 @@ void tray_manager::deactivate() { return; } - m_log.info("Deactivating tray manager"); + m_log.info("tray: Deactivating tray manager"); m_sig.detach(this); @@ -241,7 +241,7 @@ void tray_manager::reconfigure() { * TODO should we call update_width directly? */ void tray_manager::reconfigure_window() { - m_log.trace("tray: Reconfigure window (hidden=%i, clients=%i)", static_cast(m_hidden), m_clients.size()); + m_log.trace("tray: Reconfigure window (hidden=%i, clients=%i)", m_hidden, m_clients.size()); update_width(); } @@ -252,6 +252,7 @@ void tray_manager::update_width() { unsigned new_width = calculate_w(); if (m_tray_width != new_width) { m_tray_width = new_width; + m_log.trace("tray: new width (width: %d, clients: %d)", m_tray_width, m_clients.size()); m_on_update(); } } @@ -272,7 +273,7 @@ void tray_manager::reconfigure_clients() { x += m_opts.client_size.w + m_opts.spacing; } catch (const xpp::x::error::window& err) { // TODO print error - m_log.err("Failed to reconfigure client (%s), removing ... (%s)", m_connection.id(it->client()), err.what()); + m_log.err("Failed to reconfigure %s, removing ... (%s)", it->name(), err.what()); remove_client(*it, false); } } @@ -282,6 +283,7 @@ void tray_manager::reconfigure_clients() { * Refresh the bar window by clearing it along with each client window */ void tray_manager::refresh_window() { + // TODO create method that checks is_active and !m_hidden if (!is_active() || m_hidden) { return; } @@ -294,8 +296,7 @@ void tray_manager::refresh_window() { client.clear_window(); } } catch (const std::exception& e) { - m_log.err("Failed to clear tray client %s '%s' (%s)", ewmh_util::get_wm_name(client.client()), - m_connection.id(client.client()), e.what()); + m_log.err("tray: Failed to clear %s (%s)", client.name(), e.what()); } } @@ -304,9 +305,10 @@ void tray_manager::refresh_window() { /** * Redraw window + * + * TODO better name */ void tray_manager::redraw_window() { - m_log.info("Redraw tray container"); refresh_window(); } @@ -377,7 +379,7 @@ bool tray_manager::acquire_selection(xcb_window_t& other_owner) { */ void tray_manager::notify_clients() { if (is_active()) { - m_log.info("Notifying pending tray clients"); + m_log.info("tray: Notifying pending tray clients"); auto message = m_connection.make_client_message(MANAGER, m_connection.root()); message.data.data32[0] = XCB_CURRENT_TIME; message.data.data32[1] = m_atom; @@ -405,7 +407,6 @@ void tray_manager::notify_clients_delayed() { */ void tray_manager::track_selection_owner(xcb_window_t owner) { if (owner != XCB_NONE) { - m_log.trace("tray: Listen for events on the new selection window"); const unsigned mask{XCB_CW_EVENT_MASK}; const unsigned values[]{XCB_EVENT_MASK_STRUCTURE_NOTIFY}; m_connection.change_window_attributes(owner, mask, values); @@ -416,7 +417,7 @@ void tray_manager::track_selection_owner(xcb_window_t owner) { * Process client docking request */ void tray_manager::process_docking_request(xcb_window_t win) { - m_log.info("Processing docking request from '%s' (%s)", ewmh_util::get_wm_name(win), m_connection.id(win)); + m_log.info("tray: Processing docking request from '%s' (%s)", ewmh_util::get_wm_name(win), m_connection.id(win)); try { tray_client client(m_log, m_connection, m_opts.selection_owner, win, m_opts.client_size); @@ -424,16 +425,10 @@ void tray_manager::process_docking_request(xcb_window_t win) { try { client.query_xembed(); } catch (const xpp::x::error::window& err) { - m_log.err("Failed to query _XEMBED_INFO, removing client... (%s)", err.what()); + m_log.err("Failed to query _XEMBED_INFO, removing %s ... (%s)", client.name(), err.what()); return; } - m_log.trace("tray: xembed = %s", client.is_xembed_supported() ? "true" : "false"); - if (client.is_xembed_supported()) { - m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client.get_xembed().get_version(), - client.get_xembed().get_flags(), client.get_xembed().is_mapped() ? "true" : "false"); - } - client.update_client_attributes(); client.reparent(); @@ -446,8 +441,7 @@ void tray_manager::process_docking_request(xcb_window_t win) { m_clients.emplace_back(std::move(client)); } catch (const std::exception& err) { - m_log.err("Failed to setup tray client '%s' (%s) removing... (%s)", ewmh_util::get_wm_name(win), - m_connection.id(win), err.what()); + m_log.err("tray: Failed to setup tray client removing... (%s)", err.what()); return; } } @@ -519,6 +513,7 @@ void tray_manager::remove_client(xcb_window_t win, bool reconfigure) { m_clients.erase( std::remove_if(m_clients.begin(), m_clients.end(), [win](const auto& client) { return client.match(win); })); + // TODO remove param. Reconfigure if clients were deleted if (reconfigure) { tray_manager::reconfigure(); } @@ -529,8 +524,7 @@ bool tray_manager::change_visibility(bool visible) { return false; } - m_log.trace("tray: visibility_change (state=%i, activated=%i, hidden=%i)", visible, static_cast(is_active()), - static_cast(m_hidden)); + m_log.trace("tray: visibility_change (new_state)", visible ? "visible" : "hidden"); m_hidden = !visible; @@ -561,6 +555,7 @@ void tray_manager::handle(const evt::expose& evt) { * Event callback : XCB_VISIBILITY_NOTIFY */ void tray_manager::handle(const evt::visibility_notify& evt) { + // TODO for which windows is this important? if (is_active() && !m_clients.empty()) { m_log.trace("tray: Received visibility_notify for %s", m_connection.id(evt->window)); reconfigure_window(); @@ -577,7 +572,7 @@ void tray_manager::handle(const evt::client_message& evt) { // Our selection owner window was deleted if (evt->type == WM_PROTOCOLS && evt->data.data32[0] == WM_DELETE_WINDOW && evt->window == m_opts.selection_owner) { - m_log.notice("Received WM_DELETE"); + m_log.notice("Received WM_DELETE for selection owner"); deactivate(); } else if (evt->type == _NET_SYSTEM_TRAY_OPCODE && evt->format == 32) { m_log.trace("tray: Received client_message"); @@ -603,11 +598,12 @@ void tray_manager::handle(const evt::client_message& evt) { */ void tray_manager::handle(const evt::configure_request& evt) { if (is_active() && is_embedded(evt->window)) { + auto client = find_client(evt->window); try { - m_log.trace("tray: Client configure request %s", m_connection.id(evt->window)); - find_client(evt->window)->configure_notify(); + m_log.trace("%s: Client configure request", client->name()); + client->configure_notify(); } catch (const xpp::x::error::window& err) { - m_log.err("Failed to reconfigure tray client, removing... (%s)", err.what()); + m_log.err("Failed to reconfigure %s, removing... (%s)", client->name(), err.what()); remove_client(evt->window); } } @@ -618,11 +614,12 @@ void tray_manager::handle(const evt::configure_request& evt) { */ void tray_manager::handle(const evt::resize_request& evt) { if (is_active() && is_embedded(evt->window)) { + auto client = find_client(evt->window); try { - m_log.trace("tray: Received resize_request for client %s", m_connection.id(evt->window)); - find_client(evt->window)->configure_notify(); + m_log.trace("%s: Client resize request", client->name()); + client->configure_notify(); } catch (const xpp::x::error::window& err) { - m_log.err("Failed to reconfigure tray client, removing... (%s)", err.what()); + m_log.err("Failed to reconfigure %s, removing... (%s)", client->name(), err.what()); remove_client(evt->window); } } @@ -662,7 +659,7 @@ void tray_manager::handle(const evt::property_notify& evt) { return; } - m_log.trace("tray: _XEMBED_INFO: %s", m_connection.id(evt->window)); + m_log.trace("%s: _XEMBED_INFO: %s", client->name()); if (evt->state == XCB_PROPERTY_NEW_VALUE) { m_log.trace("tray: _XEMBED_INFO value has changed"); @@ -671,14 +668,12 @@ void tray_manager::handle(const evt::property_notify& evt) { try { client->query_xembed(); } catch (const xpp::x::error::window& err) { - m_log.err("Failed to query _XEMBED_INFO, removing client... (%s)", err.what()); + m_log.err("Failed to query _XEMBED_INFO, removing %s ... (%s)", client->name(), err.what()); remove_client(*client, true); return; } - m_log.trace("tray: version = 0x%x, flags = 0x%x, XEMBED_MAPPED = %s", client->get_xembed().get_version(), - client->get_xembed().get_flags(), client->get_xembed().is_mapped() ? "true" : "false"); - + // TODO only reconfigure if should_be_mapped changed if (client->get_xembed().is_mapped()) { reconfigure(); } @@ -700,7 +695,7 @@ void tray_manager::handle(const evt::reparent_notify& evt) { // Tray client was reparented to another window if (evt->parent != client->embedder()) { - m_log.info("tray: Received reparent_notify for client, remove..."); + m_log.info("%s: Received reparent_notify for client, remove...", client->name()); remove_client(*client); } } @@ -724,13 +719,11 @@ void tray_manager::handle(const evt::destroy_notify& evt) { */ void tray_manager::handle(const evt::map_notify& evt) { if (is_active() && evt->window == m_opts.selection_owner) { - m_log.trace("tray: Received map_notify"); - m_log.trace("tray: Update container mapped flag"); + m_log.trace("tray: Received map_notify for selection owner"); redraw_window(); } else if (is_embedded(evt->window)) { - m_log.trace("tray: Received map_notify"); - m_log.trace("tray: Set client mapped"); auto client = find_client(evt->window); + m_log.trace("%s: Received map_notify", client->name()); if (!client->mapped()) { client->mapped(true); @@ -744,9 +737,8 @@ void tray_manager::handle(const evt::map_notify& evt) { */ void tray_manager::handle(const evt::unmap_notify& evt) { if (is_active() && is_embedded(evt->window)) { - m_log.trace("tray: Received unmap_notify"); - m_log.trace("tray: Set client unmapped"); auto client = find_client(evt->window); + m_log.trace("%s: Received unmap_notify", client->name()); if (client->mapped()) { client->mapped(false); diff --git a/src/x11/xembed.cpp b/src/x11/xembed.cpp index 3b490d8e..1584e25a 100644 --- a/src/x11/xembed.cpp +++ b/src/x11/xembed.cpp @@ -3,6 +3,7 @@ #include #include "errors.hpp" +#include "utils/string.hpp" #include "x11/atoms.hpp" #include "x11/ewmh.hpp" @@ -27,6 +28,11 @@ namespace xembed { return (flags & XEMBED_MAPPED) == XEMBED_MAPPED; } + string info::to_string() const { + return sstream() << "xembed(version=0x" << std::hex << get_version() << ", flags=0x" << get_flags() + << ", XEMBED_MAPPED=" << std::dec << is_mapped() << ")"; + } + /** * Query _XEMBED_INFO for the given window *