Add back wlr_buffer

[emersion]

Running rootston through valgrind revealed that when resizing xwayland is destroying buffers before they are released. So merging #1060 fixes the invalid reads.

This PR is just a combination of #1050 and #1060. Fixes #1061.

cc @Ongy @Timidger @ammen99: this is a breaking change. Here's the sway PR: swaywm/sway#2135

[emersion]

The last commit stops destroying textures when the wl_buffer is destroyed. I still want to retain the same wlr_buffer ABI to allow future buffer types that don't work this way to work (ie. I still don't want to make promises about wlr_buffer.texture being non-NULL). Here are the #wayland logs:

09:27 <emersion> btw, i was wondering: how does the close animation in weston work?
09:28 <emersion> do you copy the texture when the client unmaps?
09:37 <pq> emersion, I don't think weston does a copy at that point. wl_shm is already copied, and for others we rely on the buffer reference (EGL, dmabuf) to keep the buffer around. Of course, that kind of fails if the client goes and re-uses the same storage, but if it does that, then there is no guarantee a copy would finish before that either.
09:38 <emersion> so EGL will be in charge to keep the wl_buffer around, even if the client exits?
09:38 <pq> also, re-using the same storage is a protocol violation, because the client has not received a wl_buffer.release at that point
09:39 <pq> maybe not the wl_buffer, but the underlying memory
09:39 <pq> I forget the details around wl_buffer destroy, though.
09:40 <emersion> hmm, what happens when the wl_buffer is destroyed? should we assume that the underlying memory is not valid anymore?
09:42 <emersion> (also, there are two ways this can happen - either when the client sends wl_buffer.destroy before receiving wl_buffer.release, either the client disconnects)
09:42 <pq> as long as you keep the GL texture or EGLImage or dmabuf fd or something, the memory will remain usable
09:42 <pq> IIRC
09:42 <emersion> ah
09:43 <pq> but in principle, when the wl_buffer gets destroyed, a compositor should go and destroy everything about it.
09:43 <emersion> yeah, that's what i do currently - when the wl_buffer is destroyed, also destroy the imported texture
09:44 <emersion> but that prevents to do stuff like close animations
09:44 <pq> it seems Weston keeps something around, because e.g. quitting weston-simple-egl returns to the shell before the fade ends, if I can see right
09:44 <emersion> yeah, i think so
09:45 <pq> the only real concern is the memory re-use though, the EGL or whatever reference will keep the memory allocated
09:45 <emersion> is dmabuf managing itself its buffers? (with a refcount or something)
09:46 <emersion> okay, so it should be fine to keep the imported texture when the wl_buffer is destroyed then
09:46 <pq> yes, the memory underlying a dmabuf fd is reference counted, each fd holds a reference, and so do many other handles
09:47 <emersion> ah, and if i import a dma-buf and then close the FDs, do i still hold a reference?
09:47 <emersion> ah, okay
09:47 <emersion> all right
09:47 <pq> yes, I believe so. Even just a GL texture holds a reference, even if the texture has already been deleted but the GPU is not finished yet
09:48 <emersion> i get it, thanks!
09:48 <pq> a problem only arises if the client re-uses the memory directly, without making a new allocation.
09:49 <pq> so what weston does is not exactly right I think, but it's pretty rare to not get away with it
09:57 <zubzub> so a troll client could show the dying scene from platoon when his surface is closed?
09:57 <zubzub> interesting
09:57 <pq> yup!
09:58 <pq> although, nothing prevents it from showing it before closing either :-P
09:58 <emersion> :^)
09:58 <emersion> less scary i guess
09:59 <pq> the practical risk is more like seeing garbage during the closing animation

[Ongy]

Should the comment mention that it's a protocol violation on the client side, to reuse the buffers in this case?

[emersion]

Well, it's not clear that it's a protocol violation.

[Ongy]

The texture isn't stored anywhere at this point, does it need to be freed?

[emersion]

Good catch. Fixed.

[ddevault]

WFM

[ddevault]

Thanks!