Keep a drag in draft until it stops, then render sharp once

During any drag longer than 120 ms the canvas rendered a full-resolution
frame every 128 ms under the finger. The settle timer was armed by the
first draft of a burst and not re-armed by later ones, so it counted from
the start of the gesture rather than from its last movement, fired
mid-drag, and the next coalesced event armed it again. Each of those
frames is the most expensive one the canvas draws, landing where the
frame budget is tightest.

The draft/sharp decision now lives in `refine::Refine`, apart from the
timers that carry it out. Every draft frame arms a settle timer carrying
a generation token and only the newest token is honoured, so the sharp
frame lands SETTLE_DELAY after the last movement. A request arriving
while a settle is still owed also counts as part of the gesture, so a
slow stretch of a drag (one event per frame, nothing to coalesce) no
longer renders sharp between drafts. A timer that fires with a render
already posted defers to it.

The tests drive the state machine through simulated timelines; the
long-drag case reproduced the four mid-drag sharp frames before the fix.

ui/dr-ui/src/lib.rs

@@ -58,6 +58,7 @@ mod place;
 mod preset_store;
 mod presets;
 mod recovery_ui;
+mod refine;
 mod remote;
 pub mod repairs;
 mod segmentation;
@@ -2428,77 +2429,44 @@ fn build_render_now(
 /// on the finger rather than settling on a stale frame.
 fn build_redraw(render_now: &Render) -> Rc<dyn Fn(&AppWindow)> {
     let render_now = render_now.clone();
-    let render_pending = Rc::new(std::cell::Cell::new(false));
-    let render_dirty = Rc::new(std::cell::Cell::new(false));
-    // Set while a settle render is already queued, so a burst of draft frames
-    // schedules exactly one of them rather than one apiece.
-    let settle_pending = Rc::new(std::cell::Cell::new(false));
-    // Whether a request had to be coalesced into one already queued — see the
-    // gesture note below, where this stands in for a drag boundary.
-    let was_coalesced = Rc::new(std::cell::Cell::new(false));
-    {
+    // Which frame to draw and when to settle, decided apart from the timers
+    // that carry it out — see `refine` for the rules and their tests.
+    let refine = Rc::new(RefCell::new(refine::Refine::default()));
+    Rc::new(move |window: &AppWindow| {
+        if !refine.borrow_mut().request() {
+            return;
+        }
+
+        let weak = window.as_weak();
         let render_now = render_now.clone();
-        let render_pending = render_pending.clone();
-        let render_dirty = render_dirty.clone();
-        let settle_pending = settle_pending.clone();
-        let was_coalesced = was_coalesced.clone();
-        Rc::new(move |window: &AppWindow| {
-            render_dirty.set(true);
-            if render_pending.get() {
-                was_coalesced.set(true);
+        let refine = refine.clone();
+        // A zero-delay `Timer` rather than `invoke_from_event_loop`: the
+        // latter demands `Send`, and every piece of state here is `Rc` on
+        // the UI thread by design. The delay being zero is the point — this
+        // is "after the queued input has drained", not a throttle.
+        slint::Timer::single_shot(std::time::Duration::ZERO, move || {
+            let Some(window) = weak.upgrade() else { return };
+            let Some((frame, settle)) = refine.borrow_mut().tick() else {
                 return;
-            }
-            render_pending.set(true);
+            };
+            render_now(&window, frame == refine::Frame::Draft);
+            let Some(token) = settle else { return };
 
+            // TRACES: FR-DSP-4
+            // One timer per draft frame, and only the newest is honoured, so
+            // the sharp frame waits for the gesture to stop rather than for
+            // the first draft to age. A superseded timer wakes, finds its
+            // token stale, and does nothing — cheaper than holding a `Timer`
+            // to restart, and it keeps the rule where the tests can see it.
             let weak = window.as_weak();
-            let render_now = render_now.clone();
-            let render_pending = render_pending.clone();
-            let render_dirty = render_dirty.clone();
-            let settle_pending = settle_pending.clone();
-            let was_coalesced = was_coalesced.clone();
-            // A zero-delay `Timer` rather than `invoke_from_event_loop`: the
-            // latter demands `Send`, and every piece of state here is `Rc` on
-            // the UI thread by design. The delay being zero is the point — this
-            // is "after the queued input has drained", not a throttle.
-            slint::Timer::single_shot(std::time::Duration::ZERO, move || {
-                render_pending.set(false);
+            slint::Timer::single_shot(SETTLE_DELAY, move || {
                 let Some(window) = weak.upgrade() else { return };
-                if !render_dirty.replace(false) {
-                    return;
-                }
-
-                // **What counts as "still dragging".**
-                //
-                // No control reports a gesture boundary, and threading one out
-                // of every slider, curve point and crop handle would be a lot
-                // of surface for a rendering concern. `was_coalesced` answers
-                // it instead: it is set only when a request arrived while a
-                // render was already queued, which can only mean a control
-                // moved again — that is a drag. One-off changes — a click, a
-                // reset, a resize — never coalesce, so they render sharp the
-                // first time and never draw a draft frame at all.
-                let dragging = was_coalesced.replace(false);
-                render_now(&window, dragging);
-                if !dragging {
-                    return;
-                }
-
-                if settle_pending.replace(true) {
-                    return;
-                }
-                let weak = window.as_weak();
-                let render_now = render_now.clone();
-                let settle_pending = settle_pending.clone();
-                // Long enough that an ordinary drag never reaches it, short
-                // enough that the sharp frame feels immediate on release.
-                slint::Timer::single_shot(SETTLE_DELAY, move || {
-                    settle_pending.set(false);
-                    let Some(window) = weak.upgrade() else { return };
+                if refine.borrow_mut().settle(token).is_some() {
                     render_now(&window, false);
-                });
+                }
             });
-        })
-    }
+        });
+    })
 }
 
 /// Build `show`: load the entry at `index` and put it on screen.

ui/dr-ui/src/refine.rs

@@ -0,0 +1,251 @@
+//! TRACES: FR-DSP-4
+//! When the canvas renders a draft and when it renders sharp.
+//!
+//! The decision `build_redraw` acts on, kept apart from the timers and the
+//! window that carry it out so it can be driven event by event in a test.
+//! `lib.rs` owns the plumbing: it posts a zero-delay timer when
+//! [`Refine::request`] says to, calls [`Refine::tick`] from it, and arms a
+//! [`SETTLE_DELAY`](crate::SETTLE_DELAY) timer carrying whatever token
+//! `tick` hands back, which calls [`Refine::settle`] when it fires.
+
+/// Which quality of frame to render.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum Frame {
+    /// Half resolution, while a gesture is still moving.
+    Draft,
+    /// Full resolution.
+    Sharp,
+}
+
+/// A settle timer's claim on the sharp frame, redeemed by [`Refine::settle`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct Settle(u64);
+
+/// The coalescing and settling state behind the canvas redraw.
+#[derive(Debug, Default)]
+pub(crate) struct Refine {
+    /// A render is posted and has not run yet.
+    pending: bool,
+    /// Something changed since the last render.
+    dirty: bool,
+    /// A request arrived while a render was already posted.
+    coalesced: bool,
+    /// A settle timer is armed.
+    settling: bool,
+    /// The token the most recent settle timer carries.
+    generation: u64,
+}
+
+impl Refine {
+    /// Something changed; `true` means post a render.
+    pub(crate) fn request(&mut self) -> bool {
+        self.dirty = true;
+        if self.pending {
+            self.coalesced = true;
+            return false;
+        }
+        self.pending = true;
+        true
+    }
+
+    /// The posted render runs: which frame to draw, if any, and a settle
+    /// timer to arm afterwards, if one is wanted.
+    ///
+    /// **Every draft frame arms a fresh settle timer**, and only the newest
+    /// one is honoured (see [`Self::settle`]). That is a debounce: the sharp
+    /// frame arrives [`SETTLE_DELAY`](crate::SETTLE_DELAY) after the *last*
+    /// movement, never mid-gesture. It used to arm once per burst and not
+    /// again while that timer was pending, so the timer counted from the
+    /// *first* draft of a drag and fired underneath the finger — a full
+    /// resolution render every 128 ms of any drag longer than the delay,
+    /// each one the most expensive frame the canvas draws, landing exactly
+    /// where the budget is tightest.
+    ///
+    /// **What counts as "still dragging".** No control reports a gesture
+    /// boundary, and threading one out of every slider, curve point and crop
+    /// handle would be a lot of surface for a rendering concern. Two signals
+    /// stand in for it. A request that had to coalesce into a render already
+    /// posted can only mean a control moved again. And a request arriving
+    /// while a settle is still owed belongs to the gesture that owes it: a
+    /// slow stretch of a drag, one event per frame, would otherwise render
+    /// sharp between two drafts. One-off changes — a click, a reset, a resize
+    /// — meet neither, so they render sharp the first time and never draw a
+    /// draft frame at all.
+    pub(crate) fn tick(&mut self) -> Option<(Frame, Option<Settle>)> {
+        self.pending = false;
+        if !std::mem::take(&mut self.dirty) {
+            return None;
+        }
+        let dragging = std::mem::take(&mut self.coalesced) || self.settling;
+        if !dragging {
+            return Some((Frame::Sharp, None));
+        }
+        self.settling = true;
+        self.generation += 1;
+        Some((Frame::Draft, Some(Settle(self.generation))))
+    }
+
+    /// A settle timer fired: `Some` means render sharp now.
+    ///
+    /// `None` for any timer a later draft has superseded, and also when a
+    /// render is already posted: that one will draw a draft and arm its own
+    /// timer, and a sharp frame drawn now would be replaced a moment later.
+    pub(crate) fn settle(&mut self, token: Settle) -> Option<Frame> {
+        if !self.settling || token.0 != self.generation || self.pending {
+            return None;
+        }
+        self.settling = false;
+        Some(Frame::Sharp)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The settle delay, in the simulated milliseconds below.
+    const DELAY: u64 = 120;
+
+    /// Drive a `Refine` through a timeline of input events the way the event
+    /// loop would, and record every frame rendered with the time it was.
+    ///
+    /// `inputs` are the milliseconds at which a control moved. Each posted
+    /// render runs one millisecond after the request that posted it, and a
+    /// burst of requests landing in the same millisecond coalesces into it.
+    fn run(inputs: &[u64]) -> Vec<(u64, Frame)> {
+        let mut r = Refine::default();
+        let mut frames = Vec::new();
+        let mut tick_at: Option<u64> = None;
+        let mut settles: Vec<(u64, Settle)> = Vec::new();
+        let end = inputs.last().copied().unwrap_or(0) + 10 * DELAY;
+        let mut inputs = inputs.iter().copied().peekable();
+        for now in 0..=end {
+            // Timers whose time has come, in the order they were armed.
+            if tick_at == Some(now) {
+                tick_at = None;
+                if let Some((frame, arm)) = r.tick() {
+                    frames.push((now, frame));
+                    if let Some(token) = arm {
+                        settles.push((now + DELAY, token));
+                    }
+                }
+            }
+            let due: Vec<Settle> = settles
+                .iter()
+                .filter(|(t, _)| *t == now)
+                .map(|(_, s)| *s)
+                .collect();
+            settles.retain(|(t, _)| *t != now);
+            for token in due {
+                if let Some(frame) = r.settle(token) {
+                    frames.push((now, frame));
+                }
+            }
+            while inputs.peek() == Some(&now) {
+                inputs.next();
+                if r.request() {
+                    tick_at = Some(now + 1);
+                }
+            }
+        }
+        frames
+    }
+
+    /// A drag: two moves per 16 ms frame from `from` for `len` milliseconds.
+    fn drag(from: u64, len: u64) -> Vec<u64> {
+        (from..from + len)
+            .step_by(16)
+            .flat_map(|t| [t, t])
+            .collect()
+    }
+
+    /// The sharp frames rendered in `(from, to]`.
+    fn sharp_between(frames: &[(u64, Frame)], from: u64, to: u64) -> Vec<(u64, Frame)> {
+        frames
+            .iter()
+            .copied()
+            .filter(|(t, f)| *f == Frame::Sharp && *t > from && *t <= to)
+            .collect()
+    }
+
+    #[test]
+    fn a_long_drag_renders_no_sharp_frame_until_it_stops() {
+        // The regression: before the debounce this drew sharp frames at 121,
+        // 249, 377 and 505 ms, the settle timer counting from the first draft.
+        let inputs = drag(0, 600);
+        let last = *inputs.last().unwrap();
+        let frames = run(&inputs);
+        let mid = sharp_between(&frames, 1, last + 1);
+        assert!(mid.is_empty(), "sharp frames mid-drag: {mid:?}");
+    }
+
+    #[test]
+    fn the_sharp_frame_lands_one_delay_after_the_last_movement() {
+        let inputs = drag(0, 600);
+        let last = *inputs.last().unwrap();
+        let frames = run(&inputs);
+        let sharp = sharp_between(&frames, 1, u64::MAX);
+        // The last draft renders a millisecond after the last input, and the
+        // sharp frame the settle delay after that.
+        assert_eq!(sharp, vec![(last + 1 + DELAY, Frame::Sharp)]);
+        assert_eq!(frames.last(), Some(&(last + 1 + DELAY, Frame::Sharp)));
+    }
+
+    #[test]
+    fn a_slow_stretch_inside_a_drag_stays_draft() {
+        // Coalesced moves, then one move per 40 ms (none coalesce), then stop.
+        let mut inputs = drag(0, 100);
+        inputs.extend((112..400).step_by(40));
+        let last = *inputs.last().unwrap();
+        let frames = run(&inputs);
+        let mid = sharp_between(&frames, 1, last + 1);
+        assert!(mid.is_empty(), "sharp frames mid-drag: {mid:?}");
+        assert_eq!(frames.last(), Some(&(last + 1 + DELAY, Frame::Sharp)));
+    }
+
+    #[test]
+    fn a_pause_longer_than_the_delay_settles_and_the_next_drag_drafts_again() {
+        let mut inputs = drag(0, 100);
+        inputs.extend(drag(400, 100));
+        let frames = run(&inputs);
+        let sharp = sharp_between(&frames, 1, u64::MAX);
+        assert_eq!(sharp.len(), 2, "one settle per drag: {frames:?}");
+        assert!(frames
+            .iter()
+            .any(|(t, f)| *f == Frame::Draft && (400..500).contains(t)));
+    }
+
+    #[test]
+    fn a_single_change_renders_sharp_once_and_arms_nothing() {
+        assert_eq!(run(&[5]), vec![(6, Frame::Sharp)]);
+    }
+
+    #[test]
+    fn a_superseded_timer_does_nothing() {
+        let mut r = Refine::default();
+        assert!(r.request());
+        assert!(!r.request());
+        let (f, first) = r.tick().unwrap();
+        assert_eq!(f, Frame::Draft);
+        assert!(r.request());
+        let (f, second) = r.tick().unwrap();
+        assert_eq!(f, Frame::Draft);
+        assert_eq!(r.settle(first.unwrap()), None);
+        assert_eq!(r.settle(second.unwrap()), Some(Frame::Sharp));
+        // And once redeemed, not again.
+        assert_eq!(r.settle(second.unwrap()), None);
+    }
+
+    #[test]
+    fn a_timer_firing_with_a_render_posted_defers_to_it() {
+        let mut r = Refine::default();
+        r.request();
+        r.request();
+        let (_, token) = r.tick().unwrap();
+        assert!(r.request());
+        assert_eq!(r.settle(token.unwrap()), None);
+        let (f, next) = r.tick().unwrap();
+        assert_eq!(f, Frame::Draft);
+        assert_eq!(r.settle(next.unwrap()), Some(Frame::Sharp));
+    }
+}