[PATCH wayland 7/8] client: Remove misplaced documentation about main loop intergration

[Bill Spitzak]

I looked at this about a year ago, and I think both the docs and code are
suffering from multiple implementations, and this needs to be fixed.

Right now everything is pointing in circles. I believe however that there
is a base set of calls in wayland-client-core.h and all the other functions
can be defined in them. Part of the base set is I believe these:

  int wl_display_prepare_read_queue(struct wl_display *display,
                  struct wl_event_queue *queue);
  int wl_display_read_events(struct wl_display *display);
  int wl_display_dispatch_queue_pending(struct wl_display *display,
                      struct wl_event_queue *queue);
  int wl_display_flush(struct wl_display *display);

The client api looks vastly more complicated than it actually is. If a
programmer could be well aware of what the base set is, it makes wayland
understandable.

The docs should be fixed so that all the other functions are defined in
terms of the base set. The docs should never say "this is the same as F(x)"
unless F is part of the base set. And it should NEVER describe a function
that could be implemented (even if slowly) by the base set, as anything
other than "this is the same as calling A(X), B(Y)...". Functions that are
the same except they prevent other threads from interfering between the
steps should say "ame as atomically calling A(X) and then B(Y)". Or
describe A,B when there is no base function, for instance prepare_read
"atomically makes sure the queue is empty and blocks any thread that
attempts to read events into it". It is not possible for another thread to
put any events into the queue between when the test is done and when the
blocking is turned on.

The code should also be fixed to do this whenever possible. I was looking
at this some and it seems possible, but it is really confusing. Most of the
base set functions were written after the non-base functions so the
non-base ones implement what they want directly, rather than callling the
base functions. In some cases you avoid an unlock/lock of a mutex, but this
could be done by making a private inline implementation function that
assumes the lock is already held, and having the public function lock the
mutex and call that, rather than duplicating the code.


On Sun, Oct 4, 2015 at 7:41 PM, Jonas Ådahl <jadahl at gmail.com> wrote:

> On Fri, Oct 02, 2015 at 01:36:20PM -0700, Bryce Harrington wrote:
> > On Fri, Oct 02, 2015 at 05:32:58PM +0800, Jonas Ådahl wrote:
> > > There was documentation about how to integrate the display server file
> > > descriptor in the documentation about wl_display_dispatch_pending().
> > > This is not the right place to put it, and it also had incorrect usage
> > > of the API (calling wl_display_dispatch_queue() on input on an
> unrelated
> > > fd) as an example.
> >
> > Rather than just drop the misplaced docs, shouldn't they be moved to a
> > better location?
>
> I expect it has to be changed/rewritten because there are some
> misleading parts (like call dispatch_queue() on unrelated input). That
> is why I removed it for now, with the plan to add a section about
> external main loop integration elsewhere later.
>
>
> Jonas
>
> >
> > > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > > ---
> > >  src/wayland-client.c | 22 ----------------------
> > >  1 file changed, 22 deletions(-)
> > >
> > > diff --git a/src/wayland-client.c b/src/wayland-client.c
> > > index e43d19d..ca2495f 100644
> > > --- a/src/wayland-client.c
> > > +++ b/src/wayland-client.c
> > > @@ -1636,28 +1636,6 @@ wl_display_dispatch(struct wl_display *display)
> > >   * attempt to read the display fd and simply returns zero if the main
> > >   * queue is empty, i.e., it doesn't block.
> > >   *
> > > - * This is necessary when a client's main loop wakes up on some fd
> other
> > > - * than the display fd (network socket, timer fd, etc) and calls \ref
> > > - * wl_display_dispatch_queue() from that callback. This may queue up
> > > - * events in other queues while reading all data from the display fd.
> > > - * When the main loop returns from the handler, the display fd
> > > - * no longer has data, causing a call to \em poll(2) (or similar
> > > - * functions) to block indefinitely, even though there are events
> ready
> > > - * to dispatch.
> > > - *
> > > - * To proper integrate the wayland display fd into a main loop, the
> > > - * client should always call wl_display_dispatch_pending() and then
> > > - * \ref wl_display_flush() prior to going back to sleep. At that
> point,
> > > - * the fd typically doesn't have data so attempting I/O could block,
> but
> > > - * events queued up on the default queue should be dispatched.
> > > - *
> > > - * A real-world example is a main loop that wakes up on a timerfd (or
> a
> > > - * sound card fd becoming writable, for example in a video player),
> which
> > > - * then triggers GL rendering and eventually eglSwapBuffers().
> > > - * eglSwapBuffers() may call wl_display_dispatch_queue() if it didn't
> > > - * receive the frame event for the previous frame, and as such queue
> > > - * events in the default queue.
> > > - *
> > >   * \sa wl_display_dispatch(), wl_display_dispatch_queue(),
> > >   * wl_display_flush()
> > >   *
> > > --
> > > 2.4.3
> > >
> > > _______________________________________________
> > > 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/20151005/6d4b33c4/attachment-0001.html>