fix(renderer): Handling of missing root pixmap (#1608)

Polybar had issues when there is no background set or set by a tool like imagemagick which doesn't add the root pixmap to the root window properties.

There's not much we can do about it, but at least polybar doesn't crash anymore.

Fixes #1582 
Fixes #1585 

* fix(tray_manager): only enable transparency if neccessary

Previously, we always enabled transparency

* fix(background_manager): avoid needless fetching

* fix(renderer): move logging message to correct place

* fix(background_manager): handle dummy pixmap (_XSETROOT_ID) right

* fix(background_manager): more initialization + don't free on error

Freeing on error is incorrect, since we could still be called again later in
which case we still need the resources.

* fix(background_manager): add more infos to trace logs

* fix(background): correct typo (XROOTMAP -> XROOTPMAP)

* fix(background_manager): do not report "no background" as error

* style(background_manager): use braces for if

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

* fix(background_manager): better error message for dummy pixmap

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

* style(background): some more style fixes

* fix(connection): initialize pixmap in all cases in root_pixmap()

* style(connection): improve readability using early return

include/x11/atoms.hpp

@@ -40,7 +40,7 @@ extern xcb_atom_t _NET_SYSTEM_TRAY_COLORS;
 extern xcb_atom_t WM_TAKE_FOCUS;
 extern xcb_atom_t Backlight;
 extern xcb_atom_t BACKLIGHT;
-extern xcb_atom_t _XROOTMAP_ID;
+extern xcb_atom_t _XROOTPMAP_ID;
 extern xcb_atom_t _XSETROOT_ID;
 extern xcb_atom_t ESETROOT_PMAP_ID;
 extern xcb_atom_t _COMPTON_SHADOW;

src/components/renderer.cpp

@@ -163,12 +163,11 @@ renderer::renderer(
       m_log.info("Loaded font \"%s\" (name=%s, offset=%i, file=%s)", pattern, font->name(), offset, font->file());
       *m_context << move(font);
     }
-
-    m_log.trace("Activate root background manager");
   }
 
   m_pseudo_transparency = m_conf.get<bool>("settings", "pseudo-transparency", m_pseudo_transparency);
   if (m_pseudo_transparency) {
+    m_log.trace("Activate root background manager");
     m_background = background.observe(m_bar.outer_area(false), m_window);
   }
 

src/x11/atoms.cpp

@@ -33,7 +33,7 @@ xcb_atom_t _NET_SYSTEM_TRAY_COLORS;
 xcb_atom_t WM_TAKE_FOCUS;
 xcb_atom_t Backlight;
 xcb_atom_t BACKLIGHT;
-xcb_atom_t _XROOTMAP_ID;
+xcb_atom_t _XROOTPMAP_ID;
 xcb_atom_t _XSETROOT_ID;
 xcb_atom_t ESETROOT_PMAP_ID;
 xcb_atom_t _COMPTON_SHADOW;
@@ -72,7 +72,7 @@ cached_atom ATOMS[36] = {
   {"WM_TAKE_FOCUS", sizeof("WM_TAKE_FOCUS") - 1, &WM_TAKE_FOCUS},
   {"Backlight", sizeof("Backlight") - 1, &Backlight},
   {"BACKLIGHT", sizeof("BACKLIGHT") - 1, &BACKLIGHT},
-  {"_XROOTMAP_ID", sizeof("_XROOTMAP_ID") - 1, &_XROOTMAP_ID},
+  {"_XROOTPMAP_ID", sizeof("_XROOTPMAP_ID") - 1, &_XROOTPMAP_ID},
   {"_XSETROOT_ID", sizeof("_XSETROOT_ID") - 1, &_XSETROOT_ID},
   {"ESETROOT_PMAP_ID", sizeof("ESETROOT_PMAP_ID") - 1, &ESETROOT_PMAP_ID},
   {"_COMPTON_SHADOW", sizeof("_COMPTON_SHADOW") - 1, &_COMPTON_SHADOW},

src/x11/background_manager.cpp

@@ -81,9 +81,14 @@ void background_manager::fetch_root_pixmap() {
 
   try {
     if (!m_connection.root_pixmap(&pixmap, &pixmap_depth, &pixmap_geom)) {
-      free_resources();
+      return m_log.warn("background_manager: Failed to get root pixmap, default to black (is there a wallpaper?)");
-      return m_log.err("background_manager: Failed to get root pixmap for background (realloc=%i)", realloc);
     };
+    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();
@@ -98,7 +103,7 @@ void background_manager::fetch_root_pixmap() {
       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_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++;
@@ -119,15 +124,22 @@ void background_manager::fetch_root_pixmap() {
 
 void background_manager::handle(const evt::property_notify& evt) {
   // if there are no slices to observe, don't do anything
-  if(m_slices.empty()) return;
+  if(m_slices.empty()) {
+    return;
+  }
 
-  if (evt->atom == _XROOTMAP_ID || evt->atom == _XSETROOT_ID || evt->atom == ESETROOT_PMAP_ID) {
+  if (evt->atom == _XROOTPMAP_ID || evt->atom == _XSETROOT_ID || evt->atom == ESETROOT_PMAP_ID) {
     fetch_root_pixmap();
     m_sig.emit(signals::ui::update_background());
   }
 }
 
 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());
   return false;
@@ -159,8 +171,9 @@ void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
 
   if(m_gcontext == XCB_NONE) {
     log.trace("background_manager: Allocating graphics context");
-    unsigned int mask = XCB_GC_GRAPHICS_EXPOSURES;
+    auto black_pixel = m_connection.screen()->black_pixel;
-    unsigned int value_list[1] = {0};
+    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);
   }
@@ -169,6 +182,10 @@ void bg_slice::allocate_resources(const logger& log, xcb_visualtype_t* visual) {
     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);
   }
+
+  // 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);
 }
 
 void bg_slice::free_resources() {

src/x11/connection.cpp

@@ -182,29 +182,40 @@ xcb_visualtype_t* connection::visual_type_for_id(xcb_screen_t* screen, xcb_visua
  * Query root window pixmap
  */
 bool connection::root_pixmap(xcb_pixmap_t* pixmap, int* depth, xcb_rectangle_t* rect) {
-  const xcb_atom_t pixmap_properties[3]{ESETROOT_PMAP_ID, _XROOTMAP_ID, _XSETROOT_ID};
+  *pixmap = XCB_NONE; // default value if getting the root pixmap fails
+
+  /*
+   * We try multiple properties for the root pixmap here because I am not 100% sure
+   * if all programs set them the same way. We might be able to just use _XSETROOT_ID
+   * but keeping the other as fallback should not hurt (if it does, feel free to remove).
+   *
+   * see https://metacpan.org/pod/X11::Protocol::XSetRoot#ROOT-WINDOW-PROPERTIES for description of the properties
+   * the properties here are ordered by reliability:
+   *    _XSETROOT_ID: this is usually a dummy 1x1 pixmap only for resource managment, use only as last resort
+   *    ESETROOT_PMAP_ID: according to the link above, this should usually by equal to _XROOTPMAP_ID
+   *    _XROOTPMAP_ID: this appears to be the "correct" property to use? if available, use this
+   */
+  const xcb_atom_t pixmap_properties[3]{_XROOTPMAP_ID, ESETROOT_PMAP_ID, _XSETROOT_ID};
   for (auto&& property : pixmap_properties) {
     try {
       auto prop = get_property(false, screen()->root, property, XCB_ATOM_PIXMAP, 0L, 1L);
       if (prop->format == 32 && prop->value_len == 1) {
         *pixmap = *prop.value<xcb_pixmap_t>().begin();
       }
-    } catch (const exception& err) {
+
-      continue;
+      if (*pixmap) {
-    }
+        auto geom = get_geometry(*pixmap);
-  }
+        *depth = geom->depth;
-  if (*pixmap) {
+        rect->width = geom->width;
-    try {
+        rect->height = geom->height;
-      auto geom = get_geometry(*pixmap);
+        rect->x = geom->x;
-      *depth = geom->depth;
+        rect->y = geom->y;
-      rect->width = geom->width;
+        return true;
-      rect->height = geom->height;
+      }
-      rect->x = geom->x;
-      rect->y = geom->y;
-      return true;
     } catch (const exception& err) {
       *pixmap = XCB_NONE;
       *rect = xcb_rectangle_t{0, 0, 0U, 0U};
+      continue;
     }
   }
   return false;

src/x11/tray_manager.cpp

@@ -133,7 +133,8 @@ void tray_manager::setup(const bar_settings& bar_opts) {
     m_opts.background = bar_opts.background;
   }
 
-  if (color_util::alpha_channel(m_opts.background) != 1) {
+  if (color_util::alpha_channel<unsigned char>(m_opts.background) != 255) {
+    m_log.trace("tray: enable transparency");
     m_opts.transparent = true;
   }
 
@@ -996,7 +997,7 @@ void tray_manager::handle(const evt::selection_clear& evt) {
 void tray_manager::handle(const evt::property_notify& evt) {
   if (!m_activated) {
     return;
-  } else if (evt->atom == _XROOTMAP_ID) {
+  } else if (evt->atom == _XROOTPMAP_ID) {
     redraw_window(true);
   } else if (evt->atom == _XSETROOT_ID) {
     redraw_window(true);