<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Oct 6, 2014 at 12:20 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <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 "Jasper St. Pierre" <<a href="mailto:jstpierre@mecheye.net">jstpierre@mecheye.net</a>> 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. </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> - 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. </blockquote><div> </div><div>Agreed on all 3 points </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > 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. <div><div class="h5"> > On Sun, Oct 5, 2014 at 5:30 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > Ping > > > > On Fri, Apr 11, 2014 at 1:36 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > wrote: > > > >> On Fri, Apr 11, 2014 at 2:03 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> > >> wrote: > >> > >>> On Thu, 10 Apr 2014 09:42:55 -0500 > >>> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > >>> > >>> > On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> > >>> 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 <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> 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. </div></div>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. </blockquote><div> </div><div>Fine with me. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks, pq </blockquote></div> </div></div>