From 66e4d75e6f1d623157c282293e0664358d124997 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 2 Nov 2023 12:34:40 -0400 Subject: [PATCH 1/5] Checkpoint: Reproduction with `Avatar` story --- crates/gpui2/src/app.rs | 46 ++++++++++++++++----- crates/gpui2/src/app/async_context.rs | 35 +++++++++------- crates/gpui2/src/app/test_context.rs | 20 ++++----- crates/gpui2/src/elements/img.rs | 6 ++- crates/gpui2/src/platform/mac/dispatcher.rs | 2 + crates/ui2/src/elements/avatar.rs | 20 +++++++-- crates/ui2/src/elements/player.rs | 4 +- 7 files changed, 91 insertions(+), 42 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 5a6e36080291e03600674349332295e2e62a4c7f..b20470fb1810d6dbdd084cad78ca1ea035e4f555 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -5,6 +5,7 @@ mod model_context; mod test_context; pub use async_context::*; +use derive_more::{Deref, DerefMut}; pub use entity_map::*; pub use model_context::*; use refineable::Refineable; @@ -27,7 +28,7 @@ use parking_lot::Mutex; use slotmap::SlotMap; use std::{ any::{type_name, Any, TypeId}, - cell::RefCell, + cell::{Ref, RefCell, RefMut}, marker::PhantomData, mem, ops::{Deref, DerefMut}, @@ -38,7 +39,29 @@ use std::{ }; use util::http::{self, HttpClient}; -pub struct App(Rc>); +pub struct AppCell { + app: RefCell, +} +impl AppCell { + pub fn borrow(&self) -> AppRef { + AppRef(self.app.borrow()) + } + + pub fn borrow_mut(&self, label: &str) -> AppRefMut { + let thread_id = std::thread::current().id(); + + eprintln!(">>> borrowing {thread_id:?}: {label}"); + AppRefMut(self.app.borrow_mut()) + } +} + +#[derive(Deref, DerefMut)] +pub struct AppRef<'a>(Ref<'a, AppContext>); + +#[derive(Deref, DerefMut)] +pub struct AppRefMut<'a>(RefMut<'a, AppContext>); + +pub struct App(Rc); /// Represents an application before it is fully launched. Once your app is /// configured, you'll start the app with `App::run`. @@ -61,7 +84,8 @@ impl App { let this = self.0.clone(); let platform = self.0.borrow().platform.clone(); platform.run(Box::new(move || { - let cx = &mut *this.borrow_mut(); + dbg!("run callback"); + let cx = &mut *this.borrow_mut("app::borrow_mut"); on_finish_launching(cx); })); } @@ -75,7 +99,7 @@ impl App { let this = Rc::downgrade(&self.0); self.0.borrow().platform.on_open_urls(Box::new(move |urls| { if let Some(app) = this.upgrade() { - callback(urls, &mut *app.borrow_mut()); + callback(urls, &mut *app.borrow_mut("app.rs::on_open_urls")); } })); self @@ -86,9 +110,9 @@ impl App { F: 'static + FnMut(&mut AppContext), { let this = Rc::downgrade(&self.0); - self.0.borrow_mut().platform.on_reopen(Box::new(move || { + self.0.borrow_mut("app.rs::on_reopen").platform.on_reopen(Box::new(move || { if let Some(app) = this.upgrade() { - callback(&mut app.borrow_mut()); + callback(&mut app.borrow_mut("app.rs::on_reopen(callback)")); } })); self @@ -119,7 +143,7 @@ type QuitHandler = Box LocalBoxFuture<'static, () type ReleaseListener = Box; pub struct AppContext { - this: Weak>, + this: Weak, pub(crate) platform: Rc, app_metadata: AppMetadata, text_system: Arc, @@ -157,7 +181,7 @@ impl AppContext { platform: Rc, asset_source: Arc, http_client: Arc, - ) -> Rc> { + ) -> Rc { let executor = platform.background_executor(); let foreground_executor = platform.foreground_executor(); assert!( @@ -174,8 +198,8 @@ impl AppContext { app_version: platform.app_version().ok(), }; - Rc::new_cyclic(|this| { - RefCell::new(AppContext { + Rc::new_cyclic(|this| AppCell { + app: RefCell::new(AppContext { this: this.clone(), text_system, platform, @@ -206,7 +230,7 @@ impl AppContext { layout_id_buffer: Default::default(), propagate_event: true, active_drag: None, - }) + }), }) } diff --git a/crates/gpui2/src/app/async_context.rs b/crates/gpui2/src/app/async_context.rs index 4bbab43446265a8a2c7c456b9d80702e2c027803..4fffdc693f37449e59126227fa7471aef9d6a6b6 100644 --- a/crates/gpui2/src/app/async_context.rs +++ b/crates/gpui2/src/app/async_context.rs @@ -1,15 +1,15 @@ use crate::{ - AnyView, AnyWindowHandle, AppContext, BackgroundExecutor, Context, ForegroundExecutor, Model, - ModelContext, Render, Result, Task, View, ViewContext, VisualContext, WindowContext, + AnyView, AnyWindowHandle, AppCell, AppContext, BackgroundExecutor, Context, ForegroundExecutor, + Model, ModelContext, Render, Result, Task, View, ViewContext, VisualContext, WindowContext, WindowHandle, }; use anyhow::{anyhow, Context as _}; use derive_more::{Deref, DerefMut}; -use std::{cell::RefCell, future::Future, rc::Weak}; +use std::{future::Future, rc::Weak}; #[derive(Clone)] pub struct AsyncAppContext { - pub(crate) app: Weak>, + pub(crate) app: Weak, pub(crate) background_executor: BackgroundExecutor, pub(crate) foreground_executor: ForegroundExecutor, } @@ -28,7 +28,8 @@ impl Context for AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut app = app.borrow_mut(); + dbg!("BUILD MODEL A"); + let mut app = app.borrow_mut("gpui2/async_context.rs::build_model"); Ok(app.build_model(build_model)) } @@ -41,7 +42,8 @@ impl Context for AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut app = app.borrow_mut(); + dbg!("UPDATE MODEL B"); + let mut app = app.borrow_mut("gpui2/async_context.rs::update_model"); Ok(app.update_model(handle, update)) } @@ -50,7 +52,8 @@ impl Context for AsyncAppContext { F: FnOnce(AnyView, &mut WindowContext<'_>) -> T, { let app = self.app.upgrade().context("app was released")?; - let mut lock = app.borrow_mut(); + dbg!("UPDATE WINDOW C"); + let mut lock = app.borrow_mut("gpui2/async_context::update_window"); lock.update_window(window, f) } } @@ -61,7 +64,8 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut lock = app.borrow_mut(); + dbg!("REFRESH"); + let mut lock = app.borrow_mut("async_context.rs::refresh"); lock.refresh(); Ok(()) } @@ -79,7 +83,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut lock = app.borrow_mut(); + let mut lock = app.borrow_mut("async_context.rs::update"); Ok(f(&mut *lock)) } @@ -95,7 +99,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut lock = app.borrow_mut(); + let mut lock = app.borrow_mut("open_window"); Ok(lock.open_window(options, build_root_view)) } @@ -112,7 +116,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let app = app.borrow_mut(); + let app = app.borrow_mut("has_global"); Ok(app.has_global::()) } @@ -121,7 +125,8 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let app = app.borrow_mut(); // Need this to compile + dbg!("read global"); + let app = app.borrow_mut("async_context.rs::read_global"); Ok(read(app.global(), &app)) } @@ -130,7 +135,8 @@ impl AsyncAppContext { read: impl FnOnce(&G, &AppContext) -> R, ) -> Option { let app = self.app.upgrade()?; - let app = app.borrow_mut(); + dbg!("try read global"); + let app = app.borrow_mut("async_context.rs::try_read_global"); Some(read(app.try_global()?, &app)) } @@ -142,7 +148,8 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut app = app.borrow_mut(); + dbg!("update global"); + let mut app = app.borrow_mut("async_context.rs::update_global"); Ok(app.update_global(update)) } } diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index aaf42dd4a27ba2b401516293f1f027aaeedbde65..f0ee988ebb3a939ab2e0e98b9db356ac969d7c18 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -1,15 +1,15 @@ use crate::{ AnyView, AnyWindowHandle, AppContext, AsyncAppContext, BackgroundExecutor, Context, EventEmitter, ForegroundExecutor, Model, ModelContext, Result, Task, TestDispatcher, - TestPlatform, WindowContext, + TestPlatform, WindowContext, AppCell, }; use anyhow::{anyhow, bail}; use futures::{Stream, StreamExt}; -use std::{cell::RefCell, future::Future, rc::Rc, sync::Arc, time::Duration}; +use std::{future::Future, rc::Rc, sync::Arc, time::Duration}; #[derive(Clone)] pub struct TestAppContext { - pub app: Rc>, + pub app: Rc, pub background_executor: BackgroundExecutor, pub foreground_executor: ForegroundExecutor, } @@ -24,7 +24,7 @@ impl Context for TestAppContext { where T: 'static, { - let mut app = self.app.borrow_mut(); + let mut app = self.app.borrow_mut("test_context.rs::build_model"); app.build_model(build_model) } @@ -33,7 +33,7 @@ impl Context for TestAppContext { handle: &Model, update: impl FnOnce(&mut T, &mut ModelContext<'_, T>) -> R, ) -> Self::Result { - let mut app = self.app.borrow_mut(); + let mut app = self.app.borrow_mut("test_context::update_model"); app.update_model(handle, update) } @@ -41,7 +41,7 @@ impl Context for TestAppContext { where F: FnOnce(AnyView, &mut WindowContext<'_>) -> T, { - let mut lock = self.app.borrow_mut(); + let mut lock = self.app.borrow_mut("test_context::update_window"); lock.update_window(window, f) } } @@ -65,11 +65,11 @@ impl TestAppContext { } pub fn quit(&self) { - self.app.borrow_mut().quit(); + self.app.borrow_mut("test_context.rs::quit").quit(); } pub fn refresh(&mut self) -> Result<()> { - let mut app = self.app.borrow_mut(); + let mut app = self.app.borrow_mut("test_context.rs::refresh"); app.refresh(); Ok(()) } @@ -83,7 +83,7 @@ impl TestAppContext { } pub fn update(&self, f: impl FnOnce(&mut AppContext) -> R) -> R { - let mut cx = self.app.borrow_mut(); + let mut cx = self.app.borrow_mut("test_context::update"); cx.update(f) } @@ -117,7 +117,7 @@ impl TestAppContext { &mut self, update: impl FnOnce(&mut G, &mut AppContext) -> R, ) -> R { - let mut lock = self.app.borrow_mut(); + let mut lock = self.app.borrow_mut("test_context.rs::update_global"); lock.update_global(update) } diff --git a/crates/gpui2/src/elements/img.rs b/crates/gpui2/src/elements/img.rs index 747e573ea566ec882aa965c3efa0cc01cf6fbf70..a9950bfc0a9875459976bda2898710d6a70a4cc4 100644 --- a/crates/gpui2/src/elements/img.rs +++ b/crates/gpui2/src/elements/img.rs @@ -109,7 +109,9 @@ where let corner_radii = style.corner_radii; if let Some(uri) = self.uri.clone() { - let image_future = cx.image_cache.get(uri); + // eprintln!(">>> image_cache.get({uri}"); + let image_future = cx.image_cache.get(uri.clone()); + // eprintln!("<<< image_cache.get({uri}"); if let Some(data) = image_future .clone() .now_or_never() @@ -123,7 +125,9 @@ where } else { cx.spawn(|_, mut cx| async move { if image_future.await.log_err().is_some() { + eprintln!(">>> on_next_frame"); cx.on_next_frame(|cx| cx.notify()); + eprintln!("<<< on_next_frame") } }) .detach() diff --git a/crates/gpui2/src/platform/mac/dispatcher.rs b/crates/gpui2/src/platform/mac/dispatcher.rs index f5334912c6b7aec93fed2af3c33832ff241313c9..a39688bafd346504a0a97b4b5f8164d9c2bc1520 100644 --- a/crates/gpui2/src/platform/mac/dispatcher.rs +++ b/crates/gpui2/src/platform/mac/dispatcher.rs @@ -42,6 +42,7 @@ impl PlatformDispatcher for MacDispatcher { } fn dispatch(&self, runnable: Runnable) { + println!("DISPATCH"); unsafe { dispatch_async_f( dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT.try_into().unwrap(), 0), @@ -52,6 +53,7 @@ impl PlatformDispatcher for MacDispatcher { } fn dispatch_on_main_thread(&self, runnable: Runnable) { + println!("DISPATCH ON MAIN THREAD"); unsafe { dispatch_async_f( dispatch_get_main_queue(), diff --git a/crates/ui2/src/elements/avatar.rs b/crates/ui2/src/elements/avatar.rs index ff92021b1163793835f3129657f67caa67054ea7..ca773397cacc8872e8567684819cf796980e1ef3 100644 --- a/crates/ui2/src/elements/avatar.rs +++ b/crates/ui2/src/elements/avatar.rs @@ -58,11 +58,23 @@ mod stories { .child(Avatar::new( "https://avatars.githubusercontent.com/u/1714999?v=4", )) + .child(Avatar::new( + "https://avatars.githubusercontent.com/u/326587?v=4", + )) + // .child(Avatar::new( + // "https://avatars.githubusercontent.com/u/482957?v=4", + // )) + // .child(Avatar::new( + // "https://avatars.githubusercontent.com/u/1714999?v=4", + // )) + // .child(Avatar::new( + // "https://avatars.githubusercontent.com/u/1486634?v=4", + // )) .child(Story::label(cx, "Rounded rectangle")) - .child( - Avatar::new("https://avatars.githubusercontent.com/u/1714999?v=4") - .shape(Shape::RoundedRectangle), - ) + // .child( + // Avatar::new("https://avatars.githubusercontent.com/u/1714999?v=4") + // .shape(Shape::RoundedRectangle), + // ) } } } diff --git a/crates/ui2/src/elements/player.rs b/crates/ui2/src/elements/player.rs index 8e3ad5c3a83f90273b9677937e9beb37db283649..c7b7ade1c13c62878e8ae72ce828bde30ab9d375 100644 --- a/crates/ui2/src/elements/player.rs +++ b/crates/ui2/src/elements/player.rs @@ -139,11 +139,11 @@ impl Player { } pub fn cursor_color(&self, cx: &mut ViewContext) -> Hsla { - cx.theme().styles.player.0[self.index].cursor + cx.theme().styles.player.0[self.index % cx.theme().styles.player.0.len()].cursor } pub fn selection_color(&self, cx: &mut ViewContext) -> Hsla { - cx.theme().styles.player.0[self.index].selection + cx.theme().styles.player.0[self.index % cx.theme().styles.player.0.len()].selection } pub fn avatar_src(&self) -> &str { From 0e1d2fdf21627e0f9eb18ed55d2be7913f0e5eb0 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 2 Nov 2023 12:47:06 -0400 Subject: [PATCH 2/5] Checkpoint: Narrow down error --- crates/gpui2/src/app.rs | 21 +++++++++++-------- crates/gpui2/src/app/async_context.rs | 20 +++++++++--------- crates/gpui2/src/app/test_context.rs | 14 ++++++------- crates/gpui2/src/platform.rs | 2 +- .../gpui2/src/platform/mac/display_linker.rs | 4 ++-- crates/gpui2/src/platform/mac/platform.rs | 2 +- crates/gpui2/src/platform/test/platform.rs | 2 +- crates/ui2/src/elements/avatar.rs | 3 +++ 8 files changed, 37 insertions(+), 31 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index b20470fb1810d6dbdd084cad78ca1ea035e4f555..3bc0c14f9a52cb1006ac7af803e8f4e9990ca525 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -47,10 +47,10 @@ impl AppCell { AppRef(self.app.borrow()) } - pub fn borrow_mut(&self, label: &str) -> AppRefMut { + pub fn borrow_mut(&self) -> AppRefMut { let thread_id = std::thread::current().id(); - eprintln!(">>> borrowing {thread_id:?}: {label}"); + eprintln!(">>> borrowing {thread_id:?}"); AppRefMut(self.app.borrow_mut()) } } @@ -85,7 +85,7 @@ impl App { let platform = self.0.borrow().platform.clone(); platform.run(Box::new(move || { dbg!("run callback"); - let cx = &mut *this.borrow_mut("app::borrow_mut"); + let cx = &mut *this.borrow_mut(); on_finish_launching(cx); })); } @@ -99,7 +99,7 @@ impl App { let this = Rc::downgrade(&self.0); self.0.borrow().platform.on_open_urls(Box::new(move |urls| { if let Some(app) = this.upgrade() { - callback(urls, &mut *app.borrow_mut("app.rs::on_open_urls")); + callback(urls, &mut *app.borrow_mut()); } })); self @@ -110,11 +110,14 @@ impl App { F: 'static + FnMut(&mut AppContext), { let this = Rc::downgrade(&self.0); - self.0.borrow_mut("app.rs::on_reopen").platform.on_reopen(Box::new(move || { - if let Some(app) = this.upgrade() { - callback(&mut app.borrow_mut("app.rs::on_reopen(callback)")); - } - })); + self.0 + .borrow_mut() + .platform + .on_reopen(Box::new(move || { + if let Some(app) = this.upgrade() { + callback(&mut app.borrow_mut()); + } + })); self } diff --git a/crates/gpui2/src/app/async_context.rs b/crates/gpui2/src/app/async_context.rs index 4fffdc693f37449e59126227fa7471aef9d6a6b6..f45457936c887def2750d20829a1678b8683842a 100644 --- a/crates/gpui2/src/app/async_context.rs +++ b/crates/gpui2/src/app/async_context.rs @@ -29,7 +29,7 @@ impl Context for AsyncAppContext { .upgrade() .ok_or_else(|| anyhow!("app was released"))?; dbg!("BUILD MODEL A"); - let mut app = app.borrow_mut("gpui2/async_context.rs::build_model"); + let mut app = app.borrow_mut(); Ok(app.build_model(build_model)) } @@ -43,7 +43,7 @@ impl Context for AsyncAppContext { .upgrade() .ok_or_else(|| anyhow!("app was released"))?; dbg!("UPDATE MODEL B"); - let mut app = app.borrow_mut("gpui2/async_context.rs::update_model"); + let mut app = app.borrow_mut(); Ok(app.update_model(handle, update)) } @@ -53,7 +53,7 @@ impl Context for AsyncAppContext { { let app = self.app.upgrade().context("app was released")?; dbg!("UPDATE WINDOW C"); - let mut lock = app.borrow_mut("gpui2/async_context::update_window"); + let mut lock = app.borrow_mut(); lock.update_window(window, f) } } @@ -65,7 +65,7 @@ impl AsyncAppContext { .upgrade() .ok_or_else(|| anyhow!("app was released"))?; dbg!("REFRESH"); - let mut lock = app.borrow_mut("async_context.rs::refresh"); + let mut lock = app.borrow_mut(); lock.refresh(); Ok(()) } @@ -83,7 +83,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut lock = app.borrow_mut("async_context.rs::update"); + let mut lock = app.borrow_mut(); Ok(f(&mut *lock)) } @@ -99,7 +99,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let mut lock = app.borrow_mut("open_window"); + let mut lock = app.borrow_mut(); Ok(lock.open_window(options, build_root_view)) } @@ -116,7 +116,7 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - let app = app.borrow_mut("has_global"); + let app = app.borrow_mut(); Ok(app.has_global::()) } @@ -126,7 +126,7 @@ impl AsyncAppContext { .upgrade() .ok_or_else(|| anyhow!("app was released"))?; dbg!("read global"); - let app = app.borrow_mut("async_context.rs::read_global"); + let app = app.borrow_mut(); Ok(read(app.global(), &app)) } @@ -136,7 +136,7 @@ impl AsyncAppContext { ) -> Option { let app = self.app.upgrade()?; dbg!("try read global"); - let app = app.borrow_mut("async_context.rs::try_read_global"); + let app = app.borrow_mut(); Some(read(app.try_global()?, &app)) } @@ -149,7 +149,7 @@ impl AsyncAppContext { .upgrade() .ok_or_else(|| anyhow!("app was released"))?; dbg!("update global"); - let mut app = app.borrow_mut("async_context.rs::update_global"); + let mut app = app.borrow_mut(); Ok(app.update_global(update)) } } diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index f0ee988ebb3a939ab2e0e98b9db356ac969d7c18..7ef53d3e128c888ebf604636339affe0d43dea41 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -24,7 +24,7 @@ impl Context for TestAppContext { where T: 'static, { - let mut app = self.app.borrow_mut("test_context.rs::build_model"); + let mut app = self.app.borrow_mut(); app.build_model(build_model) } @@ -33,7 +33,7 @@ impl Context for TestAppContext { handle: &Model, update: impl FnOnce(&mut T, &mut ModelContext<'_, T>) -> R, ) -> Self::Result { - let mut app = self.app.borrow_mut("test_context::update_model"); + let mut app = self.app.borrow_mut(); app.update_model(handle, update) } @@ -41,7 +41,7 @@ impl Context for TestAppContext { where F: FnOnce(AnyView, &mut WindowContext<'_>) -> T, { - let mut lock = self.app.borrow_mut("test_context::update_window"); + let mut lock = self.app.borrow_mut(); lock.update_window(window, f) } } @@ -65,11 +65,11 @@ impl TestAppContext { } pub fn quit(&self) { - self.app.borrow_mut("test_context.rs::quit").quit(); + self.app.borrow_mut().quit(); } pub fn refresh(&mut self) -> Result<()> { - let mut app = self.app.borrow_mut("test_context.rs::refresh"); + let mut app = self.app.borrow_mut(); app.refresh(); Ok(()) } @@ -83,7 +83,7 @@ impl TestAppContext { } pub fn update(&self, f: impl FnOnce(&mut AppContext) -> R) -> R { - let mut cx = self.app.borrow_mut("test_context::update"); + let mut cx = self.app.borrow_mut(); cx.update(f) } @@ -117,7 +117,7 @@ impl TestAppContext { &mut self, update: impl FnOnce(&mut G, &mut AppContext) -> R, ) -> R { - let mut lock = self.app.borrow_mut("test_context.rs::update_global"); + let mut lock = self.app.borrow_mut(); lock.update_global(update) } diff --git a/crates/gpui2/src/platform.rs b/crates/gpui2/src/platform.rs index cdce67d8c128b9bb04044ec6bf7a2252ae1c3bff..a4be21ddf36ff93c3c3b3fd9434cd3168e803bb6 100644 --- a/crates/gpui2/src/platform.rs +++ b/crates/gpui2/src/platform.rs @@ -69,7 +69,7 @@ pub(crate) trait Platform: 'static { fn set_display_link_output_callback( &self, display_id: DisplayId, - callback: Box, + callback: Box, ); fn start_display_link(&self, display_id: DisplayId); fn stop_display_link(&self, display_id: DisplayId); diff --git a/crates/gpui2/src/platform/mac/display_linker.rs b/crates/gpui2/src/platform/mac/display_linker.rs index 6d8235a3812e1520a516202f4a212cf451a463cc..b63cf24e2689d1d249016d86b82a62ffd2d3946b 100644 --- a/crates/gpui2/src/platform/mac/display_linker.rs +++ b/crates/gpui2/src/platform/mac/display_linker.rs @@ -26,13 +26,13 @@ impl MacDisplayLinker { } } -type OutputCallback = Mutex>; +type OutputCallback = Mutex>; impl MacDisplayLinker { pub fn set_output_callback( &mut self, display_id: DisplayId, - output_callback: Box, + output_callback: Box, ) { if let Some(mut system_link) = unsafe { sys::DisplayLink::on_display(display_id.0) } { let callback = Arc::new(Mutex::new(output_callback)); diff --git a/crates/gpui2/src/platform/mac/platform.rs b/crates/gpui2/src/platform/mac/platform.rs index fdc7fd6ae5d416ab284a08249e4176c5f3c10892..7065c02e8755cf226b532a694d34a0b0a436c95b 100644 --- a/crates/gpui2/src/platform/mac/platform.rs +++ b/crates/gpui2/src/platform/mac/platform.rs @@ -494,7 +494,7 @@ impl Platform for MacPlatform { fn set_display_link_output_callback( &self, display_id: DisplayId, - callback: Box, + callback: Box, ) { self.0 .lock() diff --git a/crates/gpui2/src/platform/test/platform.rs b/crates/gpui2/src/platform/test/platform.rs index 524611b620ea62eeb5e48a3ce5430a992791ae7c..b4f3c739e60921a880827422023f8ff7d3c9e3df 100644 --- a/crates/gpui2/src/platform/test/platform.rs +++ b/crates/gpui2/src/platform/test/platform.rs @@ -81,7 +81,7 @@ impl Platform for TestPlatform { fn set_display_link_output_callback( &self, _display_id: DisplayId, - _callback: Box, + _callback: Box, ) { unimplemented!() } diff --git a/crates/ui2/src/elements/avatar.rs b/crates/ui2/src/elements/avatar.rs index ca773397cacc8872e8567684819cf796980e1ef3..357e573f7ca2bd618900215547faf0693f83f4d9 100644 --- a/crates/ui2/src/elements/avatar.rs +++ b/crates/ui2/src/elements/avatar.rs @@ -62,6 +62,9 @@ mod stories { "https://avatars.githubusercontent.com/u/326587?v=4", )) // .child(Avatar::new( + // "https://avatars.githubusercontent.com/u/326587?v=4", + // )) + // .child(Avatar::new( // "https://avatars.githubusercontent.com/u/482957?v=4", // )) // .child(Avatar::new( From 1e7a216d554c4bf1cd4d03411bb20af199ff2953 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 2 Nov 2023 13:21:28 -0400 Subject: [PATCH 3/5] WIP --- crates/gpui2/src/app.rs | 16 ++++++++++++---- crates/gpui2/src/window.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 3bc0c14f9a52cb1006ac7af803e8f4e9990ca525..8de3734c4c4ec6aecd21e0c70d5cd1f16f75c3c4 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -139,12 +139,18 @@ impl App { } type ActionBuilder = fn(json: Option) -> anyhow::Result>; -type FrameCallback = Box; +type FrameCallback = Box; type Handler = Box bool + 'static>; type Listener = Box bool + 'static>; type QuitHandler = Box LocalBoxFuture<'static, ()> + 'static>; type ReleaseListener = Box; +// struct FrameConsumer { +// next_frame_callbacks: Vec, +// task: Task<()>, +// display_linker +// } + pub struct AppContext { this: Weak, pub(crate) platform: Rc, @@ -154,6 +160,7 @@ pub struct AppContext { pending_updates: usize, pub(crate) active_drag: Option, pub(crate) next_frame_callbacks: HashMap>, + pub(crate) frame_consumers: HashMap>, pub(crate) background_executor: BackgroundExecutor, pub(crate) foreground_executor: ForegroundExecutor, pub(crate) svg_renderer: SvgRenderer, @@ -204,12 +211,14 @@ impl AppContext { Rc::new_cyclic(|this| AppCell { app: RefCell::new(AppContext { this: this.clone(), - text_system, platform, app_metadata, + text_system, flushing_effects: false, pending_updates: 0, - next_frame_callbacks: Default::default(), + active_drag: None, + next_frame_callbacks: HashMap::default(), + frame_consumers: HashMap::default(), background_executor: executor, foreground_executor, svg_renderer: SvgRenderer::new(asset_source.clone()), @@ -232,7 +241,6 @@ impl AppContext { quit_observers: SubscriberSet::new(), layout_id_buffer: Default::default(), propagate_event: true, - active_drag: None, }), }) } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 0202b7521eebc36735077e3199f0322e25a80e91..6034727d8245f64f1ae0c6ab9688672e14b6bf88 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -12,7 +12,10 @@ use crate::{ use anyhow::{anyhow, Result}; use collections::HashMap; use derive_more::{Deref, DerefMut}; -use futures::channel::oneshot; +use futures::{ + channel::{mpsc, oneshot}, + StreamExt, +}; use parking_lot::RwLock; use slotmap::SlotMap; use smallvec::SmallVec; @@ -411,6 +414,31 @@ impl<'a> WindowContext<'a> { let f = Box::new(f); let display_id = self.window.display_id; + self.next_frame_callbacks + .entry(display_id) + .or_default() + .push(f); + + self.frame_consumers.entry(display_id).or_insert_with(|| { + let (tx, rx) = mpsc::unbounded::<()>(); + + self.spawn(|cx| async move { + while rx.next().await.is_some() { + let _ = cx.update(|_, cx| { + for callback in cx + .app + .next_frame_callbacks + .get_mut(&display_id) + .unwrap() + .drain(..) + { + callback(cx); + } + }); + } + }) + }); + if let Some(callbacks) = self.next_frame_callbacks.get_mut(&display_id) { callbacks.push(f); // If there was already a callback, it means that we already scheduled a frame. From 04a8ee222b9450903f3d3e8cf43b0d04757c2031 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 2 Nov 2023 12:01:22 -0600 Subject: [PATCH 4/5] Enforce a Send bound on next frame callbacks This required using mpsc channels to invoke frame callbacks on the main thread and send the receiver to the platform display link. Co-Authored-By: Julia Risley --- crates/gpui2/src/app.rs | 24 +++---- crates/gpui2/src/app/async_context.rs | 7 -- crates/gpui2/src/app/test_context.rs | 4 +- crates/gpui2/src/elements/img.rs | 2 - crates/gpui2/src/platform/mac/dispatcher.rs | 2 - crates/gpui2/src/window.rs | 74 +++++++++------------ 6 files changed, 44 insertions(+), 69 deletions(-) diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index 8de3734c4c4ec6aecd21e0c70d5cd1f16f75c3c4..38e64dcf1c6483a26e5a65c2cbfd1f4626efca44 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -39,18 +39,20 @@ use std::{ }; use util::http::{self, HttpClient}; +/// Temporary(?) wrapper around RefCell to help us debug any double borrows. +/// Strongly consider removing after stabilization. pub struct AppCell { app: RefCell, } + impl AppCell { pub fn borrow(&self) -> AppRef { AppRef(self.app.borrow()) } pub fn borrow_mut(&self) -> AppRefMut { - let thread_id = std::thread::current().id(); - - eprintln!(">>> borrowing {thread_id:?}"); + // let thread_id = std::thread::current().id(); + // dbg!("borrowed {thread_id:?}"); AppRefMut(self.app.borrow_mut()) } } @@ -84,7 +86,6 @@ impl App { let this = self.0.clone(); let platform = self.0.borrow().platform.clone(); platform.run(Box::new(move || { - dbg!("run callback"); let cx = &mut *this.borrow_mut(); on_finish_launching(cx); })); @@ -110,14 +111,11 @@ impl App { F: 'static + FnMut(&mut AppContext), { let this = Rc::downgrade(&self.0); - self.0 - .borrow_mut() - .platform - .on_reopen(Box::new(move || { - if let Some(app) = this.upgrade() { - callback(&mut app.borrow_mut()); - } - })); + self.0.borrow_mut().platform.on_reopen(Box::new(move || { + if let Some(app) = this.upgrade() { + callback(&mut app.borrow_mut()); + } + })); self } @@ -139,7 +137,7 @@ impl App { } type ActionBuilder = fn(json: Option) -> anyhow::Result>; -type FrameCallback = Box; +pub(crate) type FrameCallback = Box; type Handler = Box bool + 'static>; type Listener = Box bool + 'static>; type QuitHandler = Box LocalBoxFuture<'static, ()> + 'static>; diff --git a/crates/gpui2/src/app/async_context.rs b/crates/gpui2/src/app/async_context.rs index f45457936c887def2750d20829a1678b8683842a..e3ae78d78f770277d3c9f59e8571edc376cd8b4b 100644 --- a/crates/gpui2/src/app/async_context.rs +++ b/crates/gpui2/src/app/async_context.rs @@ -28,7 +28,6 @@ impl Context for AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - dbg!("BUILD MODEL A"); let mut app = app.borrow_mut(); Ok(app.build_model(build_model)) } @@ -42,7 +41,6 @@ impl Context for AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - dbg!("UPDATE MODEL B"); let mut app = app.borrow_mut(); Ok(app.update_model(handle, update)) } @@ -52,7 +50,6 @@ impl Context for AsyncAppContext { F: FnOnce(AnyView, &mut WindowContext<'_>) -> T, { let app = self.app.upgrade().context("app was released")?; - dbg!("UPDATE WINDOW C"); let mut lock = app.borrow_mut(); lock.update_window(window, f) } @@ -64,7 +61,6 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - dbg!("REFRESH"); let mut lock = app.borrow_mut(); lock.refresh(); Ok(()) @@ -125,7 +121,6 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - dbg!("read global"); let app = app.borrow_mut(); Ok(read(app.global(), &app)) } @@ -135,7 +130,6 @@ impl AsyncAppContext { read: impl FnOnce(&G, &AppContext) -> R, ) -> Option { let app = self.app.upgrade()?; - dbg!("try read global"); let app = app.borrow_mut(); Some(read(app.try_global()?, &app)) } @@ -148,7 +142,6 @@ impl AsyncAppContext { .app .upgrade() .ok_or_else(|| anyhow!("app was released"))?; - dbg!("update global"); let mut app = app.borrow_mut(); Ok(app.update_global(update)) } diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index 7ef53d3e128c888ebf604636339affe0d43dea41..e731dccc6eacd30444750950251bff7ec09dc02b 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -1,7 +1,7 @@ use crate::{ - AnyView, AnyWindowHandle, AppContext, AsyncAppContext, BackgroundExecutor, Context, + AnyView, AnyWindowHandle, AppCell, AppContext, AsyncAppContext, BackgroundExecutor, Context, EventEmitter, ForegroundExecutor, Model, ModelContext, Result, Task, TestDispatcher, - TestPlatform, WindowContext, AppCell, + TestPlatform, WindowContext, }; use anyhow::{anyhow, bail}; use futures::{Stream, StreamExt}; diff --git a/crates/gpui2/src/elements/img.rs b/crates/gpui2/src/elements/img.rs index a9950bfc0a9875459976bda2898710d6a70a4cc4..a35436d74ec9f668e4081eb9a9c0f1ea4168a914 100644 --- a/crates/gpui2/src/elements/img.rs +++ b/crates/gpui2/src/elements/img.rs @@ -125,9 +125,7 @@ where } else { cx.spawn(|_, mut cx| async move { if image_future.await.log_err().is_some() { - eprintln!(">>> on_next_frame"); cx.on_next_frame(|cx| cx.notify()); - eprintln!("<<< on_next_frame") } }) .detach() diff --git a/crates/gpui2/src/platform/mac/dispatcher.rs b/crates/gpui2/src/platform/mac/dispatcher.rs index a39688bafd346504a0a97b4b5f8164d9c2bc1520..f5334912c6b7aec93fed2af3c33832ff241313c9 100644 --- a/crates/gpui2/src/platform/mac/dispatcher.rs +++ b/crates/gpui2/src/platform/mac/dispatcher.rs @@ -42,7 +42,6 @@ impl PlatformDispatcher for MacDispatcher { } fn dispatch(&self, runnable: Runnable) { - println!("DISPATCH"); unsafe { dispatch_async_f( dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT.try_into().unwrap(), 0), @@ -53,7 +52,6 @@ impl PlatformDispatcher for MacDispatcher { } fn dispatch_on_main_thread(&self, runnable: Runnable) { - println!("DISPATCH ON MAIN THREAD"); unsafe { dispatch_async_f( dispatch_get_main_queue(), diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 6034727d8245f64f1ae0c6ab9688672e14b6bf88..2897c9f38e010d766b4a0ed87e544f4578cf58d6 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -410,67 +410,55 @@ 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, f: impl FnOnce(&mut WindowContext) + 'static) { - let f = Box::new(f); + 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; - self.next_frame_callbacks - .entry(display_id) - .or_default() - .push(f); - - self.frame_consumers.entry(display_id).or_insert_with(|| { - let (tx, rx) = mpsc::unbounded::<()>(); + if !self.frame_consumers.contains_key(&display_id) { + let (tx, mut rx) = mpsc::unbounded::<()>(); + self.platform.set_display_link_output_callback( + display_id, + Box::new(move |_current_time, _output_time| _ = tx.unbounded_send(())), + ); - self.spawn(|cx| async move { + let consumer_task = self.app.spawn(|cx| async move { while rx.next().await.is_some() { - let _ = cx.update(|_, cx| { + cx.update(|cx| { for callback in cx - .app .next_frame_callbacks .get_mut(&display_id) .unwrap() .drain(..) + .collect::>() { callback(cx); } - }); - } - }) - }); + }) + .ok(); - if let Some(callbacks) = self.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; - } - } else { - let mut async_cx = self.to_async(); - self.next_frame_callbacks.insert(display_id, vec![f]); - self.platform.set_display_link_output_callback( - display_id, - Box::new(move |_current_time, _output_time| { - let _ = async_cx.update(|_, cx| { - let callbacks = cx - .next_frame_callbacks - .get_mut(&display_id) - .unwrap() - .drain(..) - .collect::>(); - for callback in callbacks { - callback(cx); - } + // Flush effects, then stop the display link if no new next_frame_callbacks have been added. - if cx.next_frame_callbacks.get(&display_id).unwrap().is_empty() { + cx.update(|cx| { + if cx.next_frame_callbacks.is_empty() { cx.platform.stop_display_link(display_id); } - }); - }), - ); + }) + .ok(); + } + }); + self.frame_consumers.insert(display_id, consumer_task); + } + + if self.next_frame_callbacks.is_empty() { + self.platform.start_display_link(display_id); } - 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(); + })); } /// Spawn the future returned by the given closure on the application thread pool. From 29d8390743c3a466c6c2be43f311801cdd902d99 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 2 Nov 2023 12:28:58 -0600 Subject: [PATCH 5/5] Fix formatting --- crates/ui2/src/elements/avatar.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ui2/src/elements/avatar.rs b/crates/ui2/src/elements/avatar.rs index 357e573f7ca2bd618900215547faf0693f83f4d9..ff574a2042d2805a08a64867916b1cdea9802271 100644 --- a/crates/ui2/src/elements/avatar.rs +++ b/crates/ui2/src/elements/avatar.rs @@ -74,10 +74,10 @@ mod stories { // "https://avatars.githubusercontent.com/u/1486634?v=4", // )) .child(Story::label(cx, "Rounded rectangle")) - // .child( - // Avatar::new("https://avatars.githubusercontent.com/u/1714999?v=4") - // .shape(Shape::RoundedRectangle), - // ) + // .child( + // Avatar::new("https://avatars.githubusercontent.com/u/1714999?v=4") + // .shape(Shape::RoundedRectangle), + // ) } } }