[PATCH 02/16] shell: Remove SHELL_SURFACE_FULLSCREEN and SHELL_SURFACE_MAXIMIZED.

Kristian Høgsberg hoegsberg at gmail.com
Fri Nov 29 15:00:17 PST 2013


On Wed, Nov 27, 2013 at 03:50:18PM -0200, Rafael Antognolli wrote:
> These surface types don't exist anymore inside weston desktop shell
> implementation. They are just exposed as wl_shell surface types, but
> internally the implementation is done with surface states.
> 
> The previous
> behavior (setting a surface type unsets another one) still happens when
> using wl_shell. This change is mainly done as a refactory to allow
> xdg-shell to use the same code.
> ---
>  src/shell.c | 224 +++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 123 insertions(+), 101 deletions(-)

This patch is good, it's a nice, self-contained step towards
xdg-shell.  I just have a couple of nit-picks below.

> diff --git a/src/shell.c b/src/shell.c
> index f102e9a..cf89a84 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -237,8 +237,6 @@ enum shell_surface_type {
>  	SHELL_SURFACE_NONE,
>  	SHELL_SURFACE_TOPLEVEL,
>  	SHELL_SURFACE_TRANSIENT,
> -	SHELL_SURFACE_FULLSCREEN,
> -	SHELL_SURFACE_MAXIMIZED,
>  	SHELL_SURFACE_POPUP,
>  	SHELL_SURFACE_XWAYLAND
>  };
> @@ -298,6 +296,12 @@ struct shell_surface {
>  	struct wl_list link;
>  
>  	const struct weston_shell_client *client;
> +
> +	struct {
> +		bool maximized;
> +		bool fullscreen;
> +	} cur, next; /* surface states */

We try to avoid abbreviations like cur - can we just call it state and
next_state?

> +	bool state_changed;
>  };
>  
>  struct shell_grab {
> @@ -1450,7 +1454,7 @@ surface_touch_move(struct shell_surface *shsurf, struct weston_seat *seat)
>  	if (!shsurf)
>  		return -1;
>  
> -	if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> +	if (shsurf->cur.fullscreen)
>  		return 0;
>  	if (shsurf->grabbed)
>  		return 0;
> @@ -1541,7 +1545,7 @@ surface_move(struct shell_surface *shsurf, struct weston_seat *seat)
>  
>  	if (shsurf->grabbed)
>  		return 0;
> -	if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> +	if (shsurf->cur.fullscreen)
>  		return 0;
>  
>  	move = malloc(sizeof *move);
> @@ -1715,8 +1719,7 @@ surface_resize(struct shell_surface *shsurf,
>  {
>  	struct weston_resize_grab *resize;
>  
> -	if (shsurf->type == SHELL_SURFACE_FULLSCREEN ||
> -	    shsurf->type == SHELL_SURFACE_MAXIMIZED)
> +	if (shsurf->cur.fullscreen || shsurf->cur.maximized)
>  		return 0;
>  
>  	if (edges == 0 || edges > 15 ||
> @@ -1746,7 +1749,7 @@ shell_surface_resize(struct wl_client *client, struct wl_resource *resource,
>  	struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>  	struct weston_surface *surface;
>  
> -	if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> +	if (shsurf->cur.fullscreen)
>  		return;
>  
>  	surface = weston_surface_get_main_surface(seat->pointer->focus->surface);
> @@ -2061,26 +2064,31 @@ shell_unset_maximized(struct shell_surface *shsurf)
>  static int
>  reset_shell_surface_type(struct shell_surface *surface)
>  {
> -	switch (surface->type) {
> -	case SHELL_SURFACE_FULLSCREEN:
> +	if (surface->cur.fullscreen)
>  		shell_unset_fullscreen(surface);
> -		break;
> -	case SHELL_SURFACE_MAXIMIZED:
> +	if (surface->cur.maximized)
>  		shell_unset_maximized(surface);
> -		break;
> -	case SHELL_SURFACE_NONE:
> -	case SHELL_SURFACE_TOPLEVEL:
> -	case SHELL_SURFACE_TRANSIENT:
> -	case SHELL_SURFACE_POPUP:
> -	case SHELL_SURFACE_XWAYLAND:
> -		break;
> -	}
>  
>  	surface->type = SHELL_SURFACE_NONE;
>  	return 0;
>  }
>  
>  static void
> +set_full_output(struct shell_surface *shsurf)
> +{
> +	shsurf->saved_x = shsurf->view->geometry.x;
> +	shsurf->saved_y = shsurf->view->geometry.y;
> +	shsurf->saved_position_valid = true;
> +
> +	if (!wl_list_empty(&shsurf->rotation.transform.link)) {
> +		wl_list_remove(&shsurf->rotation.transform.link);
> +		wl_list_init(&shsurf->rotation.transform.link);
> +		weston_view_geometry_dirty(shsurf->view);
> +		shsurf->saved_rotation_valid = true;
> +	}
> +}
> +
> +static void
>  set_surface_type(struct shell_surface *shsurf)
>  {
>  	struct weston_surface *pes = shsurf->parent;
> @@ -2089,10 +2097,14 @@ set_surface_type(struct shell_surface *shsurf)
>  	reset_shell_surface_type(shsurf);
>  
>  	shsurf->type = shsurf->next_type;
> +	shsurf->cur = shsurf->next;
>  	shsurf->next_type = SHELL_SURFACE_NONE;
> +	shsurf->state_changed = false;
>  
>  	switch (shsurf->type) {
>  	case SHELL_SURFACE_TOPLEVEL:
> +		if (shsurf->cur.maximized || shsurf->cur.fullscreen)
> +			set_full_output(shsurf);
>  		break;
>  	case SHELL_SURFACE_TRANSIENT:
>  		if (pev)
> @@ -2101,20 +2113,6 @@ set_surface_type(struct shell_surface *shsurf)
>  						 pev->geometry.y + shsurf->transient.y);
>  		break;
>  
> -	case SHELL_SURFACE_MAXIMIZED:
> -	case SHELL_SURFACE_FULLSCREEN:
> -		shsurf->saved_x = shsurf->view->geometry.x;
> -		shsurf->saved_y = shsurf->view->geometry.y;
> -		shsurf->saved_position_valid = true;
> -
> -		if (!wl_list_empty(&shsurf->rotation.transform.link)) {
> -			wl_list_remove(&shsurf->rotation.transform.link);
> -			wl_list_init(&shsurf->rotation.transform.link);
> -			weston_view_geometry_dirty(shsurf->view);
> -			shsurf->saved_rotation_valid = true;
> -		}
> -		break;
> -
>  	case SHELL_SURFACE_XWAYLAND:
>  		weston_view_set_position(shsurf->view, shsurf->transient.x,
>  					 shsurf->transient.y);
> @@ -2126,9 +2124,20 @@ set_surface_type(struct shell_surface *shsurf)
>  }
>  
>  static void
> +surface_clear_next_states(struct shell_surface *shsurf)
> +{
> +	shsurf->next.maximized = false;
> +	shsurf->next.fullscreen = false;
> +
> +	if ((shsurf->next.maximized != shsurf->cur.maximized) ||
> +	    (shsurf->next.fullscreen != shsurf->cur.fullscreen))
> +		shsurf->state_changed = true;
> +}
> +
> +static void
>  set_toplevel(struct shell_surface *shsurf)
>  {
> -       shsurf->next_type = SHELL_SURFACE_TOPLEVEL;
> +	shsurf->next_type = SHELL_SURFACE_TOPLEVEL;
>  }
>  
>  static void
> @@ -2137,6 +2146,7 @@ shell_surface_set_toplevel(struct wl_client *client,
>  {
>  	struct shell_surface *surface = wl_resource_get_user_data(resource);
>  
> +	surface_clear_next_states(surface);
>  	set_toplevel(surface);
>  }
>  
> @@ -2150,6 +2160,7 @@ set_transient(struct shell_surface *shsurf,
>  	shsurf->transient.y = y;
>  	shsurf->transient.flags = flags;
>  	shsurf->next_type = SHELL_SURFACE_TRANSIENT;
> +	surface_clear_next_states(shsurf);
>  }
>  
>  static void
> @@ -2162,6 +2173,7 @@ shell_surface_set_transient(struct wl_client *client,
>  	struct weston_surface *parent =
>  		wl_resource_get_user_data(parent_resource);
>  
> +	surface_clear_next_states(shsurf);
>  	set_transient(shsurf, parent, x, y, flags);
>  }
>  
> @@ -2192,9 +2204,9 @@ get_output_panel_height(struct desktop_shell *shell,
>  }
>  
>  static void
> -shell_surface_set_maximized(struct wl_client *client,
> -			    struct wl_resource *resource,
> -			    struct wl_resource *output_resource )
> +set_maximized(struct wl_client *client,
> +	      struct wl_resource *resource,
> +	      struct wl_resource *output_resource)
>  {
>  	struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>  	struct weston_surface *es = shsurf->surface;
> @@ -2218,7 +2230,19 @@ shell_surface_set_maximized(struct wl_client *client,
>  				       shsurf->output->width,
>  				       shsurf->output->height - panel_height);
>  
> -	shsurf->next_type = SHELL_SURFACE_MAXIMIZED;
> +	shsurf->next_type = shsurf->type;
> +}
> +
> +static void
> +shell_surface_set_maximized(struct wl_client *client,
> +			    struct wl_resource *resource,
> +			    struct wl_resource *output_resource)
> +{
> +	struct shell_surface *shsurf = wl_resource_get_user_data(resource);
> +	surface_clear_next_states(shsurf);
> +	set_maximized(client, resource, output_resource);
> +	shsurf->next.maximized = true;
> +	shsurf->state_changed = true;
>  }
>  
>  static void
> @@ -2410,7 +2434,7 @@ set_fullscreen(struct shell_surface *shsurf,
>  	shsurf->fullscreen_output = shsurf->output;
>  	shsurf->fullscreen.type = method;
>  	shsurf->fullscreen.framerate = framerate;
> -	shsurf->next_type = SHELL_SURFACE_FULLSCREEN;
> +	shsurf->next_type = shsurf->type;
>  
>  	shsurf->client->send_configure(shsurf->surface, 0,
>  				       shsurf->output->width,
> @@ -2432,13 +2456,17 @@ shell_surface_set_fullscreen(struct wl_client *client,
>  	else
>  		output = NULL;
>  
> +	surface_clear_next_states(shsurf);
>  	set_fullscreen(shsurf, method, framerate, output);
> +	shsurf->next.fullscreen = true;
> +	shsurf->state_changed = true;
>  }
>  
>  static void
>  set_xwayland(struct shell_surface *shsurf, int x, int y, uint32_t flags)
>  {
>  	/* XXX: using the same fields for transient type */
> +	surface_clear_next_states(shsurf);
>  	shsurf->transient.x = x;
>  	shsurf->transient.y = y;
>  	shsurf->transient.flags = flags;
> @@ -2684,6 +2712,7 @@ shell_surface_set_popup(struct wl_client *client,
>  {
>  	struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>  
> +	surface_clear_next_states(shsurf);
>  	shsurf->type = SHELL_SURFACE_POPUP;
>  	shsurf->parent = wl_resource_get_user_data(parent_resource);
>  	shsurf->popup.shseat = get_shell_seat(wl_resource_get_user_data(seat_resource));
> @@ -3190,8 +3219,7 @@ move_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void *dat
>  		return;
>  
>  	shsurf = get_shell_surface(surface);
> -	if (shsurf == NULL || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
> -	    shsurf->type == SHELL_SURFACE_MAXIMIZED)
> +	if (shsurf == NULL || shsurf->cur.fullscreen || shsurf->cur.maximized)
>  		return;
>  
>  	surface_move(shsurf, (struct weston_seat *) seat);
> @@ -3209,8 +3237,7 @@ touch_move_binding(struct weston_seat *seat, uint32_t time, void *data)
>  		return;
>  
>  	shsurf = get_shell_surface(surface);
> -	if (shsurf == NULL || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
> -	    shsurf->type == SHELL_SURFACE_MAXIMIZED)
> +	if (shsurf == NULL || shsurf->cur.fullscreen || shsurf->cur.maximized)
>  		return;
>  
>  	surface_touch_move(shsurf, (struct weston_seat *) seat);
> @@ -3230,8 +3257,7 @@ resize_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void *d
>  		return;
>  
>  	shsurf = get_shell_surface(surface);
> -	if (!shsurf || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
> -	    shsurf->type == SHELL_SURFACE_MAXIMIZED)
> +	if (!shsurf || shsurf->cur.fullscreen || shsurf->cur.maximized)
>  		return;
>  
>  	weston_view_from_global(shsurf->view,
> @@ -3747,8 +3773,7 @@ rotate_binding(struct weston_seat *seat, uint32_t time, uint32_t button,
>  		return;
>  
>  	surface = get_shell_surface(base_surface);
> -	if (!surface || surface->type == SHELL_SURFACE_FULLSCREEN ||
> -	    surface->type == SHELL_SURFACE_MAXIMIZED)
> +	if (!surface || surface->cur.fullscreen || surface->cur.maximized)
>  		return;
>  
>  	surface_rotate(surface, seat);
> @@ -3776,6 +3801,7 @@ activate(struct desktop_shell *shell, struct weston_surface *es,
>  	struct focus_state *state;
>  	struct workspace *ws;
>  	struct weston_surface *old_es;
> +	struct shell_surface *shsurf;
>  
>  	main_surface = weston_surface_get_main_surface(es);
>  
> @@ -3790,21 +3816,20 @@ activate(struct desktop_shell *shell, struct weston_surface *es,
>  	wl_list_remove(&state->surface_destroy_listener.link);
>  	wl_signal_add(&es->destroy_signal, &state->surface_destroy_listener);
>  
> -	switch (get_shell_surface_type(main_surface)) {
> -	case SHELL_SURFACE_FULLSCREEN:
> +	shsurf = get_shell_surface(main_surface);
> +	if (shsurf->cur.fullscreen) {
>  		/* should on top of panels */
>  		shell_stack_fullscreen(get_shell_surface(main_surface));
>  		shell_configure_fullscreen(get_shell_surface(main_surface));
>  		return;
> -	default:
> -		restore_all_output_modes(shell->compositor);
> -		ws = get_current_workspace(shell);
> -		main_view = get_default_view(main_surface);
> -		if (main_view)
> -			weston_view_restack(main_view, &ws->layer.view_list);
> -		break;
>  	}
>  
> +	restore_all_output_modes(shell->compositor);
> +	ws = get_current_workspace(shell);
> +	main_view = get_default_view(main_surface);
> +	if (main_view)
> +		weston_view_restack(main_view, &ws->layer.view_list);
> +
>  	if (shell->focus_animation_type != ANIMATION_NONE)
>  		animate_focus_change(shell, ws, get_default_view(old_es), get_default_view(es));
>  }
> @@ -4241,20 +4266,23 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  	/* initial positioning, see also configure() */
>  	switch (shsurf->type) {
>  	case SHELL_SURFACE_TOPLEVEL:
> -		weston_view_set_initial_position(shsurf->view, shell);
> -		break;
> -	case SHELL_SURFACE_FULLSCREEN:
> -		center_on_output(shsurf->view, shsurf->fullscreen_output);
> -		shell_map_fullscreen(shsurf);
> -		break;
> -	case SHELL_SURFACE_MAXIMIZED:
> -		/* use surface configure to set the geometry */
> -		panel_height = get_output_panel_height(shell, shsurf->output);
> -		surface_subsurfaces_boundingbox(shsurf->surface,
> -						&surf_x, &surf_y, NULL, NULL);
> -		weston_view_set_position(shsurf->view,
> -					 shsurf->output->x - surf_x,
> -		                         shsurf->output->y + panel_height - surf_y);
> +		if (shsurf->cur.fullscreen) {
> +			center_on_output(shsurf->view, shsurf->fullscreen_output);
> +			shell_map_fullscreen(shsurf);
> +		} else if (shsurf->cur.maximized) {
> +			/* use surface configure to set the geometry */
> +			panel_height = get_output_panel_height(shell,
> +							       shsurf->output);
> +			surface_subsurfaces_boundingbox(shsurf->surface,
> +							&surf_x, &surf_y, NULL,
> +							NULL);
> +			weston_view_set_position(shsurf->view,
> +						 shsurf->output->x - surf_x,
> +						 shsurf->output->y +
> +						 panel_height - surf_y);
> +		} else {
> +			weston_view_set_initial_position(shsurf->view, shell);
> +		}
>  		break;
>  	case SHELL_SURFACE_POPUP:
>  		shell_map_popup(shsurf);
> @@ -4280,11 +4308,12 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  				       &shsurf->view->layer_link);
>  		}
>  		break;
> -	case SHELL_SURFACE_FULLSCREEN:
>  	case SHELL_SURFACE_NONE:
>  		break;
>  	case SHELL_SURFACE_XWAYLAND:
>  	default:
> +		if (shsurf->cur.fullscreen)
> +			break;
>  		ws = get_current_workspace(shell);
>  		wl_list_remove(&shsurf->view->layer_link);
>  		wl_list_insert(&ws->layer.view_list, &shsurf->view->layer_link);
> @@ -4293,7 +4322,7 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  
>  	if (shsurf->type != SHELL_SURFACE_NONE) {
>  		weston_view_update_transform(shsurf->view);
> -		if (shsurf->type == SHELL_SURFACE_MAXIMIZED) {
> +		if (shsurf->cur.maximized) {
>  			shsurf->surface->output = shsurf->output;
>  			shsurf->view->output = shsurf->output;
>  		}
> @@ -4307,8 +4336,6 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  				WL_SHELL_SURFACE_TRANSIENT_INACTIVE)
>  			break;
>  	case SHELL_SURFACE_TOPLEVEL:
> -	case SHELL_SURFACE_FULLSCREEN:
> -	case SHELL_SURFACE_MAXIMIZED:
>  		if (!shell->locked) {
>  			wl_list_for_each(seat, &compositor->seat_list, link)
>  				activate(shell, shsurf->surface, seat);
> @@ -4318,7 +4345,8 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  		break;
>  	}
>  
> -	if (shsurf->type == SHELL_SURFACE_TOPLEVEL)
> +	if ((shsurf->type == SHELL_SURFACE_TOPLEVEL) &&
> +	    !(shsurf->cur.fullscreen || shsurf->cur.maximized))
>  	{
>  		switch (shell->win_animation_type) {
>  		case ANIMATION_FADE:
> @@ -4337,14 +4365,11 @@ static void
>  configure(struct desktop_shell *shell, struct weston_surface *surface,
>  	  float x, float y, int32_t width, int32_t height)
>  {
> -	enum shell_surface_type surface_type = SHELL_SURFACE_NONE;
>  	struct shell_surface *shsurf;
>  	struct weston_view *view;
>  	int32_t surf_x, surf_y;
>  
>  	shsurf = get_shell_surface(surface);
> -	if (shsurf)
> -		surface_type = shsurf->type;
>  
>  	/* TODO:
>  	 * This should probably be changed to be more shell_surface
> @@ -4353,31 +4378,29 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
>  	wl_list_for_each(view, &surface->views, surface_link)
>  		weston_view_configure(view, x, y, width, height);
>  
> -	switch (surface_type) {
> -	case SHELL_SURFACE_FULLSCREEN:
> -		shell_stack_fullscreen(shsurf);
> -		shell_configure_fullscreen(shsurf);
> -		break;
> -	case SHELL_SURFACE_MAXIMIZED:
> -		/* setting x, y and using configure to change that geometry */
> -		surface_subsurfaces_boundingbox(shsurf->surface, &surf_x, &surf_y,
> -		                                                 NULL, NULL);
> -		shsurf->view->geometry.x = shsurf->output->x - surf_x;
> -		shsurf->view->geometry.y = shsurf->output->y +
> -			get_output_panel_height(shell,shsurf->output) - surf_y;
> -		break;
> -	case SHELL_SURFACE_TOPLEVEL:
> -		break;
> -	default:
> -		break;
> +	if (shsurf) {

The "what if shsurf is NULL" case is a left-over.  It can never happen
today, so just drop the if statement here.

> +		if (shsurf->cur.fullscreen) {
> +			shell_stack_fullscreen(shsurf);
> +			shell_configure_fullscreen(shsurf);
> +		} else if (shsurf->cur.maximized) {
> +			/* setting x, y and using configure to change that geometry */
> +			surface_subsurfaces_boundingbox(shsurf->surface,
> +							&surf_x, &surf_y,
> +							NULL, NULL);
> +			shsurf->view->geometry.x = shsurf->output->x - surf_x;
> +			shsurf->view->geometry.y = shsurf->output->y +
> +				get_output_panel_height(shell,shsurf->output) -
> +				surf_y;
> +		}
>  	}
>  
> +
>  	/* XXX: would a fullscreen surface need the same handling? */
>  	if (surface->output) {
>  		wl_list_for_each(view, &surface->views, surface_link)
>  			weston_view_update_transform(view);
>  
> -		if (surface_type == SHELL_SURFACE_MAXIMIZED)
> +		if (shsurf->cur.maximized)
>  			surface->output = shsurf->output;
>  	}
>  }
> @@ -4398,8 +4421,9 @@ shell_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy, int32
>  	if (width == 0)
>  		return;
>  
> -	if (shsurf->next_type != SHELL_SURFACE_NONE &&
> -	    shsurf->type != shsurf->next_type) {
> +	if ((shsurf->next_type != SHELL_SURFACE_NONE &&
> +	    shsurf->type != shsurf->next_type) ||
> +	    shsurf->state_changed) {
>  		set_surface_type(shsurf);
>  		type_changed = 1;
>  	}
> @@ -4836,8 +4860,6 @@ switcher_next(struct switcher *switcher)
>  	wl_list_for_each(view, &ws->layer.view_list, layer_link) {
>  		switch (get_shell_surface_type(view->surface)) {
>  		case SHELL_SURFACE_TOPLEVEL:
> -		case SHELL_SURFACE_FULLSCREEN:
> -		case SHELL_SURFACE_MAXIMIZED:
>  			if (first == NULL)
>  				first = view->surface;
>  			if (prev == switcher->current)
> @@ -4872,7 +4894,7 @@ switcher_next(struct switcher *switcher)
>  		view->alpha = 1.0;
>  
>  	shsurf = get_shell_surface(switcher->current);
> -	if (shsurf && shsurf->type ==SHELL_SURFACE_FULLSCREEN)
> +	if (shsurf && shsurf->cur.fullscreen)
>  		shsurf->fullscreen.black_view->alpha = 1.0;
>  }
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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