From 985ac4e5f2483598087fd1315cbf1f7307ef88e0 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Wed, 19 Mar 2025 05:52:20 +0800 Subject: [PATCH] gpui: Reduce `window.refresh` to improve cache hit of the cached views (#25009) Release Notes: - Improved performance when using the scroll wheel and some other mouse interactions. Based on some cache details about GPUI `AnyView::cached` that I found in the discussion of https://github.com/zed-industries/zed/discussions/24260#discussioncomment-12135749, and combined with the optimization points found in actual applications. This change may have some scenarios that I have not considered, so I just make a draft to put forward my ideas first for discussion. From my analysis, `AnyView::cached` will always invalid by Div's mouse events, because of it called `window.refresh`. I understand that (mouse move event) this is because the interface changes related to hover and mouse_move will be affected style, so `window.refresh` is required. Since Div does not have the `entity_id` of View, it is impossible to know which View should be refreshed, so the entire window can only be refreshed. With this change, we can reduce a lot of `render` method calls on ScrollWheel or Mouse Event. --- crates/gpui/src/elements/div.rs | 18 +++++++++------- crates/gpui/src/elements/list.rs | 25 +++++++++++++++++------ crates/gpui/src/elements/text.rs | 3 ++- crates/ui/src/components/indent_guides.rs | 14 ++++++------- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 6b0761ef16d424b37158a4af14bfb9dcea3cdfc6..7a9078551aa91e66359b0f9c3e9ead17077dea8f 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1708,13 +1708,14 @@ impl Interactivity { }); let was_hovered = hitbox.is_hovered(window); + let current_view = window.current_view(); window.on_mouse_event({ let hitbox = hitbox.clone(); - move |_: &MouseMoveEvent, phase, window, _| { + move |_: &MouseMoveEvent, phase, window, cx| { if phase == DispatchPhase::Capture { let hovered = hitbox.is_hovered(window); if hovered != was_hovered { - window.refresh(); + cx.notify(current_view) } } } @@ -1828,10 +1829,11 @@ impl Interactivity { { let hitbox = hitbox.clone(); let was_hovered = hitbox.is_hovered(window); - window.on_mouse_event(move |_: &MouseMoveEvent, phase, window, _cx| { + let current_view = window.current_view(); + window.on_mouse_event(move |_: &MouseMoveEvent, phase, window, cx| { let hovered = hitbox.is_hovered(window); if phase == DispatchPhase::Capture && hovered != was_hovered { - window.refresh(); + cx.notify(current_view); } }); } @@ -2117,10 +2119,11 @@ impl Interactivity { if let Some(group_hitbox) = group_hitbox { let was_hovered = group_hitbox.is_hovered(window); - window.on_mouse_event(move |_: &MouseMoveEvent, phase, window, _cx| { + let current_view = window.current_view(); + window.on_mouse_event(move |_: &MouseMoveEvent, phase, window, cx| { let hovered = group_hitbox.is_hovered(window); if phase == DispatchPhase::Capture && hovered != was_hovered { - window.refresh(); + cx.notify(current_view); } }); } @@ -2139,6 +2142,7 @@ impl Interactivity { let restrict_scroll_to_axis = style.restrict_scroll_to_axis; let line_height = window.line_height(); let hitbox = hitbox.clone(); + let current_view = window.current_view(); window.on_mouse_event(move |event: &ScrollWheelEvent, phase, window, cx| { if phase == DispatchPhase::Bubble && hitbox.is_hovered(window) { let mut scroll_offset = scroll_offset.borrow_mut(); @@ -2172,7 +2176,7 @@ impl Interactivity { scroll_offset.x += delta_x; cx.stop_propagation(); if *scroll_offset != old_scroll_offset { - window.refresh(); + cx.notify(current_view); } } }); diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 36b6bc5a10c2f3fae9a7bd76f85cf787baceb665..0e3a98c0b0c6db6ecd8d651912878ff70d17832c 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -9,8 +9,8 @@ use crate::{ point, px, size, AnyElement, App, AvailableSpace, Bounds, ContentMask, DispatchPhase, Edges, - Element, FocusHandle, GlobalElementId, Hitbox, IntoElement, Pixels, Point, ScrollWheelEvent, - Size, Style, StyleRefinement, Styled, Window, + Element, EntityId, FocusHandle, GlobalElementId, Hitbox, IntoElement, Pixels, Point, + ScrollWheelEvent, Size, Style, StyleRefinement, Styled, Window, }; use collections::VecDeque; use refineable::Refineable as _; @@ -371,6 +371,7 @@ impl StateInner { scroll_top: &ListOffset, height: Pixels, delta: Point, + current_view: EntityId, window: &mut Window, cx: &mut App, ) { @@ -413,7 +414,7 @@ impl StateInner { ); } - window.refresh(); + cx.notify(current_view); } fn logical_scroll_top(&self) -> ListOffset { @@ -847,6 +848,7 @@ impl Element for List { window: &mut Window, cx: &mut App, ) { + let current_view = window.current_view(); window.with_content_mask(Some(ContentMask { bounds }), |window| { for item in &mut prepaint.layout.item_layouts { item.element.paint(window, cx); @@ -863,6 +865,7 @@ impl Element for List { &scroll_top, height, event.delta.pixel_delta(px(20.)), + current_view, window, cx, ) @@ -967,7 +970,10 @@ mod test { #[gpui::test] fn test_reset_after_paint_before_scroll(cx: &mut TestAppContext) { - use crate::{div, list, point, px, size, Element, ListState, Styled}; + use crate::{ + div, list, point, px, size, AppContext, Context, Element, IntoElement, ListState, + Render, Styled, Window, + }; let cx = cx.add_empty_window(); @@ -981,9 +987,16 @@ mod test { offset_in_item: px(0.0), }); + struct TestView(ListState); + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + list(self.0.clone()).w_full().h_full() + } + } + // Paint - cx.draw(point(px(0.), px(0.)), size(px(100.), px(20.)), |_, _| { - list(state.clone()).w_full().h_full() + cx.draw(point(px(0.), px(0.)), size(px(100.), px(20.)), |_, cx| { + cx.new(|_| TestView(state.clone())) }); // Reset diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index 4175dcb03a4fe566d9fecae9ba74d5b2fa23da59..c58e793aff2879e32c65f7badd143d84f772e8de 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -691,6 +691,7 @@ impl Element for InteractiveText { window: &mut Window, cx: &mut App, ) { + let current_view = window.current_view(); let text_layout = self.text.layout().clone(); window.with_element_state::( global_id.unwrap(), @@ -764,7 +765,7 @@ impl Element for InteractiveText { if let Some(hover_listener) = hover_listener.as_ref() { hover_listener(updated, event.clone(), window, cx); } - window.refresh(); + cx.notify(current_view); } } } diff --git a/crates/ui/src/components/indent_guides.rs b/crates/ui/src/components/indent_guides.rs index 0d9c3f5571c3a94d354f239df7a11b96f8d636b1..312eb5163c95a90fe50aee34472a42990773871a 100644 --- a/crates/ui/src/components/indent_guides.rs +++ b/crates/ui/src/components/indent_guides.rs @@ -265,6 +265,8 @@ mod uniform_list { window: &mut Window, _cx: &mut App, ) { + let current_view = window.current_view(); + match prepaint { IndentGuidesElementPrepaintState::Static => { for indent_guide in self.indent_guides.as_ref() { @@ -326,7 +328,7 @@ mod uniform_list { window.on_mouse_event({ let prev_hovered_hitbox_id = hovered_hitbox_id; let hitboxes = hitboxes.clone(); - move |_: &MouseMoveEvent, phase, window, _cx| { + move |_: &MouseMoveEvent, phase, window, cx| { let mut hovered_hitbox_id = None; for hitbox in hitboxes.as_ref() { if hitbox.is_hovered(window) { @@ -339,15 +341,11 @@ mod uniform_list { match (prev_hovered_hitbox_id, hovered_hitbox_id) { (Some(prev_id), Some(id)) => { if prev_id != id { - window.refresh(); + cx.notify(current_view) } } - (None, Some(_)) => { - window.refresh(); - } - (Some(_), None) => { - window.refresh(); - } + (None, Some(_)) => cx.notify(current_view), + (Some(_), None) => cx.notify(current_view), (None, None) => {} } }