[PATCH wayland] client: add a public function to make a roundtrip on a custom queue

Jason Ekstrand jason at jlekstrand.net
Tue Aug 19 09:46:24 PDT 2014 

On Tue, Aug 19, 2014 at 3:58 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Wed, 16 Jul 2014 11:13:06 +0300
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
> > 2014-07-15 20:39 GMT+03:00 Daniel Stone <daniel at fooishbar.org>:
> > > Hi,
> > >
> > >
> > > On Tuesday, July 15, 2014, Giulio Camuffo <giuliocamuffo at gmail.com>
> wrote:
> > >>
> > >> 2014-07-14 22:31 GMT+03:00 Jason Ekstrand <jason at 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
> > >> > <giuliocamuffo at 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 at lists.freedesktop.org
> > >> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > >> >
> > >> >
> > >> _______________________________________________
> > >> wayland-devel mailing list
> > >> wayland-devel at lists.freedesktop.org
> > >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140819/8e9c4ef1/attachment-0001.html>

More information about the wayland-devel mailing list