Revert "scrolling: use display scale factor on pixel scroll"

This reverts commit 32d490b16c.

crates/gpui/src/app.rs

@@ -9,6 +9,7 @@ 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::*;
@@ -18,13 +19,18 @@ 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,
-    SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextStyle, TextStyleRefinement,
-    TextSystem, View, ViewContext, Window, WindowContext, WindowHandle, WindowId,
+    Keystroke, LayoutId, Menu, PathPromptOptions, Pixels, Platform, PlatformDisplay,
+    PlatformDisplayLink, Point, Render, SharedString, SubscriberSet, Subscription, SvgRenderer,
+    Task, TextStyle, TextStyleRefinement, TextSystem, View, ViewContext, Window, WindowContext,
+    WindowHandle, WindowId,
 };
 use anyhow::{anyhow, Result};
-use collections::{FxHashMap, FxHashSet, VecDeque};
-use futures::{channel::oneshot, future::LocalBoxFuture, Future};
+use collections::{hash_map, FxHashMap, FxHashSet, VecDeque};
+use futures::{
+    channel::{mpsc, oneshot},
+    future::LocalBoxFuture,
+    Future, StreamExt,
+};
 
 use slotmap::SlotMap;
 use std::{
@@ -39,7 +45,7 @@ use std::{
 };
 use util::{
     http::{self, HttpClient},
-    ResultExt,
+    measure, ResultExt,
 };
 
 /// The duration for which futures returned from [AppContext::on_app_context] or [ModelContext::on_app_quit] can run before the application fully quits.
@@ -213,7 +219,8 @@ pub struct AppContext {
     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<()>>,
+    display_links: FxHashMap<DisplayId, Box<dyn PlatformDisplayLink>>,
+    display_updates: mpsc::UnboundedSender<DisplayId>,
     pub(crate) background_executor: BackgroundExecutor,
     pub(crate) foreground_executor: ForegroundExecutor,
     pub(crate) svg_renderer: SvgRenderer,
@@ -264,6 +271,7 @@ impl AppContext {
             app_version: platform.app_version().ok(),
         };
 
+        let (display_updates_tx, mut display_updates_rx) = mpsc::unbounded();
         let app = Rc::new_cyclic(|this| AppCell {
             app: RefCell::new(AppContext {
                 this: this.clone(),
@@ -275,9 +283,10 @@ impl AppContext {
                 pending_updates: 0,
                 active_drag: None,
                 next_frame_callbacks: FxHashMap::default(),
-                frame_consumers: FxHashMap::default(),
+                display_links: FxHashMap::default(),
+                display_updates: display_updates_tx,
                 background_executor: executor,
-                foreground_executor,
+                foreground_executor: foreground_executor.clone(),
                 svg_renderer: SvgRenderer::new(asset_source.clone()),
                 asset_source,
                 image_cache: ImageCache::new(http_client),
@@ -311,6 +320,23 @@ impl AppContext {
             }
         }));
 
+        foreground_executor
+            .spawn({
+                let cx = app.borrow().to_async();
+                async move {
+                    while let Some(display_id) = display_updates_rx.next().await {
+                        if cx
+                            .update(|cx| cx.refresh_display(display_id))
+                            .log_err()
+                            .is_none()
+                        {
+                            break;
+                        }
+                    }
+                }
+            })
+            .detach();
+
         app
     }
 
@@ -483,6 +509,8 @@ impl AppContext {
             let root_view = build_root_view(&mut WindowContext::new(cx, &mut window));
             window.root_view.replace(root_view.into());
             cx.windows.get_mut(id).unwrap().replace(window);
+            // Schedule a draw right after launching the window.
+            cx.refresh();
             handle
         })
     }
@@ -652,10 +680,29 @@ impl AppContext {
                     }
                 }
             } else {
+                let mut active_display_ids = FxHashSet::default();
                 for window in self.windows.values() {
                     if let Some(window) = window.as_ref() {
-                        if window.dirty {
-                            window.platform_window.invalidate();
+                        active_display_ids.insert(window.display_id);
+                    }
+                }
+
+                self.display_links
+                    .retain(|display_id, _| active_display_ids.contains(display_id));
+                for display_id in active_display_ids {
+                    if let hash_map::Entry::Vacant(entry) = self.display_links.entry(display_id) {
+                        let tx = self.display_updates.clone();
+                        if let Some(display_link) = self
+                            .platform
+                            .start_display_link(
+                                display_id,
+                                Box::new(move || {
+                                    tx.unbounded_send(display_id).log_err();
+                                }),
+                            )
+                            .log_err()
+                        {
+                            entry.insert(display_link);
                         }
                     }
                 }
@@ -765,6 +812,32 @@ impl AppContext {
         callback(self);
     }
 
+    fn refresh_display(&mut self, display_id: DisplayId) {
+        // TODO: run this as part of an effect cycle, so that if there are any side
+        // effects, they all get executed so that we can finally draw the windows at
+        // the end.
+        if let Some(callbacks) = self.next_frame_callbacks.remove(&display_id) {
+            for callback in callbacks {
+                callback(self);
+            }
+        }
+
+        let mut dirty_handles = SmallVec::<[AnyWindowHandle; 4]>::new();
+        for window in self.windows.values() {
+            if let Some(window) = window {
+                if window.dirty && window.display_id == display_id {
+                    dirty_handles.push(window.handle);
+                }
+            }
+        }
+
+        for dirty_handle in dirty_handles {
+            dirty_handle
+                .update(self, |_, cx| measure("frame duration", || cx.draw()))
+                .log_err();
+        }
+    }
+
     /// Creates an `AsyncAppContext`, which can be cloned and has a static lifetime
     /// so it can be held across `await` points.
     pub fn to_async(&self) -> AsyncAppContext {

crates/gpui/src/platform.rs

@@ -67,13 +67,11 @@ pub(crate) trait Platform: 'static {
         options: WindowOptions,
     ) -> Box<dyn PlatformWindow>;
 
-    fn set_display_link_output_callback(
+    fn start_display_link(
         &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);
+    ) -> Result<Box<dyn PlatformDisplayLink>>;
 
     fn open_url(&self, url: &str);
     fn on_open_urls(&self, callback: Box<dyn FnMut(Vec<String>)>);
@@ -186,6 +184,8 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle {
     }
 }
 
+pub(crate) trait PlatformDisplayLink {}
+
 /// This type is public so that our test macro can generate and use it, but it should not
 /// be considered part of our public API.
 #[doc(hidden)]

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

@@ -1,76 +1,50 @@
+use crate::{DisplayId, PlatformDisplayLink};
+use anyhow::{anyhow, Result};
+use parking_lot::Mutex;
 use std::{
     ffi::c_void,
     mem,
     sync::{Arc, Weak},
 };
 
-use crate::DisplayId;
-use collections::HashMap;
-use parking_lot::Mutex;
-
-pub(crate) struct MacDisplayLinker {
-    links: HashMap<DisplayId, MacDisplayLink>,
-}
-
-struct MacDisplayLink {
-    system_link: sys::DisplayLink,
-    _output_callback: Arc<OutputCallback>,
-}
-
-impl MacDisplayLinker {
-    pub fn new() -> Self {
-        MacDisplayLinker {
-            links: Default::default(),
-        }
-    }
-}
-
 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) }
+pub(crate) struct MacDisplayLink {
+    system_link: sys::DisplayLink,
+    _callback: Arc<OutputCallback>,
+}
 
-            self.links.insert(
-                display_id,
-                MacDisplayLink {
-                    _output_callback: callback,
-                    system_link,
-                },
-            );
-        } else {
-            log::warn!("DisplayLink could not be obtained for {:?}", display_id);
+impl MacDisplayLink {
+    pub fn new(display_id: DisplayId, callback: Box<dyn FnMut() + Send>) -> Result<Self> {
+        let mut system_link = unsafe {
+            sys::DisplayLink::on_display(display_id.0)
+                .ok_or_else(|| anyhow!("could not create DisplayLink"))
+        }?;
+
+        let callback = Arc::new(Mutex::new(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);
+            system_link.start();
         }
-    }
 
-    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)
-        }
+        Ok(Self {
+            system_link,
+            _callback: callback,
+        })
     }
+}
 
-    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)
+impl Drop for MacDisplayLink {
+    fn drop(&mut self) {
+        unsafe {
+            self.system_link.stop();
         }
     }
 }
 
+impl PlatformDisplayLink for MacDisplayLink {}
+
 unsafe extern "C" fn trampoline(
     _display_link_out: *mut sys::CVDisplayLink,
     current_time: *const sys::CVTimeStamp,

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

@@ -1,9 +1,10 @@
 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, WindowOptions,
+    ForegroundExecutor, Keymap, MacDispatcher, MacDisplay, MacDisplayLink, MacTextSystem,
+    MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformDisplayLink,
+    PlatformInput, PlatformTextSystem, PlatformWindow, Result, SemanticVersion, Task,
+    WindowOptions,
 };
 use anyhow::anyhow;
 use block::ConcreteBlock;
@@ -145,7 +146,6 @@ pub(crate) struct MacPlatformState {
     background_executor: BackgroundExecutor,
     foreground_executor: ForegroundExecutor,
     text_system: Arc<MacTextSystem>,
-    display_linker: MacDisplayLinker,
     pasteboard: id,
     text_hash_pasteboard_type: id,
     metadata_pasteboard_type: id,
@@ -175,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(),
             pasteboard: unsafe { NSPasteboard::generalPasteboard(nil) },
             text_hash_pasteboard_type: unsafe { ns_string("zed-text-hash") },
             metadata_pasteboard_type: unsafe { ns_string("zed-metadata") },
@@ -497,23 +496,12 @@ impl Platform for MacPlatform {
         Box::new(MacWindow::open(handle, options, self.foreground_executor()))
     }
 
-    fn set_display_link_output_callback(
+    fn start_display_link(
         &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);
+    ) -> Result<Box<dyn PlatformDisplayLink>> {
+        Ok(Box::new(MacDisplayLink::new(display_id, callback)?))
     }
 
     fn open_url(&self, url: &str) {

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

@@ -1,7 +1,7 @@
 use crate::{
     AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, ForegroundExecutor,
-    Keymap, Platform, PlatformDisplay, PlatformTextSystem, Task, TestDisplay, TestWindow,
-    WindowOptions,
+    Keymap, Platform, PlatformDisplay, PlatformDisplayLink, PlatformTextSystem, Task, TestDisplay,
+    TestWindow, WindowOptions,
 };
 use anyhow::{anyhow, Result};
 use collections::VecDeque;
@@ -176,18 +176,14 @@ impl Platform for TestPlatform {
         Box::new(window)
     }
 
-    fn set_display_link_output_callback(
+    fn start_display_link(
         &self,
         _display_id: DisplayId,
-        mut callback: Box<dyn FnMut() + Send>,
-    ) {
-        callback()
+        _callback: Box<dyn FnMut() + Send>,
+    ) -> Result<Box<dyn PlatformDisplayLink>> {
+        todo!()
     }
 
-    fn start_display_link(&self, _display_id: DisplayId) {}
-
-    fn stop_display_link(&self, _display_id: DisplayId) {}
-
     fn open_url(&self, _url: &str) {
         unimplemented!()
     }

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::RefCell,
-    collections::hash_map::Entry,
     fmt::{Debug, Display},
     future::Future,
     hash::{Hash, Hasher},
@@ -36,7 +32,7 @@ use std::{
     },
     time::Duration,
 };
-use util::{measure, ResultExt};
+use util::ResultExt;
 
 mod element_cx;
 pub use element_cx::*;
@@ -249,7 +245,7 @@ pub struct Window {
     pub(crate) handle: AnyWindowHandle,
     pub(crate) removed: bool,
     pub(crate) platform_window: Box<dyn PlatformWindow>,
-    display_id: DisplayId,
+    pub(crate) display_id: DisplayId,
     sprite_atlas: Arc<dyn PlatformAtlas>,
     pub(crate) rem_size: Pixels,
     pub(crate) viewport_size: Size<Pixels>,
@@ -338,19 +334,14 @@ impl Window {
         let scale_factor = platform_window.scale_factor();
         let bounds = platform_window.bounds();
 
-        platform_window.on_request_frame(Box::new({
-            let mut cx = cx.to_async();
-            move || {
-                measure("frame duration", || {
-                    handle.update(&mut cx, |_, cx| cx.draw()).log_err();
-                })
-            }
-        }));
         platform_window.on_resize(Box::new({
             let mut cx = cx.to_async();
             move |_, _| {
                 handle
-                    .update(&mut cx, |_, cx| cx.window_bounds_changed())
+                    .update(&mut cx, |_, cx| {
+                        cx.window_bounds_changed();
+                        cx.draw()
+                    })
                     .log_err();
             }
         }));
@@ -642,48 +633,6 @@ impl<'a> WindowContext<'a> {
         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()