Replace CADisplayLink with CVDisplayLink (#7583)

Release Notes:

- Fixed a bug that caused Zed to render at 60fps even on ProMotion
displays.
- Fixed a bug that could saturate the main thread event loop in certain
circumstances.

---------

Co-authored-by: Thorsten <thorsten@zed.dev>
Co-authored-by: Nathan <nathan@zed.dev>
Co-authored-by: Max <max@zed.dev>

crates/gpui/build.rs

@@ -27,6 +27,7 @@ fn generate_dispatch_bindings() {
     let bindings = bindgen::Builder::default()
         .header("src/platform/mac/dispatch.h")
         .allowlist_var("_dispatch_main_q")
+        .allowlist_var("_dispatch_source_type_data_add")
         .allowlist_var("DISPATCH_QUEUE_PRIORITY_DEFAULT")
         .allowlist_var("DISPATCH_QUEUE_PRIORITY_HIGH")
         .allowlist_var("DISPATCH_TIME_NOW")
@@ -34,6 +35,13 @@ fn generate_dispatch_bindings() {
         .allowlist_function("dispatch_async_f")
         .allowlist_function("dispatch_after_f")
         .allowlist_function("dispatch_time")
+        .allowlist_function("dispatch_source_merge_data")
+        .allowlist_function("dispatch_source_create")
+        .allowlist_function("dispatch_source_set_event_handler_f")
+        .allowlist_function("dispatch_resume")
+        .allowlist_function("dispatch_suspend")
+        .allowlist_function("dispatch_source_cancel")
+        .allowlist_function("dispatch_set_context")
         .parse_callbacks(Box::new(bindgen::CargoCallbacks))
         .layout_tests(false)
         .generate()

crates/gpui/src/app.rs

@@ -18,8 +18,8 @@ use crate::WindowAppearance;
 use crate::{
     current_platform, image_cache::ImageCache, init_app_menus, Action, ActionRegistry, Any,
     AnyView, AnyWindowHandle, AppMetadata, AssetSource, BackgroundExecutor, ClipboardItem, Context,
-    DispatchPhase, DisplayId, Entity, EventEmitter, ForegroundExecutor, Global, KeyBinding, Keymap,
-    Keystroke, LayoutId, Menu, PathPromptOptions, Pixels, Platform, PlatformDisplay, Point, Render,
+    DispatchPhase, Entity, EventEmitter, ForegroundExecutor, Global, KeyBinding, Keymap, Keystroke,
+    LayoutId, Menu, PathPromptOptions, Pixels, Platform, PlatformDisplay, Point, Render,
     SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextStyle, TextStyleRefinement,
     TextSystem, View, ViewContext, Window, WindowContext, WindowHandle, WindowId,
 };
@@ -193,7 +193,6 @@ impl App {
     }
 }
 
-pub(crate) type FrameCallback = Box<dyn FnOnce(&mut AppContext)>;
 type Handler = Box<dyn FnMut(&mut AppContext) -> bool + 'static>;
 type Listener = Box<dyn FnMut(&dyn Any, &mut AppContext) -> bool + 'static>;
 type KeystrokeObserver = Box<dyn FnMut(&KeystrokeEvent, &mut WindowContext) + 'static>;
@@ -213,8 +212,6 @@ pub struct AppContext {
     pending_updates: usize,
     pub(crate) actions: Rc<ActionRegistry>,
     pub(crate) active_drag: Option<AnyDrag>,
-    pub(crate) next_frame_callbacks: FxHashMap<DisplayId, Vec<FrameCallback>>,
-    pub(crate) frame_consumers: FxHashMap<DisplayId, Task<()>>,
     pub(crate) background_executor: BackgroundExecutor,
     pub(crate) foreground_executor: ForegroundExecutor,
     pub(crate) svg_renderer: SvgRenderer,
@@ -275,8 +272,6 @@ impl AppContext {
                 flushing_effects: false,
                 pending_updates: 0,
                 active_drag: None,
-                next_frame_callbacks: FxHashMap::default(),
-                frame_consumers: FxHashMap::default(),
                 background_executor: executor,
                 foreground_executor,
                 svg_renderer: SvgRenderer::new(asset_source.clone()),

crates/gpui/src/platform.rs

@@ -81,14 +81,6 @@ pub(crate) trait Platform: 'static {
     /// Returns the appearance of the application's windows.
     fn window_appearance(&self) -> WindowAppearance;
 
-    fn set_display_link_output_callback(
-        &self,
-        display_id: DisplayId,
-        callback: Box<dyn FnMut() + Send>,
-    );
-    fn start_display_link(&self, display_id: DisplayId);
-    fn stop_display_link(&self, display_id: DisplayId);
-
     fn open_url(&self, url: &str);
     fn on_open_urls(&self, callback: Box<dyn FnMut(Vec<String>)>);
     fn prompt_for_paths(

crates/gpui/src/platform/mac.rs

@@ -2,7 +2,7 @@
 //! an origin at the bottom left of the main display.
 mod dispatcher;
 mod display;
-mod display_linker;
+mod display_link;
 mod events;
 mod metal_atlas;
 mod metal_renderer;
@@ -23,7 +23,7 @@ use std::ops::Range;
 
 pub(crate) use dispatcher::*;
 pub(crate) use display::*;
-pub(crate) use display_linker::*;
+pub(crate) use display_link::*;
 pub(crate) use metal_atlas::*;
 pub(crate) use platform::*;
 pub(crate) use text_system::*;

crates/gpui/src/platform/mac/dispatch.h

@@ -1 +1,2 @@
 #include <dispatch/dispatch.h>
+#include <dispatch/source.h>

crates/gpui/src/platform/mac/display_link.rs

@@ -1,93 +1,96 @@
-use std::{
-    ffi::c_void,
-    mem,
-    sync::{Arc, Weak},
+use crate::{
+    dispatch_get_main_queue,
+    dispatch_sys::{
+        _dispatch_source_type_data_add, dispatch_resume, dispatch_set_context,
+        dispatch_source_cancel, dispatch_source_create, dispatch_source_merge_data,
+        dispatch_source_set_event_handler_f, dispatch_source_t, dispatch_suspend,
+    },
 };
+use anyhow::Result;
+use core_graphics::display::CGDirectDisplayID;
+use std::ffi::c_void;
+use util::ResultExt;
 
-use crate::DisplayId;
-use collections::HashMap;
-use parking_lot::Mutex;
-
-pub(crate) struct MacDisplayLinker {
-    links: HashMap<DisplayId, MacDisplayLink>,
+pub struct DisplayLink {
+    display_link: sys::DisplayLink,
+    frame_requests: dispatch_source_t,
 }
 
-struct MacDisplayLink {
-    system_link: sys::DisplayLink,
-    _output_callback: Arc<OutputCallback>,
-}
-
-impl MacDisplayLinker {
-    pub fn new() -> Self {
-        MacDisplayLinker {
-            links: Default::default(),
+impl DisplayLink {
+    pub fn new(
+        display_id: CGDirectDisplayID,
+        data: *mut c_void,
+        callback: unsafe extern "C" fn(*mut c_void),
+    ) -> Result<DisplayLink> {
+        unsafe extern "C" fn display_link_callback(
+            _display_link_out: *mut sys::CVDisplayLink,
+            _current_time: *const sys::CVTimeStamp,
+            _output_time: *const sys::CVTimeStamp,
+            _flags_in: i64,
+            _flags_out: *mut i64,
+            frame_requests: *mut c_void,
+        ) -> i32 {
+            let frame_requests = frame_requests as dispatch_source_t;
+            dispatch_source_merge_data(frame_requests, 1);
+            0
         }
-    }
-}
 
-type OutputCallback = Mutex<Box<dyn FnMut() + Send>>;
-
-impl MacDisplayLinker {
-    pub fn set_output_callback(
-        &mut self,
-        display_id: DisplayId,
-        output_callback: Box<dyn FnMut() + Send>,
-    ) {
-        if let Some(mut system_link) = unsafe { sys::DisplayLink::on_display(display_id.0) } {
-            let callback = Arc::new(Mutex::new(output_callback));
-            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.insert(
-                display_id,
-                MacDisplayLink {
-                    _output_callback: callback,
-                    system_link,
-                },
+        unsafe {
+            let frame_requests = dispatch_source_create(
+                &_dispatch_source_type_data_add,
+                0,
+                0,
+                dispatch_get_main_queue(),
             );
-        } else {
-            log::warn!("DisplayLink could not be obtained for {:?}", display_id);
+            dispatch_set_context(
+                crate::dispatch_sys::dispatch_object_t {
+                    _ds: frame_requests,
+                },
+                data,
+            );
+            dispatch_source_set_event_handler_f(frame_requests, Some(callback));
+
+            let display_link = sys::DisplayLink::new(
+                display_id,
+                display_link_callback,
+                frame_requests as *mut c_void,
+            )?;
+
+            Ok(Self {
+                display_link,
+                frame_requests,
+            })
         }
     }
 
-    pub fn start(&mut self, display_id: DisplayId) {
-        if let Some(link) = self.links.get_mut(&display_id) {
-            unsafe {
-                link.system_link.start();
-            }
-        } else {
-            log::warn!("No DisplayLink callback registered for {:?}", display_id)
+    pub fn start(&mut self) -> Result<()> {
+        unsafe {
+            dispatch_resume(crate::dispatch_sys::dispatch_object_t {
+                _ds: self.frame_requests,
+            });
+            self.display_link.start()?;
         }
+        Ok(())
     }
 
-    pub fn stop(&mut self, display_id: DisplayId) {
-        if let Some(link) = self.links.get_mut(&display_id) {
-            unsafe {
-                link.system_link.stop();
-            }
-        } else {
-            log::warn!("No DisplayLink callback registered for {:?}", display_id)
+    pub fn stop(&mut self) -> Result<()> {
+        unsafe {
+            dispatch_suspend(crate::dispatch_sys::dispatch_object_t {
+                _ds: self.frame_requests,
+            });
+            self.display_link.stop()?;
         }
+        Ok(())
     }
 }
 
-unsafe extern "C" fn trampoline(
-    _display_link_out: *mut sys::CVDisplayLink,
-    current_time: *const sys::CVTimeStamp,
-    output_time: *const sys::CVTimeStamp,
-    _flags_in: i64,
-    _flags_out: *mut i64,
-    user_data: *mut c_void,
-) -> i32 {
-    if let Some((_current_time, _output_time)) = current_time.as_ref().zip(output_time.as_ref()) {
-        let output_callback: Weak<OutputCallback> =
-            Weak::from_raw(user_data as *mut OutputCallback);
-        if let Some(output_callback) = output_callback.upgrade() {
-            (output_callback.lock())()
+impl Drop for DisplayLink {
+    fn drop(&mut self) {
+        self.stop().log_err();
+        unsafe {
+            dispatch_source_cancel(self.frame_requests);
         }
-        mem::forget(output_callback);
     }
-    0
 }
 
 mod sys {
@@ -96,10 +99,12 @@ mod sys {
     //! Apple docs: [CVDisplayLink](https://developer.apple.com/documentation/corevideo/cvdisplaylinkoutputcallback?language=objc)
     #![allow(dead_code, non_upper_case_globals)]
 
+    use anyhow::Result;
+    use core_graphics::display::CGDirectDisplayID;
     use foreign_types::{foreign_type, ForeignType};
     use std::{
         ffi::c_void,
-        fmt::{Debug, Formatter, Result},
+        fmt::{self, Debug, Formatter},
     };
 
     #[derive(Debug)]
@@ -114,7 +119,7 @@ mod sys {
     }
 
     impl Debug for DisplayLink {
-        fn fmt(&self, formatter: &mut Formatter) -> Result {
+        fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
             formatter
                 .debug_tuple("DisplayLink")
                 .field(&self.as_ptr())
@@ -201,19 +206,15 @@ mod sys {
         pub fn CVDisplayLinkCreateWithActiveCGDisplays(
             display_link_out: *mut *mut CVDisplayLink,
         ) -> i32;
-        pub fn CVDisplayLinkCreateWithCGDisplay(
+        pub fn CVDisplayLinkSetCurrentCGDisplay(
+            display_link: &mut DisplayLinkRef,
             display_id: u32,
-            display_link_out: *mut *mut CVDisplayLink,
         ) -> i32;
         pub fn CVDisplayLinkSetOutputCallback(
             display_link: &mut DisplayLinkRef,
             callback: CVDisplayLinkOutputCallback,
             user_info: *mut c_void,
         ) -> i32;
-        pub fn CVDisplayLinkSetCurrentCGDisplay(
-            display_link: &mut DisplayLinkRef,
-            display_id: u32,
-        ) -> i32;
         pub fn CVDisplayLinkStart(display_link: &mut DisplayLinkRef) -> i32;
         pub fn CVDisplayLinkStop(display_link: &mut DisplayLinkRef) -> i32;
         pub fn CVDisplayLinkRelease(display_link: *mut CVDisplayLink);
@@ -221,52 +222,46 @@ mod sys {
     }
 
     impl DisplayLink {
-        /// Apple docs: [CVDisplayLinkCreateWithActiveCGDisplays](https://developer.apple.com/documentation/corevideo/1456863-cvdisplaylinkcreatewithactivecgd?language=objc)
-        pub unsafe fn new() -> Option<Self> {
-            let mut display_link: *mut CVDisplayLink = 0 as _;
-            let code = CVDisplayLinkCreateWithActiveCGDisplays(&mut display_link);
-            if code == 0 {
-                Some(DisplayLink::from_ptr(display_link))
-            } else {
-                None
-            }
-        }
-
         /// Apple docs: [CVDisplayLinkCreateWithCGDisplay](https://developer.apple.com/documentation/corevideo/1456981-cvdisplaylinkcreatewithcgdisplay?language=objc)
-        pub unsafe fn on_display(display_id: u32) -> Option<Self> {
+        pub unsafe fn new(
+            display_id: CGDirectDisplayID,
+            callback: CVDisplayLinkOutputCallback,
+            user_info: *mut c_void,
+        ) -> Result<Self> {
             let mut display_link: *mut CVDisplayLink = 0 as _;
-            let code = CVDisplayLinkCreateWithCGDisplay(display_id, &mut display_link);
-            if code == 0 {
-                Some(DisplayLink::from_ptr(display_link))
-            } else {
-                None
-            }
+
+            let code = CVDisplayLinkCreateWithActiveCGDisplays(&mut display_link);
+            anyhow::ensure!(code == 0, "could not create display link, code: {}", code);
+
+            let mut display_link = DisplayLink::from_ptr(display_link);
+
+            let code = CVDisplayLinkSetOutputCallback(&mut display_link, callback, user_info);
+            anyhow::ensure!(code == 0, "could not set output callback, code: {}", code);
+
+            let code = CVDisplayLinkSetCurrentCGDisplay(&mut display_link, display_id);
+            anyhow::ensure!(
+                code == 0,
+                "could not assign display to display link, code: {}",
+                code
+            );
+
+            Ok(display_link)
         }
     }
 
     impl DisplayLinkRef {
-        /// Apple docs: [CVDisplayLinkSetOutputCallback](https://developer.apple.com/documentation/corevideo/1457096-cvdisplaylinksetoutputcallback?language=objc)
-        pub unsafe fn set_output_callback(
-            &mut self,
-            callback: CVDisplayLinkOutputCallback,
-            user_info: *mut c_void,
-        ) {
-            assert_eq!(CVDisplayLinkSetOutputCallback(self, callback, user_info), 0);
-        }
-
-        /// Apple docs: [CVDisplayLinkSetCurrentCGDisplay](https://developer.apple.com/documentation/corevideo/1456768-cvdisplaylinksetcurrentcgdisplay?language=objc)
-        pub unsafe fn set_current_display(&mut self, display_id: u32) {
-            assert_eq!(CVDisplayLinkSetCurrentCGDisplay(self, display_id), 0);
-        }
-
         /// Apple docs: [CVDisplayLinkStart](https://developer.apple.com/documentation/corevideo/1457193-cvdisplaylinkstart?language=objc)
-        pub unsafe fn start(&mut self) {
-            assert_eq!(CVDisplayLinkStart(self), 0);
+        pub unsafe fn start(&mut self) -> Result<()> {
+            let code = CVDisplayLinkStart(self);
+            anyhow::ensure!(code == 0, "could not start display link, code: {}", code);
+            Ok(())
         }
 
         /// Apple docs: [CVDisplayLinkStop](https://developer.apple.com/documentation/corevideo/1457281-cvdisplaylinkstop?language=objc)
-        pub unsafe fn stop(&mut self) {
-            assert_eq!(CVDisplayLinkStop(self), 0);
+        pub unsafe fn stop(&mut self) -> Result<()> {
+            let code = CVDisplayLinkStop(self);
+            anyhow::ensure!(code == 0, "could not stop display link, code: {}", code);
+            Ok(())
         }
     }
 }

crates/gpui/src/platform/mac/metal_renderer.rs

@@ -32,6 +32,7 @@ const INSTANCE_BUFFER_SIZE: usize = 2 * 1024 * 1024; // This is an arbitrary dec
 pub(crate) struct MetalRenderer {
     device: metal::Device,
     layer: metal::MetalLayer,
+    presents_with_transaction: bool,
     command_queue: CommandQueue,
     paths_rasterization_pipeline_state: metal::RenderPipelineState,
     path_sprites_pipeline_state: metal::RenderPipelineState,
@@ -60,8 +61,8 @@ impl MetalRenderer {
         let layer = metal::MetalLayer::new();
         layer.set_device(&device);
         layer.set_pixel_format(MTLPixelFormat::BGRA8Unorm);
-        layer.set_presents_with_transaction(true);
         layer.set_opaque(true);
+        layer.set_maximum_drawable_count(3);
         unsafe {
             let _: () = msg_send![&*layer, setAllowsNextDrawableTimeout: NO];
             let _: () = msg_send![&*layer, setNeedsDisplayOnBoundsChange: YES];
@@ -174,6 +175,7 @@ impl MetalRenderer {
         Self {
             device,
             layer,
+            presents_with_transaction: false,
             command_queue,
             paths_rasterization_pipeline_state,
             path_sprites_pipeline_state,
@@ -198,6 +200,12 @@ impl MetalRenderer {
         &self.sprite_atlas
     }
 
+    pub fn set_presents_with_transaction(&mut self, presents_with_transaction: bool) {
+        self.presents_with_transaction = presents_with_transaction;
+        self.layer
+            .set_presents_with_transaction(presents_with_transaction);
+    }
+
     pub fn draw(&mut self, scene: &Scene) {
         let layer = self.layer.clone();
         let viewport_size = layer.drawable_size();
@@ -347,11 +355,17 @@ impl MetalRenderer {
         });
         let block = block.copy();
         command_buffer.add_completed_handler(&block);
-        command_buffer.commit();
+
         self.sprite_atlas.clear_textures(AtlasTextureKind::Path);
 
-        command_buffer.wait_until_scheduled();
-        drawable.present();
+        if self.presents_with_transaction {
+            command_buffer.commit();
+            command_buffer.wait_until_scheduled();
+            drawable.present();
+        } else {
+            command_buffer.present_drawable(drawable);
+            command_buffer.commit();
+        }
     }
 
     fn rasterize_paths(

crates/gpui/src/platform/mac/platform.rs

@@ -1,10 +1,9 @@
 use super::{events::key_to_native, BoolExt};
 use crate::{
     Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId,
-    ForegroundExecutor, Keymap, MacDispatcher, MacDisplay, MacDisplayLinker, MacTextSystem,
-    MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformInput,
-    PlatformTextSystem, PlatformWindow, Result, SemanticVersion, Task, WindowAppearance,
-    WindowOptions,
+    ForegroundExecutor, Keymap, MacDispatcher, MacDisplay, MacTextSystem, MacWindow, Menu,
+    MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformInput, PlatformTextSystem,
+    PlatformWindow, Result, SemanticVersion, Task, WindowAppearance, WindowOptions,
 };
 use anyhow::anyhow;
 use block::ConcreteBlock;
@@ -146,7 +145,6 @@ pub(crate) struct MacPlatformState {
     background_executor: BackgroundExecutor,
     foreground_executor: ForegroundExecutor,
     text_system: Arc<MacTextSystem>,
-    display_linker: MacDisplayLinker,
     instance_buffer_pool: Arc<Mutex<Vec<metal::Buffer>>>,
     pasteboard: id,
     text_hash_pasteboard_type: id,
@@ -177,7 +175,6 @@ impl MacPlatform {
             background_executor: BackgroundExecutor::new(dispatcher.clone()),
             foreground_executor: ForegroundExecutor::new(dispatcher),
             text_system: Arc::new(MacTextSystem::new()),
-            display_linker: MacDisplayLinker::new(),
             instance_buffer_pool: Arc::default(),
             pasteboard: unsafe { NSPasteboard::generalPasteboard(nil) },
             text_hash_pasteboard_type: unsafe { ns_string("zed-text-hash") },
@@ -514,25 +511,6 @@ impl Platform for MacPlatform {
         }
     }
 
-    fn set_display_link_output_callback(
-        &self,
-        display_id: DisplayId,
-        callback: Box<dyn FnMut() + Send>,
-    ) {
-        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)

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

@@ -1,10 +1,11 @@
 use super::{global_bounds_from_ns_rect, ns_string, MacDisplay, MetalRenderer, NSRange};
 use crate::{
     global_bounds_to_ns_rect, platform::PlatformInputHandler, point, px, size, AnyWindowHandle,
-    Bounds, ExternalPaths, FileDropEvent, ForegroundExecutor, GlobalPixels, KeyDownEvent,
-    Keystroke, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMoveEvent,
-    MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformWindow, Point,
-    PromptLevel, Size, Timer, WindowAppearance, WindowBounds, WindowKind, WindowOptions,
+    Bounds, DisplayLink, ExternalPaths, FileDropEvent, ForegroundExecutor, GlobalPixels,
+    KeyDownEvent, Keystroke, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent,
+    MouseMoveEvent, MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput,
+    PlatformWindow, Point, PromptLevel, Size, Timer, WindowAppearance, WindowBounds, WindowKind,
+    WindowOptions,
 };
 use block::ConcreteBlock;
 use cocoa::{
@@ -16,11 +17,11 @@ use cocoa::{
     },
     base::{id, nil},
     foundation::{
-        NSArray, NSAutoreleasePool, NSDefaultRunLoopMode, NSDictionary, NSFastEnumeration,
-        NSInteger, NSPoint, NSRect, NSSize, NSString, NSUInteger,
+        NSArray, NSAutoreleasePool, NSDictionary, NSFastEnumeration, NSInteger, NSPoint, NSRect,
+        NSSize, NSString, NSUInteger,
     },
 };
-use core_graphics::display::CGRect;
+use core_graphics::display::{CGDirectDisplayID, CGRect};
 use ctor::ctor;
 use foreign_types::ForeignTypeRef;
 use futures::channel::oneshot;
@@ -50,6 +51,7 @@ use std::{
     sync::{Arc, Weak},
     time::Duration,
 };
+use util::ResultExt;
 
 const WINDOW_STATE_IVAR: &str = "windowState";
 
@@ -168,7 +170,6 @@ unsafe fn build_classes() {
             sel!(displayLayer:),
             display_layer as extern "C" fn(&Object, Sel, id),
         );
-        decl.add_method(sel!(step:), step as extern "C" fn(&Object, Sel, id));
 
         decl.add_protocol(Protocol::get("NSTextInputClient").unwrap());
         decl.add_method(
@@ -330,7 +331,7 @@ struct MacWindowState {
     executor: ForegroundExecutor,
     native_window: id,
     native_view: NonNull<Object>,
-    display_link: id,
+    display_link: Option<DisplayLink>,
     renderer: MetalRenderer,
     kind: WindowKind,
     request_frame_callback: Option<Box<dyn FnMut()>>,
@@ -402,6 +403,21 @@ impl MacWindowState {
         }
     }
 
+    fn start_display_link(&mut self) {
+        self.stop_display_link();
+        let display_id = unsafe { display_id_for_screen(self.native_window.screen()) };
+        if let Some(mut display_link) =
+            DisplayLink::new(display_id, self.native_view.as_ptr() as *mut c_void, step).log_err()
+        {
+            display_link.start().log_err();
+            self.display_link = Some(display_link);
+        }
+    }
+
+    fn stop_display_link(&mut self) {
+        self.display_link = None;
+    }
+
     fn is_fullscreen(&self) -> bool {
         unsafe {
             let style_mask = self.native_window.styleMask();
@@ -506,11 +522,8 @@ impl MacWindow {
             let count: u64 = cocoa::foundation::NSArray::count(screens);
             for i in 0..count {
                 let screen = cocoa::foundation::NSArray::objectAtIndex(screens, i);
-                let device_description = NSScreen::deviceDescription(screen);
-                let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber");
-                let screen_number = device_description.objectForKey_(screen_number_key);
-                let screen_number: NSUInteger = msg_send![screen_number, unsignedIntegerValue];
-                if screen_number as u32 == display.id().0 {
+                let display_id = display_id_for_screen(screen);
+                if display_id == display.id().0 {
                     target_screen = screen;
                     break;
                 }
@@ -539,7 +552,7 @@ impl MacWindow {
                 executor,
                 native_window,
                 native_view: NonNull::new_unchecked(native_view),
-                display_link: nil,
+                display_link: None,
                 renderer: MetalRenderer::new(instance_buffer_pool),
                 kind: options.kind,
                 request_frame_callback: None,
@@ -695,19 +708,11 @@ impl MacWindow {
     }
 }
 
-unsafe fn start_display_link(native_screen: id, native_view: id) -> id {
-    let display_link: id =
-        msg_send![native_screen, displayLinkWithTarget: native_view selector: sel!(step:)];
-    let main_run_loop: id = msg_send![class!(NSRunLoop), mainRunLoop];
-    let _: () = msg_send![display_link, addToRunLoop: main_run_loop forMode: NSDefaultRunLoopMode];
-    display_link
-}
-
 impl Drop for MacWindow {
     fn drop(&mut self) {
         let mut this = self.0.lock();
         let window = this.native_window;
-        this.display_link = nil;
+        this.display_link.take();
         this.executor
             .spawn(async move {
                 unsafe {
@@ -1336,17 +1341,16 @@ extern "C" fn cancel_operation(this: &Object, _sel: Sel, _sender: id) {
 
 extern "C" fn window_did_change_occlusion_state(this: &Object, _: Sel, _: id) {
     let window_state = unsafe { get_window_state(this) };
-    let mut lock = window_state.lock();
+    let lock = &mut *window_state.lock();
     unsafe {
         if lock
             .native_window
             .occlusionState()
             .contains(NSWindowOcclusionState::NSWindowOcclusionStateVisible)
         {
-            lock.display_link =
-                start_display_link(lock.native_window.screen(), lock.native_view.as_ptr());
+            lock.start_display_link();
         } else {
-            lock.display_link = nil;
+            lock.stop_display_link();
         }
     }
 }
@@ -1387,14 +1391,7 @@ extern "C" fn window_did_move(this: &Object, _: Sel, _: id) {
 extern "C" fn window_did_change_screen(this: &Object, _: Sel, _: id) {
     let window_state = unsafe { get_window_state(this) };
     let mut lock = window_state.as_ref().lock();
-    unsafe {
-        let screen = lock.native_window.screen();
-        if screen == nil {
-            lock.display_link = nil;
-        } else {
-            lock.display_link = start_display_link(screen, lock.native_view.as_ptr());
-        }
-    }
+    lock.start_display_link();
 }
 
 extern "C" fn window_did_change_key_status(this: &Object, selector: Sel, _: id) {
@@ -1540,26 +1537,27 @@ extern "C" fn display_layer(this: &Object, _: Sel, _: id) {
     let window_state = unsafe { get_window_state(this) };
     let mut lock = window_state.lock();
     if let Some(mut callback) = lock.request_frame_callback.take() {
+        lock.renderer.set_presents_with_transaction(true);
+        lock.stop_display_link();
         drop(lock);
         callback();
-        window_state.lock().request_frame_callback = Some(callback);
+
+        let mut lock = window_state.lock();
+        lock.request_frame_callback = Some(callback);
+        lock.renderer.set_presents_with_transaction(false);
+        lock.start_display_link();
     }
 }
 
-extern "C" fn step(this: &Object, _: Sel, display_link: id) {
-    let window_state = unsafe { get_window_state(this) };
+unsafe extern "C" fn step(view: *mut c_void) {
+    let view = view as id;
+    let window_state = unsafe { get_window_state(&*view) };
     let mut lock = window_state.lock();
 
-    if lock.display_link == display_link {
-        if let Some(mut callback) = lock.request_frame_callback.take() {
-            drop(lock);
-            callback();
-            window_state.lock().request_frame_callback = Some(callback);
-        }
-    } else {
-        unsafe {
-            let _: () = msg_send![display_link, invalidate];
-        }
+    if let Some(mut callback) = lock.request_frame_callback.take() {
+        drop(lock);
+        callback();
+        window_state.lock().request_frame_callback = Some(callback);
     }
 }
 
@@ -1882,3 +1880,11 @@ where
         None
     }
 }
+
+unsafe fn display_id_for_screen(screen: id) -> CGDirectDisplayID {
+    let device_description = NSScreen::deviceDescription(screen);
+    let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber");
+    let screen_number = device_description.objectForKey_(screen_number_key);
+    let screen_number: NSUInteger = msg_send![screen_number, unsignedIntegerValue];
+    screen_number as CGDirectDisplayID
+}

crates/gpui/src/platform/test/platform.rs

@@ -186,18 +186,6 @@ impl Platform for TestPlatform {
         WindowAppearance::Light
     }
 
-    fn set_display_link_output_callback(
-        &self,
-        _display_id: DisplayId,
-        mut callback: Box<dyn FnMut() + Send>,
-    ) {
-        callback()
-    }
-
-    fn start_display_link(&self, _display_id: DisplayId) {}
-
-    fn stop_display_link(&self, _display_id: DisplayId) {}
-
     fn open_url(&self, url: &str) {
         *self.opened_url.borrow_mut() = Some(url.to_string())
     }

crates/gpui/src/window.rs

@@ -12,10 +12,7 @@ use crate::{
 use anyhow::{anyhow, Context as _, Result};
 use collections::FxHashSet;
 use derive_more::{Deref, DerefMut};
-use futures::{
-    channel::{mpsc, oneshot},
-    StreamExt,
-};
+use futures::channel::oneshot;
 use parking_lot::RwLock;
 use slotmap::SlotMap;
 use smallvec::SmallVec;
@@ -23,7 +20,6 @@ use std::{
     any::{Any, TypeId},
     borrow::{Borrow, BorrowMut},
     cell::{Cell, RefCell},
-    collections::hash_map::Entry,
     fmt::{Debug, Display},
     future::Future,
     hash::{Hash, Hasher},
@@ -243,6 +239,8 @@ impl<M: FocusableView + EventEmitter<DismissEvent>> ManagedView for M {}
 /// Emitted by implementers of [`ManagedView`] to indicate the view should be dismissed, such as when a view is presented as a modal.
 pub struct DismissEvent;
 
+type FrameCallback = Box<dyn FnOnce(&mut WindowContext)>;
+
 // Holds the state for a specific window.
 #[doc(hidden)]
 pub struct Window {
@@ -259,6 +257,7 @@ pub struct Window {
     pub(crate) element_id_stack: GlobalElementId,
     pub(crate) rendered_frame: Frame,
     pub(crate) next_frame: Frame,
+    next_frame_callbacks: Rc<RefCell<Vec<FrameCallback>>>,
     pub(crate) dirty_views: FxHashSet<EntityId>,
     pub(crate) focus_handles: Arc<RwLock<SlotMap<FocusId, AtomicUsize>>>,
     focus_listeners: SubscriberSet<(), AnyWindowFocusListener>,
@@ -338,14 +337,27 @@ impl Window {
         let text_system = Arc::new(WindowTextSystem::new(cx.text_system().clone()));
         let dirty = Rc::new(Cell::new(true));
         let active = Rc::new(Cell::new(false));
+        let next_frame_callbacks: Rc<RefCell<Vec<FrameCallback>>> = Default::default();
         let last_input_timestamp = Rc::new(Cell::new(Instant::now()));
 
         platform_window.on_request_frame(Box::new({
             let mut cx = cx.to_async();
             let dirty = dirty.clone();
             let active = active.clone();
+            let next_frame_callbacks = next_frame_callbacks.clone();
             let last_input_timestamp = last_input_timestamp.clone();
             move || {
+                let next_frame_callbacks = next_frame_callbacks.take();
+                if !next_frame_callbacks.is_empty() {
+                    handle
+                        .update(&mut cx, |_, cx| {
+                            for callback in next_frame_callbacks {
+                                callback(cx);
+                            }
+                        })
+                        .log_err();
+                }
+
                 if dirty.get() {
                     measure("frame duration", || {
                         handle
@@ -428,6 +440,7 @@ impl Window {
             element_id_stack: GlobalElementId::default(),
             rendered_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())),
             next_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())),
+            next_frame_callbacks,
             dirty_views: FxHashSet::default(),
             focus_handles: Arc::new(RwLock::new(SlotMap::with_key())),
             focus_listeners: SubscriberSet::new(),
@@ -670,57 +683,7 @@ impl<'a> WindowContext<'a> {
 
     /// Schedule the given closure to be run directly after the current frame is rendered.
     pub fn on_next_frame(&mut self, callback: impl FnOnce(&mut WindowContext) + 'static) {
-        let handle = self.window.handle;
-        let display_id = self.window.display_id;
-
-        let mut frame_consumers = std::mem::take(&mut self.app.frame_consumers);
-        if let Entry::Vacant(e) = frame_consumers.entry(display_id) {
-            let (tx, mut rx) = mpsc::unbounded::<()>();
-            self.platform.set_display_link_output_callback(
-                display_id,
-                Box::new(move || _ = tx.unbounded_send(())),
-            );
-
-            let consumer_task = self.app.spawn(|cx| async move {
-                while rx.next().await.is_some() {
-                    cx.update(|cx| {
-                        for callback in cx
-                            .next_frame_callbacks
-                            .get_mut(&display_id)
-                            .unwrap()
-                            .drain(..)
-                            .collect::<SmallVec<[_; 32]>>()
-                        {
-                            callback(cx);
-                        }
-                    })
-                    .ok();
-
-                    // Flush effects, then stop the display link if no new next_frame_callbacks have been added.
-
-                    cx.update(|cx| {
-                        if cx.next_frame_callbacks.is_empty() {
-                            cx.platform.stop_display_link(display_id);
-                        }
-                    })
-                    .ok();
-                }
-            });
-            e.insert(consumer_task);
-        }
-        debug_assert!(self.app.frame_consumers.is_empty());
-        self.app.frame_consumers = frame_consumers;
-
-        if self.next_frame_callbacks.is_empty() {
-            self.platform.start_display_link(display_id);
-        }
-
-        self.next_frame_callbacks
-            .entry(display_id)
-            .or_default()
-            .push(Box::new(move |cx: &mut AppContext| {
-                cx.update_window(handle, |_root_view, cx| callback(cx)).ok();
-            }));
+        RefCell::borrow_mut(&self.window.next_frame_callbacks).push(Box::new(callback));
     }
 
     /// Spawn the future returned by the given closure on the application thread pool.