Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an end bound for invalidating the cache of drawn timeline items #49

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Add an end bound for invalidating the cache of drawn timeline items #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 21 additions & 37 deletions src/home/room_screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -604,10 +604,13 @@ pub enum TimelineUpdate {
NewItems {
/// The entire list of timeline items (events) for a room.
items: Vector<Arc<TimelineItem>>,
/// 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<usize>,
/// 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.
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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`.
Expand Down
99 changes: 69 additions & 30 deletions src/sliding_sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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};
Expand Down Expand Up @@ -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<Arc<TimelineItem>>, changed_indices: Range<usize>, 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}.");
Expand Down