feat: Smoother render loop (#2188)

* Only poll when there's no pending render

The previous approach caused some unneded polls

* Animate and shape as early as possible in the frame

* Animate when frames are skipped to reduce visual artifacts

* Use the monitor refresh rate when available instead of a smoothed average

* Base the frame rate on the monitor refresh

* Remove the frame dt average calculation

* Simplify the Windows vsync by emulating the winit one

* Only render when there are pending renders

* Remove the render thread on Windows

The Winit event loop is more optimized now, and it does not really play
well with our way of signaling events. It's also one less platform
dependent thing.

* Smoother windows vsync, using DwmGetCompositionTimingInfo

* Simplify the macOS vsync implementation

* Fix typo

* Implement request redraw

* style: reformat line wrapping

---------

Co-authored-by: MultisampledNight <contact@multisamplednight.com>

Cargo.toml

@@ -71,13 +71,14 @@ winit = { version = "=0.29.4", features = ["serde"] }
 xdg = "2.4.1"
 
 [dev-dependencies]
+approx = "0.5.1"
 mockall = "0.11.0"
 scoped-env = "2.1.0"
 serial_test = "2.0.0"
 
 [target.'cfg(target_os = "windows")'.dependencies]
 # NOTE: winerror is only needed because the indirect dependency parity-tokio-ipc does not set it even if it uses it
-winapi = { version = "0.3.9", features = ["winuser", "wincon", "winerror", "dwmapi"] }
+winapi = { version = "0.3.9", features = ["winuser", "wincon", "winerror", "dwmapi", "profileapi"] }
 
 [target.'cfg(target_os = "macos")'.dependencies]
 cocoa = "0.24.0"

src/main.rs

@@ -5,6 +5,10 @@
 #[macro_use]
 extern crate neovide_derive;
 
+#[cfg(test)]
+#[macro_use]
+extern crate approx;
+
 #[macro_use]
 extern crate clap;
 
@@ -172,6 +176,7 @@ fn setup(proxy: EventLoopProxy<UserEvent>) -> Result<(WindowSize, NeovimRuntime)
     cmd_line::handle_command_line_arguments(args().collect())?;
     #[cfg(not(target_os = "windows"))]
     maybe_disown();
+
     startup_profiler();
 
     #[cfg(not(test))]

src/renderer/vsync/mod.rs

@@ -8,7 +8,11 @@ mod vsync_win;
 
 use vsync_timer::VSyncTimer;
 
-use crate::renderer::WindowedContext;
+use crate::{
+    renderer::WindowedContext, settings::SETTINGS, window::UserEvent, window::WindowSettings,
+};
+use winit::event_loop::EventLoopProxy;
+
 #[cfg(target_os = "linux")]
 use std::env;
 
@@ -30,7 +34,12 @@ pub enum VSync {
 }
 
 impl VSync {
-    pub fn new(vsync_enabled: bool, #[allow(unused_variables)] context: &WindowedContext) -> Self {
+    #[allow(unused_variables)]
+    pub fn new(
+        vsync_enabled: bool,
+        context: &WindowedContext,
+        proxy: EventLoopProxy<UserEvent>,
+    ) -> Self {
         if vsync_enabled {
             #[cfg(target_os = "linux")]
             if env::var("WAYLAND_DISPLAY").is_ok() {
@@ -41,12 +50,12 @@ impl VSync {
 
             #[cfg(target_os = "windows")]
             {
-                VSync::Windows(VSyncWin::new())
+                VSync::Windows(VSyncWin::new(proxy))
             }
 
             #[cfg(target_os = "macos")]
             {
-                VSync::Macos(VSyncMacos::new(context))
+                VSync::Macos(VSyncMacos::new(context, proxy))
             }
         } else {
             VSync::Timer(VSyncTimer::new())
@@ -65,7 +74,14 @@ impl VSync {
     }
 
     pub fn uses_winit_throttling(&self) -> bool {
-        matches!(self, VSync::WinitThrottling())
+        #[cfg(target_os = "windows")]
+        return matches!(self, VSync::WinitThrottling() | VSync::Windows(..));
+
+        #[cfg(target_os = "macos")]
+        return matches!(self, VSync::WinitThrottling() | VSync::Macos(..));
+
+        #[cfg(target_os = "linux")]
+        return matches!(self, VSync::WinitThrottling());
     }
 
     pub fn update(&mut self, #[allow(unused_variables)] context: &WindowedContext) {
@@ -75,4 +91,32 @@ impl VSync {
             _ => {}
         }
     }
+
+    pub fn get_refresh_rate(&self, context: &WindowedContext) -> f32 {
+        let settings_refresh_rate = 1.0 / SETTINGS.get::<WindowSettings>().refresh_rate as f32;
+
+        match self {
+            VSync::Timer(_) => settings_refresh_rate,
+            _ => {
+                let monitor = context.window().current_monitor();
+                monitor
+                    .and_then(|monitor| monitor.refresh_rate_millihertz())
+                    .map(|rate| 1000.0 / rate as f32)
+                    .unwrap_or_else(|| settings_refresh_rate)
+                    // We don't really want to support less than 10 FPS
+                    .min(0.1)
+            }
+        }
+    }
+
+    pub fn request_redraw(&mut self, context: &WindowedContext) {
+        match self {
+            VSync::WinitThrottling(..) => context.window().request_redraw(),
+            #[cfg(target_os = "windows")]
+            VSync::Windows(vsync) => vsync.request_redraw(),
+            #[cfg(target_os = "macos")]
+            VSync::Macos(vsync) => vsync.request_redraw(),
+            _ => {}
+        }
+    }
 }

src/renderer/vsync/vsync_macos.rs

@@ -1,60 +1,63 @@
-use std::sync::{Arc, Condvar, Mutex};
-
 use log::{error, trace, warn};
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
 
-use crate::renderer::WindowedContext;
+use winit::event_loop::EventLoopProxy;
+
+use crate::{renderer::WindowedContext, window::UserEvent};
 
 use super::macos_display_link::{
     core_video, get_display_id_of_window, MacosDisplayLink, MacosDisplayLinkCallbackArgs,
 };
 
 struct VSyncMacosDisplayLinkUserData {
-    vsync_count: Arc<(Mutex<usize>, Condvar)>,
+    proxy: EventLoopProxy<UserEvent>,
+    redraw_requested: Arc<AtomicBool>,
 }
 
 fn vsync_macos_display_link_callback(
     _args: &mut MacosDisplayLinkCallbackArgs,
     user_data: &mut VSyncMacosDisplayLinkUserData,
 ) {
-    let (lock, cvar) = &*user_data.vsync_count;
-    let mut count = lock.lock().unwrap();
-    *count += 1;
-    cvar.notify_one();
+    if user_data.redraw_requested.swap(false, Ordering::Relaxed) {
+        let _ = user_data.proxy.send_event(UserEvent::RedrawRequested);
+    }
 }
 
 pub struct VSyncMacos {
     old_display: core_video::CGDirectDisplayID,
     display_link: Option<MacosDisplayLink<VSyncMacosDisplayLinkUserData>>,
-    vsync_count: Arc<(Mutex<usize>, Condvar)>,
-    last_vsync: usize,
+    proxy: EventLoopProxy<UserEvent>,
+    redraw_requested: Arc<AtomicBool>,
 }
 
 impl VSyncMacos {
-    pub fn new(context: &WindowedContext) -> Self {
+    pub fn new(context: &WindowedContext, proxy: EventLoopProxy<UserEvent>) -> VSyncMacos {
+        let redraw_requested = AtomicBool::new(false).into();
         let mut vsync = VSyncMacos {
             old_display: 0,
             display_link: None,
-            vsync_count: Arc::new((Mutex::new(0), Condvar::new())),
-            last_vsync: 0,
+            proxy,
+            redraw_requested,
         };
 
-        vsync.display_link = vsync.create_display_link(context);
+        vsync.create_display_link(context);
 
         vsync
     }
 
-    fn create_display_link(
-        self: &mut Self,
-        context: &WindowedContext,
-    ) -> Option<MacosDisplayLink<VSyncMacosDisplayLinkUserData>> {
+    fn create_display_link(&mut self, context: &WindowedContext) {
         self.old_display = get_display_id_of_window(context.window());
 
-        let vsync_count = self.vsync_count.clone();
-
-        match MacosDisplayLink::new_from_display(
+        let display_link = match MacosDisplayLink::new_from_display(
             self.old_display,
             vsync_macos_display_link_callback,
-            VSyncMacosDisplayLinkUserData { vsync_count },
+            VSyncMacosDisplayLinkUserData {
+                proxy: self.proxy.clone(),
+                redraw_requested: Arc::clone(&self.redraw_requested),
+            },
         ) {
             Ok(display_link) => {
                 trace!("Succeeded to create display link.");
@@ -77,22 +80,21 @@ impl VSyncMacos {
                 error!("Failed to create display link, CVReturn code: {}.", code);
                 None
             }
-        }
+        };
+        self.display_link = display_link;
     }
 
-    pub fn wait_for_vsync(&mut self) {
-        let (lock, cvar) = &*self.vsync_count;
-        let count = cvar
-            .wait_while(lock.lock().unwrap(), |count| *count < self.last_vsync + 1)
-            .unwrap();
-        self.last_vsync = *count;
+    pub fn wait_for_vsync(&mut self) {}
+
+    pub fn request_redraw(&mut self) {
+        self.redraw_requested.store(true, Ordering::Relaxed);
     }
 
     pub fn update(&mut self, context: &WindowedContext) {
         let new_display = get_display_id_of_window(context.window());
         if new_display != self.old_display {
             trace!("Window moved to a new screen, try to re-create the display link.");
-            self.display_link = self.create_display_link(context);
+            self.create_display_link(context);
         }
     }
 }