Przeglądaj źródła

app: fix broken scrollback buffer loading

darkfi 1 tydzień temu
rodzic
commit
401f89603a

+ 22 - 5
bin/app/src/ui/chatview/buffer.rs

@@ -328,11 +328,23 @@ impl MsgBuffer {
         self.order.iter().rev().filter_map(|slot| self.records.get(*slot))
     }
 
-    /// The oldest loaded timestamp: the loader's resume point. None
-    /// when nothing is loaded.
+    /// The composite key of the oldest loaded *stored* record.
+    /// Derived records are excluded: a day separator's key is its day's
+    /// local midnight, which sorts below every message of that day, so
+    /// using it as the loader's resume point would skip the whole
+    /// unloaded remainder of the day. None when nothing is loaded.
+    pub fn oldest_key(&self) -> Option<(Timestamp, MessageId)> {
+        let slot = *self.order.iter().find(|slot| {
+            !self.records.get(**slot).expect("dangling slot in order").msg_type.is_derived()
+        })?;
+        let rec = self.records.get(slot).expect("dangling slot in order");
+        Some((rec.ts, rec.id))
+    }
+
+    /// The oldest loaded stored-record timestamp. None when nothing is
+    /// loaded.
     pub fn oldest_ts(&self) -> Option<Timestamp> {
-        let first = self.order.first()?;
-        Some(self.records.get(*first).expect("dangling slot in order").ts)
+        self.oldest_key().map(|(ts, _)| ts)
     }
 
     /// Total px of loaded content.
@@ -719,7 +731,12 @@ mod tests {
         // public removal go through real message ids only.
         assert!(!buf.contains(&MessageId([0; 32])));
         assert!(!buf.remove(&MessageId([0; 32])));
-        assert_eq!(buf.oldest_ts(), Some(1000));
+        // Derived-only buffer: no stored record, so no resume point.
+        assert_eq!(buf.oldest_ts(), None);
+        // With a stored record loaded, the derived records' lower
+        // timestamps are skipped.
+        assert!(buf.insert(rec(3000, b'a')));
+        assert_eq!(buf.oldest_ts(), Some(3000));
     }
 
     #[test]

+ 64 - 5
bin/app/src/ui/chatview/loader.rs

@@ -242,11 +242,14 @@ impl Loader {
         let mut batch_height = 0.;
         let touched_viewport = covered < scroll + view_h;
 
-        // Iterate newest -> older, resuming below the oldest loaded
-        // composite key.
-        let iter = match buffer.oldest_ts() {
-            Some(oldest) => {
-                let key = codec::encode_key(oldest.saturating_sub(1), &MessageId([0xff; 32]));
+        // Iterate newest -> older, resuming strictly below the oldest
+        // loaded stored record's composite key. Derived separators are
+        // excluded (their midnight key would skip the day's unloaded
+        // remainder), and the exact key keeps same-ts records with
+        // smaller ids reachable.
+        let iter = match buffer.oldest_key() {
+            Some((ts, id)) => {
+                let key = codec::encode_key(ts, &id);
                 tree.range(..key).rev()
             }
             None => tree.iter().rev(),
@@ -483,6 +486,62 @@ mod tests {
         assert_eq!(kinds, vec![b'c', b'|', b'b', b'a', b'|']);
     }
 
+    /// Backfill pumps must not skip the unloaded remainder of the
+    /// oldest loaded day: the day separator's midnight key is below the
+    /// day's messages, so a resume point taken from it would jump a
+    /// whole day back and strand the rest of the day unloadable.
+    #[test]
+    fn pump_backfills_rest_of_oldest_loaded_day() {
+        let buffer = Arc::new(AsyncMutex::new(MsgBuffer::new()));
+        let (redraw, _rx) = RedrawTrigger::new();
+        let loader = Loader::new(buffer.clone(), redraw);
+        loader.update_viewport(0., 500.);
+
+        use chrono::{Local, TimeZone};
+        let midnight = |day: i64| {
+            let date =
+                chrono::NaiveDate::from_ymd_opt(2026, 8, 29).unwrap() + chrono::Duration::days(day);
+            let dt = date.and_hms_opt(0, 0, 0).unwrap();
+            Local.from_local_datetime(&dt).unwrap().timestamp_millis() as u64
+        };
+        let m0 = midnight(0);
+        let m1 = midnight(1);
+
+        // Day 1 (newer) holds more than one batch; day 0 holds a few.
+        // Stored ids stay away from the separators' zero id (offset the
+        // day ts by an hour so id 0's composite key cannot collide).
+        let hour = 3_600_000u64;
+        let mut lines = vec![];
+        for i in 0..150u16 {
+            lines.push((m1 + hour + i as u64, i as u8));
+        }
+        for i in 150..165u16 {
+            lines.push((m0 + hour + i as u64, i as u8));
+        }
+        loader.bind("test".to_string(), fixture_db("backfill", &lines));
+
+        smol::block_on(loader.pump(Wakeup::ChannelSwitch.bit()));
+        {
+            let buffer = smol::block_on(buffer.lock());
+            // Batch cap: exactly the newest 100, all on day 1.
+            assert_eq!(buffer.len() - 1, 100, "first pump loads 100 + separator");
+            assert!(buffer.iter_display_order().all(|r| r.ts >= m1));
+        }
+
+        // The bug: this pump resumed below day 1's separator (midnight)
+        // and only found day 0, stranding day 1's older half.
+        smol::block_on(loader.pump(Wakeup::NearTop.bit()));
+        let buffer = smol::block_on(buffer.lock());
+        let ids: Vec<u8> = buffer
+            .iter_display_order()
+            .filter(|r| !r.msg_type.is_derived())
+            .map(|r| r.id.0[0])
+            .collect();
+        assert_eq!(ids.len(), 165, "everything eventually loads");
+        assert_eq!(buffer.oldest_ts(), Some(m0 + hour + 150), "reached the true oldest");
+        assert!(ids.contains(&50), "day 1's older half is present");
+    }
+
     #[test]
     fn corrupt_entries_panic_loudly() {
         let buffer = Arc::new(AsyncMutex::new(MsgBuffer::new()));

+ 28 - 19
bin/app/src/ui/chatview/mod.rs

@@ -66,7 +66,10 @@ use crate::{
     ExecutorPtr,
 };
 
-use super::{DrawUpdate, GestureAction, GestureSet, OnModify, RedrawTrigger, UIObject};
+use super::{
+    DrawUpdate, GestureAction, GestureSet, OnModify, PressedKey, PressedKeysSmoothRepeat,
+    RedrawTrigger, UIObject,
+};
 
 pub mod buffer;
 pub mod codec;
@@ -187,6 +190,8 @@ pub struct ChatView {
     /// Geometry the last draw pass saw, for reflow detection.
     last_width: SyncMutex<f32>,
     last_scale: SyncMutex<f32>,
+    /// Smooth repeat for held PageUp/PageDown scrolling.
+    key_repeat: SyncMutex<PressedKeysSmoothRepeat>,
 
     /// Weak self-reference so handlers can spawn detached tasks.
     me: Weak<Self>,
@@ -331,6 +336,7 @@ impl ChatView {
             window_scale: window_scale.clone(),
             last_width: SyncMutex::new(0.),
             last_scale: SyncMutex::new(window_scale.get()),
+            key_repeat: SyncMutex::new(PressedKeysSmoothRepeat::new(400, 50)),
 
             me: me.clone(),
         });
@@ -1424,28 +1430,31 @@ impl UIObject for ChatView {
     }
 
     async fn handle_key_down(&self, key: KeyCode, _mods: KeyMods, repeat: bool) -> bool {
-        if repeat {
-            return false
+        let dir = match key {
+            KeyCode::PageUp => 1.,
+            KeyCode::PageDown => -1.,
+            _ => return false,
+        };
+
+        // Held PageUp/PageDown scrolls smoothly: the initial press acts
+        // immediately and OS repeats are throttled to the repeat
+        // cadence; each tick coalesces into the in-flight page
+        // animation's target.
+        let actions = {
+            let mut repeater = self.key_repeat.lock();
+            repeater.key_down(PressedKey::Key(key), repeat)
+        };
+        if actions == 0 {
+            return true;
         }
 
         let rect = self.rect.get();
-        match key {
-            KeyCode::PageUp => {
-                let mut ctl = self.controller.lock();
-                ctl.page_tick(1., rect.h / 2.);
-                drop(ctl);
-                self.notify_motion();
-                true
-            }
-            KeyCode::PageDown => {
-                let mut ctl = self.controller.lock();
-                ctl.page_tick(-1., rect.h / 2.);
-                drop(ctl);
-                self.notify_motion();
-                true
-            }
-            _ => false,
+        for _ in 0..actions {
+            let mut ctl = self.controller.lock();
+            ctl.page_tick(dir, rect.h / 2.);
         }
+        self.notify_motion();
+        true
     }
 
     async fn handle_mouse_btn_down(&self, btn: MouseButton, mouse_pos: Point) -> bool {