[PATCH weston] Revert "xwm: Add icon support to the frame" and friends
Derek Foreman
derekf at osg.samsung.com
Thu Mar 29 14:53:01 UTC 2018
On 2018-03-29 09:10 AM, Derek Foreman wrote:
> 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.
Quentin has suggested on IRC that it might be better to just land the
xwayland/window-manager.c parts of this revert and keep the rest.
I'd like to see if anyone else has opinions before I resubmit half this
patch for review.
Thanks,
Derek
> 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);
>>
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
More information about the wayland-devel
mailing list