[PATCH weston 3/5] desktop-shell: use panel location to calculate correct sizes and ranges

Kristian Høgsberg hoegsberg at gmail.com
Wed Jun 18 16:25:47 PDT 2014


On Thu, May 22, 2014 at 10:41:32PM +0200, Jonny Lamb wrote:
> Now the client can let us know where the panel is using
> desktop_shell.set_panel_position, we can correctly calculate where to
> put new views and how big maximized views should be.
> ---
>  desktop-shell/shell.c | 229 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 173 insertions(+), 56 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index e586922..51683ee 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -251,9 +251,10 @@ shell_fade_startup(struct desktop_shell *shell);
>  static struct shell_seat *
>  get_shell_seat(struct weston_seat *seat);
>  
> -static int
> -get_output_panel_height(struct desktop_shell *shell,
> -			struct weston_output *output);
> +static void
> +get_output_panel_size(struct desktop_shell *shell,
> +		      struct weston_output *output,
> +		      int *width, int *height);
>  
>  static void
>  shell_surface_update_child_surface_layers(struct shell_surface *shsurf);
> @@ -351,24 +352,54 @@ shell_grab_start(struct shell_grab *grab,
>  	}
>  }
>  
> -static int
> -get_output_panel_height(struct desktop_shell *shell,
> -			struct weston_output *output)
> +static void
> +get_output_panel_size(struct desktop_shell *shell,
> +		      struct weston_output *output,
> +		      int *width,
> +		      int *height)
>  {
>  	struct weston_view *view;
> -	int panel_height = 0;
>  
> -	if (!output)
> -		return 0;
> +	if (output) {
> +		wl_list_for_each(view, &shell->panel_layer.view_list, layer_link) {
> +			if (view->surface->output == output) {
> +				float x, y;

Can we do 

  if (view->surface->output != output)
    continue;

instead to avoid the 4 level indent here?

>  
> -	wl_list_for_each(view, &shell->panel_layer.view_list, layer_link) {
> -		if (view->surface->output == output) {
> -			panel_height = view->surface->height;
> -			break;
> +				if (shell->panel_position ==
> +				    DESKTOP_SHELL_PANEL_POSITION_TOP ||
> +				    shell->panel_position ==
> +				    DESKTOP_SHELL_PANEL_POSITION_BOTTOM) {
> +
> +					weston_view_to_global_float(view,
> +								    view->surface->width, 0,
> +								    &x, &y);
> +
> +					*width = (int) x;
> +					*height = view->surface->height + (int) y;
> +
> +					return;
> +
> +				} else if (shell->panel_position ==
> +					   DESKTOP_SHELL_PANEL_POSITION_LEFT ||
> +					   shell->panel_position ==
> +					   DESKTOP_SHELL_PANEL_POSITION_RIGHT) {
> +
> +					weston_view_to_global_float(view,
> +								    0, view->surface->height,
> +								    &x, &y);
> +
> +					*width = view->surface->width + (int) x;
> +					*height = (int) y;
> +
> +					return;

It'll be cleaner and easier to read if we do this as a case statement:

  switch (shell->panel_position) {
  case DESKTOP_SHELL_PANEL_POSITION_TOP:
  case DESKTOP_SHELL_PANEL_POSITION_BOTTOM:
    ..
  }


> +				}
> +			}
>  		}
>  	}
>  
> -	return panel_height;
> +	/* output is NULL or the correct view wasn't found */
> +	*width = 0;
> +	*height = 0;
>  }
>  
>  static void
> @@ -389,13 +420,20 @@ send_configure_for_surface(struct shell_surface *shsurf)
>  		height = shsurf->output->height;
>  	} else if (state->maximized) {
>  		struct desktop_shell *shell;
> -		uint32_t panel_height = 0;
> +		int32_t panel_width = 0, panel_height = 0;
>  
>  		shell = shell_surface_get_shell(shsurf);
> -		panel_height = get_output_panel_height(shell, shsurf->output);
> +		get_output_panel_size(shell, shsurf->output,
> +				      &panel_width, &panel_height);
>  
>  		width = shsurf->output->width;
>  		height = shsurf->output->height - panel_height;
> +
> +		if (shell->panel_position == DESKTOP_SHELL_PANEL_POSITION_LEFT ||
> +		    shell->panel_position == DESKTOP_SHELL_PANEL_POSITION_RIGHT) {
> +			width = shsurf->output->width - panel_width;
> +			height = shsurf->output->height;
> +		}
>  	} else {
>  		width = 0;
>  		height = 0;
> @@ -1545,23 +1583,57 @@ static void
>  constrain_position(struct weston_move_grab *move, int *cx, int *cy)
>  {
>  	struct shell_surface *shsurf = move->base.shsurf;
> +	struct desktop_shell *shell = shsurf->shell;
>  	struct weston_pointer *pointer = move->base.grab.pointer;
> -	int x, y, panel_height, bottom;
> +	int x, y, panel_width, panel_height;
> +	int *coord;
> +	int space, surface_dimension, panel_dimension, margin;
>  	const int safety = 50;
>  
>  	x = wl_fixed_to_int(pointer->x + move->dx);
>  	y = wl_fixed_to_int(pointer->y + move->dy);
>  
> -	panel_height = get_output_panel_height(shsurf->shell,
> -					       shsurf->surface->output);
> -	bottom = y + shsurf->surface->height - shsurf->margin.bottom;
> -	if (bottom - panel_height < safety)
> -		y = panel_height + safety -
> -			shsurf->surface->height + shsurf->margin.bottom;
> +	get_output_panel_size(shsurf->shell,
> +			      shsurf->surface->output,
> +			      &panel_width, &panel_height);
> +
> +	/* in an attempt to share some code, let's define what needs
> +	 * to be compared in all the panel position cases.*/
> +	switch (shell->panel_position) {
> +	case DESKTOP_SHELL_PANEL_POSITION_TOP:
> +	default:
> +		coord = &y;
> +		surface_dimension = shsurf->surface->height;
> +		panel_dimension = panel_height;
> +		margin = shsurf->margin.bottom;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_BOTTOM:
> +		coord = &y;
> +		surface_dimension = shsurf->surface->height;
> +		panel_dimension = panel_height;
> +		margin = shsurf->margin.top;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_LEFT:
> +		coord = &x;
> +		surface_dimension = shsurf->surface->width;
> +		panel_dimension = panel_width;
> +		margin = shsurf->margin.right;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_RIGHT:
> +		coord = &x;
> +		surface_dimension = shsurf->surface->width;
> +		panel_dimension = panel_width;
> +		margin = shsurf->margin.left;
> +		break;
> +	}
> +
> +	space = *coord + surface_dimension - margin;
> +	if (space - panel_dimension < safety)
> +		*coord = panel_dimension + safety -
> +			surface_dimension + margin;

The constrain logic isn't specific to the panel location, it's trying to
prevent the title bar from moving above the top of the work area, that is
the screen area minus the panel.  I don't expect we need to do things
differently depending on whether or not we have the panel at the top of
the screen, except that it affects the position of the upper edge of the
work area.

>  
> -	if (move->client_initiated &&
> -	    y + shsurf->margin.top < panel_height)
> -		y = panel_height - shsurf->margin.top;
> +	if (move->client_initiated && *coord + margin < panel_dimension)
> +		*coord = panel_dimension - margin;
>  
>  	*cx = x;
>  	*cy = y;
> @@ -4942,8 +5014,8 @@ weston_view_set_initial_position(struct weston_view *view,
>  {
>  	struct weston_compositor *compositor = shell->compositor;
>  	int ix = 0, iy = 0;
> -	int range_x, range_y;
> -	int dx, dy, x, y, panel_height;
> +	int32_t range_x, range_y;
> +	int32_t dx, dy, x, y, panel_width, panel_height;
>  	struct weston_output *output, *target_output = NULL;
>  	struct weston_seat *seat;
>  
> @@ -4977,20 +5049,45 @@ weston_view_set_initial_position(struct weston_view *view,
>  	 * If this is negative it means that the surface is bigger than
>  	 * output.
>  	 */
> -	panel_height = get_output_panel_height(shell, target_output);
> -	range_x = target_output->width - view->surface->width;
> -	range_y = (target_output->height - panel_height) -
> -		  view->surface->height;
> +	get_output_panel_size(shell, target_output, &panel_width, &panel_height);
>  
> -	if (range_x > 0)
> -		dx = random() % range_x;
> -	else
> +	switch (shell->panel_position) {
> +	case DESKTOP_SHELL_PANEL_POSITION_TOP:
> +	default:
> +		range_x = target_output->width - view->surface->width;
> +		range_y = (target_output->height - panel_height) -
> +			view->surface->height;
>  		dx = 0;
> +		dy = panel_height;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_BOTTOM:
> +		range_x = target_output->width - view->surface->width;
> +		range_y = (target_output->height - panel_height) -
> +			view->surface->height;
> +		dx = 0;
> +		dy = (range_y >= panel_height) ? -panel_height : 0;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_LEFT:
> +		range_x = (target_output->width - panel_width) -
> +			view->surface->width;
> +		range_y = target_output->height - view->surface->height;
> +		dx = panel_width;
> +		dy = 0;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_RIGHT:
> +		range_x = (target_output->width - panel_width) -
> +			view->surface->width;
> +		range_y = target_output->height - view->surface->height;
> +		dx = (range_x >= panel_width) ? -panel_width : 0;
> +		dy = 0;
> +		break;
> +	}

Looking at this switch and some of the other code, it looks to me like we
might be better off making get_output_panel_size() be get_output_work_area()
which returns the work area, that is, the screen rect minus the panel rect.

> +
> +	if (range_x > 0)
> +		dx += random() % range_x;
>  
>  	if (range_y > 0)
> -		dy = panel_height + random() % range_y;
> -	else
> -		dy = panel_height;
> +		dy += random() % range_y;
>  
>  	x = target_output->x + dx;
>  	y = target_output->y + dy;
> @@ -4999,13 +5096,47 @@ weston_view_set_initial_position(struct weston_view *view,
>  }
>  
>  static void
> +set_maximized_position(struct desktop_shell *shell,
> +		       struct shell_surface *shsurf)
> +{
> +	int32_t surf_x, surf_y;
> +	int32_t x, y;
> +	int32_t panel_width, panel_height;
> +
> +	/* use surface configure to set the geometry */
> +	get_output_panel_size(shell, shsurf->output, &panel_width, &panel_height);
> +	surface_subsurfaces_boundingbox(shsurf->surface,
> +					&surf_x, &surf_y, NULL, NULL);
> +
> +	switch (shell->panel_position) {
> +	case DESKTOP_SHELL_PANEL_POSITION_TOP:
> +	default:
> +		x = shsurf->output->x - surf_x;
> +		y = shsurf->output->y + panel_height - surf_y;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_BOTTOM:
> +		x = shsurf->output->x - surf_x;
> +		y = shsurf->output->y - surf_y;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_LEFT:
> +		x = shsurf->output->x + panel_width - surf_x;
> +		y = shsurf->output->y - surf_y;
> +		break;
> +	case DESKTOP_SHELL_PANEL_POSITION_RIGHT:
> +		x = shsurf->output->x - surf_x;
> +		y = shsurf->output->y - surf_y;
> +		break;
> +	}
> +
> +	weston_view_set_position(shsurf->view, x, y);
> +}
> +
> +static void
>  map(struct desktop_shell *shell, struct shell_surface *shsurf,
>      int32_t sx, int32_t sy)
>  {
>  	struct weston_compositor *compositor = shell->compositor;
>  	struct weston_seat *seat;
> -	int panel_height = 0;
> -	int32_t surf_x, surf_y;
>  
>  	/* initial positioning, see also configure() */
>  	switch (shsurf->type) {
> @@ -5014,14 +5145,7 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>  			center_on_output(shsurf->view, shsurf->fullscreen_output);
>  			shell_map_fullscreen(shsurf);
>  		} else if (shsurf->state.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);
> +			set_maximized_position(shell, shsurf);
>  		} else if (!shsurf->state.relative) {
>  			weston_view_set_initial_position(shsurf->view, shell);
>  		}
> @@ -5094,7 +5218,6 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
>  {
>  	struct shell_surface *shsurf;
>  	struct weston_view *view;
> -	int32_t mx, my, surf_x, surf_y;
>  
>  	shsurf = get_shell_surface(surface);
>  
> @@ -5103,13 +5226,7 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
>  	if (shsurf->state.fullscreen)
>  		shell_configure_fullscreen(shsurf);
>  	else if (shsurf->state.maximized) {
> -		/* setting x, y and using configure to change that geometry */
> -		surface_subsurfaces_boundingbox(shsurf->surface, &surf_x, &surf_y,
> -		                                                 NULL, NULL);
> -		mx = shsurf->output->x - surf_x;
> -		my = shsurf->output->y +
> -		     get_output_panel_height(shell,shsurf->output) - surf_y;
> -		weston_view_set_position(shsurf->view, mx, my);
> +		set_maximized_position(shell, shsurf);
>  	} else {
>  		weston_view_set_position(shsurf->view, x, y);
>  	}
> -- 
> 2.0.0.rc2
> 
> _______________________________________________
> 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