[PATCH weston] xwm: Add icon support to the frame
Eric Engestrom
eric.engestrom at imgtec.com
Wed Dec 7 18:28:39 UTC 2016
On Sunday, 2016-12-04 21:20:47 +0000, Emmanuel Gil Peyrot wrote:
> This fetches the _NET_WM_ICON property of the X11 window, and use the
> first image found as the frame icon.
>
> This has been tested with various X11 programs, and improves usability
> and user-friendliness a bit.
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve at linkmauve.fr>
> ---
> clients/window.c | 4 +--
> libweston/compositor-wayland.c | 2 +-
> shared/cairo-util.h | 2 +-
> shared/frame.c | 38 ++++++++++++++++++++++++----
> xwayland/window-manager.c | 56 +++++++++++++++++++++++++++++++++++++++---
> 5 files changed, 90 insertions(+), 12 deletions(-)
>
> diff --git a/clients/window.c b/clients/window.c
> index ac35c3d..7e15969 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -2546,7 +2546,7 @@ window_frame_create(struct window *window, void *data)
>
> frame = xzalloc(sizeof *frame);
> frame->frame = frame_create(window->display->theme, 0, 0,
> - buttons, window->title);
> + buttons, window->title, NULL);
>
> frame->widget = window_add_widget(window, frame);
> frame->child = widget_add_widget(frame->widget, data);
> @@ -5444,7 +5444,7 @@ create_menu(struct display *display,
> menu->user_data = user_data;
> menu->widget = window_add_widget(menu->window, menu);
> menu->frame = frame_create(window->display->theme, 0, 0,
> - FRAME_BUTTON_NONE, NULL);
> + FRAME_BUTTON_NONE, NULL, NULL);
> fail_on_null(menu->frame, 0, __FILE__, __LINE__);
> menu->entries = entries;
> menu->count = count;
> diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
> index d1e387d..4b053bf 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -848,7 +848,7 @@ wayland_output_set_windowed(struct wayland_output *output)
> }
> }
> output->frame = frame_create(b->theme, 100, 100,
> - FRAME_BUTTON_CLOSE, title);
> + FRAME_BUTTON_CLOSE, title, NULL);
> free(title);
> if (!output->frame)
> return -1;
> diff --git a/shared/cairo-util.h b/shared/cairo-util.h
> index 84cf005..7893ca2 100644
> --- a/shared/cairo-util.h
> +++ b/shared/cairo-util.h
> @@ -126,7 +126,7 @@ enum {
>
> struct frame *
> frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
> - const char *title);
> + const char *title, cairo_surface_t *icon);
>
> void
> frame_destroy(struct frame *frame);
> diff --git a/shared/frame.c b/shared/frame.c
> index eb0cd77..69f3e90 100644
> --- a/shared/frame.c
> +++ b/shared/frame.c
> @@ -131,6 +131,27 @@ frame_button_create(struct frame *frame, const char *icon,
> return button;
> }
>
> +static struct frame_button *
> +frame_button_create_from_surface(struct frame *frame, cairo_surface_t *icon,
> + enum frame_status status_effect,
> + enum frame_button_flags flags)
> +{
> + struct frame_button *button;
+if (!icon)
+ return NULL;
That way you could rewrite frame_button_create() as a simple:
return frame_button_create_from_surface(frame,
cairo_image_surface_create_from_png(icon),
status_effect, flags);
to avoid duplicating this code.
(While at it, I'd argue `frame` should also be null-checked, but
that's clearly unrelated to this patch.)
> +
> + button = calloc(1, sizeof *button);
> + if (!button)
> + return NULL;
> +
> + button->icon = icon;
> + button->frame = frame;
> + button->flags = flags;
> + button->status_effect = status_effect;
> +
> + wl_list_insert(frame->buttons.prev, &button->link);
> +
> + return button;
> +}
> +
> static void
> frame_button_destroy(struct frame_button *button)
> {
> @@ -303,7 +324,7 @@ frame_destroy(struct frame *frame)
>
> struct frame *
> frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
> - const char *title)
> + const char *title, cairo_surface_t *icon)
> {
> struct frame *frame;
> struct frame_button *button;
> @@ -330,10 +351,17 @@ frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
> }
>
> if (title) {
> - button = frame_button_create(frame,
> - DATADIR "/weston/icon_window.png",
> - FRAME_STATUS_MENU,
> - FRAME_BUTTON_CLICK_DOWN);
> + if (icon) {
> + button = frame_button_create_from_surface(frame,
> + icon,
> + FRAME_STATUS_MENU,
> + FRAME_BUTTON_CLICK_DOWN);
> + } else {
> + button = frame_button_create(frame,
> + DATADIR "/weston/icon_window.png",
> + FRAME_STATUS_MENU,
> + FRAME_BUTTON_CLICK_DOWN);
> + }
> if (!button)
> goto free_frame;
> }
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 56d65af..65fb62b 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -131,6 +131,8 @@ struct weston_wm_window {
> xcb_window_t frame_id;
> struct frame *frame;
> cairo_surface_t *cairo_surface;
> + int icon;
> + cairo_surface_t *icon_surface;
> uint32_t surface_id;
> struct weston_surface *surface;
> struct weston_desktop_xwayland_surface *shsurf;
> @@ -413,6 +415,7 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
> { wm->atom.net_wm_state, TYPE_NET_WM_STATE },
> { wm->atom.net_wm_window_type, XCB_ATOM_ATOM, F(type) },
> { wm->atom.net_wm_name, XCB_ATOM_STRING, F(name) },
> + { wm->atom.net_wm_icon, XCB_ATOM_CARDINAL, F(icon) },
> { wm->atom.net_wm_pid, XCB_ATOM_CARDINAL, F(pid) },
> { wm->atom.motif_wm_hints, TYPE_MOTIF_WM_HINTS, 0 },
> { wm->atom.wm_client_machine, XCB_ATOM_WM_CLIENT_MACHINE, F(machine) },
> @@ -872,8 +875,9 @@ weston_wm_window_create_frame(struct weston_wm_window *window)
> buttons |= FRAME_BUTTON_MAXIMIZE;
>
> window->frame = frame_create(window->wm->theme,
> - window->width, window->height,
> - buttons, window->name);
> + window->width, window->height,
> + buttons, window->name,
> + window->icon_surface);
> frame_resize_inside(window->frame, window->width, window->height);
>
> weston_wm_window_get_frame_size(window, &width, &height);
> @@ -1146,6 +1150,48 @@ weston_wm_window_schedule_repaint(struct weston_wm_window *window)
> }
>
> static void
> +weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
> +{
> + xcb_get_property_reply_t *reply;
> + xcb_get_property_cookie_t cookie;
> + int length;
> + unsigned *data, width, height;
> +
> + /* 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, 2048);
> + reply = xcb_get_property_reply(wm->conn, cookie, NULL);
> + length = xcb_get_property_value_length(reply);
> +
> + /* This is in 32-bit words, not in bytes. */
> + if (length < 2)
> + return;
> +
> + data = xcb_get_property_value(reply);
> + width = *data++;
> + height = *data++;
> +
> + if (length < 2 + width * height)
> + return;
> +
> + if (window->icon_surface)
> + cairo_surface_destroy(window->icon_surface);
> +
> + window->icon_surface =
> + cairo_image_surface_create_for_data((unsigned char *)data,
> + CAIRO_FORMAT_ARGB32,
> + width, height, width * 4);
> +
> + /* Bail out in case anything wrong happened during surface creation. */
> + if (cairo_surface_status(window->icon_surface) != CAIRO_STATUS_SUCCESS) {
> + cairo_surface_destroy(window->icon_surface);
> + window->icon_surface = NULL;
> + }
> +}
> +
> +static void
> weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *event)
> {
> xcb_property_notify_event_t *property_notify =
> @@ -1164,8 +1210,12 @@ weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *even
> read_and_dump_property(wm, property_notify->window,
> property_notify->atom);
>
> + if (property_notify->atom == wm->atom.net_wm_icon)
> + weston_wm_handle_icon(wm, window);
> +
> if (property_notify->atom == wm->atom.net_wm_name ||
> - property_notify->atom == XCB_ATOM_WM_NAME)
> + property_notify->atom == XCB_ATOM_WM_NAME ||
> + property_notify->atom == wm->atom.net_wm_name)
This looks unrelated, and doesn't look right anyway. Am I reading this
wrong or is this essentially `(a || b || a)` ?
The rest of the patch looks good to me, but I don't know enough to give
an actual r-b.
> weston_wm_window_schedule_repaint(window);
> }
>
> --
> 2.10.2
>
More information about the wayland-devel
mailing list