[PATCH weston v4 3/3] desktop-shell: Enable per-output fade animations

Derek Foreman derekf at osg.samsung.com
Tue Aug 16 00:14:02 UTC 2016


On 10/08/16 07:25 PM, Bryce Harrington wrote:
> Instead of creating a single global fade surface across all outputs,
> create a separate surface for each output.  This lets us individually
> fade each output (or block fading if the output has an inhibiting
> surface).
> 
> When there are client surfaces with valid idle inhibits present, fade
> out only the outputs those surfaces are not visible on.  Then fade those
> out too, if the client terminates for some reason while the system is in
> sleep/idle/offscreen mode.

I'd love to see this as two patches - first the per output fade (which
fixes what I consider to be a bug in the current implementation) and
then a separate patch with the inhibit specific stuff in it...

> 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 larger during the course
> of the fade animation.
> 
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  desktop-shell/shell.c | 167 ++++++++++++++++++++++++++++++--------------------
>  desktop-shell/shell.h |  14 ++---
>  2 files changed, 109 insertions(+), 72 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 64979cd..0aae142 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -3606,10 +3606,23 @@ destroy_shell_surface(struct shell_surface *shsurf)
>  }
>  
>  static void
> +shell_fade(struct desktop_shell *shell, enum fade_type type);
> +
> +static void
>  shell_destroy_shell_surface(struct wl_resource *resource)
>  {
>  	struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>  
> +	/* Re-queue the fade animation to be evaluated if client had been inhibiting */
> +	if (shsurf->surface->inhibit_idling &&
> +	    (shsurf->shell->compositor->state == WESTON_COMPOSITOR_IDLE
> +	     || shsurf->shell->compositor->state == WESTON_COMPOSITOR_OFFSCREEN
> +	     || shsurf->shell->compositor->state == WESTON_COMPOSITOR_SLEEPING))
> +	{
> +		shsurf->surface->inhibit_idling = false;
> +		shell_fade(shsurf->shell, FADE_OUT);

What happens if we're already in the middle of fading out when a shell
surface with an inhibitor is destroyed?  Did you test that case?

> +	}
> +
>  	if (!wl_list_empty(&shsurf->popup.grab_link))
>  		remove_popup_grab(shsurf);
>  	if (shsurf->resource)
> @@ -4356,9 +4369,6 @@ xdg_shell_unversioned_dispatch(const void *implementation,
>  /***********************************/
>  
>  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;
> @@ -5356,24 +5366,26 @@ static void
>  shell_fade_done(struct weston_view_animation *animation, void *data)
>  {
>  	struct desktop_shell *shell = data;
> +	struct shell_output *shell_output;
>  
> -	shell->fade.animation = NULL;
> -
> -	switch (shell->fade.type) {
> -	case FADE_IN:
> -		weston_surface_destroy(shell->fade.view->surface);
> -		shell->fade.view = NULL;
> -		break;
> -	case FADE_OUT:
> -		lock(shell);
> -		break;
> -	default:
> -		break;
> +	wl_list_for_each(shell_output, &shell->output_list, link) {
> +		shell_output->fade.animation = NULL;
> +		switch (shell_output->fade.type) {
> +		case FADE_IN:
> +			weston_surface_destroy(shell_output->fade.view->surface);
> +			shell_output->fade.view = NULL;
> +			break;
> +		case FADE_OUT:
> +			lock(shell);
> +			break;
> +		default:
> +			break;
> +		}
>  	}
>  }
>  
>  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;
> @@ -5389,8 +5401,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);
> @@ -5405,6 +5417,8 @@ static void
>  shell_fade(struct desktop_shell *shell, enum fade_type type)
>  {
>  	float tint;
> +	struct shell_output *shell_output;
> +	uint32_t inhibit_mask = weston_compositor_inhibited_outputs(shell->compositor);
>  
>  	switch (type) {
>  	case FADE_IN:
> @@ -5414,36 +5428,41 @@ shell_fade(struct desktop_shell *shell, enum fade_type type)
>  		tint = 1.0;
>  		break;
>  	default:
> -		weston_log("shell: invalid fade type\n");
>  		return;
>  	}
>  
> -	shell->fade.type = type;
> +	/* Create a separate fade surface for each output */
> +	wl_list_for_each(shell_output, &shell->output_list, link) {
> +		if (inhibit_mask & (1 << shell_output->output->id))
> +			continue;
>  
> -	if (shell->fade.view == NULL) {
> -		shell->fade.view = shell_fade_create_surface(shell);
> -		if (!shell->fade.view)
> -			return;
> +		shell_output->fade.type = type;
>  
> -		shell->fade.view->alpha = 1.0 - tint;
> -		weston_view_update_transform(shell->fade.view);
> -	}
> +		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 (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);
> +			shell_output->fade.view->alpha = 1.0 - tint;
> +			weston_view_update_transform(shell_output->fade.view);
> +		}
> +
> +		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) {

Same as my previous question, I guess -does this clause make the fade
animation restart if it's already running?

> +			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, shell);
> +		}
>  	}
>  }
>  
> @@ -5451,6 +5470,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);
> @@ -5458,8 +5478,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;
> +		}
>  	}
>  }
>  
> @@ -5467,15 +5489,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
> @@ -5496,27 +5525,35 @@ 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) {
> +		uint32_t inhibit_mask = weston_compositor_inhibited_outputs(shell->compositor);

Considering when shell_fade_init() is run, is it even possible for there
to be any inhibiting surfaces in play yet?

Thanks,
Derek

>  
> -	weston_view_update_transform(shell->fade.view);
> -	weston_surface_damage(shell->fade.view->surface);
> +		if (shell_output->fade.view != NULL) {
> +			weston_log("%s: warning: fade surface already exists\n",
> +				   __func__);
> +			continue;
> +		}
>  
> -	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);
> +		if (inhibit_mask & (1 << shell_output->output->id))
> +			continue;
> +
> +		shell_output->fade.view = shell_fade_create_surface_for_output(shell, shell_output);
> +		if (!shell_output->fade.view)
> +			return;
> +
> +		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_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
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index f73cae5..befe0cc 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -121,6 +121,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 desktop_shell {
> @@ -188,13 +195,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;
> 



More information about the wayland-devel mailing list