From 31b6d844423a031f1da576baefba56524ba264c8 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Thu, 30 Apr 2020 00:24:35 +0200 Subject: [PATCH] WIP: Rework egl and glium errors --- anvil/src/glium_drawer.rs | 10 +-- examples/raw_legacy_drm.rs | 2 +- src/backend/drm/common/mod.rs | 20 ++++- src/backend/drm/egl/mod.rs | 28 ++++--- src/backend/drm/egl/session.rs | 4 +- src/backend/drm/egl/surface.rs | 15 +++- src/backend/drm/gbm/egl.rs | 37 ++++----- src/backend/drm/gbm/mod.rs | 21 +++++- src/backend/egl/context.rs | 63 ++++++---------- src/backend/egl/display.rs | 134 +++++++++++++-------------------- src/backend/egl/error.rs | 111 +++++++++++++++++++++++---- src/backend/egl/mod.rs | 89 ++++++++++++++++++++-- src/backend/egl/native.rs | 49 ++++++++---- src/backend/egl/surface.rs | 47 ++++++------ src/backend/graphics/glium.rs | 129 +++++++++++++++++++++++++++---- src/backend/graphics/mod.rs | 35 +++++---- src/backend/winit.rs | 14 ++-- 17 files changed, 537 insertions(+), 271 deletions(-) diff --git a/anvil/src/glium_drawer.rs b/anvil/src/glium_drawer.rs index b6e8f86..949634a 100644 --- a/anvil/src/glium_drawer.rs +++ b/anvil/src/glium_drawer.rs @@ -7,7 +7,7 @@ use glium::{ self, index::PrimitiveType, texture::{MipmapsOption, Texture2d, UncompressedFloatFormat}, - Frame, GlObject, Surface, + GlObject, Surface, }; use slog::Logger; @@ -16,7 +16,7 @@ use smithay::backend::egl::display::EGLBufferReader; use smithay::{ backend::{ egl::{BufferAccessError, EGLImages, Format}, - graphics::{gl::GLGraphicsBackend, glium::GliumGraphicsBackend}, + graphics::{gl::GLGraphicsBackend, glium::{GliumGraphicsBackend, Frame}}, }, reexports::wayland_server::protocol::{wl_buffer, wl_surface}, wayland::{ @@ -159,7 +159,7 @@ impl GliumDrawer { let images = if let Some(display) = &self.egl_buffer_reader.borrow().as_ref() { display.egl_buffer_contents(buffer) } else { - Err(BufferAccessError::NotManaged(buffer)) + Err(BufferAccessError::NotManaged(buffer, smithay::backend::egl::EGLError::BadDisplay)) }; match images { Ok(images) => { @@ -193,7 +193,7 @@ impl GliumDrawer { images: Some(images), // I guess we need to keep this alive ? }) } - Err(BufferAccessError::NotManaged(buffer)) => { + Err(BufferAccessError::NotManaged(buffer, _)) => { // this is not an EGL buffer, try SHM self.texture_from_shm_buffer(buffer) } @@ -235,7 +235,7 @@ impl GliumDrawer { pub fn render_texture( &self, - target: &mut glium::Frame, + target: &mut Frame, texture: &Texture2d, texture_kind: usize, y_inverted: bool, diff --git a/examples/raw_legacy_drm.rs b/examples/raw_legacy_drm.rs index f5dc06e..2d01b65 100644 --- a/examples/raw_legacy_drm.rs +++ b/examples/raw_legacy_drm.rs @@ -9,7 +9,7 @@ use smithay::{ common::Error, device_bind, legacy::{LegacyDrmDevice, LegacyDrmSurface}, - Device, DeviceHandler, RawSurface, Surface, + Device, DeviceHandler, RawSurface, }, reexports::{ calloop::EventLoop, diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 0c6f4eb..efa2516 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -4,8 +4,8 @@ //! use drm::control::{connector, crtc, Mode, RawResourceHandle}; - use std::path::PathBuf; +use crate::backend::graphics::SwapBuffersError; pub mod fallback; @@ -71,3 +71,21 @@ pub enum Error { #[error("Atomic Test failed for new properties on crtc ({0:?})")] TestFailed(crtc::Handle), } + +impl Into for Error { + fn into(self) -> SwapBuffersError { + match self { + x @ Error::DeviceInactive => SwapBuffersError::TemporaryFailure(Box::new(x)), + Error::Access { + errmsg: _, + dev: _, + source, + } if match source.get_ref() { + drm::SystemError::Unknown { errno: nix::errno::Errno::EBUSY } => true, + drm::SystemError::Unknown { errno: nix::errno::Errno::EINTR } => true, + _ => false, + } => SwapBuffersError::TemporaryFailure(Box::new(source)), + x => SwapBuffersError::ContextLost(Box::new(x)), + } + } +} diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index b0927e4..e0f147e 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -17,7 +17,7 @@ use wayland_server::Display; use super::{Device, DeviceHandler, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; -use crate::backend::egl::Error as EGLError; +use crate::backend::egl::{Error as EGLError, EGLError as RawEGLError, SurfaceCreationError}; #[cfg(feature = "use_system_lib")] use crate::backend::egl::{display::EGLBufferReader, EGLGraphicsBackend}; @@ -33,8 +33,11 @@ pub mod session; #[derive(thiserror::Error, Debug)] pub enum Error { /// EGL Error - #[error("EGL error: {0:?}")] + #[error("EGL error: {0:}")] EGL(#[source] EGLError), + /// EGL Error + #[error("EGL error: {0:}")] + RawEGL(#[source] RawEGLError), /// Underlying backend error #[error("Underlying backend error: {0:?}")] Underlying(#[source] U), @@ -46,7 +49,7 @@ type Arguments = (crtc::Handle, Mode, Vec); pub struct EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -58,7 +61,7 @@ where impl AsRawFd for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { fn as_raw_fd(&self) -> RawFd { @@ -69,7 +72,7 @@ where impl EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { /// Try to create a new [`EglDevice`] from an open device. @@ -124,7 +127,7 @@ where struct InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { handler: Box> + 'static>, @@ -133,7 +136,7 @@ where impl DeviceHandler for InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { type Device = D; @@ -149,7 +152,7 @@ where impl Device for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { type Surface = EglSurface<::Surface>; @@ -188,7 +191,10 @@ where context.get_config_id(), (crtc, mode, Vec::from(connectors)), ) - .map_err(Error::EGL)?; + .map_err(|err| match err { + SurfaceCreationError::EGLSurfaceCreationFailed(err) => Error::RawEGL(err), + SurfaceCreationError::NativeSurfaceCreationFailed(err) => Error::Underlying(err), + })?; Ok(EglSurface { context, surface }) } @@ -225,7 +231,7 @@ where impl EGLGraphicsBackend for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { fn bind_wl_display(&self, display: &Display) -> Result { @@ -236,7 +242,7 @@ where impl Drop for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { fn drop(&mut self) { diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs index 68006e7..4d34338 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -7,7 +7,7 @@ use drm::control::{crtc, connector, Mode}; use std::os::unix::io::RawFd; use super::EglDevice; -use crate::backend::drm::Device; +use crate::backend::drm::{Device, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; use crate::backend::session::{AsSessionObserver, SessionObserver}; @@ -22,7 +22,7 @@ impl AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay)> + AsSessionObserver + 'static, + D: Device + NativeDisplay), Error=<::Surface as Surface>::Error> + AsSessionObserver + 'static, ::Surface: NativeSurface, { fn observer(&mut self) -> EglDeviceObserver { diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs index 0cacac8..0824cba 100644 --- a/src/backend/drm/egl/surface.rs +++ b/src/backend/drm/egl/surface.rs @@ -1,5 +1,6 @@ use drm::control::{connector, crtc, Mode}; use nix::libc::c_void; +use std::convert::TryInto; use super::Error; use crate::backend::drm::Surface; @@ -21,8 +22,8 @@ where } impl Surface for EglSurface -where - N: NativeSurface + Surface, +where + N: native::NativeSurface + Surface, { type Connectors = ::Connectors; type Error = Error<::Error>; @@ -90,9 +91,15 @@ where impl GLGraphicsBackend for EglSurface where N: native::NativeSurface + Surface, + ::Error: Into + 'static, { fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.surface.swap_buffers() + if let Err(err) = self.surface.swap_buffers() { + Err(match err.try_into() { + Ok(x) => x, + Err(x) => x.into(), + }) + } else { Ok(()) } } fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -109,7 +116,7 @@ where } unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.context.make_current_with_surface(&self.surface) + self.context.make_current_with_surface(&self.surface).map_err(Into::into) } fn get_pixel_format(&self) -> PixelFormat { diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 085a1fd..d7043a2 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -7,8 +7,7 @@ use crate::backend::drm::{Device, RawDevice, Surface}; use crate::backend::egl::ffi; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; -use crate::backend::egl::Error as EglError; -use crate::backend::graphics::SwapBuffersError; +use crate::backend::egl::{Error as EglBackendError, EGLError, wrap_egl_call}; use super::{Error, GbmDevice, GbmSurface}; @@ -31,22 +30,22 @@ impl Backend for Gbm { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_gbm") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_gbm"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_KHR, display as *mut _, ptr::null())) } else if has_dp_extension("EGL_MESA_platform_gbm") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_MESA_platform_gbm"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null())) } else if has_dp_extension("EGL_MESA_platform_gbm") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_MESA_platform_gbm"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null())) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } @@ -59,7 +58,7 @@ unsafe impl NativeDisplay> for Gb true } - fn ptr(&self) -> Result { + fn ptr(&self) -> Result { Ok(self.dev.borrow().as_raw() as *const _) } @@ -69,6 +68,8 @@ unsafe impl NativeDisplay> for Gb } unsafe impl NativeSurface for GbmSurface { + type Error = Error<<::Surface as Surface>::Error>; + fn ptr(&self) -> ffi::NativeWindowType { self.0.surface.borrow().as_raw() as *const _ } @@ -77,25 +78,13 @@ unsafe impl NativeSurface for GbmSurface { self.needs_recreation() } - fn recreate(&self) -> bool { - if let Err(err) = GbmSurface::recreate(self) { - error!(self.0.logger, "Failure recreating internal resources: {}", err); - false - } else { - true - } + fn recreate(&self) -> Result<(), Self::Error> { + GbmSurface::recreate(self) } - fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + fn swap_buffers(&self) -> Result<(), Self::Error> { // this is safe since `eglSwapBuffers` will have been called exactly once // if this is used by our egl module, which is why this trait is unsafe. - match unsafe { self.page_flip() } { - Ok(()) => Ok(()), - Err(Error::FrontBuffersExhausted) => Err(SwapBuffersError::AlreadySwapped), - Err(err) => { - warn!(self.0.logger, "Page flip failed: {}", err); - Err(SwapBuffersError::Unknown(0)) - } - } + unsafe { self.page_flip() } } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a2d75b4..5d6d82e 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -10,6 +10,7 @@ //! use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface}; +use crate::backend::graphics::SwapBuffersError; use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Mode}; use drm::SystemError as DrmError; @@ -26,7 +27,7 @@ use std::sync::Once; /// Errors thrown by the [`GbmDevice`](::backend::drm::gbm::GbmDevice) /// and [`GbmSurface`](::backend::drm::gbm::GbmSurface). #[derive(thiserror::Error, Debug)] -pub enum Error { +pub enum Error { /// Creation of GBM device failed #[error("Creation of GBM device failed")] InitFailed(#[source] io::Error), @@ -256,3 +257,21 @@ impl Drop for GbmDevice { self.clear_handler(); } } + +impl Into for Error +where + E: std::error::Error + Into + 'static +{ + fn into(self) -> SwapBuffersError { + match self { + Error::FrontBuffersExhausted => SwapBuffersError::AlreadySwapped, + Error::FramebufferCreationFailed(x) if match x.get_ref() { + &drm::SystemError::Unknown { errno: nix::errno::Errno::EBUSY } => true, + &drm::SystemError::Unknown { errno: nix::errno::Errno::EINTR } => true, + _ => false + } => SwapBuffersError::TemporaryFailure(Box::new(x)), + Error::Underlying(x) => x.into(), + x => SwapBuffersError::ContextLost(Box::new(x)), + } + } +} \ No newline at end of file diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs index 938fece..7b43b69 100644 --- a/src/backend/egl/context.rs +++ b/src/backend/egl/context.rs @@ -1,10 +1,10 @@ //! EGL context related structs -use super::{ffi, Error}; +use super::{ffi, Error, MakeCurrentError, wrap_egl_call}; use crate::backend::egl::display::{EGLDisplay, EGLDisplayHandle}; use crate::backend::egl::native::NativeSurface; use crate::backend::egl::{native, EGLSurface}; -use crate::backend::graphics::{PixelFormat, SwapBuffersError}; +use crate::backend::graphics::PixelFormat; use std::os::raw::c_int; use std::ptr; use std::sync::Arc; @@ -93,21 +93,14 @@ impl EGLContext { trace!(log, "Creating EGL context..."); // TODO: Support shared contexts - let context = unsafe { + let context = wrap_egl_call(|| unsafe { ffi::egl::CreateContext( **display.display, config_id, ptr::null(), context_attributes.as_ptr(), ) - }; - - if context.is_null() { - match unsafe { ffi::egl::GetError() } as u32 { - ffi::egl::BAD_ATTRIBUTE => return Err(Error::CreationFailed), - err_no => return Err(Error::Unknown(err_no)), - } - } + }).map_err(Error::CreationFailed)?; info!(log, "EGL context created"); @@ -129,22 +122,13 @@ impl EGLContext { pub unsafe fn make_current_with_surface( &self, surface: &EGLSurface, - ) -> ::std::result::Result<(), SwapBuffersError> + ) -> Result<(), MakeCurrentError> where N: NativeSurface, { let surface_ptr = surface.surface.get(); - let ret = ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context); - - if ret == 0 { - match ffi::egl::GetError() as u32 { - ffi::egl::CONTEXT_LOST => Err(SwapBuffersError::ContextLost), - err => panic!("eglMakeCurrent failed (eglGetError returned 0x{:x})", err), - } - } else { - Ok(()) - } + wrap_egl_call(|| ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context)).map(|_| ()).map_err(Into::into) } /// Makes the OpenGL context the current context in the current thread with no surface bound. @@ -152,18 +136,9 @@ impl EGLContext { /// # Safety /// /// This function is marked unsafe, because the context cannot be made current - /// on multiple threads. - pub unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> { - let ret = ffi::egl::MakeCurrent(**self.display, ptr::null(), ptr::null(), self.context); - - if ret == 0 { - match ffi::egl::GetError() as u32 { - ffi::egl::CONTEXT_LOST => Err(SwapBuffersError::ContextLost), - err => panic!("eglMakeCurrent failed (eglGetError returned 0x{:x})", err), - } - } else { - Ok(()) - } + /// on multiple threads without being unbound again (see `unbind`) + pub unsafe fn make_current(&self) -> Result<(), MakeCurrentError> { + wrap_egl_call(|| ffi::egl::MakeCurrent(**self.display, ffi::egl::NO_SURFACE, ffi::egl::NO_SURFACE, self.context)).map(|_| ()).map_err(Into::into) } /// Returns true if the OpenGL context is the current one in the thread. @@ -180,16 +155,24 @@ impl EGLContext { pub fn get_pixel_format(&self) -> PixelFormat { self.pixel_format } + + /// Unbinds this context from the current thread, if set. + /// + /// This does nothing if this context is not the current context + pub fn unbind(&self) -> Result<(), MakeCurrentError> { + if self.is_current() { + wrap_egl_call(|| unsafe { ffi::egl::MakeCurrent(**self.display, ffi::egl::NO_SURFACE, ffi::egl::NO_SURFACE, ffi::egl::NO_CONTEXT)})?; + } + Ok(()) + } } impl Drop for EGLContext { fn drop(&mut self) { unsafe { // We need to ensure the context is unbound, otherwise it egl stalls the destroy call - if ffi::egl::GetCurrentContext() == self.context as *const _ { - ffi::egl::MakeCurrent(ptr::null(), ptr::null(), ptr::null(), ptr::null()); - } - + // ignore failures at this point + let _ = self.unbind(); ffi::egl::DestroyContext(**self.display, self.context); } } @@ -270,7 +253,7 @@ impl Default for PixelFormatRequirements { impl PixelFormatRequirements { /// Append the requirements to the given attribute list - pub fn create_attributes(&self, out: &mut Vec, logger: &slog::Logger) -> Result<(), Error> { + pub fn create_attributes(&self, out: &mut Vec, logger: &slog::Logger) -> Result<(), ()> { if let Some(hardware_accelerated) = self.hardware_accelerated { out.push(ffi::egl::CONFIG_CAVEAT as c_int); out.push(if hardware_accelerated { @@ -328,7 +311,7 @@ impl PixelFormatRequirements { if self.stereoscopy { error!(logger, "Stereoscopy is currently unsupported (sorry!)"); - return Err(Error::NoAvailablePixelFormat); + return Err(()); } Ok(()) diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index a098a92..c158bcc 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -3,7 +3,7 @@ #[cfg(feature = "use_system_lib")] use crate::backend::egl::EGLGraphicsBackend; use crate::backend::egl::{ - ffi, get_proc_address, native, BufferAccessError, EGLContext, EGLImages, EGLSurface, Error, Format, + ffi, get_proc_address, native, BufferAccessError, SurfaceCreationError, EGLContext, EGLImages, EGLSurface, Error, Format, EGLError, wrap_egl_call, }; use std::sync::Arc; @@ -44,6 +44,7 @@ impl Deref for EGLDisplayHandle { impl Drop for EGLDisplayHandle { fn drop(&mut self) { unsafe { + // ignore errors on drop ffi::egl::Terminate(self.handle); } } @@ -93,7 +94,7 @@ impl> EGLDisplay { // the first step is to query the list of extensions without any display, if supported let dp_extensions = unsafe { - let p = ffi::egl::QueryString(ffi::egl::NO_DISPLAY, ffi::egl::EXTENSIONS as i32); + let p = wrap_egl_call(|| ffi::egl::QueryString(ffi::egl::NO_DISPLAY, ffi::egl::EXTENSIONS as i32)).map_err(Error::InitFailed)?; // this possibility is available only with EGL 1.5 or EGL_EXT_platform_base, otherwise // `eglQueryString` returns an error @@ -105,23 +106,16 @@ impl> EGLDisplay { list.split(' ').map(|e| e.to_string()).collect::>() } }; - debug!(log, "EGL No-Display Extensions: {:?}", dp_extensions); - let display = - unsafe { B::get_display(ptr, |e: &str| dp_extensions.iter().any(|s| s == e), log.clone()) }; - if display == ffi::egl::NO_DISPLAY { - error!(log, "EGL Display is not valid"); - return Err(Error::DisplayNotSupported); - } + let display = unsafe { B::get_display(ptr, |e: &str| dp_extensions.iter().any(|s| s == e), log.clone()).map_err(Error::DisplayNotSupported)? }; let egl_version = { let mut major: MaybeUninit = MaybeUninit::uninit(); let mut minor: MaybeUninit = MaybeUninit::uninit(); - if unsafe { ffi::egl::Initialize(display, major.as_mut_ptr(), minor.as_mut_ptr()) } == 0 { - return Err(Error::InitFailed); - } + wrap_egl_call(|| unsafe { ffi::egl::Initialize(display, major.as_mut_ptr(), minor.as_mut_ptr()) }).map_err(Error::InitFailed)?; + let major = unsafe { major.assume_init() }; let minor = unsafe { minor.assume_init() }; @@ -134,19 +128,18 @@ impl> EGLDisplay { // the list of extensions supported by the client once initialized is different from the // list of extensions obtained earlier let extensions = if egl_version >= (1, 2) { - let p = unsafe { CStr::from_ptr(ffi::egl::QueryString(display, ffi::egl::EXTENSIONS as i32)) }; + let p = unsafe { CStr::from_ptr(wrap_egl_call(|| ffi::egl::QueryString(display, ffi::egl::EXTENSIONS as i32)).map_err(Error::InitFailed)?) }; let list = String::from_utf8(p.to_bytes().to_vec()).unwrap_or_else(|_| String::new()); list.split(' ').map(|e| e.to_string()).collect::>() } else { vec![] }; - info!(log, "EGL Extensions: {:?}", extensions); - if egl_version >= (1, 2) && unsafe { ffi::egl::BindAPI(ffi::egl::OPENGL_ES_API) } == 0 { - error!(log, "OpenGLES not supported by the underlying EGL implementation"); - return Err(Error::OpenGlesNotSupported); + if egl_version >= (1, 2) { + return Err(Error::OpenGlesNotSupported(None)); } + wrap_egl_call(|| unsafe { ffi::egl::BindAPI(ffi::egl::OPENGL_ES_API) }).map_err(|source| Error::OpenGlesNotSupported(Some(source)))?; Ok(EGLDisplay { native: RefCell::new(native), @@ -219,15 +212,15 @@ impl> EGLDisplay { } }; - reqs.create_attributes(&mut out, &self.logger)?; + reqs.create_attributes(&mut out, &self.logger).map_err(|()| Error::NoAvailablePixelFormat)?; out.push(ffi::egl::NONE as c_int); out }; // calling `eglChooseConfig` - let mut num_configs = unsafe { std::mem::zeroed() }; - if unsafe { + let mut num_configs = 0; + wrap_egl_call(|| unsafe { ffi::egl::ChooseConfig( **self.display, descriptor.as_ptr(), @@ -235,18 +228,13 @@ impl> EGLDisplay { 0, &mut num_configs, ) - } == 0 - { - return Err(Error::ConfigFailed); - } - + }).map_err(Error::ConfigFailed)?; if num_configs == 0 { return Err(Error::NoAvailablePixelFormat); } - let mut config_ids = Vec::with_capacity(num_configs as usize); - config_ids.resize_with(num_configs as usize, || unsafe { std::mem::zeroed() }); - if unsafe { + let mut config_ids: Vec = Vec::with_capacity(num_configs as usize); + wrap_egl_call(|| unsafe { ffi::egl::ChooseConfig( **self.display, descriptor.as_ptr(), @@ -254,44 +242,43 @@ impl> EGLDisplay { num_configs, &mut num_configs, ) - } == 0 - { - return Err(Error::ConfigFailed); - } + }).map_err(Error::ConfigFailed)?; + unsafe { config_ids.set_len(num_configs as usize); } // TODO: Deeper swap intervals might have some uses let desired_swap_interval = if attributes.vsync { 1 } else { 0 }; let config_ids = config_ids .into_iter() - .filter(|&config| unsafe { + .map(|config| unsafe { let mut min_swap_interval = 0; - ffi::egl::GetConfigAttrib( + wrap_egl_call(|| ffi::egl::GetConfigAttrib( **self.display, config, ffi::egl::MIN_SWAP_INTERVAL as ffi::egl::types::EGLint, &mut min_swap_interval, - ); + ))?; if desired_swap_interval < min_swap_interval { - return false; + return Ok(None); } let mut max_swap_interval = 0; - ffi::egl::GetConfigAttrib( + wrap_egl_call(|| ffi::egl::GetConfigAttrib( **self.display, config, ffi::egl::MAX_SWAP_INTERVAL as ffi::egl::types::EGLint, &mut max_swap_interval, - ); + ))?; if desired_swap_interval > max_swap_interval { - return false; + return Ok(None); } - true + Ok(Some(config)) }) - .collect::>(); + .collect::>, EGLError>>().map_err(Error::ConfigFailed)? + .into_iter().flat_map(|x| x).collect::>(); if config_ids.is_empty() { return Err(Error::NoAvailablePixelFormat); @@ -304,15 +291,12 @@ impl> EGLDisplay { macro_rules! attrib { ($display:expr, $config:expr, $attr:expr) => {{ let mut value = MaybeUninit::uninit(); - let res = ffi::egl::GetConfigAttrib( + wrap_egl_call(|| ffi::egl::GetConfigAttrib( **$display, $config, $attr as ffi::egl::types::EGLint, value.as_mut_ptr(), - ); - if res == 0 { - return Err(Error::ConfigFailed); - } + )).map_err(Error::ConfigFailed)?; value.assume_init() }}; }; @@ -357,12 +341,9 @@ impl> EGLDisplay { double_buffer: Option, config: ffi::egl::types::EGLConfig, args: N::Arguments, - ) -> Result, Error> { + ) -> Result, SurfaceCreationError> { trace!(self.logger, "Creating EGL window surface."); - let surface = self.native.borrow_mut().create_surface(args).map_err(|e| { - error!(self.logger, "EGL surface creation failed: {}", e); - Error::SurfaceCreationFailed - })?; + let surface = self.native.borrow_mut().create_surface(args).map_err(SurfaceCreationError::NativeSurfaceCreationFailed)?; EGLSurface::new( self.display.clone(), @@ -375,7 +356,7 @@ impl> EGLDisplay { .map(|x| { debug!(self.logger, "EGL surface successfully created"); x - }) + }).map_err(SurfaceCreationError::EGLSurfaceCreationFailed) } /// Returns the runtime egl version of this display @@ -426,10 +407,7 @@ impl> EGLGraphicsBackend for EGL if !self.extensions.iter().any(|s| s == "EGL_WL_bind_wayland_display") { return Err(Error::EglExtensionNotSupported(&["EGL_WL_bind_wayland_display"])); } - let res = unsafe { ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _) }; - if res == 0 { - return Err(Error::OtherEGLDisplayAlreadyBound); - } + wrap_egl_call(|| unsafe { ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _) }).map_err(Error::OtherEGLDisplayAlreadyBound)?; Ok(EGLBufferReader::new(self.display.clone(), display.c_ptr())) } } @@ -469,16 +447,15 @@ impl EGLBufferReader { buffer: WlBuffer, ) -> ::std::result::Result { let mut format: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::EGL_TEXTURE_FORMAT, &mut format, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; + let format = match format { x if x == ffi::egl::TEXTURE_RGB as i32 => Format::RGB, x if x == ffi::egl::TEXTURE_RGBA as i32 => Format::RGBA, @@ -490,40 +467,34 @@ impl EGLBufferReader { }; let mut width: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::WIDTH as i32, &mut width, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut height: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::HEIGHT as i32, &mut height, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut inverted: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::WAYLAND_Y_INVERTED_WL, &mut inverted, - ) != 0 - } { - inverted = 1; - } + ) + }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut images = Vec::with_capacity(format.num_planes()); for i in 0..format.num_planes() { @@ -533,7 +504,7 @@ impl EGLBufferReader { out.push(ffi::egl::NONE as i32); images.push({ - let image = unsafe { + let image = wrap_egl_call(|| unsafe { ffi::egl::CreateImageKHR( **self.display, ffi::egl::NO_CONTEXT, @@ -541,12 +512,8 @@ impl EGLBufferReader { buffer.as_ref().c_ptr() as *mut _, out.as_ptr(), ) - }; - if image == ffi::egl::NO_IMAGE_KHR { - return Err(BufferAccessError::EGLImageCreationFailed); - } else { - image - } + }).map_err(BufferAccessError::EGLImageCreationFailed)?; + image }); } @@ -601,6 +568,7 @@ impl Drop for EGLBufferReader { fn drop(&mut self) { if !self.wayland.is_null() { unsafe { + // ignore errors on drop ffi::egl::UnbindWaylandDisplayWL(**self.display, self.wayland as _); } } diff --git a/src/backend/egl/error.rs b/src/backend/egl/error.rs index f0b0e7a..0254628 100644 --- a/src/backend/egl/error.rs +++ b/src/backend/egl/error.rs @@ -1,3 +1,5 @@ +use super::ffi; + #[derive(thiserror::Error, Debug)] /// EGL errors pub enum Error { @@ -5,8 +7,8 @@ pub enum Error { #[error("The requested OpenGL version {0:?} is not supported")] OpenGlVersionNotSupported((u8, u8)), /// The EGL implementation does not support creating OpenGL ES contexts - #[error("The EGL implementation does not support creating OpenGL ES contexts")] - OpenGlesNotSupported, + #[error("The EGL implementation does not support creating OpenGL ES contexts. Err: {0:?}")] + OpenGlesNotSupported(#[source] Option), /// No available pixel format matched the criteria #[error("No available pixel format matched the criteria")] NoAvailablePixelFormat, @@ -14,26 +16,23 @@ pub enum Error { #[error("The expected backend '{0:?}' does not match the runtime")] NonMatchingBackend(&'static str), /// Unable to obtain a valid EGL Display - #[error("Unable to obtain a valid EGL Display")] - DisplayNotSupported, + #[error("Unable to obtain a valid EGL Display. Err: {0:}")] + DisplayNotSupported(#[source] EGLError), /// `eglInitialize` returned an error - #[error("Failed to initialize EGL")] - InitFailed, + #[error("Failed to initialize EGL. Err: {0:}")] + InitFailed(#[source] EGLError), /// Failed to configure the EGL context #[error("Failed to configure the EGL context")] - ConfigFailed, + ConfigFailed(#[source] EGLError), /// Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements - #[error("Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements")] - CreationFailed, - /// `eglCreateWindowSurface` failed - #[error("`eglCreateWindowSurface` failed")] - SurfaceCreationFailed, + #[error("Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements. Err: {0:}")] + CreationFailed(#[source] EGLError), /// The required EGL extension is not supported by the underlying EGL implementation #[error("None of the following EGL extensions is supported by the underlying EGL implementation, at least one is required: {0:?}")] EglExtensionNotSupported(&'static [&'static str]), /// Only one EGLDisplay may be bound to a given `WlDisplay` at any time #[error("Only one EGLDisplay may be bound to a given `WlDisplay` at any time")] - OtherEGLDisplayAlreadyBound, + OtherEGLDisplayAlreadyBound(#[source] EGLError), /// No EGLDisplay is currently bound to this `WlDisplay` #[error("No EGLDisplay is currently bound to this `WlDisplay`")] NoEGLDisplayBound, @@ -43,7 +42,89 @@ pub enum Error { /// Failed to create `EGLImages` from the buffer #[error("Failed to create `EGLImages` from the buffer")] EGLImageCreationFailed, - /// The reason of failure could not be determined - #[error("Unknown error: {0}")] +} + +/// Raw EGL error +#[derive(thiserror::Error, Debug)] +pub enum EGLError { + /// EGL is not initialized, or could not be initialized, for the specified EGL display connection. + #[error("EGL is not initialized, or could not be initialized, for the specified EGL display connection.")] + NotInitialized, + /// EGL cannot access a requested resource (for example a context is bound in another thread). + #[error("EGL cannot access a requested resource (for example a context is bound in another thread).")] + BadAccess, + /// EGL failed to allocate resources for the requested operation. + #[error("EGL failed to allocate resources for the requested operation.")] + BadAlloc, + /// An unrecognized attribute or attribute value was passed in the attribute list. + #[error("An unrecognized attribute or attribute value was passed in the attribute list.")] + BadAttribute, + /// An EGLContext argument does not name a valid EGL rendering context. + #[error("An EGLContext argument does not name a valid EGL rendering context.")] + BadContext, + /// An EGLConfig argument does not name a valid EGL frame buffer configuration. + #[error("An EGLConfig argument does not name a valid EGL frame buffer configuration.")] + BadConfig, + /// The current surface of the calling thread is a window, pixel buffer or pixmap that is no longer valid. + #[error("The current surface of the calling thread is a window, pixel buffer or pixmap that is no longer valid.")] + BadCurrentSurface, + /// An EGLDisplay argument does not name a valid EGL display connection. + #[error("An EGLDisplay argument does not name a valid EGL display connection.")] + BadDisplay, + /// An EGLSurface argument does not name a valid surface (window, pixel buffer or pixmap) configured for GL rendering. + #[error("An EGLSurface argument does not name a valid surface (window, pixel buffer or pixmap) configured for GL rendering.")] + BadSurface, + /// Arguments are inconsistent (for example, a valid context requires buffers not supplied by a valid surface). + #[error("Arguments are inconsistent (for example, a valid context requires buffers not supplied by a valid surface).")] + BadMatch, + /// One or more argument values are invalid. + #[error("One or more argument values are invalid.")] + BadParameter, + /// A NativePixmapType argument does not refer to a valid native pixmap. + #[error("A NativePixmapType argument does not refer to a valid native pixmap.")] + BadNativePixmap, + /// A NativeWindowType argument does not refer to a valid native window. + #[error("A NativeWindowType argument does not refer to a valid native window.")] + BadNativeWindow, + /// A power management event has occurred. The application must destroy all contexts and reinitialise OpenGL ES state and objects to continue rendering. + #[error("A power management event has occurred. The application must destroy all contexts and reinitialise OpenGL ES state and objects to continue rendering.")] + ContextLost, + /// An unknown error + #[error("An unknown error ({0:x})")] Unknown(u32), } + +impl From for EGLError { + fn from(value: u32) -> Self { + match value { + ffi::egl::NOT_INITIALIZED => EGLError::NotInitialized, + ffi::egl::BAD_ACCESS => EGLError::BadAccess, + ffi::egl::BAD_ALLOC => EGLError::BadAlloc, + ffi::egl::BAD_ATTRIBUTE => EGLError::BadAttribute, + ffi::egl::BAD_CONTEXT => EGLError::BadContext, + ffi::egl::BAD_CURRENT_SURFACE => EGLError::BadCurrentSurface, + ffi::egl::BAD_DISPLAY => EGLError::BadDisplay, + ffi::egl::BAD_SURFACE => EGLError::BadSurface, + ffi::egl::BAD_MATCH => EGLError::BadMatch, + ffi::egl::BAD_PARAMETER => EGLError::BadParameter, + ffi::egl::BAD_NATIVE_PIXMAP => EGLError::BadNativePixmap, + ffi::egl::BAD_NATIVE_WINDOW => EGLError::BadNativeWindow, + ffi::egl::CONTEXT_LOST => EGLError::ContextLost, + x => EGLError::Unknown(x), + } + } +} + +impl EGLError { + fn from_last_call() -> Result<(), EGLError> { + match unsafe { ffi::egl::GetError() as u32 } { + ffi::egl::SUCCESS => Ok(()), + x => Err(EGLError::from(x)), + } + } +} + +pub(crate) fn wrap_egl_call R>(call: F) -> Result { + let res = call(); + EGLError::from_last_call().map(|()| res) +} \ No newline at end of file diff --git a/src/backend/egl/mod.rs b/src/backend/egl/mod.rs index 506e924..6000c92 100644 --- a/src/backend/egl/mod.rs +++ b/src/backend/egl/mod.rs @@ -19,7 +19,7 @@ //! of an EGL-based [`WlBuffer`](wayland_server::protocol::wl_buffer::WlBuffer) for rendering. #[cfg(feature = "renderer_gl")] -use crate::backend::graphics::gl::{ffi as gl_ffi, GLGraphicsBackend}; +use crate::backend::graphics::{SwapBuffersError as GraphicsSwapBuffersError, gl::{ffi as gl_ffi, GLGraphicsBackend}}; use nix::libc::c_uint; use std::fmt; #[cfg(feature = "wayland_frontend")] @@ -28,7 +28,7 @@ use wayland_server::{protocol::wl_buffer::WlBuffer, Display}; pub mod context; pub use self::context::EGLContext; mod error; -pub use self::error::Error; +pub use self::error::*; use nix::libc::c_void; @@ -84,11 +84,11 @@ pub enum BufferAccessError { #[error("The corresponding context was lost")] ContextLost, /// This buffer is not managed by the EGL buffer - #[error("This buffer is not managed by EGL")] - NotManaged(WlBuffer), + #[error("This buffer is not managed by EGL. Err: {1:}")] + NotManaged(WlBuffer, #[source] EGLError), /// Failed to create `EGLImages` from the buffer - #[error("Failed to create EGLImages from the buffer")] - EGLImageCreationFailed, + #[error("Failed to create EGLImages from the buffer. Err: {0:}")] + EGLImageCreationFailed(#[source] EGLError), /// The required EGL extension is not supported by the underlying EGL implementation #[error("{0}")] EglExtensionNotSupported(#[from] EglExtensionNotSupportedError), @@ -99,8 +99,8 @@ impl fmt::Debug for BufferAccessError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> ::std::result::Result<(), fmt::Error> { match *self { BufferAccessError::ContextLost => write!(formatter, "BufferAccessError::ContextLost"), - BufferAccessError::NotManaged(_) => write!(formatter, "BufferAccessError::NotManaged"), - BufferAccessError::EGLImageCreationFailed => { + BufferAccessError::NotManaged(_, _) => write!(formatter, "BufferAccessError::NotManaged"), + BufferAccessError::EGLImageCreationFailed(_) => { write!(formatter, "BufferAccessError::EGLImageCreationFailed") } BufferAccessError::EglExtensionNotSupported(ref err) => write!(formatter, "{:?}", err), @@ -108,6 +108,78 @@ impl fmt::Debug for BufferAccessError { } } +/// Error that can occur when creating a surface. +#[derive(Debug, thiserror::Error)] +pub enum SurfaceCreationError { + /// Native Surface creation failed + #[error("Surface creation failed. Err: {0:}")] + NativeSurfaceCreationFailed(#[source] E), + /// EGL surface creation failed + #[error("EGL surface creation failed. Err: {0:}")] + EGLSurfaceCreationFailed(#[source] EGLError) +} + +/// Error that can happen when swapping buffers. +#[derive(Debug, thiserror::Error)] +pub enum SwapBuffersError { + /// Error of the underlying native surface + #[error("Underlying error: {0:?}")] + Underlying(#[source] E), + /// EGL error during `eglSwapBuffers` + #[error("{0:}")] + EGLSwapBuffers(#[source] EGLError), + /// EGL error during `eglCreateWindowSurface` + #[error("{0:}")] + EGLCreateWindowSurface(#[source] EGLError), +} + +impl std::convert::TryFrom> for GraphicsSwapBuffersError { + type Error=E; + fn try_from(value: SwapBuffersError) -> Result { + match value { + // bad surface is answered with a surface recreation in `swap_buffers` + x @ SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface) => Ok(GraphicsSwapBuffersError::TemporaryFailure(Box::new(x))), + // the rest is either never happening or are unrecoverable + x @ SwapBuffersError::EGLSwapBuffers(_) => Ok(GraphicsSwapBuffersError::ContextLost(Box::new(x))), + x @ SwapBuffersError::EGLCreateWindowSurface(_) => Ok(GraphicsSwapBuffersError::ContextLost(Box::new(x))), + SwapBuffersError::Underlying(e) => Err(e), + } + } +} + +/// Error that can happen when making a context (and surface) current on the active thread. +#[derive(thiserror::Error, Debug)] +#[error("`eglMakeCurrent` failed: {0}")] +pub struct MakeCurrentError(#[from] EGLError); + +impl From for GraphicsSwapBuffersError { + fn from(err: MakeCurrentError) -> GraphicsSwapBuffersError { + match err { + /* + From khronos docs: + If draw or read are not compatible with context, then an EGL_BAD_MATCH error is generated. + If context is current to some other thread, or if either draw or read are bound to contexts in another thread, an EGL_BAD_ACCESS error is generated. + If binding context would exceed the number of current contexts of that client API type supported by the implementation, an EGL_BAD_ACCESS error is generated. + If either draw or read are pbuffers created with eglCreatePbufferFromClientBuffer, and the underlying bound client API buffers are in use by the client API that created them, an EGL_BAD_ACCESS error is generated. + + Except for the first case all of these recoverable. This conversation is mostly used in winit & EglSurface, where compatible context and surfaces are build. + */ + x @ MakeCurrentError(EGLError::BadAccess) => GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)), + // BadSurface would result in a recreation in `eglSwapBuffers` -> recoverable + x @ MakeCurrentError(EGLError::BadSurface) => GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)), + /* + From khronos docs: + If the previous context of the calling thread has unflushed commands, and the previous surface is no longer valid, an EGL_BAD_CURRENT_SURFACE error is generated. + + This does not consern this or future `makeCurrent`-calls. + */ + x @ MakeCurrentError(EGLError::BadCurrentSurface) => GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)), + // the rest is either never happening or are unrecoverable + x => GraphicsSwapBuffersError::ContextLost(Box::new(x)), + } + } +} + /// Error that might happen when binding an `EGLImage` to a GL texture #[derive(Debug, Clone, PartialEq, thiserror::Error)] pub enum TextureCreationError { @@ -249,6 +321,7 @@ impl EGLImages { impl Drop for EGLImages { fn drop(&mut self) { for image in self.images.drain(..) { + // ignore result on drop unsafe { ffi::egl::DestroyImageKHR(**self.display, image); } diff --git a/src/backend/egl/native.rs b/src/backend/egl/native.rs index b7a6841..09fa58a 100644 --- a/src/backend/egl/native.rs +++ b/src/backend/egl/native.rs @@ -1,7 +1,6 @@ //! Type safe native types for safe context/surface creation -use super::{ffi, Error}; -use crate::backend::graphics::SwapBuffersError; +use super::{ffi, Error, EGLError, wrap_egl_call}; #[cfg(feature = "backend_winit")] use std::ptr; @@ -28,7 +27,7 @@ pub trait Backend { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay; + ) -> Result; } #[cfg(feature = "backend_winit")] @@ -42,20 +41,20 @@ impl Backend for Wayland { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_wayland") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_wayland"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_WAYLAND_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_WAYLAND_KHR, display as *mut _, ptr::null())) } else if has_dp_extension("EGL_EXT_platform_wayland") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_EXT_platform_wayland"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_WAYLAND_EXT, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_WAYLAND_EXT, display as *mut _, ptr::null())) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } @@ -74,19 +73,19 @@ impl Backend for X11 { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_x11") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_x11"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_X11_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_X11_KHR, display as *mut _, ptr::null())) } else if has_dp_extension("EGL_EXT_platform_x11") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_EXT_platform_x11"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_X11_EXT, display as *mut _, ptr::null()) + wrap_egl_call(|| ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_X11_EXT, display as *mut _, ptr::null())) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } @@ -107,7 +106,7 @@ pub unsafe trait NativeDisplay { /// Return a raw pointer EGL will accept for context creation. fn ptr(&self) -> Result; /// Create a surface - fn create_surface(&mut self, args: Self::Arguments) -> ::std::result::Result; + fn create_surface(&mut self, args: Self::Arguments) -> Result; } #[cfg(feature = "backend_winit")] @@ -166,6 +165,9 @@ unsafe impl NativeDisplay for WinitWindow { /// The returned [`NativeWindowType`](ffi::NativeWindowType) must be valid for EGL /// and there is no way to test that. pub unsafe trait NativeSurface { + /// Error of the underlying surface + type Error: std::error::Error; + /// Return a raw pointer egl will accept for surface creation. fn ptr(&self) -> ffi::NativeWindowType; @@ -184,21 +186,33 @@ pub unsafe trait NativeSurface { /// 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 + fn recreate(&self) -> Result<(), Self::Error> { + Ok(()) } /// Adds additional semantics when calling /// [EGLSurface::swap_buffers](::backend::egl::surface::EGLSurface::swap_buffers) /// /// Only implement if required by the backend. - fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + fn swap_buffers(&self) -> Result<(), Self::Error> { Ok(()) } } +/// Hack until ! gets stablized +#[derive(Debug)] +pub enum Never {} +impl std::fmt::Display for Never { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { unreachable!() } +} +impl std::error::Error for Never {} + #[cfg(feature = "backend_winit")] unsafe impl NativeSurface for XlibWindow { + // this would really be a case for this: + // type Error = !; (https://github.com/rust-lang/rust/issues/35121) + type Error = Never; + fn ptr(&self) -> ffi::NativeWindowType { self.0 as *const _ } @@ -206,7 +220,10 @@ unsafe impl NativeSurface for XlibWindow { #[cfg(feature = "backend_winit")] unsafe impl NativeSurface for wegl::WlEglSurface { + // type Error = !; + type Error = Never; + fn ptr(&self) -> ffi::NativeWindowType { self.ptr() as *const _ } -} +} \ No newline at end of file diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 1efe67c..3596ebf 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -1,8 +1,8 @@ //! EGL surface related structs -use super::{ffi, native, Error}; +use super::{ffi, native, EGLError, SwapBuffersError, wrap_egl_call}; use crate::backend::egl::display::EGLDisplayHandle; -use crate::backend::graphics::{PixelFormat, SwapBuffersError}; +use crate::backend::graphics::PixelFormat; use nix::libc::c_int; use std::sync::Arc; use std::{ @@ -41,7 +41,7 @@ impl EGLSurface { config: ffi::egl::types::EGLConfig, native: N, log: L, - ) -> Result, Error> + ) -> Result, EGLError> where L: Into>, { @@ -68,13 +68,9 @@ impl EGLSurface { out }; - let surface = unsafe { + let surface = wrap_egl_call(|| unsafe { ffi::egl::CreateWindowSurface(**display, config, native.ptr(), surface_attributes.as_ptr()) - }; - - if surface.is_null() { - return Err(Error::SurfaceCreationFailed); - } + })?; Ok(EGLSurface { display, @@ -87,35 +83,34 @@ impl EGLSurface { } /// Swaps buffers at the end of a frame. - pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { let surface = self.surface.get(); - if !surface.is_null() { - let ret = unsafe { ffi::egl::SwapBuffers(**self.display, surface as *const _) }; + let result = if !surface.is_null() { + wrap_egl_call(|| unsafe { ffi::egl::SwapBuffers(**self.display, surface as *const _) }) + .map_err(SwapBuffersError::EGLSwapBuffers) + .and_then(|_| self.native.swap_buffers().map_err(SwapBuffersError::Underlying)) + } else { Ok(()) }; - if ret == 0 { - match unsafe { ffi::egl::GetError() } as u32 { - ffi::egl::CONTEXT_LOST => return Err(SwapBuffersError::ContextLost), - err => return Err(SwapBuffersError::Unknown(err)), - }; - } else { - self.native.swap_buffers()?; + // workaround for missing `PartialEq` impl + let is_bad_surface = if let Err(SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface)) = result { true } else { false }; + + if self.native.needs_recreation() || surface.is_null() || is_bad_surface { + self.native.recreate().map_err(SwapBuffersError::Underlying)?; + if !surface.is_null() { + let _ = unsafe { ffi::egl::DestroySurface(**self.display, surface as *const _) }; } - }; - - if self.native.needs_recreation() || surface.is_null() { - self.native.recreate(); self.surface.set(unsafe { - ffi::egl::CreateWindowSurface( + wrap_egl_call(|| ffi::egl::CreateWindowSurface( **self.display, self.config_id, self.native.ptr(), self.surface_attributes.as_ptr(), - ) + )).map_err(SwapBuffersError::EGLCreateWindowSurface)? }); } - Ok(()) + result } /// Returns true if the OpenGL surface is the current one in the thread. diff --git a/src/backend/graphics/glium.rs b/src/backend/graphics/glium.rs index 5299683..32be58a 100644 --- a/src/backend/graphics/glium.rs +++ b/src/backend/graphics/glium.rs @@ -4,35 +4,30 @@ use crate::backend::graphics::{gl::GLGraphicsBackend, SwapBuffersError}; use glium::{ backend::{Backend, Context, Facade}, debug::DebugCallbackBehavior, - Frame, SwapBuffersError as GliumSwapBuffersError, + SwapBuffersError as GliumSwapBuffersError, }; use std::{ - cell::{Ref, RefCell, RefMut}, + cell::{Cell, Ref, RefCell, RefMut}, os::raw::c_void, rc::Rc, }; -impl From for GliumSwapBuffersError { - fn from(error: SwapBuffersError) -> Self { - match error { - SwapBuffersError::ContextLost => GliumSwapBuffersError::ContextLost, - SwapBuffersError::AlreadySwapped => GliumSwapBuffersError::AlreadySwapped, - SwapBuffersError::Unknown(_) => GliumSwapBuffersError::ContextLost, // TODO - } - } -} - /// Wrapper to expose `Glium` compatibility pub struct GliumGraphicsBackend { context: Rc, backend: Rc>, + // at least this type is not `Send` or even `Sync`. + // while there can be multiple Frames, they cannot in parallel call `set_finish`. + // so a buffer of the last error is sufficient, if always cleared... + error_channel: Rc>>>, } -struct InternalBackend(RefCell); +struct InternalBackend(RefCell, Rc>>>); impl GliumGraphicsBackend { fn new(backend: T) -> GliumGraphicsBackend { - let internal = Rc::new(InternalBackend(RefCell::new(backend))); + let error_channel = Rc::new(Cell::new(None)); + let internal = Rc::new(InternalBackend(RefCell::new(backend), error_channel.clone())); GliumGraphicsBackend { // cannot fail @@ -40,6 +35,7 @@ impl GliumGraphicsBackend { Context::new(internal.clone(), true, DebugCallbackBehavior::default()).unwrap() }, backend: internal, + error_channel, } } @@ -51,7 +47,7 @@ impl GliumGraphicsBackend { /// Note that destroying a [`Frame`] is immediate, even if vsync is enabled. #[inline] pub fn draw(&self) -> Frame { - Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()) + Frame(glium::Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()), self.error_channel.clone()) } /// Borrow the underlying backend. @@ -88,7 +84,20 @@ impl From for GliumGraphicsBackend { unsafe impl Backend for InternalBackend { fn swap_buffers(&self) -> Result<(), GliumSwapBuffersError> { - self.0.borrow().swap_buffers().map_err(Into::into) + if let Err(err) = self.0.borrow().swap_buffers() { + Err(match err { + SwapBuffersError::ContextLost(err) => { + self.1.set(Some(err)); + GliumSwapBuffersError::ContextLost + }, + SwapBuffersError::TemporaryFailure(err) => { + self.1.set(Some(err)); + GliumSwapBuffersError::AlreadySwapped + } + // I do not think, this may happen, but why not + SwapBuffersError::AlreadySwapped => GliumSwapBuffersError::AlreadySwapped, + }) + } else { Ok(()) } } unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -104,6 +113,94 @@ unsafe impl Backend for InternalBackend { } unsafe fn make_current(&self) { + // TODO, if this ever blows up anvil, we should probably silently ignore this. + // But I have no idea, if that may happen or what glium does, if the context is not current... + // So lets leave this in to do some real world testing self.0.borrow().make_current().expect("Context was lost") } } + +/// Omplementation of `glium::Surface`, targeting the default framebuffer. +/// +/// The back- and front-buffers are swapped when you call `finish`. +/// +/// You **must** call either `finish` or `set_finish` or else the destructor will panic. +pub struct Frame(glium::Frame, Rc>>>); + +impl Frame { + /// Stop drawing, swap the buffers, and consume the Frame. + /// + /// See the documentation of [`SwapBuffersError`] about what is being returned. + pub fn finish(mut self) -> Result<(), SwapBuffersError> { + self.set_finish() + } + + /// Stop drawing, swap the buffers. + /// + /// The Frame can now be dropped regularly. Calling `finish()` or `set_finish()` again will cause `Err(SwapBuffersError::AlreadySwapped)` to be returned. + pub fn set_finish(&mut self) -> Result<(), SwapBuffersError> { + let res = self.0.set_finish(); + let err = self.1.take(); + match (res, err) { + (Ok(()), _) => Ok(()), + (Err(GliumSwapBuffersError::AlreadySwapped), Some(err)) => Err(SwapBuffersError::TemporaryFailure(err)), + (Err(GliumSwapBuffersError::AlreadySwapped), None) => Err(SwapBuffersError::AlreadySwapped), + (Err(GliumSwapBuffersError::ContextLost), Some(err)) => Err(SwapBuffersError::ContextLost(err)), + _ => unreachable!(), + } + } +} + +impl glium::Surface for Frame { + fn clear(&mut self, rect: Option<&glium::Rect>, color: Option<(f32, f32, f32, f32)>, color_srgb: bool, + depth: Option, stencil: Option) + { + self.0.clear(rect, color, color_srgb, depth, stencil) + } + + fn get_dimensions(&self) -> (u32, u32) { + self.0.get_dimensions() + } + + fn get_depth_buffer_bits(&self) -> Option { + self.0.get_depth_buffer_bits() + } + + fn get_stencil_buffer_bits(&self) -> Option { + self.0.get_stencil_buffer_bits() + } + + fn draw<'a, 'b, V, I, U>(&mut self, v: V, i: I, program: &glium::Program, uniforms: &U, + draw_parameters: &glium::draw_parameters::DrawParameters<'_>) -> Result<(), glium::DrawError> where + V: glium::vertex::MultiVerticesSource<'b>, I: Into>, + U: glium::uniforms::Uniforms + { + self.0.draw(v, i, program, uniforms, draw_parameters) + } + + fn blit_from_frame(&self, source_rect: &glium::Rect, target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter) + { + self.0.blit_from_frame(source_rect, target_rect, filter); + } + + fn blit_from_simple_framebuffer(&self, source: &glium::framebuffer::SimpleFrameBuffer<'_>, + source_rect: &glium::Rect, target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter) + { + self.0.blit_from_simple_framebuffer(source, source_rect, target_rect, filter) + } + + fn blit_from_multioutput_framebuffer(&self, source: &glium::framebuffer::MultiOutputFrameBuffer<'_>, + source_rect: &glium::Rect, target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter) + { + self.0.blit_from_multioutput_framebuffer(source, source_rect, target_rect, filter) + } + + fn blit_color(&self, source_rect: &glium::Rect, target: &S, target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter) where S: glium::Surface + { + self.0.blit_color(source_rect, target, target_rect, filter) + } +} \ No newline at end of file diff --git a/src/backend/graphics/mod.rs b/src/backend/graphics/mod.rs index 0fb18bf..8848163 100644 --- a/src/backend/graphics/mod.rs +++ b/src/backend/graphics/mod.rs @@ -16,24 +16,33 @@ pub mod glium; pub mod software; /// Error that can happen when swapping buffers. -#[derive(Debug, Clone, PartialEq, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum SwapBuffersError { - /// The corresponding context has been lost and needs to be recreated. - /// - /// All the objects associated to it (textures, buffers, programs, etc.) - /// need to be recreated from scratch. - /// - /// Operations will have no effect. Functions that read textures, buffers, etc. - /// will return uninitialized data instead. - #[error("The context has been lost, it needs to be recreated")] - ContextLost, /// The buffers have already been swapped. /// /// This error can be returned when `swap_buffers` has been called multiple times /// without any modification in between. #[error("Buffers are already swapped, swap_buffers was called too many times")] AlreadySwapped, - /// Unknown error - #[error("Unknown error: {0:x}")] - Unknown(u32), + /// The corresponding context has been lost and needs to be recreated. + /// + /// All the objects associated to it (textures, buffers, programs, etc.) + /// need to be recreated from scratch. Underlying resources like native surfaces + /// might also need to be recreated. + /// + /// Operations will have no effect. Functions that read textures, buffers, etc. + /// will return uninitialized data instead. + #[error("The context has been lost, it needs to be recreated")] + ContextLost(Box), + /// A temporary condition caused to rendering to fail. + /// + /// Depending on the underlying error this *might* require fixing internal state of the rendering backend, + /// but failures mapped to `TemporaryFailure` are always recoverable without re-creating the entire stack, + /// as is represented by `ContextLost`. + /// + /// Proceed after investigating the source to reschedule another full rendering step or just this page_flip at a later time. + /// If the root cause cannot be discovered and subsequent renderings also fail, it is advised to fallback to + /// recreation. + #[error("A temporary condition caused the page flip to fail.")] + TemporaryFailure(Box), } diff --git a/src/backend/winit.rs b/src/backend/winit.rs index c1960bb..acf7766 100644 --- a/src/backend/winit.rs +++ b/src/backend/winit.rs @@ -3,7 +3,7 @@ use crate::backend::egl::display::EGLDisplay; use crate::backend::egl::get_proc_address; use crate::backend::{ - egl::{context::GlAttributes, native, EGLContext, EGLSurface, Error as EGLError}, + egl::{context::GlAttributes, native, EGLContext, EGLSurface, Error as EGLError, SurfaceCreationError}, graphics::{gl::GLGraphicsBackend, CursorBackend, PixelFormat, SwapBuffersError}, input::{ Axis, AxisSource, Event as BackendEvent, InputBackend, InputHandler, KeyState, KeyboardKeyEvent, @@ -16,6 +16,7 @@ use nix::libc::c_void; use std::{ cell::{Ref, RefCell}, cmp, + convert::TryInto, rc::Rc, time::Instant, }; @@ -47,6 +48,9 @@ pub enum Error { /// EGL error #[error("EGL error: {0}")] EGL(#[from] EGLError), + /// Surface Creation failed + #[error("Surface creation failed: {0}")] + SurfaceCreationError(#[from] SurfaceCreationError) } enum Window { @@ -277,8 +281,8 @@ impl GLGraphicsBackend for WinitGraphicsBackend { fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { trace!(self.logger, "Swapping buffers"); match *self.window { - Window::Wayland { ref surface, .. } => surface.swap_buffers(), - Window::X11 { ref surface, .. } => surface.swap_buffers(), + Window::Wayland { ref surface, .. } => surface.swap_buffers().map_err(|err| err.try_into().unwrap()), + Window::X11 { ref surface, .. } => surface.swap_buffers().map_err(|err| err.try_into().unwrap()), } } @@ -314,12 +318,12 @@ impl GLGraphicsBackend for WinitGraphicsBackend { ref surface, ref context, .. - } => context.make_current_with_surface(surface), + } => context.make_current_with_surface(surface).map_err(Into::into), Window::X11 { ref surface, ref context, .. - } => context.make_current_with_surface(surface), + } => context.make_current_with_surface(surface).map_err(Into::into), } }