[PATCH v4 1/2] xwm: Choose icon closest to target size

[Derek Foreman]

On 2018-03-23 02:47 PM, Scott Moreau wrote:
> Xwayland clients can offer multiple icon sizes in no particular order.
> Previously xwm was selecting the first one unconditionally. This patch
> selects the icon that matches the size closest to the target size. The
> target size is hard coded to 16 since there is only one theme and the
> data used to create the theme is hard coded.

LGTM,
Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

> ---
> 
> Changed in v2:
> 
> - Fix typo setting width to height
> 
> Changed in v3:
> 
> - Move checks for malformed input into data handling function
> 
> Changed in v4:
> 
> - #define XWM_ICON_SIZE in this patch and do not #undef it
> - Start with first icon found before choosing icon\
> - Check for NULL data in get_icon_size_from_data()
> 
>  xwayland/window-manager.c | 84 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 77 insertions(+), 7 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..5fb41bf 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -127,6 +127,8 @@ struct motif_wm_hints {
>  #define _NET_WM_MOVERESIZE_MOVE_KEYBOARD    10   /* move via keyboard */
>  #define _NET_WM_MOVERESIZE_CANCEL           11   /* cancel operation */
>  
> +#define XWM_ICON_SIZE 16 /* width and height of frame icon */
> +
>  struct weston_output_weak_ref {
>  	struct weston_output *output;
>  	struct wl_listener destroy_listener;
> @@ -1352,6 +1354,77 @@ weston_wm_window_schedule_repaint(struct weston_wm_window *window)
>  				       weston_wm_window_do_repaint, window);
>  }
>  
> +static uint32_t *
> +get_icon_size_from_data(int target_width, int target_height,
> +			uint32_t *width, uint32_t *height,
> +			uint32_t length, uint32_t *data)
> +{
> +	uint32_t *d = data, **img = NULL, *ret = NULL, w, h;
> +	int num_sizes = 0, *sizes = NULL, a1, a2, a3;
> +
> +	if (!data)
> +		return NULL;
> +
> +	*width = *height = 0;
> +
> +	while (d - data < length / 4) {
> +		w = *d++;
> +		h = *d++;
> +
> +		/* Some checks against malformed input. */
> +		if (w == 0 || h == 0 || length < 2 + w * h)
> +			goto out;
> +
> +		sizes = realloc(sizes, 2 * (num_sizes + 1) * sizeof(uint32_t));
> +		if (!sizes) {
> +			free(img);
> +			return NULL;
> +		}
> +
> +		img = realloc(img, (num_sizes + 1) * sizeof(uint32_t *));
> +		if (!img) {
> +			free(sizes);
> +			return NULL;
> +		}
> +
> +		sizes[(num_sizes * 2) + 0] = w;
> +		sizes[(num_sizes * 2) + 1] = h;
> +		img[num_sizes] = d;
> +
> +		num_sizes++;
> +		d += w * h;
> +	}
> +
> +	/* Begin with first icon in list */
> +	if (num_sizes) {
> +		*width = sizes[0];
> +		*height = sizes[1];
> +		ret = img[0];
> +	}
> +
> +	/* Choose closest match by comparing icon dimension areas */
> +	a1 = target_width * target_height;
> +
> +	while (num_sizes--) {
> +		w = sizes[(num_sizes * 2) + 0];
> +		h = sizes[(num_sizes * 2) + 1];
> +
> +		a2 = w * h;
> +		a3 = *width * *height;
> +
> +		if (abs(a2 - a1) < abs(a3 - a1)) {
> +			*width = w;
> +			*height = h;
> +			ret = img[num_sizes];
> +		}
> +	}
> +out:
> +	free(sizes);
> +	free(img);
> +
> +	return ret;
> +}
> +
>  static void
>  weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  {
> @@ -1361,9 +1434,6 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  	uint32_t *data, width, height;
>  	cairo_surface_t *new_surface;
>  
> -	/* TODO: icons don’t have any specified order, we should pick the
> -	 * closest one to the target dimension instead of the first one. */
> -
>  	cookie = xcb_get_property(wm->conn, 0, window->id,
>  	                          wm->atom.net_wm_icon, XCB_ATOM_ANY, 0,
>  	                          UINT32_MAX);
> @@ -1375,11 +1445,11 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  		return;
>  
>  	data = xcb_get_property_value(reply);
> -	width = *data++;
> -	height = *data++;
>  
> -	/* Some checks against malformed input. */
> -	if (width == 0 || height == 0 || length < 2 + width * height)
> +	data = get_icon_size_from_data(XWM_ICON_SIZE, XWM_ICON_SIZE,
> +					&width, &height, length, data);
> +
> +	if (!data)
>  		return;
>  
>  	new_surface =
>