From 9bc6f3519d625f1ee9ae158df72d93cfb35a1134 Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Fri, 11 Oct 2024 19:10:59 -0700 Subject: [PATCH 1/2] Completely redesign pagination code, support backwards pagination upon scroll Remove the separate pagination state subscriber task, which was previously being spawned once for every room. That was unnecessary and actually provided less information than just waiting for the result of the original task that actually called the paginate function. Store the most-recently-scrolled-through item index in `TimelineUiState` such that we can determine whether when the user is scrolling upwards and has just hit the top-most item, at which point we fire off a backwards pagination request. --- Cargo.lock | 93 ++++++++++------ Cargo.toml | 4 +- src/home/room_screen.rs | 237 +++++++++++++++++++++++----------------- src/home/rooms_list.rs | 11 +- src/sliding_sync.rs | 109 +++++++++--------- 5 files changed, 252 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 19d146a5..8a108a70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1621,7 +1621,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -1741,7 +1741,7 @@ dependencies = [ [[package]] name = "makepad-derive-live" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", "makepad-micro-proc-macro", @@ -1750,7 +1750,7 @@ dependencies = [ [[package]] name = "makepad-derive-wasm-bridge" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-micro-proc-macro", ] @@ -1758,7 +1758,7 @@ dependencies = [ [[package]] name = "makepad-derive-widget" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", "makepad-micro-proc-macro", @@ -1767,7 +1767,7 @@ dependencies = [ [[package]] name = "makepad-draw" version = "0.6.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "ab_glyph_rasterizer", "fxhash", @@ -1784,17 +1784,17 @@ dependencies = [ [[package]] name = "makepad-futures" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-futures-legacy" version = "0.7.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-html" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", ] @@ -1802,7 +1802,7 @@ dependencies = [ [[package]] name = "makepad-http" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-jni-sys" @@ -1813,7 +1813,7 @@ checksum = "9775cbec5fa0647500c3e5de7c850280a88335d1d2d770e5aa2332b801ba7064" [[package]] name = "makepad-live-compiler" version = "0.5.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-derive-live", "makepad-live-tokenizer", @@ -1823,7 +1823,7 @@ dependencies = [ [[package]] name = "makepad-live-id" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id-macros", ] @@ -1831,7 +1831,7 @@ dependencies = [ [[package]] name = "makepad-live-id-macros" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-micro-proc-macro", ] @@ -1839,7 +1839,7 @@ dependencies = [ [[package]] name = "makepad-live-tokenizer" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", "makepad-math", @@ -1849,7 +1849,7 @@ dependencies = [ [[package]] name = "makepad-markdown" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", ] @@ -1857,17 +1857,17 @@ dependencies = [ [[package]] name = "makepad-math" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-micro-proc-macro" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-micro-serde" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-id", "makepad-micro-serde-derive", @@ -1876,7 +1876,7 @@ dependencies = [ [[package]] name = "makepad-micro-serde-derive" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-micro-proc-macro", ] @@ -1884,12 +1884,12 @@ dependencies = [ [[package]] name = "makepad-objc-sys" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "makepad-platform" version = "0.6.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "hilog-sys", "makepad-android-state", @@ -1911,7 +1911,7 @@ dependencies = [ [[package]] name = "makepad-rustybuzz" version = "0.8.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "bitflags 1.3.2", "bytemuck", @@ -1926,7 +1926,7 @@ dependencies = [ [[package]] name = "makepad-shader-compiler" version = "0.5.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-live-compiler", ] @@ -1934,7 +1934,7 @@ dependencies = [ [[package]] name = "makepad-vector" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "resvg", "ttf-parser", @@ -1943,7 +1943,7 @@ dependencies = [ [[package]] name = "makepad-wasm-bridge" version = "0.4.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-derive-wasm-bridge", "makepad-live-id", @@ -1952,7 +1952,7 @@ dependencies = [ [[package]] name = "makepad-widgets" version = "0.6.0" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-derive-widget", "makepad-draw", @@ -1966,16 +1966,16 @@ dependencies = [ [[package]] name = "makepad-windows" version = "0.51.1" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ - "windows-core 0.51.1 (git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path)", + "windows-core 0.51.1 (git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self)", "windows-targets 0.48.5", ] [[package]] name = "makepad-zune-core" version = "0.2.14" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "bitflags 2.6.0", ] @@ -1983,7 +1983,7 @@ dependencies = [ [[package]] name = "makepad-zune-inflate" version = "0.2.54" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "simd-adler32", ] @@ -1991,7 +1991,7 @@ dependencies = [ [[package]] name = "makepad-zune-jpeg" version = "0.3.17" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-zune-core", ] @@ -1999,7 +1999,7 @@ dependencies = [ [[package]] name = "makepad-zune-png" version = "0.2.1" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "makepad-zune-core", "makepad-zune-inflate", @@ -3105,20 +3105,21 @@ dependencies = [ "objc2-core-location", "objc2-foundation", "robius-android-env", - "windows", + "windows 0.57.0", ] [[package]] name = "robius-open" -version = "0.1.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b4c6634e8febd0be2e37aefd55fd256f9f39eb5583db30c7163132aa625bfda" +checksum = "563849989e82ed61036c169afe32f95d6683b944cb667986bc6735103c69a64a" dependencies = [ "cfg-if", "icrate", "jni", "objc2", "robius-android-env", + "windows 0.54.0", ] [[package]] @@ -4036,7 +4037,7 @@ checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" [[package]] name = "ttf-parser" version = "0.21.1" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" [[package]] name = "typenum" @@ -4430,6 +4431,16 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.54.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9252e5725dbed82865af151df558e754e4a3c2c30818359eb17465f1346a1b49" +dependencies = [ + "windows-core 0.54.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows" version = "0.57.0" @@ -4452,11 +4463,21 @@ dependencies = [ [[package]] name = "windows-core" version = "0.51.1" -source = "git+https://github.com/kevinaboos/makepad?branch=apple_bundle_resource_path#7e70055ab2fda14746374d02ca7ae70f804c40de" +source = "git+https://github.com/kevinaboos/makepad?branch=portal_list_ref_immut_self#a18fde6784e71987852abaa8278d5b9e1bf9d610" dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "windows-core" +version = "0.54.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12661b9c89351d684a50a8a643ce5f608e20243b9fb84687800163429f161d65" +dependencies = [ + "windows-result 0.1.2", + "windows-targets 0.52.6", +] + [[package]] name = "windows-core" version = "0.57.0" diff --git a/Cargo.toml b/Cargo.toml index ed016d84..9f6d6396 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,12 +17,12 @@ metadata.makepad-auto-version = "zqpv-Yj-K7WNVK2I8h5Okhho46Q=" [dependencies] # makepad-widgets = { git = "https://github.com/makepad/makepad", branch = "rik" } -makepad-widgets = { git = "https://github.com/kevinaboos/makepad", branch = "apple_bundle_resource_path" } +makepad-widgets = { git = "https://github.com/kevinaboos/makepad", branch = "portal_list_ref_immut_self" } ## Including this crate automatically configures all `robius-*` crates to work with Makepad. robius-use-makepad = "0.1.0" -robius-open = "0.1.0" +robius-open = "0.1.1" ## A fork of the `directories` crate that adds support for Android by using our `robius-android-env` crate. robius-directories = { git = "https://github.com/project-robius/robius-directories", branch = "robius"} robius-location = { git = "https://github.com/project-robius/robius-location" } diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index 179fd9b0..4ca31f25 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -33,7 +33,7 @@ use crate::{ html_or_plaintext::{HtmlOrPlaintextRef, HtmlOrPlaintextWidgetRefExt}, text_or_image::{TextOrImageRef, TextOrImageWidgetRefExt}, typing_animation::TypingAnimationWidgetExt, - }, sliding_sync::{get_client, submit_async_request, take_timeline_update_receiver, MatrixRequest}, utils::{self, unix_time_millis_to_datetime, MediaFormatConst} + }, sliding_sync::{get_client, submit_async_request, take_timeline_update_receiver, MatrixRequest, PaginationDirection}, utils::{self, unix_time_millis_to_datetime, MediaFormatConst} }; use rangemap::RangeSet; @@ -1007,86 +1007,17 @@ impl Drop for RoomScreen { } } -impl RoomScreen{ - fn send_user_read_receipts_based_on_scroll_pos( - &mut self, - cx: &mut Cx, - actions: &ActionsBuf, - ) { - let portal_list = self.portal_list(id!(list)); - //stopped scrolling - if portal_list.scrolled(actions) { - return; - } - let first_index = portal_list.first_id(); - - let Some(tl_state) = self.tl_state.as_mut() else { return }; - let Some(room_id) = self.room_id.as_ref() else { return }; - if let Some(ref mut index) = tl_state.prev_first_index { - // to detect change of scroll when scroll ends - if *index != first_index { - // scroll changed - self.fully_read_timer = cx.start_interval(5.0); - let time_now = std::time::Instant::now(); - if first_index > *index { - // Store visible event messages with current time into a hashmap - let mut read_receipt_event = None; - for r in first_index .. (first_index + portal_list.visible_items() + 1) { - if let Some(v) = tl_state.items.get(r) { - if let Some(e) = v.as_event().and_then(|f| f.event_id()) { - read_receipt_event = Some(e.to_owned()); - if !tl_state.read_event_hashmap.contains_key(&e.to_string()) { - tl_state.read_event_hashmap.insert( - e.to_string(), - (room_id.clone(), e.to_owned(), time_now, false), - ); - } - } - } - } - if let Some(event_id) = read_receipt_event { - submit_async_request(MatrixRequest::ReadReceipt { room_id: room_id.clone(), event_id }); - } - let mut fully_read_receipt_event = None; - // Implements sending fully read receipts when message is scrolled out of first row - for r in *index..first_index { - if let Some(v) = tl_state.items.get(r).clone() { - if let Some(e) = v.as_event().and_then(|f| f.event_id()) { - let mut to_remove = vec![]; - for (event_id_string, (_, event_id)) in &tl_state.marked_fully_read_queue { - if e == event_id { - fully_read_receipt_event = Some(event_id.clone()); - to_remove.push(event_id_string.clone()); - } - } - for r in to_remove { - tl_state.marked_fully_read_queue.remove(&r); - } - } - } - } - if let Some(event_id) = fully_read_receipt_event { - submit_async_request(MatrixRequest::FullyReadReceipt { room_id: room_id.clone(), event_id: event_id.clone()}); - } - } - *index = first_index; - } - } else { - tl_state.prev_first_index = Some(first_index); - } - } -} - impl Widget for RoomScreen { // Handle events and actions for the RoomScreen widget and its inner Timeline view. fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) { let widget_uid = self.widget_uid(); + let portal_list = self.portal_list(id!(timeline.list)); let pane = self.user_profile_sliding_pane(id!(user_profile_sliding_pane)); // 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 { - self.process_timeline_updates(cx); + self.process_timeline_updates(cx, &portal_list); } if let Event::Actions(actions) = event { @@ -1108,7 +1039,6 @@ impl Widget for RoomScreen { } } MessageAction::ReplyPreviewClicked { reply_message_item_id, replied_to_event } => { - let mut portal_list = self.portal_list(id!(list)); let Some(tl) = self.tl_state.as_mut() else { continue; }; @@ -1149,7 +1079,6 @@ impl Widget for RoomScreen { } // Handle the highlight animation. - let portal_list = self.portal_list(id!(list)); let Some(tl) = self.tl_state.as_mut() else { return }; if let MessageHighlightAnimationState::Pending { item_id } = tl.message_highlight_animation_state { if portal_list.smooth_scroll_reached(actions) { @@ -1260,9 +1189,9 @@ impl Widget for RoomScreen { } // Set visibility of loading message banner based of pagination logic - self.send_pagination_request_based_on_scroll_pos(cx, actions); + self.send_pagination_request_based_on_scroll_pos(cx, actions, &portal_list); // Handle sending any read receipts for the current logged-in user. - self.send_user_read_receipts_based_on_scroll_pos(cx, actions); + self.send_user_read_receipts_based_on_scroll_pos(cx, actions, &portal_list); // Handle the cancel reply button being clicked. if self.button(id!(cancel_reply_button)).clicked(&actions) { @@ -1334,7 +1263,6 @@ impl Widget for RoomScreen { // Handle the jump to bottom button: update its visibility, and handle clicks. { - let mut portal_list = self.portal_list(id!(timeline.list)); let jump_to_bottom_view = self.view(id!(jump_to_bottom_view)); if portal_list.scrolled(&actions) { // TODO: is_at_end() isn't perfect, see: @@ -1520,8 +1448,7 @@ impl RoomScreen { /// Processes all pending background updates to the currently-shown timeline. /// /// Redraws this RoomScreen view if any updates were applied. - fn process_timeline_updates(&mut self, cx: &mut Cx) { - let portal_list = self.portal_list(id!(list)); + fn process_timeline_updates(&mut self, cx: &mut Cx, portal_list: &PortalListRef) { let top_space = self.view(id!(top_space)); let curr_first_id = portal_list.first_id(); let Some(tl) = self.tl_state.as_mut() else { return }; @@ -1609,12 +1536,24 @@ impl RoomScreen { tl.items = new_items; done_loading = true; } - TimelineUpdate::TimelineStartReached => { - log!("Timeline::handle_event(): timeline start reached for room {}", tl.room_id); - tl.fully_paginated = true; + TimelineUpdate::PaginationRunning(direction) => { + if direction == PaginationDirection::Backwards { + top_space.set_visible(true); + } else { + error!("Unexpected PaginationRunning update in the Forwards direction"); + } } - TimelineUpdate::PaginationIdle => { - top_space.set_visible(true); + TimelineUpdate::PaginationError { error, direction } => { + error!("Pagination error ({direction}) in room {}: {error:?}", tl.room_id); + done_loading = true; + } + TimelineUpdate::PaginationIdle { fully_paginated, direction } => { + if direction == PaginationDirection::Backwards { + done_loading = true; + tl.fully_paginated = fully_paginated; + } else { + error!("Unexpected PaginationIdle update in the Forwards direction"); + } } TimelineUpdate::EventDetailsFetched {event_id, result } => { if let Err(_e) = result { @@ -1624,7 +1563,7 @@ impl RoomScreen { // but for now we just fall through and let the final `redraw()` call re-draw the whole timeline view. } TimelineUpdate::RoomMembersFetched => { - log!("Timeline::handle_event(): room members fetched for room {}", tl.room_id); + // log!("Timeline::handle_event(): room members fetched for room {}", tl.room_id); // Here, to be most efficient, we could redraw only the user avatars and names in the timeline, // but for now we just fall through and let the final `redraw()` call re-draw the whole timeline view. } @@ -1769,6 +1708,7 @@ impl RoomScreen { replying_to: None, saved_state: SavedState::default(), message_highlight_animation_state: MessageHighlightAnimationState::default(), + last_scrolled_index: usize::MAX, prev_first_index: None, read_event_hashmap: HashMap::new(), marked_fully_read_queue: HashMap::new(), @@ -1790,7 +1730,7 @@ impl RoomScreen { submit_async_request(MatrixRequest::PaginateRoomTimeline { room_id: room_id.clone(), num_events: 50, - forwards: false, + direction: PaginationDirection::Backwards, }) } else { // log!("Note: skipping pagination request for room {} because it is already fully paginated.", room_id); @@ -1814,7 +1754,8 @@ impl RoomScreen { // Now we can process any background updates and redraw the timeline. if first_time_showing_room { - self.process_timeline_updates(cx); + let portal_list = self.portal_list(id!(list)); + self.process_timeline_updates(cx, &portal_list); } self.redraw(cx); @@ -1906,24 +1847,100 @@ impl RoomScreen { self.show_timeline(cx); self.label(id!(room_name)).set_text(&self.room_name); } - - /// Send Pagination Request when the scroll position is at the top - fn send_pagination_request_based_on_scroll_pos( + + /// Sends read receipts based on the current scroll position of the timeline. + fn send_user_read_receipts_based_on_scroll_pos( &mut self, cx: &mut Cx, actions: &ActionsBuf, + portal_list: &PortalListRef, ) { - let portal_list = self.portal_list(id!(list)); - //stopped scrolling and when scroll position is at top + //stopped scrolling if portal_list.scrolled(actions) { return; } - + let first_index = portal_list.first_id(); + + let Some(tl_state) = self.tl_state.as_mut() else { return }; let Some(room_id) = self.room_id.as_ref() else { return }; - if portal_list.scroll_position() == 0.0 { - submit_async_request(MatrixRequest::PaginateRoomTimeline { room_id: room_id.clone(), num_events: 50, forwards: false}); + if let Some(ref mut index) = tl_state.prev_first_index { + // to detect change of scroll when scroll ends + if *index != first_index { + // scroll changed + self.fully_read_timer = cx.start_interval(5.0); + let time_now = std::time::Instant::now(); + if first_index > *index { + // Store visible event messages with current time into a hashmap + let mut read_receipt_event = None; + for r in first_index .. (first_index + portal_list.visible_items() + 1) { + if let Some(v) = tl_state.items.get(r) { + if let Some(e) = v.as_event().and_then(|f| f.event_id()) { + read_receipt_event = Some(e.to_owned()); + if !tl_state.read_event_hashmap.contains_key(&e.to_string()) { + tl_state.read_event_hashmap.insert( + e.to_string(), + (room_id.clone(), e.to_owned(), time_now, false), + ); + } + } + } + } + if let Some(event_id) = read_receipt_event { + submit_async_request(MatrixRequest::ReadReceipt { room_id: room_id.clone(), event_id }); + } + let mut fully_read_receipt_event = None; + // Implements sending fully read receipts when message is scrolled out of first row + for r in *index..first_index { + if let Some(v) = tl_state.items.get(r).clone() { + if let Some(e) = v.as_event().and_then(|f| f.event_id()) { + let mut to_remove = vec![]; + for (event_id_string, (_, event_id)) in &tl_state.marked_fully_read_queue { + if e == event_id { + fully_read_receipt_event = Some(event_id.clone()); + to_remove.push(event_id_string.clone()); + } + } + for r in to_remove { + tl_state.marked_fully_read_queue.remove(&r); + } + } + } + } + if let Some(event_id) = fully_read_receipt_event { + submit_async_request(MatrixRequest::FullyReadReceipt { room_id: room_id.clone(), event_id: event_id.clone()}); + } + } + *index = first_index; + } + } else { + tl_state.prev_first_index = Some(first_index); } - + } + + /// Sends a backwards pagination request if the user is scrolling up + /// and is approaching the top of the timeline. + fn send_pagination_request_based_on_scroll_pos( + &mut self, + _cx: &mut Cx, + actions: &ActionsBuf, + portal_list: &PortalListRef, + ) { + let Some(tl) = self.tl_state.as_mut() else { return }; + if tl.fully_paginated { return }; + if !portal_list.scrolled(actions) { return }; + + let first_index = portal_list.first_id(); + if first_index == 0 && tl.last_scrolled_index > 0 { + log!("Scrolled up from item {} --> 0, sending back pagination request for room {}", + tl.last_scrolled_index, tl.room_id, + ); + submit_async_request(MatrixRequest::PaginateRoomTimeline { + room_id: tl.room_id.clone(), + num_events: 50, + direction: PaginationDirection::Backwards, + }); + } + tl.last_scrolled_index = first_index; } } @@ -1935,7 +1952,6 @@ 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 { @@ -1951,13 +1967,22 @@ pub enum TimelineUpdate { /// 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. - TimelineStartReached, + /// A notice that the background task doing pagination for this room is currently running + /// a pagination request in the given direction, and is waiting for that request to complete. + PaginationRunning(PaginationDirection), + /// An error occurred while paginating the timeline for this room. + PaginationError { + error: timeline::Error, + direction: PaginationDirection, + }, /// A notice that the background task doing pagination for this room has become idle, - /// meaning that it has completed its recent pagination request(s) and is now waiting - /// for more requests, but that the start of the timeline has not yet been reached. - PaginationIdle, + /// meaning that it has completed its recent pagination request(s). + PaginationIdle { + /// If `true`, the start of the timeline has been reached, meaning that + /// there is no need to send further pagination requests. + fully_paginated: bool, + direction: PaginationDirection, + }, /// A notice that event details have been fetched from the server, /// including a `result` that indicates whether the request was successful. EventDetailsFetched { @@ -2042,6 +2067,12 @@ struct TimelineUiState { /// If the animation was trigged, the state goes back to Off. message_highlight_animation_state: MessageHighlightAnimationState, + /// The index of the timeline item that was most recently scrolled up past it. + /// This is used to detect when the user has scrolled up past the second visible item (index 1) + /// upwards to the first visible item (index 0), which is the top of the timeline, + /// at which point we submit a backwards pagination request to fetch more events. + last_scrolled_index: usize, + prev_first_index: Option, read_event_hashmap: HashMap, marked_fully_read_queue: HashMap, diff --git a/src/home/rooms_list.rs b/src/home/rooms_list.rs index 54797d0c..8d75b67a 100644 --- a/src/home/rooms_list.rs +++ b/src/home/rooms_list.rs @@ -3,10 +3,15 @@ use crossbeam_queue::SegQueue; use makepad_widgets::*; use matrix_sdk::ruma::{MilliSecondsSinceUnixEpoch, OwnedRoomId}; -use crate::{app::AppState, sliding_sync::{submit_async_request, MatrixRequest}}; +use crate::{app::AppState, sliding_sync::{submit_async_request, MatrixRequest, PaginationDirection}}; use super::room_preview::RoomPreviewAction; +/// Whether to pre-paginate visible rooms at least once in order to +/// be able to display the latest message in the room preview, +/// and to have something to immediately show when a user first opens a room. +const PREPAGINATE_VISIBLE_ROOMS: bool = true; + live_design! { import makepad_draw::shader::std::*; import makepad_widgets::view::*; @@ -313,12 +318,12 @@ impl Widget for RoomsList { room_info.is_selected = self.current_active_room_index == Some(item_id); // Paginate the room if it hasn't been paginated yet. - if !room_info.has_been_paginated { + if PREPAGINATE_VISIBLE_ROOMS && !room_info.has_been_paginated { room_info.has_been_paginated = true; submit_async_request(MatrixRequest::PaginateRoomTimeline { room_id: room_info.room_id.clone(), num_events: 50, - forwards: false, + direction: PaginationDirection::Backwards, }); } diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs index 32d5b064..158f24dd 100644 --- a/src/sliding_sync.rs +++ b/src/sliding_sync.rs @@ -26,13 +26,12 @@ use matrix_sdk::{ use matrix_sdk_ui::{ room_list_service::{self, RoomListLoadingState}, sync_service::{self, SyncService}, - timeline::{AnyOtherFullStateEventContent, EventTimelineItem, LiveBackPaginationStatus, RepliedToInfo, TimelineDetails, TimelineItemContent}, + timeline::{AnyOtherFullStateEventContent, EventTimelineItem, RepliedToInfo, TimelineDetails, TimelineItemContent}, Timeline, }; use tokio::{ runtime::Handle, sync::mpsc::{UnboundedSender, UnboundedReceiver}, - task::JoinHandle, }; use unicode_segmentation::UnicodeSegmentation; use std::{cmp::{max, min}, collections::{BTreeMap, BTreeSet}, path:: Path, sync::{Arc, Mutex, OnceLock}}; @@ -166,6 +165,26 @@ async fn login(cli: Cli) -> Result<(Client, Option)> { } +/// Which direction to paginate in. +/// +/// * `Forwards` will retrieve later events (towards the end of the timeline), +/// which only works if the timeline is *focused* on a specific event. +/// * `Backwards`: the more typical choice, in which earlier events are retrieved +/// (towards the start of the timeline), which works in both live mode and focused mode. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PaginationDirection { + Forwards, + Backwards, +} +impl std::fmt::Display for PaginationDirection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Forwards => write!(f, "forwards"), + Self::Backwards => write!(f, "backwards"), + } + } +} + /// The set of requests for async work that can be made to the worker thread. pub enum MatrixRequest { @@ -174,12 +193,7 @@ pub enum MatrixRequest { room_id: OwnedRoomId, /// The maximum number of timeline events to fetch in each pagination batch. num_events: u16, - /// Which "direction" to paginate in: - /// * `true`: paginate forwards to retrieve later events (towards the end of the timeline), - /// which only works if the timeline is *focused* on a specific event. - /// * `false`: (default) paginate backwards to fill in earlier events (towards the start of the timeline), - /// which works if the timeline is in either live mode and focused mode. - forwards: bool, + direction: PaginationDirection, }, /// Request to fetch the full details of the given event in the given room's timeline. FetchDetailsForEvent { @@ -285,69 +299,51 @@ async fn async_worker(mut receiver: UnboundedReceiver) -> Result< while let Some(request) = receiver.recv().await { match request { - MatrixRequest::PaginateRoomTimeline { room_id, num_events, forwards } => { - let timeline = { + MatrixRequest::PaginateRoomTimeline { room_id, num_events, direction } => { + let (timeline, sender) = { let mut all_room_info = ALL_ROOM_INFO.lock().unwrap(); let Some(room_info) = all_room_info.get_mut(&room_id) else { log!("Skipping pagination request for not-yet-known room {room_id}"); continue; }; - let room_id2 = room_id.clone(); let timeline_ref = room_info.timeline.clone(); - let timeline_ref2 = timeline_ref.clone(); let sender = room_info.timeline_update_sender.clone(); - - // If the back-pagination status task is finished or doesn't exist, spawn a new one, - // but only if the timeline is not already fully paginated. - let should_spawn_pagination_status_task = match room_info.pagination_status_task.as_ref() { - Some(t) => t.is_finished(), - None => true, - }; - if should_spawn_pagination_status_task { - room_info.pagination_status_task = Some(Handle::current().spawn( async move { - if let Some((pagination_status, mut pagination_stream)) = timeline_ref2.live_back_pagination_status().await { - if !matches!(pagination_status, LiveBackPaginationStatus::Idle { hit_start_of_timeline: true }) { - while let Some(status) = pagination_stream.next().await { - log!("### Timeline {room_id2} back pagination status: {:?}", status); - match status { - LiveBackPaginationStatus::Idle { hit_start_of_timeline: false } => { - sender.send(TimelineUpdate::PaginationIdle).unwrap(); - SignalToUI::set_ui_signal(); - } - LiveBackPaginationStatus::Idle { hit_start_of_timeline: true } => { - sender.send(TimelineUpdate::TimelineStartReached).unwrap(); - SignalToUI::set_ui_signal(); - break; - } - _ => { } - } - } - } - } - })); - } - - // drop the lock on ALL_ROOM_INFO before spawning the actual pagination task. - timeline_ref + (timeline_ref, sender) }; // Spawn a new async task that will make the actual pagination request. let _paginate_task = Handle::current().spawn(async move { - let direction = if forwards { "forwards" } else { "backwards" }; log!("Sending {direction} pagination request for room {room_id}..."); - let res = if forwards { + sender.send(TimelineUpdate::PaginationRunning(direction)).unwrap(); + SignalToUI::set_ui_signal(); + + let res = if direction == PaginationDirection::Forwards { timeline.focused_paginate_forwards(num_events).await } else { timeline.paginate_backwards(num_events).await }; + match res { - Ok(_hit_start_or_end) => log!( - "Completed {direction} pagination request for room {room_id}, hit {} of timeline? {}", - if forwards { "end" } else { "start" }, - if _hit_start_or_end { "yes" } else { "no" }, - ), - Err(e) => error!("Error sending {direction} pagination request for room {room_id}: {e:?}"), + Ok(fully_paginated) => { + log!("Completed {direction} pagination request for room {room_id}, hit {} of timeline? {}", + if direction == PaginationDirection::Forwards { "end" } else { "start" }, + if fully_paginated { "yes" } else { "no" }, + ); + sender.send(TimelineUpdate::PaginationIdle { + fully_paginated, + direction, + }).unwrap(); + SignalToUI::set_ui_signal(); + } + Err(error) => { + error!("Error sending {direction} pagination request for room {room_id}: {error:?}"); + sender.send(TimelineUpdate::PaginationError { + error, + direction, + }).unwrap(); + SignalToUI::set_ui_signal(); + } } }); } @@ -507,7 +503,7 @@ async fn async_worker(mut receiver: UnboundedReceiver) -> Result< submit_async_request(MatrixRequest::PaginateRoomTimeline { room_id, num_events: 50, - forwards: false, + direction: PaginationDirection::Backwards, }); }); } @@ -765,8 +761,6 @@ struct RoomInfo { /// The UI thread can take ownership of these items receiver in order for a specific /// timeline view (currently room_sccren) to receive and display updates to this room's timeline. timeline_update_receiver: Option>, - /// The async task that is subscribed to the timeline's back-pagination status. - pagination_status_task: Option>, /// A drop guard for the event handler that represents a subscription to typing notices for this room. typing_notice_subscriber: Option, } @@ -1108,7 +1102,6 @@ async fn add_new_room(room: &room_list_service::Room) -> Result<()> { timeline, timeline_update_receiver: Some(timeline_update_receiver), timeline_update_sender, - pagination_status_task: None, typing_notice_subscriber: None, }, ); @@ -1158,7 +1151,7 @@ fn handle_ignore_user_list_subscriber(client: Client) { submit_async_request(MatrixRequest::PaginateRoomTimeline { room_id: joined_room.room_id().to_owned(), num_events: 50, - forwards: false, + direction: PaginationDirection::Backwards, }); } } From f25b9e266e8c9b0a26e4a8efe6fd791383b7926e Mon Sep 17 00:00:00 2001 From: Kevin Boos Date: Fri, 11 Oct 2024 20:49:37 -0700 Subject: [PATCH 2/2] Only paginate automatically the first time a room is opened Clean up various pagination code. Re-format the TopSpace (for pagination loading messages) to be a lightly-transparent green, and to be an Overlay atop the timeline such that it doesn't affect the positional stability of timeline items. Refactor RoomScreen event-handling functions to remove redundant calls to `self.portal_list(id!(timeline.list))`. We now call this once at the top of the `RoomScreen::handle_event()` in order to have a single `PortalListRef` that can be used across all the related event handling functions. --- src/home/room_screen.rs | 47 +++++++++++++++++++---------------------- src/sliding_sync.rs | 2 +- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index 4ca31f25..360b528c 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -597,25 +597,27 @@ live_design! { } - - // The top space is used to display a loading animation while the room is being paginated. + // The top space is used to display a loading message while the room is being paginated. TopSpace = { visible: false, width: Fill, height: Fit, - align: {x: 0.5, y: 0.5} + align: {x: 0.5, y: 0} show_bg: true, draw_bg: { - color: #ebfcf2, + color: #xDAF5E5F0, // mostly opaque light green } label =