[RFC libwayland] Track protocol object versions inside wl_proxy.

Mon Oct 6 09:45:51 PDT 2014

On Mon, Oct 6, 2014 at 12:20 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Sun, 5 Oct 2014 18:48:27 -0700
> "Jasper St. Pierre" <jstpierre at mecheye.net> wrote:
>
> > One of my issues is what happens for objects that have multiple parents,
> > like wl_buffer or wl_callback. What do we do if another person wants to
> > introduce another (like the recently proposed pointer lock extension)? We
> > can recommend that users never use wl_callback directly and instead use
> > their own types instead because there's no reason *not* to, but what do
> we
> > do about wl_buffer? Or others? Do we prohibit that going forward? Do we
> > strongly word against it? Do we invent new versioning rules for those?
>
> My opinion is that:
>
> - wl_buffer is beyond repair; we need it, it cannot be extended by
>   version bumps so it is always equivalent to its version 1, and new
>   factories will arise which create wl_buffer objects. As long as
>   wl_buffer definition version is never bumped, we are ok.
>
> - wl_callback is similarly stuck to version 1, but we can discourage
>   new cases trying to use, especially if there is *any* imaginable
>   reason one might need more than "done" with an arbitary uint32 arg.
>   And we should probably discourage wl_callback in new protocols anyway.
>

> - If someone tries to introduce a new case where an object that had
>   only one factory root (the global at the root of the interface
>   factory ancestry) will now have more than one, we first just NAK the
>   patch. If the submitter claims there is no other way to make it work,
>   we have to look closer and find another way. We need to try hard to
>   not repeat the wl_buffer design, but I acknowledge that there might
>   be a case where the wl_buffer design is the only working design.
>

Agreed on all 3 points

>
> > What about use cases much like wl_buffer, which really wants a subtyping
> > mechanism? Recommend users use a role mechanism akin to wl_surface?
> Should
> > we consider adding more infrastructure and helpers in libwayland to
> support
> > "role-d objects", much like what Pekka added to Weston recently?
>
> I don't want to propose any generic rules here for now. Let's see case
> by case if a pattern emerges later.
>
> In the wl_buffer case, we could not extend wl_buffer itself even with
> associate interface (like wl_viewport is for wl_surface, or the role
> mechanism), because EGL does not normally expose wl_buffer to client
> code.
>
> > On Sun, Oct 5, 2014 at 5:30 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> > > Ping
> > >
> > > On Fri, Apr 11, 2014 at 1:36 AM, Jason Ekstrand <jason at jlekstrand.net>
> > > wrote:
> > >
> > >> On Fri, Apr 11, 2014 at 2:03 AM, Pekka Paalanen <ppaalanen at gmail.com>
> > >> wrote:
> > >>
> > >>> On Thu, 10 Apr 2014 09:42:55 -0500
> > >>> Jason Ekstrand <jason at jlekstrand.net> wrote:
> > >>>
> > >>> > On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen <
> ppaalanen at gmail.com>
> > >>> wrote:
> > >>> >
> > >>> > > Hi Jason,
> > >>> > >
> > >>> > > thanks for working on this, it does seem very useful,
> practically a
> > >>> > > mandatory feature to support.
> > >>> > >
> > >>> >
> > >>> > Hi Pekka,
> > >>> > Yeah, I've been itching to knock this out for a while.  Just
> finally
> > >>> got
> > >>> > around to it.  Comments below.
> > >>>
> > >>> ...
> > >>>
> > >>> > > On Wed, 2 Apr 2014 09:48:29 -0500
> > >>> > > Jason Ekstrand <jason at jlekstrand.net> wrote:
> > >>> > >
> > >>> > > > It's worth noting that there is one small
> backwards-compatability
> > >>> issue
> > >>> > > > here.  Namely, if the client is built against protocol stubs
> from
> > >>> an
> > >>> > > > earlier version of libwayland but links against a library built
> > >>> against a
> > >>> > > > newer version, then all objects created by the client will
> report a
> > >>> > > version
> > >>> > > > of 1.  This is because the old api uses
> > >>> wl_proxy_marshal_constructor in
> > >>> > > > wl_registry_bind so all objects will inherit the protocol
> version
> > >>> of
> > >>> > > > wl_display which is 1.  The library the client linked against
> is
> > >>> aware of
> > >>> > > > the wl_proxy_version function but has no way of knowing that
> the
> > >>> library
> > >>> > > > does not.
> > >>> > >
> > >>> > > I was about to say that wl_registry_bind() does pass the version
> to
> > >>> > > wl_proxy_marshal_constructor, but that indeed is in the request
> > >>> > > arguments and not a mandatory argument.
> > >>> > >
> > >>> > > But wl_proxy_marshal_constructor() would still have all the
> > >>> information
> > >>> > > it needs, if we special-case wl_registry.bind inside it. Ugly,
> but I
> > >>> > > guess it'd work for wl_registry. Would that make the
> > >>> > > backward-compatibility issue go away? In all other cases you
> would
> > >>> take
> > >>> > > the version from the parent wl_proxy, which you always have
> available
> > >>> > > in wl_proxy_marshal_constructor(), and the versioned variant
> would
> > >>> not
> > >>> > > be needed?
> > >>> > >
> > >>> >
> > >>> > Yes, we could do that, and I considered it.  However, it would only
> > >>> bump
> > >>> > the compatibility issue back to wl_proxy_marshal_constructor.
> Older
> > >>> code
> > >>> > that uses wl_proxy_create inside of wl_registry_bind is still in
> > >>> trouble.
> > >>> >  I don't think it's a huge savings to just bump the compatibility
> > >>> issues
> > >>> > back to 1.3 rather than 1.4/1.5.  In the long run, I don't think
> it's
> > >>> worth
> > >>> > the mess that we would create inside wl_proxy_marshal_constructor.
> > >>>
> > >>> Ok, I didn't even know to think that far back. Makes sense.
> > >>>
> > >>> > > But I guess it would still be broken on any other request that
> used
> > >>> the
> > >>> > > interfaceless format of new_id?
> > >>> > >
> > >>> >
> > >>> > Nope, that's not a problem.  The wayland-scanner program doesn't
> > >>> actually
> > >>> > special case wl_registry_bind but interfaceless new_id's in
> general.
> > >>> >  Anything else that specifies a new_id with no interface will hit
> the
> > >>> same
> > >>> > code-path and get wl_proxy_marshal_constructor_versioned.
> > >>>
> > >>> I meant if we special-case wl_registry.bind, then all other requests
> > >>> using interfaceless new_ids would still be in trouble. But yes, a
> moot
> > >>> point now.
> > >>>
> > >>> > > > One possible solution for this is to set the version of
> wl_display
> > >>> to
> > >>> > > zero
> > >>> > > > and use zero to mean "unversioned".  Then, if a library wants
> to
> > >>> use
> > >>> > > > something that's not strictly backwards-compatable, it can
> check
> > >>> for zero
> > >>> > > > and use whatever it's non-versioned fallback is.
> > >>> > >
> > >>> >
> > >>> > Thoughts on this? ^^
> > >>>
> > >>> Well... if you don't know the version, is there a difference between
> > >>> assuming it is 1, and knowning it is unknown and then assuming
> whatever?
> > >>>
> > >>> As I see it, the only benefit of knowing when you don't know, is that
> > >>> you could then explicitly assume a higher version than 1, and then
> die
> > >>> on a protocol error if you are wrong. I'm not sure if that is better
> > >>> than assuming 1 which will always work if the application only
> accepts
> > >>> that.
> > >>>
> > >>
> > >> There is a case where I would do something different on unknown vs.
> > >> version == 1.  In fact, it's the exact came case that inspired me to
> > >> actually sit down and write this.  The wl_surface.damage request,
> from the
> > >> perspective of EGL implementations, is completely broken on wl_surface
> > >> versions 2 and 3 (trying to fix it for 4).  An EGL implementation
> could
> > >> check for unknown, 2, or 3 and just do wl_surface.damage(0, 0,
> INT32_MAX,
> > >> INT32_MAX) in both eglSwapBuffers and eglSwapBuffersWithDamageEXT to
> work
> > >> around this.  Then, if the version is 2 or >= 4, they could just
> damage the
> > >> surface correctly.  It's kind of a specific example and the only
> reason why
> > >> we care is because we broke stuff at wl_surface version 2 but it's an
> > >> example.
>
> Jason, I can't think of any technical reason to reject that proposition,
> and I don't have an alternative to propose, so whatever works, I think.
>
> But, I would advice that we wait before implementing this, because we
> at Collabora got an idea that would make verifying the correctness of
> any solution easier: automated backwards compatiblity testing. I intend
> to file an enchancement bug about it. Essentially it would mean a test
> bot would be building and running weston and some test client against
> different releases of Wayland, and verifying it actually works. It
> should also include building against one version of Wayland and running
> against another. I cannot imagine any developer bothering to do that
> kind of testing manually.
>
> So how about we wait a bit and see if we can get this kind of testing
> rig set up? I can't promise it yet, but it's a too powerful idea to not
> try.
>

Fine with me.

>
>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20141006/575b5417/attachment.html>