[PATCH 1/2] xdg_shell: Add a new shell protocol.

[Rafael Antognolli]

Hi,

On Tue, Oct 8, 2013 at 4:07 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> Hi,
>
> sorry for a late reply, I'm still around 600 emails behind of
> wayland-devel at ...
>
>
> On Fri, 19 Jul 2013 16:42:13 -0300
> antognolli at gmail.com wrote:
>
>> From: Rafael Antognolli <rafael.antognolli at intel.com>
>>
>> xdg_shell is a protocol aimed to substitute wl_shell in the long term,
>> but will not be part of the wayland core protocol. It starts as a
>> non-stable API, aimed to be used as a development place at first, and
>> once features are defined as required by several desktop shells, we can
>> finally make it stable.
>> ---
>>  configure.ac                  |   1 +
>>  protocol/Makefile.am          |   3 +
>>  protocol/xdg-surface.xml      | 326 ++++++++++++++++++++++++++++++++++++++++++
>>  src/Makefile.am               |   2 +-
>>  src/wayland-xdg-surface.pc.in |  10 ++
>>  5 files changed, 341 insertions(+), 1 deletion(-)
>>  create mode 100644 protocol/xdg-surface.xml
>>  create mode 100644 src/wayland-xdg-surface.pc.in
>>
>> diff --git a/configure.ac b/configure.ac
>> index 536df9e..61579c8 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -141,6 +141,7 @@ AC_CONFIG_FILES([Makefile
>>                src/wayland-server.pc
>>                src/wayland-client.pc
>>                src/wayland-scanner.pc
>> +              src/wayland-xdg-surface.pc
>>                src/wayland-version.h
>>                protocol/Makefile
>>                tests/Makefile])
>> diff --git a/protocol/Makefile.am b/protocol/Makefile.am
>> index 08690b3..69b5363 100644
>> --- a/protocol/Makefile.am
>> +++ b/protocol/Makefile.am
>> @@ -1 +1,4 @@
>>  EXTRA_DIST = wayland.xml
>> +
>> +protocoldir = $(datadir)/wayland/protocol
>> +protocol_DATA = xdg-surface.xml
>> diff --git a/protocol/xdg-surface.xml b/protocol/xdg-surface.xml
>> new file mode 100644
>> index 0000000..de64018
>> --- /dev/null
>> +++ b/protocol/xdg-surface.xml
>> @@ -0,0 +1,326 @@
>> +<?xml version="1.0" encoding="UTF-8"?>
>> +<protocol name="xdg_surface">
>> +
>> +  <copyright>
>> +    Copyright © 2008-2013 Kristian Høgsberg
>> +    Copyright © 2013      Rafael Antognolli
>> +    Copyright © 2010-2013 Intel Corporation
>> +
>> +    Permission to use, copy, modify, distribute, and sell this
>> +    software and its documentation for any purpose is hereby granted
>> +    without fee, provided that the above copyright notice appear in
>> +    all copies and that both that copyright notice and this permission
>> +    notice appear in supporting documentation, and that the name of
>> +    the copyright holders not be used in advertising or publicity
>> +    pertaining to distribution of the software without specific,
>> +    written prior permission.  The copyright holders make no
>> +    representations about the suitability of this software for any
>> +    purpose.  It is provided "as is" without express or implied
>> +    warranty.
>> +
>> +    THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> +    SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> +    FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> +    SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> +    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
>> +    AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
>> +    ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
>> +    THIS SOFTWARE.
>> +  </copyright>
>> +
>> +  <interface name="xdg_shell" version="1">
>> +    <description summary="create desktop-style surfaces">
>> +      This interface is implemented by servers that provide
>> +      desktop-style user interfaces.
>> +
>> +      It allows clients to associate a xdg_surface with
>> +      a basic surface.
>> +    </description>
>> +
>> +    <request name="create_xdg_surface">
>> +      <description summary="create a shell surface from a surface">
>> +     Create a shell surface for an existing surface.
>
> I know Rob suggested naming this as "create", but I think there is more
> to think about.
>
> wl_compositor.create_surface is definitely a "create" method, because
> it creates a new independent object in both protocol and semantically
> (a new surface).
>
> Others like wl_shell.get_shell_surface, xdg_shell.create_xdg_surface,
> wl_subcompositor.get_subsurface do create a new protocol object, but
> not a new object semantically. These methods create an extension
> interface to an existing "real" object, and this extension object
> cannot function without the real object. That was my rationale for
> using "get" in the method name: I am just retrieving a handle to the
> extended interface.

OK, it makes sense. Is there consensus about this? If so, I'll just
bring it back to get_xdg_surface.

>> +     Only one shell surface can be associated with a given surface.
>> +      </description>
>> +      <arg name="id" type="new_id" interface="xdg_surface"/>
>> +      <arg name="surface" type="object" interface="wl_surface"/>
>> +    </request>
>> +  </interface>
>> +
>> +  <interface name="xdg_surface" version="1">
>> +
>> +    <description summary="desktop-style metadata interface">
>> +      An interface that may be implemented by a wl_surface, for
>> +      implementations that provide a desktop-style user interface.
>> +
>> +      It provides requests to treat surfaces like toplevel, fullscreen
>> +      or popup windows, move, resize or maximize them, associate
>> +      metadata like title and class, etc.
>> +
>> +      On the server side the object is automatically destroyed when
>> +      the related wl_surface is destroyed.  On client side,
>> +      xdg_surface_destroy() must be called before destroying
>> +      the wl_surface object.
>
> Would it make sense to change this to follow the "inert protocol
> object" idiom?
>
> Instead of making the wrong destruction order prone to fatal protocol
> errors, destroying the real object would only make the interface object
> inert until destroyed. Would this be for the better allowing easier
> code in toolkits, or for the worse due to tolerating sloppier(?) code?

Not sure if it will help much from the toolkit point of view, I think
we can live with any solution (speaking for EFL).

But I can change it to the "inert protocol object" to keep consistency
with the other extensions.

>> +    </description>
>
> I could not see a destructor request for this interface. I believe all
> interfaces should have a destructor request defined, unless there is a
> good reason to not define one (wl_callback is the rare example of
> that). Otherwise it is possible that some later change makes us realize
> we really do need it to avoid temporary leaking or other problems in the
> compositor, like happened with some of the input related objects.

Hmm... I don't know, I think I just copied what we had for wl_shell.
Should it have a destructor too?

> This extended interface (protocol object) gives the wl_surface a role
> (doesn't it?). What should happen when the extended interface is
> destroyed? Should the role be reset?
>
> Would be nice to see consistency in behaviour between different
> extension interfaces.

OK, will look at this.

> Thanks,
> pq



-- 
Rafael Antognolli