Checkpoint

Antonio Scandurra created

Change summary

crates/gpui3/src/app.rs                         |  9 -
crates/gpui3/src/platform.rs                    | 19 +--
crates/gpui3/src/platform/mac/display_linker.rs | 54 +++++-------
crates/gpui3/src/platform/mac/platform.rs       | 81 +++++++++++-------
crates/gpui3/src/platform/test.rs               | 20 +++
crates/gpui3/src/window.rs                      | 44 ++++++---
6 files changed, 125 insertions(+), 102 deletions(-)

Detailed changes

crates/gpui3/src/app.rs 🔗

@@ -9,9 +9,8 @@ use refineable::Refineable;
 
 use crate::{
     current_platform, image_cache::ImageCache, AssetSource, Context, DisplayId, Executor, LayoutId,
-    MainThread, MainThreadOnly, Platform, PlatformDisplayLinker, RootView, SubscriberSet,
-    SvgRenderer, Task, TextStyle, TextStyleRefinement, TextSystem, Window, WindowContext,
-    WindowHandle, WindowId,
+    MainThread, MainThreadOnly, Platform, RootView, SubscriberSet, SvgRenderer, Task, TextStyle,
+    TextStyleRefinement, TextSystem, Window, WindowContext, WindowHandle, WindowId,
 };
 use anyhow::{anyhow, Result};
 use collections::{HashMap, VecDeque};
@@ -55,7 +54,6 @@ impl App {
                 this: this.clone(),
                 text_system: Arc::new(TextSystem::new(platform.text_system())),
                 pending_updates: 0,
-                display_linker: platform.display_linker(),
                 next_frame_callbacks: Default::default(),
                 platform: MainThreadOnly::new(platform, executor.clone()),
                 executor,
@@ -94,10 +92,9 @@ type FrameCallback = Box<dyn FnOnce(&mut WindowContext) + Send>;
 
 pub struct AppContext {
     this: Weak<Mutex<AppContext>>,
-    platform: MainThreadOnly<dyn Platform>,
+    pub(crate) platform: MainThreadOnly<dyn Platform>,
     text_system: Arc<TextSystem>,
     pending_updates: usize,
-    pub(crate) display_linker: Arc<dyn PlatformDisplayLinker>,
     pub(crate) next_frame_callbacks: HashMap<DisplayId, Vec<FrameCallback>>,
     pub(crate) executor: Executor,
     pub(crate) svg_renderer: SvgRenderer,

crates/gpui3/src/platform.rs 🔗

@@ -40,7 +40,6 @@ pub(crate) fn current_platform() -> Arc<dyn Platform> {
 
 pub(crate) trait Platform: 'static {
     fn executor(&self) -> Executor;
-    fn display_linker(&self) -> Arc<dyn PlatformDisplayLinker>;
     fn text_system(&self) -> Arc<dyn PlatformTextSystem>;
 
     fn run(&self, on_finish_launching: Box<dyn 'static + FnOnce()>);
@@ -59,6 +58,14 @@ pub(crate) trait Platform: 'static {
         handle: AnyWindowHandle,
         options: WindowOptions,
     ) -> Box<dyn PlatformWindow>;
+
+    fn set_display_link_output_callback(
+        &self,
+        display_id: DisplayId,
+        callback: Box<dyn FnMut(&VideoTimestamp, &VideoTimestamp)>,
+    );
+    fn start_display_link(&self, display_id: DisplayId);
+    fn stop_display_link(&self, display_id: DisplayId);
     // fn add_status_item(&self, _handle: AnyWindowHandle) -> Box<dyn PlatformWindow>;
 
     fn open_url(&self, url: &str);
@@ -154,16 +161,6 @@ pub trait PlatformDispatcher: Send + Sync {
     fn dispatch_on_main_thread(&self, task: Runnable);
 }
 
-pub trait PlatformDisplayLinker: Send + Sync {
-    fn set_output_callback(
-        &self,
-        display_id: DisplayId,
-        callback: Box<dyn FnMut(&VideoTimestamp, &VideoTimestamp)>,
-    );
-    fn start(&self, display_id: DisplayId);
-    fn stop(&self, display_id: DisplayId);
-}
-
 pub trait PlatformTextSystem: Send + Sync {
     fn add_fonts(&self, fonts: &[Arc<Vec<u8>>]) -> Result<()>;
     fn all_font_families(&self) -> Vec<String>;

crates/gpui3/src/platform/mac/display_linker.rs 🔗

@@ -4,27 +4,23 @@ use std::{
     sync::{Arc, Weak},
 };
 
-use crate::{DisplayId, Executor, MainThreadOnly, PlatformDisplayLinker};
+use crate::DisplayId;
 use collections::HashMap;
 use parking_lot::Mutex;
 pub use sys::CVTimeStamp as VideoTimestamp;
 
-pub struct MacDisplayLinker {
-    links: Mutex<HashMap<DisplayId, MacDisplayLink>>,
-    executor: Executor,
+pub(crate) struct MacDisplayLinker {
+    links: HashMap<DisplayId, MacDisplayLink>,
 }
 
 struct MacDisplayLink {
-    system_link: MainThreadOnly<sys::DisplayLink>,
+    system_link: sys::DisplayLink,
     _output_callback: Arc<OutputCallback>,
 }
 
-unsafe impl Send for MacDisplayLink {}
-
 impl MacDisplayLinker {
-    pub fn new(executor: Executor) -> Self {
+    pub fn new() -> Self {
         MacDisplayLinker {
-            executor,
             links: Default::default(),
         }
     }
@@ -32,9 +28,9 @@ impl MacDisplayLinker {
 
 type OutputCallback = Mutex<Box<dyn FnMut(&VideoTimestamp, &VideoTimestamp)>>;
 
-impl PlatformDisplayLinker for MacDisplayLinker {
-    fn set_output_callback(
-        &self,
+impl MacDisplayLinker {
+    pub fn set_output_callback(
+        &mut self,
         display_id: DisplayId,
         output_callback: Box<dyn FnMut(&VideoTimestamp, &VideoTimestamp)>,
     ) {
@@ -43,11 +39,11 @@ impl PlatformDisplayLinker for MacDisplayLinker {
             let weak_callback_ptr: *const OutputCallback = Arc::downgrade(&callback).into_raw();
             unsafe { system_link.set_output_callback(trampoline, weak_callback_ptr as *mut c_void) }
 
-            self.links.lock().insert(
+            self.links.insert(
                 display_id,
                 MacDisplayLink {
                     _output_callback: callback,
-                    system_link: MainThreadOnly::new(Arc::new(system_link), self.executor.clone()),
+                    system_link,
                 },
             );
         } else {
@@ -56,26 +52,20 @@ impl PlatformDisplayLinker for MacDisplayLinker {
         }
     }
 
-    fn start(&self, display_id: DisplayId) {
-        if let Some(link) = self.links.lock().get_mut(&display_id) {
+    pub fn start(&mut self, display_id: DisplayId) {
+        if let Some(link) = self.links.get_mut(&display_id) {
             unsafe {
-                let system_link = link.system_link.clone();
-                self.executor
-                    .run_on_main(move || system_link.borrow_on_main_thread().start())
-                    .detach();
+                link.system_link.start();
             }
         } else {
             log::warn!("No DisplayLink callback registered for {:?}", display_id)
         }
     }
 
-    fn stop(&self, display_id: DisplayId) {
-        if let Some(link) = self.links.lock().get_mut(&display_id) {
+    pub fn stop(&mut self, display_id: DisplayId) {
+        if let Some(link) = self.links.get_mut(&display_id) {
             unsafe {
-                let system_link = link.system_link.clone();
-                self.executor
-                    .run_on_main(move || system_link.borrow_on_main_thread().stop())
-                    .detach();
+                link.system_link.stop();
             }
         } else {
             log::warn!("No DisplayLink callback registered for {:?}", display_id)
@@ -218,16 +208,16 @@ mod sys {
             display_link_out: *mut *mut CVDisplayLink,
         ) -> i32;
         pub fn CVDisplayLinkSetOutputCallback(
-            display_link: &DisplayLinkRef,
+            display_link: &mut DisplayLinkRef,
             callback: CVDisplayLinkOutputCallback,
             user_info: *mut c_void,
         ) -> i32;
         pub fn CVDisplayLinkSetCurrentCGDisplay(
-            display_link: &DisplayLinkRef,
+            display_link: &mut DisplayLinkRef,
             display_id: u32,
         ) -> i32;
-        pub fn CVDisplayLinkStart(display_link: &DisplayLinkRef) -> i32;
-        pub fn CVDisplayLinkStop(display_link: &DisplayLinkRef) -> i32;
+        pub fn CVDisplayLinkStart(display_link: &mut DisplayLinkRef) -> i32;
+        pub fn CVDisplayLinkStop(display_link: &mut DisplayLinkRef) -> i32;
         pub fn CVDisplayLinkRelease(display_link: *mut CVDisplayLink);
         pub fn CVDisplayLinkRetain(display_link: *mut CVDisplayLink) -> *mut CVDisplayLink;
     }
@@ -272,12 +262,12 @@ mod sys {
         }
 
         /// Apple docs: [CVDisplayLinkStart](https://developer.apple.com/documentation/corevideo/1457193-cvdisplaylinkstart?language=objc)
-        pub unsafe fn start(&self) {
+        pub unsafe fn start(&mut self) {
             assert_eq!(CVDisplayLinkStart(self), 0);
         }
 
         /// Apple docs: [CVDisplayLinkStop](https://developer.apple.com/documentation/corevideo/1457281-cvdisplaylinkstop?language=objc)
-        pub unsafe fn stop(&self) {
+        pub unsafe fn stop(&mut self) {
             assert_eq!(CVDisplayLinkStop(self), 0);
         }
     }

crates/gpui3/src/platform/mac/platform.rs 🔗

@@ -2,8 +2,8 @@ use super::BoolExt;
 use crate::{
     AnyWindowHandle, ClipboardItem, CursorStyle, DisplayId, Event, Executor, MacDispatcher,
     MacDisplay, MacDisplayLinker, MacTextSystem, MacWindow, PathPromptOptions, Platform,
-    PlatformDisplay, PlatformDisplayLinker, PlatformTextSystem, PlatformWindow, Result,
-    SemanticVersion, WindowOptions,
+    PlatformDisplay, PlatformTextSystem, PlatformWindow, Result, SemanticVersion, VideoTimestamp,
+    WindowOptions,
 };
 use anyhow::anyhow;
 use block::ConcreteBlock;
@@ -145,6 +145,7 @@ pub struct MacPlatform(Mutex<MacPlatformState>);
 pub struct MacPlatformState {
     executor: Executor,
     text_system: Arc<MacTextSystem>,
+    display_linker: MacDisplayLinker,
     pasteboard: id,
     text_hash_pasteboard_type: id,
     metadata_pasteboard_type: id,
@@ -166,6 +167,7 @@ impl MacPlatform {
         Self(Mutex::new(MacPlatformState {
             executor: Executor::new(Arc::new(MacDispatcher)),
             text_system: Arc::new(MacTextSystem::new()),
+            display_linker: MacDisplayLinker::new(),
             pasteboard: unsafe { NSPasteboard::generalPasteboard(nil) },
             text_hash_pasteboard_type: unsafe { ns_string("zed-text-hash") },
             metadata_pasteboard_type: unsafe { ns_string("zed-metadata") },
@@ -348,10 +350,6 @@ impl Platform for MacPlatform {
         self.0.lock().executor.clone()
     }
 
-    fn display_linker(&self) -> Arc<dyn PlatformDisplayLinker> {
-        Arc::new(MacDisplayLinker::new(self.executor()))
-    }
-
     fn text_system(&self) -> Arc<dyn PlatformTextSystem> {
         self.0.lock().text_system.clone()
     }
@@ -487,6 +485,25 @@ impl Platform for MacPlatform {
         Box::new(MacWindow::open(handle, options, self.executor()))
     }
 
+    fn set_display_link_output_callback(
+        &self,
+        display_id: DisplayId,
+        callback: Box<dyn FnMut(&VideoTimestamp, &VideoTimestamp)>,
+    ) {
+        self.0
+            .lock()
+            .display_linker
+            .set_output_callback(display_id, callback);
+    }
+
+    fn start_display_link(&self, display_id: DisplayId) {
+        self.0.lock().display_linker.start(display_id);
+    }
+
+    fn stop_display_link(&self, display_id: DisplayId) {
+        self.0.lock().display_linker.stop(display_id);
+    }
+
     fn open_url(&self, url: &str) {
         unsafe {
             let url = NSURL::alloc(nil)
@@ -675,6 +692,32 @@ impl Platform for MacPlatform {
         }
     }
 
+    // fn on_menu_command(&self, callback: Box<dyn FnMut(&dyn Action)>) {
+    //     self.0.lock().menu_command = Some(callback);
+    // }
+
+    // fn on_will_open_menu(&self, callback: Box<dyn FnMut()>) {
+    //     self.0.lock().will_open_menu = Some(callback);
+    // }
+
+    // fn on_validate_menu_command(&self, callback: Box<dyn FnMut(&dyn Action) -> bool>) {
+    //     self.0.lock().validate_menu_command = Some(callback);
+    // }
+
+    // fn set_menus(&self, menus: Vec<Menu>, keystroke_matcher: &KeymapMatcher) {
+    //     unsafe {
+    //         let app: id = msg_send![APP_CLASS, sharedApplication];
+    //         let mut state = self.0.lock();
+    //         let actions = &mut state.menu_actions;
+    //         app.setMainMenu_(self.create_menu_bar(
+    //             menus,
+    //             app.delegate(),
+    //             actions,
+    //             keystroke_matcher,
+    //         ));
+    //     }
+    // }
+
     fn set_cursor_style(&self, style: CursorStyle) {
         unsafe {
             let new_cursor: id = match style {
@@ -741,32 +784,6 @@ impl Platform for MacPlatform {
         }
     }
 
-    // fn on_menu_command(&self, callback: Box<dyn FnMut(&dyn Action)>) {
-    //     self.0.lock().menu_command = Some(callback);
-    // }
-
-    // fn on_will_open_menu(&self, callback: Box<dyn FnMut()>) {
-    //     self.0.lock().will_open_menu = Some(callback);
-    // }
-
-    // fn on_validate_menu_command(&self, callback: Box<dyn FnMut(&dyn Action) -> bool>) {
-    //     self.0.lock().validate_menu_command = Some(callback);
-    // }
-
-    // fn set_menus(&self, menus: Vec<Menu>, keystroke_matcher: &KeymapMatcher) {
-    //     unsafe {
-    //         let app: id = msg_send![APP_CLASS, sharedApplication];
-    //         let mut state = self.0.lock();
-    //         let actions = &mut state.menu_actions;
-    //         app.setMainMenu_(self.create_menu_bar(
-    //             menus,
-    //             app.delegate(),
-    //             actions,
-    //             keystroke_matcher,
-    //         ));
-    //     }
-    // }
-
     fn read_from_clipboard(&self) -> Option<ClipboardItem> {
         let state = self.0.lock();
         unsafe {

crates/gpui3/src/platform/test.rs 🔗

@@ -15,10 +15,6 @@ impl Platform for TestPlatform {
         unimplemented!()
     }
 
-    fn display_linker(&self) -> std::sync::Arc<dyn crate::PlatformDisplayLinker> {
-        unimplemented!()
-    }
-
     fn text_system(&self) -> std::sync::Arc<dyn crate::PlatformTextSystem> {
         unimplemented!()
     }
@@ -71,6 +67,22 @@ impl Platform for TestPlatform {
         unimplemented!()
     }
 
+    fn set_display_link_output_callback(
+        &self,
+        _display_id: DisplayId,
+        _callback: Box<dyn FnMut(&crate::VideoTimestamp, &crate::VideoTimestamp)>,
+    ) {
+        unimplemented!()
+    }
+
+    fn start_display_link(&self, _display_id: DisplayId) {
+        unimplemented!()
+    }
+
+    fn stop_display_link(&self, _display_id: DisplayId) {
+        unimplemented!()
+    }
+
     fn open_url(&self, _url: &str) {
         unimplemented!()
     }

crates/gpui3/src/window.rs 🔗

@@ -2,7 +2,7 @@ use crate::{
     px, size, AnyBox, AnyView, AppContext, AsyncWindowContext, AvailableSpace, BorrowAppContext,
     Bounds, BoxShadow, Context, Corners, DevicePixels, DisplayId, Edges, Effect, Element, EntityId,
     Event, EventEmitter, FontId, GlobalElementId, GlyphId, Handle, Hsla, ImageData, IsZero,
-    LayoutId, MainThread, MainThreadOnly, MonochromeSprite, MouseMoveEvent, Path, Pixels,
+    LayoutId, MainThread, MainThreadOnly, MonochromeSprite, MouseMoveEvent, Path, Pixels, Platform,
     PlatformAtlas, PlatformWindow, Point, PolychromeSprite, Quad, Reference, RenderGlyphParams,
     RenderImageParams, RenderSvgParams, ScaledPixels, SceneBuilder, Shadow, SharedString, Size,
     Style, Subscription, TaffyLayoutEngine, Task, Underline, UnderlineStyle, WeakHandle,
@@ -190,17 +190,17 @@ impl<'a, 'w> WindowContext<'a, 'w> {
     pub fn on_next_frame(&mut self, f: impl FnOnce(&mut WindowContext) + Send + 'static) {
         let f = Box::new(f);
         let display_id = self.window.display_id;
-        let async_cx = self.to_async();
-        let app_cx = self.app_mut();
-        match app_cx.next_frame_callbacks.entry(display_id) {
-            collections::hash_map::Entry::Occupied(mut entry) => {
-                if entry.get().is_empty() {
-                    app_cx.display_linker.start(display_id);
+        self.run_on_main(move |cx| {
+            if let Some(callbacks) = cx.next_frame_callbacks.get_mut(&display_id) {
+                callbacks.push(f);
+                // If there was already a callback, it means that we already scheduled a frame.
+                if callbacks.len() > 1 {
+                    return;
                 }
-                entry.get_mut().push(f);
-            }
-            collections::hash_map::Entry::Vacant(entry) => {
-                app_cx.display_linker.set_output_callback(
+            } else {
+                let async_cx = cx.to_async();
+                cx.next_frame_callbacks.insert(display_id, vec![f]);
+                cx.platform().set_display_link_output_callback(
                     display_id,
                     Box::new(move |_current_time, _output_time| {
                         let _ = async_cx.update(|cx| {
@@ -214,16 +214,20 @@ impl<'a, 'w> WindowContext<'a, 'w> {
                                 callback(cx);
                             }
 
-                            if cx.next_frame_callbacks.get(&display_id).unwrap().is_empty() {
-                                cx.display_linker.stop(display_id);
-                            }
+                            cx.run_on_main(move |cx| {
+                                if cx.next_frame_callbacks.get(&display_id).unwrap().is_empty() {
+                                    cx.platform().stop_display_link(display_id);
+                                }
+                            })
+                            .detach();
                         });
                     }),
                 );
-                app_cx.display_linker.start(display_id);
-                entry.insert(vec![f]);
             }
-        }
+
+            cx.platform().start_display_link(display_id);
+        })
+        .detach();
     }
 
     pub fn spawn<Fut, R>(
@@ -709,6 +713,12 @@ impl<'a, 'w> WindowContext<'a, 'w> {
     }
 }
 
+impl<'a, 'w> MainThread<WindowContext<'a, 'w>> {
+    fn platform(&self) -> &dyn Platform {
+        self.platform.borrow_on_main_thread()
+    }
+}
+
 impl Context for WindowContext<'_, '_> {
     type EntityContext<'a, 'w, T: 'static + Send + Sync> = ViewContext<'a, 'w, T>;
     type Result<T> = T;