From 061537b91a2aab432abeec761af2bcf80e3576ff Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 6 Dec 2024 14:24:42 -0800 Subject: [PATCH 1/3] pixman: Store strong reference to `WlBuffer` in `PixmanImage` 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. --- src/backend/renderer/pixman/mod.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/backend/renderer/pixman/mod.rs b/src/backend/renderer/pixman/mod.rs index ace6a51297ed..a39bc51869d6 100644 --- a/src/backend/renderer/pixman/mod.rs +++ b/src/backend/renderer/pixman/mod.rs @@ -25,7 +25,7 @@ use crate::{ wayland::{compositor::SurfaceData, shm}, }; #[cfg(feature = "wayland_frontend")] -use wayland_server::{protocol::wl_buffer, Resource, Weak}; +use wayland_server::protocol::wl_buffer; #[cfg(all( feature = "wayland_frontend", @@ -89,7 +89,7 @@ struct PixmanDmabufMapping { #[derive(Debug)] struct PixmanImageInner { #[cfg(feature = "wayland_frontend")] - buffer: Option>, + buffer: Option, dmabuf: Option, image: RefCell>, _flipped: bool, /* TODO: What about flipped textures? */ @@ -101,13 +101,6 @@ struct PixmanImage(Rc); impl PixmanImage { #[profiling::function] fn accessor<'l>(&'l self) -> Result, PixmanError> { - #[cfg(feature = "wayland_frontend")] - let buffer = if let Some(buffer) = self.0.buffer.as_ref() { - Some(buffer.upgrade().map_err(|_| PixmanError::BufferDestroyed)?) - } else { - None - }; - let guard = if let Some(mapping) = self.0.dmabuf.as_ref() { let dmabuf = mapping.dmabuf.upgrade().ok_or(PixmanError::BufferDestroyed)?; Some(DmabufReadGuard::new(dmabuf)?) @@ -117,7 +110,7 @@ impl PixmanImage { Ok(TextureAccessor { #[cfg(feature = "wayland_frontend")] - buffer, + buffer: self.0.buffer.clone(), image: &self.0.image, _guard: guard, }) @@ -1127,7 +1120,7 @@ impl ImportMemWl for PixmanRenderer { std::result::Result::<_, PixmanError>::Ok(image) })??; Ok(PixmanTexture(PixmanImage(Rc::new(PixmanImageInner { - buffer: Some(buffer.downgrade()), + buffer: Some(buffer.clone()), dmabuf: None, image: RefCell::new(image), _flipped: false, From 376acc04576d9b7c1ccdb482eb890bb029f10d59 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 6 Dec 2024 14:29:52 -0800 Subject: [PATCH 2/3] pixman: Call `sync_plane` when importing dmabuf 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. --- src/backend/renderer/pixman/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/renderer/pixman/mod.rs b/src/backend/renderer/pixman/mod.rs index a39bc51869d6..05f4eec095f4 100644 --- a/src/backend/renderer/pixman/mod.rs +++ b/src/backend/renderer/pixman/mod.rs @@ -739,6 +739,9 @@ impl PixmanRenderer { }); } + dmabuf.sync_plane(0, DmabufSyncFlags::START | DmabufSyncFlags::READ)?; + dmabuf.sync_plane(0, DmabufSyncFlags::END | DmabufSyncFlags::READ)?; + let image: Image<'_, '_> = unsafe { pixman::Image::from_raw_mut( format, From 493c2306716365b365e9b5a2a211cde0454e48ae Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 6 Dec 2024 15:03:27 -0800 Subject: [PATCH 3/3] gles: Evict entries from `dmabuf_cache` regardless of `wayland_frontend` Without `wayland_frontend`, this was pushing to the cache, but never removing buffers. That presumably is not correct. --- src/backend/renderer/gles/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backend/renderer/gles/mod.rs b/src/backend/renderer/gles/mod.rs index eae24a0faa36..a59dd3cd1525 100644 --- a/src/backend/renderer/gles/mod.rs +++ b/src/backend/renderer/gles/mod.rs @@ -627,7 +627,6 @@ impl GlesRenderer { #[profiling::function] fn cleanup(&mut self) { - #[cfg(feature = "wayland_frontend")] self.dmabuf_cache.retain(|entry, _tex| !entry.is_gone()); // Free outdated buffer resources // TODO: Replace with `drain_filter` once it lands