Skip to content

Commit

Permalink
Stable positioning of timeline items across background updates is wor…
Browse files Browse the repository at this point in the history
…king.

However, it could be more efficient by having the background timeline subscriber task
include some information about where the changes were made,
which would allow us to avoid searching the entire timeline list for the ID of
the event that was previously shown at the top.

Also added some infra for avoiding redundant drawing work based on
whether a timeline item has been changed in the background,
but that is not yet being used.
  • Loading branch information
kevinaboos committed Feb 15, 2024
1 parent 6ea6e8b commit d24d0fb
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 50 deletions.
58 changes: 29 additions & 29 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ metadata.makepad-auto-version = "zqpv-Yj-K7WNVK2I8h5Okhho46Q="


[dependencies]
makepad-widgets = { git = "https://github.com/makepad/makepad", branch = "rik" }
# makepad-widgets = { git = "https://github.com/makepad/makepad", branch = "rik" }
makepad-widgets = { git = "https://github.com/kevinaboos/makepad", branch = "extend_portal_list_item_api" }


anyhow = "1.0"
Expand Down
49 changes: 39 additions & 10 deletions src/home/room_screen.rs
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::{ops::DerefMut, sync::{Arc, Mutex}, collections::BTreeMap};
use std::{collections::BTreeMap, ops::{DerefMut, RangeInclusive}, sync::{Arc, Mutex}};

use imbl::Vector;
use makepad_widgets::*;
Expand Down Expand Up @@ -602,9 +602,15 @@ impl RoomScreenRef {
/// A message that is sent from a background async task to a room's timeline view
/// for the purpose of update the Timeline UI contents or metadata.
pub enum TimelineUpdate {
/// A update containing the entire list of timeline items for a room,
/// indicating that it has been updated in the background.
NewItems(Vector<Arc<TimelineItem>>),
/// The content of a room's timeline was updated in the background.
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,
},
/// A notice that the start of the timeline has been reached, meaning that
/// there is no need to send further backwards pagination requests.
TimelineStartReached,
Expand Down Expand Up @@ -641,9 +647,15 @@ struct TimelineUiState {
/// When `true`, further backwards pagination requests will not be sent.
fully_paginated: bool,

/// The list of items (events) in this room's timeline that our client currently knows about.
/// The list of items (events) in this room's timeline that our client currently knows about,
/// and the display state of each item (whether it needs to be redrawn).
items: Vector<Arc<TimelineItem>>,

/// The range of items that have been updated since the last time the timeline was drawn.
/// This range is set on each background update to ensure that no changes items are missed;
/// thus, it is a conservative estimate that may include more items than necessary.
_updated_items: RangeInclusive<usize>,

/// The channel receiver for timeline updates for this room.
///
/// Here we use a synchronous (non-async) channel because the receiver runs
Expand Down Expand Up @@ -714,6 +726,7 @@ impl TimelineRef {
// We assume timelines being viewed for the first time haven't been fully paginated.
fully_paginated: false,
items: Vector::new(),
updated_items: usize::MIN ..= usize::MAX,
update_receiver,
media_cache: MediaCache::new(MediaFormatConst::File),
saved_state: SavedState::default(),
Expand Down Expand Up @@ -750,12 +763,27 @@ impl Widget for Timeline {
// Currently, a Signal event is only used to tell this widget that its timeline events
// have been updated in the background.
if let Event::Signal = event {
let portal_list = self.portal_list(id!(list));
let Some(tl) = self.tl_state.as_mut() else { return };
let mut done_loading = false;
while let Ok(update) = tl.update_receiver.try_recv() {
match update {
TimelineUpdate::NewItems(items) => {
TimelineUpdate::NewItems { items, index_of_first_change } => {
// 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.
let first_id = portal_list.first_id();
if let Some(top_event_id) = tl.items.get(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 from top event index {first_id} to index {idx}");
portal_list.set_first_id(idx);
break;
}
}
}
tl.items = items;

log!("Timeline::handle_event(): TODO: use index_of_first_change: {index_of_first_change}");
}
TimelineUpdate::TimelineStartReached => {
log!("Timeline::handle_event(): timeline start reached for room {}", tl.room_id);
Expand Down Expand Up @@ -958,7 +986,7 @@ fn populate_message_view(
} else {
live_id!(Message)
};
let item = list.item(cx, item_id, template).unwrap();
let (item, _existed) = list.item_with_existed(cx, item_id, template).unwrap();
item.label(id!(content.message)).set_text(&text.body);
item
}
Expand All @@ -984,7 +1012,7 @@ fn populate_message_view(
} else {
live_id!(ImageMessage)
};
let item = list.item(cx, item_id, template).unwrap();
let (item, _existed) = list.item_with_existed(cx, item_id, template).unwrap();

let img_ref = item.image(id!(body.content.message));
if let Some(data) = media_cache.try_get_media_or_fetch(mxc_uri, None) {
Expand Down Expand Up @@ -1166,7 +1194,8 @@ fn populate_membership_change_view(
format!("denied {}'s request to join this room.", change_user_id),
};

let item = list.item(cx, item_id, live_id!(SmallStateEvent)).unwrap();
let (item, _existed) = list.item_with_existed(cx, item_id, live_id!(SmallStateEvent)).unwrap();

set_timestamp(&item, id!(left_container.timestamp), event_tl_item.timestamp());
let username = set_avatar_and_get_username(
cx,
Expand All @@ -1192,7 +1221,7 @@ fn populate_profile_change_view(
event_tl_item: &EventTimelineItem,
change: &MemberProfileChange,
) -> WidgetRef {
let item = list.item(cx, item_id, live_id!(SmallStateEvent)).unwrap();
let (item, _existed) = list.item_with_existed(cx, item_id, live_id!(SmallStateEvent)).unwrap();
let username = set_avatar_and_get_username(
cx,
item.avatar(id!(avatar)),
Expand Down
Loading

0 comments on commit d24d0fb

Please sign in to comment.