diff --git a/examples/colorful_triangle.rs b/examples/colorful_triangle.rs index abebb56..cb451ea 100644 --- a/examples/colorful_triangle.rs +++ b/examples/colorful_triangle.rs @@ -17,22 +17,22 @@ struct Args { fn main() { let args = Args::parse(); - let drm_display = - just_gl::drm::DrmDisplay::new(&args.card_path, args.connector.as_ref()).unwrap(); + let drm_display = just_gl::drm::DrmDisplay::new(&args.card_path, args.connector.as_ref()) + .expect("Could not create a `DrmDisplay`"); let mut window = just_gl::window::Window::new(drm_display); let glium_display = just_gl::gl::init(&window); let mut triangle = Triangle::new(&glium_display); let count = 60; for i in 0..count { - window.draw(|| { - use glium::Surface; - let ratio = i as f32 / count as f32; - let mut frame = glium_display.draw(); - frame.clear_color(0.2 * ratio, 0.0, 0.5, 1.0); - triangle.draw(&mut frame); - frame.finish().unwrap(); - }); + use glium::Surface; + let ratio = i as f32 / count as f32; + let mut frame = glium_display.draw(); + frame.clear_color(0.2 * ratio, 0.0, 0.5, 1.0); + triangle.draw(&mut frame); + frame.finish().expect("Could not finish the frame"); + // SAFETY: frame.finish() above takes care of swapping buffers correctly. + unsafe { window.present() } } window.restore_original_display(); diff --git a/src/drm.rs b/src/drm.rs index af2d852..2ca02ab 100644 --- a/src/drm.rs +++ b/src/drm.rs @@ -157,9 +157,15 @@ impl DrmDisplay { Some(DrmDisplay { gbm_device, crtc, connector, mode, width, height }) } - pub(crate) fn set_mode_with_framebuffer(&self, fb: Option) { + pub(crate) fn set_mode_with_framebuffer(&self, fb: FramebufferHandle) { self.gbm_device - .set_crtc(self.crtc.handle(), fb, (0, 0), &[self.connector.handle()], Some(self.mode)) + .set_crtc( + self.crtc.handle(), + Some(fb), + (0, 0), + &[self.connector.handle()], + Some(self.mode), + ) .expect("set_crtc failed"); } diff --git a/src/window.rs b/src/window.rs index f0345b9..b5b0411 100644 --- a/src/window.rs +++ b/src/window.rs @@ -2,11 +2,21 @@ use crate::drm::DrmDisplay; use drm::control::{framebuffer::Handle as FramebufferHandle, Device as ControlDevice}; use gbm::{BufferObject, BufferObjectFlags, Format as BufferFormat, Surface}; +enum DisplayState { + Init, + ModeSet { _buffer_object: BufferObject }, + PageFlipped { _buffer_object: BufferObject }, +} + pub struct Window { pub(crate) gbm_surface: Surface, pub(crate) drm_display: DrmDisplay, - pub(crate) frame_count: usize, - previous_bo: Option>, + // NOTE(mbernat): Buffer objects obtained from a surface + // return to the surface's queue when dropped. + // + // Unfortunately, this behavior is not very well documented, + // see `Surface::lock_front_buffer()` implementation for details. + display_state: DisplayState, } impl Window { @@ -20,28 +30,34 @@ impl Window { let gbm_surface: Surface = drm_display .gbm_device .create_surface_with_modifiers(drm_display.width, drm_display.height, format, modifiers) - .unwrap(); + .expect("Could not create a GBM surface"); */ let usage = BufferObjectFlags::SCANOUT | BufferObjectFlags::RENDERING; let gbm_surface: Surface = drm_display .gbm_device .create_surface(drm_display.width, drm_display.height, format, usage) - .unwrap(); + .expect("Could not create a GBM surface"); - Window { gbm_surface, drm_display, frame_count: 0, previous_bo: None } + Window { gbm_surface, drm_display, display_state: DisplayState::Init } //, frame_count: 0, previous_buffer_object: None } } /// # Safety - /// this must be called exactly once after `eglSwapBuffers`, - /// which happens e.g. in `Frame::finish()`. - unsafe fn present(&mut self) { + /// The buffer we are trying to present must be valid. + /// Defining validity precisely needs more work but it likely involves + /// writing to the buffer so that it does not contain uninitialized memory. + /// + /// One way to achieve this is with `glium`'s `Frame::finish()`, + /// which calls `eglSwapBuffers()` internally and that in turns calls + /// `glFlush()` to write to the buffer. + pub unsafe fn present(&mut self) { // TODO(mbernat): move this elsewhere let depth_bits = 24; let bits_per_pixel = 32; // SAFETY: we offloaded the `lock_front_buffer()` precondition to our caller - let mut buffer_object = unsafe { self.gbm_surface.lock_front_buffer().unwrap() }; + let mut buffer_object = unsafe { self.gbm_surface.lock_front_buffer() } + .expect("Could not obtain a buffer object from the GBM surface"); // NOTE(mbernat): Frame buffer recycling: // we store an FB handle in buffer object's user_data() and reuse the FB when it exists @@ -53,44 +69,33 @@ impl Window { .drm_display .gbm_device .add_framebuffer(&buffer_object, depth_bits, bits_per_pixel) - .unwrap(); + .expect("Could not add a frame buffer"); buffer_object.set_userdata(fb).expect("Could not set buffer object user data"); fb }; - if self.frame_count == 0 { - // NOTE(mbernat): It's possible to avoid initial mode setting - // since we're keeping the previous mode: we could just call page_flip directly. - self.drm_display.set_mode_with_framebuffer(Some(fb)); - } else { - self.drm_display.page_flip(fb); - } - - // NOTE(mbernat): This buffer object returns to the surface's queue upon destruction. - // If we were to release it here, it would be again available from - // `Surface::lock_front_buffer()` next and the app would be effectively single-buffered. - // So, we keep it around for one frame, which is fine for double buffering. - self.previous_bo.replace(buffer_object); - - self.frame_count += 1; - } - - pub fn restore_original_display(&self) { - let handle = self.drm_display.crtc.framebuffer().unwrap(); - if self.drm_display.gbm_device.get_framebuffer(handle).is_ok() { - self.drm_display.set_mode_with_framebuffer(self.drm_display.crtc.framebuffer()); - } - } - - pub fn draw(&mut self, mut drawer: impl FnMut()) { - drawer(); - // SAFETY: eglSwapBuffers is called by `frame.finish()` in drawer() - unsafe { self.present() }; - - // The first page flip is scheduled after frame #1 (which is the second frame) - // Yes, this is very stupid, just testing if it works + self.display_state = match &self.display_state { + DisplayState::Init => { + // NOTE(mbernat): Displays often use their preferred modes. + // If that's already the case we can avoid initial mode setting and go + // straight to page flipping as a small optimization. + self.drm_display.set_mode_with_framebuffer(fb); + DisplayState::ModeSet { _buffer_object: buffer_object } + }, + DisplayState::ModeSet { .. } | DisplayState::PageFlipped { .. } => { + self.drm_display.page_flip(fb); + // The buffer object we store here will hang around for a frame + // and will be dropped by this match arm in the next frame; + // this is sufficient for double buffering. + DisplayState::PageFlipped { _buffer_object: buffer_object } + }, + }; - if self.frame_count > 1 { + // Page flips are scheduled asynchronously, so we need to await their completion. + if matches!(self.display_state, DisplayState::PageFlipped { .. }) { + // This call is blocking and should not be called when no events are expected. + // Its implementation is just a read from the DRM file descriptor, which should + // be replaced by e.g. an `epoll` over multiple sources in a proper event loop. let _events = self.drm_display.gbm_device.receive_events().expect("Could not receive events"); @@ -99,6 +104,17 @@ impl Window { // but with the current setup there is nothing we need to do. } } + + pub fn restore_original_display(&self) { + let handle = self + .drm_display + .crtc + .framebuffer() + .expect("Window should have a CRTC framebuffer handle"); + if let Ok(fb) = self.drm_display.gbm_device.get_framebuffer(handle) { + self.drm_display.set_mode_with_framebuffer(fb.handle()); + } + } } mod rwh_impl {