[PATCH weston] Revert "xwm: Add icon support to the frame" and friends
Derek Foreman
derekf at osg.samsung.com
Thu Mar 29 14:10:27 UTC 2018
On 2018-03-29 08:59 AM, Derek Foreman wrote:
> This reverts commit bef761796c2ada6344d227142af4a0f40b9760dd.
> This (partially) reverts commit 4d1cd36c9ea041688f92cd8981e43b5fe3b52409.
> - the frame_destroy in weston_wm_window_destroy() remains
> This reverts commit 44fc1be913ab2faad0414f50e51d58310302d065.
> This (partially) reverts commit e277276b850bad39ed6995be5a82f24aa6b17bf1.
> - only the custom icon bits are removed
> This reverts commit 6b58ea8c43ac81e519bd418efbf24687a5d731b8.
>
> The new xwm icon code has proven to be leaky and incomplete, and while
> we have patches under consideration to fix the rest of its known problems
> they still require changes and review cycles. Reverting this all for now
> for the upcoming release.
>
Perhaps I should've been more clear as to what "incomplete" means in
this commit log.
The current code will pick the first available icon unconditionally,
regardless as to whether this fits on the titlebar, and no scaling is done.
So, as an example, here running terminology under xwayland will result
in picking a 128x128 icon, and drawing it as a 16 high 128 wide piece of
the icon on the title bar. when the window closes for some reason the
whole icon appears during fade out.
It all looks pretty embarrassing.
That said, all the known leaks have been fixed, it's just visually
disappointing.
Thanks,
Derek
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
> clients/window.c | 4 +-
> libweston/compositor-wayland.c | 2 +-
> shared/cairo-util.h | 6 +--
> shared/frame.c | 76 +++++++++---------------------------
> xwayland/window-manager.c | 88 +-----------------------------------------
> 5 files changed, 24 insertions(+), 152 deletions(-)
>
> diff --git a/clients/window.c b/clients/window.c
> index bcf2b017..82fbd572 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -2523,7 +2523,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, NULL);
> + buttons, window->title);
>
> frame->widget = window_add_widget(window, frame);
> frame->child = widget_add_widget(frame->widget, data);
> @@ -5398,7 +5398,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, NULL);
> + FRAME_BUTTON_NONE, 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 111c4c09..5629b7f4 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -869,7 +869,7 @@ wayland_output_set_windowed(struct wayland_output *output)
> return -1;
> }
> output->frame = frame_create(b->theme, 100, 100,
> - FRAME_BUTTON_CLOSE, output->title, NULL);
> + FRAME_BUTTON_CLOSE, output->title);
> if (!output->frame)
> return -1;
>
> diff --git a/shared/cairo-util.h b/shared/cairo-util.h
> index 6fd11f6b..9481e58c 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, cairo_surface_t *icon);
> + const char *title);
>
> void
> frame_destroy(struct frame *frame);
> @@ -135,10 +135,6 @@ frame_destroy(struct frame *frame);
> int
> frame_set_title(struct frame *frame, const char *title);
>
> -/* May set FRAME_STATUS_REPAINT */
> -void
> -frame_set_icon(struct frame *frame, cairo_surface_t *icon);
> -
> /* May set FRAME_STATUS_REPAINT */
> void
> frame_set_flag(struct frame *frame, enum frame_flag flag);
> diff --git a/shared/frame.c b/shared/frame.c
> index e8a5cad6..83bd7922 100644
> --- a/shared/frame.c
> +++ b/shared/frame.c
> @@ -109,9 +109,9 @@ struct frame {
> };
>
> 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)
> +frame_button_create(struct frame *frame, const char *icon,
> + enum frame_status status_effect,
> + enum frame_button_flags flags)
> {
> struct frame_button *button;
>
> @@ -119,7 +119,12 @@ frame_button_create_from_surface(struct frame *frame, cairo_surface_t *icon,
> if (!button)
> return NULL;
>
> - button->icon = icon;
> + button->icon = cairo_image_surface_create_from_png(icon);
> + if (!button->icon) {
> + free(button);
> + return NULL;
> + }
> +
> button->frame = frame;
> button->flags = flags;
> button->status_effect = status_effect;
> @@ -129,30 +134,6 @@ frame_button_create_from_surface(struct frame *frame, cairo_surface_t *icon,
> return button;
> }
>
> -static struct frame_button *
> -frame_button_create(struct frame *frame, const char *icon_name,
> - enum frame_status status_effect,
> - enum frame_button_flags flags)
> -{
> - struct frame_button *button;
> - cairo_surface_t *icon;
> -
> - icon = cairo_image_surface_create_from_png(icon_name);
> - if (cairo_surface_status(icon) != CAIRO_STATUS_SUCCESS)
> - goto error;
> -
> - button = frame_button_create_from_surface(frame, icon, status_effect,
> - flags);
> - if (!button)
> - goto error;
> -
> - return button;
> -
> -error:
> - cairo_surface_destroy(icon);
> - return NULL;
> -}
> -
> static void
> frame_button_destroy(struct frame_button *button)
> {
> @@ -325,7 +306,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, cairo_surface_t *icon)
> + const char *title)
> {
> struct frame *frame;
> struct frame_button *button;
> @@ -352,23 +333,16 @@ frame_create(struct theme *t, int32_t width, int32_t height, uint32_t buttons,
> }
>
> if (title) {
> - if (icon) {
> - button = frame_button_create_from_surface(frame,
> - icon,
> - FRAME_STATUS_MENU,
> - FRAME_BUTTON_CLICK_DOWN);
> - } else {
> - char *name = file_name_with_datadir("icon_window.png");
> + char *name = file_name_with_datadir("icon_window.png");
> + if (!name)
> + goto free_frame;
>
> - if (!name)
> - goto free_frame;
> + button = frame_button_create(frame,
> + name,
> + FRAME_STATUS_MENU,
> + FRAME_BUTTON_CLICK_DOWN);
> + free(name);
>
> - button = frame_button_create(frame,
> - name,
> - FRAME_STATUS_MENU,
> - FRAME_BUTTON_CLICK_DOWN);
> - free(name);
> - }
> if (!button)
> goto free_frame;
> }
> @@ -448,20 +422,6 @@ frame_set_title(struct frame *frame, const char *title)
> return 0;
> }
>
> -void
> -frame_set_icon(struct frame *frame, cairo_surface_t *icon)
> -{
> - struct frame_button *button;
> - wl_list_for_each(button, &frame->buttons, link) {
> - if (button->status_effect != FRAME_STATUS_MENU)
> - continue;
> - if (button->icon)
> - cairo_surface_destroy(button->icon);
> - button->icon = icon;
> - frame->status |= FRAME_STATUS_REPAINT;
> - }
> -}
> -
> void
> frame_set_flag(struct frame *frame, enum frame_flag flag)
> {
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 62087941..6f5c3443 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -138,8 +138,6 @@ struct weston_wm_window {
> xcb_window_t frame_id;
> struct frame *frame;
> cairo_surface_t *cairo_surface;
> - int icon;
> - cairo_surface_t *icon_surface; /* A temporary slot, to be passed to frame on creation */
> uint32_t surface_id;
> struct weston_surface *surface;
> struct weston_desktop_xwayland_surface *shsurf;
> @@ -475,7 +473,6 @@ weston_wm_window_read_properties(struct weston_wm_window *window)
> { wm->atom.net_wm_state, TYPE_NET_WM_STATE, NULL },
> { 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, NULL },
> { wm->atom.wm_client_machine, XCB_ATOM_WM_CLIENT_MACHINE, F(machine) },
> @@ -976,10 +973,8 @@ 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->icon_surface);
> - window->icon_surface = NULL;
> + window->width, window->height,
> + buttons, window->name);
> frame_resize_inside(window->frame, window->width, window->height);
>
> weston_wm_window_get_frame_size(window, &width, &height);
> @@ -1318,70 +1313,6 @@ weston_wm_window_schedule_repaint(struct weston_wm_window *window)
> weston_wm_window_do_repaint, window);
> }
>
> -static void
> -handle_icon_surface_destroy(void *data)
> -{
> - free(data);
> -}
> -
> -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;
> - uint32_t length;
> - 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);
> - 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) {
> - free(reply);
> - 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) {
> - free(reply);
> - return;
> - }
> -
> - new_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(new_surface) != CAIRO_STATUS_SUCCESS) {
> - cairo_surface_destroy(new_surface);
> - free(reply);
> - return;
> - }
> -
> - if (window->icon_surface)
> - cairo_surface_destroy(window->icon_surface);
> -
> - cairo_surface_set_user_data(new_surface, NULL, reply,
> - &handle_icon_surface_destroy);
> -
> - if (window->frame)
> - frame_set_icon(window->frame, new_surface);
> - else /* We don’t have a frame yet */
> - window->icon_surface = new_surface;
> -}
> -
> static void
> weston_wm_handle_property_notify(struct weston_wm *wm, xcb_generic_event_t *event)
> {
> @@ -1402,19 +1333,6 @@ 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) {
> - if (property_notify->state != XCB_PROPERTY_DELETE) {
> - weston_wm_handle_icon(wm, window);
> - } else {
> - if (window->frame)
> - frame_set_icon(window->frame, NULL);
> - if (window->icon_surface)
> - cairo_surface_destroy(window->icon_surface);
> - window->icon_surface = NULL;
> - }
> - weston_wm_window_schedule_repaint(window);
> - }
> -
> if (property_notify->atom == wm->atom.net_wm_name ||
> property_notify->atom == XCB_ATOM_WM_NAME)
> weston_wm_window_schedule_repaint(window);
> @@ -1475,8 +1393,6 @@ weston_wm_window_destroy(struct weston_wm_window *window)
> wl_event_source_remove(window->repaint_source);
> if (window->cairo_surface)
> cairo_surface_destroy(window->cairo_surface);
> - if (window->icon_surface)
> - cairo_surface_destroy(window->icon_surface);
>
> if (window->frame_id) {
> xcb_reparent_window(wm->conn, window->id, wm->wm_window, 0, 0);
>
More information about the wayland-devel
mailing list