[PATCH v2 weston] Partially revert "xwm: Add icon support to the frame" and friends

[Quentin Glidic]

On 3/30/18 6:56 PM, Derek Foreman wrote:
> This (partially) reverts commit bef761796c2ada6344d227142af4a0f40b9760dd.
> This (partially) reverts commit 4d1cd36c9ea041688f92cd8981e43b5fe3b52409.
> This (partially) reverts commit 44fc1be913ab2faad0414f50e51d58310302d065.
> This (partially) 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.  Currently the known
> leaks have been squashed, but it still picks wrong sized icons and
> does no scaling, which can lead to very strange rendering.  At window
> close time the wrong sized icon appears above the window during fade out.
> 
> This patch reverts the mostly solid bits and keeps the unfinished
> bits behind in favor of a simpler revert than removing the whole
> thing.
> 
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com> > ---
> 
> Changes from v1:
> 
> Only reverts the changes in window-manager.c (and updates frame_create call
> to pass NULL for icon surface.

Yeah, looks safer to me:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>

I’m sure Scott will submit a complete feature soon, this way we can 
focus the review on the real bits. :-)

Thanks,


> 
>   xwayland/window-manager.c | 88 ++---------------------------------------------
>   1 file changed, 2 insertions(+), 86 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 62087941..06370b70 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, NULL);
>   	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);
> 


-- 

Quentin “Sardem FF7” Glidic