[PATCH weston v6 1/6] desktop-shell: Enable per-output fade animations

Quentin Glidic sardemff7+wayland at sardemff7.net
Fri Sep 9 09:45:18 UTC 2016


On 09/09/2016 04:31, Bryce Harrington wrote:
> Instead of creating a single global fade surface across all outputs,
> create a separate surface for each output.  This will permit
> e.g. individual fades for each output (or blocking the fade-outs if
> inhibiting idling as will come in a later patch.)
>
> This also fixes a potential issue if on multihead layout spanning a
> desktop wider than 8096 (or higher than 8096), the fade animation may
> not completely cover all surfaces.
>
> This assumes the output geometry doesn't change to become larger during
> the course of the fade animation.
>
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>

v6 should be the last for this one. A few non-blocking comments inline, 
but with these “fixed”:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>

> ---
>  desktop-shell/shell.c | 138 ++++++++++++++++++++++++++++----------------------
>  desktop-shell/shell.h |  14 ++---
>  2 files changed, 84 insertions(+), 68 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index a43c2e2..bd84b83 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -196,6 +196,9 @@ surface_rotate(struct shell_surface *surface, struct weston_pointer *pointer);
>  static void
>  shell_fade_startup(struct desktop_shell *shell);
>
> +static void
> +shell_fade(struct desktop_shell *shell, enum fade_type type);
> +
>  static struct shell_seat *
>  get_shell_seat(struct weston_seat *seat);
>
> @@ -2811,9 +2814,6 @@ static const struct weston_desktop_api shell_desktop_api = {
>   * end of libweston-desktop *
>   * ************************ */
>  static void
> -shell_fade(struct desktop_shell *shell, enum fade_type type);
> -
> -static void
>  configure_static_view(struct weston_view *ev, struct weston_layer *layer)
>  {
>  	struct weston_view *v, *next;
> @@ -3796,16 +3796,16 @@ unlock(struct desktop_shell *shell)
>  }
>
>  static void
> -shell_fade_done(struct weston_view_animation *animation, void *data)
> +shell_fade_done_for_output(struct weston_view_animation *animation, void *data)
>  {
> -	struct desktop_shell *shell = data;
> +	struct shell_output *shell_output = data;
> +	struct desktop_shell *shell = shell_output->shell;
>
> -	shell->fade.animation = NULL;
> -
> -	switch (shell->fade.type) {
> +	shell_output->fade.animation = NULL;
> +	switch (shell_output->fade.type) {
>  	case FADE_IN:
> -		weston_surface_destroy(shell->fade.view->surface);
> -		shell->fade.view = NULL;
> +		weston_surface_destroy(shell_output->fade.view->surface);
> +		shell_output->fade.view = NULL;
>  		break;
>  	case FADE_OUT:
>  		lock(shell);
> @@ -3816,7 +3816,7 @@ shell_fade_done(struct weston_view_animation *animation, void *data)
>  }
>
>  static struct weston_view *
> -shell_fade_create_surface(struct desktop_shell *shell)
> +shell_fade_create_surface_for_output(struct desktop_shell *shell, struct shell_output *shell_output)
>  {
>  	struct weston_compositor *compositor = shell->compositor;
>  	struct weston_surface *surface;
> @@ -3832,8 +3832,8 @@ shell_fade_create_surface(struct desktop_shell *shell)
>  		return NULL;
>  	}
>
> -	weston_surface_set_size(surface, 8192, 8192);
> -	weston_view_set_position(view, 0, 0);
> +	weston_surface_set_size(surface, shell_output->output->width, shell_output->output->height);
> +	weston_view_set_position(view, shell_output->output->x, shell_output->output->y);
>  	weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1.0);
>  	weston_layer_entry_insert(&compositor->fade_layer.view_list,
>  				  &view->layer_link);
> @@ -3848,6 +3848,7 @@ static void
>  shell_fade(struct desktop_shell *shell, enum fade_type type)
>  {
>  	float tint;
> +	struct shell_output *shell_output;
>
>  	switch (type) {
>  	case FADE_IN:
> @@ -3857,36 +3858,38 @@ shell_fade(struct desktop_shell *shell, enum fade_type type)
>  		tint = 1.0;
>  		break;
>  	default:
> -		weston_log("shell: invalid fade type\n");

Is that removed on purpose?


>  		return;
>  	}
>
> -	shell->fade.type = type;
> +	/* Create a separate fade surface for each output */
> +	wl_list_for_each(shell_output, &shell->output_list, link) {
> +		shell_output->fade.type = type;
>
> -	if (shell->fade.view == NULL) {
> -		shell->fade.view = shell_fade_create_surface(shell);
> -		if (!shell->fade.view)
> -			return;
> +		if (shell_output->fade.view == NULL) {
> +			shell_output->fade.view = shell_fade_create_surface_for_output(shell, shell_output);
> +			if (!shell_output->fade.view)
> +				return;

If you do not clean the already created views up, maybe you can use 
"continue", at worst some memory will be freed and you’ll get more 
views. :-)


>
> -		shell->fade.view->alpha = 1.0 - tint;
> -		weston_view_update_transform(shell->fade.view);
> -	}
> +			shell_output->fade.view->alpha = 1.0 - tint;
> +			weston_view_update_transform(shell_output->fade.view);
> +		}
>
> -	if (shell->fade.view->output == NULL) {
> -		/* If the black view gets a NULL output, we lost the
> -		 * last output and we'll just cancel the fade.  This
> -		 * happens when you close the last window under the
> -		 * X11 or Wayland backends. */
> -		shell->locked = false;
> -		weston_surface_destroy(shell->fade.view->surface);
> -		shell->fade.view = NULL;
> -	} 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 (shell_output->fade.view->output == NULL) {
> +			/* If the black view gets a NULL output, we lost the
> +			 * last output and we'll just cancel the fade.  This
> +			 * happens when you close the last window under the
> +			 * X11 or Wayland backends. */
> +			shell->locked = false;
> +			weston_surface_destroy(shell_output->fade.view->surface);
> +			shell_output->fade.view = NULL;
> +		} else if (shell_output->fade.animation) {
> +			weston_fade_update(shell_output->fade.animation, tint);
> +		} else {
> +			shell_output->fade.animation =
> +				weston_fade_run(shell_output->fade.view,
> +						1.0 - tint, tint, 300.0,
> +						shell_fade_done_for_output, shell_output);
> +		}
>  	}
>  }
>
> @@ -3894,6 +3897,7 @@ static void
>  do_shell_fade_startup(void *data)
>  {
>  	struct desktop_shell *shell = data;
> +	struct shell_output *shell_output;
>
>  	if (shell->startup_animation_type == ANIMATION_FADE) {
>  		shell_fade(shell, FADE_IN);
> @@ -3901,8 +3905,10 @@ do_shell_fade_startup(void *data)
>  		weston_log("desktop shell: "
>  			   "unexpected fade-in animation type %d\n",
>  			   shell->startup_animation_type);
> -		weston_surface_destroy(shell->fade.view->surface);
> -		shell->fade.view = NULL;
> +		wl_list_for_each(shell_output, &shell->output_list, link) {
> +			weston_surface_destroy(shell_output->fade.view->surface);
> +			shell_output->fade.view = NULL;
> +		}
>  	}
>  }
>
> @@ -3910,15 +3916,22 @@ static void
>  shell_fade_startup(struct desktop_shell *shell)
>  {
>  	struct wl_event_loop *loop;
> +	struct shell_output *shell_output;
> +	bool has_fade = false;
>
> -	if (!shell->fade.startup_timer)
> -		return;
> +	wl_list_for_each(shell_output, &shell->output_list, link) {
> +		if (!shell_output->fade.startup_timer)
> +			continue;
>
> -	wl_event_source_remove(shell->fade.startup_timer);
> -	shell->fade.startup_timer = NULL;
> +		wl_event_source_remove(shell_output->fade.startup_timer);
> +		shell_output->fade.startup_timer = NULL;
> +		has_fade = true;
> +	}
>
> -	loop = wl_display_get_event_loop(shell->compositor->wl_display);
> -	wl_event_loop_add_idle(loop, do_shell_fade_startup, shell);
> +	if (has_fade) {
> +		loop = wl_display_get_event_loop(shell->compositor->wl_display);
> +		wl_event_loop_add_idle(loop, do_shell_fade_startup, shell);
> +	}
>  }
>
>  static int
> @@ -3939,27 +3952,30 @@ shell_fade_init(struct desktop_shell *shell)
>  	 */
>
>  	struct wl_event_loop *loop;
> -
> -	if (shell->fade.view != NULL) {
> -		weston_log("%s: warning: fade surface already exists\n",
> -			   __func__);
> -		return;
> -	}
> +	struct shell_output *shell_output;
>
>  	if (shell->startup_animation_type == ANIMATION_NONE)
>  		return;
>
> -	shell->fade.view = shell_fade_create_surface(shell);
> -	if (!shell->fade.view)
> -		return;
> +	wl_list_for_each(shell_output, &shell->output_list, link) {
> +		if (shell_output->fade.view != NULL) {
> +			weston_log("%s: warning: fade surface already exists\n",
> +				   __func__);

Not sure why this warning is there in the first place, but i guess it’ll 
trigger for all outputs (except OOM in the other loop?), that makes 
duplicates messages with no bonus information.


> +			continue;
> +		}
> +
> +		shell_output->fade.view = shell_fade_create_surface_for_output(shell, shell_output);
> +		if (!shell_output->fade.view)
> +			return;

Same here for OOM return/continue.

Cheers,


>
> -	weston_view_update_transform(shell->fade.view);
> -	weston_surface_damage(shell->fade.view->surface);
> +		weston_view_update_transform(shell_output->fade.view);
> +		weston_surface_damage(shell_output->fade.view->surface);
>
> -	loop = wl_display_get_event_loop(shell->compositor->wl_display);
> -	shell->fade.startup_timer =
> -		wl_event_loop_add_timer(loop, fade_startup_timeout, shell);
> -	wl_event_source_timer_update(shell->fade.startup_timer, 15000);
> +		loop = wl_display_get_event_loop(shell->compositor->wl_display);
> +		shell_output->fade.startup_timer =
> +			wl_event_loop_add_timer(loop, fade_startup_timeout, shell);
> +		wl_event_source_timer_update(shell_output->fade.startup_timer, 15000);
> +	}
>  }
>
>  static void
> @@ -3974,7 +3990,7 @@ idle_handler(struct wl_listener *listener, void *data)
>  		weston_seat_break_desktop_grabs(seat);
>
>  	shell_fade(shell, FADE_OUT);
> -	/* lock() is called from shell_fade_done() */
> +	/* lock() is called from shell_fade_done_for_output() */
>  }
>
>  static void
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index a1cea75..063641d 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -122,6 +122,13 @@ struct shell_output {
>
>  	struct weston_surface *background_surface;
>  	struct wl_listener background_surface_listener;
> +
> +	struct {
> +		struct weston_view *view;
> +		struct weston_view_animation *animation;
> +		enum fade_type type;
> +		struct wl_event_source *startup_timer;
> +	} fade;
>  };
>
>  struct weston_desktop;
> @@ -192,13 +199,6 @@ struct desktop_shell {
>  		struct wl_list surfaces;
>  	} input_panel;
>
> -	struct {
> -		struct weston_view *view;
> -		struct weston_view_animation *animation;
> -		enum fade_type type;
> -		struct wl_event_source *startup_timer;
> -	} fade;
> -
>  	struct exposay exposay;
>
>  	bool allow_zap;
>


-- 

Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list