Core protocol change; [RFC v2] Wayland presentation extension

2014-02-17 Thread Pekka Paalanen
Hi,

there is one important thing in the below spec I really need to
highlight! See further below.


On Thu, 30 Jan 2014 17:35:17 +0200
Pekka Paalanen  wrote:

> Hi,
> 
> it's time for a take two on the Wayland presentation extension.
> 
> 
>   1. Introduction
> 
> The v1 proposal is here:
> http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html
> 
> In v2 the basic idea is the same: you can queue frames with a
> target presentation time, and you can get accurate presentation
> feedback. All the details are new, though. The re-design started
> from the wish to handle resizing better, preferably without
> clearing the buffer queue.
> 
> All the changed details are probably too much to describe here,
> so it is maybe better to look at this as a new proposal. It
> still does build on Frederic's work, and everyone who commented
> on it. Special thanks to Axel Davy for his counter-proposal and
> fighting with me on IRC. :-)
> 
> Some highlights:
> 
> - Accurate presentation feedback is possible also without
>   queueing.
> 
> - You can queue also EGL-based rendering, and get presentation
>   feedback if you want. Also EGL can do this internally, too, as
>   long as EGL and the app do not try to use queueing at the same time.
> 
> - More detailed presentation feedback to better allow predicting
>   future display refreshes.
> 
> - If wl_viewport is used, neither video resolution changes nor
>   surface (window) size changes alone require clearing the queue.
>   Video can continue playing even during resizes.
> 
> The protocol interfaces are arranged as
> 
>   global.method(wl_surface, ...)
> 
> just for brewity. We could as well do the factory approach:
> 
>   o = global.get_presentation(wl_surface)
>   o.method(...)
> 
> Or if we wanted to make it a mandatory part of the Wayland core
> protocol, we could just extend wl_surface itself:
> 
>   wl_surface.method(...)
> 
> and put the clock_id event in wl_compositor. That all is still
> open and fairly uninteresting, so let's concentrate on the other
> details.
> 
> The proposal refers to wl_viewport.set_source and
> wl_viewport.destination requests, which do not yet exist in the
> scaler protocol extension. These are just the wl_viewport.set
> arguments split into separate src and dst requests.
> 
> Here is the new proposal, some design rationale follows. Please,
> do ask why something is designed like it is if it puzzles you. I
> have a load of notes I couldn't clean up for this email. This
> does not even intend to completely solve all XWayland needs, but
> for everything native on Wayland I hope it is sufficient.
> 
> 
>   2. The protocol specification
> 
> 
> 
> 
>   
> Copyright © 2013-2014 Collabora, Ltd.
> 
> Permission to use, copy, modify, distribute, and sell this
> software and its documentation for any purpose is hereby granted
> without fee, provided that the above copyright notice appear in
> all copies and that both that copyright notice and this permission
> notice appear in supporting documentation, and that the name of
> the copyright holders not be used in advertising or publicity
> pertaining to distribution of the software without specific,
> written prior permission.  The copyright holders make no
> representations about the suitability of this software for any
> purpose.  It is provided "as is" without express or implied
> warranty.
> 
> THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> THIS SOFTWARE.
>   
> 
>   
> 
>   The main features of this interface are accurate presentation
>   timing feedback, and queued wl_surface content updates to ensure
>   smooth video playback while maintaining audio/video
>   synchronization. Some features use the concept of a presentation
>   clock, which is defined in presentation.clock_id event.
> 
>   Requests 'feedback' and 'queue' can be regarded as additional
>   wl_surface methods. They are part of the double-buffered
>   surface state update mechanism, where other requests first set
>   up the state and then wl_surface.commit atomically applies the
>   state into use. In other words, wl_surface.commit submits a
>   content update.
> 
>   Interface wl_surface has requests to set surface related state
>   and buffer related state, because there is no separate interface
>   for buffer state alone. Queueing requires separating the surface
>   from buffer state, and buffer state can be queued while s

Re: Core protocol change; [RFC v2] Wayland presentation extension

2014-02-17 Thread Jason Ekstrand
On Feb 17, 2014 2:35 AM, "Pekka Paalanen"  wrote:
>
> Hi,
>
> there is one important thing in the below spec I really need to
> highlight! See further below.
>
>
> On Thu, 30 Jan 2014 17:35:17 +0200
> Pekka Paalanen  wrote:
>
> > Hi,
> >
> > it's time for a take two on the Wayland presentation extension.
> >
> >
> >   1. Introduction
> >
> > The v1 proposal is here:
> >
http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html
> >
> > In v2 the basic idea is the same: you can queue frames with a
> > target presentation time, and you can get accurate presentation
> > feedback. All the details are new, though. The re-design started
> > from the wish to handle resizing better, preferably without
> > clearing the buffer queue.
> >
> > All the changed details are probably too much to describe here,
> > so it is maybe better to look at this as a new proposal. It
> > still does build on Frederic's work, and everyone who commented
> > on it. Special thanks to Axel Davy for his counter-proposal and
> > fighting with me on IRC. :-)
> >
> > Some highlights:
> >
> > - Accurate presentation feedback is possible also without
> >   queueing.
> >
> > - You can queue also EGL-based rendering, and get presentation
> >   feedback if you want. Also EGL can do this internally, too, as
> >   long as EGL and the app do not try to use queueing at the same time.
> >
> > - More detailed presentation feedback to better allow predicting
> >   future display refreshes.
> >
> > - If wl_viewport is used, neither video resolution changes nor
> >   surface (window) size changes alone require clearing the queue.
> >   Video can continue playing even during resizes.
> >
> > The protocol interfaces are arranged as
> >
> >   global.method(wl_surface, ...)
> >
> > just for brewity. We could as well do the factory approach:
> >
> >   o = global.get_presentation(wl_surface)
> >   o.method(...)
> >
> > Or if we wanted to make it a mandatory part of the Wayland core
> > protocol, we could just extend wl_surface itself:
> >
> >   wl_surface.method(...)
> >
> > and put the clock_id event in wl_compositor. That all is still
> > open and fairly uninteresting, so let's concentrate on the other
> > details.
> >
> > The proposal refers to wl_viewport.set_source and
> > wl_viewport.destination requests, which do not yet exist in the
> > scaler protocol extension. These are just the wl_viewport.set
> > arguments split into separate src and dst requests.
> >
> > Here is the new proposal, some design rationale follows. Please,
> > do ask why something is designed like it is if it puzzles you. I
> > have a load of notes I couldn't clean up for this email. This
> > does not even intend to completely solve all XWayland needs, but
> > for everything native on Wayland I hope it is sufficient.
> >
> >
> >   2. The protocol specification
> >
> > 
> > 
> >
> >   
> > Copyright © 2013-2014 Collabora, Ltd.
> >
> > Permission to use, copy, modify, distribute, and sell this
> > software and its documentation for any purpose is hereby granted
> > without fee, provided that the above copyright notice appear in
> > all copies and that both that copyright notice and this permission
> > notice appear in supporting documentation, and that the name of
> > the copyright holders not be used in advertising or publicity
> > pertaining to distribution of the software without specific,
> > written prior permission.  The copyright holders make no
> > representations about the suitability of this software for any
> > purpose.  It is provided "as is" without express or implied
> > warranty.
> >
> > THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> > AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> > ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> > THIS SOFTWARE.
> >   
> >
> >   
> > 
> >   The main features of this interface are accurate presentation
> >   timing feedback, and queued wl_surface content updates to ensure
> >   smooth video playback while maintaining audio/video
> >   synchronization. Some features use the concept of a presentation
> >   clock, which is defined in presentation.clock_id event.
> >
> >   Requests 'feedback' and 'queue' can be regarded as additional
> >   wl_surface methods. They are part of the double-buffered
> >   surface state update mechanism, where other requests first set
> >   up the state and then wl_surface.commit atomically applies the
> >   state into use. In other words, wl_surface.commit submits a
> >   content update.
> >
> >   Interface

Re: Core protocol change; [RFC v2] Wayland presentation extension

2014-02-18 Thread Pekka Paalanen
Hi Jason,

thanks for the reply. I think I wrote more like a flamebait than a
solution there. After sleeping over it and discussing with Axel, I think
changing the existing core protocol behaviour is not appropriate, and
we need to find another way around this.

More below.

On Mon, 17 Feb 2014 10:30:54 -0600
Jason Ekstrand  wrote:

> On Feb 17, 2014 2:35 AM, "Pekka Paalanen"  wrote:
> >
> > Hi,
> >
> > there is one important thing in the below spec I really need to
> > highlight! See further below.
> >
> >
> > On Thu, 30 Jan 2014 17:35:17 +0200
> > Pekka Paalanen  wrote:
> >
> > > Hi,
> > >
> > > it's time for a take two on the Wayland presentation extension.
> > >
> > >
> > >   1. Introduction
> > >
> > > The v1 proposal is here:
> > >
> http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html
> > >
> > > In v2 the basic idea is the same: you can queue frames with a
> > > target presentation time, and you can get accurate presentation
> > > feedback. All the details are new, though. The re-design started
> > > from the wish to handle resizing better, preferably without
> > > clearing the buffer queue.
> > >
> > > All the changed details are probably too much to describe here,
> > > so it is maybe better to look at this as a new proposal. It
> > > still does build on Frederic's work, and everyone who commented
> > > on it. Special thanks to Axel Davy for his counter-proposal and
> > > fighting with me on IRC. :-)
> > >
> > > Some highlights:
> > >
> > > - Accurate presentation feedback is possible also without
> > >   queueing.
> > >
> > > - You can queue also EGL-based rendering, and get presentation
> > >   feedback if you want. Also EGL can do this internally, too, as
> > >   long as EGL and the app do not try to use queueing at the same time.
> > >
> > > - More detailed presentation feedback to better allow predicting
> > >   future display refreshes.
> > >
> > > - If wl_viewport is used, neither video resolution changes nor
> > >   surface (window) size changes alone require clearing the queue.
> > >   Video can continue playing even during resizes.
> > >
> > > The protocol interfaces are arranged as
> > >
> > >   global.method(wl_surface, ...)
> > >
> > > just for brewity. We could as well do the factory approach:
> > >
> > >   o = global.get_presentation(wl_surface)
> > >   o.method(...)
> > >
> > > Or if we wanted to make it a mandatory part of the Wayland core
> > > protocol, we could just extend wl_surface itself:
> > >
> > >   wl_surface.method(...)
> > >
> > > and put the clock_id event in wl_compositor. That all is still
> > > open and fairly uninteresting, so let's concentrate on the other
> > > details.
> > >
> > > The proposal refers to wl_viewport.set_source and
> > > wl_viewport.destination requests, which do not yet exist in the
> > > scaler protocol extension. These are just the wl_viewport.set
> > > arguments split into separate src and dst requests.
> > >
> > > Here is the new proposal, some design rationale follows. Please,
> > > do ask why something is designed like it is if it puzzles you. I
> > > have a load of notes I couldn't clean up for this email. This
> > > does not even intend to completely solve all XWayland needs, but
> > > for everything native on Wayland I hope it is sufficient.
> > >
> > >
> > >   2. The protocol specification
> > >
> > > 
> > > 
> > >
> > >   
> > > Copyright © 2013-2014 Collabora, Ltd.
> > >
> > > Permission to use, copy, modify, distribute, and sell this
> > > software and its documentation for any purpose is hereby granted
> > > without fee, provided that the above copyright notice appear in
> > > all copies and that both that copyright notice and this permission
> > > notice appear in supporting documentation, and that the name of
> > > the copyright holders not be used in advertising or publicity
> > > pertaining to distribution of the software without specific,
> > > written prior permission.  The copyright holders make no
> > > representations about the suitability of this software for any
> > > purpose.  It is provided "as is" without express or implied
> > > warranty.
> > >
> > > THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > > SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > > FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > > SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > > WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> > > AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> > > ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> > > THIS SOFTWARE.
> > >   
> > >
> > >   
> > > 

Since this spec doc is a wall of text, I'll add some headings here to
make it more readable. I have these headings in my WIP branch as xml
comments. The WIP xml file is here:
http://cgit.collabora.c

Re: Core protocol change; [RFC v2] Wayland presentation extension

2014-02-18 Thread Axel Davy

Hi,

If you read the above paragraph carefully, you see that the last
sentence CHANGES EXISTING WAYLAND CORE PROTOCOL BEHAVIOUR.

The change is very subtle. It means, that without a wl_surface.attach,
the buffer state is no longer applied on commit at all! To recap, the
buffer state is:
- frame callbacks (!)
- set_buffer_transform
- set_buffer_scale
- the src_* arguments of wl_viewport.set

The reason is explained in my recent email:
http://lists.freedesktop.org/archives/wayland-devel/2014-February/013293.html

An immediate commit without an attach should not apply any buffer
state, because previous queueing of frames may have left buffer state
that is incorrect for the currently showing buffer. Immediate commits
without attach are used to update surface (and shell!) state, and
applying incorrect buffer state could cause a visual glitch.

We could claim, that this change in the core protocol exists only if
the presentation extension is advertised by the server, but that would
cause a lot more work to fix clients that get bit by this change, rather
than fix the clients to always attach a wl_buffer when they want to
change buffer state, even if it is the same buffer they just attached
and committed already.

Therefore I would like to bring the concepts of surface state and
buffer state to the core protocol, and have the core procotol define
that buffer state is applied only if there is an attach.

In the past, we already changed the wl_surface.attach semantics to not
re-attach the "current" buffer again, when there is a wl_surface.commit.
The practical consequence of that was that a commit without an attach
cannot cause any wl_buffer on this surface to become reserved and
readable by the server, and hence no (new) wl.buffer.release would be
posted either.

That means that clients already need to re-attach the same wl_buffer
again, if they changed the buffer contents and want to show the new
image. I think this should mitigate the impact of the core protocol
change.

I guess the only interesting case is the frame callback, and whether
anyone (ab)uses it without an attach.

Would this proposition be acceptable?


Thanks,
pq



As Jason pointed, it's useful to be able to do frame+commit when we 
want, outside of the part of the program doing attach+damage+commit, and 
get the frame callback.

I agree the definition of the frame callback should be clarified though.

I think a clarification of frame shouldn't break existing programs.

I don't see the point of queuing frame callbacks, and that should be 
more apparent if we change frame meaning.


I've seen you proposed the change:

-   A client can request a frame callback even without an attach,
-   damage, or any other state changes. wl_surface.commit triggers
-   display update, so the callback event will arrive after the next
-   output refresh where the surface is visible.
-

Why not replace that by:

A client can request a frame callback even without an attach,
damage, or any other state changes. The frequency at which the frame
callbacks are called by the compositor is not defined,
but the client can expect it to be similar to the refresh rate if the 
surface is focused and visible on the screen.


Axel Davy


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Core protocol change; [RFC v2] Wayland presentation extension

2014-02-19 Thread Pekka Paalanen
On Tue, 18 Feb 2014 18:34:24 +0100
Axel Davy  wrote:

> Hi,
> > If you read the above paragraph carefully, you see that the last
> > sentence CHANGES EXISTING WAYLAND CORE PROTOCOL BEHAVIOUR.
> >
> > The change is very subtle. It means, that without a wl_surface.attach,
> > the buffer state is no longer applied on commit at all! To recap, the
> > buffer state is:
> > - frame callbacks (!)
> > - set_buffer_transform
> > - set_buffer_scale
> > - the src_* arguments of wl_viewport.set
> >
> > The reason is explained in my recent email:
> > http://lists.freedesktop.org/archives/wayland-devel/2014-February/013293.html
> >
> > An immediate commit without an attach should not apply any buffer
> > state, because previous queueing of frames may have left buffer state
> > that is incorrect for the currently showing buffer. Immediate commits
> > without attach are used to update surface (and shell!) state, and
> > applying incorrect buffer state could cause a visual glitch.
> >
> > We could claim, that this change in the core protocol exists only if
> > the presentation extension is advertised by the server, but that would
> > cause a lot more work to fix clients that get bit by this change, rather
> > than fix the clients to always attach a wl_buffer when they want to
> > change buffer state, even if it is the same buffer they just attached
> > and committed already.
> >
> > Therefore I would like to bring the concepts of surface state and
> > buffer state to the core protocol, and have the core procotol define
> > that buffer state is applied only if there is an attach.
> >
> > In the past, we already changed the wl_surface.attach semantics to not
> > re-attach the "current" buffer again, when there is a wl_surface.commit.
> > The practical consequence of that was that a commit without an attach
> > cannot cause any wl_buffer on this surface to become reserved and
> > readable by the server, and hence no (new) wl.buffer.release would be
> > posted either.
> >
> > That means that clients already need to re-attach the same wl_buffer
> > again, if they changed the buffer contents and want to show the new
> > image. I think this should mitigate the impact of the core protocol
> > change.
> >
> > I guess the only interesting case is the frame callback, and whether
> > anyone (ab)uses it without an attach.
> >
> > Would this proposition be acceptable?
> >
> >
> > Thanks,
> > pq
> >
> 
> As Jason pointed, it's useful to be able to do frame+commit when we 
> want, outside of the part of the program doing attach+damage+commit, and 
> get the frame callback.
> I agree the definition of the frame callback should be clarified though.
> 
> I think a clarification of frame shouldn't break existing programs.

Yeah, not breaking existing programs is the rule. I hoped no-one would
have abused it yet.

> I don't see the point of queuing frame callbacks, and that should be 
> more apparent if we change frame meaning.

With EGL, if frame callbacks are queued and swap_interval = 1, the EGL
implementation will block the app on the next draw or swap until the
queued buffer is applied in the compositor. Is this useful or harmful?

The app is intentionally queueing with swap_interval = 1, so what
should it expect to happen?

> I've seen you proposed the change:
> 
> - A client can request a frame callback even without an attach,
> - damage, or any other state changes. wl_surface.commit triggers
> - display update, so the callback event will arrive after the next
> - output refresh where the surface is visible.
> -
> 
> Why not replace that by:
> 
> A client can request a frame callback even without an attach,
> damage, or any other state changes. The frequency at which the frame
> callbacks are called by the compositor is not defined,
> but the client can expect it to be similar to the refresh rate if the 
> surface is focused and visible on the screen.

I'll reply to this in the new thread I started with the patch "[PATCH
wayland] protocol: strike the note of frame callback triggering time",
where I proposed that.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel