[PATCH weston v6 1/6] desktop-shell: Enable per-output fade animations
Bryce Harrington
bryce at osg.samsung.com
Fri Sep 9 20:12:54 UTC 2016
On Fri, Sep 09, 2016 at 11:45:18AM +0200, Quentin Glidic wrote:
> 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?
No, guess it got caught when I was tidying up the patchset.
> > 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. :-)
ok
> >
> >- 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.
That's true, although it would only be one per output.
Besides, from what I understand of the code this condition should not
occur in practice. So if it did show up, there could be a programming
error and I guess if the warning was excessively visible that would not
necessarily be a bad thing. (Maybe this could even be changed to an
assert?)
Anyway, you're right that putting log messages in loops is potentially
spammy, but given the above I'm not worried about it in this particular
case.
> >+ 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.
Thanks, I'll post a replacement for this patch with these changes and
your R-b.
Bryce
> 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