From 334504a5b20794c11d87a874c105aa14ba5cc721 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Fri, 3 Dec 2021 10:27:46 +0000 Subject: [PATCH] animation: Prototype a way of handling state and applying it separately Handling visibility state ad-hoc has shortcomings in testability and locality, and this module attmepts to rectify that by creating an explicit state machine, along with a loop to drive it. Actually applying state is not implemented here. --- src/animation.rs | 442 +++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 2 files changed, 443 insertions(+) create mode 100644 src/animation.rs diff --git a/src/animation.rs b/src/animation.rs new file mode 100644 index 00000000..0d455644 --- /dev/null +++ b/src/animation.rs @@ -0,0 +1,442 @@ +/* Copyright (C) 2020 Purism SPC + * SPDX-License-Identifier: GPL-3.0+ + */ + +/*! Animation state trackers and drivers. + * Concerns the presentation layer. + * + * Documentation and comments in this module + * are meant to be read from the top to bottom. */ + +use crate::logging; +use glib; +use std::cmp; +use std::sync::mpsc; +use std::time::{ Duration, Instant }; + +// Traits +use crate::logging::Warn; + + +/// The keyboard should hide after this has elapsed to prevent flickering. +const HIDING_TIMEOUT: Duration = Duration::from_millis(200); + + +/// Events that the state tracker processes +#[derive(Clone)] +pub enum Event { + ClaimVisible, + /// The panel is not needed + ReleaseVisible, + /// The user requested the panel to go down + ForceHide, + /// Event triggered because a moment in time passed. + /// Use to animate state transitions. + /// The value is the ideal arrival time. + TimeoutReached(Instant), +} + +/// The outwardly visible state of visibility +#[derive(PartialEq, Debug)] +pub enum Outcome { + Visible, + Hidden, +} + + +/// The actual logic of visibility animation. +/// It keeps the pael visible for a short time period after each hide request. +/// This prevents flickering on quick successive enable/disable events. +/// It does not treat user-driven hiding in a special way. +/// +/// This is the "functional core". +/// All state changes return the next state and the optimal time for the next check. +/// +/// This state tracker can be driven by any event loop. +#[derive(Clone, PartialEq, Debug)] +enum VisibilityTracker { + Visible, + /// Wait until the instant is reached and then hide immediately. + /// Depending on the relation to current time, it means either visible or hidden. + HiddenAfter(Instant), +} + +use self::VisibilityTracker::*; + +impl VisibilityTracker { + fn apply_event(self, event: Event, now: Instant) -> Self { + match event { + Event::ClaimVisible => Visible, + Event::ReleaseVisible => match self { + Visible => HiddenAfter(now + HIDING_TIMEOUT), + other => other, + }, + Event::ForceHide => match self { + // Special case to avoid unneeded state changes. + HiddenAfter(when) => HiddenAfter(cmp::min(when, now)), + _ => HiddenAfter(now), + }, + // The tracker doesn't change just because time is passing. + Event::TimeoutReached(_) => self, + } + } + + /// Returns the state visible to the outside + fn get_outcome(&self, now: Instant) -> Outcome { + let visible = match self { + Visible => true, + HiddenAfter(hide_after) => *hide_after > now, + }; + if visible { + Outcome::Visible + } else { + Outcome::Hidden + } + } + + /// Returns the next time to update the state. + fn get_next_wake(&self, now: Instant) -> Option { + match self { + HiddenAfter(next) => { + if *next > now { Some(*next) } + else { None } + }, + _ => None, + } + } +} + +/* If we performed updates in a tight loop, + * the Tracker would have been all we need. + * + * loop { + * event = current_event() + * outcome = update_state(event) + * window.apply(outcome) + * } + * + * This is enough to process all events, + * and keep the window always in sync with the current state. + * + * However, we're trying to be conservative, + * and not waste time performing updates that don't change state, + * so we have to react to events that end up influencing the state. + * + * One complication from that is that animation steps + * are not a response to events coming from the owner of the loop, + * but are needed by the loop itself. + * + * This is where the rest of bugs hide: + * too few scheduled wakeups mean missed updates and wrong visible state. + * Too many wakeups can slow down the process, or make animation jittery. + * The loop iteration is kept as a pure function to stay testable. + */ + +/// This keeps the state of the tracker loop between iterations +#[derive(Clone)] +struct LoopState { + state: VisibilityTracker, + scheduled_wakeup: Option, +} + +impl LoopState { + fn new(initial_state: VisibilityTracker) -> Self { + Self { + state: initial_state, + scheduled_wakeup: None, + } + } +} + +/// A single iteration of the loop, updating its persistent state. +/// - updates tracker state, +/// - determines outcome, +/// - determines next scheduled animation wakeup, +/// and because this is a pure function, it's easily testable. +/// It returns the new state, and the optional message to send onwards. +fn handle_loop_event( + mut loop_state: LoopState, + event: Event, + now: Instant, +) -> (LoopState, Option) { + // Forward current public state to the consumer. + // This doesn't take changes into account, + // but we're only sending updates as a response to events, + // so no-ops shouldn't dominate. + loop_state.state = loop_state.state.apply_event(event.clone(), now); + let outcome = loop_state.state.get_outcome(now); + + // Timeout events are special: they affect the scheduled timeout. + loop_state.scheduled_wakeup = match event { + Event::TimeoutReached(when) => { + if when > now { + // Special handling for scheduled events coming in early. + // Wait at least 10 ms to avoid Zeno's paradox. + // This is probably not needed though, + // if the `now` contains the desired time of the event. + // But then what about time "reversing"? + Some(cmp::max( + when, + now + Duration::from_millis(10), + )) + } else { + // There's only one timeout in flight, and it's this one. + // It's about to complete, and then the tracker can be cleared. + // I'm not sure if this is strictly necessary. + None + } + }, + _ => loop_state.scheduled_wakeup.clone(), + }; + + // Reschedule timeout if the new state calls for it. + let scheduled = &loop_state.scheduled_wakeup; + let desired = loop_state.state.get_next_wake(now); + + loop_state.scheduled_wakeup = match (scheduled, desired) { + (&Some(scheduled), Some(next)) => { + if scheduled > next { + // State wants a wake to happen before the one which is already scheduled. + // The previous state is removed in order to only ever keep one in flight. + // That hopefully avoids pileups, + // e.g. because the system is busy + // and the user keeps doing something that queues more events. + Some(next) + } else { + // Not changing the case when the wanted wake is *after* scheduled, + // because wakes are not expensive as long as they don't pile up, + // and I can't see a pileup potential when it doesn't retrigger itself. + // Skipping an expected event is much more dangerous. + Some(scheduled) + } + }, + (None, Some(next)) => Some(next), + // No need to change the unneeded wake - see above. + // (Some(_), None) => ... + (other, _) => other.clone(), + }; + + (loop_state, Some(outcome)) +} + + +/* + * The tracker loop needs to be driven somehow, + * and connected to the external world, + * both on the side of receiving and sending events. + * + * That's going to be implementation-dependent, + * connecting to some external mechanisms + * for time, messages, and threading/callbacks. + * + * This is the "imperative shell" part of the software, + * and no longer unit-testable. + */ + +use std::thread; +type Sender = mpsc::Sender; +type UISender = glib::Sender; + +/// This loop driver spawns a new thread which updates the state in a loop, +/// in response to incoming events. +/// It sends outcomes to the glib main loop using a channel. +/// The outcomes are applied by the UI end of the channel. +// This could still be reasonably tested, +// by creating a glib::Sender and checking what messages it receives. +#[derive(Clone)] +pub struct ThreadLoopDriver { + thread: Sender, +} + +impl ThreadLoopDriver { + pub fn new(ui: UISender) -> Self { + let (sender, receiver) = mpsc::channel(); + let saved_sender = sender.clone(); + thread::spawn(move || { + let mut state = LoopState::new(VisibilityTracker::Visible); + loop { + match receiver.recv() { + Ok(event) => { + state = Self::handle_loop_event(&sender, state, event, &ui); + }, + Err(e) => { + logging::print(logging::Level::Bug, &format!("Senders hung up, aborting: {}", e)); + return; + }, + }; + } + }); + + Self { + thread: saved_sender, + } + } + + pub fn send(&self, event: Event) -> Result<(), mpsc::SendError> { + self.thread.send(event) + } + + fn handle_loop_event(loop_sender: &Sender, state: LoopState, event: Event, ui: &UISender) -> LoopState { + let now = Instant::now(); + + let (new_state, outcome) = handle_loop_event(state.clone(), event, now); + + if let Some(outcome) = outcome { + ui.send(outcome) + .or_warn(&mut logging::Print, logging::Problem::Bug, "Can't send to UI"); + } + + if new_state.scheduled_wakeup != state.scheduled_wakeup { + if let Some(when) = new_state.scheduled_wakeup { + Self::schedule_timeout_wake(loop_sender, when); + } + } + + new_state + } + + fn schedule_timeout_wake(loop_sender: &Sender, when: Instant) { + let sender = loop_sender.clone(); + thread::spawn(move || { + let now = Instant::now(); + thread::sleep(when - now); + sender.send(Event::TimeoutReached(when)) + .or_warn(&mut logging::Print, logging::Problem::Warning, "Can't wake visibility manager"); + }); + } +} + +/// For calling in only +mod c { + use super::*; + + use util::c::{ ArcWrapped, Wrapped }; + + #[no_mangle] + pub extern "C" + fn squeek_animation_visibility_manager_new(sender: ArcWrapped) + -> Wrapped + { + let sender = sender.clone_ref(); + let sender = sender.lock().unwrap(); + Wrapped::new(ThreadLoopDriver::new(sender.clone())) + } + + #[no_mangle] + pub extern "C" + fn squeek_animation_visibility_manager_send_claim_visible(mgr: Wrapped) { + let sender = mgr.clone_ref(); + let sender = sender.borrow(); + sender.send(Event::ClaimVisible) + .or_warn(&mut logging::Print, logging::Problem::Warning, "Can't send to visibility manager"); + } + + #[no_mangle] + pub extern "C" + fn squeek_animation_visibility_manager_send_force_hide(sender: Wrapped) { + let sender = sender.clone_ref(); + let sender = sender.borrow(); + sender.send(Event::ForceHide) + .or_warn(&mut logging::Print, logging::Problem::Warning, "Can't send to visibility manager"); + } +} + + +#[cfg(test)] +mod test { + use super::*; + + /// Test the original delay scenario: no flicker on quick switches. + #[test] + fn hide_show() { + let start = Instant::now(); // doesn't matter when. It would be better to have a reproducible value though + let mut now = start; + let state = VisibilityTracker::Visible; + let state = state.apply_event(Event::ReleaseVisible, now); + // Check 100ms at 1ms intervals. It should remain visible. + for _i in 0..100 { + now += Duration::from_millis(1); + assert_eq!( + state.get_outcome(now), + Outcome::Visible, + "Hidden when it should remain visible: {:?}", + now.saturating_duration_since(start), + ) + } + + let state = state.apply_event(Event::ClaimVisible, now); + + assert_eq!(state.get_outcome(now), Outcome::Visible); + } + + /// Make sure that hiding works when requested legitimately + #[test] + fn hide() { + let start = Instant::now(); // doesn't matter when. It would be better to have a reproducible value though + let mut now = start; + let state = VisibilityTracker::Visible; + let state = state.apply_event(Event::ReleaseVisible, now); + + while let Outcome::Visible = state.get_outcome(now) { + now += Duration::from_millis(1); + assert!( + now < start + Duration::from_millis(250), + "Hiding too slow: {:?}", + now.saturating_duration_since(start), + ); + } + } + + /// Check against the false showing bug. + /// Expectation: it will get hidden and not appear again + #[test] + fn false_show() { + let start = Instant::now(); // doesn't matter when. It would be better to have a reproducible value though + let mut now = start; + let state = VisibilityTracker::Visible; + // This reflects the sequence from Wayland: + // disable, disable, enable, disable + // all in a single batch. + let state = state.apply_event(Event::ReleaseVisible, now); + let state = state.apply_event(Event::ReleaseVisible, now); + let state = state.apply_event(Event::ClaimVisible, now); + let state = state.apply_event(Event::ReleaseVisible, now); + + while let Outcome::Visible = state.get_outcome(now) { + now += Duration::from_millis(1); + assert!( + now < start + Duration::from_millis(250), + "Still not hidden: {:?}", + now.saturating_duration_since(start), + ); + } + + // One second without appearing again + for _i in 0..1000 { + now += Duration::from_millis(1); + assert_eq!( + state.get_outcome(now), + Outcome::Hidden, + "Appeared unnecessarily: {:?}", + now.saturating_duration_since(start), + ); + } + } + + #[test] + fn schedule_hide() { + let start = Instant::now(); // doesn't matter when. It would be better to have a reproducible value though + let mut now = start; + + let l = LoopState::new(VisibilityTracker::Visible); + let (l, outcome) = handle_loop_event(l, Event::ReleaseVisible, now); + assert_eq!(outcome, Some(Outcome::Visible)); + assert_eq!(l.scheduled_wakeup, Some(now + HIDING_TIMEOUT)); + + now += HIDING_TIMEOUT; + + let (l, outcome) = handle_loop_event(l, Event::TimeoutReached(now), now); + assert_eq!(outcome, Some(Outcome::Hidden)); + assert_eq!(l.scheduled_wakeup, None); + } +} diff --git a/src/lib.rs b/src/lib.rs index a133e9ee..0673e4a0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ extern crate xkbcommon; mod logging; mod action; +mod animation; pub mod data; mod drawing; pub mod float_ord;