From 7b4459f649e1e80f3a1f1b66eaea778a10bfc10a Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Fri, 5 Jun 2020 22:57:52 +0200 Subject: [PATCH] drm: Make surfaces `Send` --- Cargo.toml | 2 +- anvil/src/udev.rs | 8 +- examples/raw_nvidia.rs | 8 +- src/backend/drm/atomic/mod.rs | 12 +- src/backend/drm/atomic/session.rs | 22 ++- src/backend/drm/atomic/surface.rs | 87 +++++---- src/backend/drm/egl/mod.rs | 9 +- src/backend/drm/egl/session.rs | 8 +- src/backend/drm/egl/surface.rs | 20 +- src/backend/drm/eglstream/egl.rs | 36 ++-- src/backend/drm/eglstream/mod.rs | 27 +-- src/backend/drm/eglstream/session.rs | 24 +-- src/backend/drm/eglstream/surface.rs | 103 ++++++----- src/backend/drm/gbm/egl.rs | 14 +- src/backend/drm/gbm/mod.rs | 93 ++++------ src/backend/drm/gbm/session.rs | 24 +-- src/backend/drm/gbm/surface.rs | 265 +++++++++++++++++---------- src/backend/drm/legacy/mod.rs | 12 +- src/backend/drm/legacy/session.rs | 12 +- src/backend/drm/legacy/surface.rs | 22 ++- src/backend/egl/context.rs | 8 +- src/backend/egl/display.rs | 3 + src/backend/egl/surface.rs | 55 +++--- 23 files changed, 492 insertions(+), 382 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ad6e8bf..94e33a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ bitflags = "1" calloop = "0.6.2" dbus = { version = "0.8", optional = true } drm = { version = "^0.4.0", git = "https://github.com/drakulix/drm-rs", branch = "develop", optional = true } -gbm = { version = "^0.6.0", git = "https://github.com/drakulix/gbm.rs", branch = "develop", optional = true, default-features = false, features = ["drm-support"] } +gbm = { version = "^0.6.0", git = "https://github.com/drakulix/gbm.rs", branch = "thread-safe", optional = true, default-features = false, features = ["drm-support"] } glium = { version = "0.27.0", optional = true, default-features = false } image = { version = "0.23.0", optional = true } input = { version = "0.5", default-features = false, optional = true } diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index b1cdfa3..dad9b04 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -17,13 +17,13 @@ use smithay::backend::egl::{display::EGLBufferReader, EGLGraphicsBackend}; use smithay::{ backend::{ drm::{ - atomic::AtomicDrmDevice, + atomic::{AtomicDrmDevice, AtomicDrmSurface}, common::fallback::{FallbackDevice, FallbackSurface}, device_bind, egl::{EglDevice, EglSurface}, eglstream::{egl::EglStreamDeviceBackend, EglStreamDevice, EglStreamSurface}, gbm::{egl::Gbm as EglGbmBackend, GbmDevice, GbmSurface}, - legacy::LegacyDrmDevice, + legacy::{LegacyDrmDevice, LegacyDrmSurface}, DevPath, Device, DeviceHandler, Surface, }, graphics::{CursorBackend, SwapBuffersError}, @@ -86,8 +86,8 @@ type RenderDevice = FallbackDevice< >, >; type RenderSurface = FallbackSurface< - EglSurface, LegacyDrmDevice>>>, - EglSurface, LegacyDrmDevice>>>, + EglSurface, LegacyDrmSurface>>>, + EglSurface, LegacyDrmSurface>>>, >; pub fn run_udev( diff --git a/examples/raw_nvidia.rs b/examples/raw_nvidia.rs index f8c7ce1..081ff56 100644 --- a/examples/raw_nvidia.rs +++ b/examples/raw_nvidia.rs @@ -8,15 +8,15 @@ use slog::Drain; use smithay::{ backend::{ drm::{ - atomic::AtomicDrmDevice, - common::fallback::{EitherError, FallbackDevice}, + atomic::{AtomicDrmDevice, AtomicDrmSurface}, + common::fallback::{EitherError, FallbackDevice, FallbackSurface}, common::Error as DrmError, device_bind, egl::{EglDevice, EglSurface, Error as EglError}, eglstream::{ egl::EglStreamDeviceBackend, EglStreamDevice, EglStreamSurface, Error as EglStreamError, }, - legacy::LegacyDrmDevice, + legacy::{LegacyDrmDevice, LegacyDrmSurface}, Device, DeviceHandler, }, graphics::glium::GliumGraphicsBackend, @@ -145,7 +145,7 @@ pub struct DrmHandlerImpl { GliumGraphicsBackend< EglSurface< EglStreamSurface< - FallbackDevice, LegacyDrmDevice>, + FallbackSurface, LegacyDrmSurface>, >, >, >, diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index b40bd83..1d2e1f9 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -21,10 +21,10 @@ use std::cell::RefCell; use std::collections::HashMap; use std::os::unix::io::{AsRawFd, RawFd}; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::sync::{ atomic::{AtomicBool, Ordering}, - Arc, + Arc, Weak, }; use drm::control::{atomic::AtomicModeReq, AtomicCommitFlags, Device as ControlDevice, Event}; @@ -49,7 +49,7 @@ pub mod session; /// Open raw drm device utilizing atomic mode-setting pub struct AtomicDrmDevice { - dev: Rc>, + dev: Arc>, dev_id: dev_t, active: Arc, backends: Rc>>>>, @@ -360,7 +360,7 @@ impl AtomicDrmDevice { } Ok(AtomicDrmDevice { - dev: Rc::new(dev), + dev: Arc::new(dev), dev_id, active, backends: Rc::new(RefCell::new(HashMap::new())), @@ -414,7 +414,7 @@ impl Device for AtomicDrmDevice { return Err(Error::SurfaceWithoutConnectors(crtc)); } - let backend = Rc::new(AtomicDrmSurfaceInternal::new( + let backend = Arc::new(AtomicDrmSurfaceInternal::new( self.dev.clone(), crtc, mode, @@ -422,7 +422,7 @@ impl Device for AtomicDrmDevice { self.logger.new(o!("crtc" => format!("{:?}", crtc))), )?); - self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); + self.backends.borrow_mut().insert(crtc, Arc::downgrade(&backend)); Ok(AtomicDrmSurface(backend)) } diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index 5b0d365..5b7cfbc 100644 --- a/src/backend/drm/atomic/session.rs +++ b/src/backend/drm/atomic/session.rs @@ -13,9 +13,9 @@ use std::collections::HashMap; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Weak as WeakArc}; -use super::{AtomicDrmDevice, AtomicDrmSurfaceInternal, Dev}; +use super::{surface::CursorState, AtomicDrmDevice, AtomicDrmSurfaceInternal, Dev}; use crate::backend::drm::{common::Error, DevPath, Surface}; use crate::{ backend::session::Signal as SessionSignal, @@ -26,18 +26,18 @@ use crate::{ /// linked to the [`AtomicDrmDevice`](AtomicDrmDevice) /// it was created from. pub struct AtomicDrmDeviceObserver { - dev: Weak>, + dev: WeakArc>, dev_id: dev_t, privileged: bool, active: Arc, - backends: Weak>>>>, + backends: Weak>>>>, logger: ::slog::Logger, } impl Linkable for AtomicDrmDevice { fn link(&mut self, signaler: Signaler) { let mut observer = AtomicDrmDeviceObserver { - dev: Rc::downgrade(&self.dev), + dev: Arc::downgrade(&self.dev), dev_id: self.dev_id, active: self.active.clone(), privileged: self.dev.privileged, @@ -72,7 +72,7 @@ impl AtomicDrmDeviceObserver { // TODO: Clear overlay planes (if we ever use them) if let Some(backends) = self.backends.upgrade() { - for surface in backends.borrow().values().filter_map(Weak::upgrade) { + for surface in backends.borrow().values().filter_map(WeakArc::upgrade) { // other ttys that use no cursor, might not clear it themselves. // This makes sure our cursor won't stay visible. // @@ -187,7 +187,7 @@ impl AtomicDrmDeviceObserver { // // Lets do that, by creating a garbage/non-matching current-state. if let Some(backends) = self.backends.upgrade() { - for surface in backends.borrow().values().filter_map(Weak::upgrade) { + for surface in backends.borrow().values().filter_map(WeakArc::upgrade) { let mut current = surface.state.write().unwrap(); // lets force a non matching state @@ -202,9 +202,11 @@ impl AtomicDrmDeviceObserver { surface.use_mode(mode)?; // drop cursor state to force setting the cursor again. - surface.cursor.position.set(None); - surface.cursor.hotspot.set((0, 0)); - surface.cursor.framebuffer.set(None); + *surface.cursor.lock().unwrap() = CursorState { + position: None, + hotspot: (0, 0), + framebuffer: None, + }; } } } diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index d1c14c6..82bf2da 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -6,11 +6,9 @@ use drm::control::{ }; use drm::Device as BasicDevice; -use std::cell::Cell; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; -use std::rc::Rc; -use std::sync::{atomic::Ordering, RwLock}; +use std::sync::{atomic::Ordering, Arc, Mutex, RwLock}; use failure::ResultExt as FailureResultExt; @@ -20,9 +18,9 @@ use crate::backend::graphics::CursorBackend; #[derive(Debug, Clone, PartialEq, Eq)] pub struct CursorState { - pub position: Cell>, - pub hotspot: Cell<(u32, u32)>, - pub framebuffer: Cell>, + pub position: Option<(u32, u32)>, + pub hotspot: (u32, u32), + pub framebuffer: Option, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -39,14 +37,14 @@ pub struct Planes { } pub(in crate::backend::drm) struct AtomicDrmSurfaceInternal { - pub(super) dev: Rc>, + pub(super) dev: Arc>, pub(in crate::backend::drm) crtc: crtc::Handle, - pub(super) cursor: CursorState, + pub(super) cursor: Mutex, pub(in crate::backend::drm) planes: Planes, pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, - pub(super) test_buffer: Cell>, + pub(super) test_buffer: Mutex>, } impl AsRawFd for AtomicDrmSurfaceInternal { @@ -60,7 +58,7 @@ impl ControlDevice for AtomicDrmSurfaceInternal {} impl AtomicDrmSurfaceInternal { pub(crate) fn new( - dev: Rc>, + dev: Arc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], @@ -167,16 +165,16 @@ impl AtomicDrmSurfaceInternal { let surface = AtomicDrmSurfaceInternal { dev, crtc, - cursor: CursorState { - position: Cell::new(None), - framebuffer: Cell::new(None), - hotspot: Cell::new((0, 0)), - }, + cursor: Mutex::new(CursorState { + position: None, + framebuffer: None, + hotspot: (0, 0), + }), planes: Planes { primary, cursor }, state: RwLock::new(state), pending: RwLock::new(pending), logger, - test_buffer: Cell::new(None), + test_buffer: Mutex::new(None), }; Ok(surface) @@ -202,10 +200,13 @@ impl AtomicDrmSurfaceInternal { dev: self.dev_path(), source, })?; - if let Some((old_db, old_fb)) = self.test_buffer.replace(Some((db, fb))) { + + let mut test_buffer = self.test_buffer.lock().unwrap(); + if let Some((old_db, old_fb)) = test_buffer.take() { let _ = self.destroy_framebuffer(old_fb); let _ = self.destroy_dumb_buffer(old_db); }; + *test_buffer = Some((db, fb)); Ok(fb) } @@ -213,7 +214,7 @@ impl AtomicDrmSurfaceInternal { impl Drop for AtomicDrmSurfaceInternal { fn drop(&mut self) { - if let Some((db, fb)) = self.test_buffer.take() { + if let Some((db, fb)) = self.test_buffer.lock().unwrap().take() { let _ = self.destroy_framebuffer(fb); let _ = self.destroy_dumb_buffer(db); } @@ -613,7 +614,7 @@ impl CursorBackend for AtomicDrmSurfaceInternal { } trace!(self.logger, "New cursor position ({},{}) pending", x, y); - self.cursor.position.set(Some((x, y))); + self.cursor.lock().unwrap().position = Some((x, y)); Ok(()) } @@ -628,21 +629,20 @@ impl CursorBackend for AtomicDrmSurfaceInternal { trace!(self.logger, "Setting the new imported cursor"); - if let Some(fb) = self.cursor.framebuffer.get().take() { + let mut cursor = self.cursor.lock().unwrap(); + + if let Some(fb) = cursor.framebuffer.take() { let _ = self.destroy_framebuffer(fb); } - self.cursor.framebuffer.set(Some( - self.add_planar_framebuffer(buffer, &[0; 4], 0) - .compat() - .map_err(|source| Error::Access { - errmsg: "Failed to import cursor", - dev: self.dev_path(), - source, - })?, - )); - - self.cursor.hotspot.set(hotspot); + cursor.framebuffer = Some(self.add_planar_framebuffer(buffer, &[0; 4], 0).compat().map_err( + |source| Error::Access { + errmsg: "Failed to import cursor", + dev: self.dev_path(), + source, + }, + )?); + cursor.hotspot = hotspot; Ok(()) } @@ -828,19 +828,17 @@ impl AtomicDrmSurfaceInternal { ); } - let cursor_pos = self.cursor.position.get(); - let cursor_fb = self.cursor.framebuffer.get(); - // if there is a cursor, we add the cursor plane to the request as well. // this synchronizes cursor movement with rendering, which reduces flickering. - if let (Some(pos), Some(fb)) = (cursor_pos, cursor_fb) { + let mut cursor = self.cursor.lock().unwrap(); + if let (Some(pos), Some(fb)) = (cursor.position, cursor.framebuffer) { match self.get_framebuffer(fb).compat().map_err(|source| Error::Access { errmsg: "Error getting cursor fb", dev: self.dev_path(), source, }) { Ok(cursor_info) => { - let hotspot = self.cursor.hotspot.get(); + let hotspot = cursor.hotspot; // again like the primary plane we need to set crtc and framebuffer. req.add_property( @@ -898,7 +896,7 @@ impl AtomicDrmSurfaceInternal { } Err(err) => { warn!(self.logger, "Cursor FB invalid: {}. Skipping.", err); - self.cursor.framebuffer.set(None); + cursor.framebuffer = None; } } } @@ -997,7 +995,7 @@ impl AtomicDrmSurfaceInternal { /// Open raw crtc utilizing atomic mode-setting pub struct AtomicDrmSurface( - pub(in crate::backend::drm) Rc>, + pub(in crate::backend::drm) Arc>, ); impl AsRawFd for AtomicDrmSurface { @@ -1080,3 +1078,16 @@ impl RawSurface for AtomicDrmSurface { RawSurface::page_flip(&*self.0, framebuffer) } } + +#[cfg(test)] +mod test { + use super::AtomicDrmSurface; + use std::fs::File; + + fn is_send() {} + + #[test] + fn surface_is_send() { + is_send::>(); + } +} diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 8252367..b9397d8 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -16,7 +16,8 @@ use nix::libc::dev_t; use std::cell::RefCell; use std::collections::HashMap; use std::os::unix::io::{AsRawFd, RawFd}; -use std::rc::{Rc, Weak}; +use std::rc::Rc; +use std::sync::{Arc, Weak as WeakArc}; #[cfg(feature = "use_system_lib")] use wayland_server::Display; @@ -49,7 +50,7 @@ pub enum Error); -type BackendRef = Weak::Surface>>; +type BackendRef = WeakArc::Surface>>; /// Representation of an egl device to create egl rendering surfaces pub struct EglDevice @@ -214,8 +215,8 @@ where SurfaceCreationError::NativeSurfaceCreationFailed(err) => Error::Underlying(err), })?; - let backend = Rc::new(EglSurfaceInternal { context, surface }); - self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); + let backend = Arc::new(EglSurfaceInternal { context, surface }); + self.backends.borrow_mut().insert(crtc, Arc::downgrade(&backend)); Ok(EglSurface(backend)) } diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs index cdcbf64..f8ab7ab 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -7,6 +7,7 @@ use drm::control::{connector, crtc, Mode}; use std::cell::RefCell; use std::collections::HashMap; use std::rc::{Rc, Weak}; +use std::sync::{atomic::Ordering, Weak as WeakArc}; use super::{EglDevice, EglSurfaceInternal}; use crate::backend::drm::{Device, Surface}; @@ -23,7 +24,7 @@ use crate::{ /// linked to the [`EglDevice`](EglDevice) it was /// created from. pub struct EglDeviceObserver { - backends: Weak>>>>, + backends: Weak>>>>, } impl Linkable for EglDevice @@ -63,7 +64,10 @@ impl EglDeviceObserver { if let Some(backends) = self.backends.upgrade() { for (_crtc, backend) in backends.borrow().iter() { if let Some(backend) = backend.upgrade() { - let old_surface = backend.surface.surface.replace(std::ptr::null()); + let old_surface = backend + .surface + .surface + .swap(std::ptr::null_mut(), Ordering::SeqCst); if !old_surface.is_null() { unsafe { ffi::egl::DestroySurface(**backend.surface.display, old_surface as *const _); diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs index a6592a3..4fabceb 100644 --- a/src/backend/drm/egl/surface.rs +++ b/src/backend/drm/egl/surface.rs @@ -1,6 +1,7 @@ use drm::control::{connector, crtc, Mode}; use nix::libc::c_void; use std::convert::TryInto; +use std::sync::Arc; use super::Error; use crate::backend::drm::Surface; @@ -12,10 +13,8 @@ use crate::backend::graphics::gl::GLGraphicsBackend; use crate::backend::graphics::PixelFormat; use crate::backend::graphics::{CursorBackend, SwapBuffersError}; -use std::rc::Rc; - /// Egl surface for rendering -pub struct EglSurface(pub(super) Rc>); +pub struct EglSurface(pub(super) Arc>); pub(super) struct EglSurfaceInternal where @@ -136,3 +135,18 @@ where self.0.surface.get_pixel_format() } } + +#[cfg(test)] +mod test { + use super::EglSurface; + use crate::backend::drm::gbm::GbmSurface; + use crate::backend::drm::legacy::LegacyDrmSurface; + use std::fs::File; + + fn is_send() {} + + #[test] + fn surface_is_send() { + is_send::>>>(); + } +} diff --git a/src/backend/drm/eglstream/egl.rs b/src/backend/drm/eglstream/egl.rs index 1d5ee20..861e5e1 100644 --- a/src/backend/drm/eglstream/egl.rs +++ b/src/backend/drm/eglstream/egl.rs @@ -5,13 +5,13 @@ //! #[cfg(feature = "backend_drm_atomic")] -use crate::backend::drm::atomic::AtomicDrmDevice; +use crate::backend::drm::atomic::AtomicDrmSurface; #[cfg(all(feature = "backend_drm_atomic", feature = "backend_drm_legacy"))] -use crate::backend::drm::common::fallback::{EitherError, FallbackDevice, FallbackSurface}; +use crate::backend::drm::common::fallback::{EitherError, FallbackSurface}; #[cfg(any(feature = "backend_drm_atomic", feature = "backend_drm_legacy"))] use crate::backend::drm::common::Error as DrmError; #[cfg(feature = "backend_drm_legacy")] -use crate::backend::drm::legacy::LegacyDrmDevice; +use crate::backend::drm::legacy::LegacyDrmSurface; use crate::backend::drm::{Device, RawDevice, RawSurface, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; use crate::backend::egl::{ @@ -37,9 +37,10 @@ pub struct EglStreamDeviceBackend { impl Backend for EglStreamDeviceBackend where - EglStreamSurface: NativeSurface::Surface as Surface>::Error>>, + EglStreamSurface<::Surface>: + NativeSurface::Surface as Surface>::Error>>, { - type Surface = EglStreamSurface; + type Surface = EglStreamSurface<::Surface>; type Error = Error<<::Surface as Surface>::Error>; // create an EGLDisplay for the EGLstream platform @@ -73,7 +74,8 @@ where unsafe impl NativeDisplay> for EglStreamDevice where - EglStreamSurface: NativeSurface::Surface as Surface>::Error>>, + EglStreamSurface<::Surface>: + NativeSurface::Surface as Surface>::Error>>, { type Arguments = (crtc::Handle, Mode, Vec); @@ -100,7 +102,8 @@ where fn create_surface( &mut self, args: Self::Arguments, - ) -> Result, Error<<::Surface as Surface>::Error>> { + ) -> Result::Surface>, Error<<::Surface as Surface>::Error>> + { Device::create_surface(self, args.0, args.1, &args.2) } } @@ -111,7 +114,7 @@ where // as a result, we need three implemenations for atomic, legacy and fallback... #[cfg(feature = "backend_drm_atomic")] -unsafe impl NativeSurface for EglStreamSurface> { +unsafe impl NativeSurface for EglStreamSurface> { type Error = Error; unsafe fn create( @@ -141,17 +144,12 @@ unsafe impl NativeSurface for EglStreamSurface, surface: ffi::egl::types::EGLSurface, ) -> Result<(), SwapBuffersError>> { - if let Some((buffer, fb)) = self.0.commit_buffer.take() { - let _ = self.0.crtc.destroy_framebuffer(fb); - let _ = self.0.crtc.destroy_dumb_buffer(buffer); - } - self.flip(self.0.crtc.0.crtc, display, surface) } } #[cfg(feature = "backend_drm_legacy")] -unsafe impl NativeSurface for EglStreamSurface> { +unsafe impl NativeSurface for EglStreamSurface> { type Error = Error; unsafe fn create( @@ -181,17 +179,13 @@ unsafe impl NativeSurface for EglStreamSurface, surface: ffi::egl::types::EGLSurface, ) -> Result<(), SwapBuffersError>> { - if let Some((buffer, fb)) = self.0.commit_buffer.take() { - let _ = self.0.crtc.destroy_framebuffer(fb); - let _ = self.0.crtc.destroy_dumb_buffer(buffer); - } self.flip(self.0.crtc.0.crtc, display, surface) } } #[cfg(all(feature = "backend_drm_atomic", feature = "backend_drm_legacy"))] unsafe impl NativeSurface - for EglStreamSurface, LegacyDrmDevice>> + for EglStreamSurface, LegacyDrmSurface>> { type Error = Error>; @@ -230,10 +224,6 @@ unsafe impl NativeSurface display: &Arc, surface: ffi::egl::types::EGLSurface, ) -> Result<(), SwapBuffersError> { - if let Some((buffer, fb)) = self.0.commit_buffer.take() { - let _ = self.0.crtc.destroy_framebuffer(fb); - let _ = self.0.crtc.destroy_dumb_buffer(buffer); - } let crtc = match &self.0.crtc { FallbackSurface::Preference(dev) => dev.0.crtc, FallbackSurface::Fallback(dev) => dev.0.crtc, diff --git a/src/backend/drm/eglstream/mod.rs b/src/backend/drm/eglstream/mod.rs index 9c8d320..3d8db14 100644 --- a/src/backend/drm/eglstream/mod.rs +++ b/src/backend/drm/eglstream/mod.rs @@ -25,12 +25,13 @@ use drm::SystemError as DrmError; use failure::ResultExt; use nix::libc::dev_t; -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::collections::HashMap; use std::ffi::CStr; use std::os::unix::fs::MetadataExt; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; +use std::sync::{Arc, Mutex, Weak as WeakArc}; use std::{fs, ptr}; mod surface; @@ -78,11 +79,12 @@ pub enum Error = WeakArc::Surface>>; /// Representation of an open egl stream device to create egl rendering surfaces pub struct EglStreamDevice { pub(self) dev: EGLDeviceEXT, raw: D, - backends: Rc>>>>, + backends: Rc>>>, logger: ::slog::Logger, #[cfg(feature = "backend_session")] links: Vec, @@ -227,7 +229,7 @@ impl EglStreamDevice { struct InternalDeviceHandler { handler: Box> + 'static>, - backends: Weak>>>>, + backends: Weak>>>, logger: ::slog::Logger, } @@ -254,7 +256,7 @@ impl DeviceHandler for InternalDeviceHan } impl Device for EglStreamDevice { - type Surface = EglStreamSurface; + type Surface = EglStreamSurface<::Surface>; fn device_id(&self) -> dev_t { self.raw.device_id() @@ -277,30 +279,31 @@ impl Device for EglStreamDevice { crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], - ) -> Result, Error<<::Surface as Surface>::Error>> { + ) -> Result::Surface>, Error<<::Surface as Surface>::Error>> + { info!(self.logger, "Initializing EglStreamSurface"); let drm_surface = Device::create_surface(&mut self.raw, crtc, mode, connectors).map_err(Error::Underlying)?; // initialize a buffer for the cursor image - let cursor = Cell::new(Some(( + let cursor = Some(( self.raw .create_dumb_buffer((1, 1), PixelFormat::ARGB8888) .compat() .map_err(Error::BufferCreationFailed)?, (0, 0), - ))); + )); - let backend = Rc::new(EglStreamSurfaceInternal { + let backend = Arc::new(EglStreamSurfaceInternal { crtc: drm_surface, - cursor, - stream: RefCell::new(None), - commit_buffer: Cell::new(None), + cursor: Mutex::new(cursor), + stream: Mutex::new(None), + commit_buffer: Mutex::new(None), logger: self.logger.new(o!("crtc" => format!("{:?}", crtc))), locked: std::sync::atomic::AtomicBool::new(false), }); - self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); + self.backends.borrow_mut().insert(crtc, Arc::downgrade(&backend)); Ok(EglStreamSurface(backend)) } diff --git a/src/backend/drm/eglstream/session.rs b/src/backend/drm/eglstream/session.rs index 8904a24..a85cc5c 100644 --- a/src/backend/drm/eglstream/session.rs +++ b/src/backend/drm/eglstream/session.rs @@ -4,7 +4,7 @@ //! use super::{EglStreamDevice, EglStreamSurfaceInternal}; -use crate::backend::drm::{RawDevice, Surface}; +use crate::backend::drm::{Device, RawDevice, RawSurface}; use crate::backend::egl::ffi; use crate::backend::session::Signal as SessionSignal; use crate::signaling::{Linkable, Signaler}; @@ -12,14 +12,15 @@ use crate::signaling::{Linkable, Signaler}; use std::cell::RefCell; use std::collections::HashMap; use std::rc::{Rc, Weak}; +use std::sync::Weak as WeakArc; use drm::control::{crtc, Device as ControlDevice}; /// [`SessionObserver`](SessionObserver) /// linked to the [`EglStreamDevice`](EglStreamDevice) it was /// created from. -pub struct EglStreamDeviceObserver { - backends: Weak>>>>, +pub struct EglStreamDeviceObserver { + backends: Weak>>>>, logger: ::slog::Logger, } @@ -30,7 +31,7 @@ where fn link(&mut self, signaler: Signaler) { let lower_signal = Signaler::new(); self.raw.link(lower_signal.clone()); - let mut observer = EglStreamDeviceObserver { + let mut observer = EglStreamDeviceObserver::<::Surface> { backends: Rc::downgrade(&self.backends), logger: self.logger.clone(), }; @@ -52,19 +53,19 @@ where } } -impl EglStreamDeviceObserver { +impl EglStreamDeviceObserver { fn pause(&mut self) { if let Some(backends) = self.backends.upgrade() { for (_, backend) in backends.borrow().iter() { if let Some(backend) = backend.upgrade() { // destroy/disable the streams so it will not submit any pending frames - if let Some((display, stream)) = backend.stream.replace(None) { + if let Some((display, stream)) = backend.stream.lock().unwrap().take() { unsafe { - ffi::egl::DestroyStreamKHR(display.handle, stream); + ffi::egl::DestroyStreamKHR(display.handle, stream.0); } } // framebuffers will be likely not valid anymore, lets just recreate those after activation. - if let Some((buffer, fb)) = backend.commit_buffer.take() { + if let Some((buffer, fb)) = backend.commit_buffer.lock().unwrap().take() { let _ = backend.crtc.destroy_framebuffer(fb); let _ = backend.crtc.destroy_dumb_buffer(buffer); } @@ -77,17 +78,18 @@ impl EglStreamDeviceObserver { if let Some(backends) = self.backends.upgrade() { for (_, backend) in backends.borrow().iter() { if let Some(backend) = backend.upgrade() { - if let Some((cursor, hotspot)) = backend.cursor.get() { + let cursor = backend.cursor.lock().unwrap(); + if let Some((ref cursor, ref hotspot)) = &*cursor { if backend .crtc .set_cursor2( backend.crtc.crtc(), - Some(&cursor), + Some(cursor), (hotspot.0 as i32, hotspot.1 as i32), ) .is_err() { - if let Err(err) = backend.crtc.set_cursor(backend.crtc.crtc(), Some(&cursor)) { + if let Err(err) = backend.crtc.set_cursor(backend.crtc.crtc(), Some(cursor)) { warn!(self.logger, "Failed to reset cursor: {}", err) } } diff --git a/src/backend/drm/eglstream/surface.rs b/src/backend/drm/eglstream/surface.rs index 2c231d0..3d70284 100644 --- a/src/backend/drm/eglstream/surface.rs +++ b/src/backend/drm/eglstream/surface.rs @@ -1,21 +1,19 @@ -use super::super::{Device, RawDevice, RawSurface, Surface}; +use super::super::{RawSurface, Surface}; use super::Error; use drm::buffer::format::PixelFormat; -use drm::control::{connector, crtc, dumbbuffer::DumbBuffer, framebuffer, Device as ControlDevice, Mode}; +use drm::control::{connector, crtc, dumbbuffer::DumbBuffer, framebuffer, Mode}; #[cfg(feature = "backend_drm")] use failure::ResultExt; #[cfg(feature = "backend_drm")] use image::{ImageBuffer, Rgba}; use nix::libc::{c_int, c_void}; -use std::cell::{Cell, RefCell}; use std::ffi::CStr; use std::ptr; -use std::rc::Rc; use std::sync::{ atomic::{AtomicBool, Ordering}, - Arc, + Arc, Mutex, }; #[cfg(feature = "backend_drm")] @@ -30,18 +28,24 @@ use crate::backend::egl::{display::EGLDisplayHandle, wrap_egl_call, EGLError, Sw #[cfg(feature = "backend_drm")] use crate::backend::graphics::CursorBackend; -pub(in crate::backend::drm) struct EglStreamSurfaceInternal { - pub(in crate::backend::drm) crtc: ::Surface, - pub(in crate::backend::drm) cursor: Cell>, - pub(in crate::backend::drm) stream: RefCell, EGLStreamKHR)>>, - pub(in crate::backend::drm) commit_buffer: Cell>, +// We do not want to mark the whole surface as `Send` in case we screw up somewhere else +// and because S needs to be `Send` as well for this to work. +pub(super) struct StreamHandle(pub(super) EGLStreamKHR); +// EGLStreamKHR can be moved between threads +unsafe impl Send for StreamHandle {} + +pub(in crate::backend::drm) struct EglStreamSurfaceInternal { + pub(in crate::backend::drm) crtc: S, + pub(in crate::backend::drm) cursor: Mutex>, + pub(super) stream: Mutex, StreamHandle)>>, + pub(in crate::backend::drm) commit_buffer: Mutex>, pub(in crate::backend::drm) locked: AtomicBool, pub(in crate::backend::drm) logger: ::slog::Logger, } -impl Surface for EglStreamSurfaceInternal { - type Connectors = <::Surface as Surface>::Connectors; - type Error = Error<<::Surface as Surface>::Error>; +impl Surface for EglStreamSurfaceInternal { + type Connectors = ::Connectors; + type Error = Error<::Error>; fn crtc(&self) -> crtc::Handle { self.crtc.crtc() @@ -75,23 +79,23 @@ impl Surface for EglStreamSurfaceInternal { self.crtc.pending_mode() } - fn use_mode(&self, mode: Mode) -> Result<(), Error<<::Surface as Surface>::Error>> { + fn use_mode(&self, mode: Mode) -> Result<(), Error<::Error>> { self.crtc.use_mode(mode).map_err(Error::Underlying) } } -impl Drop for EglStreamSurfaceInternal { +impl Drop for EglStreamSurfaceInternal { fn drop(&mut self) { - if let Some((buffer, _)) = self.cursor.get() { + if let Some((buffer, _)) = self.cursor.lock().unwrap().take() { let _ = self.crtc.destroy_dumb_buffer(buffer); } - if let Some((buffer, fb)) = self.commit_buffer.take() { + if let Some((buffer, fb)) = self.commit_buffer.lock().unwrap().take() { let _ = self.crtc.destroy_framebuffer(fb); let _ = self.crtc.destroy_dumb_buffer(buffer); } - if let Some((display, stream)) = self.stream.replace(None) { + if let Some((display, stream)) = self.stream.lock().unwrap().take() { unsafe { - egl::DestroyStreamKHR(display.handle, stream); + egl::DestroyStreamKHR(display.handle, stream.0); } } } @@ -110,7 +114,7 @@ impl Drop for EglStreamSurfaceInternal { // Note that this might still appear a little choppy, we should just use software cursors // on eglstream devices by default and only use this, if the user really wants it. #[cfg(feature = "backend_drm")] -impl CursorBackend for EglStreamSurfaceInternal { +impl CursorBackend for EglStreamSurfaceInternal { type CursorFormat = ImageBuffer, Vec>; type Error = Error; @@ -178,7 +182,7 @@ impl CursorBackend for EglStreamSurfaceInternal { } // and store it - if let Some((old, _)) = self.cursor.replace(Some((cursor, hotspot))) { + if let Some((old, _)) = self.cursor.lock().unwrap().replace((cursor, hotspot)) { if self.crtc.destroy_dumb_buffer(old).is_err() { warn!(self.logger, "Failed to free old cursor"); } @@ -189,14 +193,14 @@ impl CursorBackend for EglStreamSurfaceInternal { } /// egl stream surface for rendering -pub struct EglStreamSurface( - pub(in crate::backend::drm) Rc>, +pub struct EglStreamSurface( + pub(in crate::backend::drm) Arc>, ); -impl EglStreamSurface { +impl EglStreamSurface { /// Check if underlying gbm resources need to be recreated. pub fn needs_recreation(&self) -> bool { - self.0.crtc.commit_pending() || self.0.stream.borrow().is_none() + self.0.crtc.commit_pending() || self.0.stream.lock().unwrap().is_none() } // An EGLStream is basically the pump that requests and consumes images to display them. @@ -205,12 +209,13 @@ impl EglStreamSurface { &self, display: &Arc, output_attribs: &[isize], - ) -> Result::Surface as Surface>::Error>> { + ) -> Result::Error>> { + let mut stream = self.0.stream.lock().unwrap(); // drop old steam, if it already exists - if let Some((display, stream)) = self.0.stream.replace(None) { + if let Some((display, stream)) = stream.take() { // ignore result unsafe { - ffi::egl::DestroyStreamKHR(display.handle, stream); + ffi::egl::DestroyStreamKHR(display.handle, stream.0); } } @@ -225,7 +230,7 @@ impl EglStreamSurface { .create_dumb_buffer((w as u32, h as u32), PixelFormat::ARGB8888) { if let Ok(fb) = self.0.crtc.add_framebuffer(&buffer) { - if let Some((buffer, fb)) = self.0.commit_buffer.replace(Some((buffer, fb))) { + if let Some((buffer, fb)) = self.0.commit_buffer.lock().unwrap().replace((buffer, fb)) { let _ = self.0.crtc.destroy_framebuffer(fb); let _ = self.0.crtc.destroy_dumb_buffer(buffer); } @@ -388,21 +393,21 @@ impl EglStreamSurface { }; // okay, we have a config, lets create the stream. - let stream = unsafe { ffi::egl::CreateStreamKHR(display.handle, stream_attributes.as_ptr()) }; - if stream == ffi::egl::NO_STREAM_KHR { + let raw_stream = unsafe { ffi::egl::CreateStreamKHR(display.handle, stream_attributes.as_ptr()) }; + if raw_stream == ffi::egl::NO_STREAM_KHR { error!(self.0.logger, "Failed to create egl stream"); return Err(Error::DeviceStreamCreationFailed); } // we have a stream, lets connect it to our output layer - if unsafe { ffi::egl::StreamConsumerOutputEXT(display.handle, stream, layer) } == 0 { + if unsafe { ffi::egl::StreamConsumerOutputEXT(display.handle, raw_stream, layer) } == 0 { error!(self.0.logger, "Failed to link Output Layer as Stream Consumer"); return Err(Error::DeviceStreamCreationFailed); } - let _ = self.0.stream.replace(Some((display.clone(), stream))); + *stream = Some((display.clone(), StreamHandle(raw_stream))); - Ok(stream) + Ok(raw_stream) } pub(super) fn create_surface( @@ -411,7 +416,7 @@ impl EglStreamSurface { config_id: ffi::egl::types::EGLConfig, _surface_attribs: &[c_int], output_attribs: &[isize], - ) -> Result<*const c_void, Error<<::Surface as Surface>::Error>> { + ) -> Result<*const c_void, Error<::Error>> { // our surface needs a stream let stream = self.create_stream(display, output_attribs)?; @@ -450,7 +455,7 @@ impl EglStreamSurface { crtc: crtc::Handle, display: &Arc, surface: ffi::egl::types::EGLSurface, - ) -> Result<(), SwapBuffersError::Surface as Surface>::Error>>> { + ) -> Result<(), SwapBuffersError::Error>>> { // if we have already swapped the buffer successfully, we need to free it again. // // we need to do this here, because the call may fail (compare this with gbm's unlock_buffer...). @@ -466,7 +471,7 @@ impl EglStreamSurface { ffi::egl::NONE as isize, ]; - if let Ok(stream) = self.0.stream.try_borrow() { + if let Ok(stream) = self.0.stream.try_lock() { // lets try to acquire the frame. // this may fail, if the buffer is still in use by the gpu, // e.g. the flip was not done yet. In this case this call fails as `BUSY`. @@ -474,7 +479,7 @@ impl EglStreamSurface { wrap_egl_call(|| unsafe { ffi::egl::StreamConsumerAcquireAttribNV( display.handle, - *stream, + stream.0, acquire_attributes.as_ptr(), ); }) @@ -503,9 +508,9 @@ impl EglStreamSurface { } } -impl Surface for EglStreamSurface { - type Connectors = <::Surface as Surface>::Connectors; - type Error = Error<<::Surface as Surface>::Error>; +impl Surface for EglStreamSurface { + type Connectors = ::Connectors; + type Error = Error<::Error>; fn crtc(&self) -> crtc::Handle { self.0.crtc() @@ -545,7 +550,7 @@ impl Surface for EglStreamSurface { } #[cfg(feature = "backend_drm_legacy")] -impl CursorBackend for EglStreamSurface { +impl CursorBackend for EglStreamSurface { type CursorFormat = ImageBuffer, Vec>; type Error = Error; @@ -561,3 +566,17 @@ impl CursorBackend for EglStreamSurface { self.0.set_cursor_representation(buffer, hotspot) } } + +#[cfg(test)] +mod test { + use super::EglStreamSurface; + use crate::backend::drm::legacy::LegacyDrmSurface; + use std::fs::File; + + fn is_send() {} + + #[test] + fn surface_is_send() { + is_send::>>(); + } +} diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 83e64c3..2e89beb 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -4,7 +4,7 @@ //! [`GbmDevice`](GbmDevice) and [`GbmSurface`](GbmSurface). //! -use crate::backend::drm::{Device, RawDevice, Surface}; +use crate::backend::drm::{Device, RawDevice, RawSurface, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; use crate::backend::egl::{display::EGLDisplayHandle, ffi}; use crate::backend::egl::{ @@ -27,7 +27,7 @@ pub struct Gbm { } impl Backend for Gbm { - type Surface = GbmSurface; + type Surface = GbmSurface<::Surface>; type Error = Error<<::Surface as Surface>::Error>; // this creates an EGLDisplay for the gbm platform. @@ -75,19 +75,19 @@ unsafe impl NativeDisplay> for Gb } fn ptr(&self) -> Result { - Ok(self.dev.borrow().as_raw() as *const _) + Ok(self.dev.lock().unwrap().as_raw() as *const _) } fn create_surface( &mut self, args: Self::Arguments, - ) -> Result, Error<<::Surface as Surface>::Error>> { + ) -> Result::Surface>, Error<<::Surface as Surface>::Error>> { Device::create_surface(self, args.0, args.1, &args.2) } } -unsafe impl NativeSurface for GbmSurface { - type Error = Error<<::Surface as Surface>::Error>; +unsafe impl NativeSurface for GbmSurface { + type Error = Error<::Error>; unsafe fn create( &self, @@ -101,7 +101,7 @@ unsafe impl NativeSurface for GbmSurface { ffi::egl::CreateWindowSurface( display.handle, config_id, - self.0.surface.borrow().as_raw() as *const _, + self.0.surface.lock().unwrap().as_raw() as *const _, surface_attributes.as_ptr(), ) }) diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index 28fc5bf..d2f561d 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -16,15 +16,14 @@ use crate::backend::graphics::SwapBuffersError; use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Mode}; use drm::SystemError as DrmError; -use gbm::{self, BufferObjectFlags, Format as GbmFormat}; use nix::libc::dev_t; -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::collections::HashMap; use std::io; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; -use std::sync::Once; +use std::sync::{Arc, Mutex, Once, Weak as WeakArc}; /// Errors thrown by the [`GbmDevice`](::backend::drm::gbm::GbmDevice) /// and [`GbmSurface`](::backend::drm::gbm::GbmSurface). @@ -74,10 +73,12 @@ pub mod session; static LOAD: Once = Once::new(); +type SurfaceInternalRef = WeakArc::Surface>>; /// Representation of an open gbm device to create rendering surfaces pub struct GbmDevice { - pub(self) dev: Rc>>, - backends: Rc>>>>, + pub(self) dev: Arc>>, + pub(self) raw: D, + backends: Rc>>>, #[cfg(feature = "backend_session")] links: Vec, logger: ::slog::Logger, @@ -112,7 +113,10 @@ impl GbmDevice { debug!(log, "Creating gbm device"); Ok(GbmDevice { // Open the gbm device from the drm device - dev: Rc::new(RefCell::new(gbm::Device::new(dev).map_err(Error::InitFailed)?)), + dev: Arc::new(Mutex::new(unsafe { + gbm::Device::new_from_fd(dev.as_raw_fd()).map_err(Error::InitFailed)? + })), + raw: dev, backends: Rc::new(RefCell::new(HashMap::new())), #[cfg(feature = "backend_session")] links: Vec::new(), @@ -123,7 +127,7 @@ impl GbmDevice { struct InternalDeviceHandler { handler: Box> + 'static>, - backends: Weak>>>>, + backends: Weak>>>, logger: ::slog::Logger, } @@ -153,14 +157,14 @@ impl DeviceHandler for InternalDeviceHan } impl Device for GbmDevice { - type Surface = GbmSurface; + type Surface = GbmSurface<::Surface>; fn device_id(&self) -> dev_t { - self.dev.borrow().device_id() + self.raw.device_id() } fn set_handler(&mut self, handler: impl DeviceHandler + 'static) { - self.dev.borrow_mut().set_handler(InternalDeviceHandler { + self.raw.set_handler(InternalDeviceHandler { handler: Box::new(handler), backends: Rc::downgrade(&self.backends), logger: self.logger.clone(), @@ -168,7 +172,7 @@ impl Device for GbmDevice { } fn clear_handler(&mut self) { - self.dev.borrow_mut().clear_handler(); + self.raw.clear_handler(); } fn create_surface( @@ -176,85 +180,54 @@ impl Device for GbmDevice { crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], - ) -> Result, Error<<::Surface as Surface>::Error>> { + ) -> Result::Surface>, Error<<::Surface as Surface>::Error>> { info!(self.logger, "Initializing GbmSurface"); - let drm_surface = Device::create_surface(&mut **self.dev.borrow_mut(), crtc, mode, connectors) + let drm_surface = self + .raw + .create_surface(crtc, mode, connectors) .map_err(Error::Underlying)?; - // initialize the surface - let (w, h) = drm_surface.pending_mode().size(); - let surface = self - .dev - .borrow() - .create_surface( - w as u32, - h as u32, - GbmFormat::XRGB8888, - BufferObjectFlags::SCANOUT | BufferObjectFlags::RENDERING, - ) - .map_err(Error::SurfaceCreationFailed)?; - - // initialize a buffer for the cursor image - let cursor = Cell::new(( - self.dev - .borrow() - .create_buffer_object( - 1, - 1, - GbmFormat::ARGB8888, - BufferObjectFlags::CURSOR | BufferObjectFlags::WRITE, - ) - .map_err(Error::BufferCreationFailed)?, - (0, 0), - )); - - let backend = Rc::new(GbmSurfaceInternal { - dev: self.dev.clone(), - surface: RefCell::new(surface), - crtc: drm_surface, - cursor, - current_frame_buffer: Cell::new(None), - front_buffer: Cell::new(None), - next_buffer: Cell::new(None), - recreated: Cell::new(true), - logger: self.logger.new(o!("crtc" => format!("{:?}", crtc))), - }); - self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); + let backend = Arc::new(GbmSurfaceInternal::new( + self.dev.clone(), + drm_surface, + self.logger.new(o!("crtc" => format!("{:?}", crtc))), + )?); + self.backends.borrow_mut().insert(crtc, Arc::downgrade(&backend)); Ok(GbmSurface(backend)) } fn process_events(&mut self) { - self.dev.borrow_mut().process_events() + self.raw.process_events() } fn resource_handles(&self) -> Result::Surface as Surface>::Error>> { - Device::resource_handles(&**self.dev.borrow()).map_err(Error::Underlying) + Device::resource_handles(&self.raw).map_err(Error::Underlying) } fn get_connector_info(&self, conn: connector::Handle) -> std::result::Result { - self.dev.borrow().get_connector_info(conn) + self.raw.get_connector_info(conn) } fn get_crtc_info(&self, crtc: crtc::Handle) -> std::result::Result { - self.dev.borrow().get_crtc_info(crtc) + self.raw.get_crtc_info(crtc) } fn get_encoder_info(&self, enc: encoder::Handle) -> std::result::Result { - self.dev.borrow().get_encoder_info(enc) + self.raw.get_encoder_info(enc) } fn get_framebuffer_info( &self, fb: framebuffer::Handle, ) -> std::result::Result { - self.dev.borrow().get_framebuffer_info(fb) + self.raw.get_framebuffer_info(fb) } fn get_plane_info(&self, plane: plane::Handle) -> std::result::Result { - self.dev.borrow().get_plane_info(plane) + self.raw.get_plane_info(plane) } } impl AsRawFd for GbmDevice { fn as_raw_fd(&self) -> RawFd { - self.dev.borrow().as_raw_fd() + self.raw.as_raw_fd() } } diff --git a/src/backend/drm/gbm/session.rs b/src/backend/drm/gbm/session.rs index 46112ca..cd68393 100644 --- a/src/backend/drm/gbm/session.rs +++ b/src/backend/drm/gbm/session.rs @@ -4,12 +4,11 @@ //! use drm::control::crtc; -use gbm::BufferObject; use std::cell::RefCell; use std::collections::HashMap; use std::rc::{Rc, Weak}; -use super::{GbmDevice, GbmSurfaceInternal}; +use super::{GbmDevice, SurfaceInternalRef}; use crate::backend::drm::{Device, RawDevice}; use crate::backend::graphics::CursorBackend; use crate::{ @@ -21,8 +20,8 @@ use crate::{ /// linked to the [`GbmDevice`](GbmDevice) it was /// created from. pub(crate) struct GbmDeviceObserver { - backends: Weak>>>>, - logger: ::slog::Logger, + backends: Weak>>>, + _logger: ::slog::Logger, } impl Linkable for GbmDevice @@ -32,10 +31,10 @@ where { fn link(&mut self, signaler: Signaler) { let lower_signal = Signaler::new(); - self.dev.borrow_mut().link(lower_signal.clone()); - let mut observer = GbmDeviceObserver { + self.raw.link(lower_signal.clone()); + let mut observer = GbmDeviceObserver:: { backends: Rc::downgrade(&self.backends), - logger: self.logger.clone(), + _logger: self.logger.clone(), }; let token = signaler.register(move |&signal| match signal { @@ -66,15 +65,8 @@ where // reset cursor { - use ::drm::control::Device; - - let &(ref cursor, ref hotspot): &(BufferObject<()>, (u32, u32)) = - unsafe { &*backend.cursor.as_ptr() }; - if backend.crtc.set_cursor_representation(cursor, *hotspot).is_err() { - if let Err(err) = backend.dev.borrow().set_cursor(*crtc, Some(cursor)) { - error!(self.logger, "Failed to reset cursor. Error: {}", err); - } - } + let cursor = backend.cursor.lock().unwrap(); + let _ = backend.crtc.set_cursor_representation(&cursor.0, cursor.1); } } else { crtcs.push(*crtc); diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 7d4b1d2..c3add63 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -1,99 +1,155 @@ -use super::super::{Device, RawDevice, RawSurface, Surface}; +use super::super::{RawSurface, Surface}; use super::Error; -use drm::control::{connector, crtc, framebuffer, Device as ControlDevice, Mode}; +use drm::control::{connector, crtc, framebuffer, Mode}; use failure::ResultExt; -use gbm::{self, BufferObject, BufferObjectFlags, Format as GbmFormat, SurfaceBufferHandle}; +use gbm::{self, BufferObject, BufferObjectFlags, Format as GbmFormat}; use image::{ImageBuffer, Rgba}; -use std::cell::{Cell, RefCell}; -use std::rc::Rc; +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, +}; use crate::backend::graphics::CursorBackend; -pub(super) struct GbmSurfaceInternal { - pub(super) dev: Rc>>, - pub(super) surface: RefCell>, - pub(super) crtc: ::Surface, - pub(super) cursor: Cell<(BufferObject<()>, (u32, u32))>, - pub(super) current_frame_buffer: Cell>, - pub(super) front_buffer: Cell>>, - pub(super) next_buffer: Cell>>, - pub(super) recreated: Cell, +pub struct Buffers { + pub(super) current_frame_buffer: Option, + pub(super) front_buffer: Option>, + pub(super) next_buffer: Option>, +} + +pub(super) struct GbmSurfaceInternal { + pub(super) dev: Arc>>, + pub(super) surface: Mutex>, + pub(super) crtc: S, + pub(super) cursor: Mutex<(BufferObject<()>, (u32, u32))>, + pub(super) buffers: Mutex, + pub(super) recreated: AtomicBool, pub(super) logger: ::slog::Logger, } -impl GbmSurfaceInternal { +impl GbmSurfaceInternal { + pub(super) fn new( + dev: Arc>>, + drm_surface: S, + logger: ::slog::Logger, + ) -> Result, Error<::Error>> { + // initialize the surface + let (w, h) = drm_surface.pending_mode().size(); + let surface = dev + .lock() + .unwrap() + .create_surface( + w as u32, + h as u32, + GbmFormat::XRGB8888, + BufferObjectFlags::SCANOUT | BufferObjectFlags::RENDERING, + ) + .map_err(Error::SurfaceCreationFailed)?; + + // initialize a buffer for the cursor image + let cursor = ( + dev.lock() + .unwrap() + .create_buffer_object( + 1, + 1, + GbmFormat::ARGB8888, + BufferObjectFlags::CURSOR | BufferObjectFlags::WRITE, + ) + .map_err(Error::BufferCreationFailed)?, + (0, 0), + ); + + Ok(GbmSurfaceInternal { + dev, + surface: Mutex::new(surface), + crtc: drm_surface, + cursor: Mutex::new(cursor), + buffers: Mutex::new(Buffers { + current_frame_buffer: None, + front_buffer: None, + next_buffer: None, + }), + recreated: AtomicBool::new(true), + logger, + }) + } + pub(super) fn unlock_buffer(&self) { // after the page swap is finished we need to release the rendered buffer. // this is called from the PageFlipHandler trace!(self.logger, "Releasing old front buffer"); - self.front_buffer.set(self.next_buffer.replace(None)); + let mut buffers = self.buffers.lock().unwrap(); + buffers.front_buffer = buffers.next_buffer.take(); // drop and release the old buffer } - pub unsafe fn page_flip(&self) -> Result<(), Error<<::Surface as Surface>::Error>> { - let res = { - let nb = self.next_buffer.take(); - let res = nb.is_some(); - self.next_buffer.set(nb); - res - }; - if res { - // We cannot call lock_front_buffer anymore without releasing the previous buffer, which will happen when the page flip is done - warn!(self.logger, "Tried to swap with an already queued flip"); - return Err(Error::FrontBuffersExhausted); - } - - // supporting only one buffer would cause a lot of inconvinience and - // would most likely result in a lot of flickering. - // neither weston, wlc or wlroots bother with that as well. - // so we just assume we got at least two buffers to do flipping. - let mut next_bo = self - .surface - .borrow() - .lock_front_buffer() - .map_err(|_| Error::FrontBufferLockFailed)?; - - // create a framebuffer if the front buffer does not have one already - // (they are reused internally by gbm) - let maybe_fb = next_bo - .userdata() - .map_err(|_| Error::InvalidInternalState)? - .cloned(); - let fb = if let Some(info) = maybe_fb { - info - } else { - let fb = self - .crtc - .add_planar_framebuffer(&*next_bo, &[0; 4], 0) - .compat() - .map_err(Error::FramebufferCreationFailed)?; - next_bo.set_userdata(fb).unwrap(); - fb - }; - self.next_buffer.set(Some(next_bo)); - - if cfg!(debug_assertions) { - if let Err(err) = self.crtc.get_framebuffer(fb) { - error!(self.logger, "Cached framebuffer invalid: {:?}: {}", fb, err); + pub unsafe fn page_flip(&self) -> Result<(), Error<::Error>> { + let (result, fb) = { + let mut buffers = self.buffers.lock().unwrap(); + if buffers.next_buffer.is_some() { + // We cannot call lock_front_buffer anymore without releasing the previous buffer, which will happen when the page flip is done + warn!(self.logger, "Tried to swap with an already queued flip"); + return Err(Error::FrontBuffersExhausted); } - } - // if we re-created the surface, we need to commit the new changes, as we might trigger a modeset - let result = if self.recreated.get() { - debug!(self.logger, "Commiting new state"); - self.crtc.commit(fb).map_err(Error::Underlying) - } else { - trace!(self.logger, "Queueing Page flip"); - RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying) + // supporting only one buffer would cause a lot of inconvinience and + // would most likely result in a lot of flickering. + // neither weston, wlc or wlroots bother with that as well. + // so we just assume we got at least two buffers to do flipping. + let mut next_bo = self + .surface + .lock() + .unwrap() + .lock_front_buffer() + .map_err(|_| Error::FrontBufferLockFailed)?; + + // create a framebuffer if the front buffer does not have one already + // (they are reused by gbm) + let maybe_fb = next_bo + .userdata() + .map_err(|_| Error::InvalidInternalState)? + .cloned(); + let fb = if let Some(info) = maybe_fb { + info + } else { + let fb = self + .crtc + .add_planar_framebuffer(&next_bo, &[0; 4], 0) + .compat() + .map_err(Error::FramebufferCreationFailed)?; + next_bo.set_userdata(fb).unwrap(); + fb + }; + buffers.next_buffer = Some(next_bo); + + if cfg!(debug_assertions) { + if let Err(err) = self.crtc.get_framebuffer(fb) { + error!(self.logger, "Cached framebuffer invalid: {:?}: {}", fb, err); + } + } + + // if we re-created the surface, we need to commit the new changes, as we might trigger a modeset + ( + if self.recreated.load(Ordering::SeqCst) { + debug!(self.logger, "Commiting new state"); + self.crtc.commit(fb).map_err(Error::Underlying) + } else { + trace!(self.logger, "Queueing Page flip"); + RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying) + }, + fb, + ) }; // if it was successful, we can clear the re-created state match result { Ok(_) => { - self.recreated.set(false); - self.current_frame_buffer.set(Some(fb)); + self.recreated.store(false, Ordering::SeqCst); + let mut buffers = self.buffers.lock().unwrap(); + buffers.current_frame_buffer = Some(fb); Ok(()) } Err(err) => { @@ -106,7 +162,7 @@ impl GbmSurfaceInternal { } // this function is called, if we e.g. need to create the surface to match a new mode. - pub fn recreate(&self) -> Result<(), Error<<::Surface as Surface>::Error>> { + pub fn recreate(&self) -> Result<(), Error<::Error>> { let (w, h) = self.pending_mode().size(); // Recreate the surface and the related resources to match the new @@ -114,7 +170,8 @@ impl GbmSurfaceInternal { debug!(self.logger, "(Re-)Initializing surface (with mode: {}:{})", w, h); let surface = self .dev - .borrow_mut() + .lock() + .unwrap() .create_surface( w as u32, h as u32, @@ -127,9 +184,9 @@ impl GbmSurfaceInternal { self.clear_framebuffers(); // Drop the old surface after cleanup - *self.surface.borrow_mut() = surface; + *self.surface.lock().unwrap() = surface; - self.recreated.set(true); + self.recreated.store(true, Ordering::SeqCst); Ok(()) } @@ -137,7 +194,8 @@ impl GbmSurfaceInternal { // if the underlying drm-device is closed and re-opened framebuffers may get invalided. // here we clear them just to be sure, they get recreated on the next page_flip. pub fn clear_framebuffers(&self) { - if let Some(Ok(Some(fb))) = self.next_buffer.take().map(|mut bo| bo.take_userdata()) { + let mut buffers = self.buffers.lock().unwrap(); + if let Some(Ok(Some(fb))) = buffers.next_buffer.take().map(|mut bo| bo.take_userdata()) { if let Err(err) = self.crtc.destroy_framebuffer(fb) { warn!( self.logger, @@ -146,7 +204,7 @@ impl GbmSurfaceInternal { } } - if let Some(Ok(Some(fb))) = self.front_buffer.take().map(|mut bo| bo.take_userdata()) { + if let Some(Ok(Some(fb))) = buffers.front_buffer.take().map(|mut bo| bo.take_userdata()) { if let Err(err) = self.crtc.destroy_framebuffer(fb) { warn!( self.logger, @@ -157,9 +215,9 @@ impl GbmSurfaceInternal { } } -impl Surface for GbmSurfaceInternal { - type Connectors = <::Surface as Surface>::Connectors; - type Error = Error<<::Surface as Surface>::Error>; +impl Surface for GbmSurfaceInternal { + type Connectors = ::Connectors; + type Error = Error<::Error>; fn crtc(&self) -> crtc::Handle { self.crtc.crtc() @@ -199,13 +257,13 @@ impl Surface for GbmSurfaceInternal { } #[cfg(feature = "backend_drm")] -impl CursorBackend for GbmSurfaceInternal +impl CursorBackend for GbmSurfaceInternal where - ::Surface: CursorBackend, - <::Surface as CursorBackend>::Error: ::std::error::Error + Send, + S: CursorBackend, + ::Error: ::std::error::Error + Send, { type CursorFormat = ImageBuffer, Vec>; - type Error = Error<<::Surface as CursorBackend>::Error>; + type Error = Error<::Error>; fn set_cursor_position(&self, x: u32, y: u32) -> Result<(), Self::Error> { self.crtc.set_cursor_position(x, y).map_err(Error::Underlying) @@ -222,7 +280,8 @@ where // import the cursor into a buffer we can render let mut cursor = self .dev - .borrow_mut() + .lock() + .unwrap() .create_buffer_object( w, h, @@ -243,12 +302,12 @@ where .map_err(Error::Underlying)?; // and store it - self.cursor.set((cursor, hotspot)); + *self.cursor.lock().unwrap() = (cursor, hotspot); Ok(()) } } -impl Drop for GbmSurfaceInternal { +impl Drop for GbmSurfaceInternal { fn drop(&mut self) { // Drop framebuffers attached to the userdata of the gbm surface buffers. // (They don't implement drop, as they need the device) @@ -257,9 +316,9 @@ impl Drop for GbmSurfaceInternal { } /// Gbm surface for rendering -pub struct GbmSurface(pub(super) Rc>); +pub struct GbmSurface(pub(super) Arc>); -impl GbmSurface { +impl GbmSurface { /// Flips the underlying buffers. /// /// The surface will report being already flipped until the matching event @@ -295,9 +354,9 @@ impl GbmSurface { } } -impl Surface for GbmSurface { - type Connectors = <::Surface as Surface>::Connectors; - type Error = Error<<::Surface as Surface>::Error>; +impl Surface for GbmSurface { + type Connectors = ::Connectors; + type Error = Error<::Error>; fn crtc(&self) -> crtc::Handle { self.0.crtc() @@ -337,13 +396,13 @@ impl Surface for GbmSurface { } #[cfg(feature = "backend_drm")] -impl CursorBackend for GbmSurface +impl CursorBackend for GbmSurface where - ::Surface: CursorBackend, - <::Surface as CursorBackend>::Error: ::std::error::Error + Send, + S: CursorBackend, + ::Error: ::std::error::Error + Send, { type CursorFormat = ImageBuffer, Vec>; - type Error = Error<<::Surface as CursorBackend>::Error>; + type Error = Error<::Error>; fn set_cursor_position(&self, x: u32, y: u32) -> Result<(), Self::Error> { self.0.set_cursor_position(x, y) @@ -357,3 +416,17 @@ where self.0.set_cursor_representation(buffer, hotspot) } } + +#[cfg(test)] +mod test { + use super::GbmSurface; + use crate::backend::drm::legacy::LegacyDrmSurface; + use std::fs::File; + + fn is_send() {} + + #[test] + fn surface_is_send() { + is_send::>>(); + } +} diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 0b9c0c3..cc07a66 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -29,9 +29,9 @@ use nix::sys::stat::fstat; use std::cell::RefCell; use std::collections::HashMap; use std::os::unix::io::{AsRawFd, RawFd}; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use failure::{Fail, ResultExt}; @@ -44,7 +44,7 @@ pub mod session; /// Open raw drm device utilizing legacy mode-setting pub struct LegacyDrmDevice { - dev: Rc>, + dev: Arc>, dev_id: dev_t, active: Arc, backends: Rc>>>>, @@ -205,7 +205,7 @@ impl LegacyDrmDevice { } Ok(LegacyDrmDevice { - dev: Rc::new(dev), + dev: Arc::new(dev), dev_id, active, backends: Rc::new(RefCell::new(HashMap::new())), @@ -324,7 +324,7 @@ impl Device for LegacyDrmDevice { return Err(Error::SurfaceWithoutConnectors(crtc)); } - let backend = Rc::new(LegacyDrmSurfaceInternal::new( + let backend = Arc::new(LegacyDrmSurfaceInternal::new( self.dev.clone(), crtc, mode, @@ -332,7 +332,7 @@ impl Device for LegacyDrmDevice { self.logger.new(o!("crtc" => format!("{:?}", crtc))), )?); - self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); + self.backends.borrow_mut().insert(crtc, Arc::downgrade(&backend)); Ok(LegacyDrmSurface(backend)) } diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index 8c254c1..0571a50 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -13,7 +13,7 @@ use std::collections::{HashMap, HashSet}; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Weak as WeakArc}; use super::{Dev, DevPath, Error, LegacyDrmDevice, LegacyDrmSurfaceInternal}; use crate::{ @@ -25,18 +25,18 @@ use crate::{ /// linked to the [`LegacyDrmDevice`](LegacyDrmDevice) /// it was created from. pub(crate) struct LegacyDrmDeviceObserver { - dev: Weak>, + dev: WeakArc>, dev_id: dev_t, privileged: bool, active: Arc, - backends: Weak>>>>, + backends: Weak>>>>, logger: ::slog::Logger, } impl Linkable for LegacyDrmDevice { fn link(&mut self, signaler: Signaler) { let mut observer = LegacyDrmDeviceObserver { - dev: Rc::downgrade(&self.dev), + dev: Arc::downgrade(&self.dev), dev_id: self.dev_id, active: self.active.clone(), privileged: self.dev.privileged, @@ -69,7 +69,7 @@ impl LegacyDrmDeviceObserver { } if let Some(device) = self.dev.upgrade() { if let Some(backends) = self.backends.upgrade() { - for surface in backends.borrow().values().filter_map(Weak::upgrade) { + for surface in backends.borrow().values().filter_map(WeakArc::upgrade) { // other ttys that use no cursor, might not clear it themselves. // This makes sure our cursor won't stay visible. // @@ -139,7 +139,7 @@ impl LegacyDrmDeviceObserver { let mut used_connectors = HashSet::new(); if let Some(backends) = self.backends.upgrade() { - for surface in backends.borrow().values().filter_map(Weak::upgrade) { + for surface in backends.borrow().values().filter_map(WeakArc::upgrade) { let mut current = surface.state.write().unwrap(); let pending = surface.pending.read().unwrap(); diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 9e6917c..383e12e 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -7,8 +7,7 @@ use drm::Device as BasicDevice; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; -use std::rc::Rc; -use std::sync::{atomic::Ordering, RwLock}; +use std::sync::{atomic::Ordering, Arc, RwLock}; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; @@ -24,7 +23,7 @@ pub struct State { } pub(in crate::backend::drm) struct LegacyDrmSurfaceInternal { - pub(super) dev: Rc>, + pub(super) dev: Arc>, pub(in crate::backend::drm) crtc: crtc::Handle, pub(super) state: RwLock, pub(super) pending: RwLock, @@ -314,7 +313,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { impl LegacyDrmSurfaceInternal { pub(crate) fn new( - dev: Rc>, + dev: Arc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], @@ -472,7 +471,7 @@ impl Drop for LegacyDrmSurfaceInternal { /// Open raw crtc utilizing legacy mode-setting pub struct LegacyDrmSurface( - pub(in crate::backend::drm) Rc>, + pub(in crate::backend::drm) Arc>, ); impl AsRawFd for LegacyDrmSurface { @@ -555,3 +554,16 @@ impl RawSurface for LegacyDrmSurface { RawSurface::page_flip(&*self.0, framebuffer) } } + +#[cfg(test)] +mod test { + use super::LegacyDrmSurface; + use std::fs::File; + + fn is_send() {} + + #[test] + fn surface_is_send() { + is_send::>(); + } +} diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs index a1ddf6b..06cb889 100644 --- a/src/backend/egl/context.rs +++ b/src/backend/egl/context.rs @@ -7,7 +7,7 @@ use crate::backend::egl::{native, EGLSurface}; use crate::backend::graphics::PixelFormat; use std::os::raw::c_int; use std::ptr; -use std::sync::Arc; +use std::sync::{atomic::Ordering, Arc}; /// EGL context for rendering pub struct EGLContext { @@ -16,6 +16,9 @@ pub struct EGLContext { config_id: ffi::egl::types::EGLConfig, pixel_format: PixelFormat, } +// EGLContexts can be moved between threads safely +unsafe impl Send for EGLContext {} +unsafe impl Sync for EGLContext {} impl EGLContext { /// Create a new [`EGLContext`] from a given [`NativeDisplay`](native::NativeDisplay) @@ -124,8 +127,7 @@ impl EGLContext { where N: NativeSurface, { - let surface_ptr = surface.surface.get(); - + let surface_ptr = surface.surface.load(Ordering::SeqCst); wrap_egl_call(|| ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context)) .map(|_| ()) .map_err(Into::into) diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index 4880c04..79233d8 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -32,6 +32,9 @@ pub struct EGLDisplayHandle { /// ffi EGLDisplay ptr pub handle: ffi::egl::types::EGLDisplay, } +// EGLDisplay has an internal Mutex +unsafe impl Send for EGLDisplayHandle {} +unsafe impl Sync for EGLDisplayHandle {} impl Deref for EGLDisplayHandle { type Target = ffi::egl::types::EGLDisplay; diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index ea9a198..37bc7ff 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -4,22 +4,26 @@ use super::{ffi, native, EGLError, SurfaceCreationError, SwapBuffersError}; use crate::backend::egl::display::EGLDisplayHandle; use crate::backend::graphics::PixelFormat; use nix::libc::c_int; -use std::sync::Arc; -use std::{ - cell::Cell, - ops::{Deref, DerefMut}, +use std::ops::{Deref, DerefMut}; +use std::sync::{ + atomic::{AtomicPtr, Ordering}, + Arc, }; /// EGL surface of a given EGL context for rendering pub struct EGLSurface { pub(crate) display: Arc, native: N, - pub(crate) surface: Cell, + pub(crate) surface: AtomicPtr, config_id: ffi::egl::types::EGLConfig, pixel_format: PixelFormat, surface_attributes: Vec, logger: ::slog::Logger, } +// safe because EGLConfig can be moved between threads +// and the other types are thread-safe +unsafe impl Send for EGLSurface {} +unsafe impl Sync for EGLSurface {} impl Deref for EGLSurface { type Target = N; @@ -80,7 +84,7 @@ impl EGLSurface { Ok(EGLSurface { display, native, - surface: Cell::new(surface), + surface: AtomicPtr::new(surface as *mut _), config_id: config, pixel_format, surface_attributes, @@ -90,7 +94,7 @@ impl EGLSurface { /// Swaps buffers at the end of a frame. pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { - let surface = self.surface.get(); + let surface = self.surface.load(Ordering::SeqCst); let result = if !surface.is_null() { self.native.swap_buffers(&self.display, surface) @@ -106,21 +110,25 @@ impl EGLSurface { }; if self.native.needs_recreation() || surface.is_null() || is_bad_surface { - if !surface.is_null() { + let previous = self.surface.compare_and_swap( + surface, + unsafe { + self.native + .create(&self.display, self.config_id, &self.surface_attributes) + .map_err(|err| match err { + SurfaceCreationError::EGLSurfaceCreationFailed(err) => { + SwapBuffersError::EGLCreateWindowSurface(err) + } + SurfaceCreationError::NativeSurfaceCreationFailed(err) => { + SwapBuffersError::Underlying(err) + } + })? as *mut _ + }, + Ordering::SeqCst, + ); + if previous == surface && !surface.is_null() { let _ = unsafe { ffi::egl::DestroySurface(**self.display, surface as *const _) }; } - self.surface.set(unsafe { - self.native - .create(&self.display, self.config_id, &self.surface_attributes) - .map_err(|err| match err { - SurfaceCreationError::EGLSurfaceCreationFailed(err) => { - SwapBuffersError::EGLCreateWindowSurface(err) - } - SurfaceCreationError::NativeSurfaceCreationFailed(err) => { - SwapBuffersError::Underlying(err) - } - })? - }); // if a recreation is pending anyway, ignore page-flip errors. // lets see if we still fail after the next commit. @@ -135,9 +143,10 @@ impl EGLSurface { /// Returns true if the OpenGL surface is the current one in the thread. pub fn is_current(&self) -> bool { + let surface = self.surface.load(Ordering::SeqCst); unsafe { - ffi::egl::GetCurrentSurface(ffi::egl::DRAW as _) == self.surface.get() as *const _ - && ffi::egl::GetCurrentSurface(ffi::egl::READ as _) == self.surface.get() as *const _ + ffi::egl::GetCurrentSurface(ffi::egl::DRAW as _) == surface as *const _ + && ffi::egl::GetCurrentSurface(ffi::egl::READ as _) == surface as *const _ } } @@ -155,7 +164,7 @@ impl EGLSurface { impl Drop for EGLSurface { fn drop(&mut self) { unsafe { - ffi::egl::DestroySurface(**self.display, self.surface.get() as *const _); + ffi::egl::DestroySurface(**self.display, *self.surface.get_mut() as *const _); } } }