Defer drawing the window until the CoreAnimation displayLayer: method is called (#3592)

GPUI (both 1 and 2) currently performs rendering, layout, and painting
at the end of every effect cycle. This leads to poor performance when
the app receives events more frequently than the display refreshes. Such
rapid events can come from a terminal, an LSP, or a mouse with a high
polling rate.

This PR changes GPUI so that we don't render until the OS notifies us
that the content will be presented, via the `displayLayer:` callback.

Because render, layout, and paint have side effects that are sometimes
relied on in tests, we currently keep the old behavior (drawing after
every effects cycle) in tests.

This is similar to what @ForLoveOfCats explored in
https://github.com/zed-industries/zed/pull/3542, but slightly simpler,
in that we're not using the display link. As a follow-up, we could use
the display link to start rendering earlier, to possibly reduce latency
further.

crates/gpui2/src/app.rs

@@ -9,7 +9,6 @@ use derive_more::{Deref, DerefMut};
 pub use entity_map::*;
 pub use model_context::*;
 use refineable::Refineable;
-use smallvec::SmallVec;
 use smol::future::FutureExt;
 #[cfg(any(test, feature = "test-support"))]
 pub use test_context::*;
@@ -597,21 +596,25 @@ impl AppContext {
             }
         }
 
-        let dirty_window_ids = self
+        for window in self.windows.values() {
-            .windows
+            if let Some(window) = window.as_ref() {
-            .iter()
-            .filter_map(|(_, window)| {
-                let window = window.as_ref()?;
                 if window.dirty {
-                    Some(window.handle.clone())
+                    window.platform_window.invalidate();
-                } else {
-                    None
                 }
-            })
+            }
-            .collect::<SmallVec<[_; 8]>>();
+        }
 
-        for dirty_window_handle in dirty_window_ids {
+        #[cfg(any(test, feature = "test-support"))]
-            dirty_window_handle.update(self, |_, cx| cx.draw()).unwrap();
+        for window in self
+            .windows
+            .values()
+            .filter_map(|window| {
+                let window = window.as_ref()?;
+                window.dirty.then_some(window.handle)
+            })
+            .collect::<Vec<_>>()
+        {
+            self.update_window(window, |_, cx| cx.draw()).unwrap();
         }
     }
 

crates/gpui2/src/platform.rs

@@ -46,6 +46,8 @@ pub(crate) fn current_platform() -> Rc<dyn Platform> {
     Rc::new(MacPlatform::new())
 }
 
+pub type DrawWindow = Box<dyn FnMut() -> Result<Scene>>;
+
 pub(crate) trait Platform: 'static {
     fn background_executor(&self) -> BackgroundExecutor;
     fn foreground_executor(&self) -> ForegroundExecutor;
@@ -66,6 +68,7 @@ pub(crate) trait Platform: 'static {
         &self,
         handle: AnyWindowHandle,
         options: WindowOptions,
+        draw: DrawWindow,
     ) -> Box<dyn PlatformWindow>;
 
     fn set_display_link_output_callback(
@@ -163,7 +166,7 @@ pub trait PlatformWindow {
     fn on_close(&self, callback: Box<dyn FnOnce()>);
     fn on_appearance_changed(&self, callback: Box<dyn FnMut()>);
     fn is_topmost_for_position(&self, position: Point<Pixels>) -> bool;
-    fn draw(&self, scene: Scene);
+    fn invalidate(&self);
 
     fn sprite_atlas(&self) -> Arc<dyn PlatformAtlas>;
 

crates/gpui2/src/platform/mac/platform.rs

@@ -3,7 +3,8 @@ use crate::{
     Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId,
     ForegroundExecutor, InputEvent, Keymap, MacDispatcher, MacDisplay, MacDisplayLinker,
     MacTextSystem, MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay,
-    PlatformTextSystem, PlatformWindow, Result, SemanticVersion, VideoTimestamp, WindowOptions,
+    PlatformTextSystem, PlatformWindow, Result, Scene, SemanticVersion, VideoTimestamp,
+    WindowOptions,
 };
 use anyhow::anyhow;
 use block::ConcreteBlock;
@@ -490,8 +491,14 @@ impl Platform for MacPlatform {
         &self,
         handle: AnyWindowHandle,
         options: WindowOptions,
+        draw: Box<dyn FnMut() -> Result<Scene>>,
     ) -> Box<dyn PlatformWindow> {
-        Box::new(MacWindow::open(handle, options, self.foreground_executor()))
+        Box::new(MacWindow::open(
+            handle,
+            options,
+            draw,
+            self.foreground_executor(),
+        ))
     }
 
     fn set_display_link_output_callback(

crates/gpui2/src/platform/mac/window.rs

@@ -1,10 +1,10 @@
 use super::{display_bounds_from_native, ns_string, MacDisplay, MetalRenderer, NSRange};
 use crate::{
-    display_bounds_to_native, point, px, size, AnyWindowHandle, Bounds, ExternalPaths,
+    display_bounds_to_native, point, px, size, AnyWindowHandle, Bounds, DrawWindow, ExternalPaths,
     FileDropEvent, ForegroundExecutor, GlobalPixels, InputEvent, KeyDownEvent, Keystroke,
     Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent,
     Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point,
-    PromptLevel, Scene, Size, Timer, WindowAppearance, WindowBounds, WindowKind, WindowOptions,
+    PromptLevel, Size, Timer, WindowAppearance, WindowBounds, WindowKind, WindowOptions,
 };
 use block::ConcreteBlock;
 use cocoa::{
@@ -45,6 +45,7 @@ use std::{
     sync::{Arc, Weak},
     time::Duration,
 };
+use util::ResultExt;
 
 const WINDOW_STATE_IVAR: &str = "windowState";
 
@@ -318,7 +319,7 @@ struct MacWindowState {
     executor: ForegroundExecutor,
     native_window: id,
     renderer: MetalRenderer,
-    scene_to_render: Option<Scene>,
+    draw: Option<DrawWindow>,
     kind: WindowKind,
     event_callback: Option<Box<dyn FnMut(InputEvent) -> bool>>,
     activate_callback: Option<Box<dyn FnMut(bool)>>,
@@ -454,6 +455,7 @@ impl MacWindow {
     pub fn open(
         handle: AnyWindowHandle,
         options: WindowOptions,
+        draw: DrawWindow,
         executor: ForegroundExecutor,
     ) -> Self {
         unsafe {
@@ -545,7 +547,7 @@ impl MacWindow {
                 executor,
                 native_window,
                 renderer: MetalRenderer::new(true),
-                scene_to_render: None,
+                draw: Some(draw),
                 kind: options.kind,
                 event_callback: None,
                 activate_callback: None,
@@ -958,9 +960,8 @@ impl PlatformWindow for MacWindow {
         }
     }
 
-    fn draw(&self, scene: Scene) {
+    fn invalidate(&self) {
-        let mut this = self.0.lock();
+        let this = self.0.lock();
-        this.scene_to_render = Some(scene);
         unsafe {
             let _: () = msg_send![this.native_window.contentView(), setNeedsDisplay: YES];
         }
@@ -1442,8 +1443,11 @@ extern "C" fn set_frame_size(this: &Object, _: Sel, size: NSSize) {
 extern "C" fn display_layer(this: &Object, _: Sel, _: id) {
     unsafe {
         let window_state = get_window_state(this);
-        let mut window_state = window_state.as_ref().lock();
+        let mut draw = window_state.lock().draw.take().unwrap();
-        if let Some(scene) = window_state.scene_to_render.take() {
+        let scene = draw().log_err();
+        let mut window_state = window_state.lock();
+        window_state.draw = Some(draw);
+        if let Some(scene) = scene {
             window_state.renderer.draw(&scene);
         }
     }

crates/gpui2/src/platform/test/platform.rs

@@ -1,6 +1,7 @@
 use crate::{
     AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, ForegroundExecutor,
-    Keymap, Platform, PlatformDisplay, PlatformTextSystem, TestDisplay, TestWindow, WindowOptions,
+    Keymap, Platform, PlatformDisplay, PlatformTextSystem, Scene, TestDisplay, TestWindow,
+    WindowOptions,
 };
 use anyhow::{anyhow, Result};
 use collections::VecDeque;
@@ -135,6 +136,7 @@ impl Platform for TestPlatform {
         &self,
         handle: AnyWindowHandle,
         options: WindowOptions,
+        _draw: Box<dyn FnMut() -> Result<Scene>>,
     ) -> Box<dyn crate::PlatformWindow> {
         *self.active_window.lock() = Some(handle);
         Box::new(TestWindow::new(

crates/gpui2/src/platform/test/window.rs

@@ -1,7 +1,7 @@
 use crate::{
     px, AtlasKey, AtlasTextureId, AtlasTile, Pixels, PlatformAtlas, PlatformDisplay,
-    PlatformInputHandler, PlatformWindow, Point, Scene, Size, TestPlatform, TileId,
+    PlatformInputHandler, PlatformWindow, Point, Size, TestPlatform, TileId, WindowAppearance,
-    WindowAppearance, WindowBounds, WindowOptions,
+    WindowBounds, WindowOptions,
 };
 use collections::HashMap;
 use parking_lot::Mutex;
@@ -20,7 +20,6 @@ pub(crate) struct TestWindowHandlers {
 
 pub struct TestWindow {
     pub(crate) bounds: WindowBounds,
-    current_scene: Mutex<Option<Scene>>,
     display: Rc<dyn PlatformDisplay>,
     pub(crate) window_title: Option<String>,
     pub(crate) input_handler: Option<Arc<Mutex<Box<dyn PlatformInputHandler>>>>,
@@ -37,7 +36,6 @@ impl TestWindow {
     ) -> Self {
         Self {
             bounds: options.bounds,
-            current_scene: Default::default(),
             display,
             platform,
             input_handler: None,
@@ -166,8 +164,8 @@ impl PlatformWindow for TestWindow {
         unimplemented!()
     }
 
-    fn draw(&self, scene: crate::Scene) {
+    fn invalidate(&self) {
-        self.current_scene.lock().replace(scene);
+        // (self.draw.lock())().unwrap();
     }
 
     fn sprite_atlas(&self) -> sync::Arc<dyn crate::PlatformAtlas> {

crates/gpui2/src/window.rs

@@ -7,9 +7,9 @@ use crate::{
     ModelContext, Modifiers, MonochromeSprite, MouseButton, MouseMoveEvent, MouseUpEvent, Path,
     Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point,
     PolychromeSprite, PromptLevel, Quad, Render, RenderGlyphParams, RenderImageParams,
-    RenderSvgParams, ScaledPixels, SceneBuilder, Shadow, SharedString, Size, Style, SubscriberSet,
+    RenderSvgParams, ScaledPixels, Scene, SceneBuilder, Shadow, SharedString, Size, Style,
-    Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, VisualContext,
+    SubscriberSet, Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View,
-    WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS,
+    VisualContext, WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS,
 };
 use anyhow::{anyhow, Context as _, Result};
 use collections::HashMap;
@@ -223,8 +223,8 @@ pub struct Window {
     bounds: WindowBounds,
     bounds_observers: SubscriberSet<(), AnyObserver>,
     active: bool,
-    activation_observers: SubscriberSet<(), AnyObserver>,
     pub(crate) dirty: bool,
+    activation_observers: SubscriberSet<(), AnyObserver>,
     pub(crate) last_blur: Option<Option<FocusId>>,
     pub(crate) focus: Option<FocusId>,
 }
@@ -278,7 +278,14 @@ impl Window {
         options: WindowOptions,
         cx: &mut AppContext,
     ) -> Self {
-        let platform_window = cx.platform.open_window(handle, options);
+        let platform_window = cx.platform.open_window(
+            handle,
+            options,
+            Box::new({
+                let mut cx = cx.to_async();
+                move || handle.update(&mut cx, |_, cx| cx.draw())
+            }),
+        );
         let display_id = platform_window.display().id();
         let sprite_atlas = platform_window.sprite_atlas();
         let mouse_position = platform_window.mouse_position();
@@ -349,8 +356,8 @@ impl Window {
             bounds,
             bounds_observers: SubscriberSet::new(),
             active: false,
+            dirty: false,
             activation_observers: SubscriberSet::new(),
-            dirty: true,
             last_blur: None,
             focus: None,
         }
@@ -673,7 +680,7 @@ impl<'a> WindowContext<'a> {
         self.window.viewport_size = self.window.platform_window.content_size();
         self.window.bounds = self.window.platform_window.bounds();
         self.window.display_id = self.window.platform_window.display().id();
-        self.window.dirty = true;
+        self.notify();
 
         self.window
             .bounds_observers
@@ -1174,7 +1181,7 @@ impl<'a> WindowContext<'a> {
     }
 
     /// Draw pixels to the display for this window based on the contents of its scene.
-    pub(crate) fn draw(&mut self) {
+    pub(crate) fn draw(&mut self) -> Scene {
         self.text_system().start_frame();
         self.window.platform_window.clear_input_handler();
         self.window.layout_engine.as_mut().unwrap().clear();
@@ -1226,15 +1233,15 @@ impl<'a> WindowContext<'a> {
         mem::swap(&mut window.rendered_frame, &mut window.next_frame);
 
         let scene = self.window.rendered_frame.scene_builder.build();
-        self.window.platform_window.draw(scene);
         let cursor_style = self
             .window
             .requested_cursor_style
             .take()
             .unwrap_or(CursorStyle::Arrow);
         self.platform.set_cursor_style(cursor_style);
-
         self.window.dirty = false;
+
+        scene
     }
 
     /// Dispatch a mouse or keyboard event on the window.