[RFC libwayland] Track protocol object versions inside wl_proxy.

[Jason Ekstrand]

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


> It looks like a trade-off between an "unknown method" protocol error /
> events that never come, and the application complaining the server is
> too old, when things go wrong.
>
> I can't say.
>
>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140411/5a1ef0cd/attachment.html>