<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Aug 19, 2014 at 3:58 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"><div class="">On Wed, 16 Jul 2014 11:13:06 +0300 Giulio Camuffo <<a href="mailto:giuliocamuffo@gmail.com">giuliocamuffo@gmail.com</a>> wrote: > 2014-07-15 20:39 GMT+03:00 Daniel Stone <<a href="mailto:daniel@fooishbar.org">daniel@fooishbar.org</a>>: > > Hi, > > > > > > On Tuesday, July 15, 2014, Giulio Camuffo <<a href="mailto:giuliocamuffo@gmail.com">giuliocamuffo@gmail.com</a>> wrote: > >> > >> 2014-07-14 22:31 GMT+03:00 Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>>: > >> > Guilio, > >> > Would it be better to name it wl_event_queue_roundtrip and just have it > >> > take > >> > the wl_event_queue? I guess it is sort-of a wl_display operation, but > >> > you > >> > could argue it either way. Thoughts? > >> > >> You have a point here, it makes more sense. > > > > > > TBH I'd rather steer clear of that nomenclature, since the 'queue' in an > > immediate request context implies delayed dispatch, rather than on a queue. > > I didn't realize you could read it as "queue a roundtrip on a > wl_event". I think that the meaning is quite obvious once you know > that there is a wl_event_queue type in Wayland. </div>Hi, hm, that's a tricky one. Yes, the name wl_event_queue_roundtrip() is confusing when out of context, but as the argument is a wl_event_queue, that name is the logical and unfortunate result. Another problem with wl_display_roundtrip_queue() is that it gets both wl_display and wl_event_queue as arguments. Therefore the function would need to check that the wl_event_queue indeed belongs to the given wl_display. It's just a thing one can get wrong, and it has no benefits that I see. wl_display_roundtrip_queue() taking only wl_event_queue as an argument would be strange, because the name implies a wl_display method. (Up until this point, I would have agreed with Giulio and Jason...) But, now I see, that we already have: int wl_display_dispatch_queue(struct wl_display *display, struct wl_event_queue *queue); int wl_display_dispatch_queue_pending(struct wl_display *display, struct wl_event_queue *queue); int wl_display_prepare_read_queue(struct wl_display *display, struct wl_event_queue *queue); So there is precendent, that redundant arguments are ok, and wl_display_roundtrip_queue() would indeed follow the existing scheme. Btw. I do not see anywhere, where the libwayland-client code would be checking that the wl_display and the wl_event_queue would actually be related. Would be a good thing to add more checks to the public API functions for that. I think I would go with wl_display_roundtrip_queue() here. Would you agree? </blockquote><div> </div><div>Yeah, you've convinced me. I don't like the name (and, by extension, don't like the others) but it goes better with the current API. Let's go with that. </div> <div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks, pq <div class="HOEnZb"><div class="h5"> > >> > On Mon, Jul 14, 2014 at 7:15 AM, Giulio Camuffo > >> > <<a href="mailto:giuliocamuffo@gmail.com">giuliocamuffo@gmail.com</a>> > >> > wrote: > >> >> > >> >> wl_display_roundtrip() works on the default queue. Add a parallel > >> >> wl_display_roundtrip_queue(). > >> >> --- > >> >> > >> >> v3: fixed dispatch call in place of dispatch_queue > >> >> documented the queue parameter > >> >> src/wayland-client.c | 24 +++++++++++++++++++++--- > >> >> src/wayland-client.h | 2 ++ > >> >> 2 files changed, 23 insertions(+), 3 deletions(-) > >> >> > >> >> diff --git a/src/wayland-client.c b/src/wayland-client.c > >> >> index e8aab7e..d2c1b5c 100644 > >> >> --- a/src/wayland-client.c > >> >> +++ b/src/wayland-client.c > >> >> @@ -834,15 +834,16 @@ static const struct wl_callback_listener > >> >> sync_listener = { > >> >> /** Block until all pending request are processed by the server > >> >> * > >> >> * \param display The display context object > >> >> + * \param queue The queue on which to run the roundtrip > >> >> * \return The number of dispatched events on success or -1 on failure > >> >> * > >> >> * Blocks until the server process all currently issued requests and > >> >> - * sends out pending events on all event queues. > >> >> + * sends out pending events on the event queue. > >> >> * > >> >> * \memberof wl_display > >> >> */ > >> >> WL_EXPORT int > >> >> -wl_display_roundtrip(struct wl_display *display) > >> >> +wl_display_roundtrip_queue(struct wl_display *display, struct > >> >> wl_event_queue *queue) > >> >> { > >> >> struct wl_callback *callback; > >> >> int done, ret = 0; > >> >> @@ -851,9 +852,10 @@ wl_display_roundtrip(struct wl_display *display) > >> >> callback = wl_display_sync(display); > >> >> if (callback == NULL) > >> >> return -1; > >> >> + wl_proxy_set_queue(callback, queue); > >> >> wl_callback_add_listener(callback, &sync_listener, &done); > >> >> while (!done && ret >= 0) > >> >> - ret = wl_display_dispatch(display); > >> >> + ret = wl_display_dispatch_queue(display, queue); > >> >> > >> >> if (ret == -1 && !done) > >> >> wl_callback_destroy(callback); > >> >> @@ -861,6 +863,22 @@ wl_display_roundtrip(struct wl_display *display) > >> >> return ret; > >> >> } > >> >> > >> >> +/** Block until all pending request are processed by the server > >> >> + * > >> >> + * \param display The display context object > >> >> + * \return The number of dispatched events on success or -1 on failure > >> >> + * > >> >> + * Blocks until the server process all currently issued requests and > >> >> + * sends out pending events on the default event queue. > >> >> + * > >> >> + * \memberof wl_display > >> >> + */ > >> >> +WL_EXPORT int > >> >> +wl_display_roundtrip(struct wl_display *display) > >> >> +{ > >> >> + wl_display_roundtrip_queue(display, &display->default_queue); > >> >> +} > >> >> + > >> >> static int > >> >> create_proxies(struct wl_proxy *sender, struct wl_closure *closure) > >> >> { > >> >> diff --git a/src/wayland-client.h b/src/wayland-client.h > >> >> index 2a32785..4377207 100644 > >> >> --- a/src/wayland-client.h > >> >> +++ b/src/wayland-client.h > >> >> @@ -163,6 +163,8 @@ int wl_display_dispatch_pending(struct wl_display > >> >> *display); > >> >> int wl_display_get_error(struct wl_display *display); > >> >> > >> >> int wl_display_flush(struct wl_display *display); > >> >> +int wl_display_roundtrip_queue(struct wl_display *display, > >> >> + struct wl_event_queue *queue); > >> >> int wl_display_roundtrip(struct wl_display *display); > >> >> struct wl_event_queue *wl_display_create_queue(struct wl_display > >> >> *display); > >> >> > >> >> -- > >> >> 2.0.1 > >> >> > >> >> _______________________________________________ > >> >> wayland-devel mailing list > >> >> <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > >> >> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> > >> > > >> > > >> _______________________________________________ > >> wayland-devel mailing list > >> <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > >> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> > _______________________________________________ > wayland-devel mailing list > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </div></div></blockquote></div> </div></div>