<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 6, 2014 at 12:20 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sun, 5 Oct 2014 18:48:27 -0700<br>
<span class="">"Jasper St. Pierre" <<a href="mailto:jstpierre@mecheye.net">jstpierre@mecheye.net</a>> wrote:<br>
<br>
</span><span class="">> One of my issues is what happens for objects that have multiple parents,<br>
> like wl_buffer or wl_callback. What do we do if another person wants to<br>
> introduce another (like the recently proposed pointer lock extension)? We<br>
> can recommend that users never use wl_callback directly and instead use<br>
> their own types instead because there's no reason *not* to, but what do we<br>
> do about wl_buffer? Or others? Do we prohibit that going forward? Do we<br>
> strongly word against it? Do we invent new versioning rules for those?<br>
<br>
</span>My opinion is that:<br>
<br>
- wl_buffer is beyond repair; we need it, it cannot be extended by<br>
  version bumps so it is always equivalent to its version 1, and new<br>
  factories will arise which create wl_buffer objects. As long as<br>
  wl_buffer definition version is never bumped, we are ok.<br>
<br>
- wl_callback is similarly stuck to version 1, but we can discourage<br>
  new cases trying to use, especially if there is *any* imaginable<br>
  reason one might need more than "done" with an arbitary uint32 arg.<br>
  And we should probably discourage wl_callback in new protocols anyway.<br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- If someone tries to introduce a new case where an object that had<br>
  only one factory root (the global at the root of the interface<br>
  factory ancestry) will now have more than one, we first just NAK the<br>
  patch. If the submitter claims there is no other way to make it work,<br>
  we have to look closer and find another way. We need to try hard to<br>
  not repeat the wl_buffer design, but I acknowledge that there might<br>
  be a case where the wl_buffer design is the only working design.<br></blockquote><div><br></div><div>Agreed on all 3 points<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> What about use cases much like wl_buffer, which really wants a subtyping<br>
> mechanism? Recommend users use a role mechanism akin to wl_surface? Should<br>
> we consider adding more infrastructure and helpers in libwayland to support<br>
> "role-d objects", much like what Pekka added to Weston recently?<br>
<br>
</span>I don't want to propose any generic rules here for now. Let's see case<br>
by case if a pattern emerges later.<br>
<br>
In the wl_buffer case, we could not extend wl_buffer itself even with<br>
associate interface (like wl_viewport is for wl_surface, or the role<br>
mechanism), because EGL does not normally expose wl_buffer to client<br>
code.<br>
<div><div class="h5"><br>
> On Sun, Oct 5, 2014 at 5:30 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
> > Ping<br>
> ><br>
> > On Fri, Apr 11, 2014 at 1:36 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
> > wrote:<br>
> ><br>
> >> On Fri, Apr 11, 2014 at 2:03 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>><br>
> >> wrote:<br>
> >><br>
> >>> On Thu, 10 Apr 2014 09:42:55 -0500<br>
> >>> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> >>><br>
> >>> > On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>><br>
> >>> wrote:<br>
> >>> ><br>
> >>> > > Hi Jason,<br>
> >>> > ><br>
> >>> > > thanks for working on this, it does seem very useful, practically a<br>
> >>> > > mandatory feature to support.<br>
> >>> > ><br>
> >>> ><br>
> >>> > Hi Pekka,<br>
> >>> > Yeah, I've been itching to knock this out for a while.  Just finally<br>
> >>> got<br>
> >>> > around to it.  Comments below.<br>
> >>><br>
> >>> ...<br>
> >>><br>
> >>> > > On Wed, 2 Apr 2014 09:48:29 -0500<br>
> >>> > > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> >>> > ><br>
> >>> > > > It's worth noting that there is one small backwards-compatability<br>
> >>> issue<br>
> >>> > > > here.  Namely, if the client is built against protocol stubs from<br>
> >>> an<br>
> >>> > > > earlier version of libwayland but links against a library built<br>
> >>> against a<br>
> >>> > > > newer version, then all objects created by the client will report a<br>
> >>> > > version<br>
> >>> > > > of 1.  This is because the old api uses<br>
> >>> wl_proxy_marshal_constructor in<br>
> >>> > > > wl_registry_bind so all objects will inherit the protocol version<br>
> >>> of<br>
> >>> > > > wl_display which is 1.  The library the client linked against is<br>
> >>> aware of<br>
> >>> > > > the wl_proxy_version function but has no way of knowing that the<br>
> >>> library<br>
> >>> > > > does not.<br>
> >>> > ><br>
> >>> > > I was about to say that wl_registry_bind() does pass the version to<br>
> >>> > > wl_proxy_marshal_constructor, but that indeed is in the request<br>
> >>> > > arguments and not a mandatory argument.<br>
> >>> > ><br>
> >>> > > But wl_proxy_marshal_constructor() would still have all the<br>
> >>> information<br>
> >>> > > it needs, if we special-case wl_registry.bind inside it. Ugly, but I<br>
> >>> > > guess it'd work for wl_registry. Would that make the<br>
> >>> > > backward-compatibility issue go away? In all other cases you would<br>
> >>> take<br>
> >>> > > the version from the parent wl_proxy, which you always have available<br>
> >>> > > in wl_proxy_marshal_constructor(), and the versioned variant would<br>
> >>> not<br>
> >>> > > be needed?<br>
> >>> > ><br>
> >>> ><br>
> >>> > Yes, we could do that, and I considered it.  However, it would only<br>
> >>> bump<br>
> >>> > the compatibility issue back to wl_proxy_marshal_constructor.  Older<br>
> >>> code<br>
> >>> > that uses wl_proxy_create inside of wl_registry_bind is still in<br>
> >>> trouble.<br>
> >>> >  I don't think it's a huge savings to just bump the compatibility<br>
> >>> issues<br>
> >>> > back to 1.3 rather than 1.4/1.5.  In the long run, I don't think it's<br>
> >>> worth<br>
> >>> > the mess that we would create inside wl_proxy_marshal_constructor.<br>
> >>><br>
> >>> Ok, I didn't even know to think that far back. Makes sense.<br>
> >>><br>
> >>> > > But I guess it would still be broken on any other request that used<br>
> >>> the<br>
> >>> > > interfaceless format of new_id?<br>
> >>> > ><br>
> >>> ><br>
> >>> > Nope, that's not a problem.  The wayland-scanner program doesn't<br>
> >>> actually<br>
> >>> > special case wl_registry_bind but interfaceless new_id's in general.<br>
> >>> >  Anything else that specifies a new_id with no interface will hit the<br>
> >>> same<br>
> >>> > code-path and get wl_proxy_marshal_constructor_versioned.<br>
> >>><br>
> >>> I meant if we special-case wl_registry.bind, then all other requests<br>
> >>> using interfaceless new_ids would still be in trouble. But yes, a moot<br>
> >>> point now.<br>
> >>><br>
> >>> > > > One possible solution for this is to set the version of wl_display<br>
> >>> to<br>
> >>> > > zero<br>
> >>> > > > and use zero to mean "unversioned".  Then, if a library wants to<br>
> >>> use<br>
> >>> > > > something that's not strictly backwards-compatable, it can check<br>
> >>> for zero<br>
> >>> > > > and use whatever it's non-versioned fallback is.<br>
> >>> > ><br>
> >>> ><br>
> >>> > Thoughts on this? ^^<br>
> >>><br>
> >>> Well... if you don't know the version, is there a difference between<br>
> >>> assuming it is 1, and knowning it is unknown and then assuming whatever?<br>
> >>><br>
> >>> As I see it, the only benefit of knowing when you don't know, is that<br>
> >>> you could then explicitly assume a higher version than 1, and then die<br>
> >>> on a protocol error if you are wrong. I'm not sure if that is better<br>
> >>> than assuming 1 which will always work if the application only accepts<br>
> >>> that.<br>
> >>><br>
> >><br>
> >> There is a case where I would do something different on unknown vs.<br>
> >> version == 1.  In fact, it's the exact came case that inspired me to<br>
> >> actually sit down and write this.  The wl_surface.damage request, from the<br>
> >> perspective of EGL implementations, is completely broken on wl_surface<br>
> >> versions 2 and 3 (trying to fix it for 4).  An EGL implementation could<br>
> >> check for unknown, 2, or 3 and just do wl_surface.damage(0, 0, INT32_MAX,<br>
> >> INT32_MAX) in both eglSwapBuffers and eglSwapBuffersWithDamageEXT to work<br>
> >> around this.  Then, if the version is 2 or >= 4, they could just damage the<br>
> >> surface correctly.  It's kind of a specific example and the only reason why<br>
> >> we care is because we broke stuff at wl_surface version 2 but it's an<br>
> >> example.<br>
<br>
</div></div>Jason, I can't think of any technical reason to reject that proposition,<br>
and I don't have an alternative to propose, so whatever works, I think.<br>
<br>
But, I would advice that we wait before implementing this, because we<br>
at Collabora got an idea that would make verifying the correctness of<br>
any solution easier: automated backwards compatiblity testing. I intend<br>
to file an enchancement bug about it. Essentially it would mean a test<br>
bot would be building and running weston and some test client against<br>
different releases of Wayland, and verifying it actually works. It<br>
should also include building against one version of Wayland and running<br>
against another. I cannot imagine any developer bothering to do that<br>
kind of testing manually.<br>
<br>
So how about we wait a bit and see if we can get this kind of testing<br>
rig set up? I can't promise it yet, but it's a too powerful idea to not<br>
try.<br></blockquote><div><br></div><div>Fine with me.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div></div>