From 053a7eb2d8c884ae11e32ad8319508bed5bbdeeb Mon Sep 17 00:00:00 2001 From: Patrick Ziegler Date: Mon, 5 Sep 2022 09:58:12 +0200 Subject: [PATCH] fix: Crash on root pixmap and window depth mismatch (#2813) The background_manager used the root window depth for creating its pixmaps, but when the root pixmap has a different depth, this causes the copy_area_checked call to fail and crash the bar. We now load the root pixmap before finding the visual and creating the slice pixmaps. Fixes #2798 --- CHANGELOG.md | 4 +- include/x11/background_manager.hpp | 78 +++++--- src/components/bar.cpp | 5 +- src/x11/background_manager.cpp | 312 ++++++++++++++++++----------- src/x11/tray_manager.cpp | 2 +- 5 files changed, 255 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74fb3f4c..5136ae1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Waiting for double click interval on modules that don't have a double click action ([`#2663`](https://github.com/polybar/polybar/issues/2663), [`#2695`](https://github.com/polybar/polybar/pull/2695)) -- renderer: Small gaps when rendering emojis ([`#2785`](https://github.com/polybar/polybar/issues/2785), [`#2802`](https://github.com/polybar/polybar/pull/2802)) +- renderer: + - Small gaps when rendering emojis ([`#2785`](https://github.com/polybar/polybar/issues/2785), [`#2802`](https://github.com/polybar/polybar/pull/2802)) + - Crash when using pseudo-transparency with certain wallpapers ([`#2798`](https://github.com/polybar/polybar/issues/2798), [`#2813`](https://github.com/polybar/polybar/pull/2813)) - config: - Error reporting for deprecated config values ([`#2724`](https://github.com/polybar/polybar/issues/2724)) - Also monitor include-files for changes when --reload is set ([`#675`](https://github.com/polybar/polybar/issues/675), [`#2759`](https://github.com/polybar/polybar/pull/2759)) diff --git a/include/x11/background_manager.hpp b/include/x11/background_manager.hpp index e3ce3ced..f8d5efe5 100644 --- a/include/x11/background_manager.hpp +++ b/include/x11/background_manager.hpp @@ -16,7 +16,7 @@ class logger; namespace cairo { class surface; class xcb_surface; -} // namespace cairo +} // namespace cairo class bg_slice { public: @@ -25,32 +25,35 @@ class bg_slice { bg_slice(const bg_slice&) = delete; bg_slice& operator=(const bg_slice&) = delete; - /** - * Get the current desktop background at the location of this slice. - * The returned pointer is only valid as long as the slice itself is alive. - * - * This function is fast, since the current desktop background is cached. - */ - cairo::surface* get_surface() const { - return m_surface.get(); - } + cairo::surface* get_surface() const; + + void clear(); + void copy(xcb_pixmap_t root_pixmap, int depth, xcb_rectangle_t geom, xcb_visualtype_t* visual); private: - bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window, xcb_visualtype_t* visual); + bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window); - // standard components connection& m_connection; + const logger& m_log; - // area covered by this slice + /** + * Area covered by this slice + * + * Area is relative to given window + */ xcb_rectangle_t m_rect{0, 0, 0U, 0U}; xcb_window_t m_window; - // cache for the root window background at this slice's position + /** + * Cache for the root window background at this slice's position + */ xcb_pixmap_t m_pixmap{XCB_NONE}; unique_ptr m_surface; xcb_gcontext_t m_gcontext{XCB_NONE}; + int m_depth{0}; - void allocate_resources(const logger& log, xcb_visualtype_t* visual); + void ensure_resources(int depth, xcb_visualtype_t* visual); + void allocate_resources(xcb_visualtype_t* visual); void free_resources(); friend class background_manager; @@ -101,23 +104,52 @@ class background_manager : public signal_receiver> m_slices; - // required values for fetching the root window's background - xcb_visualtype_t* m_visual{nullptr}; - - // true if we are currently attached as a listener for desktop background changes - bool m_attached{false}; - void allocate_resources(); void free_resources(); - void fetch_root_pixmap(); + void on_background_change(); + + void update_slice(bg_slice& slice); + + bool has_pixmap() const; + void ensure_pixmap(); + void load_pixmap(); + void clear_pixmap(); + + /** + * The loaded root pixmap + */ + xcb_pixmap_t m_pixmap{XCB_NONE}; + int m_pixmap_depth{0}; + xcb_rectangle_t m_pixmap_geom{0, 0, 0, 0}; + + /** + * Tracks whether we were able to load a pixmap. + */ + bool m_pixmap_load_failed{false}; + + /** + * Visual matching the root pixmap's depth. + * + * Only valid if m_pixmap is set + */ + xcb_visualtype_t* m_visual{nullptr}; }; POLYBAR_NS_END diff --git a/src/components/bar.cpp b/src/components/bar.cpp index b3f9695d..a41bb9bf 100644 --- a/src/components/bar.cpp +++ b/src/components/bar.cpp @@ -866,7 +866,10 @@ void bar::handle(const evt::property_notify& evt) { } } -void bar::handle(const evt::configure_notify&) { +void bar::handle(const evt::configure_notify& evt) { + if (evt->window != m_opts.window) { + return; + } // The absolute position of the window in the root may be different after configuration is done // (for example, because the parent is not positioned at 0/0 in the root window). // Notify components that the geometry may have changed (used by the background manager for example). diff --git a/src/x11/background_manager.cpp b/src/x11/background_manager.cpp index 8dc2b132..acfd31ff 100644 --- a/src/x11/background_manager.cpp +++ b/src/x11/background_manager.cpp @@ -1,13 +1,16 @@ -#include "cairo/surface.hpp" +#include "x11/background_manager.hpp" + +#include + #include "cairo/context.hpp" -#include "events/signal.hpp" +#include "cairo/surface.hpp" #include "components/config.hpp" #include "components/logger.hpp" -#include "x11/atoms.hpp" -#include "x11/connection.hpp" -#include "x11/background_manager.hpp" +#include "events/signal.hpp" #include "utils/factory.hpp" #include "utils/math.hpp" +#include "x11/atoms.hpp" +#include "x11/connection.hpp" POLYBAR_NS @@ -15,11 +18,8 @@ background_manager& background_manager::make() { return *factory_util::singleton(connection::make(), signal_emitter::make(), logger::make()); } -background_manager::background_manager( - connection& conn, signal_emitter& sig, const logger& log) - : m_connection(conn) - , m_sig(sig) - , m_log(log) { +background_manager::background_manager(connection& conn, signal_emitter& sig, const logger& log) + : m_connection(conn), m_sig(sig), m_log(log) { m_sig.attach(this); } @@ -30,173 +30,245 @@ background_manager::~background_manager() { std::shared_ptr background_manager::observe(xcb_rectangle_t rect, xcb_window_t window) { // allocate a slice - activate(); - auto slice = std::shared_ptr(new bg_slice(m_connection, m_log, rect, window, m_visual)); - - // make sure that we receive a notification when the background changes - if(!m_attached) { - m_connection.ensure_event_mask(m_connection.root(), XCB_EVENT_MASK_PROPERTY_CHANGE); - m_connection.flush(); - m_connection.attach_sink(this, SINK_PRIORITY_SCREEN); - m_attached = true; - } + auto slice = std::shared_ptr(new bg_slice(m_connection, m_log, rect, window)); // if the slice is empty, don't add to slices if (slice->m_rect.width == 0 || slice->m_rect.height == 0) { return slice; } + activate(); + m_slices.push_back(slice); - fetch_root_pixmap(); + + update_slice(*slice); return slice; } +void background_manager::activate() { + attach(); +} + void background_manager::deactivate() { - if(m_attached) { - m_connection.detach_sink(this, SINK_PRIORITY_SCREEN); - m_attached = false; - } + detach(); free_resources(); } +/** + * Attaches a listener to listen for background changes on the root window. + */ +void background_manager::attach() { + if (!m_attached) { + // make sure that we receive a notification when the background changes + m_connection.ensure_event_mask(m_connection.root(), XCB_EVENT_MASK_PROPERTY_CHANGE); + m_connection.flush(); + m_connection.attach_sink(this, SINK_PRIORITY_SCREEN); + m_attached = true; + } +} -void background_manager::activate() { - if(!m_visual) { - m_log.trace("background_manager: Finding root visual"); - m_visual = m_connection.visual_type_for_id(m_connection.screen(), m_connection.screen()->root_visual); - m_log.trace("background_manager: Got root visual with depth %d", m_connection.screen()->root_depth); +/** + * Stops listening for background changes + */ +void background_manager::detach() { + if (m_attached) { + m_connection.detach_sink(this, SINK_PRIORITY_SCREEN); + m_attached = false; } } void background_manager::free_resources() { - m_visual = nullptr; + clear_pixmap(); } -void background_manager::fetch_root_pixmap() { - m_log.trace("background_manager: Fetching pixmap"); +/** + * Changes required when the background may have changed. + * + * 1. Delete cached pixmaps + * 2. Update all slices (loads new pixmaps on demand) + * 3. Notify about new background + */ +void background_manager::on_background_change() { + clear_pixmap(); + m_pixmap_load_failed = false; - int pixmap_depth; - xcb_pixmap_t pixmap; - xcb_rectangle_t pixmap_geom; - - try { - if (!m_connection.root_pixmap(&pixmap, &pixmap_depth, &pixmap_geom)) { - return m_log.warn("background_manager: Failed to get root pixmap, default to black (is there a wallpaper?)"); - }; - m_log.trace("background_manager: root pixmap (%d:%d) %dx%d+%d+%d", pixmap, pixmap_depth, - pixmap_geom.width, pixmap_geom.height, pixmap_geom.x, pixmap_geom.y); - - if (pixmap_depth == 1 && pixmap_geom.width == 1 && pixmap_geom.height == 1) { - return m_log.err("background_manager: Cannot find root pixmap, try a different tool to set the desktop background"); + for (auto it = m_slices.begin(); it != m_slices.end();) { + auto slice = it->lock(); + if (!slice) { + it = m_slices.erase(it); + continue; } - for (auto it = m_slices.begin(); it != m_slices.end(); ) { - auto slice = it->lock(); - if (!slice) { - it = m_slices.erase(it); - continue; - } - - // fill the slice - auto translated = m_connection.translate_coordinates(slice->m_window, m_connection.screen()->root, slice->m_rect.x, slice->m_rect.y); - auto src_x = math_util::cap(translated->dst_x, pixmap_geom.x, int16_t(pixmap_geom.x + pixmap_geom.width)); - auto src_y = math_util::cap(translated->dst_y, pixmap_geom.y, int16_t(pixmap_geom.y + pixmap_geom.height)); - auto w = math_util::cap(slice->m_rect.width, uint16_t(0), uint16_t(pixmap_geom.width - (src_x - pixmap_geom.x))); - auto h = math_util::cap(slice->m_rect.height, uint16_t(0), uint16_t(pixmap_geom.height - (src_y - pixmap_geom.y))); - m_log.trace("background_manager: Copying from root pixmap (%d:%d) %dx%d+%d+%d", pixmap, pixmap_depth, w, h, src_x, src_y); - m_connection.copy_area_checked(pixmap, slice->m_pixmap, slice->m_gcontext, src_x, src_y, 0, 0, w, h); - - it++; - } - - // if there are no active slices, deactivate - if (m_slices.empty()) { - m_log.trace("background_manager: deactivating because there are no slices to observe"); - deactivate(); - } - - } catch(const exception& err) { - m_log.err("background_manager: Failed to copy slice of root pixmap (%s)", err.what()); - throw; + update_slice(*slice); + it++; } -} - -void background_manager::handle(const evt::property_notify& evt) { - // if there are no slices to observe, don't do anything - if(m_slices.empty()) { + // if there are no active slices, deactivate + if (m_slices.empty()) { + m_log.trace("background_manager: deactivating because there are no slices to observe"); + deactivate(); return; } + m_sig.emit(signals::ui::update_background()); +} + +void background_manager::update_slice(bg_slice& slice) { + ensure_pixmap(); + + if (has_pixmap()) { + slice.copy(m_pixmap, m_pixmap_depth, m_pixmap_geom, m_visual); + } else { + slice.clear(); + } +} + +void background_manager::handle(const evt::property_notify& evt) { if (evt->atom == _XROOTPMAP_ID || evt->atom == _XSETROOT_ID || evt->atom == ESETROOT_PMAP_ID) { - fetch_root_pixmap(); - m_sig.emit(signals::ui::update_background()); + m_log.trace("background_manager: root pixmap change"); + on_background_change(); } } bool background_manager::on(const signals::ui::update_geometry&) { - // if there are no slices to observe, don't do anything - if(m_slices.empty()) { - return false; - } - - fetch_root_pixmap(); - m_sig.emit(signals::ui::update_background()); + m_log.trace("background_manager: update_geometry"); + on_background_change(); return false; } +bool background_manager::has_pixmap() const { + return m_pixmap != XCB_NONE; +} -bg_slice::bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window, xcb_visualtype_t* visual) - : m_connection(conn) - , m_rect(rect) - , m_window(window) { - try { - allocate_resources(log, visual); - } catch(...) { - free_resources(); - throw; +void background_manager::ensure_pixmap() { + // Only try to load the root pixmap if we haven't already loaded it and the previous load didn't fail. + if (!has_pixmap() && !m_pixmap_load_failed) { + load_pixmap(); + m_pixmap_load_failed = !has_pixmap(); } } +void background_manager::load_pixmap() { + int old_depth = m_pixmap_depth; + clear_pixmap(); + + try { + if (!m_connection.root_pixmap(&m_pixmap, &m_pixmap_depth, &m_pixmap_geom)) { + m_log.warn("background_manager: Failed to get root pixmap, default to black (is there a wallpaper?)"); + return; + } + } catch (const exception& err) { + m_log.err("background_manager: Failed to get root pixmap, default to black (%s)", err.what()); + clear_pixmap(); + return; + } + + m_log.trace("background_manager: root pixmap (0x%x:%d) %dx%d+%d+%d", m_pixmap, m_pixmap_depth, m_pixmap_geom.width, + m_pixmap_geom.height, m_pixmap_geom.x, m_pixmap_geom.y); + + if (m_pixmap_depth == 1 && m_pixmap_geom.width == 1 && m_pixmap_geom.height == 1) { + m_log.err("background_manager: Cannot find root pixmap, try a different tool to set the desktop background"); + clear_pixmap(); + return; + } + + // Only reload visual if depth changed + if (old_depth != m_pixmap_depth) { + m_visual = m_connection.visual_type(m_connection.screen(), m_pixmap_depth); + + if (!m_visual) { + m_log.err("background_manager: Cannot find visual for root pixmap (depth: %d)", m_pixmap_depth); + clear_pixmap(); + return; + } + } +} + +void background_manager::clear_pixmap() { + if (has_pixmap()) { + m_pixmap = XCB_NONE; + m_pixmap_depth = 0; + m_pixmap_geom = {0, 0, 0, 0}; + m_visual = nullptr; + } +} + +bg_slice::bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window) + : m_connection(conn), m_log(log), m_rect(rect), m_window(window) {} + bg_slice::~bg_slice() { free_resources(); } -void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) { - if(m_pixmap == XCB_NONE) { - log.trace("background_manager: Allocating pixmap"); - m_pixmap = m_connection.generate_id(); - m_connection.create_pixmap(m_connection.screen()->root_depth, m_pixmap, m_window, m_rect.width, m_rect.height); - } +/** + * Get the current desktop background at the location of this slice. + * The returned pointer is only valid as long as the slice itself is alive. + * + * This function is fast, since the current desktop background is cached. + */ +cairo::surface* bg_slice::get_surface() const { + return m_surface.get(); +} - if(m_gcontext == XCB_NONE) { - log.trace("background_manager: Allocating graphics context"); - auto black_pixel = m_connection.screen()->black_pixel; - unsigned int mask = XCB_GC_GRAPHICS_EXPOSURES | XCB_GC_FOREGROUND | XCB_GC_BACKGROUND; - unsigned int value_list[3] = {black_pixel, black_pixel, 0}; - m_gcontext = m_connection.generate_id(); - m_connection.create_gc(m_gcontext, m_pixmap, mask, value_list); - } +void bg_slice::clear() { + ensure_resources(0, nullptr); +} - if(!m_surface) { - log.trace("background_manager: Allocating cairo surface"); - m_surface = make_unique(m_connection, m_pixmap, visual, m_rect.width, m_rect.height); - } +void bg_slice::copy(xcb_pixmap_t root_pixmap, int depth, xcb_rectangle_t geom, xcb_visualtype_t* visual) { + assert(root_pixmap); + assert(depth > 0); + ensure_resources(depth, visual); + assert(m_pixmap); - // fill (to provide a default in the case that fetching the background fails) - xcb_rectangle_t rect{0, 0, m_rect.width, m_rect.height}; - m_connection.poly_fill_rectangle(m_pixmap, m_gcontext, 1, &rect); + // fill the slice + auto translated = m_connection.translate_coordinates(m_window, m_connection.screen()->root, m_rect.x, m_rect.y); + // Coordinates of the slice in the root pixmap + auto src_x = math_util::cap(translated->dst_x, geom.x, int16_t(geom.x + geom.width)); + auto src_y = math_util::cap(translated->dst_y, geom.y, int16_t(geom.y + geom.height)); + auto w = math_util::cap(m_rect.width, uint16_t(0), uint16_t(geom.width - (src_x - geom.x))); + auto h = math_util::cap(m_rect.height, uint16_t(0), uint16_t(geom.height - (src_y - geom.y))); + m_log.trace( + "background_manager: Copying from root pixmap (0x%x:%d) %dx%d+%d+%d", root_pixmap, depth, w, h, src_x, src_y); + m_connection.copy_area_checked(root_pixmap, m_pixmap, m_gcontext, src_x, src_y, 0, 0, w, h); +} + +void bg_slice::ensure_resources(int depth, xcb_visualtype_t* visual) { + if (m_depth != depth) { + m_depth = depth; + + free_resources(); + + if (depth != 0) { + allocate_resources(visual); + } + } +} + +void bg_slice::allocate_resources(xcb_visualtype_t* visual) { + m_log.trace("background_manager: Allocating pixmap"); + m_pixmap = m_connection.generate_id(); + m_connection.create_pixmap(m_depth, m_pixmap, m_window, m_rect.width, m_rect.height); + + m_log.trace("background_manager: Allocating graphics context"); + auto black_pixel = m_connection.screen()->black_pixel; + unsigned int mask = XCB_GC_FOREGROUND | XCB_GC_BACKGROUND | XCB_GC_GRAPHICS_EXPOSURES; + unsigned int value_list[3] = {black_pixel, black_pixel, 0}; + m_gcontext = m_connection.generate_id(); + m_connection.create_gc(m_gcontext, m_pixmap, mask, value_list); + + m_log.trace("background_manager: Allocating cairo surface"); + m_surface = make_unique(m_connection, m_pixmap, visual, m_rect.width, m_rect.height); } void bg_slice::free_resources() { m_surface.reset(); - if(m_pixmap != XCB_NONE) { + if (m_pixmap != XCB_NONE) { m_connection.free_pixmap(m_pixmap); m_pixmap = XCB_NONE; } - if(m_gcontext != XCB_NONE) { + if (m_gcontext != XCB_NONE) { m_connection.free_gc(m_gcontext); m_gcontext = XCB_NONE; } diff --git a/src/x11/tray_manager.cpp b/src/x11/tray_manager.cpp index 442a0601..f9efcb46 100644 --- a/src/x11/tray_manager.cpp +++ b/src/x11/tray_manager.cpp @@ -405,7 +405,7 @@ void tray_manager::reconfigure_bg(bool realloc) { cairo::surface* surface = m_bg_slice->get_surface(); if (!surface) { - return m_log.err("tray: no root surface"); + return; } m_context->clear();