Cleanup selection of visual for bar window

include/components/renderer.hpp

@@ -102,7 +102,7 @@ class renderer : public renderer_interface,
   const bar_settings& m_bar;
   std::shared_ptr<bg_slice> m_background;
 
-  int m_depth{32};
+  int m_depth{-1};
   xcb_window_t m_window;
   xcb_colormap_t m_colormap;
   xcb_visualtype_t* m_visual;

include/x11/connection.hpp

@@ -123,8 +123,8 @@ class connection : public detail::connection_base<connection&, XPP_EXTENSION_LIS
   void send_client_message(const xcb_client_message_event_t& message, xcb_window_t target,
       unsigned int event_mask = 0xFFFFFF, bool propagate = false) const;
 
-  xcb_visualtype_t* visual_type(xcb_screen_t* screen, int match_depth = 32);
+  xcb_visualtype_t* visual_type(xcb_visual_class_t class_, int match_depth);
-  xcb_visualtype_t* visual_type_for_id(xcb_screen_t* screen, xcb_visualid_t visual_id);
+  xcb_visualtype_t* visual_type_for_id(xcb_visualid_t visual_id);
 
   bool root_pixmap(xcb_pixmap_t* pixmap, int* depth, xcb_rectangle_t* rect);
 

src/components/renderer.cpp

@@ -47,30 +47,23 @@ renderer::renderer(connection& conn, signal_emitter& sig, const config& conf, co
     , m_bar(forward<const bar_settings&>(bar))
     , m_rect(m_bar.inner_area()) {
   m_sig.attach(this);
-  m_log.trace("renderer: Get TrueColor visual");
-  {
-    if ((m_visual = m_connection.visual_type(m_connection.screen(), 32)) == nullptr) {
-      m_log.err("No 32-bit TrueColor visual found...");
 
-      if ((m_visual = m_connection.visual_type(m_connection.screen(), 24)) == nullptr) {
+  m_log.trace("renderer: Get TrueColor visual");
-        m_log.err("No 24-bit TrueColor visual found...");
+  if ((m_visual = m_connection.visual_type(XCB_VISUAL_CLASS_TRUE_COLOR, 32)) != nullptr) {
-      } else {
+    m_depth = 32;
+  } else if ((m_visual = m_connection.visual_type(XCB_VISUAL_CLASS_TRUE_COLOR, 24)) != nullptr) {
     m_depth = 24;
+  } else {
+    throw application_error("Could not find a 24 or 32-bit TrueColor visual");
   }
-    }
+
-    if (m_visual == nullptr) {
+  m_log.info("renderer: Using %d-bit visual", m_depth);
-      throw application_error("No matching TrueColor");
-    }
-  }
 
   m_log.trace("renderer: Allocate colormap");
-  {
   m_colormap = m_connection.generate_id();
   m_connection.create_colormap(XCB_COLORMAP_ALLOC_NONE, m_colormap, m_connection.screen()->root, m_visual->visual_id);
-  }
 
   m_log.trace("renderer: Allocate output window");
-  {
   // clang-format off
   m_window = winspec(m_connection)
     << cw_size(m_bar.size)
@@ -88,7 +81,6 @@ renderer::renderer(connection& conn, signal_emitter& sig, const config& conf, co
     << cw_params_override_redirect(m_bar.override_redirect)
     << cw_flush(true);
   // clang-format on
-  }
 
   m_log.trace("renderer: Allocate window pixmaps");
   {

src/x11/background_manager.cpp

@@ -1,13 +1,14 @@
-#include "cairo/surface.hpp"
+#include "x11/background_manager.hpp"
+
 #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 "events/signal.hpp"
-#include "x11/connection.hpp"
-#include "x11/background_manager.hpp"
 #include "utils/factory.hpp"
 #include "utils/math.hpp"
+#include "x11/atoms.hpp"
+#include "x11/connection.hpp"
 
 POLYBAR_NS
 
@@ -15,11 +16,8 @@ background_manager& background_manager::make() {
   return *factory_util::singleton<background_manager>(connection::make(), signal_emitter::make(), logger::make());
 }
 
-background_manager::background_manager(
+background_manager::background_manager(connection& conn, signal_emitter& sig, const logger& log)
-    connection& conn, signal_emitter& sig, const logger& log)
+    : m_connection(conn), m_sig(sig), m_log(log) {
-  : m_connection(conn)
-  , m_sig(sig)
-  , m_log(log) {
   m_sig.attach(this);
 }
 
@@ -34,7 +32,7 @@ std::shared_ptr<bg_slice> background_manager::observe(xcb_rectangle_t rect, xcb_
   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) {
+  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);
@@ -52,18 +50,17 @@ std::shared_ptr<bg_slice> background_manager::observe(xcb_rectangle_t rect, xcb_
 }
 
 void background_manager::deactivate() {
-  if(m_attached) {
+  if (m_attached) {
     m_connection.detach_sink(this, SINK_PRIORITY_SCREEN);
     m_attached = false;
   }
   free_resources();
 }
 
-
 void background_manager::activate() {
-  if(!m_visual) {
+  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_visual = m_connection.visual_type_for_id(m_connection.screen()->root_visual);
     m_log.trace("background_manager: Got root visual with depth %d", m_connection.screen()->root_depth);
   }
 }
@@ -83,14 +80,15 @@ void background_manager::fetch_root_pixmap() {
     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,
+    m_log.trace("background_manager: root pixmap (%d:%d) %dx%d+%d+%d", pixmap, pixmap_depth, pixmap_geom.width,
-                pixmap_geom.width, pixmap_geom.height, pixmap_geom.x, pixmap_geom.y);
+        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");
+      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(); ) {
+    for (auto it = m_slices.begin(); it != m_slices.end();) {
       auto slice = it->lock();
       if (!slice) {
         it = m_slices.erase(it);
@@ -98,12 +96,15 @@ void background_manager::fetch_root_pixmap() {
       }
 
       // 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 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)));
+      auto h =
-      m_log.trace("background_manager: Copying from root pixmap (%d:%d) %dx%d+%d+%d", pixmap, pixmap_depth, w, h, src_x, src_y);
+          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++;
@@ -115,16 +116,15 @@ void background_manager::fetch_root_pixmap() {
       deactivate();
     }
 
-  } catch(const exception& err) {
+  } catch (const exception& err) {
     m_log.err("background_manager: Failed to copy slice of root pixmap (%s)", err.what());
     throw;
   }
-
 }
 
 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 (m_slices.empty()) {
     return;
   }
 
@@ -136,7 +136,7 @@ void background_manager::handle(const evt::property_notify& evt) {
 
 bool background_manager::on(const signals::ui::update_geometry&) {
   // if there are no slices to observe, don't do anything
-  if(m_slices.empty()) {
+  if (m_slices.empty()) {
     return false;
   }
 
@@ -145,14 +145,12 @@ bool background_manager::on(const signals::ui::update_geometry&) {
   return false;
 }
 
-
+bg_slice::bg_slice(
-bg_slice::bg_slice(connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window, xcb_visualtype_t* visual)
+    connection& conn, const logger& log, xcb_rectangle_t rect, xcb_window_t window, xcb_visualtype_t* visual)
-  : m_connection(conn)
+    : m_connection(conn), m_rect(rect), m_window(window) {
-  , m_rect(rect)
-  , m_window(window) {
   try {
     allocate_resources(log, visual);
-  } catch(...) {
+  } catch (...) {
     free_resources();
     throw;
   }
@@ -163,13 +161,13 @@ bg_slice::~bg_slice() {
 }
 
 void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
-  if(m_pixmap == XCB_NONE) {
+  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) {
+  if (m_gcontext == XCB_NONE) {
     log.trace("background_manager: Allocating graphics context");
     auto black_pixel = m_connection.screen()->black_pixel;
     uint32_t mask = 0;
@@ -183,7 +181,7 @@ void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
     m_connection.create_gc(m_gcontext, m_pixmap, mask, value_list.data());
   }
 
-  if(!m_surface) {
+  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);
   }
@@ -196,12 +194,12 @@ void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
 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/connection.cpp

@@ -81,6 +81,8 @@ string connection::id(xcb_window_t w) const {
 
 /**
  * Get pointer to the default xcb screen
+ *
+ * TODO remove realloc param. Create explicit realloc function
  */
 xcb_screen_t* connection::screen(bool realloc) {
   if (m_screen == nullptr || realloc) {
@@ -139,35 +141,12 @@ void connection::send_client_message(
  * Try to get a visual type for the given screen that
  * matches the given depth
  */
-xcb_visualtype_t* connection::visual_type(xcb_screen_t* screen, int match_depth) {
+xcb_visualtype_t* connection::visual_type(xcb_visual_class_t class_, int match_depth) {
-  xcb_depth_iterator_t depth_iter = xcb_screen_allowed_depths_iterator(screen);
+  return xcb_aux_find_visual_by_attrs(screen(), class_, match_depth);
-  if (depth_iter.data) {
-    for (; depth_iter.rem; xcb_depth_next(&depth_iter)) {
-      if (match_depth == 0 || match_depth == depth_iter.data->depth) {
-        for (auto it = xcb_depth_visuals_iterator(depth_iter.data); it.rem; xcb_visualtype_next(&it)) {
-          return it.data;
-        }
-      }
-    }
-    if (match_depth > 0) {
-      return visual_type(screen, 0);
-    }
-  }
-  return nullptr;
 }
 
-xcb_visualtype_t* connection::visual_type_for_id(xcb_screen_t* screen, xcb_visualid_t visual_id) {
+xcb_visualtype_t* connection::visual_type_for_id(xcb_visualid_t visual_id) {
-  xcb_depth_iterator_t depth_iter = xcb_screen_allowed_depths_iterator(screen);
+  return xcb_aux_find_visual_by_id(screen(), visual_id);
-  if (depth_iter.data) {
-    for (; depth_iter.rem; xcb_depth_next(&depth_iter)) {
-      for (auto it = xcb_depth_visuals_iterator(depth_iter.data); it.rem; xcb_visualtype_next(&it)) {
-        if (it.data->visual_id == visual_id) {
-          return it.data;
-        }
-      }
-    }
-  }
-  return nullptr;
 }
 
 /**

src/x11/tray_manager.cpp

@@ -548,8 +548,7 @@ void tray_manager::create_bg() {
   }
 
   if (!m_surface) {
-    xcb_visualtype_t* visual =
+    xcb_visualtype_t* visual = m_connection.visual_type_for_id(m_connection.screen()->root_visual);
-        m_connection.visual_type_for_id(m_connection.screen(), m_connection.screen()->root_visual);
     if (!visual) {
       return m_log.err("Failed to get root visual for tray background");
     }