[PATCH weston v3 8/8] desktop-shell: Enable per-output fade animations
Pekka Paalanen
ppaalanen at gmail.com
Thu May 26 15:02:19 UTC 2016
On Thu, 7 Apr 2016 16:44:23 -0700
Bryce Harrington <bryce at osg.samsung.com> 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).
Hi,
sounds good.
> 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.
Oh here it is, the reaction to surfaces disappearing. However, since
all the idle-inhibit is implemented in the core, the core should also
handle the disappearing inhibitors. And appearing inhibitors, maybe?
Just as the shells decide what is "active", the core would decide which
output is on or going idle/off. After all, all of the idle-inhibit
implementation is in the core, and it should maintain accurate state on
what is going on. This is related to my comments in patch 3.
If shells need much more fine-grained control over inhibit, maybe you
will rename the weston_surface_{activate,deactivate}() for that case? Or
maybe shells would like to control per-output directly instead of
per-surface? I don't know, do we have use cases?
Shells could probably use a per-output signal for things like the
fade-in/out, so the core can just drive output states and others can
listen to what they want rather than dig things out the hard way. I
still think the compositor-wide sleep states should be separated from
the per-output states.
> 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.
That is a reasonable corner-case to ignore, excellent to mention it here.
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
> v3: Fix failure to resume fade-out animation when inhibiting client exits
>
> desktop-shell/shell.c | 166 +++++++++++++++++++++++++++++++-------------------
> desktop-shell/shell.h | 14 ++---
> 2 files changed, 109 insertions(+), 71 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 48e4e30..78ec665 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -3545,10 +3545,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);
> + }
> +
> if (!wl_list_empty(&shsurf->popup.grab_link))
> remove_popup_grab(shsurf);
> wl_list_remove(wl_resource_get_link(shsurf->resource));
> @@ -4285,9 +4298,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;
> @@ -5224,24 +5234,27 @@ 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;
> + wl_list_for_each(shell_output, &shell->output_list, link) {
> + shell_output->fade.animation = NULL;
shell_fade_done() will be called once for each animation created, so it
will already be called once for each output. There is no need to
iterate over every output for each output.
>
> - 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;
> + 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;
> @@ -5257,8 +5270,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);
Some quite long lines hare and above.
> 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);
> @@ -5271,6 +5284,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_output_inhibited_outputs(shell->compositor);
>
> switch (type) {
> case FADE_IN:
> @@ -5280,36 +5295,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) {
Broken indent.
> + 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. */
Because the black surface is no longer covering all outputs, this
comment is no longer true. Now NULL means the one output just
disappeared.
> + 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, shell);
> + }
> }
> }
>
> @@ -5317,6 +5337,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);
> @@ -5324,8 +5345,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;
> + }
> }
> }
>
> @@ -5333,15 +5356,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
> @@ -5362,27 +5392,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_output_inhibited_outputs(shell->compositor);
You may want to put that call outside of the loop.
> +
> + if (shell_output->fade.view != NULL) {
> + weston_log("%s: warning: fade surface already exists\n",
> + __func__);
> + continue;
> + }
> +
> + if (inhibit_mask & (1 << shell_output->output->id))
> + continue;
>
> - weston_view_update_transform(shell->fade.view);
> - weston_surface_damage(shell->fade.view->surface);
> + 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->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);
Do not duplicate this timer for every output, we only need one timer.
See the comment on top of this function on what it's for.
> + }
> }
>
> static void
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index 2ef23f4..0e5a4d1 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -114,6 +114,13 @@ struct shell_output {
> struct exposay_output eoutput;
> struct wl_listener destroy_listener;
> struct wl_list link;
> +
> + struct {
> + struct weston_view *view;
> + struct weston_view_animation *animation;
> + enum fade_type type;
> + struct wl_event_source *startup_timer;
> + } fade;
> };
>
> struct desktop_shell {
> @@ -179,13 +186,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;
>
> uint32_t binding_modifier;
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/20160526/89a47772/attachment.sig>
More information about the wayland-devel
mailing list