diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 372be36..45b36bb 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -87,19 +87,20 @@ unsafe impl NativeSurface for GbmSurface { self.0.surface.borrow().as_raw() as *const _ } - fn swap_buffers(&self, flip: F) -> ::std::result::Result<(), SwapBuffersError> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { - if self.0.crtc.commit_pending() || { - let fb = self.0.front_buffer.take(); - let res = fb.is_none(); - self.0.front_buffer.set(fb); - res - } { - self.recreate(flip).map_err(|_| SwapBuffersError::ContextLost) + fn needs_recreation(&self) -> bool { + self.0.crtc.commit_pending() + } + + fn recreate(&self) -> bool { + if let Err(err) = GbmSurface::recreate(self) { + error!(self.0.logger, "Failure recreating internal resources: {:?}", err); + false } else { - self.page_flip(flip) + true } } + + fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + self.page_flip() + } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a09043f..cbf4c63 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -128,7 +128,7 @@ impl Device for GbmDevice { info!(self.logger, "Initializing GbmSurface"); let drm_surface = Device::create_surface(&mut **self.dev.borrow_mut(), crtc, mode, connectors) - .chain_err(|| ErrorKind::UnderlyingBackendError)?; + .chain_err(|| ErrorKind::UnderlyingBackendError)?; let (w, h) = mode.size(); let surface = self @@ -161,6 +161,7 @@ impl Device for GbmDevice { 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)); diff --git a/src/backend/drm/gbm/session.rs b/src/backend/drm/gbm/session.rs index f66245d..1894a5f 100644 --- a/src/backend/drm/gbm/session.rs +++ b/src/backend/drm/gbm/session.rs @@ -45,11 +45,11 @@ impl { pub(super) current_frame_buffer: Cell>, pub(super) front_buffer: Cell>>, pub(super) next_buffer: Cell>>, + pub(super) recreated: Cell, pub(super) logger: ::slog::Logger, } @@ -29,17 +30,12 @@ impl GbmSurfaceInternal { 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 - if let Some(next_buffer) = self.next_buffer.replace(None) { - trace!(self.logger, "Releasing old front buffer"); - self.front_buffer.set(Some(next_buffer)); - // drop and release the old buffer - } + trace!(self.logger, "Releasing old front buffer"); + self.front_buffer.set(self.next_buffer.replace(None)); + // drop and release the old buffer } - pub fn page_flip(&self, flip: F) -> ::std::result::Result<(), SwapBuffersError> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { + pub fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> { let res = { let nb = self.next_buffer.take(); let res = nb.is_some(); @@ -52,9 +48,6 @@ impl GbmSurfaceInternal { return Err(SwapBuffersError::AlreadySwapped); } - // flip normally - flip()?; - // 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. @@ -81,17 +74,21 @@ impl GbmSurfaceInternal { self.next_buffer.set(Some(next_bo)); trace!(self.logger, "Queueing Page flip"); - self.crtc.page_flip(fb.handle())?; + if self.recreated.get() { + self.crtc + .commit(fb.handle()) + .map_err(|_| SwapBuffersError::ContextLost)?; + self.recreated.set(false); + } else { + self.crtc.page_flip(fb.handle())?; + } self.current_frame_buffer.set(Some(fb)); Ok(()) } - pub fn recreate(&self, flip: F) -> Result<()> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { + pub fn recreate(&self) -> Result<()> { let (w, h) = self.pending_mode().size(); // Recreate the surface and the related resources to match the new @@ -107,44 +104,17 @@ impl GbmSurfaceInternal { BufferObjectFlags::SCANOUT | BufferObjectFlags::RENDERING, ).chain_err(|| ErrorKind::SurfaceCreationFailed)?; - // Clean up next_buffer - { - if let Some(mut old_bo) = self.next_buffer.take() { - if let Ok(Some(fb)) = old_bo.take_userdata() { - if let Err(err) = framebuffer::destroy(&self.crtc, fb.handle()) { - warn!( - self.logger, - "Error releasing old back_buffer framebuffer: {:?}", err - ); - } - } + // Clean up buffers + if let Some(Ok(Some(fb))) = self.next_buffer.take().map(|mut bo| bo.take_userdata()) { + if let Err(err) = framebuffer::destroy(&self.crtc, fb.handle()) { + warn!( + self.logger, + "Error releasing old back_buffer framebuffer: {:?}", err + ); } } - flip()?; - - // Cleanup front_buffer and init the first screen on the new front_buffer - // (must be done before calling page_flip for the first time) - let old_front_bo = self.front_buffer.replace({ - let mut front_bo = surface - .lock_front_buffer() - .chain_err(|| ErrorKind::FrontBufferLockFailed)?; - - debug!(self.logger, "FrontBuffer color format: {:?}", front_bo.format()); - - // we also need a new framebuffer for the front buffer - let fb = framebuffer::create(&self.crtc, &*front_bo) - .chain_err(|| ErrorKind::UnderlyingBackendError)?; - - self.crtc - .commit(fb.handle()) - .chain_err(|| ErrorKind::UnderlyingBackendError)?; - - self.current_frame_buffer.set(Some(fb)); - front_bo.set_userdata(fb).unwrap(); - Some(front_bo) - }); - if let Some(Ok(Some(fb))) = old_front_bo.map(|mut bo| bo.take_userdata()) { + if let Some(Ok(Some(fb))) = self.front_buffer.take().map(|mut bo| bo.take_userdata()) { if let Err(err) = framebuffer::destroy(&self.crtc, fb.handle()) { warn!( self.logger, @@ -156,6 +126,8 @@ impl GbmSurfaceInternal { // Drop the old surface after cleanup *self.surface.borrow_mut() = surface; + self.recreated.set(true); + Ok(()) } } @@ -308,18 +280,12 @@ impl Drop for GbmSurfaceInternal { pub struct GbmSurface(pub(super) Rc>); impl GbmSurface { - pub fn page_flip(&self, flip: F) -> ::std::result::Result<(), SwapBuffersError> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { - self.0.page_flip(flip) + pub fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> { + self.0.page_flip() } - pub fn recreate(&self, flip: F) -> Result<()> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { - self.0.recreate(flip) + pub fn recreate(&self) -> Result<()> { + self.0.recreate() } } diff --git a/src/backend/egl/native.rs b/src/backend/egl/native.rs index 2cfb9f7..1edc445 100644 --- a/src/backend/egl/native.rs +++ b/src/backend/egl/native.rs @@ -167,14 +167,31 @@ unsafe impl NativeDisplay for WinitWindow { pub unsafe trait NativeSurface { /// Return a raw pointer egl will accept for surface creation. fn ptr(&self) -> ffi::NativeWindowType; + + /// Will be called to check if any internal resources will need + /// to be recreated. Old resources must be used until `recreate` + /// was called. + /// + /// Only needs to be recreated, if this shall sometimes return true. + /// The default implementation always returns false. + fn needs_recreation(&self) -> bool { + false + } + + /// Instructs the surface to recreate internal resources + /// + /// Must only be implemented if `needs_recreation` can return `true`. + /// Returns true on success. + /// If this call was successful `ptr()` *should* return something different. + fn recreate(&self) -> bool { + true + } + /// Adds additional semantics when calling EGLSurface::swap_buffers /// - /// Only implement if required by the backend, flip must be called during this call. - fn swap_buffers(&self, flip: F) -> ::std::result::Result<(), SwapBuffersError> - where - F: FnOnce() -> ::std::result::Result<(), SwapBuffersError>, - { - flip() + /// Only implement if required by the backend. + fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + Ok(()) } } diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 3e11ce5..3d6b15b 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -2,7 +2,9 @@ use super::{error::*, ffi, native, EGLContext}; use backend::graphics::SwapBuffersError; +use nix::libc::c_int; use std::{ + cell::Cell, ops::{Deref, DerefMut}, rc::{Rc, Weak}, }; @@ -12,7 +14,9 @@ pub struct EGLSurface { context: Weak, display: Weak, native: N, - surface: ffi::egl::types::EGLSurface, + surface: Cell, + config_id: ffi::egl::types::EGLConfig, + surface_attributes: Vec, } impl Deref for EGLSurface { @@ -50,28 +54,49 @@ impl EGLSurface { context: Rc::downgrade(&context.context), display: Rc::downgrade(&context.display), native, - surface, + surface: Cell::new(surface), + config_id: context.config_id, + surface_attributes: context.surface_attributes.clone(), }) } /// Swaps buffers at the end of a frame. pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.native.swap_buffers(|| { + let surface = self.surface.get(); + + if !surface.is_null() { if let Some(display) = self.display.upgrade() { - let ret = unsafe { ffi::egl::SwapBuffers((*display) as *const _, self.surface as *const _) }; + let ret = unsafe { ffi::egl::SwapBuffers((*display) as *const _, surface as *const _) }; if ret == 0 { match unsafe { ffi::egl::GetError() } as u32 { - ffi::egl::CONTEXT_LOST => Err(SwapBuffersError::ContextLost), - err => Err(SwapBuffersError::Unknown(err)), - } + ffi::egl::CONTEXT_LOST => return Err(SwapBuffersError::ContextLost), + err => return Err(SwapBuffersError::Unknown(err)), + }; } else { - Ok(()) + return Ok(()); } } else { - Err(SwapBuffersError::ContextLost) + return Err(SwapBuffersError::ContextLost); } - }) + self.native.swap_buffers()?; + }; + + if self.native.needs_recreation() || surface.is_null() { + if let Some(display) = self.display.upgrade() { + self.native.recreate(); + self.surface.set(unsafe { + ffi::egl::CreateWindowSurface( + *display, + self.config_id, + self.native.ptr(), + self.surface_attributes.as_ptr(), + ) + }); + } + } + + Ok(()) } /// Makes the OpenGL context the current context in the current thread. @@ -84,8 +109,8 @@ impl EGLSurface { if let (Some(display), Some(context)) = (self.display.upgrade(), self.context.upgrade()) { let ret = ffi::egl::MakeCurrent( (*display) as *const _, - self.surface as *const _, - self.surface as *const _, + self.surface.get() as *const _, + self.surface.get() as *const _, (*context) as *const _, ); @@ -106,8 +131,8 @@ impl EGLSurface { pub fn is_current(&self) -> bool { if self.context.upgrade().is_some() { unsafe { - ffi::egl::GetCurrentSurface(ffi::egl::DRAW as _) == self.surface as *const _ - && ffi::egl::GetCurrentSurface(ffi::egl::READ as _) == self.surface as *const _ + ffi::egl::GetCurrentSurface(ffi::egl::DRAW as _) == self.surface.get() as *const _ + && ffi::egl::GetCurrentSurface(ffi::egl::READ as _) == self.surface.get() as *const _ } } else { false @@ -119,7 +144,7 @@ impl Drop for EGLSurface { fn drop(&mut self) { if let Some(display) = self.display.upgrade() { unsafe { - ffi::egl::DestroySurface((*display) as *const _, self.surface as *const _); + ffi::egl::DestroySurface((*display) as *const _, self.surface.get() as *const _); } } }