On Tue, 1 Mar 2016 11:41:21 -0800 Bill Spitzak <[email protected]> wrote:
> On Thu, Feb 18, 2016 at 4:12 AM, Pekka Paalanen <[email protected]> wrote: > > > On Tue, 16 Feb 2016 14:26:53 -0800 > > Bill Spitzak <[email protected]> wrote: > > > > > On Mon, Feb 15, 2016 at 7:48 PM, Jonas Ådahl <[email protected]> wrote: > > > > > > The proxy wrapper approach can be tested out by appling this patch: > > > > > > https://lists.freedesktop.org/archives/wayland-devel/2015-June/023054.html > > > > > > > > > Paraphrased here: > > > > > > > In short, instead of > > > > > > > > bar = wl_foo_get_bar(foo); > > > > wl_proxy_set_queue((struct wl_proxy *) bar, queue); > > > > wl_bar_add_listener(bar, ...); > > > > > > > > with this RFC a client does > > > > > > > > foo_wrapper = wl_proxy_create_wrapper((struct wl_proxy *) foo); > > > > wl_proxy_set_queue((struct wl_proxy *) foo_wrapper, queue); > > > > bar = wl_foo_get(foo_wrapper); > > > > wl_bar_add_listener(bar, ...); > > > > > > This seems unnecessarily complex. Instead the wl_proxy can be in an > > > unassigned state (identified by the id being set to a special value that > > > will never be used otherwise). This avoids the extra object, allows the > > > same code to be used even if there are different constructors for bar, > > and > > > allows other setup before the id is assigned (for instance > > > wl_bar_add_listener is not thread safe and above examples rely on the > > queue > > > being the one belonging to the current thread): > > > > > > bar = wl_bar_new(); > > > wl_proxy_set_queue((struct wl_proxy*)bar, queue); > > > wl_bar_add_listener(bar, ...); > > > wl_foo_get_bar_id(foo, bar, ...); > > > > That seems like it would work, if we were to create a completely new > > API for handling object creation. It just changes the whole C bindings > > API paradigm. > > > > Currently the C API is generated for this: > > > > struct wl_foo *foo_obj = ...; > > struct wl_bar *bar_obj = wl_foo_create_bar(foo_obj); > > > > To implement your proposal, we have to abandon wl_foo_create_bar() as > > legacy, and do this instead: > > > > struct wl_foo *foo_obj = ...; > > struct wl_bar *bar_obj; > > > > bar_obj = wl_bar_new(); > > wl_proxy_set_queue((wl_proxy *)bar_obj, queue); > > wl_bar_add_listener(bar_obj, ...); > > /* whatever else to init bar_obj */ > > wl_foo_create_barEX(foo_obj, bar_obj); > > > > Yeah, that should work. And it should avoid the problem fixed in > > > > https://cgit.freedesktop.org/wayland/wayland/commit/?id=853c24e6998f747150e4233cf41bfa8268964cc2 > > because wl_bar_new() would not allocate an id like wl_proxy_create() > > did. > > > > The problem is it's fundamentally different to the existing C bindings > > API and even the libwayland-client ABI. > > > > It might be worth investigating, though. If it is possible to migrate > > to the new API without breaking anything, it just could be worth it. > > > > It sure looks like it would be easy to emulate the current api atop this > new one, and thus migration should be possible without breaking anything. > The main problem is that functions like wl_proxy_set_queue have to work > both before and after the id is selected, however they are already doing > the "hard" version where the id is set and messages may be coming in, so > most of the fixes would be to avoid unnecessary locking when they are > called without an id. > > Looking at the generated documentation and how hard it is to figure out how > to make an object, I think an even better idea is to move the "constructor" > to the namespace most programmers expect. In the above instance it is a > "bar" function, not a "foo" function. This changes the name and argument > list of the last function in your example: > > bar_obj = wl_bar_new(); > wl_proxy_set_queue((wl_proxy *)bar_obj, queue); > wl_bar_add_listener(bar_obj, ...); > /* whatever else to init bar_obj */ > wl_bar_create(bar_obj, foo_obj, ...); > > Hopefully the wl_bar_create function will appear in the documentation where > the programmer expects, in the same page as wl_bar_destroy and all the > other wl_bar_xyz methods. I think you have forgot the fact, that objects in the protocol sense cannot be created out of thin air. They are always created by a request on an object of a *different* type. Therefore the name must be wl_foo_create_bar() for wl_foo.create_bar request, not wl_bar_create() which would mean a wl_bar.create request. Also the "_bar" in "create_bar" is already encoded in the XML spec, so you cannot just mutilate it as you want. This is not a manually implemented C function you can just arbitrarily decide how to lay out, it must be generated from the XML description. Thanks, pq > This is also nice in that no EX is needed in the name. > > There are a couple instances where there is more than one constructor, and > perhaps there already is a request called "create", in which case the best > I can think of is to call them "create1", "create2", etc. C++ could remove > this and rely on argument type overloading. > > I'm not sure if anything can be done about objects received in events. > Probably best to leave it as-is (the client is required to set the listener > in the event handler, and cannot do any other changes). A link in the > documentation from the object to the events that produce it would be nice > however.
pgpJ7eUn6PM3I.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
