wl_surface::attach(NULL) release previous buffer?

[Pekka Paalanen]

On Fri, 15 Sep 2023 14:12:25 +0100
John Cox <jc at kynesim.co.uk> wrote:

> Hi
> 
> >On Thu, 14 Sep 2023 12:24:42 +0100
> >John Cox <jc at kynesim.co.uk> wrote:
> >  
> >> Hi
> >> 
> >> A, hopefully, simple question - should I expect a wl_buffer::release
> >> event from the buffer previously committed to a surface after I've
> >> attached (and invalidated & committed) a NULL buffer to the surface? it
> >> doesn't seem to happen for me (I have WAYLAND_DEBUG=1 logs showing it
> >> not happening).  
> >
> >Theoretically yes, because committing a NULL buffer should delete any
> >content of the surface, so there is no reason for the compositor to
> >keep the buffer.
> >
> >It may not be immediate, though, so a simple roundtrip is not
> >guaranteed to be enough. An unfinished output frame may cause the
> >compositor to still need the previous buffer for a moment longer, and
> >the compositor might need to paint one more output refresh to ensure
> >the buffer is no longer in use.
> >
> >Then there are also client related considerations like if the
> >wl_surface is actually a synchronized sub-surface, then committing a
> >NULL buffer won't take effect until the parent has been committed and
> >taken effect.
> >
> >Another thing is that some wl_surface roles might forbid committing a
> >NULL buffer, but you'd notice that from a protocol error. I don't
> >remember what roles that might be.
> >  
> >> If I shouldn't expect a release - when is it safe to reuse/free the
> >> buffer storage?  
> >
> >To re-use the storage, you do need to receive wl_buffer.release, or an
> >equivalent if you are using an extension like explicit sync.  
> 
> Many thanks for your comprehensive answer. I believe that I've accounted
> for all of the special cases you mention: I've waited long enough via
> artificial debug delay, its on an async subplane, the window is visibly
> black and it works as expected with an egl backend rether the the pixman
> one. I know that our pixman backend has been patched but I wanted to
> check what I was expecting before trying to raise an issue with them.

Right, sounds like a compositor bug. Also the wl_surface should not become
black, it should disappear altogether. Or maybe you mean the parent
wl_surface remains and is black, that's fine.

> As an aside one of the more disapointing lines I read in the API docn
> was "If a pending wl_buffer has been committed to more than one
> wl_surface, the delivery of wl_buffer.release events becomes undefined."
> Yes there may be ways of working round this but it feels wrong.

Right, the design simply didn't consider the multiple surfaces case.
Ideally the release event would have been a wl_callback just like
wl_surface.frame is. IIRC the explicit sync extension fixes this.


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/20230915/63666a69/attachment.sig>