Use struct conv for blur kernel as well

Instead of storing them as an array of xfixed.

Might cause some performance overhead for the new backend, because
it is allocating a buffer to do the conversion every frame. Will fix
later.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>

src/backend/gl/gl_common.c

@@ -15,6 +15,7 @@
 #include "region.h"
 #include "string_utils.h"
 #include "utils.h"
+#include "kernel.h"
 
 #include "backend/gl/gl_common.h"
 
@@ -678,13 +679,10 @@ bool gl_create_blur_filters(session_t *ps, gl_blur_shader_t *passes, const gl_ca
 	}
 
 	for (int i = 0; i < MAX_BLUR_PASS && ps->o.blur_kerns[i]; ++i) {
-		xcb_render_fixed_t *kern = ps->o.blur_kerns[i];
+		auto kern = ps->o.blur_kerns[i];
-		if (!kern)
-			break;
-
 		// Build shader
-		int wid = XFIXED_TO_DOUBLE(kern[0]), hei = XFIXED_TO_DOUBLE(kern[1]);
+		int width = kern->w, height = kern->h;
-		int nele = wid * hei - 1;
+		int nele = width * height - 1;
 		unsigned int len =
 		    strlen(FRAG_SHADER_BLUR_PREFIX) + strlen(sampler_type) +
 		    strlen(extension) + (strlen(shader_add) + strlen(texture_func) + 42) * nele +
@@ -696,15 +694,16 @@ bool gl_create_blur_filters(session_t *ps, gl_blur_shader_t *passes, const gl_ca
 		assert(strlen(shader_str) < len);
 
 		double sum = 0.0;
-		for (int j = 0; j < hei; ++j) {
+		for (int j = 0; j < height; ++j) {
-			for (int k = 0; k < wid; ++k) {
+			for (int k = 0; k < width; ++k) {
-				if (hei / 2 == j && wid / 2 == k)
+				if (height / 2 == j && width / 2 == k)
 					continue;
-				double val = XFIXED_TO_DOUBLE(kern[2 + j * wid + k]);
+				double val = kern->data[j * width + k];
-				if (0.0 == val)
+				if (val == 0) {
 					continue;
+				}
 				sum += val;
-				sprintf(pc, shader_add, val, texture_func, k - wid / 2, j - hei / 2);
+				sprintf(pc, shader_add, val, texture_func, k - width / 2, j - height / 2);
 				pc += strlen(pc);
 				assert(strlen(shader_str) < len);
 			}

src/backend/xrender.c

@@ -5,11 +5,11 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include <xcb/xcb.h>
-#include <xcb/present.h>
-#include <xcb/sync.h>
-#include <xcb/render.h>
 #include <xcb/composite.h>
+#include <xcb/present.h>
+#include <xcb/render.h>
+#include <xcb/sync.h>
+#include <xcb/xcb.h>
 
 #include "backend/backend.h"
 #include "backend/backend_common.h"
@@ -154,19 +154,13 @@ static void compose(void *backend_data, session_t *ps, win *w, void *win_data, i
 	                     0, dst_x, dst_y, w->widthb, w->heightb);
 }
 
-/**
+static bool
- * Reset filter on a <code>Picture</code>.
+blur(void *backend_data, session_t *ps, double opacity, const region_t *reg_paint) {
- */
-static inline void xrfilter_reset(session_t *ps, xcb_render_picture_t p) {
-	const char *filter = "Nearest";
-	xcb_render_set_picture_filter(ps->c, p, strlen(filter), filter, 0, NULL);
-}
-
-static bool blur(void *backend_data, session_t *ps, double opacity, const region_t *reg_paint) {
 	struct _xrender_data *xd = backend_data;
 	const pixman_box32_t *reg = pixman_region32_extents((region_t *)reg_paint);
 	const int height = reg->y2 - reg->y1;
 	const int width = reg->x2 - reg->x1;
+	static const char *default_filter = "Nearest";
 
 	// Create a buffer for storing blurred picture, make it just big enough
 	// for the blur region
@@ -202,20 +196,18 @@ static bool blur(void *backend_data, session_t *ps, double opacity, const region
 	int i;
 	for (i = 0; ps->o.blur_kerns[i]; i++) {
 		assert(i < MAX_BLUR_PASS - 1);
-		xcb_render_fixed_t *convolution_blur = ps->o.blur_kerns[i];
-		int kwid = XFIXED_TO_DOUBLE(convolution_blur[0]),
-		    khei = XFIXED_TO_DOUBLE(convolution_blur[1]);
 
 		// Copy from source picture to destination. The filter must
 		// be applied on source picture, to get the nearby pixels outside the
 		// window.
-		xcb_render_set_picture_filter(ps->c, src_pict, strlen(XRFILTER_CONVOLUTION),
+		// TODO cache converted blur_kerns
-		                              XRFILTER_CONVOLUTION, kwid * khei + 2, convolution_blur);
+		x_set_picture_convolution_kernel(ps->c, src_pict, ps->o.blur_kerns[i]);
 
 		if (ps->o.blur_kerns[i + 1] || i == 0) {
 			// This is not the last pass, or this is the first pass
-			xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, src_pict, XCB_NONE,
+			xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, src_pict,
-			                     dst_pict, src_x, src_y, 0, 0, 0, 0, width, height);
+			                     XCB_NONE, dst_pict, src_x, src_y, 0, 0, 0, 0,
+			                     width, height);
 		} else {
 			// This is the last pass, and this is also not the first
 			xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_OVER, src_pict,
@@ -223,7 +215,9 @@ static bool blur(void *backend_data, session_t *ps, double opacity, const region
 			                     reg->y1, width, height);
 		}
 
-		xrfilter_reset(ps, src_pict);
+		// reset filter
+		xcb_render_set_picture_filter(ps->c, src_pict, strlen(default_filter),
+		                              default_filter, 0, NULL);
 
 		src_pict = tmp_picture[current];
 		dst_pict = tmp_picture[!current];
@@ -243,8 +237,8 @@ static bool blur(void *backend_data, session_t *ps, double opacity, const region
 	return true;
 }
 
-static void
+static void render_win(void *backend_data, session_t *ps, win *w, void *win_data,
-render_win(void *backend_data, session_t *ps, win *w, void *win_data, const region_t *reg_paint) {
+                       const region_t *reg_paint) {
 	struct _xrender_data *xd = backend_data;
 	struct _xrender_win_data *wd = win_data;
 
@@ -278,8 +272,9 @@ render_win(void *backend_data, session_t *ps, win *w, void *win_data, const regi
 		// Handle invert color
 		x_set_picture_clip_region(ps->c, wd->rendered_pict, 0, 0, &reg_paint_local);
 
-		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_DIFFERENCE, xd->white_pixel, XCB_NONE,
+		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_DIFFERENCE,
-		                     wd->rendered_pict, 0, 0, 0, 0, 0, 0, w->widthb, w->heightb);
+		                     xd->white_pixel, XCB_NONE, wd->rendered_pict, 0, 0,
+		                     0, 0, 0, 0, w->widthb, w->heightb);
 		// We use an extra PictOpInReverse operation to get correct pixel
 		// alpha. There could be a better solution.
 		if (win_has_alpha(w))
@@ -305,8 +300,9 @@ render_win(void *backend_data, session_t *ps, win *w, void *win_data, const regi
 
 		// Step 2: multiply alpha value
 		// XXX test
-		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, xd->white_pixel, alpha_pict,
+		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, xd->white_pixel,
-		                     wd->rendered_pict, 0, 0, 0, 0, 0, 0, w->widthb, w->heightb);
+		                     alpha_pict, wd->rendered_pict, 0, 0, 0, 0, 0, 0,
+		                     w->widthb, w->heightb);
 	}
 
 	if (w->dim) {
@@ -361,7 +357,8 @@ static void *prepare_win(void *backend_data, session_t *ps, win *w) {
 	//     leave this here until we have chance to re-think the backend API
 	if (w->shadow) {
 		xcb_pixmap_t pixmap;
-		build_shadow(ps, 1, w->widthb, w->heightb, xd->shadow_pixel, &pixmap, &wd->shadow_pict);
+		build_shadow(ps, 1, w->widthb, w->heightb, xd->shadow_pixel, &pixmap,
+		             &wd->shadow_pict);
 		xcb_free_pixmap(ps->c, pixmap);
 	}
 	return wd;
@@ -389,12 +386,12 @@ static void *init(session_t *ps) {
 
 	xd->black_pixel = solid_picture(ps, true, 1, 0, 0, 0);
 	xd->white_pixel = solid_picture(ps, true, 1, 1, 1, 1);
-	xd->shadow_pixel =
+	xd->shadow_pixel = solid_picture(ps, true, 1, ps->o.shadow_red,
-	    solid_picture(ps, true, 1, ps->o.shadow_red, ps->o.shadow_green, ps->o.shadow_blue);
+	                                 ps->o.shadow_green, ps->o.shadow_blue);
 
 	if (ps->overlay != XCB_NONE) {
-		xd->target =
+		xd->target = x_create_picture_with_visual_and_pixmap(
-		    x_create_picture_with_visual_and_pixmap(ps->c, ps->vis, ps->overlay, 0, NULL);
+		    ps->c, ps->vis, ps->overlay, 0, NULL);
 		xd->target_win = ps->overlay;
 	} else {
 		xcb_render_create_picture_value_list_t pa = {
@@ -411,16 +408,17 @@ static void *init(session_t *ps) {
 		abort();
 	}
 
-	xd->back_pixmap =
+	xd->back_pixmap = x_create_pixmap(ps->c, pictfmt->depth, ps->root, ps->root_width,
-	    x_create_pixmap(ps->c, pictfmt->depth, ps->root, ps->root_width, ps->root_height);
+	                                  ps->root_height);
-	xd->back = x_create_picture_with_pictfmt_and_pixmap(ps->c, pictfmt, xd->back_pixmap, 0, NULL);
+	xd->back = x_create_picture_with_pictfmt_and_pixmap(ps->c, pictfmt,
+	                                                    xd->back_pixmap, 0, NULL);
 
 	xcb_pixmap_t root_pixmap = x_get_root_back_pixmap(ps);
 	if (root_pixmap == XCB_NONE) {
 		xd->root_pict = solid_picture(ps, false, 1, 0.5, 0.5, 0.5);
 	} else {
-		xd->root_pict =
+		xd->root_pict = x_create_picture_with_visual_and_pixmap(
-		    x_create_picture_with_visual_and_pixmap(ps->c, ps->vis, root_pixmap, 0, NULL);
+		    ps->c, ps->vis, root_pixmap, 0, NULL);
 	}
 
 	if (ps->present_exists) {
@@ -475,8 +473,9 @@ static void present(void *backend_data, session_t *ps) {
 		// To make sure rendering won't get stuck if user toggles vsync on the
 		// fly.
 		xcb_sync_reset_fence(ps->c, xd->idle_fence);
-		xcb_present_pixmap(ps->c, xd->target_win, xd->back_pixmap, 0, XCB_NONE, XCB_NONE,
+		xcb_present_pixmap(ps->c, xd->target_win, xd->back_pixmap, 0, XCB_NONE,
-		                   0, 0, XCB_NONE, XCB_NONE, xd->idle_fence, 0, 0, 1, 0, 0, NULL);
+		                   XCB_NONE, 0, 0, XCB_NONE, XCB_NONE, xd->idle_fence, 0,
+		                   0, 1, 0, 0, NULL);
 	} else {
 		// compose() sets clip region, so clear it first to make
 		// sure we update the whole screen.
@@ -484,8 +483,9 @@ static void present(void *backend_data, session_t *ps) {
 
 		// TODO buffer-age-like optimization might be possible here.
 		//      but that will require a different backend API
-		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, xd->back, XCB_NONE, xd->target,
+		xcb_render_composite(ps->c, XCB_RENDER_PICT_OP_SRC, xd->back, XCB_NONE,
-		                     0, 0, 0, 0, 0, 0, ps->root_width, ps->root_height);
+		                     xd->target, 0, 0, 0, 0, 0, 0, ps->root_width,
+		                     ps->root_height);
 	}
 }
 

src/common.h

@@ -127,10 +127,6 @@
 // Window opacity / dim state changed
 #define WFLAG_OPCT_CHANGE   0x0004
 
-// xcb-render specific macros
-#define XFIXED_TO_DOUBLE(value) (((double) (value)) / 65536)
-#define DOUBLE_TO_XFIXED(value) ((xcb_render_fixed_t) (((double) (value)) * 65536))
-
 // === Types ===
 typedef struct glx_fbconfig glx_fbconfig_t;
 

src/config.c

@@ -17,6 +17,7 @@
 #include "log.h"
 #include "region.h"
 #include "types.h"
+#include "kernel.h"
 #include "win.h"
 
 #include "config.h"
@@ -43,109 +44,116 @@ parse_long(const char *s, long *dest) {
 }
 
 /**
- * Parse a floating-point number in matrix.
+ * Parse a floating-point number in from a string,
+ * also strips the trailing space and comma after the number.
+ *
+ * @param[in]  src  string to parse
+ * @param[out] dest return the number parsed from the string
+ * @return          pointer to the last character parsed
  */
 const char *
-parse_matrix_readnum(const char *src, double *dest) {
+parse_readnum(const char *src, double *dest) {
   const char *pc = NULL;
   double val = strtod_simple(src, &pc);
   if (!pc || pc == src) {
     log_error("No number found: %s", src);
     return src;
   }
-
+  while (*pc && (isspace(*pc) || *pc == ',')) {
-  while (*pc && (isspace(*pc) || ',' == *pc))
     ++pc;
-
+  }
   *dest = val;
-
   return pc;
 }
 
 /**
  * Parse a matrix.
+ *
+ * @param[in]  src    the blur kernel string
+ * @param[out] endptr return where the end of kernel is in the string
+ * @param[out] hasneg whether the kernel has negative values
  */
-xcb_render_fixed_t *
+conv *
-parse_matrix(const char *src, const char **endptr, bool *hasneg) {
+parse_blur_kern(const char *src, const char **endptr, bool *hasneg) {
-  int wid = 0, hei = 0;
+  int width = 0, height = 0;
   *hasneg = false;
 
   const char *pc = NULL;
 
   // Get matrix width and height
-  {
   double val = 0.0;
-    if (src == (pc = parse_matrix_readnum(src, &val)))
+  if (src == (pc = parse_readnum(src, &val)))
     goto err1;
   src = pc;
-    wid = val;
+  width = val;
-    if (src == (pc = parse_matrix_readnum(src, &val)))
+  if (src == (pc = parse_readnum(src, &val)))
     goto err1;
   src = pc;
-    hei = val;
+  height = val;
-  }
 
   // Validate matrix width and height
-  if (wid <= 0 || hei <= 0) {
+  if (width <= 0 || height <= 0) {
-    log_error("Invalid matrix width/height.");
+    log_error("Blue kernel width/height can't be negative.");
     goto err1;
   }
-  if (!(wid % 2 && hei % 2)) {
+  if (!(width % 2 && height % 2)) {
-    log_error("Width/height not odd.");
+    log_error("Blur kernel idth/height must be odd.");
     goto err1;
   }
-  if (wid > 16 || hei > 16)
+  if (width > 16 || height > 16)
-    log_warn("Matrix width/height too large, may slow down"
+    log_warn("Blur kernel width/height too large, may slow down"
              "rendering, and/or consume lots of memory");
 
   // Allocate memory
-  auto matrix = ccalloc(wid * hei + 2, xcb_render_fixed_t);
+  conv *matrix = cvalloc(sizeof(conv) + width * height * sizeof(double));
 
   // Read elements
-  {
+  int skip = height / 2 * width + width / 2;
-    int skip = hei / 2 * wid + wid / 2;
+  for (int i = 0; i < width * height; ++i) {
-    for (int i = 0; i < wid * hei; ++i) {
     // Ignore the center element
     if (i == skip) {
-        matrix[2 + i] = DOUBLE_TO_XFIXED(0);
+      matrix->data[i] = 0;
       continue;
     }
-      double val = 0;
+    if (src == (pc = parse_readnum(src, &val))) {
-      if (src == (pc = parse_matrix_readnum(src, &val)))
       goto err2;
-      src = pc;
-      if (val < 0) *hasneg = true;
-      matrix[2 + i] = DOUBLE_TO_XFIXED(val);
     }
+    src = pc;
+    if (val < 0) {
+      *hasneg = true;
+    }
+    matrix->data[i] = val;
   }
 
   // Detect trailing characters
-  for ( ;*pc && ';' != *pc; ++pc)
+  for (;*pc && *pc != ';'; pc++) {
-    if (!isspace(*pc) && ',' != *pc) {
+    if (!isspace(*pc) && *pc != ',') {
-      log_error("Trailing characters in matrix string.");
+      // TODO isspace is locale aware, be careful
+      log_error("Trailing characters in blur kernel string.");
       goto err2;
     }
+  }
 
   // Jump over spaces after ';'
-  if (';' == *pc) {
+  if (*pc == ';') {
-    ++pc;
+    pc++;
-    while (*pc && isspace(*pc))
+    while (*pc && isspace(*pc)) {
       ++pc;
     }
+  }
 
   // Require an end of string if endptr is not provided, otherwise
   // copy end pointer to endptr
-  if (endptr)
+  if (endptr) {
     *endptr = pc;
-  else if (*pc) {
+  } else if (*pc) {
-    log_error("Only one matrix expected.");
+    log_error("Only one blur kernel expected.");
     goto err2;
   }
 
   // Fill in width and height
-  matrix[0] = DOUBLE_TO_XFIXED(wid);
+  matrix->w = width;
-  matrix[1] = DOUBLE_TO_XFIXED(hei);
+  matrix->h = height;
-
   return matrix;
 
 err2:
@@ -154,25 +162,19 @@ err1:
   return NULL;
 }
 
-/**
- * Parse a convolution kernel.
- *
- * Output:
- *   hasneg: whether the convolution kernel has negative values
- */
-xcb_render_fixed_t *
-parse_conv_kern(const char *src, const char **endptr, bool *hasneg) {
-  return parse_matrix(src, endptr, hasneg);
-}
-
 /**
  * Parse a list of convolution kernels.
  *
- * Output:
+ * @param[in]  src    string to parse
- *   hasneg: whether any of the convolution kernel has negative values
+ * @param[out] dest   pointer to an array of kernels, must points to an array
+ *                    of `max` elements.
+ * @param[in]  max    maximum number of kernels supported
+ * @param[out] hasneg whether any of the kernels have negative values
+ * @return            if the `src` string is a valid kernel list string
  */
 bool
-parse_conv_kern_lst(const char *src, xcb_render_fixed_t **dest, int max, bool *hasneg) {
+parse_blur_kern_lst(const char *src, conv **dest, int max, bool *hasneg) {
+  // TODO just return a predefined kernels, not parse predefined strings...
   static const struct {
     const char *name;
     const char *kern_str;
@@ -188,11 +190,11 @@ parse_conv_kern_lst(const char *src, xcb_render_fixed_t **dest, int max, bool *h
   };
 
   *hasneg = false;
-
   for (unsigned int i = 0;
-      i < sizeof(CONV_KERN_PREDEF) / sizeof(CONV_KERN_PREDEF[0]); ++i)
+       i < sizeof(CONV_KERN_PREDEF) / sizeof(CONV_KERN_PREDEF[0]); ++i) {
     if (!strcmp(CONV_KERN_PREDEF[i].name, src))
-      return parse_conv_kern_lst(CONV_KERN_PREDEF[i].kern_str, dest, max, hasneg);
+      return parse_blur_kern_lst(CONV_KERN_PREDEF[i].kern_str, dest, max, hasneg);
+  }
 
   int i = 0;
   const char *pc = src;
@@ -207,8 +209,11 @@ parse_conv_kern_lst(const char *src, xcb_render_fixed_t **dest, int max, bool *h
   i = 0;
   while (pc && *pc && i < max - 1) {
     bool tmp_hasneg;
-    if (!(dest[i++] = parse_conv_kern(pc, &pc, &tmp_hasneg)))
+    dest[i] = parse_blur_kern(pc, &pc, &tmp_hasneg);
+    if (!dest[i]) {
       return false;
+    }
+    i++;
     *hasneg |= tmp_hasneg;
   }
 

src/config.h

@@ -24,6 +24,7 @@
 #include "compiler.h"
 #include "win.h"
 #include "types.h"
+#include "kernel.h"
 
 typedef struct session session_t;
 
@@ -207,7 +208,7 @@ typedef struct options_t {
 	/// Background blur blacklist. A linked list of conditions.
 	c2_lptr_t *blur_background_blacklist;
 	/// Blur convolution kernel.
-	xcb_render_fixed_t *blur_kerns[MAX_BLUR_PASS];
+	conv *blur_kerns[MAX_BLUR_PASS];
 	/// How much to dim an inactive window. 0.0 - 1.0, 0 to disable.
 	double inactive_dim;
 	/// Whether to use fixed inactive dim opacity, instead of deciding
@@ -245,13 +246,8 @@ extern const char *const VSYNC_STRS[NUM_VSYNC + 1];
 extern const char *const BACKEND_STRS[NUM_BKEND + 1];
 
 attr_warn_unused_result bool parse_long(const char *, long *);
-attr_warn_unused_result const char *parse_matrix_readnum(const char *, double *);
-attr_warn_unused_result xcb_render_fixed_t *
-parse_matrix(const char *, const char **, bool *hasneg);
-attr_warn_unused_result xcb_render_fixed_t *
-parse_conv_kern(const char *, const char **, bool *hasneg);
 attr_warn_unused_result bool
-parse_conv_kern_lst(const char *, xcb_render_fixed_t **, int, bool *hasneg);
+parse_blur_kern_lst(const char *, conv **, int, bool *hasneg);
 attr_warn_unused_result bool parse_geometry(session_t *, const char *, region_t *);
 attr_warn_unused_result bool parse_rule_opacity(c2_lptr_t **, const char *);
 

src/config_libconfig.c

@@ -379,7 +379,7 @@ char *parse_config_libconfig(options_t *opt, const char *config_file, bool *shad
       &opt->blur_background_fixed);
   // --blur-kern
   if (config_lookup_string(&cfg, "blur-kern", &sval) &&
-      !parse_conv_kern_lst(sval, opt->blur_kerns, MAX_BLUR_PASS, conv_kern_hasneg)) {
+      !parse_blur_kern_lst(sval, opt->blur_kerns, MAX_BLUR_PASS, conv_kern_hasneg)) {
     log_fatal("Cannot parse \"blur-kern\"");
     goto err;
   }

src/opengl.c

@@ -23,6 +23,7 @@
 #include "utils.h"
 #include "win.h"
 #include "region.h"
+#include "kernel.h"
 #include "backend/gl/gl_common.h"
 #include "backend/gl/glx.h"
 
@@ -357,16 +358,13 @@ glx_init_blur(session_t *ps) {
     }
 
     for (int i = 0; i < MAX_BLUR_PASS && ps->o.blur_kerns[i]; ++i) {
-      xcb_render_fixed_t *kern = ps->o.blur_kerns[i];
+      auto kern = ps->o.blur_kerns[i];
-      if (!kern)
-        break;
-
       glx_blur_pass_t *ppass = &ps->psglx->blur_passes[i];
 
       // Build shader
       {
-        int wid = XFIXED_TO_DOUBLE(kern[0]), hei = XFIXED_TO_DOUBLE(kern[1]);
+        int width = kern->w, height = kern->h;
-        int nele = wid * hei - 1;
+        int nele = width * height - 1;
         unsigned int len = strlen(FRAG_SHADER_BLUR_PREFIX) +
                            strlen(sampler_type) +
                            strlen(extension) +
@@ -380,15 +378,16 @@ glx_init_blur(session_t *ps) {
         assert(strlen(shader_str) < len);
 
         double sum = 0.0;
-        for (int j = 0; j < hei; ++j) {
+        for (int j = 0; j < height; ++j) {
-          for (int k = 0; k < wid; ++k) {
+          for (int k = 0; k < width; ++k) {
-            if (hei / 2 == j && wid / 2 == k)
+            if (height / 2 == j && width / 2 == k)
               continue;
-            double val = XFIXED_TO_DOUBLE(kern[2 + j * wid + k]);
+            double val = kern->data[j * width + k];
-            if (0.0 == val)
+            if (val == 0) {
               continue;
+	    }
             sum += val;
-            sprintf(pc, shader_add, val, texture_func, k - wid / 2, j - hei / 2);
+            sprintf(pc, shader_add, val, texture_func, k - width / 2, j - height / 2);
             pc += strlen(pc);
             assert(strlen(shader_str) < len);
           }

src/options.c

@@ -711,7 +711,7 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable,
 			break;
 		case 301:
 			// --blur-kern
-			if (!parse_conv_kern_lst(optarg, opt->blur_kerns,
+			if (!parse_blur_kern_lst(optarg, opt->blur_kerns,
 			                         MAX_BLUR_PASS, &conv_kern_hasneg))
 				exit(1);
 			break;
@@ -818,35 +818,18 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable,
 
 	// Fill default blur kernel
 	if (opt->blur_background && !opt->blur_kerns[0]) {
-		// Convolution filter parameter (box blur)
+		bool ret = parse_blur_kern_lst("3x3box", opt->blur_kerns, MAX_BLUR_PASS, &conv_kern_hasneg);
-		// gaussian or binomial filters are definitely superior, yet looks
+		assert(ret);
-		// like they aren't supported as of xorg-server-1.13.0
-		static const xcb_render_fixed_t convolution_blur[] = {
-		    // Must convert to XFixed with DOUBLE_TO_XFIXED()
-		    // Matrix size
-		    DOUBLE_TO_XFIXED(3),
-		    DOUBLE_TO_XFIXED(3),
-		    // Matrix
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		    DOUBLE_TO_XFIXED(1),
-		};
-		opt->blur_kerns[0] = ccalloc(ARR_SIZE(convolution_blur), xcb_render_fixed_t);
-		memcpy(opt->blur_kerns[0], convolution_blur, sizeof(convolution_blur));
 	}
 
-	if (opt->resize_damage < 0)
+	if (opt->resize_damage < 0) {
 		log_warn("Negative --resize-damage will not work correctly.");
+	}
 
-	if (opt->backend == BKEND_XRENDER && conv_kern_hasneg)
+	if (opt->backend == BKEND_XRENDER && conv_kern_hasneg) {
 		log_warn("A convolution kernel with negative values may not work "
 		         "properly under X Render backend.");
 	}
+}
 
 // vim: set noet sw=8 ts=8 :

src/render.c

@@ -582,18 +582,6 @@ static inline void win_paint_shadow(session_t *ps, win *w, region_t *reg_paint)
 	       w->shadow_paint.ptex, reg_paint, NULL);
 }
 
-/**
- * Normalize a convolution kernel.
- */
-static inline void normalize_conv_kern(int wid, int hei, xcb_render_fixed_t *kern) {
-	double sum = 0.0;
-	for (int i = 0; i < wid * hei; ++i)
-		sum += XFIXED_TO_DOUBLE(kern[i]);
-	double factor = 1.0 / sum;
-	for (int i = 0; i < wid * hei; ++i)
-		kern[i] = DOUBLE_TO_XFIXED(XFIXED_TO_DOUBLE(kern[i]) * factor);
-}
-
 /**
  * @brief Blur an area on a buffer.
  *
@@ -630,8 +618,9 @@ static bool xr_blur_dst(session_t *ps, xcb_render_picture_t tgt_buffer, int x, i
 	for (int i = 0; blur_kerns[i]; ++i) {
 		assert(i < MAX_BLUR_PASS - 1);
 		xcb_render_fixed_t *convolution_blur = blur_kerns[i];
-		int kwid = XFIXED_TO_DOUBLE(convolution_blur[0]),
+		// `x / 65536.0` converts from X fixed point to double
-		    khei = XFIXED_TO_DOUBLE(convolution_blur[1]);
+		int kwid = ((double)convolution_blur[0]) / 65536.0,
+		    khei = ((double)convolution_blur[1]) / 65536.0;
 		bool rd_from_tgt = (tgt_buffer == src_pict);
 
 		// Copy from source picture to destination. The filter must
@@ -685,7 +674,9 @@ static inline void win_blur_background(session_t *ps, win *w, xcb_render_picture
 	case BKEND_XR_GLX_HYBRID: {
 		// Normalize blur kernels
 		for (int i = 0; i < MAX_BLUR_PASS; ++i) {
-			xcb_render_fixed_t *kern_src = ps->o.blur_kerns[i];
+			// Note: `x * 65536` converts double `x` to a X fixed point
+			// representation. `x / 65536` is the other way.
+			auto kern_src = ps->o.blur_kerns[i];
 			xcb_render_fixed_t *kern_dst = ps->blur_kerns_cache[i];
 			assert(i < MAX_BLUR_PASS);
 			if (!kern_src) {
@@ -693,30 +684,33 @@ static inline void win_blur_background(session_t *ps, win *w, xcb_render_picture
 				break;
 			}
 
-			assert(!kern_dst ||
+			assert(!kern_dst || (kern_src->w == kern_dst[0] / 65536 &&
-			       (kern_src[0] == kern_dst[0] && kern_src[1] == kern_dst[1]));
+			                     kern_src->h == kern_dst[1] / 65536));
 
 			// Skip for fixed factor_center if the cache exists already
 			if (ps->o.blur_background_fixed && kern_dst)
 				continue;
 
-			int kwid = XFIXED_TO_DOUBLE(kern_src[0]),
-			    khei = XFIXED_TO_DOUBLE(kern_src[1]);
-
 			// Allocate cache space if needed
 			if (!kern_dst) {
-				kern_dst = ccalloc(kwid * khei + 2, xcb_render_fixed_t);
+				kern_dst = ccalloc(kern_src->w * kern_src->h + 2,
+				                   xcb_render_fixed_t);
 				ps->blur_kerns_cache[i] = kern_dst;
 			}
 
+			double sum = factor_center;
+			for (int j = 0; j < kern_src->w * kern_src->h; j++) {
+				sum += kern_src->data[j];
+			}
+			// Copy src to dst, normalizing in the process
+			for (int j = 0; j < kern_src->w * kern_src->h; j++) {
+				kern_dst[j + 2] = kern_src->data[j] / sum * 65536;
+			}
 			// Modify the factor of the center pixel
-			kern_src[2 + (khei / 2) * kwid + kwid / 2] =
+			kern_dst[2 + (kern_src->h / 2) * kern_src->w + kern_src->w / 2] =
-			    DOUBLE_TO_XFIXED(factor_center);
+			    factor_center / sum * 65536;
-
+			kern_dst[0] = kern_src->w * 65536;
-			// Copy over
+			kern_dst[1] = kern_src->h * 65536;
-			memcpy(kern_dst, kern_src,
-			       (kwid * khei + 2) * sizeof(xcb_render_fixed_t));
-			normalize_conv_kern(kwid, khei, kern_dst + 2);
 		}
 
 		// Minimize the region we try to blur, if the window itself is not
@@ -1017,8 +1011,8 @@ void paint_all(session_t *ps, win *const t, bool ignore_damage) {
 			glFlush();
 		glXWaitX();
 		assert(ps->tgt_buffer.pixmap);
-		paint_bind_tex(ps, &ps->tgt_buffer, ps->root_width, ps->root_height, false,
+		paint_bind_tex(ps, &ps->tgt_buffer, ps->root_width, ps->root_height,
-		               ps->depth, ps->vis, !ps->o.glx_no_rebind_pixmap);
+		               false, ps->depth, ps->vis, !ps->o.glx_no_rebind_pixmap);
 		if (ps->o.vsync_use_glfinish)
 			glFinish();
 		else

src/x.c

@@ -17,6 +17,7 @@
 #include "region.h"
 #include "compiler.h"
 #include "common.h"
+#include "kernel.h"
 #include "x.h"
 #include "log.h"
 #include "backend/gl/glx.h"
@@ -513,3 +514,29 @@ bool x_fence_sync(xcb_connection_t *c, xcb_sync_fence_t f) {
   }
   return true;
 }
+
+// xcb-render specific macros
+#define XFIXED_TO_DOUBLE(value) (((double) (value)) / 65536)
+#define DOUBLE_TO_XFIXED(value) ((xcb_render_fixed_t) (((double) (value)) * 65536))
+
+/**
+ * Set the picture filter of a xrender picture to a convolution
+ * kernel.
+ *
+ * @param c   xcb connection
+ * @param pict the picture
+ * @param kern the convolution kernel
+ */
+void
+x_set_picture_convolution_kernel(xcb_connection_t *c,
+                                 xcb_render_picture_t pict, conv *kernel) {
+  auto buf = ccalloc(kernel->w * kernel->h + 2, xcb_render_fixed_t);
+  static const char *filter = "convolution";
+  buf[0] = DOUBLE_TO_XFIXED(kernel->w);
+  buf[1] = DOUBLE_TO_XFIXED(kernel->h);
+  for (int i = 0; i < kernel->w * kernel->h; i++) {
+    buf[i + 2] = DOUBLE_TO_XFIXED(kernel->data[i]);
+  }
+  xcb_render_set_picture_filter(c, pict, sizeof(filter), filter, kernel->w * kernel->h + 2, buf);
+  free(buf);
+}

src/x.h

@@ -12,6 +12,7 @@
 
 #include "compiler.h"
 #include "region.h"
+#include "kernel.h"
 
 typedef struct session session_t;
 
@@ -167,3 +168,15 @@ xcb_pixmap_t x_get_root_back_pixmap(session_t *ps);
 bool x_is_root_back_pixmap_atom(session_t *ps, xcb_atom_t atom);
 
 bool x_fence_sync(xcb_connection_t *, xcb_sync_fence_t);
+
+/**
+ * Set the picture filter of a xrender picture to a convolution
+ * kernel.
+ *
+ * @param c   xcb connection
+ * @param pict the picture
+ * @param kern the convolution kernel
+ */
+void
+x_set_picture_convolution_kernel(xcb_connection_t *c,
+                                 xcb_render_picture_t pict, conv *kernel);