Defer drawing the scene until macOS's `display_layer` callback

Antonio Scandurra and Max Brunsfeld created

Co-Authored-By: Max Brunsfeld <max@zed.dev>

Change summary

crates/gpui2/src/app.rs                    | 20 -----------------
crates/gpui2/src/platform.rs               |  5 +++
crates/gpui2/src/platform/mac/platform.rs  | 11 ++++++++-
crates/gpui2/src/platform/mac/window.rs    | 22 ++++++++++++--------
crates/gpui2/src/platform/test/platform.rs |  4 ++
crates/gpui2/src/platform/test/window.rs   |  4 --
crates/gpui2/src/window.rs                 | 26 +++++++++++++----------
7 files changed, 46 insertions(+), 46 deletions(-)

Detailed changes

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::*;
@@ -596,23 +595,6 @@ impl AppContext {
                 break;
             }
         }
-
-        let dirty_window_ids = self
-            .windows
-            .iter()
-            .filter_map(|(_, window)| {
-                let window = window.as_ref()?;
-                if window.dirty {
-                    Some(window.handle.clone())
-                } else {
-                    None
-                }
-            })
-            .collect::<SmallVec<[_; 8]>>();
-
-        for dirty_window_handle in dirty_window_ids {
-            dirty_window_handle.update(self, |_, cx| cx.draw()).unwrap();
-        }
     }
 
     /// Repeatedly called during `flush_effects` to release any entities whose
@@ -731,7 +713,7 @@ impl AppContext {
     fn apply_refresh_effect(&mut self) {
         for window in self.windows.values_mut() {
             if let Some(window) = window.as_mut() {
-                window.dirty = true;
+                window.platform_window.invalidate();
             }
         }
     }

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) {
-        let mut this = self.0.lock();
-        this.scene_to_render = Some(scene);
+    fn invalidate(&self) {
+        let this = self.0.lock();
         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();
-        if let Some(scene) = window_state.scene_to_render.take() {
+        let mut draw = window_state.lock().draw.take().unwrap();
+        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 🔗

@@ -166,9 +166,7 @@ impl PlatformWindow for TestWindow {
         unimplemented!()
     }
 
-    fn draw(&self, scene: crate::Scene) {
-        self.current_scene.lock().replace(scene);
-    }
+    fn invalidate(&self) {}
 
     fn sprite_atlas(&self) -> sync::Arc<dyn crate::PlatformAtlas> {
         self.sprite_atlas.clone()

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,
-    Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, VisualContext,
-    WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS,
+    RenderSvgParams, ScaledPixels, Scene, SceneBuilder, Shadow, SharedString, Size, Style,
+    SubscriberSet, Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View,
+    VisualContext, WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS,
 };
 use anyhow::{anyhow, Context as _, Result};
 use collections::HashMap;
@@ -224,7 +224,6 @@ pub struct Window {
     bounds_observers: SubscriberSet<(), AnyObserver>,
     active: bool,
     activation_observers: SubscriberSet<(), AnyObserver>,
-    pub(crate) dirty: bool,
     pub(crate) last_blur: Option<Option<FocusId>>,
     pub(crate) focus: Option<FocusId>,
 }
@@ -278,7 +277,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();
@@ -350,7 +356,6 @@ impl Window {
             bounds_observers: SubscriberSet::new(),
             active: false,
             activation_observers: SubscriberSet::new(),
-            dirty: true,
             last_blur: None,
             focus: None,
         }
@@ -401,7 +406,7 @@ impl<'a> WindowContext<'a> {
 
     /// Mark the window as dirty, scheduling it to be redrawn on the next frame.
     pub fn notify(&mut self) {
-        self.window.dirty = true;
+        self.window.platform_window.invalidate();
     }
 
     /// Close this window.
@@ -673,7 +678,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 +1179,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) {
+    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,7 +1231,6 @@ 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
@@ -1234,7 +1238,7 @@ impl<'a> WindowContext<'a> {
             .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.