Pixman (and gles) renderer fixes #1602
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a weak `WlBuffer` is used, it isn't possible to read the contents of a destroyed shm buffer. But the Wayland protocol specifies that destroying an attached buffer shouldn't change the contents of the surface. A `wayland_server::Weak` will not upgrade to a strong reference for a destroyed object even if other strong references exist, like the strong reference to the `WlBuffer` in `WaylandSurfaceRenderElement`. This fixes an error when a buffer is destroyed by libdecor, which crashed a version of Anvil patched to use pixman.
Previously, `PixmanRenderer::import_dmabuf` didn't error for an fd that isn't a dmabuf. Which caused an error later when trying to use this ioctl. I'm not aware of a better way to test this, so we can just add a `sync_plane` here.
Without `wayland_frontend`, this was pushing to the cache, but never removing buffers. That presumably is not correct.
ids1024
force-pushed
the
pixman-buffer
branch
from
f91320a to
493c230
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an error I noticed in #1497 where it the renderer couldn't access an shm buffer that had been destroyed. I believe using a strong reference to the
WlBufferwon't cause issues here.For dmabuf buffers, retaining a strong
Dmabufwould not be correct, since the caching is based on strong references to theDmabuf. I've tested that the same error doesn't occur with dmabufs though. I guess as long as there is a strong reference in theWaylandSurfaceRenderElement, the user data of theWlBufferhas a strong reference to theDmabuf.So no corresponding change is needed for dmabufs, though perhaps its worth thinking if caching could be done differently. It could cache by
Weak<WlBuffer>instead ofWeakDmabufby implementingImportDmaWl. Though usingWeakDmabufallows this to be used with dmabufs that aren't from Wayland. (And it could be nice to disentangle renderers fromwayland_backend, but that would also require anShmbuftype or such, instead of just aWlBufferreference...)Doing tests with dmabufs, I also noticed that
PixmanRenderer::import_dmabuf()succeeds for non dmabuf fds (then anvil crashed later when trying to usesync_plane). Seems good to test this inimport_dmabuf()and error there, and I can't think of a better way at the moment than callingsync_planethere.Comparing the gles renderer, I also noticed it was pushing to
dmabuf_cacheregardless ofwayland_frontend, but only evicting from the cache with that feature. So I removed that#[cfg].