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

@@ -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))

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<cairo::xcb_surface> 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<SIGN_PRIORITY_SCREEN, signals:
   void activate();
   void deactivate();
 
+  void attach();
+  void detach();
+  /**
+   * True if we are currently attached as a listener for desktop background changes
+   */
+  bool m_attached{false};
+
   // references to standard components
   connection& m_connection;
   signal_emitter& m_sig;
   const logger& m_log;
 
-  // list of slices that need to be filled with the desktop background
+  /**
+   * List of slices that need to be filled with the desktop background
+   */
   std::vector<std::weak_ptr<bg_slice>> 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

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).

src/x11/background_manager.cpp

@@ -1,13 +1,16 @@
-#include "cairo/surface.hpp"
+#include "x11/background_manager.hpp"
+
+#include <cassert>
+
 #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<background_manager>(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<bg_slice> background_manager::observe(xcb_rectangle_t rect, xcb_window_t window) {
   // allocate a slice
-  activate();
-  auto slice = std::shared_ptr<bg_slice>(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<bg_slice>(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<cairo::xcb_surface>(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<cairo::xcb_surface>(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;
   }

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();