[PATCH 2/2 v2] xwm: Scale icon if size is not the target size
Scott Moreau
oreaus at gmail.com
Fri Mar 23 19:16:03 UTC 2018
Hi Derek,
On Fri, Mar 23, 2018 at 11:59 AM, Derek Foreman <derekf at osg.samsung.com>
wrote:
> 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.
>
I agree it could use a rename from cairo_resize_surface to something else.
Any suggestions?
>
> > +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?
>
Yes it is possible, but the point of this is to yield a better quality.
Scaling directly to the target size can have undesirable results.
>
> > +
> > + 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...)
>
I see. I will make get_icon_size_from_data() check for 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.
>
I had my editor set to 4 wide tabs, will fix.
>
> >
> > 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...
>
Noted.
>
> > +#undef ICON_SIZE
> >
> > static void
> > weston_wm_handle_property_notify(struct weston_wm *wm,
> xcb_generic_event_t *event)
> >
>
> Thanks,
> Derek
>
Thanks,
Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180323/1309c91a/attachment.html>
More information about the wayland-devel
mailing list