From 402e14dd0ab842dc28459d8adef196f6e3786f58 Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Tue, 30 Apr 2024 22:27:47 +0200 Subject: [PATCH] refactor(core/ui): T3T1 flow animation [no changelog] --- core/embed/rust/src/ui/component/image.rs | 1 - core/embed/rust/src/ui/component/map.rs | 9 +- core/embed/rust/src/ui/component/qr_code.rs | 1 - core/embed/rust/src/ui/component/timeout.rs | 1 - core/embed/rust/src/ui/flow/base.rs | 21 +-- core/embed/rust/src/ui/flow/flow.rs | 139 ++++++++++------- core/embed/rust/src/ui/flow/page.rs | 147 +++++++++++++----- core/embed/rust/src/ui/flow/store.rs | 54 +------ .../src/ui/model_mercury/component/dialog.rs | 1 - .../src/ui/model_mercury/component/frame.rs | 12 +- 10 files changed, 217 insertions(+), 169 deletions(-) diff --git a/core/embed/rust/src/ui/component/image.rs b/core/embed/rust/src/ui/component/image.rs index 4a3fe34ee1..959ccf2c49 100644 --- a/core/embed/rust/src/ui/component/image.rs +++ b/core/embed/rust/src/ui/component/image.rs @@ -72,7 +72,6 @@ impl crate::trace::Trace for Image { } } -#[derive(Clone)] pub struct BlendedImage { bg: Icon, fg: Icon, diff --git a/core/embed/rust/src/ui/component/map.rs b/core/embed/rust/src/ui/component/map.rs index 4b171a1281..31b7c421e1 100644 --- a/core/embed/rust/src/ui/component/map.rs +++ b/core/embed/rust/src/ui/component/map.rs @@ -1,7 +1,6 @@ use super::{Component, Event, EventCtx}; use crate::ui::{geometry::Rect, shape::Renderer}; -#[derive(Clone)] pub struct MsgMap { inner: T, func: F, @@ -57,11 +56,11 @@ impl crate::ui::flow::Swipable for MsgMap where T: Component + crate::ui::flow::Swipable, { - fn can_swipe(&self, direction: crate::ui::component::SwipeDirection) -> bool { - self.inner.can_swipe(direction) + fn swipe_start(&mut self, ctx: &mut EventCtx, direction: super::SwipeDirection) -> bool { + self.inner.swipe_start(ctx, direction) } - fn swiped(&mut self, ctx: &mut EventCtx, direction: crate::ui::component::SwipeDirection) { - self.inner.swiped(ctx, direction) + fn swipe_finished(&self) -> bool { + self.inner.swipe_finished() } } diff --git a/core/embed/rust/src/ui/component/qr_code.rs b/core/embed/rust/src/ui/component/qr_code.rs index 953af10ed8..6b13e116d8 100644 --- a/core/embed/rust/src/ui/component/qr_code.rs +++ b/core/embed/rust/src/ui/component/qr_code.rs @@ -25,7 +25,6 @@ const CORNER_RADIUS: u8 = 4; const DARK: Color = Color::rgb(0, 0, 0); const LIGHT: Color = Color::rgb(0xff, 0xff, 0xff); -#[derive(Clone)] pub struct Qr { text: String, border: i16, diff --git a/core/embed/rust/src/ui/component/timeout.rs b/core/embed/rust/src/ui/component/timeout.rs index 8aac3ba8dd..6da62e7255 100644 --- a/core/embed/rust/src/ui/component/timeout.rs +++ b/core/embed/rust/src/ui/component/timeout.rs @@ -7,7 +7,6 @@ use crate::{ }, }; -#[derive(Clone)] pub struct Timeout { time_ms: u32, timer: Option, diff --git a/core/embed/rust/src/ui/flow/base.rs b/core/embed/rust/src/ui/flow/base.rs index 061f0ac85d..0c202bec9c 100644 --- a/core/embed/rust/src/ui/flow/base.rs +++ b/core/embed/rust/src/ui/flow/base.rs @@ -16,18 +16,21 @@ impl SwipeDirection { } /// Component must implement this trait in order to be part of swipe-based flow. -/// The process of receiving a swipe is two-step, because in order to render the -/// transition animation Flow makes a copy of the pre-swipe state of the -/// component to render it along with the post-swipe state. +/// +/// Default implementation ignores every swipe. pub trait Swipable { - /// Return true if component can handle swipe in a given direction. - fn can_swipe(&self, _direction: SwipeDirection) -> bool { + /// Attempt a swipe. Return false if component in its current state doesn't + /// accept a swipe in the given direction. Start a transition animation + /// if true is returned. + fn swipe_start(&mut self, _ctx: &mut EventCtx, _direction: SwipeDirection) -> bool { false } - /// Make component react to swipe event. Only called if component returned - /// true in the previous function. - fn swiped(&mut self, _ctx: &mut EventCtx, _direction: SwipeDirection) {} + /// Return true when transition animation is finished. SwipeFlow needs to + /// know this in order to resume normal input processing. + fn swipe_finished(&self) -> bool { + true + } } /// Component::Msg for component parts of a flow. Converting results of @@ -68,7 +71,7 @@ impl Decision { } /// Encodes the flow logic as a set of states, and transitions between them -/// triggered by events and swipes. +/// triggered by events and swipes. Roughly the C in MVC. pub trait FlowState where Self: Sized + Copy + Eq + ToPrimitive, diff --git a/core/embed/rust/src/ui/flow/flow.rs b/core/embed/rust/src/ui/flow/flow.rs index 29c9c178ea..e832b67d71 100644 --- a/core/embed/rust/src/ui/flow/flow.rs +++ b/core/embed/rust/src/ui/flow/flow.rs @@ -23,21 +23,28 @@ pub struct SwipeFlow { state: Q, /// FlowStore with all screens/components. store: S, - /// `Some` when state transition animation is in progress. - transition: Option>, + /// `Transition::None` when state transition animation is in not progress. + transition: Transition, /// Swipe detector. swipe: Swipe, /// Animation parameter. anim_offset: Offset, } -struct Transition { - /// State we are transitioning _from_. - prev_state: Q, - /// Animation progress. - animation: Animation, - /// Direction of the slide animation. - direction: SwipeDirection, +enum Transition { + /// SwipeFlow is performing transition between different states. + External { + /// State we are transitioning _from_. + prev_state: Q, + /// Animation progress. + animation: Animation, + /// Direction of the slide animation. + direction: SwipeDirection, + }, + /// Transition runs in child component, we forward events and wait. + Internal, + /// No transition. + None, } impl SwipeFlow { @@ -45,14 +52,18 @@ impl SwipeFlow { Ok(Self { state: init, store, - transition: None, + transition: Transition::None, swipe: Swipe::new().down().up().left().right(), anim_offset: Offset::zero(), }) } fn goto(&mut self, ctx: &mut EventCtx, direction: SwipeDirection, state: Q) { - self.transition = Some(Transition { + if state == self.state { + self.transition = Transition::Internal; + return; + } + self.transition = Transition::External { prev_state: self.state, animation: Animation::new( Offset::zero(), @@ -61,58 +72,71 @@ impl SwipeFlow { Instant::now(), ), direction, - }); + }; self.state = state; ctx.request_anim_frame(); - ctx.request_paint() + ctx.request_paint(); } fn render_state<'s>(&'s self, state: Q, target: &mut impl Renderer<'s>) { self.store.render(state.index(), target) } - fn render_transition<'s>(&'s self, transition: &Transition, target: &mut impl Renderer<'s>) { - let off = transition.animation.value(Instant::now()); - - if self.state == transition.prev_state { - target.with_origin(off, &|target| { - self.store.render_cloned(target); - }); - } else { - target.with_origin(off, &|target| { - self.render_state(transition.prev_state, target); - }); - } - target.with_origin( - off - transition.direction.as_offset(self.anim_offset), - &|target| { - self.render_state(self.state, target); - }, - ); + fn render_transition<'s>( + &'s self, + prev_state: &Q, + animation: &Animation, + direction: &SwipeDirection, + target: &mut impl Renderer<'s>, + ) { + let off = animation.value(Instant::now()); + target.with_origin(off, &|target| { + self.render_state(*prev_state, target); + }); + target.with_origin(off - direction.as_offset(self.anim_offset), &|target| { + self.render_state(self.state, target); + }); } - fn handle_transition(&mut self, ctx: &mut EventCtx) { - if let Some(transition) = &self.transition { - if transition.animation.finished(Instant::now()) { - self.transition = None; - unwrap!(self.store.clone(None)); // Free the clone. - - let msg = self.store.event(self.state.index(), ctx, Event::Attach); - assert!(msg.is_none()) - } else { - ctx.request_anim_frame(); + fn handle_transition(&mut self, ctx: &mut EventCtx, event: Event) -> Option { + let i = self.state.index(); + let mut finished = false; + let result = match &self.transition { + Transition::External { animation, .. } + if matches!(event, Event::Timer(EventCtx::ANIM_FRAME_TIMER)) => + { + if animation.finished(Instant::now()) { + finished = true; + ctx.request_paint(); + self.store.event(i, ctx, Event::Attach) + } else { + ctx.request_anim_frame(); + ctx.request_paint(); + None + } } - ctx.request_paint(); + Transition::External { .. } => None, // ignore all events until animation finishes + Transition::Internal => { + let msg = self.store.event(i, ctx, event); + if self.store.map_swipable(i, |s| s.swipe_finished()) { + finished = true; + }; + msg + } + Transition::None => unreachable!(), + }; + if finished { + self.transition = Transition::None; } + return result; } fn handle_swipe_child(&mut self, ctx: &mut EventCtx, direction: SwipeDirection) -> Decision { let i = self.state.index(); - if self.store.map_swipable(i, |s| s.can_swipe(direction)) { - // Before handling the swipe we make a copy of the original state so that we - // can render both states in the transition animation. - unwrap!(self.store.clone(Some(i))); - self.store.map_swipable(i, |s| s.swiped(ctx, direction)); + if self + .store + .map_swipable(i, |s| s.swipe_start(ctx, direction)) + { Decision::Goto(self.state, direction) } else { Decision::Nothing @@ -142,14 +166,8 @@ impl Component for SwipeFlow { } fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { - // TODO: are there any events we want to send to all? timers perhaps? - if let Event::Timer(EventCtx::ANIM_FRAME_TIMER) = event { - self.handle_transition(ctx); - return None; - } - // Ignore events while transition is running. - if self.transition.is_some() { - return None; + if !matches!(self.transition, Transition::None) { + return self.handle_transition(ctx, event); } let mut decision = Decision::Nothing; @@ -173,10 +191,13 @@ impl Component for SwipeFlow { fn paint(&mut self) {} fn render<'s>(&'s self, target: &mut impl Renderer<'s>) { - if let Some(transition) = &self.transition { - self.render_transition(transition, target) - } else { - self.render_state(self.state, target) + match &self.transition { + Transition::None | Transition::Internal => self.render_state(self.state, target), + Transition::External { + prev_state, + animation, + direction, + } => self.render_transition(prev_state, animation, direction, target), } } } diff --git a/core/embed/rust/src/ui/flow/page.rs b/core/embed/rust/src/ui/flow/page.rs index 8c986975b7..81bf465214 100644 --- a/core/embed/rust/src/ui/flow/page.rs +++ b/core/embed/rust/src/ui/flow/page.rs @@ -1,42 +1,101 @@ -use crate::ui::{ - component::{Component, Event, EventCtx, Paginate, SwipeDirection}, - flow::base::Swipable, - geometry::{Axis, Rect}, - shape::Renderer, +use crate::{ + micropython::gc::Gc, + time::{Duration, Instant}, + ui::{ + animation::Animation, + component::{Component, Event, EventCtx, Paginate, SwipeDirection}, + flow::base::Swipable, + geometry::{Axis, Offset, Rect}, + shape::Renderer, + }, }; +pub struct Transition { + /// Clone of the component before page change. + cloned: Gc, + /// Animation progress. + animation: Animation, + /// Direction of the slide animation. + direction: SwipeDirection, +} + +const ANIMATION_DURATION: Duration = Duration::from_millis(333); + /// Allows any implementor of `Paginate` to be part of `Swipable` UI flow. -#[derive(Clone)] +/// Renders sliding animation when changing pages. pub struct SwipePage { inner: T, + bounds: Rect, axis: Axis, pages: usize, current: usize, + transition: Option>, } -impl SwipePage { +impl SwipePage { pub fn vertical(inner: T) -> Self { Self { inner, + bounds: Rect::zero(), axis: Axis::Vertical, pages: 1, current: 0, + transition: None, } } + + fn handle_transition(ctx: &mut EventCtx, event: Event, transition: &mut Transition) -> bool { + let mut finished = false; + if let Event::Timer(EventCtx::ANIM_FRAME_TIMER) = event { + if transition.animation.finished(Instant::now()) { + finished = true; + } else { + ctx.request_anim_frame(); + } + ctx.request_paint() + } + finished + } + + fn render_transition<'s>( + &'s self, + transition: &'s Transition, + target: &mut impl Renderer<'s>, + ) { + let off = transition.animation.value(Instant::now()); + target.in_clip(self.bounds, &|target| { + target.with_origin(off, &|target| { + transition.cloned.render(target); + }); + target.with_origin( + off - transition.direction.as_offset(self.bounds.size()), + &|target| { + self.inner.render(target); + }, + ); + }); + } } -impl Component for SwipePage { +impl Component for SwipePage { type Msg = T::Msg; fn place(&mut self, bounds: Rect) -> Rect { - let result = self.inner.place(bounds); + self.bounds = self.inner.place(bounds); self.pages = self.inner.page_count(); - result + self.bounds } fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { - let msg = self.inner.event(ctx, event); - msg + if let Some(t) = &mut self.transition { + let finished = Self::handle_transition(ctx, event, t); + if finished { + // FIXME: how to ensure the Gc allocation is returned? + self.transition = None + } + return None; + } + self.inner.event(ctx, event) } fn paint(&mut self) { @@ -44,33 +103,49 @@ impl Component for SwipePage { } fn render<'s>(&'s self, target: &mut impl Renderer<'s>) { + if let Some(t) = &self.transition { + return self.render_transition(t, target); + } self.inner.render(target) } } -impl Swipable for SwipePage { - fn can_swipe(&self, direction: SwipeDirection) -> bool { +impl Swipable for SwipePage { + fn swipe_start(&mut self, ctx: &mut EventCtx, direction: SwipeDirection) -> bool { match (self.axis, direction) { - (Axis::Horizontal, SwipeDirection::Up | SwipeDirection::Down) => false, - (Axis::Vertical, SwipeDirection::Left | SwipeDirection::Right) => false, - (_, SwipeDirection::Left | SwipeDirection::Up) => self.current + 1 < self.pages, - (_, SwipeDirection::Right | SwipeDirection::Down) => self.current > 0, - } + // Wrong direction + (Axis::Horizontal, SwipeDirection::Up | SwipeDirection::Down) => return false, + (Axis::Vertical, SwipeDirection::Left | SwipeDirection::Right) => return false, + // Begin + (_, SwipeDirection::Right | SwipeDirection::Down) if self.current == 0 => return false, + // End + (_, SwipeDirection::Left | SwipeDirection::Up) if self.current + 1 >= self.pages => { + return false + } + _ => {} + }; + self.transition = Some(Transition { + cloned: unwrap!(Gc::new(self.inner.clone())), + animation: Animation::new( + Offset::zero(), + direction.as_offset(self.bounds.size()), + ANIMATION_DURATION, + Instant::now(), + ), + direction, + }); + self.current = match direction { + SwipeDirection::Left | SwipeDirection::Up => (self.current + 1).min(self.pages - 1), + SwipeDirection::Right | SwipeDirection::Down => self.current.saturating_sub(1), + }; + self.inner.change_page(self.current); + ctx.request_anim_frame(); + ctx.request_paint(); + true } - fn swiped(&mut self, ctx: &mut EventCtx, direction: SwipeDirection) { - match (self.axis, direction) { - (Axis::Horizontal, SwipeDirection::Up | SwipeDirection::Down) => return, - (Axis::Vertical, SwipeDirection::Left | SwipeDirection::Right) => return, - (_, SwipeDirection::Left | SwipeDirection::Up) => { - self.current = (self.current + 1).min(self.pages - 1); - } - (_, SwipeDirection::Right | SwipeDirection::Down) => { - self.current = self.current.saturating_sub(1); - } - } - self.inner.change_page(self.current); - ctx.request_paint(); + fn swipe_finished(&self) -> bool { + self.transition.is_none() } } @@ -85,7 +160,6 @@ where } /// Make any component swipable by ignoring all swipe events. -#[derive(Clone)] pub struct IgnoreSwipe(T); impl IgnoreSwipe { @@ -114,12 +188,7 @@ impl Component for IgnoreSwipe { } } -impl Swipable for IgnoreSwipe { - fn can_swipe(&self, _direction: SwipeDirection) -> bool { - false - } - fn swiped(&mut self, _ctx: &mut EventCtx, _direction: SwipeDirection) {} -} +impl Swipable for IgnoreSwipe {} #[cfg(feature = "ui_debug")] impl crate::trace::Trace for IgnoreSwipe diff --git a/core/embed/rust/src/ui/flow/store.rs b/core/embed/rust/src/ui/flow/store.rs index cc116f7f9b..ec883a894a 100644 --- a/core/embed/rust/src/ui/flow/store.rs +++ b/core/embed/rust/src/ui/flow/store.rs @@ -14,9 +14,7 @@ use crate::micropython::gc::Gc; /// `FlowStore` is essentially `Vec>` except that /// `trait Component` is not object-safe so it ends up being a kind of /// recursively-defined tuple. -/// -/// Additionally the store makes it possible to make a clone of one of its -/// items, in order to make it possible to render transition animations. +/// Implementors are something like the V in MVC. pub trait FlowStore { /// Call `Component::place` on all elements. fn place(&mut self, bounds: Rect) -> Rect; @@ -34,14 +32,8 @@ pub trait FlowStore { /// Forward `Swipable` methods to i-th element. fn map_swipable(&mut self, i: usize, func: impl FnOnce(&mut dyn Swipable) -> T) -> T; - /// Make a clone of i-th element, or free all clones if None is given. - fn clone(&mut self, i: Option) -> Result<(), error::Error>; - - /// Call `Component::render` on the cloned element. - fn render_cloned<'s>(&'s self, target: &mut impl Renderer<'s>); - /// Add a Component to the end of a `FlowStore`. - fn add + MaybeTrace + Swipable + Clone>( + fn add + MaybeTrace + Swipable>( self, elem: E, ) -> Result @@ -80,12 +72,7 @@ impl FlowStore for FlowEmpty { panic!() } - fn clone(&mut self, _i: Option) -> Result<(), error::Error> { - Ok(()) - } - fn render_cloned<'s>(&'s self, _target: &mut impl Renderer<'s>) {} - - fn add + MaybeTrace + Swipable + Clone>( + fn add + MaybeTrace + Swipable>( self, elem: E, ) -> Result @@ -94,7 +81,6 @@ impl FlowStore for FlowEmpty { { Ok(FlowComponent { elem: Gc::new(elem)?, - cloned: None, next: Self, }) } @@ -104,9 +90,6 @@ struct FlowComponent, P> { /// Component allocated on micropython heap. pub elem: Gc, - /// Clone. - pub cloned: Option>, - /// Nested FlowStore. pub next: P, } @@ -125,7 +108,7 @@ impl, P> FlowComponent { impl FlowStore for FlowComponent where - E: Component + MaybeTrace + Swipable + Clone, + E: Component + MaybeTrace + Swipable, P: FlowStore, { fn place(&mut self, bounds: Rect) -> Rect { @@ -167,33 +150,7 @@ where } } - fn clone(&mut self, i: Option) -> Result<(), error::Error> { - match i { - None => { - // FIXME: how to ensure the allocation is returned? - self.cloned = None; - self.next.clone(None)? - } - Some(0) => { - self.cloned = Some(Gc::new(self.as_ref().clone())?); - self.next.clone(None)? - } - Some(i) => { - self.cloned = None; - self.next.clone(Some(i - 1))? - } - } - Ok(()) - } - - fn render_cloned<'s>(&'s self, target: &mut impl Renderer<'s>) { - if let Some(cloned) = &self.cloned { - cloned.render(target) - } - self.next.render_cloned(target); - } - - fn add + MaybeTrace + Swipable + Clone>( + fn add + MaybeTrace + Swipable>( self, elem: F, ) -> Result @@ -202,7 +159,6 @@ where { Ok(FlowComponent { elem: self.elem, - cloned: None, next: self.next.add(elem)?, }) } diff --git a/core/embed/rust/src/ui/model_mercury/component/dialog.rs b/core/embed/rust/src/ui/model_mercury/component/dialog.rs index 68cee9a3a2..55947f4abc 100644 --- a/core/embed/rust/src/ui/model_mercury/component/dialog.rs +++ b/core/embed/rust/src/ui/model_mercury/component/dialog.rs @@ -97,7 +97,6 @@ where } } -#[derive(Clone)] pub struct IconDialog { image: Child, paragraphs: Paragraphs>, diff --git a/core/embed/rust/src/ui/model_mercury/component/frame.rs b/core/embed/rust/src/ui/model_mercury/component/frame.rs index 29a0f8fe6f..3463976134 100644 --- a/core/embed/rust/src/ui/model_mercury/component/frame.rs +++ b/core/embed/rust/src/ui/model_mercury/component/frame.rs @@ -250,11 +250,15 @@ impl crate::ui::flow::Swipable for Frame where T: Component + crate::ui::flow::Swipable, { - fn can_swipe(&self, direction: crate::ui::component::SwipeDirection) -> bool { - self.inner().can_swipe(direction) + fn swipe_start( + &mut self, + ctx: &mut EventCtx, + direction: crate::ui::component::SwipeDirection, + ) -> bool { + self.update_content(ctx, |ctx, inner| inner.swipe_start(ctx, direction)) } - fn swiped(&mut self, ctx: &mut EventCtx, direction: crate::ui::component::SwipeDirection) { - self.update_content(ctx, |ctx, inner| inner.swiped(ctx, direction)) + fn swipe_finished(&self) -> bool { + self.inner().swipe_finished() } }