[PATCH weston] desktop-shell: Properly handle lowered fullscreen surfaces

Kristian Høgsberg hoegsberg at gmail.com
Tue Apr 29 16:37:54 PDT 2014


On Thu, Jan 30, 2014 at 02:01:10PM +0100, pochu27 at gmail.com wrote:
> From: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>
> 
> 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
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list