diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index d01fd4f..dff1563 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -1,7 +1,7 @@ //! A room screen is the UI page that displays a single Room's timeline of events/messages //! along with a message input bar at the bottom. -use std::{collections::BTreeMap, ops::DerefMut, sync::{Arc, Mutex}}; +use std::{collections::BTreeMap, ops::{DerefMut, Range}, sync::{Arc, Mutex}}; use imbl::Vector; use makepad_widgets::*; @@ -604,10 +604,13 @@ pub enum TimelineUpdate { NewItems { /// The entire list of timeline items (events) for a room. items: Vector>, - /// The index of the first item in the `items` list that has changed. - /// Any items before this index are assumed to be unchanged and need not be redrawn, - /// while any items after this index are assumed to be changed and must be redrawn. - index_of_first_change: usize, + /// The range of indices in the `items` list that have been changed in this update + /// and thus must be removed from any caches of drawn items in the timeline. + /// Any items outside of this range are assumed to be unchanged and need not be redrawn. + changed_indices: Range, + /// Whether to clear the entire cache of drawn items in the timeline. + /// This supercedes `index_of_first_change` and is used when the entire timeline is being redrawn. + clear_cache: bool, }, /// A notice that the start of the timeline has been reached, meaning that /// there is no need to send further backwards pagination requests. @@ -793,21 +796,28 @@ impl Widget for Timeline { let mut done_loading = false; while let Ok(update) = tl.update_receiver.try_recv() { match update { - TimelineUpdate::NewItems { items, index_of_first_change } => { + TimelineUpdate::NewItems { items, changed_indices, clear_cache } => { // Determine which item is currently visible the top of the screen // so that we can jump back to that position instantly after applying this update. if let Some(top_event_id) = tl.items.get(orig_first_id).map(|item| item.unique_id()) { for (idx, item) in items.iter().enumerate() { if item.unique_id() == top_event_id { - log!("Timeline::handle_event(): jumping view from top event index {orig_first_id} to index {idx}"); - portal_list.set_first_id(idx); + if orig_first_id != idx { + log!("Timeline::handle_event(): jumping view from top event index {orig_first_id} to index {idx}"); + portal_list.set_first_id(idx); + } break; } } } - tl.content_drawn_since_last_update.remove(index_of_first_change .. items.len()); - tl.profile_drawn_since_last_update.remove(index_of_first_change .. items.len()); - // log!("Timeline::handle_event(): index_of_first_change: {index_of_first_change}, items len: {}\ncontent drawn: {:#?}\nprofile drawn: {:#?}", items.len(), tl.content_drawn_since_last_update, tl.profile_drawn_since_last_update); + if clear_cache { + tl.content_drawn_since_last_update.clear(); + tl.profile_drawn_since_last_update.clear(); + } else { + tl.content_drawn_since_last_update.remove(changed_indices.clone()); + tl.profile_drawn_since_last_update.remove(changed_indices.clone()); + // log!("Timeline::handle_event(): changed_indices: {changed_indices:?}, items len: {}\ncontent drawn: {:#?}\nprofile drawn: {:#?}", items.len(), tl.content_drawn_since_last_update, tl.profile_drawn_since_last_update); + } tl.items = items; } TimelineUpdate::TimelineStartReached => { @@ -1015,32 +1025,6 @@ impl ItemDrawnStatus { } } -// TODO: return this `ItemDrawnStatus` from the populate_*_view functions and use it to determine -// if that item ID can be added to the `drawn_since_last_update` range set (only if both are true). -// For now, we should only add items that are fully drawn to the range set, -// as we don't want to accidentally miss redrawing updated items that were only partially drawn. -// In this way, we won't consider an item fully drawn until both its profile and content are fully drawn. -// **** -// Note: we'll also need to differentiate between: -// an avatar not existing at all (considered fully drawn) -// vs an avatar not being "ready" or not being fetched yet (considered not fully drawn) - -// **** -// Also, we should split `drawn_since_last_update` into two separate `RangeSet`s: -// -- one for items whose CONTENT has been drawn fully, and -// -- one for items whose PROFILE has been drawn fully. -// This way, we can redraw the profile of an item without redrawing its content, and vice versa --> efficient! -// **** -// We should also use a range to specify `index_of_first_change` AND index of last change, -// such that we can support diff operations like set (editing/updating a single event). -// To do so, we'll have to send interim message updates to the UI thread rather than always sending the entire batch of diffs, -// but that's no problem because sending those updates is already very cheap. -// Plus, we already have plans to split up the batches across multiple update messages in the future, -// in order to support conveying more detailed info about which items were actually changed and at which indices -// (e.g., we'll eventually send one update per contiguous set of changed items, rather than one update per entire batch of items). - - - /// Creates, populates, and adds a Message liveview widget to the given `PortalList` /// with the given `item_id`. diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 78a12a9..f7f7af4 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -2,6 +2,7 @@ use anyhow::{Result, bail}; use clap::Parser; use eyeball_im::VectorDiff; use futures_util::{StreamExt, pin_mut}; +use imbl::Vector; use makepad_widgets::{SignalToUI, error, log}; use matrix_sdk::{ Client, @@ -20,13 +21,13 @@ use matrix_sdk::{ media::MediaRequest, Room, }; -use matrix_sdk_ui::{timeline::{SlidingSyncRoomExt, PaginationOptions, BackPaginationStatus, TimelineItemContent, AnyOtherFullStateEventContent}, Timeline}; +use matrix_sdk_ui::{timeline::{AnyOtherFullStateEventContent, BackPaginationStatus, PaginationOptions, SlidingSyncRoomExt, TimelineItem, TimelineItemContent}, Timeline}; use tokio::{ runtime::Handle, sync::mpsc::{UnboundedSender, UnboundedReceiver}, task::JoinHandle, }; use unicode_segmentation::UnicodeSegmentation; -use std::{cmp::min, collections::BTreeMap, sync::{Arc, Mutex, OnceLock}}; +use std::{cmp::{max, min}, collections::BTreeMap, ops::Range, sync::{Arc, Mutex, OnceLock}}; use url::Url; use crate::{home::{rooms_list::{self, RoomPreviewEntry, RoomsListUpdate, RoomPreviewAvatar, enqueue_rooms_list_update}, room_screen::TimelineUpdate}, media_cache::MediaCacheEntry, utils::MEDIA_THUMBNAIL_FORMAT}; @@ -703,84 +704,122 @@ async fn timeline_subscriber_handler( sender.send(TimelineUpdate::NewItems { items: timeline_items.clone(), - index_of_first_change: 0, + changed_indices: usize::MIN..usize::MAX, + clear_cache: true, }).expect("Error: timeline update sender couldn't send update with initial items!"); + let send_update = |timeline_items: Vector>, changed_indices: Range, clear_cache: bool, num_updates: usize| { + if num_updates > 0 { + // log!("timeline_subscriber: applied {num_updates} updates for room {room_id}. Clear cache? {clear_cache}. Changes: {changed_indices:?}."); + sender.send(TimelineUpdate::NewItems { + items: timeline_items, + changed_indices, + clear_cache, + }).expect("Error: timeline update sender couldn't send update with new items!"); + + // Send a Makepad-level signal to update this room's timeline UI view. + SignalToUI::set_ui_signal(); + } + }; + const LOG_DIFFS: bool = false; while let Some(batch) = subscriber.next().await { - let num_updates = batch.len(); + let mut num_updates = 0; let mut index_of_first_change = usize::MAX; + let mut index_of_last_change = usize::MIN; + let mut clear_cache = false; // whether to clear the entire cache of items for diff in batch { match diff { VectorDiff::Append { values } => { + let _values_len = values.len(); index_of_first_change = min(index_of_first_change, timeline_items.len()); - if LOG_DIFFS { log!("timeline_subscriber: diff Append {}, index_of_first_change: {index_of_first_change}", values.len()); } timeline_items.extend(values); + index_of_last_change = max(index_of_last_change, timeline_items.len()); + if LOG_DIFFS { log!("timeline_subscriber: diff Append {_values_len}. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Clear => { if LOG_DIFFS { log!("timeline_subscriber: diff Clear"); } - index_of_first_change = 0; + clear_cache = true; timeline_items.clear(); + num_updates += 1; } VectorDiff::PushFront { value } => { if LOG_DIFFS { log!("timeline_subscriber: diff PushFront"); } - index_of_first_change = 0; + clear_cache = true; timeline_items.push_front(value); + num_updates += 1; } VectorDiff::PushBack { value } => { - if LOG_DIFFS { log!("timeline_subscriber: diff PushBack"); } index_of_first_change = min(index_of_first_change, timeline_items.len()); timeline_items.push_back(value); + index_of_last_change = max(index_of_last_change, timeline_items.len()); + if LOG_DIFFS { log!("timeline_subscriber: diff PushBack. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::PopFront => { if LOG_DIFFS { log!("timeline_subscriber: diff PopFront"); } - index_of_first_change = 0; + clear_cache = true; timeline_items.pop_front(); + num_updates += 1; } VectorDiff::PopBack => { - if LOG_DIFFS { log!("timeline_subscriber: diff PopBack"); } timeline_items.pop_back(); - index_of_first_change = min(index_of_first_change, timeline_items.len().saturating_sub(1)); + index_of_first_change = min(index_of_first_change, timeline_items.len()); + index_of_last_change = usize::MAX; + if LOG_DIFFS { log!("timeline_subscriber: diff PopBack. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Insert { index, value } => { - if LOG_DIFFS { log!("timeline_subscriber: diff Insert at {index}"); } - index_of_first_change = min(index_of_first_change, index); + if index == 0 { + clear_cache = true; + } else { + index_of_first_change = min(index_of_first_change, index); + index_of_last_change = usize::MAX; + } timeline_items.insert(index, value); + if LOG_DIFFS { log!("timeline_subscriber: diff Insert at {index}. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Set { index, value } => { - if LOG_DIFFS { log!("timeline_subscriber: diff Set at {index}"); } index_of_first_change = min(index_of_first_change, index); + index_of_last_change = max(index_of_last_change, index.saturating_add(1)); timeline_items.set(index, value); + if LOG_DIFFS { log!("timeline_subscriber: diff Set at {index}. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Remove { index } => { - if LOG_DIFFS { log!("timeline_subscriber: diff Remove at {index}"); } - index_of_first_change = min(index_of_first_change, index.saturating_sub(1)); + if index == 0 { + clear_cache = true; + } else { + index_of_first_change = min(index_of_first_change, index.saturating_sub(1)); + index_of_last_change = usize::MAX; + } timeline_items.remove(index); + if LOG_DIFFS { log!("timeline_subscriber: diff Remove at {index}. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Truncate { length } => { - if LOG_DIFFS { log!("timeline_subscriber: diff Truncate to length {length}"); } - index_of_first_change = min(index_of_first_change, length.saturating_sub(1)); + if length == 0 { + clear_cache = true; + } else { + index_of_first_change = min(index_of_first_change, length.saturating_sub(1)); + index_of_last_change = usize::MAX; + } timeline_items.truncate(length); + if LOG_DIFFS { log!("timeline_subscriber: diff Truncate to length {length}. Changes: {index_of_first_change}..{index_of_last_change}"); } + num_updates += 1; } VectorDiff::Reset { values } => { if LOG_DIFFS { log!("timeline_subscriber: diff Reset, new length {}", values.len()); } - index_of_first_change = 0; // we must assume that all items have changed. + clear_cache = true; // we must assume all items have changed. timeline_items = values; + num_updates += 1; } } } - if num_updates > 0 { - log!("timeline_subscriber: applied {num_updates} updates for room {room_id}; first change at index {index_of_first_change}."); - - sender.send(TimelineUpdate::NewItems { - items: timeline_items.clone(), - index_of_first_change, - }).expect("Error: timeline update sender couldn't send update with new items!"); - - // Send a Makepad-level signal to update this room's timeline UI view. - SignalToUI::set_ui_signal(); - } + send_update(timeline_items.clone(), index_of_first_change..index_of_last_change, clear_cache, num_updates); } error!("Error: unexpectedly ended timeline subscriber for room {room_id}.");