From 8454bbe18340208286572a01e32f2d1a84440308 Mon Sep 17 00:00:00 2001 From: Joe Wilm Date: Mon, 27 Jun 2016 19:46:24 -0700 Subject: [PATCH] Enable vsync The input/pty processing loop previously consumed input on a channel and hit the rendering when the queue became empty. This had a couple of problems 1. It was possible to be overwhelmed with input and not give the renderer an opportunity to update the screen. This gave the appearance of locking up. 2. Multiple frames could be rendered for a single vblank (redundant work) 3. Open loop rendering would inevitably have buffer swapping out of sync with vblanks and visual tearing would occur. This commit enables vsync on the glutin window. The rendering was all moved onto a separate thread from input/pty processing to support vsync. However, the rendering thread must be 100% synchronized with the updater thread. There's now a Mutex on the Term, and an atomic bool to ask the input/pty processing to yield to the renderer. One aspect of this feature that hasn't been worked out is how to limit the frame rate. Currently, it just free runs at the screen refresh rate. The initial attempt here included the input/pty processor holding a lock while waiting for input. This *almost* worked, but there was a (not uncommon) edge case where the terminal state was "dirty" but the renderer was not ready to draw. Instead of blocking on the refresh issue, it's being punted to after an MVP is released. The overhead of drawing at 60Hz was profiled to be ~5% CPU usage, and this is deemed acceptable for an MVP. --- Cargo.lock | 17 +++++++ Cargo.toml | 1 + src/main.rs | 134 ++++++++++++++++++++++++++++++++-------------------- 3 files changed, 100 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 79bd5805..ec79f5fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,6 +10,7 @@ dependencies = [ "glutin 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "notify 2.6.1 (git+https://github.com/passcod/rsnotify)", + "parking_lot 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -544,6 +545,17 @@ dependencies = [ "shared_library 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "parking_lot" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", + "smallvec 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "pkg-config" version = "0.3.8" @@ -622,6 +634,11 @@ name = "slab" version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "smallvec" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "tempfile" version = "2.1.3" diff --git a/Cargo.toml b/Cargo.toml index 6a8430a0..d069daf9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ notify = { git = "https://github.com/passcod/rsnotify" } bitflags = "*" font = { path = "./font" } errno = "0.1.6" +parking_lot = "0.2.6" [build-dependencies] gl_generator = "0.5" diff --git a/src/main.rs b/src/main.rs index fb062fc6..714831da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,6 +11,7 @@ extern crate glutin; extern crate cgmath; extern crate notify; extern crate errno; +extern crate parking_lot; #[macro_use] extern crate bitflags; @@ -28,8 +29,10 @@ mod term; mod util; use std::io::{Read, Write, BufWriter}; -use std::sync::Arc; -use std::sync::mpsc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{mpsc, Arc}; + +use parking_lot::Mutex; use font::FontDesc; use grid::Grid; @@ -55,7 +58,6 @@ enum ShouldExit { struct WriteNotifier<'a, W: Write + 'a>(&'a mut W); impl<'a, W: Write> input::Notify for WriteNotifier<'a, W> { fn notify(&mut self, message: &str) { - println!("writing: {:?} [{} bytes]", message.as_bytes(), message.as_bytes().len()); self.0.write(message.as_bytes()).unwrap(); } } @@ -119,8 +121,10 @@ static FONT_STYLE: &'static str = "Regular"; fn main() { - let window = glutin::WindowBuilder::new().build().unwrap(); - window.set_title("Alacritty"); + let window = glutin::WindowBuilder::new() + .with_vsync() + .with_title("Alacritty") + .build().unwrap(); // window.set_window_resize_callback(Some(resize_callback as fn(u32, u32))); gl::load_with(|symbol| window.get_proc_address(symbol) as *const _); @@ -163,7 +167,8 @@ fn main() { }; let mut glyph_cache = GlyphCache::new(rasterizer, desc, font_size); - + let needs_render = Arc::new(AtomicBool::new(true)); + let needs_render2 = needs_render.clone(); let (tx, rx) = mpsc::channel(); let reader_tx = tx.clone(); @@ -174,14 +179,74 @@ fn main() { } }); - let mut terminal = Term::new(tty, grid); + let terminal = Arc::new(Mutex::new(Term::new(tty, grid))); + let term_ref = terminal.clone(); let mut meter = Meter::new(); let mut pty_parser = ansi::Parser::new(); let window = Arc::new(window); let window_ref = window.clone(); - let render_thread = thread::spawn_named("Galaxy", move || { + + let update_thread = thread::spawn_named("Update", move || { + 'main_loop: loop { + let mut writer = BufWriter::new(&writer); + let mut input_processor = input::Processor::new(); + + // Handle case where renderer didn't acquire lock yet + if needs_render.load(Ordering::Acquire) { + ::std::thread::yield_now(); + continue; + } + + if process_should_exit() { + break; + } + + // Block waiting for next event and handle it + let event = match rx.recv() { + Ok(e) => e, + Err(mpsc::RecvError) => break, + }; + + // Need mutable terminal for updates; lock it. + let mut terminal = terminal.lock(); + let res = handle_event(event, + &mut writer, + &mut *terminal, + &mut pty_parser, + &mut input_processor); + if res == ShouldExit::Yes { + break; + } + + // Handle Any events that are in the queue + loop { + match rx.try_recv() { + Ok(e) => { + let res = handle_event(e, + &mut writer, + &mut *terminal, + &mut pty_parser, + &mut input_processor); + + if res == ShouldExit::Yes { + break; + } + }, + Err(mpsc::TryRecvError::Disconnected) => break 'main_loop, + Err(mpsc::TryRecvError::Empty) => break, + } + + // Release the lock if a render is needed + if needs_render.load(Ordering::Acquire) { + break; + } + } + } + }); + + let render_thread = thread::spawn_named("Render", move || { let _ = unsafe { window.make_current() }; unsafe { gl::Viewport(0, 0, width as i32, height as i32); @@ -195,55 +260,19 @@ fn main() { glyph_cache.init(&mut api); }); - let mut input_processor = input::Processor::new(); - - 'main_loop: loop { - { - let mut writer = BufWriter::new(&writer); - - // Block waiting for next event - match rx.recv() { - Ok(e) => { - let res = handle_event(e, - &mut writer, - &mut terminal, - &mut pty_parser, - &mut input_processor); - if res == ShouldExit::Yes { - break; - } - }, - Err(mpsc::RecvError) => break, - } - - // Handle Any events that have been queued - loop { - match rx.try_recv() { - Ok(e) => { - let res = handle_event(e, - &mut writer, - &mut terminal, - &mut pty_parser, - &mut input_processor); - - if res == ShouldExit::Yes { - break; - } - }, - Err(mpsc::TryRecvError::Disconnected) => break 'main_loop, - Err(mpsc::TryRecvError::Empty) => break, - } - - // TODO make sure this doesn't block renders - } - } - + loop { unsafe { gl::ClearColor(0.0, 0.0, 0.00, 1.0); gl::Clear(gl::COLOR_BUFFER_BIT); } { + // Flag that it's time for render + needs_render2.store(true, Ordering::Release); + // Acquire term lock + let mut terminal = term_ref.lock(); + // Have the lock, ok to lower flag + needs_render2.store(false, Ordering::Relaxed); let _sampler = meter.sampler(); renderer.with_api(&props, |mut api| { @@ -267,7 +296,7 @@ fn main() { window.swap_buffers().unwrap(); if process_should_exit() { - break 'main_loop; + break; } } }); @@ -283,6 +312,7 @@ fn main() { reader_thread.join().ok(); render_thread.join().ok(); + update_thread.join().ok(); println!("Goodbye"); }