PolytreeDE
/
alacritty
mirror of https://github.com/alacritty/alacritty.git
1
0
Fork 0
Code Releases Activity

Fix pty read sometimes not triggering draw 

There was a lot of complexity around the threadsafe `Flag` type and
waking up the event loop. The idea was to prevent unnecessary calls to
the glutin window's wakeup_event_loop() method which can be expensive.
This complexity made it difficult to get synchronization between the pty
reader and the render thread correct. Now, the `dirty` flag on the
terminal is also used to prevent spurious wakeups. It is only changed
when the mutex is held, so race conditions associated with that flag
shouldn't happen.
This commit is contained in:
Joe Wilm 2016-12-29 21:38:22 -05:00
parent b704dafb24
commit a91a3f2dce
7 changed files with 37 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/display.rs
View File

@ -213,12 +213,6 @@ impl Display {
///
/// This call may block if vsync is enabled
pub fn draw(&mut self, mut terminal: MutexGuard<Term>, config: &Config, selection: &Selection) {
// This is a hack since sometimes we get stuck waiting for events
// in the main loop otherwise.
//
// TODO figure out why this is necessary
self.window.clear_wakeup_flag();
// Clear dirty flag
terminal.dirty = false;

26
src/event.rs
View File

@ -105,7 +105,6 @@ impl<N: Notify> Processor<N> {
fn handle_event<'a>(
processor: &mut input::Processor<'a, N>,
event: glutin::Event,
wakeup_request: &mut bool,
ref_test: bool,
resize_tx: &mpsc::Sender<(u32, u32)>,
) {
@ -135,18 +134,14 @@ impl<N: Notify> Processor<N> {
},
glutin::Event::Resized(w, h) => {
resize_tx.send((w, h)).expect("send new size");
// Previously, this marked the terminal state as "dirty", but
// now the wakeup_request controls whether a display update is
// triggered.
*wakeup_request = true;
processor.ctx.terminal.dirty = true;
},
glutin::Event::KeyboardInput(state, _code, key, mods, string) => {
processor.process_key(state, key, mods, string);
},
glutin::Event::MouseInput(state, button) => {
processor.mouse_input(state, button);
*wakeup_request = true;
processor.ctx.terminal.dirty = true;
},
glutin::Event::MouseMoved(x, y) => {
let x = limit(x, 0, processor.ctx.size_info.width as i32);
@ -155,17 +150,17 @@ impl<N: Notify> Processor<N> {
processor.mouse_moved(x as u32, y as u32);
if !processor.ctx.selection.is_empty() {
*wakeup_request = true;
processor.ctx.terminal.dirty = true;
}
},
glutin::Event::Focused(true) => {
*wakeup_request = true;
processor.ctx.terminal.dirty = true;
},
glutin::Event::MouseWheel(scroll_delta, touch_phase) => {
processor.on_mouse_wheel(scroll_delta, touch_phase);
},
glutin::Event::Awakened => {
*wakeup_request = true;
processor.ctx.terminal.dirty = true;
},
_ => (),
}
@ -176,13 +171,11 @@ impl<N: Notify> Processor<N> {
&mut self,
term: &'a FairMutex<Term>,
window: &Window
) -> (MutexGuard<'a, Term>, bool) {
let mut wakeup_request = false;
) -> MutexGuard<'a, Term> {
// Terminal is lazily initialized the first time an event is returned
// from the blocking WaitEventsIterator. Otherwise, the pty reader would
// be blocked the entire time we wait for input!
let terminal;
let mut terminal;
{
// Ditto on lazy initialization for context and processor.
@ -195,7 +188,6 @@ impl<N: Notify> Processor<N> {
Processor::handle_event(
&mut processor,
$event,
&mut wakeup_request,
self.ref_test,
&self.resize_tx,
)
@ -206,7 +198,7 @@ impl<N: Notify> Processor<N> {
Some(event) => {
terminal = term.lock();
context = ActionContext {
terminal: &terminal,
terminal: &mut terminal,
notifier: &mut self.notifier,
selection: &mut self.selection,
mouse: &mut self.mouse,
@ -230,7 +222,7 @@ impl<N: Notify> Processor<N> {
}
}
(terminal, wakeup_request)
terminal
}
pub fn update_config(&mut self, config: &Config) {

11
src/event_loop.rs
View File

@ -211,9 +211,14 @@ impl<Io> EventLoop<Io>
state.parser.advance(&mut *terminal, *byte, &mut self.pty);
}
terminal.dirty = true;
self.display.notify();
// Only request a draw if one hasn't already been requested.
//
// This is a performance optimization even if only for X11
// which is very expensive to hammer on the even loop wakeup
if !terminal.dirty {
self.display.notify();
terminal.dirty = true;
}
},
Err(err) => {
match err.kind() {

2
src/input.rs
View File

@ -49,7 +49,7 @@ pub struct Processor<'a, N: 'a> {
pub struct ActionContext<'a, N: 'a> {
pub notifier: &'a mut N,
pub terminal: &'a Term,
pub terminal: &'a mut Term,
pub selection: &'a mut Selection,
pub mouse: &'a mut Mouse,
pub size_info: &'a term::SizeInfo,

13
src/main.rs
View File

@ -79,7 +79,8 @@ fn run(mut config: Config, options: cli::Options) -> Result<(), Box<Error>> {
// This object contains all of the state about what's being displayed. It's
// wrapped in a clonable mutex since both the I/O loop and display need to
// access it.
let terminal = Arc::new(FairMutex::new(Term::new(display.size().to_owned())));
let terminal = Term::new(display.size().to_owned());
let terminal = Arc::new(FairMutex::new(terminal));
// Create the pty
//
@ -129,20 +130,20 @@ fn run(mut config: Config, options: cli::Options) -> Result<(), Box<Error>> {
// Main display loop
loop {
// Process input and window events
let (mut terminal, wakeup_request) = processor.process_events(&terminal, display.window());
let mut terminal = processor.process_events(&terminal, display.window());
// Handle config reloads
let config_updated = config_monitor.as_ref()
config_monitor.as_ref()
.and_then(|monitor| monitor.pending_config())
.map(|new_config| {
config = new_config;
display.update_config(&config);
processor.update_config(&config);
true
}).unwrap_or(false);
terminal.dirty = true;
});
// Maybe draw the terminal
if wakeup_request || config_updated {
if terminal.needs_draw() {
// Handle pending resize events
//
// The second argument is a list of types that want to be notified

7
src/term/mod.rs
View File

@ -298,7 +298,7 @@ impl Term {
let scroll_region = Line(0)..grid.num_lines();
Term {
dirty: true,
dirty: false,
grid: grid,
alt_grid: alt,
alt: false,
@ -313,6 +313,11 @@ impl Term {
}
}
#[inline]
pub fn needs_draw(&self) -> bool {
self.dirty
}
pub fn string_from_selection(&self, span: &Span) -> String {
/// Need a generic push() for the Append trait
trait PushChar {

75
src/window.rs
View File

@ -11,8 +11,6 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::convert::From;
use std::fmt::{self, Display};
use std::ops::Deref;
@ -57,16 +55,11 @@ type Result<T> = ::std::result::Result<T, Error>;
/// Wraps the underlying windowing library to provide a stable API in Alacritty
pub struct Window {
glutin_window: glutin::Window,
/// This flag allows calls to wakeup_event_loop to be coalesced. Waking the
/// event loop is potentially very slow (and indeed is on X11).
flag: Flag
}
/// Threadsafe APIs for the window
pub struct Proxy {
inner: glutin::WindowProxy,
flag: Flag,
}
/// Information about where the window is being displayed
@ -98,46 +91,6 @@ pub struct Pixels<T>(pub T);
#[derive(Debug, Copy, Clone)]
pub struct Points<T>(pub T);
/// A wrapper around glutin's `WaitEventsIterator` that clears the wakeup
/// optimization flag on drop.
pub struct WaitEventsIterator<'a> {
inner: glutin::WaitEventsIterator<'a>,
flag: Flag
}
impl<'a> Iterator for WaitEventsIterator<'a> {
type Item = glutin::Event;
#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.inner.next()
}
}
impl<'a> Drop for WaitEventsIterator<'a> {
fn drop(&mut self) {
self.flag.set(false);
}
}
#[derive(Clone)]
pub struct Flag(pub Arc<AtomicBool>);
impl Flag {
pub fn new(initial_value: bool) -> Flag {
Flag(Arc::new(AtomicBool::new(initial_value)))
}
#[inline]
pub fn get(&self) -> bool {
self.0.load(Ordering::Acquire)
}
#[inline]
pub fn set(&self, value: bool) {
self.0.store(value, Ordering::Release)
}
}
pub trait ToPoints {
fn to_points(&self, scale: f32) -> Size<Points<u32>>;
}
@ -265,7 +218,6 @@ impl Window {
Ok(Window {
glutin_window: window,
flag: Flag::new(false),
})
}
@ -308,7 +260,6 @@ impl Window {
pub fn create_window_proxy(&self) -> Proxy {
Proxy {
inner: self.glutin_window.create_window_proxy(),
flag: self.flag.clone(),
}
}
@ -319,26 +270,18 @@ impl Window {
.map_err(From::from)
}
/// Block waiting for events
/// Poll for any available events
#[inline]
pub fn wait_events(&self) -> WaitEventsIterator {
WaitEventsIterator {
inner: self.glutin_window.wait_events(),
flag: self.flag.clone()
}
}
/// Clear the wakeup optimization flag
pub fn clear_wakeup_flag(&self) {
self.flag.set(false);
pub fn poll_events(&self) -> glutin::PollEventsIterator {
self.glutin_window.poll_events()
}
/// Block waiting for events
///
/// FIXME should return our own type
#[inline]
pub fn poll_events(&self) -> glutin::PollEventsIterator {
self.glutin_window.poll_events()
pub fn wait_events(&self) -> glutin::WaitEventsIterator {
self.glutin_window.wait_events()
}
}
@ -348,13 +291,7 @@ impl Proxy {
/// This is useful for triggering a draw when the renderer would otherwise
/// be waiting on user input.
pub fn wakeup_event_loop(&self) {
// Only wake up the window event loop if it hasn't already been
// signaled. This is a really important optimization because waking up
// the event loop redundantly burns *a lot* of cycles.
if !self.flag.get() {
self.inner.wakeup_event_loop();
self.flag.set(true);
}
self.inner.wakeup_event_loop();
}
}