From 880eaa268b20cca964c39329b0b83a94890b4449 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 22 Mar 2022 17:03:24 -0700 Subject: [PATCH] Coalesce followed view updates only within one frame Co-Authored-By: Nathan Sobo --- crates/gpui/src/app.rs | 113 +++++++++++++++++++++++++++--- crates/workspace/src/workspace.rs | 29 ++++---- 2 files changed, 115 insertions(+), 27 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 1e4448de98ef5017845c0d552b20778f456c6a89..d5a7fcc6e6fc03830c1ed5da925edb3f2da79a33 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1224,7 +1224,17 @@ impl MutableAppContext { } fn defer(&mut self, callback: Box) { - self.pending_effects.push_back(Effect::Deferred(callback)) + self.pending_effects.push_back(Effect::Deferred { + callback, + after_window_update: false, + }) + } + + pub fn after_window_update(&mut self, callback: impl 'static + FnOnce(&mut MutableAppContext)) { + self.pending_effects.push_back(Effect::Deferred { + callback: Box::new(callback), + after_window_update: true, + }) } pub(crate) fn notify_model(&mut self, model_id: usize) { @@ -1640,6 +1650,7 @@ impl MutableAppContext { fn flush_effects(&mut self) { self.pending_flushes = self.pending_flushes.saturating_sub(1); + let mut after_window_update_callbacks = Vec::new(); if !self.flushing_effects && self.pending_flushes == 0 { self.flushing_effects = true; @@ -1675,7 +1686,16 @@ impl MutableAppContext { Effect::ViewNotification { window_id, view_id } => { self.notify_view_observers(window_id, view_id) } - Effect::Deferred(callback) => callback(self), + Effect::Deferred { + callback, + after_window_update, + } => { + if after_window_update { + after_window_update_callbacks.push(callback); + } else { + callback(self) + } + } Effect::ModelRelease { model_id, model } => { self.notify_release_observers(model_id, model.as_any()) } @@ -1707,12 +1727,18 @@ impl MutableAppContext { } if self.pending_effects.is_empty() { - self.flushing_effects = false; - self.pending_notifications.clear(); - break; - } else { - refreshing = false; + for callback in after_window_update_callbacks.drain(..) { + callback(self); + } + + if self.pending_effects.is_empty() { + self.flushing_effects = false; + self.pending_notifications.clear(); + break; + } } + + refreshing = false; } } } @@ -2347,7 +2373,10 @@ pub enum Effect { window_id: usize, view_id: usize, }, - Deferred(Box), + Deferred { + callback: Box, + after_window_update: bool, + }, ModelRelease { model_id: usize, model: Box, @@ -2413,7 +2442,7 @@ impl Debug for Effect { .field("window_id", window_id) .field("view_id", view_id) .finish(), - Effect::Deferred(_) => f.debug_struct("Effect::Deferred").finish(), + Effect::Deferred { .. } => f.debug_struct("Effect::Deferred").finish(), Effect::ModelRelease { model_id, .. } => f .debug_struct("Effect::ModelRelease") .field("model_id", model_id) @@ -2945,6 +2974,18 @@ impl<'a, T: View> ViewContext<'a, T> { })) } + pub fn after_window_update( + &mut self, + callback: impl 'static + FnOnce(&mut T, &mut ViewContext), + ) { + let handle = self.handle(); + self.app.after_window_update(move |cx| { + handle.update(cx, |view, cx| { + callback(view, cx); + }) + }) + } + pub fn propagate_action(&mut self) { self.app.halt_action_dispatch = false; } @@ -4424,7 +4465,7 @@ mod tests { use smol::future::poll_once; use std::{ cell::Cell, - sync::atomic::{AtomicUsize, Ordering::SeqCst}, + sync::atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst}, }; #[crate::test(self)] @@ -4622,6 +4663,58 @@ mod tests { assert_eq!(*events.borrow(), ["before notify"]); } + #[crate::test(self)] + fn test_defer_and_after_window_update(cx: &mut MutableAppContext) { + struct View { + render_count: usize, + } + + impl Entity for View { + type Event = usize; + } + + impl super::View for View { + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + post_inc(&mut self.render_count); + Empty::new().boxed() + } + + fn ui_name() -> &'static str { + "View" + } + } + + let (_, view) = cx.add_window(Default::default(), |_| View { render_count: 0 }); + let called_defer = Rc::new(AtomicBool::new(false)); + let called_after_window_update = Rc::new(AtomicBool::new(false)); + + view.update(cx, |this, cx| { + assert_eq!(this.render_count, 1); + cx.defer({ + let called_defer = called_defer.clone(); + move |this, _| { + assert_eq!(this.render_count, 1); + called_defer.store(true, SeqCst); + } + }); + cx.after_window_update({ + let called_after_window_update = called_after_window_update.clone(); + move |this, cx| { + assert_eq!(this.render_count, 2); + called_after_window_update.store(true, SeqCst); + cx.notify(); + } + }); + assert!(!called_defer.load(SeqCst)); + assert!(!called_after_window_update.load(SeqCst)); + cx.notify(); + }); + + assert!(called_defer.load(SeqCst)); + assert!(called_after_window_update.load(SeqCst)); + assert_eq!(view.read(cx).render_count, 3); + } + #[crate::test(self)] fn test_view_handles(cx: &mut MutableAppContext) { struct View { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 55338130dab81644332e2112a24afbab038c38c4..2d8891b1e05f44abed00c49cc00c39e0d849d343 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -458,26 +458,21 @@ impl ItemHandle for ViewHandle { && !pending_update_scheduled.load(SeqCst) { pending_update_scheduled.store(true, SeqCst); - cx.spawn({ + cx.after_window_update({ let pending_update = pending_update.clone(); let pending_update_scheduled = pending_update_scheduled.clone(); - move |this, mut cx| async move { - this.update(&mut cx, |this, cx| { - pending_update_scheduled.store(false, SeqCst); - this.update_followers( - proto::update_followers::Variant::UpdateView( - proto::UpdateView { - id: item.id() as u64, - variant: pending_update.borrow_mut().take(), - leader_id: leader_id.map(|id| id.0), - }, - ), - cx, - ); - }); + move |this, cx| { + pending_update_scheduled.store(false, SeqCst); + this.update_followers( + proto::update_followers::Variant::UpdateView(proto::UpdateView { + id: item.id() as u64, + variant: pending_update.borrow_mut().take(), + leader_id: leader_id.map(|id| id.0), + }), + cx, + ); } - }) - .detach(); + }); } }