[PATCH wayland v2] protocol: define the concept of wl_surface role

[Pekka Paalanen]

On Wed, 20 Aug 2014 13:26:44 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Tue, 19 Aug 2014 10:16:34 -0700
> Jason Ekstrand <jason at jlekstrand.net> wrote:
> 
> > Pekka,
> > I have one nitpick below.  However, if we can't find a good solution, I'm
> > ok with pushing as-is.
> > 
> > Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
> > 
> > 
> > On Tue, Aug 19, 2014 at 1:41 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > 
> > > On Tue, 19 Aug 2014 10:29:21 +0300
> > > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > >
> > > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > >
> > > > Define what a role is, and what restrictions there are.
> > > >
> > > > A change to existing behaviour is that a role cannot be changed at all
> > > > once set. However, this is unlikely to cause problems, as there is no
> > > > reason to re-use wl_surfaces in clients.
> > > >
> > > > v2: give more concrete examples of roles, define losing a role, Jasper
> > > > rewrote the paragraph on how a role is set.
> > > >
> > > > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > > ---
> > > >  protocol/wayland.xml | 26 ++++++++++++++++++++++++--
> > > >  1 file changed, 24 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > > index 2d57f69..d3fcaec 100644
> > > > --- a/protocol/wayland.xml
> > > > +++ b/protocol/wayland.xml
> > > > @@ -973,8 +973,30 @@
> > > >        local coordinates of the pixel content, in case a buffer_transform
> > > >        or a buffer_scale is used.
> > > >
> > > > -      Surfaces are also used for some special purposes, e.g. as
> > > > -      cursor images for pointers, drag icons, etc.
> > > > +      A surface without a "role" is fairly useless, a compositor does
> > > > +      not know where, when or how to present it. The role is the
> > > > +      purpose of a wl_surface. Examples of roles are a cursor for a
> > > > +      pointer (as set by wl_pointer.set_cursor), a drag icon
> > > > +      (wl_data_device.start_drag), a sub-surface
> > > > +      (wl_subcompositor.get_subsurface), and a window as defined by a
> > > > +      shell protocol (e.g. wl_shell.get_shell_surface).
> > > > +
> > > > +      A surface can have only one role at a time. Initially a
> > > > +      wl_surface does not have a role. Once a wl_surface is given a
> > > > +      role, it can never be given a different role again, even if the
> > > > +      wl_surface loses the role in between.
> > >
> > 
> > I don't really like the term "looses its role".  Once a surface is a
> > "cursor surface" it is always a cursor surface, it just might not be the
> > *current* currsor surface.  How about:
> 
> Ok, we have to fix the wl_pointer spec, the spec for d&d, and review
> others. We would need to do that anyway, since at least
> wl_pointer.set_cursor does not say anything about a role right now.
> 
> > "A surface can only have one role in its lifetime.  Even if the surface
> > stops being actively used in its role, it can never be used in any other
> > role.  For instance, once you call wl_pointer.set_cursor on a surface, the
> > surface is now a "cursor surface".  Even if it stops being the active
> > cursor, the wl_surface object retains the "cursor" role cannot be used in a
> > different role."
> > 
> > Thoughts?
> 
> Ok, though I'd probably put it in other words. Does everyone agree that
> "losing a role" is not a thing at all?
> 
> That brings us back to
> http://lists.freedesktop.org/archives/wayland-devel/2014-August/016550.html
> on the implementation commentary.
> 
> It also means we have to change the wl_subsurface spec. I don't recall
> off-hand if it requires changing other specs. The cursor and drag icon
> never had a spec to begin with in this sense, and wl_shell_surface does
> not even have destructor protocol, so simply cannot lose that role.
> 
> Okay, I will write a patch where setting a role is permanent. Let's see
> how it looks like. I think someone else needs to implement it in Weston
> though, before Friday so we get it into the alpha for testing.
> Hopefully no-one was re-using wl_surfaces yet...
> 
> If that patch gets accepted, I can write the follow-ups to fix cursor,
> drag icon, and sub-surface specs.

Eh, so what do we say about the role state, when the role object gets
destroyed?

Let's say there is a wl_surface with xdg_surface, and it is a mapped
top-level floating window.

What happens when the xdg_surface object is destroyed, but the
wl_surface remains? Does the window stay up, or does it get unmapped?

After that, if the client creates a new xdg_surface for the same
wl_surface that got its old xdg_surface destroyed, what happens? Is it
an error? Does the window reappear at the same place where it left,
i.e. is the role state preserved?

Looking at the current xdg-shell.xml, role-specific state gets lost,
and the window is unmapped. So that is everything I already specified
except actually losing the role. Seems like re-creating the same role
must be allowed for wl_pointer.set_cursor to keep working, because that
request sets the role, and the role is permanent now.

Which in the end makes this change just a verbal one, to avoid the
phrase "lose the role".

Okay, maybe I can work with that.


Thanks,
pq