[PATCH wayland v4] protocol: Add wl_surface.damage_buffer
Jason Ekstrand
jason at jlekstrand.net
Fri Nov 27 09:33:41 PST 2015
On Nov 27, 2015 1:35 AM, "Pekka Paalanen" <ppaalanen at gmail.com> wrote:
>
> On Fri, 27 Nov 2015 16:47:19 +0800
> Jonas Ådahl <jadahl at gmail.com> wrote:
>
> > On Thu, Nov 26, 2015 at 03:44:12PM -0600, Derek Foreman wrote:
> > > wl_surface.damage uses surface local co-ordinates.
> > >
> > > Buffer scale and buffer transforms came along, and EGL surfaces
> > > have no understanding of them.
> > >
> > > Theoretically, clients pass damage rectangles - in Y-inverted surface
> > > co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation
> > > passed them on to wayland. However, for this to work the EGL
> > > implementation must be able to flip those rectangles into the space
> > > the compositor is expecting, but it's unable to do so because it
> > > doesn't know the height of the transformed buffer.
> > >
> > > So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers
> > > has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function.
> > >
> > > wl_surface.damage_buffer allows damage to be registered on a surface
> > > in buffer co-ordinates, avoiding this problem.
> > >
> > > Credit where it's due, these ideas are not entirely my own:
> > > Over a year ago the idea of changing damage co-ordinates to buffer
> > > co-ordinates was suggested (by Jason Ekstrand), and it was at least
> > > partially rejected and abandoned. At the time it was also suggested
> > > (by Pekka Paalanen) that adding a new wl_surface.damage_buffer request
> > > was another option.
> > >
> > > This will eventually resolve:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=78190
> > > by making the problem irrelevant.
> > >
> > > Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> >
> > Reviewed-by: Jonas Ådahl <jadahl at gmail.com>, with a couple of comments
> > below.
>
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> ...
>
> > > ---
> > >
> > > Changes from v3:
> > > replaced the last paragraph with Pekka's version
> > > (with one tiny grammar change)
> > >
> > > protocol/wayland.xml | 49
+++++++++++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 47 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > index f9e6d76..4e97bb9 100644
> > > --- a/protocol/wayland.xml
> > > +++ b/protocol/wayland.xml
> > > @@ -176,7 +176,7 @@
> > > </event>
> > > </interface>
> > >
> > > - <interface name="wl_compositor" version="3">
> > > + <interface name="wl_compositor" version="4">
> > > <description summary="the compositor singleton">
> > > A compositor. This object is a singleton global. The
> > > compositor is in charge of combining the contents of multiple
> > > @@ -986,7 +986,7 @@
> > > </event>
> > > </interface>
> > >
> > > - <interface name="wl_surface" version="3">
> > > + <interface name="wl_surface" version="4">
> > > <description summary="an onscreen surface">
> > > A surface is a rectangular area that is displayed on the
screen.
> > > It has a location, size and pixel contents.
> > > @@ -1109,6 +1109,10 @@
> > > wl_surface.commit assigns pending damage as the current damage,
> > > and clears pending damage. The server will clear the current
> > > damage as it repaints the surface.
> > > +
> > > + Alternatively, damage can be posted with wl_surface.damage_buffer
> > > + which uses buffer co-ordinates instead of surface co-ordinates,
> > > + and is probably the preferred and intuitive way of doing this.
> > > </description>
> > >
> > > <arg name="x" type="int"/>
> > > @@ -1325,6 +1329,47 @@
> > > </description>
> > > <arg name="scale" type="int"/>
> > > </request>
> > > +
> > > + <!-- Version 4 additions -->
> >
> > Bikeshed mode enabled: missing empty line between "Version N additions"
> > and actual additions.
> >
> > > + <request name="damage_buffer" since="4">
> > > + <description summary="mark part of the surface damaged using
buffer co-ordinates">
> > > + This request is used to describe the regions where the pending
> > > + buffer is different from the current surface contents, and where
> > > + the surface therefore needs to be repainted. The compositor
> > > + ignores the parts of the damage that fall outside of the surface.
> > > +
> > > + Damage is double-buffered state, see wl_surface.commit.
> > > +
> > > + The damage rectangle is specified in buffer coordinates.
> > > +
> > > + The initial value for pending damage is empty: no damage.
> > > + wl_surface.damage_buffer adds pending damage: the new pending
> > > + damage is the union of old pending damage and the given rectangle.
> > > +
> > > + wl_surface.commit assigns pending damage as the current damage,
> > > + and clears pending damage. The server will clear the current
> > > + damage as it repaints the surface.
> > > +
> > > + This request differs from wl_surface.damage in only one way - it
> > > + takes damage in buffer co-ordinates instead of surface local
> > > + co-ordinates. While this generally is more intuitive than surface
> > > + co-ordinates, it is especially desirable when using wp_viewport
> > > + or when a drawing library (like EGL) is unaware of buffer scale
> > > + and buffer transform.
> > > +
> > > + Since it is impossible to convert between buffer and surface
> > > + co-ordinates until all the possible parameters affecting the
> > > + surface size and the buffer-surface mapping are known at
> > > + wl_surface.commit time, damage from wl_surface.damage and
> > > + wl_surface.damage_buffer must be accumulated separately in a
> > > + compositor and combined during wl_surface.commit at the earliest.
> >
> > As Jason already pointed out, this looks out of place. With some
> > imagination, it might also not entirely true. A compositor may put
> > damage "together" in a single list, with "transform this way" flags next
> > to regions in different coordinate spaces, and then later process and
> > combine at commit when all information is available. Sure, it makes
> > little sense to do things this way, but it wouldn't be wrong while
> > still, depending on how interpreted, not really doing what the above
> > paragraph said.
>
> Yeah. It's implementation guideline for compositor writers. Maybe it
> should have started by stating that, so that client writers don't get
> confused.
>
> I suppose we all agree that in lack of better wording for the moment,
> this is good to land. If someone comes up with a better wording, we can
> patch that later.
How about something like this:
Note: Because buffer transformation changes and damage requests may be
interleaved in the protocol stream, It is impossible to determine the
actual mapping between surface and buffer damage until wl_surface.commit
time. Therefore, compositors wishing to take both kinds of damage into
account will have to accumulate damage from the two requests separately and
only transform from one to the other after receiving the wl_surface.attach.
That says more or less the same thing but puts the focus on the protocol
rather than implantation. It also leaves open the possibility of different
compositor implementations that don't bother trying to combine the two.
--Jason
> I restored a couple of CCs, and since 26th was thanksgiving in the US,
> maybe this should land early next week if no-one objects.
>
>
> Thanks,
> pq
>
> > > + </description>
> > > +
> > > + <arg name="x" type="int"/>
> > > + <arg name="y" type="int"/>
> > > + <arg name="width" type="int"/>
> > > + <arg name="height" type="int"/>
> > > + </request>
> > > </interface>
> > >
> > > <interface name="wl_seat" version="5">
> > > --
> > > 2.6.2
> > >
> > > _______________________________________________
> > > 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/20151127/4821f281/attachment-0001.html>
More information about the wayland-devel
mailing list