[PATCH] documentation: clarify the need for wl_surface.damage

[Pekka Paalanen]

On Tue, 3 Oct 2017 13:14:32 -0400
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Tue, Oct 03, 2017 at 10:02:03AM +0300, Pekka Paalanen wrote:
> > On Mon,  2 Oct 2017 17:39:56 +0200
> > Mahdi Khanalizadeh <mahdi at khanalizadeh.com> wrote:
> >   
> > > Add an explanation for wl_surface.attach why a wl_surface.damage request
> > > is necessary. Explicitly declare it implementation defined behaviour if the
> > > wl_surface.damage request is omitted to give the compositor some leeway
> > > on how it handles attaches.
> > > 
> > > Signed-off-by: Mahdi Khanalizadeh <mahdi at khanalizadeh.com>
> > > ---
> > >  protocol/wayland.xml | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > > index aabc7ae..6f6cc11 100644
> > > --- a/protocol/wayland.xml
> > > +++ b/protocol/wayland.xml
> > > @@ -1365,6 +1365,11 @@
> > >  	wl_buffer before receiving the wl_buffer.release event, the surface
> > >  	contents become undefined immediately.
> > >  
> > > +	Attaching a buffer should always be accompanied by a
> > > +	wl_surface.damage request to signal the compositor that the
> > > +	contents of the buffer have changed. Otherwise it is implementation
> > > +	defined whether the wl_surface.attach request has any visible effect.  
> > 
> > Hi,
> > 
> > thank you for the clarification, I assume this is as a response to our
> > IRC discussion.
> > 
> > How about phrasing it: "Attaching a buffer should always be accompanied
> > by at least one wl_surface.damage request to signal the compositor
> > which parts of the surface contents are changed."?
> > 
> > - There can be multiple damage requests to build up a complex region.
> > - It's not about buffer contents, it is about surface contents.
> > - We don't want to imply that you must always give full surface-sized
> >   damage when the buffer is swapped.
> > 
> > However, most of the new paragraph are redundant with the first
> > paragraph for the damage request. Could you instead add one more
> > sentence to the first paragraph explaining that the compositor has no
> > obligation to use the new contents outside of the given damage?  
> 
> In a way, the compositor MAY be obliged to use the new content if it
> didn't make a copy of the previous one.

Hi Jonas,

that is just a compositor implementation detail and irrelevant to the
spec. A client has no way to know or rely on any specific behaviour
outside of the damage regions.

It is not against the spec for the compositor to optimize such that it
never looks at areas that did not receive damage, e.g. updating surface
mipmaps.

> As in, it must switch to the new
> buffer and send the corrpesponding wl_buffer::release events on the old
> one, even if there was no damage, if the buffers are used in a zero-copy
> manner. If it already made a copy, there is of course no obligation to use
> the new content, as the wl_buffer::release event would probably already
> be sent after having made the copy.

A compositor might do both as well (mipmaps came to mind).

wl_buffer.release events have no implication whatsoever on what areas a
compositor might repaint or copy. The release only means that the
compositor does not read from that buffer anymore.

Damage is usually used for two purposes: updating compositor internal
copies or derived images if necessary, and ensuring screen updates
(repaint).

> I don't think we should imply that it is illegal to not attach damage
> anyhow, as there may be valid use cases. We should rather make it clear
> that there the regular semantics are still applied (regarding use count
> etc) and that it also means the the client are saying the buffers are
> equal pixel by pixel.

I agree, not sending damage should not be illegal.

One use case could be window resizing, where during interactive or
animated resize the buffers used are way larger than necessary to avoid
buffer mapping overheads, but once the resize completes, the app wants
to switch to exactly sized buffers without any change in surface
content.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20171004/c405a575/attachment.sig>