[PATCH 1/2] xdg_shell: Add a new shell protocol.
Rafael Antognolli
antognolli at gmail.com
Tue Oct 15 15:20:57 CEST 2013
On Tue, Oct 15, 2013 at 8:28 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Tue, 15 Oct 2013 07:30:54 -0300
> Rafael Antognolli <antognolli at gmail.com> wrote:
>
>> 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.
>
> I have only my opinion, and Rob disagreed, don't know about
> others or concensus.
So I'm waiting for more feedback. I'm keeping get_xdg_surface for now.
>> >> + 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?
>
> In hindsight, it probably should. I guess roles were originally thought
> to be "set once and forever" with the rationale that wl_surfaces are
> cheap to create, so it wasn't really needed at the time. Nowadays when
> we are getting an increasing number of extensions to wl_surface and
> more state, it is becoming heavier. One could also argue whether
> re-purposing a surface is needed at all, or a good practice.
>
> Actually, we probably did not even have the concept of a role when
> wl_shell_surface was designed.
>
> Personally I feel that the rule "define a destructor request unless
> there is a good reason not to" is a safe guideline. Again, just my
> opinion.
>
> At least every protocol object should have a way to get destroyed,
> other than the client disconnecting. I bet everyone can agree on that,
> but I would also add "without introducing tricky error conditions".
OK, makes sense, I'm adding it 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.
>
> As a rule of thumb, anything that sets weston_surface::configure is a
> role. That should let you find them all, including pointer cursors and
> dnd icons (IIRC these roles are reversable).
Right, but what would happen if hte role is reset? From the docs,
subsurfaces are unmapped if the interface is destroyed. Should we do
the same in the xdg_surface case?
--
Rafael Antognolli
More information about the wayland-devel
mailing list