[PATCH wayland v2] protocol: try to clarify frame callback semantics

Sun Feb 23 10:55:06 PST 2014

On Feb 23, 2014 1:45 AM, "Pekka Paalanen" <ppaalanen at gmail.com> wrote:
>
> Hi,
>
> thanks for all the comments, it's encouraging to see a concensus
> emerging. One reply below...
>
> On Sat, 22 Feb 2014 07:50:01 -0600
> Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> > On Feb 22, 2014 2:44 AM, "Axel Davy" <axel.davy at ens.fr> wrote:
> > >
> > > Hi,
> > >
> > > I like very much the rewording proposed by Pekka.
> > >
> > > But I dislike your proposition to send frame callbacks right away if
the
> > attached buffer has been attached for a long time.
> > >
> > > Your argument seems to be that the client may manage to get to the
next
> > pageflip if the frame callback is called right away. But with this
> > argument, I don't see why this behaviour would be only for buffers
attached
> > long ago (and then we refresh at a higher frequency than the screen
refresh)
> > >
> > > Moreover we may say we can always get the two behaviours with client
side
> > code:
> > > . If we keep the current behaviour, the client could know it has
attached
> > a buffer for a long time (and that the frame callback it had put, was
> > already called), so if it wants to try to hit next pageflip, it could
just
> > commit right away with a new attach
> >
> > Yes, this is what they should be doing.  The more I think about the
frame
> > callback, the more I think that clients should just put one with every
new
> > draw and just track whether that one has been released or not.
> >
> > Unfortunately, we have to do something reasonable in the case where the
> > client requests a frame without drawing.  I don't want to restrict the
> > server too much on what it does in that case.  It may, for instance, be
> > running on some sort of refresh-on-demand hardware and have no concept
of
> > "next flip".  When the client asks for a frame callback, it is
effectively
> > asking when is a good time to repaint.  If now is a good time, the
server
> > should be allowed to say so.
> >
> > Pekka, one thing I forgot to mention that should probably be added is
that
> > we really should guarantee that frame callbacks get fired in the same
order
> > they are requested (per surface order, not global order).
>
> Ah, ok. That should be easy to implement by just being careful in
> how the callback lists are managed. Can you elaborate on a use case
> where this is especially useful?
>
> Recall that frame callbacks never get "discarded", except when the
> surface is destroyed(?). If the update it was part of gets
> overridden, the callbacks simply move on to the new update that
> overwrote the old one, which means that several update's worth of
> callbacks can trigger at the same time. I guess this is somehow
> behind your proposition?

Yes, it is. First off,  we may want to use the language "same order as they
are committed" instead of "same order as they are requested" to make all
frame callbacks on the same commit the same.

I looked at the code and it seems that Weston already does this, so it
shouldn't break anything or take any extra work to implement.  Really, it's
a nice guarantee for the client that takes basically no work to implement
in the server.  If a client normally throttles on frame callbacks and then
has to suddenly repaint or some reason, it may allow it to avoid the extra
logic for detecting the old frame callback and throwing it out.  I don't
know how big of a deal that is because they're all timestamped anyway, but
it's kind of nice.

More importantly, it cleans up some of the edge-cases.  Most of the time,
it's going to be one frame call back per application component per attach.
However, this may not always be the case.  In particular, it makes the
frame+commit case slightly better defined by providing the simple guarantee
that the callback from the frame+commit will come after the callback from
the last attach.

Maybe that's not a good reason for the aditional server-side requirement,
but it just seems to clean things up nicely.
--Jason

>
>
> Thanks,
> pq
>
>
> > > . With your proposition the client could always attach (and perhaps
> > +damage) with a frame+commit (even with the old buffer not released),
to be
> > sure to get current behaviour.
> >
> > What do you mean by "current behavior"? And why would they want it?
> >
> > >
> > > I don't think having to do an attach with the old buffer is a good
idea,
> > and I favor Pekka's proposition.
> >
> > I wasn't arguing against it. :-)
> >
> > >
> > > Axel Davy
> > >
> > > On 22/02/2014, Jason Ekstrand wrote :
> > >>
> > >> Pekka,
> > >> Sorry this e-mail took so long to send.  Not much time lately.  The
> > first time or two I read this suggested re-wording I didn't like it, but
> > now it's starting to grow on me.  I still kind of like the idea of "the
> > buffer you sent is now in use, go ahead and send the next one" but I
don't
> > know that it's that much better or that it actually changes anything.
> > >>
> > >> The big thing I'd like to leave open (and I think your change does)
is
> > the following:  Suppose a client commits a buffer and then, several
seconds
> > later (after the attached buffer was first used), the user does
something
> > that causes the client to refresh.  If it does a frame+commit without an
> > attach, the server should be able to respond immediately without waiting
> > for another pageflip.  This way the client may be able to render in time
> > for the next flip.  Sure, the client might be too slow and miss the
flip,
> > but that's really no worse than waiting before sending the frame
callback.
> > >>
> > >> Point is, it should be a compositor decision and I think you made
that
> > clear enough.
> > >>
> > >> Looks good to me.
> > >> --Jason Ekstrand
> > >>
> > >> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> > >>
> > >> On Fri, Feb 21, 2014 at 7:46 AM, Pekka Paalanen <ppaalanen at gmail.com>
> > wrote:
> > >>>
> > >>> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > >>>
> > >>> "the callback event will arrive after the next output refresh" is
wrong,
> > >>> if you interpret "output refresh" as framebuffer flip or the moment
when
> > >>> the new pixels turn into light the first time. Weston has probably
never
> > >>> worked this way.
> > >>>
> > >>> Weston triggers the frame callbacks when it submits repainting
commands
> > >>> to the GPU, which is before the framebuffer flip.
> > >>>
> > >>> Strike the incorrect claim, and the rest of the paragraph which no
> > >>> longer offers useful information.
> > >>>
> > >>> As a replacement, expand on the "throttling and driving animations"
> > >>> characteristic. The main purpose is to let clients animate at the
> > >>> display refresh rate, while avoiding drawing frames that will never
be
> > >>> presented.
> > >>>
> > >>> The new claim is that the server should give some time between
> > >>> triggering frame callbacks and repainting itself, for clients to
draw
> > >>> and commit. This is somewhat intimate with the repaint scheduling
> > >>> algorithm a compositor uses, but hopefully the right intention.
> > >>>
> > >>>
> > >>> Another point of this update is to imply, that frame callbacks
should
> > >>> not be used to count compositor repaint cycles nor monitor refresh
> > >>> cycles. It has never been guaranteed to work. Removing the mention
of
> > >>> frame callback without an attach hopefully discourages such use.
> > >>>
> > >>> v2: don't just remove a paragraph, but add useful information about
the
> > >>> request's intent.
> > >>>
> > >>> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > >>> Cc: Axel Davy <axel.davy at ens.fr>
> > >>> Cc: Jason Ekstrand <jason at jlekstrand.net>
> > >>> ---
> > >>>  protocol/wayland.xml | 26 ++++++++++++++++++--------
> > >>>  1 file changed, 18 insertions(+), 8 deletions(-)
> > >>>
> > >>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > >>> index e1edbe5..6e370ad 100644
> > >>> --- a/protocol/wayland.xml
> > >>> +++ b/protocol/wayland.xml
> > >>> @@ -1059,22 +1059,32 @@
> > >>>      </request>
> > >>>
> > >>>      <request name="frame">
> > >>> -      <description summary="request repaint feedback">
> > >>> -       Request notification when the next frame is displayed.
Useful
> > >>> -       for throttling redrawing operations, and driving animations.
> > >>> +      <description summary="request a frame throttling hint">
> > >>> +       Request a notification when it is a good time start drawing
a
> > new
> > >>> +       frame, by creating a frame callback. This is useful for
> > throttling
> > >>> +       redrawing operations, and driving animations.
> > >>> +
> > >>> +       When a client is animating on a wl_surface, it can use the
> > 'frame'
> > >>> +       request to get notified when it is a good time to draw and
> > commit the
> > >>> +       next frame of animation. If the client commits an update
> > earlier than
> > >>> +       that, it is likely that some updates will not make it to the
> > display,
> > >>> +       and the client is wasting resources by drawing too often.
> > >>> +
> > >>>         The frame request will take effect on the next
> > wl_surface.commit.
> > >>>         The notification will only be posted for one frame unless
> > >>>         requested again.
> > >>>
> > >>> +       The server must send the notifications so that a client
> > >>> +       will not send excessive updates, while still allowing
> > >>> +       the highest possible update rate for clients that wait for
the
> > reply
> > >>> +       before drawing again. The server should give some time for
the
> > client
> > >>> +       to draw and commit after sending the frame callback events
to
> > let them
> > >>> +       hit the next output refresh.
> > >>> +
> > >>>         A server should avoid signalling the frame callbacks if the
> > >>>         surface is not visible in any way, e.g. the surface is
> > off-screen,
> > >>>         or completely obscured by other opaque surfaces.
> > >>>
> > >>> -       A client can request a frame callback even without an
attach,
> > >>> -       damage, or any other state changes. wl_surface.commit
triggers a
> > >>> -       display update, so the callback event will arrive after the
next
> > >>> -       output refresh where the surface is visible.
> > >>> -
> > >>>         The object returned by this request will be destroyed by the
> > >>>         compositor after the callback is fired and as such the
client
> > must not
> > >>>         attempt to use it after that point.
> > >>> --
> > >>> 1.8.3.2
> > >>>
> > >>
> > >
>
>
> --
> Pekka Paalanen
> http://www.iki.fi/pq/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140223/464ca8e3/attachment-0001.html>