Merge pull request #2225 from jlindgren90/icon-scaling

Improve icon display especially at larger output scales

include/buffer.h

@@ -61,6 +61,18 @@ struct lab_data_buffer *buffer_adopt_cairo_surface(cairo_surface_t *surface);
 struct lab_data_buffer *buffer_create_cairo(uint32_t logical_width,
 	uint32_t logical_height, float scale);
 
+/*
+ * Create a buffer from an image surface, for display as an icon.
+ *
+ * The surface is either adopted by the buffer (which takes ownership),
+ * or copied and then destroyed.
+ *
+ * This function allows non-ARGB32 source images and converts to
+ * CAIRO_FORMAT_ARGB32 if needed.
+ */
+struct lab_data_buffer *buffer_convert_cairo_surface_for_icon(
+	cairo_surface_t *surface, uint32_t icon_size, float scale);
+
 /*
  * Create a buffer which holds (and takes ownership of) raw pixel data
  * in pre-multiplied ARGB32 format.

include/common/box.h

@@ -4,14 +4,23 @@
 
 #include <wlr/util/box.h>
 
-bool
-box_contains(struct wlr_box *box_super, struct wlr_box *box_sub);
+bool box_contains(struct wlr_box *box_super, struct wlr_box *box_sub);
 
-bool
-box_intersects(struct wlr_box *box_a, struct wlr_box *box_b);
+bool box_intersects(struct wlr_box *box_a, struct wlr_box *box_b);
 
 /* Returns the bounding box of 2 boxes */
-void
-box_union(struct wlr_box *box_dest, struct wlr_box *box_a, struct wlr_box *box_b);
+void box_union(struct wlr_box *box_dest, struct wlr_box *box_a,
+	struct wlr_box *box_b);
+
+/*
+ * Fits and centers a content box (width & height) within a bounding box
+ * (max_width & max_height). The content box is downscaled if necessary
+ * (preserving aspect ratio) but not upscaled.
+ *
+ * The returned x & y coordinates are the centered content position
+ * relative to the top-left corner of the bounding box.
+ */
+struct wlr_box box_fit_within(int width, int height, int max_width,
+	int max_height);
 
 #endif /* LABWC_BOX_H */

include/icon-loader.h

@@ -7,6 +7,6 @@ struct server;
 void icon_loader_init(struct server *server);
 void icon_loader_finish(struct server *server);
 struct lab_data_buffer *icon_loader_lookup(struct server *server,
-	const char *app_id, int size, int scale);
+	const char *app_id, int size, float scale);
 
 #endif /* LABWC_ICON_LOADER_H */

include/img/img-png.h

@@ -4,6 +4,7 @@
 
 struct lab_data_buffer;
 
-void img_png_load(const char *filename, struct lab_data_buffer **buffer);
+void img_png_load(const char *filename, struct lab_data_buffer **buffer,
+	int size, float scale);
 
 #endif /* LABWC_IMG_PNG_H */

include/img/img-svg.h

@@ -5,6 +5,6 @@
 struct lab_data_buffer;
 
 void img_svg_load(const char *filename, struct lab_data_buffer **buffer,
-	int size);
+	int size, float scale);
 
 #endif /* LABWC_IMG_SVG_H */

include/img/img-xpm.h

@@ -4,6 +4,7 @@
 
 struct lab_data_buffer;
 
-void img_xpm_load(const char *filename, struct lab_data_buffer **buffer);
+void img_xpm_load(const char *filename, struct lab_data_buffer **buffer,
+	int size, float scale);
 
 #endif /* LABWC_IMG_XPM_H */

include/labwc.h

@@ -546,6 +546,14 @@ struct wlr_box output_usable_area_scaled(struct output *output);
 void handle_output_power_manager_set_mode(struct wl_listener *listener,
 	void *data);
 void output_enable_adaptive_sync(struct wlr_output *output, bool enabled);
+
+/**
+ * output_max_scale() - get maximum scale factor of all usable outputs.
+ * Used when loading/rendering resources (e.g. icons) that may be
+ * displayed on any output.
+ */
+float output_max_scale(struct server *server);
+
 void new_tearing_hint(struct wl_listener *listener, void *data);
 
 void server_init(struct server *server);

include/ssd-internal.h

@@ -149,7 +149,7 @@ struct ssd_part *add_scene_button(struct wl_list *part_list,
 	struct lab_data_buffer *buffers[LAB_BS_ALL + 1], int x,
 	struct view *view);
 void update_window_icon_buffer(struct wlr_scene_node *button_node,
-	struct wlr_buffer *buffer);
+	struct lab_data_buffer *buffer);
 
 /* SSD internal helpers */
 struct ssd_part *ssd_get_part(

src/buffer.c

@@ -29,6 +29,7 @@
 #include <drm_fourcc.h>
 #include <wlr/interfaces/wlr_buffer.h>
 #include "buffer.h"
+#include "common/box.h"
 #include "common/mem.h"
 
 static const struct wlr_buffer_impl data_buffer_impl;
@@ -134,6 +135,58 @@ buffer_create_cairo(uint32_t logical_width, uint32_t logical_height, float scale
 	return buffer;
 }
 
+struct lab_data_buffer *
+buffer_convert_cairo_surface_for_icon(cairo_surface_t *surface,
+		uint32_t icon_size, float scale)
+{
+	assert(cairo_surface_get_type(surface) == CAIRO_SURFACE_TYPE_IMAGE);
+
+	/*
+	 * Compute logical size for display and decide whether we can
+	 * use the image data directly (fast path). Requirements are:
+	 *
+	 *  - The pixel format must be ARGB32.
+	 *  - The image must not be so large as to need downsampling by
+	 *    more than 2x when displayed at the target scale. wlr_scene
+	 *    uses linear interpolation without pixel averaging, which
+	 *    starts to skip samples if downsampling more than 2x,
+	 *    resulting in a grainy look.
+	 */
+	int width = cairo_image_surface_get_width(surface);
+	int height = cairo_image_surface_get_height(surface);
+	struct wlr_box logical =
+		box_fit_within(width, height, icon_size, icon_size);
+	struct lab_data_buffer *buffer;
+
+	if (cairo_image_surface_get_format(surface) == CAIRO_FORMAT_ARGB32
+			&& width <= 2 * logical.width * scale
+			&& height <= 2 * logical.height * scale) {
+		buffer = buffer_adopt_cairo_surface(surface);
+		/* set logical size for display */
+		buffer->logical_width = logical.width;
+		buffer->logical_height = logical.height;
+	} else {
+		/* convert to ARGB32 and scale for display (slow path) */
+		buffer = buffer_create_cairo(logical.width,
+			logical.height, scale);
+
+		cairo_t *cairo = buffer->cairo;
+		cairo_scale(cairo, (double)logical.width / width,
+			(double)logical.height / height);
+		cairo_set_source_surface(cairo, surface, 0, 0);
+		cairo_pattern_set_filter(cairo_get_source(cairo),
+			CAIRO_FILTER_GOOD);
+		cairo_paint(cairo);
+
+		/* ensure pixel data is updated */
+		cairo_surface_flush(buffer->surface);
+		/* destroy original surface */
+		cairo_surface_destroy(surface);
+	}
+
+	return buffer;
+}
+
 struct lab_data_buffer *
 buffer_create_from_data(void *pixel_data, uint32_t width, uint32_t height,
 		uint32_t stride)

src/common/box.c

@@ -47,3 +47,29 @@ box_union(struct wlr_box *box_dest, struct wlr_box *box_a, struct wlr_box *box_b
 	box_dest->width = x2 - x1;
 	box_dest->height = y2 - y1;
 }
+
+struct wlr_box
+box_fit_within(int width, int height, int max_width, int max_height)
+{
+	struct wlr_box box;
+
+	if (width <= max_width && height <= max_height) {
+		/* No downscaling needed */
+		box.width = width;
+		box.height = height;
+	} else if (width * max_height > height * max_width) {
+		/* Wider content, fit width */
+		box.width = max_width;
+		box.height = (height * max_width + (width / 2)) / width;
+	} else {
+		/* Taller content, fit height */
+		box.width = (width * max_height + (height / 2)) / height;
+		box.height = max_height;
+	}
+
+	/* Compute centered position */
+	box.x = (max_width - box.width) / 2;
+	box.y = (max_height - box.height) / 2;
+
+	return box;
+}

src/icon-loader.c

@@ -5,6 +5,7 @@
 #include <string.h>
 #include <strings.h>
 #include <wlr/util/log.h>
+#include "common/macros.h"
 #include "common/mem.h"
 #include "common/string-helpers.h"
 #include "config.h"
@@ -219,7 +220,8 @@ get_db_entry_by_id_fuzzy(struct sfdo_desktop_db *db, const char *app_id)
 }
 
 struct lab_data_buffer *
-icon_loader_lookup(struct server *server, const char *app_id, int size, int scale)
+icon_loader_lookup(struct server *server, const char *app_id, int size,
+		float scale)
 {
 	struct icon_loader *loader = server->icon_loader;
 	if (!loader) {
@@ -236,16 +238,23 @@ icon_loader_lookup(struct server *server, const char *app_id, int size, int scal
 		icon_name = sfdo_desktop_entry_get_icon(entry, NULL);
 	}
 
+	/*
+	 * libsfdo doesn't support loading icons for fractional scales,
+	 * so round down and increase the icon size to compensate.
+	 */
+	int lookup_scale = MAX((int)scale, 1);
+	int lookup_size = lroundf(size * scale / lookup_scale);
+
 	struct icon_ctx ctx = {0};
 	int ret;
 	if (!icon_name) {
 		/* fall back to app id */
-		ret = process_rel_name(&ctx, app_id, loader, size, scale);
+		ret = process_rel_name(&ctx, app_id, loader, lookup_size, lookup_scale);
 	} else if (icon_name[0] == '/') {
 		ret = process_abs_name(&ctx, icon_name);
 	} else {
 		/* this should be the case for most icons */
-		ret = process_rel_name(&ctx, icon_name, loader, size, scale);
+		ret = process_rel_name(&ctx, icon_name, loader, lookup_size, lookup_scale);
 	}
 	if (ret < 0) {
 		return NULL;
@@ -257,15 +266,15 @@ icon_loader_lookup(struct server *server, const char *app_id, int size, int scal
 
 	switch (ctx.format) {
 	case SFDO_ICON_FILE_FORMAT_PNG:
-		img_png_load(ctx.path, &icon_buffer);
+		img_png_load(ctx.path, &icon_buffer, size, scale);
 		break;
 	case SFDO_ICON_FILE_FORMAT_SVG:
 #if HAVE_RSVG
-		img_svg_load(ctx.path, &icon_buffer, size * scale);
+		img_svg_load(ctx.path, &icon_buffer, size, scale);
 #endif
 		break;
 	case SFDO_ICON_FILE_FORMAT_XPM:
-		img_xpm_load(ctx.path, &icon_buffer);
+		img_xpm_load(ctx.path, &icon_buffer, size, scale);
 		break;
 	}
 

src/img/img-png.c

@@ -43,7 +43,8 @@ ispng(const char *filename)
 #undef PNG_BYTES_TO_CHECK
 
 void
-img_png_load(const char *filename, struct lab_data_buffer **buffer)
+img_png_load(const char *filename, struct lab_data_buffer **buffer, int size,
+		float scale)
 {
 	if (*buffer) {
 		wlr_buffer_drop(&(*buffer)->base);
@@ -62,22 +63,6 @@ img_png_load(const char *filename, struct lab_data_buffer **buffer)
 		cairo_surface_destroy(image);
 		return;
 	}
-	cairo_surface_flush(image);
 
-	if (cairo_image_surface_get_format(image) == CAIRO_FORMAT_ARGB32) {
-		/* we can wrap ARGB32 image surfaces directly */
-		*buffer = buffer_adopt_cairo_surface(image);
-		return;
-	}
-
-	/* convert to ARGB32 by painting to a new surface */
-	uint32_t w = cairo_image_surface_get_width(image);
-	uint32_t h = cairo_image_surface_get_height(image);
-	*buffer = buffer_create_cairo(w, h, 1);
-	cairo_t *cairo = (*buffer)->cairo;
-	cairo_set_source_surface(cairo, image, 0, 0);
-	cairo_paint(cairo);
-	cairo_surface_flush((*buffer)->surface);
-	/* destroy original surface */
-	cairo_surface_destroy(image);
+	*buffer = buffer_convert_cairo_surface_for_icon(image, size, scale);
 }

src/img/img-svg.c

@@ -15,8 +15,8 @@
 #include "labwc.h"
 
 void
-img_svg_load(const char *filename, struct lab_data_buffer **buffer,
-		int size)
+img_svg_load(const char *filename, struct lab_data_buffer **buffer, int size,
+		float scale)
 {
 	if (*buffer) {
 		wlr_buffer_drop(&(*buffer)->base);
@@ -39,7 +39,7 @@ img_svg_load(const char *filename, struct lab_data_buffer **buffer,
 		return;
 	}
 
-	*buffer = buffer_create_cairo(size, size, 1.0);
+	*buffer = buffer_create_cairo(size, size, scale);
 	cairo_surface_t *image = (*buffer)->surface;
 	cairo_t *cr = (*buffer)->cairo;
 

src/img/img-xpm.c

@@ -286,9 +286,8 @@ file_buffer(enum buf_op op, struct file_handle *h)
 	return NULL;
 }
 
-/* This function does all the work. */
-static struct lab_data_buffer *
-pixbuf_create_from_xpm(struct file_handle *handle)
+static cairo_surface_t *
+xpm_load_to_surface(struct file_handle *handle)
 {
 	const char *buffer = file_buffer(op_header, handle);
 	if (!buffer) {
@@ -332,7 +331,7 @@ pixbuf_create_from_xpm(struct file_handle *handle)
 
 	char *name_buf = xzalloc(n_col * (cpp + 1));
 	struct xpm_color *colors = znew_n(struct xpm_color, n_col);
-	uint32_t *data = znew_n(uint32_t, w * h);
+	cairo_surface_t *surface = NULL;
 	struct xpm_color *fallbackcolor = NULL;
 	char pixel_str[32]; /* cpp < 32 */
 
@@ -357,14 +356,20 @@ pixbuf_create_from_xpm(struct file_handle *handle)
 		}
 	}
 
+	surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w, h);
+	uint32_t *data = (uint32_t *)cairo_image_surface_get_data(surface);
+	int stride = cairo_image_surface_get_stride(surface) / sizeof(uint32_t);
+
 	for (int ycnt = 0; ycnt < h; ycnt++) {
-		uint32_t *pixtmp = data + w * ycnt;
+		uint32_t *pixtmp = data + stride * ycnt;
 		int wbytes = w * cpp;
 
 		buffer = file_buffer(op_body, handle);
 		if (!buffer || (strlen(buffer) < (size_t)wbytes)) {
 			/* Advertised width doesn't match pixels */
 			wlr_log(WLR_DEBUG, "Dimensions do not match data");
+			cairo_surface_destroy(surface);
+			surface = NULL;
 			goto out;
 		}
 
@@ -382,24 +387,19 @@ pixbuf_create_from_xpm(struct file_handle *handle)
 			*pixtmp++ = color->argb;
 		}
 	}
-
-	g_hash_table_destroy(color_hash);
-	free(colors);
-	free(name_buf);
-
-	return buffer_create_from_data(data, w, h, 4 * w);
+	/* let cairo know pixel data has been modified */
+	cairo_surface_mark_dirty(surface);
 
 out:
 	g_hash_table_destroy(color_hash);
 	free(colors);
 	free(name_buf);
-	free(data);
-
-	return NULL;
+	return surface;
 }
 
 void
-img_xpm_load(const char *filename, struct lab_data_buffer **buffer)
+img_xpm_load(const char *filename, struct lab_data_buffer **buffer, int size,
+		float scale)
 {
 	if (*buffer) {
 		wlr_buffer_drop(&(*buffer)->base);
@@ -413,8 +413,11 @@ img_xpm_load(const char *filename, struct lab_data_buffer **buffer)
 		return;
 	}
 
-	*buffer = pixbuf_create_from_xpm(&h);
-	if (!(*buffer)) {
+	cairo_surface_t *surface = xpm_load_to_surface(&h);
+	if (surface) {
+		*buffer = buffer_convert_cairo_surface_for_icon(surface, size,
+			scale);
+	} else {
 		wlr_log(WLR_ERROR, "error loading '%s'", filename);
 	}
 

src/output.c

@@ -1067,3 +1067,17 @@ output_enable_adaptive_sync(struct wlr_output *output, bool enabled)
 			enabled ? "en" : "dis", output->name);
 	}
 }
+
+float
+output_max_scale(struct server *server)
+{
+	/* Never return less than 1, in case outputs are disabled */
+	float scale = 1;
+	struct output *output;
+	wl_list_for_each(output, &server->outputs, link) {
+		if (output_is_usable(output)) {
+			scale = MAX(scale, output->wlr_output->scale);
+		}
+	}
+	return scale;
+}

src/ssd/ssd-part.c

@@ -2,6 +2,7 @@
 
 #include <assert.h>
 #include "buffer.h"
+#include "common/box.h"
 #include "common/list.h"
 #include "common/mem.h"
 #include "labwc.h"
@@ -80,34 +81,16 @@ add_scene_buffer(struct wl_list *list, enum ssd_part_type type,
 }
 
 static struct wlr_box
-get_scale_box(struct wlr_buffer *buffer, double container_width,
-		double container_height)
+get_scale_box(struct lab_data_buffer *buffer, int container_width,
+		int container_height)
 {
-	struct wlr_box icon_geo = {
-		.width = buffer->width,
-		.height = buffer->height
-	};
-
-	/* Scale down buffer if required */
-	if (icon_geo.width && icon_geo.height) {
-		double scale = MIN(container_width / icon_geo.width,
-			container_height / icon_geo.height);
-		if (scale < 1.0f) {
-			icon_geo.width = (double)icon_geo.width * scale;
-			icon_geo.height = (double)icon_geo.height * scale;
-		}
-	}
-
-	/* Center buffer on both axis */
-	icon_geo.x = (container_width - icon_geo.width) / 2;
-	icon_geo.y = (container_height - icon_geo.height) / 2;
-
-	return icon_geo;
+	return box_fit_within(buffer->logical_width, buffer->logical_height,
+		container_width, container_height);
 }
 
 void
 update_window_icon_buffer(struct wlr_scene_node *button_node,
-		struct wlr_buffer *buffer)
+		struct lab_data_buffer *buffer)
 {
 	struct wlr_scene_buffer *scene_buffer =
 		wlr_scene_buffer_from_node(button_node);
@@ -116,7 +99,7 @@ update_window_icon_buffer(struct wlr_scene_node *button_node,
 		rc.theme->window_button_width,
 		rc.theme->title_height);
 
-	wlr_scene_buffer_set_buffer(scene_buffer, buffer);
+	wlr_scene_buffer_set_buffer(scene_buffer, &buffer->base);
 	wlr_scene_buffer_set_dest_size(scene_buffer,
 		icon_geo.width, icon_geo.height);
 	wlr_scene_node_set_position(button_node, icon_geo.x, icon_geo.y);
@@ -145,11 +128,11 @@ add_scene_button(struct wl_list *part_list, enum ssd_part_type type,
 		if (!buffers[state_set]) {
 			continue;
 		}
-		struct wlr_buffer *icon_buffer = &buffers[state_set]->base;
+		struct lab_data_buffer *icon_buffer = buffers[state_set];
 		struct wlr_box icon_geo = get_scale_box(icon_buffer,
 			rc.theme->window_button_width, rc.theme->title_height);
 		struct ssd_part *icon_part = add_scene_buffer(part_list, type,
-			parent, icon_buffer, icon_geo.x, icon_geo.y);
+			parent, &icon_buffer->base, icon_geo.x, icon_geo.y);
 		/* Make sure big icons are scaled down if necessary */
 		wlr_scene_buffer_set_dest_size(
 			wlr_scene_buffer_from_node(icon_part->node),

src/ssd/ssd-titlebar.c

@@ -604,8 +604,16 @@ ssd_update_window_icon(struct ssd *ssd)
 	int hpad = theme->window_button_width / 10;
 	int icon_size = MIN(theme->window_button_width - 2 * hpad,
 		theme->title_height - 2 * theme->padding_height);
-	/* TODO: take into account output scales */
-	int icon_scale = 1;
+
+	/*
+	 * Load/render icons at the max scale of any usable output (at
+	 * this point in time). We don't want to be constantly reloading
+	 * icons as views are moved between outputs.
+	 *
+	 * TODO: currently there's no signal to reload/render icons if
+	 * outputs are reconfigured and the max scale changes.
+	 */
+	float icon_scale = output_max_scale(ssd->view->server);
 
 	struct lab_data_buffer *icon_buffer = icon_loader_lookup(
 		ssd->view->server, app_id, icon_size, icon_scale);
@@ -627,7 +635,7 @@ ssd_update_window_icon(struct ssd *ssd)
 		for (uint8_t state_set = 0; state_set <= LAB_BS_ALL; state_set++) {
 			if (button->nodes[state_set]) {
 				update_window_icon_buffer(button->nodes[state_set],
-					&icon_buffer->base);
+					icon_buffer);
 			}
 		}
 	} FOR_EACH_END

src/theme.c

@@ -246,18 +246,20 @@ load_button(struct theme *theme, struct button *b, int active)
 
 	zdrop(buffer);
 
+	int size = theme->title_height - 2 * theme->padding_height;
+	float scale = 1; /* TODO: account for output scale */
+
 	/* PNG */
 	get_button_filename(filename, sizeof(filename), b->name,
 		active ? "-active.png" : "-inactive.png");
-	img_png_load(filename, buffer);
+	img_png_load(filename, buffer, size, scale);
 
 #if HAVE_RSVG
 	/* SVG */
 	if (!*buffer) {
-		int size = theme->title_height - 2 * theme->padding_height;
 		get_button_filename(filename, sizeof(filename), b->name,
 			active ? "-active.svg" : "-inactive.svg");
-		img_svg_load(filename, buffer, size);
+		img_svg_load(filename, buffer, size, scale);
 	}
 #endif