[PATCH weston 1/2] desktop-shell: Do not use animation code when no outputs are present

Pekka Paalanen ppaalanen at gmail.com
Thu Aug 11 09:19:57 UTC 2016


On Fri,  5 Aug 2016 15:32:05 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> It leads to crash when trying to select an output that drives
> the animation.
> 
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  desktop-shell/exposay.c     | 10 +++++++++
>  desktop-shell/input-panel.c |  9 ++++----
>  desktop-shell/shell.c       | 55 ++++++++++++++++++++++++++-------------------
>  3 files changed, 47 insertions(+), 27 deletions(-)
> 
> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
> index b11a7f7..1841554 100644
> --- a/desktop-shell/exposay.c
> +++ b/desktop-shell/exposay.c
> @@ -113,6 +113,11 @@ exposay_animate_in_done(struct weston_view_animation *animation, void *data)
>  static void
>  exposay_animate_in(struct exposay_surface *esurface)
>  {
> +	struct weston_compositor *ec = esurface->shell->compositor;
> +
> +	if (wl_list_empty(&ec->output_list))
> +		return;
> +
>  	exposay_in_flight_inc(esurface->shell);
>  
>  	weston_move_scale_run(esurface->view,
> @@ -136,6 +141,11 @@ exposay_animate_out_done(struct weston_view_animation *animation, void *data)
>  static void
>  exposay_animate_out(struct exposay_surface *esurface)
>  {
> +	struct weston_compositor *ec = esurface->shell->compositor;
> +
> +	if (wl_list_empty(&ec->output_list))
> +		return;
> +
>  	exposay_in_flight_inc(esurface->shell);
>  
>  	/* Remove the static transformation set up by
> diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
> index 2e18e28..a3ab05f 100644
> --- a/desktop-shell/input-panel.c
> +++ b/desktop-shell/input-panel.c
> @@ -93,10 +93,11 @@ show_input_panel_surface(struct input_panel_surface *ipsurf)
>  	if (ipsurf->anim)
>  		weston_view_animation_destroy(ipsurf->anim);
>  
> -	ipsurf->anim =
> -		weston_slide_run(ipsurf->view,
> -				 ipsurf->surface->height * 0.9, 0,
> -				 input_panel_slide_done, ipsurf);
> +	if (!wl_list_empty(&shell->compositor->output_list))
> +		ipsurf->anim =
> +			weston_slide_run(ipsurf->view,
> +					 ipsurf->surface->height * 0.9, 0,
> +					 input_panel_slide_done, ipsurf);
>  }
>  
>  static void
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 64979cd..8e975f0 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -850,10 +850,11 @@ focus_state_surface_destroy(struct wl_listener *listener, void *data)
>  			if (state->ws->focus_animation)
>  				weston_view_animation_destroy(state->ws->focus_animation);
>  
> -			state->ws->focus_animation = weston_fade_run(
> -				state->ws->fsurf_front->view,
> -				state->ws->fsurf_front->view->alpha, 0.0, 300,
> -				focus_animation_done, state->ws);
> +			if (!wl_list_empty(&shell->compositor->output_list))
> +				state->ws->focus_animation = weston_fade_run(
> +					state->ws->fsurf_front->view,
> +					state->ws->fsurf_front->view->alpha, 0.0, 300,
> +					focus_animation_done, state->ws);
>  		}
>  
>  		wl_list_remove(&state->link);
> @@ -1030,24 +1031,29 @@ animate_focus_change(struct desktop_shell *shell, struct workspace *ws,
>  					  &ws->fsurf_front->view->layer_link);
>  
>  	if (focus_surface_created) {
> -		ws->focus_animation = weston_fade_run(
> -			ws->fsurf_front->view,
> -			ws->fsurf_front->view->alpha, 0.4, 300,
> -			focus_animation_done, ws);
> +		if (!wl_list_empty(&shell->compositor->output_list))
> +			ws->focus_animation = weston_fade_run(
> +				ws->fsurf_front->view,
> +				ws->fsurf_front->view->alpha, 0.4, 300,
> +				focus_animation_done, ws);
>  	} else if (from) {
>  		weston_layer_entry_insert(&from->layer_link,
>  					  &ws->fsurf_back->view->layer_link);
> -		ws->focus_animation = weston_stable_fade_run(
> -			ws->fsurf_front->view, 0.0,
> -			ws->fsurf_back->view, 0.4,
> -			focus_animation_done, ws);
> +
> +		if (!wl_list_empty(&shell->compositor->output_list))
> +			ws->focus_animation = weston_stable_fade_run(
> +				ws->fsurf_front->view, 0.0,
> +				ws->fsurf_back->view, 0.4,
> +				focus_animation_done, ws);
>  	} else if (to) {
>  		weston_layer_entry_insert(&ws->layer.view_list,
>  					  &ws->fsurf_back->view->layer_link);
> -		ws->focus_animation = weston_stable_fade_run(
> -			ws->fsurf_front->view, 0.0,
> -			ws->fsurf_back->view, 0.4,
> -			focus_animation_done, ws);
> +
> +		if (!wl_list_empty(&shell->compositor->output_list))
> +			ws->focus_animation = weston_stable_fade_run(
> +				ws->fsurf_front->view, 0.0,
> +				ws->fsurf_back->view, 0.4,
> +				focus_animation_done, ws);
>  	}
>  }
>  
> @@ -3670,8 +3676,9 @@ handle_resource_destroy(struct wl_listener *listener, void *data)
>  	pixman_region32_fini(&shsurf->surface->input);
>  	pixman_region32_init(&shsurf->surface->input);
>  	if (shsurf->shell->win_close_animation_type == ANIMATION_FADE) {
> -		weston_fade_run(shsurf->view, 1.0, 0.0, 300.0,
> -				fade_out_done, shsurf);
> +		if (!wl_list_empty(&shsurf->surface->compositor->output_list))
> +			weston_fade_run(shsurf->view, 1.0, 0.0, 300.0,
> +					fade_out_done, shsurf);
>  	} else {
>  		weston_surface_destroy(shsurf->surface);
>  	}
> @@ -5440,10 +5447,11 @@ shell_fade(struct desktop_shell *shell, enum fade_type type)
>  	} else if (shell->fade.animation) {
>  		weston_fade_update(shell->fade.animation, tint);
>  	} else {
> -		shell->fade.animation =
> -			weston_fade_run(shell->fade.view,
> -					1.0 - tint, tint, 300.0,
> -					shell_fade_done, shell);
> +		if (!wl_list_empty(&shell->compositor->output_list))
> +			shell->fade.animation =
> +				weston_fade_run(shell->fade.view,
> +						1.0 - tint, tint, 300.0,
> +						shell_fade_done, shell);
>  	}
>  }
>  
> @@ -5733,7 +5741,8 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  	}
>  
>  	if (shsurf->type == SHELL_SURFACE_TOPLEVEL &&
> -	    !shsurf->state.maximized && !shsurf->state.fullscreen)
> +	    !shsurf->state.maximized && !shsurf->state.fullscreen &&
> +	    !wl_list_empty(&shsurf->surface->compositor->output_list))
>  	{
>  		switch (shell->win_animation_type) {
>  		case ANIMATION_FADE:

Hi Armin,

I had a chat with Quentin on this, and the problem with this patch is
that the "done" callback gets never called. E.g. shell_fade_done() is
quite important to ensure it gets called.

This would be better fixed in animation.c for all users. How about
something like this:

In weston_view_animation_create(), if view->output is NULL, schedule an
idle task with wl_event_loop_add_idle() instead that will then call
weston_view_animation_destroy().

That should guarantee that the reset and done callbacks get called
properly for all animation types and uses. I don't think you could do
the equivalent directly from weston_view_animation_create() because the
callers expect a non-NULL return value and might not expect the done
callback to be called that early.

The same applies for patch 2.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160811/a5ae7af7/attachment-0001.sig>


More information about the wayland-devel mailing list