On 2018-03-22 01:04 AM, Scott Moreau wrote:
> This scales the icon cairo surface for the titlebar if it isn't the
> target size.
> 
> shared/cairo-util: Add surface resizing function to be used for this
> case and other potential cases.
> ---
> 
> Changed in v2:
> 
> - Rebase to [PATCH 1/1 v3] xwm: Choose icon closest to target size
> 
>  shared/cairo-util.c       | 61 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  shared/cairo-util.h       |  4 ++++
>  xwayland/window-manager.c |  9 ++++++-
>  3 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/shared/cairo-util.c b/shared/cairo-util.c
> index d71e0ed..b91b5bc 100644
> --- a/shared/cairo-util.c
> +++ b/shared/cairo-util.c
> @@ -142,6 +142,67 @@ blur_surface(cairo_surface_t *surface, int margin)
>       return 0;
>  }
>  
> +static cairo_surface_t *
> +scale_surface(cairo_surface_t *source, cairo_filter_t filter,
> +                                                     double width, double 
> height)
> +{
> +     cairo_surface_t *dest;
> +     cairo_t *cr;
> +     int old_width, old_height;
> +
> +     old_width = cairo_image_surface_get_width(source);
> +     old_height = cairo_image_surface_get_height(source);
> +
> +     dest = cairo_surface_create_similar(source, CAIRO_CONTENT_COLOR_ALPHA,
> +                                                             width, height);
> +     cr = cairo_create (dest);
> +
> +     cairo_scale (cr, width / old_width, height / old_height);
> +     cairo_set_source_surface (cr, source, 0, 0);
> +
> +     cairo_pattern_set_extend (cairo_get_source(cr), CAIRO_EXTEND_REFLECT);
> +
> +     cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
> +
> +     cairo_paint (cr);
> +
> +     cairo_destroy (cr);
> +
> +     cairo_surface_destroy(source);

This function looks like it solves the problem without leaking anything
- would love to see more review from others with cairo experience.

> +
> +     return dest;
> +}
> +
> +cairo_surface_t *

I would prefer this function be named something else, so a casual reader
with no knowledge of cairo API (like me!) doesn't think it looks like a
cairo function name.

> +cairo_resize_surface(cairo_surface_t *source, cairo_filter_t filter,
> +                                                     int width, int height)
> +{
> +     if (!filter)
> +             filter = CAIRO_FILTER_BEST;
> +
> +     while((cairo_image_surface_get_width(source) / 2.0f) > width)
> +             source = scale_surface(source, filter,
> +                             cairo_image_surface_get_width(source) / 2.0f,
> +                             cairo_image_surface_get_height(source));
> +
> +     while((cairo_image_surface_get_height(source) / 2.0f) > height)
> +             source = scale_surface(source, filter,
> +                             cairo_image_surface_get_width(source),
> +                             cairo_image_surface_get_height(source) / 2.0f);
> +
> +     while((cairo_image_surface_get_width(source) * 2.0f) < width)
> +             source = scale_surface(source, filter,
> +                             cairo_image_surface_get_width(source) * 2.0f,
> +                             cairo_image_surface_get_height(source));
> +
> +     while((cairo_image_surface_get_height(source) * 2.0f) < height)
> +             source = scale_surface(source, filter,
> +                             cairo_image_surface_get_width(source),
> +                             cairo_image_surface_get_height(source) * 2.0f);

This chain of operations feels complicated - is it not possible to
calculate the optimal target size and just call scale_surface once?

> +
> +     return scale_surface(source, filter, width, height);
> +}
> +
>  void
>  render_shadow(cairo_t *cr, cairo_surface_t *surface,
>             int x, int y, int width, int height, int margin, int top_margin)
> diff --git a/shared/cairo-util.h b/shared/cairo-util.h
> index 6fd11f6..3bd2b0c 100644
> --- a/shared/cairo-util.h
> +++ b/shared/cairo-util.h
> @@ -49,6 +49,10 @@ rounded_rect(cairo_t *cr, int x0, int y0, int x1, int y1, 
> int radius);
>  cairo_surface_t *
>  load_cairo_surface(const char *filename);
>  
> +cairo_surface_t *
> +cairo_resize_surface(cairo_surface_t *source, cairo_filter_t filter,
> +                                                     int width, int height);
> +
>  struct theme {
>       cairo_surface_t *active_frame;
>       cairo_surface_t *inactive_frame;
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 30c4b1c..6057490 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -1416,6 +1416,7 @@ get_icon_size_from_data(int target_width, int 
> target_height,
>       return ret;
>  }
>  
> +#define ICON_SIZE 16
>  static void
>  weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
>  {
> @@ -1437,7 +1438,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
> weston_wm_window *window)
>  
>       data = xcb_get_property_value(reply);
>

Just occurred to me...for the last patch I guess.  I think something
needs to check data for NULLness here (or get_icon_size_from_data be
made safe against a NULL data...)

> -     data = get_icon_size_from_data(16, 16, &width, &height, length, data);
> +     data = get_icon_size_from_data(ICON_SIZE, ICON_SIZE,
> +                                                                     &width, 
> &height, length, data);

Too many tabs (we use 8 wide tabs...)
There are a couple of places like that.

>  
>       if (!data)
>               return;
> @@ -1453,11 +1455,16 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
> weston_wm_window *window)
>               return;
>       }
>  
> +     if (width != ICON_SIZE || height != ICON_SIZE)
> +             new_surface =
> +                     cairo_resize_surface(new_surface, 0, ICON_SIZE, 
> ICON_SIZE);
> +
>       if (window->frame)
>               frame_set_icon(window->frame, new_surface);
>       else /* We don’t have a frame yet */
>               window->icon_surface = new_surface;
>  }

As mentioned on other patch, I think the define for this should be in
the other patch and not get #undef...

> +#undef ICON_SIZE
>  
>  static void
>  weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t 
> *event)
> 

Thanks,
Derek
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to