Remove EGLBufferReader from buffer_type and buffer_dimensions api

anvil/src/drawing.rs

@@ -5,7 +5,7 @@ use std::cell::RefCell;
 use slog::Logger;
 use smithay::{
     backend::{
-        renderer::{BufferType, Frame, ImportAll, Renderer, Texture, Transform},
+        renderer::{buffer_type, BufferType, Frame, ImportAll, Renderer, Texture, Transform},
         SwapBuffersError,
     },
     reexports::wayland_server::protocol::{wl_buffer, wl_surface},
@@ -96,7 +96,7 @@ where
 
                         match renderer.import_buffer(&buffer, Some(&attributes), &damage) {
                             Some(Ok(m)) => {
-                                if let Some(BufferType::Shm) = renderer.buffer_type(&buffer) {
+                                if let Some(BufferType::Shm) = buffer_type(&buffer) {
                                     buffer.release();
                                 }
                                 data.texture = Some(Box::new(BufferTextures {

anvil/src/shell.rs

@@ -4,8 +4,6 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-#[cfg(feature = "egl")]
-use smithay::backend::egl::display::EGLBufferReader;
 use smithay::{
     backend::renderer::buffer_dimensions,
     reexports::{
@@ -37,7 +35,7 @@ use smithay::{
 };
 
 use crate::{
-    state::{AnvilState, Backend},
+    state::AnvilState,
     window_map::{Kind as SurfaceKind, PopupKind, WindowMap},
 };
 
@@ -324,10 +322,7 @@ pub struct ShellHandles {
     pub window_map: Rc<RefCell<MyWindowMap>>,
 }
 
-pub fn init_shell<BackendData: Backend + 'static>(
+pub fn init_shell<BackendData: 'static>(display: &mut Display, log: ::slog::Logger) -> ShellHandles {
-    display: &mut Display,
-    log: ::slog::Logger,
-) -> ShellHandles {
     // Create the compositor
     let (compositor_token, _, _) = compositor_init(
         display,
@@ -335,19 +330,7 @@ pub fn init_shell<BackendData: Backend + 'static>(
             SurfaceEvent::Commit => {
                 let anvil_state = ddata.get::<AnvilState<BackendData>>().unwrap();
                 let window_map = anvil_state.window_map.as_ref();
-                #[cfg(feature = "egl")]
+                surface_commit(&surface, ctoken, &*window_map)
-                {
-                    surface_commit(
-                        &surface,
-                        ctoken,
-                        anvil_state.backend_data.egl_reader().as_ref(),
-                        &*window_map,
-                    )
-                }
-                #[cfg(not(feature = "egl"))]
-                {
-                    surface_commit(&surface, ctoken, &*window_map)
-                }
             }
         },
         log.clone(),
@@ -858,7 +841,6 @@ impl SurfaceData {
 fn surface_commit(
     surface: &wl_surface::WlSurface,
     token: CompositorToken<Roles>,
-    #[cfg(feature = "egl")] egl_reader: Option<&EGLBufferReader>,
     window_map: &RefCell<MyWindowMap>,
 ) {
     #[cfg(feature = "xwayland")]
@@ -932,14 +914,7 @@ fn surface_commit(
         match attributes.buffer.take() {
             Some(BufferAssignment::NewBuffer { buffer, .. }) => {
                 // new contents
-                #[cfg(feature = "egl")]
+                next_state.dimensions = buffer_dimensions(&buffer);
-                {
-                    next_state.dimensions = buffer_dimensions(&buffer, egl_reader);
-                }
-                #[cfg(not(feature = "egl"))]
-                {
-                    next_state.dimensions = buffer_dimensions(&buffer);
-                }
                 next_state.buffer = Some(buffer);
             }
             Some(BufferAssignment::Removed) => {

anvil/src/state.rs

@@ -20,8 +20,6 @@ use smithay::{
     },
 };
 
-#[cfg(feature = "egl")]
-use smithay::backend::egl::display::EGLBufferReader;
 #[cfg(feature = "xwayland")]
 use smithay::xwayland::{XWayland, XWaylandEvent};
 
@@ -177,6 +175,4 @@ impl<BackendData: Backend + 'static> AnvilState<BackendData> {
 
 pub trait Backend {
     fn seat_name(&self) -> String;
-    #[cfg(feature = "egl")]
-    fn egl_reader(&self) -> Option<EGLBufferReader>;
 }

anvil/src/udev.rs

@@ -60,7 +60,6 @@ use smithay::{
 use smithay::{
     backend::{
         drm::DevPath,
-        egl::display::EGLBufferReader,
         renderer::{ImportDma, ImportEgl},
         udev::primary_gpu,
     },
@@ -91,12 +90,6 @@ pub struct UdevData {
 }
 
 impl Backend for UdevData {
-    #[cfg(feature = "egl")]
-    fn egl_reader(&self) -> Option<EGLBufferReader> {
-        self.backends
-            .values()
-            .find_map(|backend| backend.renderer.borrow().egl_reader().cloned())
-    }
     fn seat_name(&self) -> String {
         self.session.seat()
     }

anvil/src/winit.rs

@@ -1,13 +1,7 @@
 use std::{cell::RefCell, rc::Rc, sync::atomic::Ordering, time::Duration};
 
 #[cfg(feature = "egl")]
-use smithay::{
+use smithay::{backend::renderer::ImportDma, wayland::dmabuf::init_dmabuf_global};
-    backend::{
-        egl::display::EGLBufferReader,
-        renderer::{ImportDma, ImportEgl},
-    },
-    wayland::dmabuf::init_dmabuf_global,
-};
 use smithay::{
     backend::{input::InputBackend, renderer::Frame, winit, SwapBuffersError},
     reexports::{
@@ -25,14 +19,9 @@ use slog::Logger;
 use crate::drawing::*;
 use crate::state::{AnvilState, Backend};
 
-pub struct WinitData(Rc<RefCell<winit::WinitGraphicsBackend>>);
+pub struct WinitData;
 
 impl Backend for WinitData {
-    #[cfg(feature = "egl")]
-    fn egl_reader(&self) -> Option<EGLBufferReader> {
-        self.0.borrow_mut().renderer().egl_reader().cloned()
-    }
-
     fn seat_name(&self) -> String {
         String::from("winit")
     }
@@ -75,7 +64,7 @@ pub fn run_winit(
     let mut state = AnvilState::init(
         display.clone(),
         event_loop.handle(),
-        WinitData(renderer.clone()),
+        WinitData,
         log.clone(),
     );
 

src/backend/egl/display.rs

@@ -5,6 +5,8 @@ use std::ffi::CStr;
 use std::mem::MaybeUninit;
 use std::ops::Deref;
 use std::sync::Arc;
+#[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))]
+use std::sync::{Mutex, Weak};
 
 use libc::c_void;
 use nix::libc::c_int;
@@ -24,6 +26,11 @@ use crate::backend::egl::{
 #[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))]
 use crate::backend::egl::{BufferAccessError, EGLBuffer, Format};
 
+#[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))]
+lazy_static! {
+    pub(crate) static ref BUFFER_READER: Mutex<Option<WeakBufferReader>> = Mutex::new(None);
+}
+
 /// Wrapper around [`ffi::EGLDisplay`](ffi::egl::types::EGLDisplay) to ensure display is only destroyed
 /// once all resources bound to it have been dropped.
 #[derive(Debug)]
@@ -535,7 +542,18 @@ impl EGLDisplay {
             ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _)
         })
         .map_err(Error::OtherEGLDisplayAlreadyBound)?;
-        Ok(EGLBufferReader::new(self.display.clone(), display.c_ptr()))
+        let reader = EGLBufferReader::new(self.display.clone(), display.c_ptr());
+        let mut global = BUFFER_READER.lock().unwrap();
+        if global.as_ref().and_then(|x| x.upgrade()).is_some() {
+            warn!(
+                self.logger,
+                "Double bind_wl_display, smithay does not support this, please report"
+            );
+        }
+        *global = Some(WeakBufferReader {
+            display: Arc::downgrade(&self.display),
+        });
+        Ok(reader)
     }
 }
 
@@ -663,6 +681,24 @@ pub struct EGLBufferReader {
     wayland: Option<Arc<*mut wl_display>>,
 }
 
+#[cfg(feature = "use_system_lib")]
+pub(crate) struct WeakBufferReader {
+    display: Weak<EGLDisplayHandle>,
+}
+
+#[cfg(feature = "use_system_lib")]
+impl WeakBufferReader {
+    pub fn upgrade(&self) -> Option<EGLBufferReader> {
+        Some(EGLBufferReader {
+            display: self.display.upgrade()?,
+            wayland: None,
+        })
+    }
+}
+
+#[cfg(feature = "use_system_lib")]
+unsafe impl Send for EGLBufferReader {}
+
 #[cfg(feature = "use_system_lib")]
 impl EGLBufferReader {
     fn new(display: Arc<EGLDisplayHandle>, wayland: *mut wl_display) -> Self {
@@ -825,7 +861,7 @@ impl EGLBufferReader {
 #[cfg(feature = "use_system_lib")]
 impl Drop for EGLBufferReader {
     fn drop(&mut self) {
-        if let Ok(wayland) = Arc::try_unwrap(self.wayland.take().unwrap()) {
+        if let Some(wayland) = self.wayland.take().and_then(|x| Arc::try_unwrap(x).ok()) {
             if !wayland.is_null() {
                 unsafe {
                     // ignore errors on drop

src/backend/renderer/mod.rs

@@ -25,7 +25,10 @@ use crate::backend::allocator::{dmabuf::Dmabuf, Format};
     feature = "backend_egl",
     feature = "use_system_lib"
 ))]
-use crate::backend::egl::{display::EGLBufferReader, Error as EglError};
+use crate::backend::egl::{
+    display::{EGLBufferReader, BUFFER_READER},
+    Error as EglError,
+};
 
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
 /// Possible transformations to two-dimensional planes
@@ -404,27 +407,6 @@ pub trait ImportAll: Renderer {
         surface: Option<&SurfaceAttributes>,
         damage: &[Rectangle],
     ) -> Option<Result<<Self as Renderer>::TextureId, <Self as Renderer>::Error>>;
-
-    /// Returns the *type* of a wl_buffer
-    ///
-    /// *Note*: Different to [`buffer_type`] this variant uses its internal `EGLBufferReader`, if
-    /// - the underlying Renderer supports `ImportEgl`,
-    /// - smithay was compiled with the `backend_egl` and `use_system_lib`,
-    /// - and the underlying Renderer bound via [`ImportEgl::bind_wl_display`] successfully.
-    ///
-    /// Returns `None` if the type is not known to smithay
-    /// or otherwise not supported (e.g. not initialized using one of smithays [`crate::wayland`]-handlers).
-    fn buffer_type(&self, buffer: &wl_buffer::WlBuffer) -> Option<BufferType>;
-
-    /// Returns the dimensions of a wl_buffer
-    ///
-    /// *Note*: This will only return dimensions for buffer types known to smithay (see [`buffer_type`]).
-    ///
-    /// *Note*: Different to [`buffer_type`] this variant uses its internal `EGLBufferReader`, if
-    /// - the underlying Renderer supports `ImportEgl`,
-    /// - smithay was compiled with the `backend_egl` and `use_system_lib`,
-    /// - and the underlying Renderer bound via [`ImportEgl::bind_wl_display`] successfully.
-    fn buffer_dimensions(&self, buffer: &wl_buffer::WlBuffer) -> Option<(i32, i32)>;
 }
 
 // TODO: Do this with specialization, when possible and do default implementations
@@ -440,21 +422,13 @@ impl<R: Renderer + ImportShm + ImportEgl + ImportDma> ImportAll for R {
         surface: Option<&SurfaceAttributes>,
         damage: &[Rectangle],
     ) -> Option<Result<<Self as Renderer>::TextureId, <Self as Renderer>::Error>> {
-        match buffer_type(buffer, self.egl_reader()) {
+        match buffer_type(buffer) {
             Some(BufferType::Shm) => Some(self.import_shm_buffer(buffer, surface, damage)),
             Some(BufferType::Egl) => Some(self.import_egl_buffer(buffer)),
             Some(BufferType::Dma) => Some(self.import_dma_buffer(buffer)),
             _ => None,
         }
     }
-
-    fn buffer_type(&self, buffer: &wl_buffer::WlBuffer) -> Option<BufferType> {
-        buffer_type(buffer, self.egl_reader())
-    }
-
-    fn buffer_dimensions(&self, buffer: &wl_buffer::WlBuffer) -> Option<(i32, i32)> {
-        buffer_dimensions(buffer, self.egl_reader())
-    }
 }
 
 #[cfg(all(
@@ -474,14 +448,6 @@ impl<R: Renderer + ImportShm + ImportDma> ImportAll for R {
             _ => None,
         }
     }
-
-    fn buffer_type(&self, buffer: &wl_buffer::WlBuffer) -> Option<BufferType> {
-        buffer_type(buffer)
-    }
-
-    fn buffer_dimensions(&self, buffer: &wl_buffer::WlBuffer) -> Option<(i32, i32)> {
-        buffer_dimensions(buffer)
-    }
 }
 
 #[cfg(feature = "wayland_frontend")]
@@ -501,94 +467,58 @@ pub enum BufferType {
 ///
 /// Returns `None` if the type is not known to smithay
 /// or otherwise not supported (e.g. not initialized using one of smithays [`crate::wayland`]-handlers).
-#[cfg(all(
+#[cfg(feature = "wayland_frontend")]
-    feature = "wayland_frontend",
+pub fn buffer_type(buffer: &wl_buffer::WlBuffer) -> Option<BufferType> {
-    feature = "backend_egl",
-    feature = "use_system_lib"
-))]
-pub fn buffer_type(
-    buffer: &wl_buffer::WlBuffer,
-    egl_buffer_reader: Option<&EGLBufferReader>,
-) -> Option<BufferType> {
     if buffer.as_ref().user_data().get::<Dmabuf>().is_some() {
-        Some(BufferType::Dma)
+        return Some(BufferType::Dma);
-    } else if egl_buffer_reader
+    }
+
+    #[cfg(all(feature = "backend_egl", feature = "use_system_lib"))]
+    if BUFFER_READER
+        .lock()
+        .unwrap()
         .as_ref()
+        .and_then(|x| x.upgrade())
         .and_then(|x| x.egl_buffer_dimensions(&buffer))
         .is_some()
     {
-        Some(BufferType::Egl)
+        return Some(BufferType::Egl);
-    } else if crate::wayland::shm::with_buffer_contents(&buffer, |_, _| ()).is_ok() {
-        Some(BufferType::Shm)
-    } else {
-        None
     }
-}
 
-/// Returns the *type* of a wl_buffer
+    if crate::wayland::shm::with_buffer_contents(&buffer, |_, _| ()).is_ok() {
-///
+        return Some(BufferType::Shm);
-/// Returns `None` if the type is not recognized by smithay or otherwise not supported.
-#[cfg(all(
-    feature = "wayland_frontend",
-    not(all(feature = "backend_egl", feature = "use_system_lib"))
-))]
-pub fn buffer_type(buffer: &wl_buffer::WlBuffer) -> Option<BufferType> {
-    if buffer.as_ref().user_data().get::<Dmabuf>().is_some() {
-        Some(BufferType::Dma)
-    } else if crate::wayland::shm::with_buffer_contents(&buffer, |_, _| ()).is_ok() {
-        Some(BufferType::Shm)
-    } else {
-        None
     }
+
+    None
 }
 
 /// Returns the dimensions of a wl_buffer
 ///
 /// *Note*: This will only return dimensions for buffer types known to smithay (see [`buffer_type`])
-#[cfg(all(
+#[cfg(feature = "wayland_frontend")]
-    feature = "wayland_frontend",
-    feature = "backend_egl",
-    feature = "use_system_lib"
-))]
-pub fn buffer_dimensions(
-    buffer: &wl_buffer::WlBuffer,
-    egl_buffer_reader: Option<&EGLBufferReader>,
-) -> Option<(i32, i32)> {
-    use crate::backend::allocator::Buffer;
-
-    if let Some(buf) = buffer.as_ref().user_data().get::<Dmabuf>() {
-        Some((buf.width() as i32, buf.height() as i32))
-    } else if let Some((w, h)) = egl_buffer_reader
-        .as_ref()
-        .and_then(|x| x.egl_buffer_dimensions(&buffer))
-    {
-        Some((w, h))
-    } else if let Ok((w, h)) =
-        crate::wayland::shm::with_buffer_contents(&buffer, |_, data| (data.width, data.height))
-    {
-        Some((w, h))
-    } else {
-        None
-    }
-}
-
-/// Returns the dimensions of a wl_buffer
-///
-/// *Note*: This will only return dimensions for buffer types known to smithay (see [`buffer_type`])
-#[cfg(all(
-    feature = "wayland_frontend",
-    not(all(feature = "backend_egl", feature = "use_system_lib"))
-))]
 pub fn buffer_dimensions(buffer: &wl_buffer::WlBuffer) -> Option<(i32, i32)> {
     use crate::backend::allocator::Buffer;
 
     if let Some(buf) = buffer.as_ref().user_data().get::<Dmabuf>() {
-        Some((buf.width() as i32, buf.height() as i32))
+        return Some((buf.width() as i32, buf.height() as i32));
-    } else if let Ok((w, h)) =
+    }
+
+    #[cfg(all(feature = "backend_egl", feature = "use_system_lib"))]
+    if let Some((w, h)) = BUFFER_READER
+        .lock()
+        .unwrap()
+        .as_ref()
+        .and_then(|x| x.upgrade())
+        .and_then(|x| x.egl_buffer_dimensions(&buffer))
+    {
+        return Some((w, h));
+    }
+
+    if let Ok((w, h)) =
         crate::wayland::shm::with_buffer_contents(&buffer, |_, data| (data.width, data.height))
     {
-        Some((w, h))
+        return Some((w, h));
-    } else {
-        None
     }
+
+    None
 }