LinuxKMS: Clean up internal page flip callback API

Remove the hack of calling present() from our swap_buffers() forwarder (and let it really only forward), and instead call it explicitly through the shared egl_display member. The way we can pass along the callback instead of storing it separately.
slint-ui · Jan 4, 2024 · 8193d4d · 8193d4d
1 parent f5bf6e5
commit 8193d4d
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 36 deletions.
diff --git a/internal/backends/linuxkms/display/egldisplay.rs b/internal/backends/linuxkms/display/egldisplay.rs
@@ -42,6 +42,7 @@ enum PageFlipState {
     InitialBufferPosted,
     WaitingForPageFlip {
         _buffer_to_keep_alive_until_flip: gbm::BufferObject<OwnedFramebufferHandle>,
+        ready_for_next_animation_frame: Box<dyn FnOnce()>,
     },
     ReadyForNextBuffer,
 }
@@ -57,18 +58,13 @@ pub struct EglDisplay {
     drm_device: SharedFd,
     pub size: PhysicalWindowSize,
     page_flip_event_source_registered: Cell<bool>,
-    next_animation_frame_callback: Cell<Option<Box<dyn FnOnce()>>>,
 }
 
 impl EglDisplay {
-    pub fn set_next_animation_frame_callback(
+    pub fn present(
         &self,
         ready_for_next_animation_frame: Box<dyn FnOnce()>,
-    ) {
-        self.next_animation_frame_callback.set(Some(ready_for_next_animation_frame));
-    }
-
-    pub fn present(&self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+    ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
         let mut front_buffer = unsafe {
             self.gbm_surface
                 .lock_front_buffer()
@@ -91,25 +87,22 @@ impl EglDisplay {
                 .page_flip(self.crtc, fb, drm::control::PageFlipFlags::EVENT, None)
                 .map_err(|e| format!("Error presenting fb: {e}"))?;
 
-            *self.page_flip_state.borrow_mut() =
-                PageFlipState::WaitingForPageFlip { _buffer_to_keep_alive_until_flip: last_buffer };
+            *self.page_flip_state.borrow_mut() = PageFlipState::WaitingForPageFlip {
+                _buffer_to_keep_alive_until_flip: last_buffer,
+                ready_for_next_animation_frame,
+            };
         } else {
             self.gbm_device
                 .set_crtc(self.crtc, Some(fb), (0, 0), &[self.connector.handle()], Some(self.mode))
                 .map_err(|e| format!("Error presenting fb: {e}"))?;
             *self.page_flip_state.borrow_mut() = PageFlipState::InitialBufferPosted;
 
-            if let Some(next_animation_frame_callback) = self.next_animation_frame_callback.take() {
-                // We can render the next frame right away, if needed, since we have at least two buffers. The callback
-                // will decide (will check if animation is running). However invoke the callback through the event loop
-                // instead of directly, so that if it decides to set `needs_redraw` to true, the event loop will process it.
-                i_slint_core::timers::Timer::single_shot(
-                    std::time::Duration::default(),
-                    move || {
-                        next_animation_frame_callback();
-                    },
-                )
-            }
+            // We can render the next frame right away, if needed, since we have at least two buffers. The callback
+            // will decide (will check if animation is running). However invoke the callback through the event loop
+            // instead of directly, so that if it decides to set `needs_redraw` to true, the event loop will process it.
+            i_slint_core::timers::Timer::single_shot(std::time::Duration::default(), move || {
+                ready_for_next_animation_frame();
+            })
         }
 
         Ok(())
@@ -143,12 +136,12 @@ impl super::Presenter for EglDisplay {
                     .receive_events()?
                     .any(|event| matches!(event, drm::control::Event::PageFlip(..)))
                 {
-                    *this.page_flip_state.borrow_mut() = PageFlipState::ReadyForNextBuffer;
-
-                    if let Some(next_animation_frame_callback) =
-                        this.next_animation_frame_callback.take()
+                    if let PageFlipState::WaitingForPageFlip {
+                        ready_for_next_animation_frame,
+                        ..
+                    } = this.page_flip_state.replace(PageFlipState::ReadyForNextBuffer)
                     {
-                        next_animation_frame_callback();
+                        ready_for_next_animation_frame();
                     }
                 }
                 Ok(calloop::PostAction::Continue)
@@ -163,8 +156,7 @@ impl super::Presenter for EglDisplay {
         &self,
         ready_for_next_animation_frame: Box<dyn FnOnce()>,
     ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        self.set_next_animation_frame_callback(ready_for_next_animation_frame);
-        self.present()
+        self.present(ready_for_next_animation_frame)
     }
 
     fn is_ready_to_present(&self) -> bool {
@@ -352,6 +344,5 @@ pub fn try_create_egl_display(
         drm_device,
         size: window_size,
         page_flip_event_source_registered: Cell::new(false),
-        next_animation_frame_callback: Default::default(),
     })
 }
diff --git a/internal/backends/linuxkms/renderer/femtovg.rs b/internal/backends/linuxkms/renderer/femtovg.rs
@@ -27,11 +27,10 @@ pub struct FemtoVGRendererAdapter {
 struct GlContextWrapper {
     glutin_context: glutin::context::PossiblyCurrentContext,
     glutin_surface: glutin::surface::Surface<glutin::surface::WindowSurface>,
-    egl_display: Rc<EglDisplay>,
 }
 
 impl GlContextWrapper {
-    fn new(egl_display: Rc<EglDisplay>) -> Result<Self, PlatformError> {
+    fn new(egl_display: &EglDisplay) -> Result<Self, PlatformError> {
         let width: std::num::NonZeroU32 = egl_display.size.width.try_into().map_err(|_| {
             format!(
                 "Attempting to create window surface with an invalid width: {}",
@@ -114,7 +113,7 @@ impl GlContextWrapper {
         drop(window_handle);
         drop(display_handle);
 
-        Ok(Self { glutin_context: context, glutin_surface: surface, egl_display })
+        Ok(Self { glutin_context: context, glutin_surface: surface })
     }
 }
 
@@ -136,8 +135,7 @@ unsafe impl i_slint_renderer_femtovg::OpenGLInterface for GlContextWrapper {
                 format!("FemtoVG: Error swapping buffers: {glutin_error}").into()
             },
         )?;
-
-        self.egl_display.present()
+        Ok(())
     }
 
     fn resize(
@@ -162,7 +160,7 @@ impl FemtoVGRendererAdapter {
 
         let renderer = Box::new(Self {
             renderer: i_slint_renderer_femtovg::FemtoVGRenderer::new(GlContextWrapper::new(
-                egl_display.clone(),
+                &egl_display,
             )?)?,
             egl_display,
         });
@@ -188,15 +186,16 @@ impl crate::fullscreenwindowadapter::FullscreenRenderer for FemtoVGRendererAdapt
         draw_mouse_cursor_callback: &dyn Fn(&mut dyn ItemRenderer),
         ready_for_next_animation_frame: Box<dyn FnOnce()>,
     ) -> Result<(), PlatformError> {
-        self.egl_display.set_next_animation_frame_callback(ready_for_next_animation_frame);
         self.renderer.render_transformed_with_post_callback(
             rotation.degrees(),
             rotation.translation_after_rotation(self.egl_display.size),
             self.egl_display.size,
             Some(&|item_renderer| {
                 draw_mouse_cursor_callback(item_renderer);
             }),
-        )
+        )?;
+        self.egl_display.present(ready_for_next_animation_frame)?;
+        Ok(())
     }
     fn size(&self) -> i_slint_core::api::PhysicalSize {
         self.egl_display.size