On Tue, 15 Oct 2013 07:30:54 -0300 Rafael Antognolli <[email protected]> wrote:
> Hi, > > On Tue, Oct 8, 2013 at 4:07 AM, Pekka Paalanen <[email protected]> wrote: > > Hi, > > > > sorry for a late reply, I'm still around 600 emails behind of > > wayland-devel@... > > > > > > On Fri, 19 Jul 2013 16:42:13 -0300 > > [email protected] wrote: > > > >> From: Rafael Antognolli <[email protected]> > >> > >> 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. > >> + 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". > > 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). Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
