On Tue, Aug 19, 2014 at 3:58 AM, Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Wed, 16 Jul 2014 11:13:06 +0300 > Giulio Camuffo <giuliocamu...@gmail.com> wrote: > > > 2014-07-15 20:39 GMT+03:00 Daniel Stone <dan...@fooishbar.org>: > > > Hi, > > > > > > > > > On Tuesday, July 15, 2014, Giulio Camuffo <giuliocamu...@gmail.com> > wrote: > > >> > > >> 2014-07-14 22:31 GMT+03:00 Jason Ekstrand <ja...@jlekstrand.net>: > > >> > 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. > > 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? > 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. --Jason > > > Thanks, > pq > > > >> > On Mon, Jul 14, 2014 at 7:15 AM, Giulio Camuffo > > >> > <giuliocamu...@gmail.com> > > >> > 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 > > >> >> wayland-devel@lists.freedesktop.org > > >> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > >> > > > >> > > > >> _______________________________________________ > > >> wayland-devel mailing list > > >> wayland-devel@lists.freedesktop.org > > >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > _______________________________________________ > > wayland-devel mailing list > > wayland-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > >
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel