From 46a563a1468c9f364870cb99fe58cf02d5174618 Mon Sep 17 00:00:00 2001 From: Julian Montes de Oca Date: Fri, 20 Sep 2024 20:33:56 +0800 Subject: [PATCH] Add caching of RoomScreen and RoomsList --- src/app.rs | 1 - src/home/main_content.rs | 5 +- src/home/room_preview.rs | 5 + src/home/rooms_sidebar.rs | 28 +---- src/shared/cached_widget.rs | 203 ++++++++++++++++++++++++++++++++++++ src/shared/mod.rs | 2 + 6 files changed, 219 insertions(+), 25 deletions(-) create mode 100644 src/shared/cached_widget.rs diff --git a/src/app.rs b/src/app.rs index 91e3b79d..a2a0424d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -150,7 +150,6 @@ impl MatchEvent for App { room_index: _, room_name, } => { - // TODO there seems to be unnecessary redrawing and/or fetching backwards in the timeline self.app_state.rooms_panel.selected_room = Some(Room { id: room_id.clone(), name: room_name.clone(), diff --git a/src/home/main_content.rs b/src/home/main_content.rs index 4b6689d5..faf7356d 100644 --- a/src/home/main_content.rs +++ b/src/home/main_content.rs @@ -11,6 +11,7 @@ live_design! { import crate::shared::styles::*; import crate::shared::search_bar::SearchBar; + import crate::shared::cached_widget::CachedWidget; import crate::home::room_screen::RoomScreen; import crate::home::welcome_screen::WelcomeScreen; @@ -30,7 +31,9 @@ live_design! { rooms = { align: {x: 0.5, y: 0.5} width: Fill, height: Fill - room_screen = {} + { + room_screen = {} + } } } } diff --git a/src/home/room_preview.rs b/src/home/room_preview.rs index 459fd2c8..5f18fac9 100644 --- a/src/home/room_preview.rs +++ b/src/home/room_preview.rs @@ -242,6 +242,11 @@ impl Widget for RoomPreviewContent { if cx.get_global::().is_desktop() { self.update_preview_colors(cx, room_info.is_selected); + } else if room_info.is_selected { + // Mobile doesn't have a selected state. Always use the default colors. + // We call the update in case the app was resized from desktop to mobile while the room was selected. + // This can be optimized by only calling this when the app is resized. + self.update_preview_colors(cx, false); } } self.view.draw_walk(cx, scope, walk) diff --git a/src/home/rooms_sidebar.rs b/src/home/rooms_sidebar.rs index 32b17cd0..357c8a82 100644 --- a/src/home/rooms_sidebar.rs +++ b/src/home/rooms_sidebar.rs @@ -10,6 +10,7 @@ live_design! { import crate::shared::adaptive_view::AdaptiveView; import crate::home::rooms_list::RoomsList; + import crate::shared::cached_widget::CachedWidget; ICON_COLLAPSE = dep("crate://self/resources/icons/collapse.svg") ICON_ADD = dep("crate://self/resources/icons/add.svg") @@ -147,7 +148,9 @@ live_design! { } } } - {} + { + rooms_list = {} + } } RoomsSideBar = { @@ -168,35 +171,14 @@ live_design! { pub struct RoomsView { #[deref] view: View, - - #[rust] - new_walk: Walk } impl Widget for RoomsView { fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { self.view.handle_event(cx, event, scope); - // TODO(julian): remove this - if let Event::KeyDown(ke) = event { - if ke.key_code == KeyCode::ArrowLeft { - self.new_walk = self.walk.clone(); - self.new_walk.width = Size::Fixed(self.new_walk.width.fixed_or_zero() - 5.0); - self.walk = self.new_walk.clone(); - self.redraw(cx); - } - - if ke.key_code == KeyCode::ArrowRight { - self.new_walk = self.walk.clone(); - self.new_walk.width = Size::Fixed(self.new_walk.width.fixed_or_zero() + 5.0); - self.walk = self.new_walk.clone(); - self.redraw(cx); - } - } } fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { - // TODO(julian): remove this - self.view.draw_walk(cx, scope, self.new_walk) - // self.view.draw_walk(cx, scope, walk) + self.view.draw_walk(cx, scope, walk) } } diff --git a/src/shared/cached_widget.rs b/src/shared/cached_widget.rs new file mode 100644 index 00000000..468a589c --- /dev/null +++ b/src/shared/cached_widget.rs @@ -0,0 +1,203 @@ +use std::collections::HashMap; + +use makepad_widgets::*; + +live_design! { + import makepad_widgets::base::*; + import makepad_widgets::theme_desktop_dark::*; + + CachedWidget = {{CachedWidget}} {} +} + +/// A widget that caches and reuses its child widget across multiple instances. +/// +/// `CachedWidget` is designed to optimize performance and memory usage by ensuring +/// that only one instance of a child widget is created and shared across multiple +/// uses in the UI. This is particularly useful for complex widgets that are used +/// in different parts of the UI but should maintain a single state. +/// +/// # Usage +/// +/// In the DSL, you can use `CachedWidget` as follows: +/// +/// ``` +/// { +/// my_widget = {} +/// } +/// ``` +/// +/// The child widget will be created once and cached. +/// Subsequent uses of this `CachedWidget` with the same child id (`mid_widget`) will reuse the cached instance. +/// Note that only one child is supported per `CachedWidget`. +/// +/// CachedWidget supports Makepad's widget finding mechanism, allowing child widgets to be located as expected. +/// +/// # Implementation Details +/// +/// - Uses a global `WidgetWrapperCache` to store cached widgets +/// - Handles widget creation and caching in the `after_apply` hook +/// - Delegates most widget operations (like event handling and drawing) to the cached child widget +/// +/// # Note +/// +/// While `CachedWidget` can significantly improve performance for complex, frequently used widgets, +/// it should be used judiciously. Overuse of caching can lead to unexpected behavior if not managed properly. +#[derive(Live, LiveRegisterWidget, WidgetRef)] +pub struct CachedWidget { + #[rust] + area: Area, + + #[walk] + walk: Walk, + + /// The ID of the child widget template + #[rust] + template_id: LiveId, + + /// The cached child widget template + #[rust] + template: Option, + + /// The cached child widget instance + #[rust] + widget: Option, +} + +impl LiveHook for CachedWidget { + fn before_apply( + &mut self, + _cx: &mut Cx, + apply: &mut Apply, + _index: usize, + _nodes: &[LiveNode], + ) { + if let ApplyFrom::UpdateFromDoc { .. } = apply.from { + self.template = None; + } + } + + /// Handles the application of instance properties to this CachedWidget. + /// + /// In the case of `CachedWidget` This method is responsible for setting up the template + /// for the child widget, and applying any changes to an existing widget instance. + fn apply_value_instance( + &mut self, + cx: &mut Cx, + apply: &mut Apply, + index: usize, + nodes: &[LiveNode], + ) -> usize { + if nodes[index].is_instance_prop() { + if let Some(live_ptr) = apply.from.to_live_ptr(cx, index) { + if self.template.is_some() { + nodes.skip_node(index); + error!("CachedWidget only supports one child widget, skipping additional instances"); + } + let id = nodes[index].id; + self.template_id = id; + self.template = Some(live_ptr); + + if let Some(widget) = &mut self.widget { + widget.apply(cx, apply, index, nodes); + } + } + } else { + cx.apply_error_no_matching_field(live_error_origin!(), index, nodes); + } + nodes.skip_node(index) + } + + /// Handles the creation or retrieval of the cached widget after applying changes. + /// + /// This method is called after all properties have been applied to the widget. + /// It ensures that the child widget is properly cached and retrieved from the global cache. + fn after_apply(&mut self, cx: &mut Cx, _apply: &mut Apply, _index: usize, _nodes: &[LiveNode]) { + // Ensure the global widget cache exists + if !cx.has_global::() { + cx.set_global(WidgetWrapperCache::default()) + } + + if self.widget.is_none() { + // Try to retrieve the widget from the global cache + if let Some(widget) = cx + .get_global::() + .map + .get_mut(&self.template_id) + { + self.widget = Some(widget.clone()); + } else { + // If not in cache, create a new widget and add it to the cache + let widget = WidgetRef::new_from_ptr(cx, self.template); + cx.get_global::() + .map + .insert(self.template_id, widget.clone()); + self.widget = Some(widget); + } + } + } +} + +impl WidgetNode for CachedWidget { + fn walk(&mut self, cx: &mut Cx) -> Walk { + if let Some(widget) = &self.widget { + widget.walk(cx) + } else { + self.walk + } + } + fn area(&self) -> Area { + if let Some(widget) = &self.widget { + widget.area() + } else { + self.area + } + } + + fn redraw(&mut self, cx: &mut Cx) { + self.area.redraw(cx); + } + + // Searches for widgets within this CachedWidget based on the given path. + fn find_widgets(&self, path: &[LiveId], cached: WidgetCache, results: &mut WidgetSet) { + let Some(widget) = self.widget.as_ref() else { return }; + if self.template_id == path[0] { + if path.len() == 1 { + // If the child widget is the target widget, add it to the results + results.push(widget.clone()); + } else { + // If not, continue searching in the child widget + widget.find_widgets(&path[1..], cached, results); + } + } + } + + fn uid_to_widget(&self, uid: WidgetUid) -> WidgetRef { + if let Some(widget) = &self.widget { + return widget.uid_to_widget(uid); + } + WidgetRef::empty() + } +} + +impl Widget for CachedWidget { + fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { + if let Some(widget) = &self.widget { + widget.handle_event(cx, event, scope); + } + } + + fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep { + if let Some(widget) = &self.widget { + return widget.draw_walk(cx, scope, walk); + } + + DrawStep::done() + } +} + +impl CachedWidget {} + +#[derive(Default)] +pub struct WidgetWrapperCache { + map: HashMap, +} diff --git a/src/shared/mod.rs b/src/shared/mod.rs index 1cae4abd..53f00dea 100644 --- a/src/shared/mod.rs +++ b/src/shared/mod.rs @@ -8,6 +8,7 @@ pub mod html_or_plaintext; pub mod search_bar; pub mod styles; pub mod text_or_image; +pub mod cached_widget; pub fn live_design(cx: &mut Cx) { // Order matters here, as some widget definitions depend on others. @@ -19,4 +20,5 @@ pub fn live_design(cx: &mut Cx) { text_or_image::live_design(cx); html_or_plaintext::live_design(cx); adaptive_view::live_design(cx); + cached_widget::live_design(cx); }