Make SpaceLobbyScreen tree generation much more efficient (less cloning)

Move `AvatarStatus` out of `user_profile` module since it's now used
for not just User avatars but also Room/Space avatars.

Author: kevinaboos

src/home/mod.rs

@@ -1,4 +1,4 @@
-use makepad_widgets::{Cx, LiveRegister};
+use makepad_widgets::Cx;
 
 pub mod add_room;
 pub mod edited_indicator;
@@ -34,7 +34,6 @@ pub fn live_design(cx: &mut Cx) {
     location_preview::live_design(cx);
     add_room::live_design(cx);
     space_lobby::live_design(cx);
-    space_lobby::DrawTreeLine::live_register(cx);
     rooms_list_entry::live_design(cx);
     rooms_list_header::live_design(cx);
     rooms_list::live_design(cx);

src/home/navigation_tab_bar.rs

@@ -31,10 +31,10 @@
 use makepad_widgets::*;
 use crate::{
     avatar_cache::{self, AvatarCacheEntry}, login::login_screen::LoginAction, logout::logout_confirm_modal::LogoutAction, profile::{
-        user_profile::{AvatarState, UserProfile},
+        user_profile::UserProfile,
         user_profile_cache::{self, UserProfileUpdate},
     }, shared::{
-        avatar::AvatarWidgetExt,
+        avatar::{AvatarState, AvatarWidgetExt},
         callout_tooltip::{CalloutTooltipOptions, TooltipAction, TooltipPosition},
         styles::*,
         verification_badge::VerificationBadgeWidgetExt,

src/home/room_screen.rs

@@ -26,12 +26,12 @@ use ruma::{OwnedUserId, events::{AnySyncMessageLikeEvent, AnySyncTimelineEvent,
 
 use crate::{
     app::AppStateAction, avatar_cache, event_preview::{plaintext_body_of_timeline_item, text_preview_of_encrypted_message, text_preview_of_member_profile_change, text_preview_of_other_message_like, text_preview_of_other_state, text_preview_of_room_membership_change, text_preview_of_timeline_item}, home::{edited_indicator::EditedIndicatorWidgetRefExt, link_preview::{LinkPreviewCache, LinkPreviewRef, LinkPreviewWidgetRefExt}, loading_pane::{LoadingPaneState, LoadingPaneWidgetExt}, room_image_viewer::{get_image_name_and_filesize, populate_matrix_image_modal}, rooms_list::RoomsListRef, tombstone_footer::SuccessorRoomDetails}, media_cache::{MediaCache, MediaCacheEntry}, profile::{
-        user_profile::{AvatarState, ShowUserProfileAction, UserProfile, UserProfileAndRoomId, UserProfilePaneInfo, UserProfileSlidingPaneRef, UserProfileSlidingPaneWidgetExt},
+        user_profile::{ShowUserProfileAction, UserProfile, UserProfileAndRoomId, UserProfilePaneInfo, UserProfileSlidingPaneRef, UserProfileSlidingPaneWidgetExt},
         user_profile_cache,
     },
     room::{BasicRoomDetails, room_input_bar::RoomInputBarState, typing_notice::TypingNoticeWidgetExt},
     shared::{
-        avatar::AvatarWidgetRefExt, callout_tooltip::{CalloutTooltipOptions, TooltipAction, TooltipPosition}, confirmation_modal::ConfirmationModalContent, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupItem, PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt
+        avatar::{AvatarState, AvatarWidgetRefExt}, callout_tooltip::{CalloutTooltipOptions, TooltipAction, TooltipPosition}, confirmation_modal::ConfirmationModalContent, html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt, RobrixHtmlLinkAction}, image_viewer::{ImageViewerAction, ImageViewerMetaData, LoadState}, jump_to_bottom_button::{JumpToBottomButtonWidgetExt, UnreadMessageCount}, popup_list::{PopupItem, PopupKind, enqueue_popup_notification}, restore_status_view::RestoreStatusViewWidgetExt, styles::*, text_or_image::{TextOrImageAction, TextOrImageRef, TextOrImageWidgetRefExt}, timestamp::TimestampWidgetRefExt
     },
     sliding_sync::{BackwardsPaginateUntilEventRequest, MatrixRequest, PaginationDirection, TimelineEndpoints, TimelineRequestSender, UserPowerLevels, get_client, submit_async_request, take_timeline_endpoints}, utils::{self, ImageFormat, MEDIA_THUMBNAIL_FORMAT, RoomNameId, unix_time_millis_to_datetime}
 };

src/home/space_lobby.rs

@@ -9,14 +9,12 @@
 use std::collections::{HashMap, HashSet};
 use imbl::Vector;
 use makepad_widgets::*;
-use matrix_sdk::ruma::OwnedRoomId;
+use matrix_sdk::{RoomState, ruma::OwnedRoomId};
 use matrix_sdk_ui::spaces::SpaceRoom;
+use ruma::room::JoinRuleSummary;
 use tokio::sync::mpsc::UnboundedSender;
 use crate::{
-    home::rooms_list::RoomsListRef,
-    shared::avatar::{AvatarWidgetExt, AvatarWidgetRefExt},
-    space_service_sync::{SpaceRequest, SpaceRoomExt, SpaceRoomListAction},
-    utils::{self, RoomNameId},
+    home::rooms_list::RoomsListRef, shared::avatar::{AvatarState, AvatarWidgetExt, AvatarWidgetRefExt}, space_service_sync::{SpaceRequest, SpaceRoomExt, SpaceRoomListAction}, utils::{self, RoomNameId}
 };
 
 
@@ -618,15 +616,63 @@ impl Widget for RoomEntry {
     }
 }
 
+/// The subset of info in [`SpaceRoom`] that we display for each room/space.
+struct SpaceRoomInfo {
+    id: OwnedRoomId,
+    name: String,
+    #[allow(unused)]
+    topic: Option<String>,
+    #[allow(unused)]
+    room_avatar: AvatarState,
+    num_joined_members: u64,
+    #[allow(unused)]
+    state: Option<RoomState>,
+    #[allow(unused)]
+    join_rule: Option<JoinRuleSummary>,
+    /// If `Some`, this is a space. If `None`, it's a room.
+    children_count: Option<u64>,
+}
+impl SpaceRoomInfo {
+    fn is_space(&self) -> bool {
+        self.children_count.is_some()
+    }
+}
+impl From<&SpaceRoom> for SpaceRoomInfo {
+    fn from(space_room: &SpaceRoom) -> Self {
+        SpaceRoomInfo {
+            id: space_room.room_id.clone(),
+            name: space_room.display_name.clone(),
+            topic: space_room.topic.clone(),
+            room_avatar: AvatarState::Known(space_room.avatar_url.clone()),
+            num_joined_members: space_room.num_joined_members,
+            state: space_room.state.clone(),
+            join_rule: space_room.join_rule.clone(),
+            children_count: space_room.is_space().then_some(space_room.children_count),
+        }
+    }
+}
+impl From<SpaceRoom> for SpaceRoomInfo {
+    fn from(space_room: SpaceRoom) -> Self {
+        SpaceRoomInfo {
+            children_count: space_room.is_space().then_some(space_room.children_count),
+            id: space_room.room_id,
+            name: space_room.display_name,
+            topic: space_room.topic,
+            room_avatar: AvatarState::Known(space_room.avatar_url),
+            num_joined_members: space_room.num_joined_members,
+            state: space_room.state,
+            join_rule: space_room.join_rule,
+        }
+    }
+}
 
 /// An entry in the tree to be displayed.
-#[derive(Clone, Debug)]
 #[allow(clippy::large_enum_variant)]
 enum TreeEntry {
     /// A regular space or room entry.
     Item {
-        /// The detailed info about this space or room.
-        info: SpaceRoom,
+        /// The info needed to display this space or room.
+        info: SpaceRoomInfo,
         /// The nesting level (0 = direct child of the displayed space).
         level: usize,
         /// Whether this entry is the last child of its parent.
@@ -685,7 +731,7 @@ impl Widget for SpaceLobbyScreen {
                 if let SubspaceEntryAction::Clicked { space_id: room_id } = action.as_widget_action().cast() {
                     self.toggle_space_expansion(cx, &room_id);
                 }
-                
+
                 // Handle RoomEntry clicks
                 if let RoomEntryAction::Clicked { room_id: _ } = action.as_widget_action().cast() {
                     // TODO: Navigate to the room
@@ -728,7 +774,7 @@ impl Widget for SpaceLobbyScreen {
                             if info.is_space() {
                                 let item = list.item(cx, item_id, id!(subspace_entry));
                                 if let Some(mut inner) = item.borrow_mut::<SubspaceEntry>() {
-                                    inner.space_id = Some(info.room_id.clone());
+                                    inner.space_id = Some(info.id.clone());
                                 }
 
                                 // Configure tree lines
@@ -740,21 +786,21 @@ impl Widget for SpaceLobbyScreen {
                                 }
 
                                 // Expand icon
-                                let is_expanded = self.expanded_spaces.contains(&info.room_id);
+                                let is_expanded = self.expanded_spaces.contains(&info.id);
                                 let angle = if is_expanded { 180.0 } else { 90.0 };
                                 item.icon(ids!(expand_icon)).apply_over(cx, live! {
                                     draw_icon: { rotation_angle: (angle) }
                                 });
 
                                 // Avatar
                                 let avatar_ref = item.avatar(ids!(avatar));
-                                let first_char = utils::user_name_first_letter(&info.display_name);
+                                let first_char = utils::user_name_first_letter(&info.name);
                                 avatar_ref.show_text(cx, None, None, first_char.unwrap_or("#"));
 
                                 // Text
-                                item.label(ids!(content.name_label)).set_text(cx, &info.display_name);
-                                let info_text = if info.children_count > 0 {
-                                    format!("{} members · {} rooms", info.num_joined_members, info.children_count)
+                                item.label(ids!(content.name_label)).set_text(cx, &info.name);
+                                let info_text = if let Some(c) = info.children_count && c > 0 {
+                                    format!("{} members · {} rooms", info.num_joined_members, c)
                                 } else {
                                     format!("{} members", info.num_joined_members)
                                 };
@@ -764,7 +810,7 @@ impl Widget for SpaceLobbyScreen {
                             } else {
                                 let item = list.item(cx, item_id, id!(room_entry));
                                 if let Some(mut inner) = item.borrow_mut::<RoomEntry>() {
-                                    inner.room_id = Some(info.room_id.clone());
+                                    inner.room_id = Some(info.id.clone());
                                 }
 
                                 // Configure tree lines
@@ -777,11 +823,11 @@ impl Widget for SpaceLobbyScreen {
 
                                 // Avatar
                                 let avatar_ref = item.avatar(ids!(avatar));
-                                let first_char = utils::user_name_first_letter(&info.display_name);
+                                let first_char = utils::user_name_first_letter(&info.name);
                                 avatar_ref.show_text(cx, None, None, first_char.unwrap_or("#"));
 
                                 // Text
-                                item.label(ids!(content.name_label)).set_text(cx, &info.display_name);
+                                item.label(ids!(content.name_label)).set_text(cx, &info.name);
                                 let info_text = format!("{} members", info.num_joined_members);
                                 item.label(ids!(content.info_label)).set_text(cx, &info_text);
 
@@ -882,8 +928,8 @@ impl SpaceLobbyScreen {
     /// can be displayed in the SpaceLobbyScreen's PortalList.
     //
     // Note: this is intentionally *not* a method (it doesn't take &mut self),
-    // in order to make it possible to recursively call it while borrowing only select
-    // fields of `Self`.
+    // in order to make it possible to recursively call it while immutably borrowing
+    // only select fields of `Self`.
     fn build_tree_for_space(
         children_cache: &HashMap<OwnedRoomId, Vector<SpaceRoom>>,
         expanded_spaces: &HashSet<OwnedRoomId>,
@@ -907,11 +953,11 @@ impl SpaceLobbyScreen {
 
 
         let count = sorted_children.len();
-        for (i, child) in sorted_children.iter().enumerate() {
+        for (i, child) in sorted_children.into_iter().enumerate() {
             let is_last = i == count - 1;
 
             tree_entries.push(TreeEntry::Item {
-                info: (*child).clone(),
+                info: SpaceRoomInfo::from(child),
                 level,
                 is_last,
                 parent_mask,

src/profile/user_profile.rs

@@ -1,53 +1,13 @@
-use std::{borrow::Cow, ops::{Deref, DerefMut}, sync::Arc};
+//! Widgets and types related to displaying info about a user profile.
+
+use std::{borrow::Cow, ops::{Deref, DerefMut}};
 use makepad_widgets::*;
-use matrix_sdk::{room::{RoomMember, RoomMemberRole}, ruma::{events::room::member::MembershipState, OwnedMxcUri, OwnedRoomId, OwnedUserId}};
+use matrix_sdk::{room::{RoomMember, RoomMemberRole}, ruma::{events::room::member::MembershipState, OwnedRoomId, OwnedUserId}};
 use crate::{
-    avatar_cache::{self, AvatarCacheEntry}, shared::{avatar::AvatarWidgetExt, popup_list::{enqueue_popup_notification, PopupItem, PopupKind}}, sliding_sync::{current_user_id, is_user_ignored, submit_async_request, MatrixRequest}, utils
+    avatar_cache::{self, AvatarCacheEntry}, shared::{avatar::{AvatarState, AvatarWidgetExt}, popup_list::{PopupItem, PopupKind, enqueue_popup_notification}}, sliding_sync::{MatrixRequest, current_user_id, is_user_ignored, submit_async_request}, utils
 };
 use super::user_profile_cache;
 
-/// The currently-known state of a user's avatar.
-#[derive(Clone)]
-pub enum AvatarState {
-    /// It isn't yet known if this user has an avatar.
-    Unknown,
-    /// It is known that this user does or does not have an avatar.
-    Known(Option<OwnedMxcUri>),
-    /// This user does have an avatar, and it has been fetched successfully.
-    Loaded(Arc<[u8]>),
-    /// This user does have an avatar, but we failed to fetch it.
-    Failed,
-}
-impl std::fmt::Debug for AvatarState {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            AvatarState::Unknown        => write!(f, "Unknown"),
-            AvatarState::Known(Some(_)) => write!(f, "Known(Some)"),
-            AvatarState::Known(None)    => write!(f, "Known(None)"),
-            AvatarState::Loaded(data)   => write!(f, "Loaded({} bytes)", data.len()),
-            AvatarState::Failed         => write!(f, "Failed"),
-        }
-    }
-}
-impl AvatarState {
-    /// Returns the avatar data, if in the `Loaded` state.
-    pub fn data(&self) -> Option<&Arc<[u8]>> {
-        if let AvatarState::Loaded(data) = self {
-            Some(data)
-        } else {
-            None
-        }
-    }
-
-    /// Returns the avatar URI, if in the `Known` state and it exists.
-    pub fn uri(&self) -> Option<&OwnedMxcUri> {
-        if let AvatarState::Known(Some(uri)) = self {
-            Some(uri)
-        } else {
-            None
-        }
-    }
-}
 
 /// Information retrieved about a user: their displayable name, ID, and known avatar state.
 #[derive(Clone, Debug)]

src/profile/user_profile_cache.rs

@@ -7,7 +7,7 @@ use makepad_widgets::{warning, Cx, SignalToUI};
 use matrix_sdk::{room::RoomMember, ruma::{OwnedRoomId, OwnedUserId, UserId}};
 use std::{cell::RefCell, collections::{btree_map::Entry, BTreeMap}};
 
-use crate::{profile::user_profile::AvatarState, sliding_sync::{submit_async_request, MatrixRequest}};
+use crate::{shared::avatar::AvatarState, sliding_sync::{submit_async_request, MatrixRequest}};
 
 use super::user_profile::UserProfile;
 

src/shared/avatar.rs

@@ -11,9 +11,10 @@ use std::sync::Arc;
 use makepad_widgets::*;
 use matrix_sdk::{ruma::{EventId, OwnedRoomId, OwnedUserId, RoomId, UserId}};
 use matrix_sdk_ui::timeline::{Profile, TimelineDetails};
+use ruma::OwnedMxcUri;
 
 use crate::{
-    avatar_cache::{self, AvatarCacheEntry}, profile::{user_profile::{AvatarState, ShowUserProfileAction, UserProfile, UserProfileAndRoomId}, user_profile_cache}, sliding_sync::{submit_async_request, MatrixRequest}, utils
+    avatar_cache::{self, AvatarCacheEntry}, profile::{user_profile::{ShowUserProfileAction, UserProfile, UserProfileAndRoomId}, user_profile_cache}, sliding_sync::{submit_async_request, MatrixRequest}, utils
 };
 
 live_design! {
@@ -450,3 +451,47 @@ impl From<(OwnedUserId, Option<String>, OwnedRoomId, Arc<[u8]>)> for AvatarImage
         Self { user_id, username, room_id, img_data }
     }
 }
+
+
+/// The currently-known state of an avatar for a user, room, or space.
+#[derive(Clone)]
+pub enum AvatarState {
+    /// It isn't yet known if this user/room/space has an avatar.
+    Unknown,
+    /// It is known that this user/room/space does or does not have an avatar.
+    Known(Option<OwnedMxcUri>),
+    /// The avatar is known to exist and has been fetched successfully.
+    Loaded(Arc<[u8]>),
+    /// The avatar is known to exist but could not be fetched.
+    Failed,
+}
+impl std::fmt::Debug for AvatarState {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            AvatarState::Unknown        => write!(f, "Unknown"),
+            AvatarState::Known(Some(_)) => write!(f, "Known(Some)"),
+            AvatarState::Known(None)    => write!(f, "Known(None)"),
+            AvatarState::Loaded(data)   => write!(f, "Loaded({} bytes)", data.len()),
+            AvatarState::Failed         => write!(f, "Failed"),
+        }
+    }
+}
+impl AvatarState {
+    /// Returns the avatar data, if in the `Loaded` state.
+    pub fn data(&self) -> Option<&Arc<[u8]>> {
+        if let AvatarState::Loaded(data) = self {
+            Some(data)
+        } else {
+            None
+        }
+    }
+
+    /// Returns the avatar URI, if in the `Known` state and it exists.
+    pub fn uri(&self) -> Option<&OwnedMxcUri> {
+        if let AvatarState::Known(Some(uri)) = self {
+            Some(uri)
+        } else {
+            None
+        }
+    }
+}

src/sliding_sync.rs

@@ -32,12 +32,10 @@ use crate::{
     app::AppStateAction, app_data_dir, avatar_cache::AvatarUpdate, event_preview::text_preview_of_timeline_item, home::{
         add_room::KnockResultAction, invite_screen::{JoinRoomResultAction, LeaveRoomResultAction}, link_preview::{LinkPreviewData, LinkPreviewDataNonNumeric, LinkPreviewRateLimitResponse}, room_screen::TimelineUpdate, rooms_list::{self, InvitedRoomInfo, InviterInfo, JoinedRoomInfo, RoomsListUpdate, enqueue_rooms_list_update}, rooms_list_header::RoomsListHeaderAction, tombstone_footer::SuccessorRoomDetails
     }, login::login_screen::LoginAction, logout::{logout_confirm_modal::LogoutAction, logout_state_machine::{LogoutConfig, is_logout_in_progress, logout_with_state_machine}}, media_cache::{MediaCacheEntry, MediaCacheEntryRef}, persistence::{self, ClientSessionPersisted, load_app_state}, profile::{
-        user_profile::{AvatarState, UserProfile},
+        user_profile::UserProfile,
         user_profile_cache::{UserProfileUpdate, enqueue_user_profile_update},
     }, room::{FetchedRoomAvatar, FetchedRoomPreview, RoomPreviewAction}, shared::{
-        html_or_plaintext::MatrixLinkPillState,
-        jump_to_bottom_button::UnreadMessageCount,
-        popup_list::{PopupItem, PopupKind, enqueue_popup_notification}
+        avatar::AvatarState, html_or_plaintext::MatrixLinkPillState, jump_to_bottom_button::UnreadMessageCount, popup_list::{PopupItem, PopupKind, enqueue_popup_notification}
     }, space_service_sync::space_service_loop, utils::{self, AVATAR_THUMBNAIL_FORMAT, RoomNameId, avatar_from_room_name}, verification::add_verification_event_handlers_and_sync_client
 };