diff --git a/anvil/src/state.rs b/anvil/src/state.rs index 0ee6433..671601b 100644 --- a/anvil/src/state.rs +++ b/anvil/src/state.rs @@ -138,7 +138,7 @@ impl AnvilState { let cursor_status = Arc::new(Mutex::new(CursorImageStatus::Default)); let cursor_status2 = cursor_status.clone(); - let pointer = seat.add_pointer(shell_handles.token.clone(), move |new_status| { + let pointer = seat.add_pointer(shell_handles.token, move |new_status| { // TODO: hide winit system cursor when relevant *cursor_status2.lock().unwrap() = new_status }); diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index a67ecd9..b40bd83 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -2,11 +2,21 @@ //! [`RawDevice`](RawDevice) and [`RawSurface`](RawSurface) //! implementations using the atomic mode-setting infrastructure. //! +//! Atomic mode-setting (previously referred to a nuclear page-flip) is a new api of the Direct Rendering +//! Manager subsystem of the linux kernel. Adaptations of this api can be found in BSD kernels. +//! +//! This api is objectively better than the outdated legacy-api, but not supported by every driver. +//! Initialization will fail, if the api is unsupported. The legacy-api is also wrapped by smithay +//! and may be used instead in these cases. Currently there are no features in smithay that are +//! exclusive to the atomic api. +//! //! Usually this implementation will wrapped into a [`GbmDevice`](::backend::drm::gbm::GbmDevice). //! Take a look at `anvil`s source code for an example of this. //! //! For an example how to use this standalone, take a look at the raw_atomic_drm example. //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use std::cell::RefCell; use std::collections::HashMap; @@ -86,9 +96,11 @@ impl Drop for Dev { // Here we restore the card/tty's to it's previous state. // In case e.g. getty was running on the tty sets the correct framebuffer again, // so that getty will be visible. + // We do exit correctly if this fails, but the user will be presented with + // a black screen if no display handler takes control again. + // create an atomic mode request consisting of all properties we captured on creation. let mut req = AtomicModeReq::new(); - fn add_multiple_props( req: &mut AtomicModeReq, old_state: &[(T, PropertyValueSet)], @@ -213,6 +225,8 @@ impl AtomicDrmDevice { let dev_id = fstat(fd.as_raw_fd()).map_err(Error::UnableToGetDeviceId)?.st_rdev; + // we wrap some of the internal states in another struct to share with + // the surfaces and event loop handlers. let active = Arc::new(AtomicBool::new(true)); let mut dev = Dev { fd, @@ -223,13 +237,22 @@ impl AtomicDrmDevice { logger: log.clone(), }; - // we want to modeset, so we better be the master, if we run via a tty session + // we need to be the master to do modesetting if we run via a tty session. + // This is only needed on older kernels. Newer kernels grant this permission, + // if no other process is already the *master*. So we skip over this error. if dev.acquire_master_lock().is_err() { warn!(log, "Unable to become drm master, assuming unprivileged mode"); dev.privileged = false; }; - // enable the features we need + // Enable the features we need. + // We technically could use the atomic api without universal plane support. + // But the two are almost exclusively implemented together, as plane synchronization + // is one of the killer-features of the atomic api. + // + // We could bould more abstractions in smithay for devices with partial support, + // but for now we role with the oldest possible api (legacy) and the newest feature set + // we can use (atomic + universal planes), although we barely use planes yet. dev.set_client_capability(ClientCapability::UniversalPlanes, true) .compat() .map_err(|source| Error::Access { @@ -245,7 +268,7 @@ impl AtomicDrmDevice { source, })?; - // enumerate (and save) the current device state + // Enumerate (and save) the current device state. let res_handles = ControlDevice::resource_handles(&dev) .compat() .map_err(|source| Error::Access { @@ -264,11 +287,16 @@ impl AtomicDrmDevice { let mut old_state = dev.old_state.clone(); let mut mapping = dev.prop_mapping.clone(); + // This helper function takes a snapshot of the current device properties. + // (everything in the atomic api is set via properties.) dev.add_props(res_handles.connectors(), &mut old_state.0)?; dev.add_props(res_handles.crtcs(), &mut old_state.1)?; dev.add_props(res_handles.framebuffers(), &mut old_state.2)?; dev.add_props(planes, &mut old_state.3)?; + // And because the mapping is not consistent across devices, + // we also need to lookup the handle for a property name. + // And we do this a fair bit, so lets cache that mapping. dev.map_props(res_handles.connectors(), &mut mapping.0)?; dev.map_props(res_handles.crtcs(), &mut mapping.1)?; dev.map_props(res_handles.framebuffers(), &mut mapping.2)?; @@ -278,6 +306,17 @@ impl AtomicDrmDevice { dev.prop_mapping = mapping; trace!(log, "Mapping: {:#?}", dev.prop_mapping); + // If the user does not explicitly requests us to skip this, + // we clear out the complete connector<->crtc mapping on device creation. + // + // The reason is, that certain operations may be racy otherwise. Surfaces can + // exist on different threads: as a result, we cannot really enumerate the current state + // (it might be changed on another thread during the enumeration). And commits can fail, + // if e.g. a connector is already bound to another surface, which is difficult to analyse at runtime. + // + // An easy workaround is to set a known state on device creation, so we can only + // run into these errors on our own and not because previous compositors left the device + // in a funny state. if disable_connectors { // Disable all connectors as initial state let mut req = AtomicModeReq::new(); diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index 9d4b39e..5b0d365 100644 --- a/src/backend/drm/atomic/session.rs +++ b/src/backend/drm/atomic/session.rs @@ -75,6 +75,8 @@ impl AtomicDrmDeviceObserver { for surface in backends.borrow().values().filter_map(Weak::upgrade) { // other ttys that use no cursor, might not clear it themselves. // This makes sure our cursor won't stay visible. + // + // This usually happens with getty, and a cursor on top of a kernel console looks very weird. if let Err(err) = surface.clear_plane(surface.planes.cursor) { warn!( self.logger, @@ -123,6 +125,13 @@ impl AtomicDrmDeviceObserver { } fn reset_state(&mut self) -> Result<(), Error> { + // reset state sets the connectors into a known state (all disabled), + // for the same reasons we do this on device creation. + // + // We might end up with conflicting commit requirements, if we want to restore our state, + // on top of the state the previous compositor left the device in. + // This is because we do commits per surface and not per device, so we do a global + // commit here, to fix any conflicts. if let Some(dev) = self.dev.upgrade() { let res_handles = ControlDevice::resource_handles(&*dev) .compat() @@ -172,6 +181,11 @@ impl AtomicDrmDeviceObserver { source, })?; + // because we change the state and disabled everything, + // we want to force a commit (instead of a page-flip) on all used surfaces + // for the next rendering step to re-activate the connectors. + // + // 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) { let mut current = surface.state.write().unwrap(); @@ -187,7 +201,7 @@ impl AtomicDrmDeviceObserver { }; surface.use_mode(mode)?; - // drop cursor state + // 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); diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 481e3d7..d1c14c6 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -111,6 +111,10 @@ impl AtomicDrmSurfaceInternal { source, })?; + // the current set of connectors are those, that already have the correct `CRTC_ID` set. + // so we collect them for `current_state` and set the user-given once in `pending_state`. + // + // If they don't match, `commit_pending` will return true and they will be changed on the next `commit`. let mut current_connectors = HashSet::new(); for conn in res_handles.connectors() { let crtc_prop = dev @@ -152,6 +156,9 @@ impl AtomicDrmSurfaceInternal { connectors: connectors.iter().copied().collect(), }; + // we need to find planes for this crtc. + // (cursor and primary planes are usually available once for every crtc, + // so this is a very naive algorithm.) let (primary, cursor) = AtomicDrmSurfaceInternal::find_planes(&dev, crtc).ok_or(Error::NoSuitablePlanes { crtc, @@ -175,6 +182,8 @@ impl AtomicDrmSurfaceInternal { Ok(surface) } + // we need a framebuffer to do test commits, which we use to verify our pending state. + // here we create a dumbbuffer for that purpose. fn create_test_buffer(&self, mode: &Mode) -> Result { let (w, h) = mode.size(); let db = self @@ -367,6 +376,7 @@ impl Surface for AtomicDrmSurfaceInternal { } fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> { + // the test would also prevent this, but the error message is far less helpful if connectors.is_empty() { return Err(Error::SurfaceWithoutConnectors(self.crtc)); } @@ -466,6 +476,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { "Preparing Commit.\n\tCurrent: {:?}\n\tPending: {:?}\n", *current, *pending ); + // we need the differences to know, which connectors need to change properties let current_conns = current.connectors.clone(); let pending_conns = pending.connectors.clone(); let mut removed = current_conns.difference(&pending_conns); @@ -493,6 +504,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { trace!(self.logger, "Testing screen config"); + // test the new config and return the request if it would be accepted by the driver. let req = { let req = self.build_request( &mut added, @@ -533,8 +545,11 @@ impl RawSurface for AtomicDrmSurfaceInternal { let result = self .atomic_commit( &[ + // on the atomic api we can modeset and trigger a page_flip event on the same call! AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::AllowModeset, + // we also do not need to wait for completion, like with `set_crtc`. + // and have tested this already, so we do not expect any errors later down the line. AtomicCommitFlags::Nonblock, ], req, @@ -558,6 +573,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { return Err(Error::DeviceInactive); } + // page flips work just like commits with fewer parameters.. let req = self.build_request( &mut [].iter(), &mut [].iter(), @@ -567,6 +583,9 @@ impl RawSurface for AtomicDrmSurfaceInternal { None, )?; + // .. and without `AtomicCommitFlags::AllowModeset`. + // If we would set anything here, that would require a modeset, this would fail, + // indicating a problem in our assumptions. trace!(self.logger, "Queueing page flip: {:?}", req); self.atomic_commit( &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock], @@ -583,6 +602,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { } } +// this whole implementation just queues the cursor state for the next commit. impl CursorBackend for AtomicDrmSurfaceInternal { type CursorFormat = dyn Buffer; type Error = Error; @@ -708,8 +728,13 @@ impl AtomicDrmSurfaceInternal { mode: Option, blob: Option>, ) -> Result { + // okay, here we build the actual requests used by the surface. let mut req = AtomicModeReq::new(); + // requests consist out of a set of properties and their new values + // for different drm objects (crtc, plane, connector, ...). + + // for every connector that is new, we need to set our crtc_id for conn in new_connectors { req.add_property( *conn, @@ -718,6 +743,10 @@ impl AtomicDrmSurfaceInternal { ); } + // for every connector that got removed, we need to set no crtc_id. + // (this is a bit problematic, because this means we need to remove, commit, add, commit + // in the right order to move a connector to another surface. otherwise we disable the + // the connector here again...) for conn in removed_connectors { req.add_property( *conn, @@ -726,16 +755,19 @@ impl AtomicDrmSurfaceInternal { ); } + // we need to set the new mode, if there is one if let Some(blob) = blob { req.add_property(self.crtc, self.crtc_prop_handle(self.crtc, "MODE_ID")?, blob); } + // we also need to set this crtc active req.add_property( self.crtc, self.crtc_prop_handle(self.crtc, "ACTIVE")?, property::Value::Boolean(true), ); + // and we need to set the framebuffer for our primary plane if let Some(fb) = framebuffer { req.add_property( planes.primary, @@ -744,12 +776,14 @@ impl AtomicDrmSurfaceInternal { ); } + // if there is a new mode, we shoudl also make sure the plane is connected if let Some(mode) = mode { req.add_property( planes.primary, self.plane_prop_handle(planes.primary, "CRTC_ID")?, property::Value::CRTC(Some(self.crtc)), ); + // we can take different parts of a plane ... req.add_property( planes.primary, self.plane_prop_handle(planes.primary, "SRC_X")?, @@ -763,6 +797,7 @@ impl AtomicDrmSurfaceInternal { req.add_property( planes.primary, self.plane_prop_handle(planes.primary, "SRC_W")?, + // these are 16.16. fixed point property::Value::UnsignedRange((mode.size().0 as u64) << 16), ); req.add_property( @@ -770,6 +805,7 @@ impl AtomicDrmSurfaceInternal { self.plane_prop_handle(planes.primary, "SRC_H")?, property::Value::UnsignedRange((mode.size().1 as u64) << 16), ); + // .. onto different coordinated on the crtc, but we just use a 1:1 mapping. req.add_property( planes.primary, self.plane_prop_handle(planes.primary, "CRTC_X")?, @@ -795,6 +831,8 @@ 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) { match self.get_framebuffer(fb).compat().map_err(|source| Error::Access { errmsg: "Error getting cursor fb", @@ -804,11 +842,13 @@ impl AtomicDrmSurfaceInternal { Ok(cursor_info) => { let hotspot = self.cursor.hotspot.get(); + // again like the primary plane we need to set crtc and framebuffer. req.add_property( planes.cursor, self.plane_prop_handle(planes.cursor, "CRTC_ID")?, property::Value::CRTC(Some(self.crtc)), ); + // copy the whole plane req.add_property( planes.cursor, self.plane_prop_handle(planes.cursor, "SRC_X")?, @@ -829,6 +869,7 @@ impl AtomicDrmSurfaceInternal { self.plane_prop_handle(planes.cursor, "SRC_H")?, property::Value::UnsignedRange((cursor_info.size().1 as u64) << 16), ); + // but this time add this at some very specific coordinates of the crtc req.add_property( planes.cursor, self.plane_prop_handle(planes.cursor, "CRTC_X")?, @@ -865,6 +906,9 @@ impl AtomicDrmSurfaceInternal { Ok(req) } + // primary and cursor planes are almost always unique to a crtc. + // otherwise we would be in trouble and would need to figure this out + // on the device level to find the best plane combination. fn find_planes(card: &Dev, crtc: crtc::Handle) -> Option<(plane::Handle, plane::Handle)> { let res = card.resource_handles().expect("Could not list resources"); let planes = card.plane_handles().expect("Could not list planes"); @@ -918,6 +962,10 @@ impl AtomicDrmSurfaceInternal { )) } + // this helper function clears the contents of a single plane. + // this is mostly used to remove the cursor, e.g. on tty switch, + // as other compositors might not make use of other planes, + // leaving our cursor as a relict of a better time on the screen. pub(super) fn clear_plane(&self, plane: plane::Handle) -> Result<(), Error> { let mut req = AtomicModeReq::new(); diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 2f17d41..8252367 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -7,6 +7,8 @@ //! to let your compositor render. //! Take a look at `anvil`s source code for an example of this. //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use drm::control::{connector, crtc, encoder, framebuffer, plane, Mode, ResourceHandles}; use drm::SystemError as DrmError; diff --git a/src/backend/drm/eglstream/egl.rs b/src/backend/drm/eglstream/egl.rs index 538fb00..1d5ee20 100644 --- a/src/backend/drm/eglstream/egl.rs +++ b/src/backend/drm/eglstream/egl.rs @@ -42,6 +42,7 @@ where type Surface = EglStreamSurface; type Error = Error<<::Surface as Surface>::Error>; + // create an EGLDisplay for the EGLstream platform unsafe fn get_display( display: ffi::NativeDisplayType, attribs: &[ffi::EGLint], @@ -104,6 +105,11 @@ where } } +// we need either a `crtc` or a `plane` for EGLStream initializations, +// which totally breaks our abstraction.. (This is normally an implementation details of `RawSurface`-implementations). +// +// as a result, we need three implemenations for atomic, legacy and fallback... + #[cfg(feature = "backend_drm_atomic")] unsafe impl NativeSurface for EglStreamSurface> { type Error = Error; diff --git a/src/backend/drm/eglstream/mod.rs b/src/backend/drm/eglstream/mod.rs index c8bc001..9c8d320 100644 --- a/src/backend/drm/eglstream/mod.rs +++ b/src/backend/drm/eglstream/mod.rs @@ -12,6 +12,8 @@ //! [`FallbackDevice`](::backend::drm::common::fallback::FallbackDevice). //! Take a look at `anvil`s source code for an example of this. //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use super::{Device, DeviceHandler, RawDevice, Surface}; @@ -144,6 +146,7 @@ impl EglStreamDevice { .map_err(|_| Error::DeviceIsNoEGLStreamDevice)?; } + // we can now query the amount of devices implementing the required extension let mut num_devices = 0; wrap_egl_call(|| ffi::egl::QueryDevicesEXT(0, ptr::null_mut(), &mut num_devices)) .map_err(Error::FailedToEnumerateDevices)?; @@ -151,6 +154,7 @@ impl EglStreamDevice { return Err(Error::DeviceIsNoEGLStreamDevice); } + // afterwards we can allocate a buffer large enough and query the actual device (this is a common pattern in egl). let mut devices = Vec::with_capacity(num_devices as usize); wrap_egl_call(|| ffi::egl::QueryDevicesEXT(num_devices, devices.as_mut_ptr(), &mut num_devices)) .map_err(Error::FailedToEnumerateDevices)?; @@ -160,8 +164,10 @@ impl EglStreamDevice { devices .into_iter() .find(|device| { + // we may get devices, that are - well - NO_DEVICE... *device != ffi::egl::NO_DEVICE_EXT && { + // the device then also needs EGL_EXT_device_drm let device_extensions = { let p = ffi::egl::QueryDeviceStringEXT(*device, ffi::egl::EXTENSIONS as i32); if p.is_null() { @@ -177,6 +183,14 @@ impl EglStreamDevice { device_extensions.iter().any(|s| *s == "EGL_EXT_device_drm") } && { + // and we want to get the file descriptor to check, that we found + // the device the user wants to initialize. + // + // notice how this is kinda the other way around. + // EGL_EXT_device_query expects use to find all devices using this extension... + // But there is no way, we are going to replace our udev-interface with this, so we list devices + // just to find the id of the one, that we actually want, because we cannot + // request it directly afaik... let path = { let p = ffi::egl::QueryDeviceStringEXT( *device, @@ -199,6 +213,7 @@ impl EglStreamDevice { .ok_or(Error::DeviceIsNoEGLStreamDevice)? }; + // okay the device is compatible and found, ready to go. Ok(EglStreamDevice { dev: device, raw, diff --git a/src/backend/drm/eglstream/surface.rs b/src/backend/drm/eglstream/surface.rs index 1423c05..2c231d0 100644 --- a/src/backend/drm/eglstream/surface.rs +++ b/src/backend/drm/eglstream/surface.rs @@ -107,7 +107,7 @@ impl Drop for EglStreamSurfaceInternal { // That way we can use hardware cursors at least on all drm-backends (including atomic), although // this is a little hacky. Overlay planes however are completely out of question for now. // -// Note that this might still appear a little chuppy, we should just use software cursors +// 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 { @@ -155,6 +155,8 @@ impl CursorBackend for EglStreamSurfaceInternal { trace!(self.logger, "Setting the new imported cursor"); + // call the drm-functions directly to bypass ::commit/::page_flip and eglstream... + if self .crtc .set_cursor2( @@ -197,6 +199,8 @@ impl EglStreamSurface { self.0.crtc.commit_pending() || self.0.stream.borrow().is_none() } + // An EGLStream is basically the pump that requests and consumes images to display them. + // The heart of this weird api. Any changes to its configuration, require a re-creation. pub(super) fn create_stream( &self, display: &Arc, @@ -230,6 +234,7 @@ impl EglStreamSurface { } } + // again enumerate extensions let extensions = { let p = unsafe { CStr::from_ptr(ffi::egl::QueryString(display.handle, ffi::egl::EXTENSIONS as i32)) }; @@ -237,6 +242,7 @@ impl EglStreamSurface { list.split(' ').map(|e| e.to_string()).collect::>() }; + // we need quite a bunch to implement a full-blown renderer. if !extensions.iter().any(|s| *s == "EGL_EXT_output_base") || !extensions.iter().any(|s| *s == "EGL_EXT_output_drm") || !extensions.iter().any(|s| *s == "EGL_KHR_stream") @@ -315,6 +321,9 @@ impl EglStreamSurface { // TEST END } + // alright, if the surface appears to be supported, we need an "output layer". + // this is basically just a fancy name for a `crtc` or a `plane`. + // those are exactly whats inside the `output_attribs` depending on underlying device. let mut num_layers = 0; if unsafe { ffi::egl::GetOutputLayersEXT( @@ -354,11 +363,18 @@ impl EglStreamSurface { layers.set_len(num_layers as usize); } + // lets just use the first layer and try to set the swap interval. + // this is needed to make sure `eglSwapBuffers` does not block. let layer = layers[0]; unsafe { ffi::egl::OutputLayerAttribEXT(display.handle, layer, ffi::egl::SWAP_INTERVAL_EXT as i32, 0); } + // The stream needs to know, it needs to request frames + // as soon as another one is rendered (we do not want to build a buffer and delay frames), + // which is handled by STREAM_FIFO_LENGTH_KHR = 0. + // We also want to "acquire" the frames manually. Like this we can request page-flip events + // to drive our event loop. Otherwise we would have no way to know rendering is finished. let stream_attributes = { let mut out: Vec = Vec::with_capacity(7); out.push(ffi::egl::STREAM_FIFO_LENGTH_KHR as i32); @@ -371,12 +387,14 @@ impl EglStreamSurface { out }; + // 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 { 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 { error!(self.0.logger, "Failed to link Output Layer as Stream Consumer"); return Err(Error::DeviceStreamCreationFailed); @@ -394,6 +412,7 @@ impl EglStreamSurface { _surface_attribs: &[c_int], output_attribs: &[isize], ) -> Result<*const c_void, Error<<::Surface as Surface>::Error>> { + // our surface needs a stream let stream = self.create_stream(display, output_attribs)?; let (w, h) = self.current_mode().size(); @@ -408,6 +427,8 @@ impl EglStreamSurface { out }; + // the stream is already connected to the consumer (output layer) during creation. + // we now connect the producer (out egl surface, that we render to). let surface = unsafe { ffi::egl::CreateStreamProducerSurfaceKHR( display.handle, @@ -430,7 +451,15 @@ impl EglStreamSurface { display: &Arc, surface: ffi::egl::types::EGLSurface, ) -> Result<(), SwapBuffersError::Surface as Surface>::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...). + // if it fails we do not want to swap, because that would block and as a result deadlock us. if self.0.locked.load(Ordering::SeqCst) { + // which means in eglstream terms: "acquire it". + // here we set the user data of the page_flip event + // (which is also only triggered if we manually acquire frames). + // This is the crtc id like always to get the matching surface for the device handler later. let acquire_attributes = [ ffi::egl::DRM_FLIP_EVENT_DATA_NV as isize, Into::::into(crtc) as isize, @@ -438,6 +467,9 @@ impl EglStreamSurface { ]; if let Ok(stream) = self.0.stream.try_borrow() { + // 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`. let res = if let Some(&(ref display, ref stream)) = stream.as_ref() { wrap_egl_call(|| unsafe { ffi::egl::StreamConsumerAcquireAttribNV( @@ -450,6 +482,8 @@ impl EglStreamSurface { } else { Err(Error::StreamFlipFailed(EGLError::NotInitialized)) }; + + // so we need to unlock on success and return on failure. if res.is_ok() { self.0.locked.store(false, Ordering::SeqCst); } else { @@ -458,6 +492,7 @@ impl EglStreamSurface { } } + // so if we are not locked any more we can send the next frame by calling swap buffers. if !self.0.locked.load(Ordering::SeqCst) { wrap_egl_call(|| unsafe { ffi::egl::SwapBuffers(***display, surface as *const _) }) .map_err(SwapBuffersError::EGLSwapBuffers)?; diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 6e8d830..83e64c3 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -30,6 +30,7 @@ impl Backend for Gbm { type Surface = GbmSurface; type Error = Error<<::Surface as Surface>::Error>; + // this creates an EGLDisplay for the gbm platform. unsafe fn get_display( display: ffi::NativeDisplayType, attribs: &[ffi::EGLint], diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index 89e3305..28fc5bf 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -8,6 +8,8 @@ //! To use these types standalone, you will need to consider the special requirements //! of [`GbmSurface::page_flip`](::backend::drm::gbm::GbmSurface::page_flip). //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface}; use crate::backend::graphics::SwapBuffersError; @@ -132,6 +134,8 @@ impl DeviceHandler for InternalDeviceHan if let Some(backends) = self.backends.upgrade() { if let Some(surface) = backends.borrow().get(&crtc) { if let Some(surface) = surface.upgrade() { + // here we unlock the buffer again, that was locked during rendering, + // to make sure it is always unlocked after a successful page_flip. surface.unlock_buffer(); self.handler.vblank(crtc); } diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 2076905..7d4b1d2 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -56,7 +56,7 @@ impl GbmSurfaceInternal { .map_err(|_| Error::FrontBufferLockFailed)?; // create a framebuffer if the front buffer does not have one already - // (they are reused by gbm) + // (they are reused internally by gbm) let maybe_fb = next_bo .userdata() .map_err(|_| Error::InvalidInternalState)? @@ -80,6 +80,7 @@ impl GbmSurfaceInternal { } } + // 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) @@ -88,6 +89,7 @@ impl GbmSurfaceInternal { RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying) }; + // if it was successful, we can clear the re-created state match result { Ok(_) => { self.recreated.set(false); @@ -95,12 +97,15 @@ impl GbmSurfaceInternal { Ok(()) } Err(err) => { + // if there was an error we need to free the buffer again, + // otherwise we may never lock again. self.unlock_buffer(); Err(err) } } } + // 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>> { let (w, h) = self.pending_mode().size(); @@ -129,6 +134,8 @@ impl GbmSurfaceInternal { Ok(()) } + // 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()) { if let Err(err) = self.crtc.destroy_framebuffer(fb) { diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 1897222..0b9c0c3 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -2,11 +2,20 @@ //! [`RawDevice`](RawDevice) and [`RawSurface`](RawSurface) //! implementations using the legacy mode-setting infrastructure. //! +//! Legacy mode-setting refers to the now outdated, but still supported direct manager api +//! of the linux kernel. Adaptations of this api can be found in BSD kernels. +//! +//! The newer and objectively better api is known as atomic-modesetting (or nuclear-page-flip), +//! however this api is not supported by every driver, so this is provided for backwards compatibility. +//! Currenly there are no features in smithay, that are exclusive to the atomic api. +//! //! Usually this implementation will be wrapped into a [`GbmDevice`](::backend::drm::gbm::GbmDevice). //! Take a look at `anvil`s source code for an example of this. //! //! For an example how to use this standalone, take a look at the `raw_legacy_drm` example. //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use super::{common::Error, DevPath, Device, DeviceHandler, RawDevice}; @@ -66,6 +75,8 @@ impl Drop for Dev { // Here we restore the tty to it's previous state. // In case e.g. getty was running on the tty sets the correct framebuffer again, // so that getty will be visible. + // We do exit correctly, if this fails, but the user will be presented with + // a black screen, if no display handler takes control again. let old_state = self.old_state.clone(); for (handle, (info, connectors)) in old_state { if let Err(err) = self.set_crtc( @@ -88,7 +99,7 @@ impl Drop for Dev { } impl LegacyDrmDevice { - /// Create a new [`LegacyDrmDevice`] from an open drm node + /// Create a new [`LegacyDrmDevice`] from an open drm node. /// /// # Arguments /// @@ -115,6 +126,8 @@ impl LegacyDrmDevice { .map_err(Error::UnableToGetDeviceId)? .st_rdev; + // we wrap some of the internal state in another struct to share with + // the surfaces and event loop handlers. let active = Arc::new(AtomicBool::new(true)); let mut dev = Dev { fd: dev, @@ -124,13 +137,17 @@ impl LegacyDrmDevice { logger: log.clone(), }; - // we want to modeset, so we better be the master, if we run via a tty session + // We want to modeset, so we better be the master, if we run via a tty session. + // This is only needed on older kernels. Newer kernels grant this permission, + // if no other process is already the *master*. So we skip over this error. if dev.acquire_master_lock().is_err() { warn!(log, "Unable to become drm master, assuming unprivileged mode"); dev.privileged = false; }; - // enumerate (and save) the current device state + // Enumerate (and save) the current device state. + // We need to keep the previous device configuration to restore the state later, + // so we query everything, that we can set. let res_handles = ControlDevice::resource_handles(&dev) .compat() .map_err(|source| Error::Access { @@ -165,11 +182,18 @@ impl LegacyDrmDevice { } } + // If the user does not explicitly requests us to skip this, + // we clear out the complete connector<->crtc mapping on device creation. + // + // The reason is, that certain operations may be racy otherwise, as surfaces can + // exist on different threads. As a result, we cannot enumerate the current state + // on surface creation (it might be changed on another thread during the enumeration). + // An easy workaround is to set a known state on device creation. if disable_connectors { dev.set_connector_state(res_handles.connectors().iter().copied(), false)?; for crtc in res_handles.crtcs() { - // null commit + // null commit (necessary to trigger removal on the kernel side with the legacy api.) dev.set_crtc(*crtc, None, (0, 0), &[], None) .compat() .map_err(|source| Error::Access { @@ -199,6 +223,7 @@ impl Dev { connectors: impl Iterator, enabled: bool, ) -> Result<(), Error> { + // for every connector... for conn in connectors { let info = self .get_connector(conn) @@ -208,7 +233,9 @@ impl Dev { dev: self.dev_path(), source, })?; + // that is currently connected ... if info.state() == connector::State::Connected { + // get a list of it's properties. let props = self .get_properties(conn) .compat() @@ -218,7 +245,9 @@ impl Dev { source, })?; let (handles, _) = props.as_props_and_values(); + // for every handle ... for handle in handles { + // get information of that property let info = self .get_property(*handle) .compat() @@ -227,7 +256,9 @@ impl Dev { dev: self.dev_path(), source, })?; + // to find out, if we got the handle of the "DPMS" property ... if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { + // so we can use that to turn on / off the connector self.set_property( conn, *handle, diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index 5ceda28..8c254c1 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -72,6 +72,8 @@ impl LegacyDrmDeviceObserver { for surface in backends.borrow().values().filter_map(Weak::upgrade) { // other ttys that use no cursor, might not clear it themselves. // This makes sure our cursor won't stay visible. + // + // This usually happens with getty, and a cursor on top of a kernel console looks very weird. let _ = (*device).set_cursor( surface.crtc, Option::<&drm::control::dumbbuffer::DumbBuffer>::None, @@ -119,6 +121,11 @@ impl LegacyDrmDeviceObserver { } impl LegacyDrmDeviceObserver { + // reset state enumerates the actual current state of the drm device + // and applies that to the `current_state` as saved in the surfaces. + // + // This re-sync is necessary after a tty swap, as the pipeline might + // be left in a different state. fn reset_state(&mut self) -> Result<(), Error> { // lets enumerate it the current state if let Some(dev) = self.dev.upgrade() { diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 035a88c..9e6917c 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -70,10 +70,13 @@ impl CursorBackend for LegacyDrmSurfaceInternal { trace!(self.logger, "Setting the new imported cursor"); + // set_cursor2 allows us to set the hotspot, but is not supported by every implementation. if self .set_cursor2(self.crtc, Some(buffer), (hotspot.0 as i32, hotspot.1 as i32)) .is_err() { + // the cursor will be slightly misplaced, when using the function for hotspots other then (0, 0), + // but that is still better then no cursor. self.set_cursor(self.crtc, Some(buffer)) .compat() .map_err(|source| Error::Access { @@ -221,7 +224,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { self.dev.set_connector_state(removed.copied(), false)?; if conn_removed { - // We need to do a null commit to free graphics pipelines + // null commit (necessary to trigger removal on the kernel side with the legacy api.) self.set_crtc(self.crtc, None, (0, 0), &[], None) .compat() .map_err(|source| Error::Access { @@ -246,6 +249,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { } debug!(self.logger, "Setting screen"); + // do a modeset and attach the given framebuffer self.set_crtc( self.crtc, Some(framebuffer), @@ -266,6 +270,11 @@ impl RawSurface for LegacyDrmSurfaceInternal { *current = pending.clone(); + // set crtc does not trigger page_flip events, so we immediately queue a flip + // with the same framebuffer. + // this will result in wasting a frame, because this flip will need to wait + // for `set_crtc`, but is necessary to drive the event loop and thus provide + // a more consistent api. ControlDevice::page_flip( self, self.crtc, @@ -316,7 +325,8 @@ impl LegacyDrmSurfaceInternal { "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors ); - // Try to enumarate the current state to set the initial state variable correctly + // Try to enumarate the current state to set the initial state variable correctly. + // We need an accurate state to handle `commit_pending`. let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access { errmsg: "Error loading crtc info", dev: dev.dev_path(), @@ -377,6 +387,13 @@ impl LegacyDrmSurfaceInternal { Ok(surface) } + // we use this function to verify, if a certain connector/mode combination + // is valid on our crtc. We do this with the most basic information we have: + // - is there a matching encoder + // - does the connector support the provided Mode. + // + // Better would be some kind of test commit to ask the driver, + // but that only exists for the atomic api. fn check_connector(&self, conn: connector::Handle, mode: &Mode) -> Result { let info = self .get_connector(conn) diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index e370554..f5835cc 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -1,38 +1,153 @@ //! -//! //! This module provides Traits reprensentating open devices //! and their surfaces to render contents. //! -//! --- +//! # Abstractions //! -//! Initialization of devices happens through an open file descriptor -//! of a drm device. +//! This is a model of what the user typically perceives as a `Device` and a `Surface`. +//! The meaning of these words is very overloaded in the general computer graphics context, +//! so let me give you a quick run down, what these mean in the context of smithay's public api: //! -//! --- +//! ## Device //! -//! Initialization of surfaces happens through the types provided by -//! [`drm-rs`](drm). +//! A device is some sort of rendering device. It exposes certain properties, which are directly derived +//! from the *device* as perceived by the direct rendering manager api (drm). These resources consists +//! out of connectors, encoders, framebuffers, planes and crtcs. //! -//! Four entities are relevant for the initialization procedure. -//! -//! [`crtc`](drm::control::crtc)s represent scanout engines -//! of the device pointer to one framebuffer. +//! [`crtc`](drm::control::crtc)s represent scanout engines of the device pointer to one framebuffer. //! Their responsibility is to read the data of the framebuffer and export it into an "Encoder". -//! The number of crtc's represent the number of independant output devices the hardware may handle. +//! The number of crtc's represent the number of independent output devices the hardware may handle. //! -//! An [`encoder`](drm::control::encoder) encodes the data of -//! connected crtcs into a video signal for a fixed set of connectors. +//! On modern graphic cards it is better to think about the `crtc` as some sort of rendering engine. +//! You can only have so many different pictures, you may display, as you have `crtc`s, but a single image +//! may be put onto multiple displays. +//! +//! An [`encoder`](drm::control::encoder) encodes the data of connected crtcs into a video signal for a fixed set of connectors. //! E.g. you might have an analog encoder based on a DAG for VGA ports, but another one for digital ones. //! Also not every encoder might be connected to every crtc. //! -//! A [`connector`](drm::control::connector) represents a port -//! on your computer, possibly with a connected monitor, TV, capture card, etc. +//! A [`connector`](drm::control::connector) represents a port on your computer, possibly with a connected monitor, TV, capture card, etc. +//! +//! A [`framebuffer`](drm::control::framebuffer) represents a buffer you may be rendering to, see `Surface` below. +//! +//! Planes are another layer on top of the crtcs, which allow us to layer multiple images on top of each other more efficiently +//! then by combining the rendered images in the rendering phase, e.g. via OpenGL. They are internally used by smithay to provide +//! hardware-accelerated cursors. More features regarding planes may be added in the future, but they are mostly optional and +//! mainly provide possibilies for optimizations. +//! +//! The main functionality of a `Device` in smithay is to give access to all these properties for the user to +//! choose an appropriate rendering configuration. What that means is defined by the requirements and constraints documented +//! in the specific device implementations. The second functionality is the creation of a `Surface`. +//! Surface creation requires a `crtc` (which cannot be the same as another existing `Surface`'s crtc), +//! as well as a `Mode` and a set of `connectors`. +//! +//! smithay does not make sure that `connectors` are not already in use by another `Surface`. Overlapping `connector`-Sets may +//! be an error or result in undefined rendering behavior depending on the `Surface` implementation. +//! +//! ## Surface +//! +//! A surface is a part of a `Device` that may output a picture to a number of connectors. It pumps pictures of buffers to outputs. //! //! On surface creation a matching encoder for your `encoder`-`connector` is automatically selected, //! if it exists, which means you still need to check your configuration. //! -//! At last a [`Mode`](drm::control::Mode) needs to be selected, -//! supported by the `crtc` in question. +//! A surface consists of one `crtc` that is rendered to by the user. This is fixed for the `Surface`s lifetime and cannot be changed. +//! A surface also always needs at least one connector to output the resulting image to as well as a `Mode` that is valid for the given connector. +//! +//! The state of a `Surface` is double-buffered, meaning all operations that chance the set of `connector`s or their `Mode` are stored and +//! only applied on the next commit. `Surface`s do their best to validate these changes, if possible. +//! +//! A commit/page_flip may be triggered by any other method the `Surface` has, but needs to be documented as such. +//! The most low-level `Surface`s also implement `RawSurface`, which let you trigger a commit explicitly. +//! +//! ## RawDevice +//! +//! A low-level device that produces not only `Surface`s, but `RawSurface`s. +//! +//! ## RawSurface +//! +//! RawSurfaces provide two additional functions related to rendering: `commit` and `page_flip`. +//! Both attach the contents of a framebuffer, resulting in it's contents being displayed on the set connectors. +//! +//! `commit` also applies any pending changes to the current `Mode` and `connector`-Sets. +//! You can easily check, if there are pending changes and avoid the costly commit by using `commit_pending`, +//! which compares the current and pending state internally. +//! +//! # Rendering +//! +//! To perform actual rendering operations you need to render your image to a framebuffer. +//! Several types of framebuffers exists. +//! +//! The simplest one - the `Dumbbuffer` is a simple bitmap, usually very slow and not hardware-accelerated. +//! It should be avoided at any cost, but works on all devices and can be used directly with a `RawSurface`. +//! Rendering to it can be done manually by coping bitmaps around (cpu rendering). +//! +//! Accelerated buffers are usually hardware manufacturer dependent. A problem, which is mostly solved by +//! the General Buffer Manager (gbm-)API. `gbm::Buffer`s can only be rendered to by egl. Appropriate hardware-accelerated +//! buffers are picked automatically. gbm even may fall back to `Dumbbuffers` on unsupported devices and +//! egl-support is then provided by Mesa's software renderer `llvmpipe`. +//! +//! An alternative api to `gbm` is the so called `eglstream`-api, proposed (and implemented) by nvidia. +//! It is currenly only supported by nvidias proprietary driver, which is also not accelerated by gbm, although other +//! manufacturers could in theory implement the standard, just like nvidia could implement gbm-support in their driver. +//! Ongoing discussions about these api's may result in another (but hopefully unified) api in the future. +//! +//! # Implementations +//! +//! Smithay provided these different functionalities as wrappers around many `Device` and `Surface` implementations. +//! +//! At the low-level we have: +//! +//! - [`AtomicDrmDevice`](atomic::AtomicDrmDevice) (and [`AtomicDrmSurface`](atomic::AtomicDrmSurface)), which implement the traits directly using the modern atomic drm-api. +//! - [`LegacyDrmDevice`](legacy::LegacyDrmDevice) (and [`LegacyDrmSurface`](legacy::LegacyDrmSurface)), which implement the traits directly using the outdated drm-api. +//! +//! Both of these also implement `RawDevice` and `RawSurface`. +//! +//! On top of that the following wrappers add buffer allocation apis: +//! +//! - [`GbmDevice`](gbm::GbmDevice) (and [`GbmSurface`](gbm::Surface)), which replace the direct rendering capabilities of any underlying `RawSurface` with an egl-specific `page_flip` function. +//! - [`EglStreamDevice`](eglstream::EglStreamDevice) (and [`EglStreamSurface`](eglstream::EglStreamSurface)), which replace the direct rendering capabilities of any underlying `RawSurface` with functionality to create EGLStreams. +//! +//! On top of that the [`EglDevice`](egl::EglDevice) (and [`EglSurface`](egl::EglSurface)) replace the manual buffer management +//! with an implementation of smithays [`GLGraphicsBackend`](::backend::graphics::gl::GLGraphicsBackend) to initialize the OpenGL-API for rendering. +//! +//! Additionally the [`FallbackDevice`](fallback::FallbackDevice) provides an easy wrapper to automatically pick an appropriate device. +//! E.g. it can initialize an `AtomicDrmDevice` or fallback to a `LegacyDrmDevice` with the atomic api is not supported by the graphics card. +//! It can also check for a loaded nvidia-driver for a given device and choose between an `EglStreamDevice` or a `GbmDevice`. +//! +//! Any layer of this chain can be implemented and therefore extended by the user. All building blocks are exposed to add addtional +//! buffer management solutions or rendering apis out of tree. Additionally all of these implementations can be excluded from a build +//! by disabling their corresponding feature. +//! +//! The most versatile type currently provided by smithay, that should be able to initialize almost every +//! common graphics card is +//! ```rust,ignore +//! type RenderDevice = FallbackDevice< +//! EglDevice< +//! EglGbmBackend, LegacyDrmDevice>>, +//! GbmDevice, LegacyDrmDevice>>, +//! >, +//! EglDevice< +//! EglStreamDeviceBackend, LegacyDrmDevice>>, +//! EglStreamDevice, LegacyDrmDevice>>, +//! > +//! >; +//! ``` +//! +//! # Event handling +//! +//! Devices can be attached to an eventloop using `device_bind`. +//! Events triggered by a drm-device correspond to finished page-flips. +//! After submitting a page_flip (possibly through a high-level api, such as a `EglSurface`) +//! the GPU may take some time until the new framebuffer is displayed. Rendering the next +//! frame should be delayed until the flip is done. +//! +//! Naive implementations should thus be driven by the drm device events to synchronize rendering +//! with the device. More advanced implementations may take into account that, if no change has occurred, +//! a page_flip may waste resources and therefore delay rendering until the buffer change from a client +//! is received or input is registered, etc. Because a flip may fail (which will result in no event), +//! a robust rescheduling implementation for missed frames or intentionally skipped flips should be +//! in place. //! use drm::{ diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index 03a9b55..4880c04 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -90,6 +90,7 @@ impl> EGLDisplay { }; debug!(log, "EGL No-Display Extensions: {:?}", dp_extensions); + // we create an EGLDisplay let display = unsafe { B::get_display( ptr, @@ -103,6 +104,7 @@ impl> EGLDisplay { return Err(Error::DisplayNotSupported); } + // We can then query the egl api version let egl_version = { let mut major: MaybeUninit = MaybeUninit::uninit(); let mut minor: MaybeUninit = MaybeUninit::uninit(); @@ -137,6 +139,7 @@ impl> EGLDisplay { }; info!(log, "EGL Extensions: {:?}", extensions); + // egl <= 1.2 does not support OpenGL ES (maybe we want to support OpenGL in the future?) if egl_version <= (1, 2) { return Err(Error::OpenGlesNotSupported(None)); } @@ -222,7 +225,7 @@ impl> EGLDisplay { out }; - // calling `eglChooseConfig` + // Try to find configs that match out criteria let mut num_configs = 0; wrap_egl_call(|| unsafe { ffi::egl::ChooseConfig( @@ -258,7 +261,6 @@ impl> EGLDisplay { } let desired_swap_interval = if attributes.vsync { 1 } else { 0 }; - // try to select a config with the desired_swap_interval // (but don't fail, as the margin might be very small on some cards and most configs are fine) let config_id = config_ids @@ -319,6 +321,7 @@ impl> EGLDisplay { }}; }; + // return the format that was selected for our config let desc = unsafe { PixelFormat { hardware_accelerated: attrib!(self.display, config_id, ffi::egl::CONFIG_CAVEAT) diff --git a/src/backend/egl/ffi.rs b/src/backend/egl/ffi.rs index 3918d8f..3687347 100644 --- a/src/backend/egl/ffi.rs +++ b/src/backend/egl/ffi.rs @@ -49,7 +49,7 @@ pub mod egl { use std::sync::Once; lazy_static! { - pub static ref LIB: Library = { Library::new("libEGL.so.1").expect("Failed to load LibEGL") }; + pub static ref LIB: Library = Library::new("libEGL.so.1").expect("Failed to load LibEGL"); } pub static LOAD: Once = Once::new(); diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 00d1d42..ea9a198 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -122,6 +122,8 @@ impl EGLSurface { })? }); + // if a recreation is pending anyway, ignore page-flip errors. + // lets see if we still fail after the next commit. result.map_err(|err| { debug!(self.logger, "Hiding page-flip error *before* recreation: {}", err); SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface) diff --git a/src/backend/graphics/glium.rs b/src/backend/graphics/glium.rs index 9f619d5..4e04f93 100644 --- a/src/backend/graphics/glium.rs +++ b/src/backend/graphics/glium.rs @@ -125,7 +125,7 @@ unsafe impl Backend for InternalBackend { } } -/// Omplementation of `glium::Surface`, targeting the default framebuffer. +/// Implementation of `glium::Surface`, targeting the default framebuffer. /// /// The back- and front-buffers are swapped when you call `finish`. ///