fix(tray): correctly handle transparency when using offset (#1571)

This patch adds support for observing multiple slices of the desktop background.
This is used for the tray so that it doesn't have to rely on the bar's rect to
get the desktop background. In particular, it now handles the case where the
tray is not contained fully within the bar's outer rect (for example, when using tray-offset-{x,y})

Co-Authored-By: bennofs <benno.fuenfstueck@gmail.com>

include/components/renderer.hpp

@@ -2,6 +2,7 @@
 
 #include <bitset>
 #include <cairo/cairo.h>
+#include <memory>
 
 #include "cairo/fwd.hpp"
 #include "common.hpp"
@@ -18,6 +19,7 @@ class connection;
 class config;
 class logger;
 class background_manager;
+class bg_slice;
 // }}}
 
 using std::map;
@@ -96,7 +98,7 @@ class renderer
   const config& m_conf;
   const logger& m_log;
   const bar_settings& m_bar;
-  background_manager& m_background;
+  std::shared_ptr<bg_slice> m_background;
 
   int m_depth{32};
   xcb_window_t m_window;

include/x11/background_manager.hpp

@@ -1,5 +1,8 @@
 #pragma once
 
+#include <memory>
+#include <vector>
+
 #include "common.hpp"
 #include "events/signal_fwd.hpp"
 #include "events/signal_receiver.hpp"
@@ -16,6 +19,44 @@ namespace cairo {
   class xcb_surface;
 }
 
+class bg_slice {
+ public:
+  ~bg_slice();
+  // copying bg_slices is not allowed
+  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();
+  }
+
+ private:
+  bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window, xcb_visualtype_t* visual);
+
+  // standard components
+  connection& m_connection;
+
+  // area covered by this slice
+  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
+  xcb_pixmap_t m_pixmap{XCB_NONE};
+  unique_ptr<cairo::xcb_surface> m_surface;
+  xcb_gcontext_t m_gcontext{XCB_NONE};
+
+  void allocate_resources(const logger& log, xcb_visualtype_t* visual);
+  void free_resources();
+
+  friend class background_manager;
+};
+
 /**
  * \brief Class to keep track of the desktop background used to support pseudo-transparency
  *
@@ -42,48 +83,35 @@ class background_manager : public signal_receiver<SIGN_PRIORITY_SCREEN, signals:
    * Starts observing a rectangular slice of the desktop background.
    *
    * After calling this function, you can obtain the current slice of the desktop background
-   * with background_manager::get_surface. Whenever the background slice changes (for example,
+   * by calling get_surface on the returned bg_slice object.
-   * due to bar position changes or because the user changed the desktop background) the class
+   * Whenever the background slice changes (for example, due to bar position changes or because
-   * emits a signals::ui::update_background event.
+   * the user changed the desktop background) the class emits a signals::ui::update_background event.
+   *
+   * You should only call this function once and then re-use the returned bg_slice because the bg_slice
+   * caches the background. If you don't need the background anymore, destroy the shared_ptr to free up
+   * resources.
    *
-   * \param window This should be set to the bar window.
    * \param rect Slice of the background to observe (coordinates relative to window).
-   *             Typically set to the outer area of the bar.
+   * \param window Coordinates are interpreted relative to this window
    */
-  void activate(xcb_window_t window, xcb_rectangle_t rect);
+  std::shared_ptr<bg_slice> observe(xcb_rectangle_t rect, xcb_window_t window);
-
-
-  /**
-   * Stops observing the desktop background and frees all resources.
-   */
-  void deactivate();
-
-  /**
-   * Retrieve the current desktop background slice.
-   *
-   * This function returns a slice of the desktop background that has the size of the rectangle
-   * given to background_manager::activate. As the slice is cached by the manager, this function
-   * is fast.
-   */
-  cairo::surface* get_surface() const;
 
   void handle(const evt::property_notify& evt);
   bool on(const signals::ui::update_geometry&);
  private:
+  void activate();
+  void deactivate();
+
   // references to standard components
   connection& m_connection;
   signal_emitter& m_sig;
   const logger& m_log;
 
-  // these are set by activate
+  // list of slices that need to be filled with the desktop background
-  xcb_window_t m_window;
+  std::vector<std::weak_ptr<bg_slice>> m_slices;
-  xcb_rectangle_t m_rect{0, 0, 0U, 0U};
 
   // required values for fetching the root window's background
   xcb_visualtype_t* m_visual{nullptr};
-  xcb_gcontext_t m_gcontext{XCB_NONE};
-  xcb_pixmap_t m_pixmap{XCB_NONE};
-  unique_ptr<cairo::xcb_surface> m_surface;
 
   // true if we are currently attached as a listener for desktop background changes
   bool m_attached{false};

include/x11/tray_manager.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <chrono>
+#include <memory>
 
 #include "cairo/context.hpp"
 #include "cairo/surface.hpp"
@@ -35,6 +36,7 @@ using namespace std::chrono_literals;
 class connection;
 struct xembed_data;
 class background_manager;
+class bg_slice;
 
 struct tray_settings {
   tray_settings() = default;
@@ -106,8 +108,8 @@ class tray_manager
 
   int calculate_x(unsigned width, bool abspos = true) const;
   int calculate_y(bool abspos = true) const;
-  unsigned int calculate_w() const;
+  unsigned short int calculate_w() const;
-  unsigned int calculate_h() const;
+  unsigned short int calculate_h() const;
 
   int calculate_client_x(const xcb_window_t& win);
   int calculate_client_y();
@@ -138,7 +140,8 @@ class tray_manager
   connection& m_connection;
   signal_emitter& m_sig;
   const logger& m_log;
-  background_manager& m_background;
+  background_manager& m_background_manager;
+  std::shared_ptr<bg_slice> m_bg_slice;
   vector<shared_ptr<tray_client>> m_clients;
 
   tray_settings m_opts{};

src/components/renderer.cpp

@@ -41,7 +41,6 @@ renderer::renderer(
     , m_conf(conf)
     , m_log(logger)
     , m_bar(forward<const bar_settings&>(bar))
-    , m_background(background)
     , m_rect(m_bar.inner_area()) {
 
   m_sig.attach(this);
@@ -166,10 +165,12 @@ renderer::renderer(
     }
 
     m_log.trace("Activate root background manager");
-    m_background.activate(m_window, m_bar.outer_area(false));
   }
 
   m_pseudo_transparency = m_conf.get<bool>("settings", "pseudo-transparency", m_pseudo_transparency);
+  if (m_pseudo_transparency) {
+    m_background = background.observe(m_bar.outer_area(false), m_window);
+  }
 
   m_comp_bg = m_conf.get<cairo_operator_t>("settings", "compositing-background", m_comp_bg);
   m_comp_fg = m_conf.get<cairo_operator_t>("settings", "compositing-foreground", m_comp_fg);
@@ -308,7 +309,7 @@ void renderer::end() {
     cairo_pattern_t* barcontents{};
     m_context->pop(&barcontents); // corresponding push is in renderer::begin
 
-    auto root_bg = m_background.get_surface();
+    auto root_bg = m_background->get_surface();
     if (root_bg != nullptr) {
       m_log.trace_x("renderer: root background");
       *m_context << *root_bg;

src/x11/background_manager.cpp

@@ -28,29 +28,27 @@ background_manager::~background_manager() {
   free_resources();
 }
 
-cairo::surface* background_manager::get_surface() const {
+std::shared_ptr<bg_slice> background_manager::observe(xcb_rectangle_t rect, xcb_window_t window) {
-  return m_surface.get();
+  // allocate a slice
-}
+  activate();
-
+  auto slice = std::shared_ptr<bg_slice>(new bg_slice(m_connection, m_log, rect, window, m_visual));
-void background_manager::activate(xcb_window_t window, xcb_rectangle_t rect) {
-  // ensure that we start from a clean state
-  //
-  // the size of the pixmap may need to be changed, etc.
-  // so the easiest way is to just re-allocate everything.
-  // it may be possible to be more clever here, but activate is
-  // not supposed to be called often so this shouldn't be a problem.
-  free_resources();
 
   // 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;
   }
 
-  m_window = window;
+  // if the slice is empty, don't add to slices
-  m_rect = rect;
+  if (slice->m_rect.width == 0 || slice->m_rect.height == 0) {
+    return slice;
+  }
+
+  m_slices.push_back(slice);
   fetch_root_pixmap();
+  return slice;
 }
 
 void background_manager::deactivate() {
@@ -59,61 +57,22 @@ void background_manager::deactivate() {
     m_attached = false;
   }
   free_resources();
-  m_rect = xcb_rectangle_t{0, 0, 0, 0};
 }
 
 
-void background_manager::allocate_resources() {
+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);
   }
-
-  if(m_pixmap == XCB_NONE) {
-    m_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);
-  }
-
-  if(m_gcontext == XCB_NONE) {
-    m_log.trace("background_manager: Allocating graphics context");
-    unsigned int mask = XCB_GC_GRAPHICS_EXPOSURES;
-    unsigned int value_list[1] = {0};
-    m_gcontext = m_connection.generate_id();
-    m_connection.create_gc(m_gcontext, m_pixmap, mask, value_list);
-  }
-
-  if(!m_surface) {
-    m_log.trace("background_manager: Allocating cairo surface");
-    m_surface = make_unique<cairo::xcb_surface>(m_connection, m_pixmap, m_visual, m_rect.width, m_rect.height);
-  }
-
-  if(m_attached) {
-    m_connection.detach_sink(this, SINK_PRIORITY_SCREEN);
-    m_attached = false;
-  }
-
 }
 
 void background_manager::free_resources() {
-  m_surface.release();
   m_visual = nullptr;
-
-  if(m_pixmap != XCB_NONE) {
-    m_connection.free_pixmap(m_pixmap);
-    m_pixmap = XCB_NONE;
-  }
-
-  if(m_gcontext != XCB_NONE) {
-    m_connection.free_gc(m_gcontext);
-    m_gcontext = XCB_NONE;
-  }
 }
 
 void background_manager::fetch_root_pixmap() {
-  allocate_resources();
-
   m_log.trace("background_manager: Fetching pixmap");
 
   int pixmap_depth;
@@ -121,30 +80,46 @@ void background_manager::fetch_root_pixmap() {
   xcb_rectangle_t pixmap_geom;
 
   try {
-    auto translated = m_connection.translate_coordinates(m_window, m_connection.screen()->root, m_rect.x, m_rect.y);
     if (!m_connection.root_pixmap(&pixmap, &pixmap_depth, &pixmap_geom)) {
       free_resources();
       return m_log.err("background_manager: Failed to get root pixmap for background (realloc=%i)", realloc);
     };
 
-    auto src_x = math_util::cap(translated->dst_x, pixmap_geom.x, int16_t(pixmap_geom.x + pixmap_geom.width));
+    for (auto it = m_slices.begin(); it != m_slices.end(); ) {
-    auto src_y = math_util::cap(translated->dst_y, pixmap_geom.y, int16_t(pixmap_geom.y + pixmap_geom.height));
+      auto slice = it->lock();
-    auto h = math_util::min(m_rect.height, pixmap_geom.height);
+      if (!slice) {
-    auto w = math_util::min(m_rect.width, pixmap_geom.width);
+        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) %dx%d+%dx%d", pixmap, 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();
+    }
 
-    m_log.trace("background_manager: Copying from root pixmap (%d) %dx%d+%dx%d", pixmap, w, h, src_x, src_y);
-    m_connection.copy_area_checked(pixmap, m_pixmap, m_gcontext, src_x, src_y, 0, 0, w, h);
   } catch(const exception& err) {
     m_log.err("background_manager: Failed to copy slice of root pixmap (%s)", err.what());
-    free_resources();
     throw;
   }
 
 }
 
 void background_manager::handle(const evt::property_notify& evt) {
-  // if region that we should observe is empty, don't do anything
+  // if there are no slices to observe, don't do anything
-  if(m_rect.width == 0 || m_rect.height == 0) return;
+  if(m_slices.empty()) return;
 
   if (evt->atom == _XROOTMAP_ID || evt->atom == _XSETROOT_ID || evt->atom == ESETROOT_PMAP_ID) {
     fetch_root_pixmap();
@@ -158,4 +133,56 @@ bool background_manager::on(const signals::ui::update_geometry&) {
   return false;
 }
 
+
+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;
+  }
+}
+
+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);
+  }
+
+  if(m_gcontext == XCB_NONE) {
+    log.trace("background_manager: Allocating graphics context");
+    unsigned int mask = XCB_GC_GRAPHICS_EXPOSURES;
+    unsigned int value_list[1] = {0};
+    m_gcontext = m_connection.generate_id();
+    m_connection.create_gc(m_gcontext, m_pixmap, mask, value_list);
+  }
+
+  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::free_resources() {
+  m_surface.release();
+
+  if(m_pixmap != XCB_NONE) {
+    m_connection.free_pixmap(m_pixmap);
+    m_pixmap = XCB_NONE;
+  }
+
+  if(m_gcontext != XCB_NONE) {
+    m_connection.free_gc(m_gcontext);
+    m_gcontext = XCB_NONE;
+  }
+}
+
 POLYBAR_NS_END

src/x11/tray_manager.cpp

@@ -44,7 +44,7 @@ tray_manager::make_type tray_manager::make() {
 }
 
 tray_manager::tray_manager(connection& conn, signal_emitter& emitter, const logger& logger, background_manager& back)
-  : m_connection(conn), m_sig(emitter), m_log(logger), m_background(back) {
+  : m_connection(conn), m_sig(emitter), m_log(logger), m_background_manager(back) {
   m_connection.attach_sink(this, SINK_PRIORITY_TRAY);
 }
 
@@ -338,6 +338,11 @@ void tray_manager::reconfigure_window() {
   auto width = calculate_w();
   auto x = calculate_x(width);
 
+  if (m_opts.transparent) {
+    xcb_rectangle_t rect{0, 0, calculate_w(), calculate_h()};
+    m_bg_slice = m_background_manager.observe(rect, m_tray);
+  }
+
   if (width > 0) {
     m_log.trace("tray: New window values, width=%d, x=%d", width, x);
 
@@ -388,22 +393,18 @@ void tray_manager::reconfigure_bg(bool realloc) {
   m_log.trace("tray: Reconfigure bg (realloc=%i)", realloc);
 
 
-  auto w = calculate_w();
-  auto x = calculate_x(w, false);
-  auto y = calculate_y(false);
-
   if(!m_context) {
     return m_log.err("tray: no context for drawing the background");
   }
 
-  cairo::surface* surface = m_background.get_surface();
+  cairo::surface* surface = m_bg_slice->get_surface();
   if(!surface) {
     return m_log.err("tray: no root surface");
   }
 
   m_context->clear();
   *m_context << CAIRO_OPERATOR_SOURCE << *m_surface;
-  cairo_set_source_surface(*m_context, *surface, -x, -y);
+  cairo_set_source_surface(*m_context, *surface, 0, 0);
   m_context->paint();
   *m_context << CAIRO_OPERATOR_OVER << m_opts.background;
   m_context->paint();
@@ -491,6 +492,12 @@ void tray_manager::create_window() {
   m_tray = win << cw_flush(true);
   m_log.info("Tray window: %s", m_connection.id(m_tray));
 
+  // activate the background manager if we have transparency
+  if (m_opts.transparent) {
+    xcb_rectangle_t rect{0, 0, calculate_w(), calculate_h()};
+    m_bg_slice = m_background_manager.observe(rect, m_tray);
+  }
+
   const unsigned int shadow{0};
   m_connection.change_property(XCB_PROP_MODE_REPLACE, m_tray, _COMPTON_SHADOW, XCB_ATOM_CARDINAL, 32, 1, &shadow);
 }
@@ -790,7 +797,7 @@ int tray_manager::calculate_y(bool abspos) const {
 /**
  * Calculate width of tray window
  */
-unsigned int tray_manager::calculate_w() const {
+unsigned short int tray_manager::calculate_w() const {
   unsigned int width = m_opts.spacing;
   unsigned int count{0};
   for (auto&& client : m_clients) {
@@ -805,7 +812,7 @@ unsigned int tray_manager::calculate_w() const {
 /**
  * Calculate height of tray window
  */
-unsigned int tray_manager::calculate_h() const {
+unsigned short int tray_manager::calculate_h() const {
   return m_opts.height_fill;
 }