On Thu, Jan 30, 2014 at 02:01:10PM +0100, [email protected] wrote:
> From: Emilio Pozuelo Monfort <[email protected]>
> 
> lower_fullscreen_surface() was removing fullscreen surfaces from
> the fullscreen layer and inserting them in the normal workspace
> layer. However, those fullscreen surfaces were never put back in
> the fullscreen layer, causing bugs such as unrelated surfaces
> being drawn between a fullscreen surface and its black view.
> 
> Change the lower_fullscreen_surface() logic so that it lowers
> fullscreen surfaces to the workspace layer *and* hides the
> black views. Make this reversible by re-configuring the lowered
> fullscreen surface: when it is re-configured, the black view
> will be shown again and the surface will be restacked in the
> fullscreen layer.

Hi Emilio,

Sorry for the looong delay in looking at this.  The patch looks good and
fixes the issues below.  Applied - I do feel like we should try to come
up with a better mechanism for highlighting a window on hover instead of
reusing activate with a configure flag.  Once we finish the xdg-shell
changes, we'll have an 'active' surface state, that we can use for this,
so let's revisit this when we land that.

Kristian

> https://bugs.freedesktop.org/show_bug.cgi?id=73575
> https://bugs.freedesktop.org/show_bug.cgi?id=74221
> https://bugs.freedesktop.org/show_bug.cgi?id=74222
> ---
>  desktop-shell/exposay.c |  8 +++----
>  desktop-shell/shell.c   | 56 
> +++++++++++++++++++++++++++++++++----------------
>  desktop-shell/shell.h   |  5 +----
>  3 files changed, 42 insertions(+), 27 deletions(-)
> 
> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
> index fe7a3a7..f09224f 100644
> --- a/desktop-shell/exposay.c
> +++ b/desktop-shell/exposay.c
> @@ -141,7 +141,7 @@ exposay_highlight_surface(struct desktop_shell *shell,
>       shell->exposay.row_current = esurface->row;
>       shell->exposay.column_current = esurface->column;
>  
> -     activate(shell, view->surface, shell->exposay.seat);
> +     activate(shell, view->surface, shell->exposay.seat, false);
>       shell->exposay.focus_current = view;
>  }
>  
> @@ -283,8 +283,6 @@ exposay_layout(struct desktop_shell *shell)
>               if (shell->exposay.focus_current == esurface->view)
>                       highlight = esurface;
>  
> -             set_alpha_if_fullscreen(get_shell_surface(view->surface));
> -
>               exposay_animate_in(esurface);
>  
>               i++;
> @@ -502,10 +500,10 @@ exposay_transition_inactive(struct desktop_shell 
> *shell, int switch_focus)
>        * to the new. */
>       if (switch_focus && shell->exposay.focus_current)
>               activate(shell, shell->exposay.focus_current->surface,
> -                      shell->exposay.seat);
> +                      shell->exposay.seat, true);
>       else if (shell->exposay.focus_prev)
>               activate(shell, shell->exposay.focus_prev->surface,
> -                      shell->exposay.seat);
> +                      shell->exposay.seat, true);
>  
>       wl_list_for_each(esurface, &shell->exposay.surface_list, link)
>               exposay_animate_out(esurface);
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 3087042..a8a0537 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -173,6 +173,7 @@ struct shell_surface {
>       struct {
>               bool maximized;
>               bool fullscreen;
> +             bool lowered; /* fullscreen but lowered, see 
> lower_fullscreen_layer() */
>               bool relative;
>       } state, next_state; /* surface states */
>       bool state_changed;
> @@ -223,13 +224,6 @@ struct shell_seat {
>       } popup_grab;
>  };
>  
> -void
> -set_alpha_if_fullscreen(struct shell_surface *shsurf)
> -{
> -     if (shsurf && shsurf->state.fullscreen)
> -             shsurf->fullscreen.black_view->alpha = 0.25;
> -}
> -
>  static struct desktop_shell *
>  shell_surface_get_shell(struct shell_surface *shsurf);
>  
> @@ -611,7 +605,7 @@ focus_state_surface_destroy(struct wl_listener *listener, 
> void *data)
>       shell = state->seat->compositor->shell_interface.shell;
>       if (next) {
>               state->keyboard_focus = NULL;
> -             activate(shell, next, state->seat);
> +             activate(shell, next, state->seat, true);
>       } else {
>               if (shell->focus_animation_type == ANIMATION_DIM_LAYER) {
>                       if (state->ws->focus_animation)
> @@ -1762,10 +1756,10 @@ busy_cursor_grab_button(struct weston_pointer_grab 
> *base,
>       struct weston_seat *seat = grab->grab.pointer->seat;
>  
>       if (shsurf && button == BTN_LEFT && state) {
> -             activate(shsurf->shell, shsurf->surface, seat);
> +             activate(shsurf->shell, shsurf->surface, seat, true);
>               surface_move(shsurf, seat);
>       } else if (shsurf && button == BTN_RIGHT && state) {
> -             activate(shsurf->shell, shsurf->surface, seat);
> +             activate(shsurf->shell, shsurf->surface, seat, true);
>               surface_rotate(shsurf, seat);
>       }
>  }
> @@ -2036,7 +2030,7 @@ shell_surface_calculate_layer_link (struct 
> shell_surface *shsurf)
>       switch (shsurf->type) {
>       case SHELL_SURFACE_POPUP:
>       case SHELL_SURFACE_TOPLEVEL:
> -             if (shsurf->state.fullscreen) {
> +             if (shsurf->state.fullscreen && !shsurf->state.lowered) {
>                       return &shsurf->shell->fullscreen_layer.view_list;
>               } else if (shsurf->parent) {
>                       /* Move the surface to its parent layer so
> @@ -2533,6 +2527,8 @@ shell_ensure_fullscreen_black_view(struct shell_surface 
> *shsurf)
>                      &shsurf->fullscreen.black_view->layer_link);
>       weston_view_geometry_dirty(shsurf->fullscreen.black_view);
>       weston_surface_damage(shsurf->surface);
> +
> +     shsurf->state.lowered = false;
>  }
>  
>  /* Create black surface and append it to the associated fullscreen surface.
> @@ -2549,6 +2545,10 @@ shell_configure_fullscreen(struct shell_surface 
> *shsurf)
>       if (shsurf->fullscreen.type != 
> WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER)
>               restore_output_mode(output);
>  
> +     /* Reverse the effect of lower_fullscreen_layer() */
> +     wl_list_remove(&shsurf->view->layer_link);
> +     wl_list_insert(&shsurf->shell->fullscreen_layer.view_list, 
> &shsurf->view->layer_link);
> +
>       shell_ensure_fullscreen_black_view(shsurf);
>  
>       surface_subsurfaces_boundingbox(shsurf->surface, &surf_x, &surf_y,
> @@ -4201,8 +4201,12 @@ rotate_binding(struct weston_seat *seat, uint32_t 
> time, uint32_t button,
>       surface_rotate(surface, seat);
>  }
>  
> -/* Move all fullscreen layers down to the current workspace in a 
> non-reversible
> - * manner. This should be used when implementing shell-wide overlays, such as
> +/* Move all fullscreen layers down to the current workspace and hide their
> + * black views. The surfaces' state is set to both fullscreen and lowered,
> + * and this is reversed when such a surface is re-configured, see
> + * shell_configure_fullscreen() and shell_ensure_fullscreen_black_view().
> + *
> + * This should be used when implementing shell-wide overlays, such as
>   * the alt-tab switcher, which need to de-promote fullscreen layers. */
>  void
>  lower_fullscreen_layer(struct desktop_shell *shell)
> @@ -4214,16 +4218,32 @@ lower_fullscreen_layer(struct desktop_shell *shell)
>       wl_list_for_each_reverse_safe(view, prev,
>                                     &shell->fullscreen_layer.view_list,
>                                     layer_link) {
> +             struct shell_surface *shsurf = get_shell_surface(view->surface);
> +
> +             if (!shsurf)
> +                     continue;
> +
> +             /* We can have a non-fullscreen popup for a fullscreen surface
> +              * in the fullscreen layer. */
> +             if (shsurf->state.fullscreen) {
> +                     /* Hide the black view */
> +                     
> wl_list_remove(&shsurf->fullscreen.black_view->layer_link);
> +                     
> wl_list_init(&shsurf->fullscreen.black_view->layer_link);
> +             }
> +
> +             /* Lower the view to the workspace layer */
>               wl_list_remove(&view->layer_link);
>               wl_list_insert(&ws->layer.view_list, &view->layer_link);
>               weston_view_damage_below(view);
>               weston_surface_damage(view->surface);
> +
> +             shsurf->state.lowered = true;
>       }
>  }
>  
>  void
>  activate(struct desktop_shell *shell, struct weston_surface *es,
> -      struct weston_seat *seat)
> +      struct weston_seat *seat, bool configure)
>  {
>       struct weston_surface *main_surface;
>       struct focus_state *state;
> @@ -4245,7 +4265,7 @@ activate(struct desktop_shell *shell, struct 
> weston_surface *es,
>       shsurf = get_shell_surface(main_surface);
>       assert(shsurf);
>  
> -     if (shsurf->state.fullscreen)
> +     if (shsurf->state.fullscreen && configure)
>               shell_configure_fullscreen(shsurf);
>       else
>               restore_all_output_modes(shell->compositor);
> @@ -4294,7 +4314,7 @@ activate_binding(struct weston_seat *seat,
>       if (get_shell_surface_type(main_surface) == SHELL_SURFACE_NONE)
>               return;
>  
> -     activate(shell, focus, seat);
> +     activate(shell, focus, seat, true);
>  }
>  
>  static void
> @@ -4700,7 +4720,7 @@ map(struct desktop_shell *shell, struct shell_surface 
> *shsurf,
>               if (shell->locked)
>                       break;
>               wl_list_for_each(seat, &compositor->seat_list, link)
> -                     activate(shell, shsurf->surface, seat);
> +                     activate(shell, shsurf->surface, seat, true);
>               break;
>       case SHELL_SURFACE_POPUP:
>       case SHELL_SURFACE_NONE:
> @@ -5103,7 +5123,7 @@ switcher_destroy(struct switcher *switcher)
>  
>       if (switcher->current)
>               activate(switcher->shell, switcher->current,
> -                      (struct weston_seat *) keyboard->seat);
> +                      (struct weston_seat *) keyboard->seat, true);
>       wl_list_remove(&switcher->listener.link);
>       weston_keyboard_end_grab(keyboard);
>       if (keyboard->input_method_resource)
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index dbb2854..54a7f32 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -189,9 +189,6 @@ struct desktop_shell {
>       char *client;
>  };
>  
> -void
> -set_alpha_if_fullscreen(struct shell_surface *shsurf);
> -
>  struct weston_output *
>  get_default_output(struct weston_compositor *compositor);
>  
> @@ -209,7 +206,7 @@ lower_fullscreen_layer(struct desktop_shell *shell);
>  
>  void
>  activate(struct desktop_shell *shell, struct weston_surface *es,
> -      struct weston_seat *seat);
> +      struct weston_seat *seat, bool configure);
>  
>  void
>  exposay_binding(struct weston_seat *seat,
> -- 
> 1.8.5.3
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to