[PATCH weston 1/3] desktop-shell: add set_panel_position to help place views onscreen

[Pekka Paalanen]

On Wed, 20 Aug 2014 10:37:00 +0200
Jonny Lamb <jonny.lamb at collabora.co.uk> wrote:

> Panels are always assumed to be on the top edge of the output. If this
> is not the case views will be placed under the panel, wherever it is,
> and maximize doesn't use the correct space allocated for views.
> 
> By telling the server on which edge the panel is located, it can
> correctly calculate where to put new views and how big maximized views
> should be.
> ---
>  desktop-shell/shell.c      | 29 ++++++++++++++++++++++++++---
>  desktop-shell/shell.h      |  2 ++
>  protocol/desktop-shell.xml | 20 +++++++++++++++++++-
>  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index ec72287..3b03513 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -4124,13 +4124,34 @@ desktop_shell_desktop_ready(struct wl_client *client,
>  	shell_fade_startup(shell);
>  }
>  
> +static void
> +desktop_shell_set_panel_position(struct wl_client *client,
> +				 struct wl_resource *resource,
> +				 uint32_t position)
> +{
> +	struct desktop_shell *shell = wl_resource_get_user_data(resource);
> +
> +	if (position != DESKTOP_SHELL_PANEL_POSITION_TOP &&
> +	    position != DESKTOP_SHELL_PANEL_POSITION_BOTTOM &&
> +	    position != DESKTOP_SHELL_PANEL_POSITION_LEFT &&
> +	    position != DESKTOP_SHELL_PANEL_POSITION_RIGHT) {
> +		wl_resource_post_error(resource,
> +				       WL_DISPLAY_ERROR_INVALID_OBJECT,
> +				       "bad position argument");

Since 'resource' refers to a desktop_shell object, you should not use
a WL_DISPLAY_ERROR_* code, because it is in the wrong namespace.

We will need to add an error enum into desktop_shell. I would be fine
with just one generic error code, since this is a private protocol.

There may be similar existing abuses to fix in shell.c, but those can
be left for another patch.

> +		return;
> +	}
> +
> +	shell->panel_position = position;
> +}
> +
>  static const struct desktop_shell_interface desktop_shell_implementation = {
>  	desktop_shell_set_background,
>  	desktop_shell_set_panel,
>  	desktop_shell_set_lock_surface,
>  	desktop_shell_unlock,
>  	desktop_shell_set_grab_surface,
> -	desktop_shell_desktop_ready
> +	desktop_shell_desktop_ready,
> +	desktop_shell_set_panel_position
>  };
>  
>  static enum shell_surface_type
> @@ -5344,7 +5365,7 @@ bind_desktop_shell(struct wl_client *client,
>  	struct wl_resource *resource;
>  
>  	resource = wl_resource_create(client, &desktop_shell_interface,
> -				      MIN(version, 2), id);
> +				      MIN(version, 3), id);
>  
>  	if (client == shell->child.client) {
>  		wl_resource_set_implementation(resource,
> @@ -6265,7 +6286,7 @@ module_init(struct weston_compositor *ec,
>  		return -1;
>  
>  	if (wl_global_create(ec->wl_display,
> -			     &desktop_shell_interface, 2,
> +			     &desktop_shell_interface, 3,
>  			     shell, bind_desktop_shell) == NULL)
>  		return -1;
>  
> @@ -6279,6 +6300,8 @@ module_init(struct weston_compositor *ec,
>  
>  	shell->child.deathstamp = weston_compositor_get_time();
>  
> +	shell->panel_position = DESKTOP_SHELL_PANEL_POSITION_TOP;
> +
>  	setup_output_destroy_handler(ec, shell);
>  
>  	loop = wl_display_get_event_loop(ec->wl_display);
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index e994cdb..317f1dc 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -205,6 +205,8 @@ struct desktop_shell {
>  	struct wl_listener output_move_listener;
>  	struct wl_list output_list;
>  
> +	uint32_t panel_position;

Could this be 'enum desktop_shell_panel_position' instead of uint32_t?

> +
>  	char *client;
>  };
>  
> diff --git a/protocol/desktop-shell.xml b/protocol/desktop-shell.xml
> index fdcb17b..57c204a 100644
> --- a/protocol/desktop-shell.xml
> +++ b/protocol/desktop-shell.xml
> @@ -1,6 +1,6 @@
>  <protocol name="desktop">
>  
> -  <interface name="desktop_shell" version="2">
> +  <interface name="desktop_shell" version="3">
>      <description summary="create desktop widgets and helpers">
>        Traditional user interfaces can rely on this interface to define the
>        foundations of typical desktops. Currently it's possible to set up
> @@ -94,6 +94,24 @@
>        </description>
>      </request>
>  
> +    <!-- Version 3 additions -->
> +
> +    <enum name="panel_position">
> +      <entry name="top" value="0"/>
> +      <entry name="bottom" value="1"/>
> +      <entry name="left" value="2"/>
> +      <entry name="right" value="3"/>
> +    </enum>
> +
> +    <request name="set_panel_position" since="3">
> +      <arg name="position" type="uint"/>
> +      <description summary="set panel position">
> +        Tell the shell which side of the screen the panel is
> +        located. This is so that new windows do not overlap the panel
> +        and maximized windows maximize properly.
> +      </description>
> +    </request>
> +
>    </interface>
>  
>    <interface name="screensaver" version="1">

Looks good, I assume the compositor knows from the panel surface's size
how much it needs to reserve for it.


Thanks,
pq