From 4472a363eace1045248ef732e32d55388643d6e7 Mon Sep 17 00:00:00 2001 From: cepetr Date: Fri, 8 Mar 2024 13:00:13 +0100 Subject: [PATCH] refactor(core): prepare for non mutable paint function [no changelog] --- .../rust/src/ui/component/text/formatted.rs | 16 ++++--- .../src/ui/model_tr/component/flow_pages.rs | 9 ++-- .../component/input_methods/choice.rs | 12 +++--- .../rust/src/ui/model_tt/component/fido.rs | 43 +++++++++++++------ .../model_tt/component/keyboard/passphrase.rs | 11 ++--- .../src/ui/model_tt/component/simple_page.rs | 7 +-- 6 files changed, 64 insertions(+), 34 deletions(-) diff --git a/core/embed/rust/src/ui/component/text/formatted.rs b/core/embed/rust/src/ui/component/text/formatted.rs index 14a1b7f8f..aa360a4e2 100644 --- a/core/embed/rust/src/ui/component/text/formatted.rs +++ b/core/embed/rust/src/ui/component/text/formatted.rs @@ -11,9 +11,11 @@ use super::{ op::OpTextLayout, }; +use core::cell::RefCell; + #[derive(Clone)] pub struct FormattedText { - op_layout: OpTextLayout, + op_layout: RefCell>, vertical: Alignment, char_offset: usize, y_offset: i16, @@ -22,7 +24,7 @@ pub struct FormattedText { impl FormattedText { pub fn new(op_layout: OpTextLayout) -> Self { Self { - op_layout, + op_layout: RefCell::new(op_layout), vertical: Alignment::Start, char_offset: 0, y_offset: 0, @@ -34,13 +36,14 @@ impl FormattedText { self } - fn layout_content(&mut self, sink: &mut dyn LayoutSink) -> LayoutFit { + fn layout_content(&self, sink: &mut dyn LayoutSink) -> LayoutFit { self.op_layout + .borrow_mut() .layout_ops(self.char_offset, Offset::y(self.y_offset), sink) } fn align_vertically(&mut self, content_height: i16) { - let bounds_height = self.op_layout.layout.bounds.height(); + let bounds_height = self.op_layout.borrow().layout.bounds.height(); if content_height >= bounds_height { self.y_offset = 0; return; @@ -122,7 +125,7 @@ impl Component for FormattedText { type Msg = Never; fn place(&mut self, bounds: Rect) -> Rect { - self.op_layout.place(bounds); + self.op_layout.borrow_mut().place(bounds); let height = self.layout_content(&mut TextNoOp).height(); self.align_vertically(height); bounds @@ -138,7 +141,7 @@ impl Component for FormattedText { #[cfg(feature = "ui_bounds")] fn bounds(&self, sink: &mut dyn FnMut(Rect)) { - sink(self.op_layout.layout.bounds) + sink(self.op_layout.borrow().layout.bounds) } } @@ -153,6 +156,7 @@ impl FormattedText { // TODO: how to solve it "properly", without the `clone`? // (changing `trace` to `&mut self` had some other isses...) self.op_layout + .borrow() .clone() .layout_content(self.char_offset, sink) } diff --git a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs index 5ac8e7691..62f8b7314 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs @@ -97,7 +97,7 @@ where btn_actions: ButtonActions, formatted: FormattedText, ) -> Self { - Self { + let mut page = Self { formatted, btn_layout, btn_actions, @@ -105,7 +105,9 @@ where page_count: 1, title: None, slim_arrows: false, - } + }; + page.change_page(page.current_page); + page } } @@ -127,7 +129,6 @@ where } pub fn paint(&mut self) { - self.change_page(self.current_page); self.formatted.paint(); } @@ -193,10 +194,12 @@ where pub fn go_to_prev_page(&mut self) { self.current_page -= 1; + self.change_page(self.current_page); } pub fn go_to_next_page(&mut self) { self.current_page += 1; + self.change_page(self.current_page); } pub fn get_current_page(&self) -> usize { diff --git a/core/embed/rust/src/ui/model_tr/component/input_methods/choice.rs b/core/embed/rust/src/ui/model_tr/component/input_methods/choice.rs index cd46425e1..e2d7709d9 100644 --- a/core/embed/rust/src/ui/model_tr/component/input_methods/choice.rs +++ b/core/embed/rust/src/ui/model_tr/component/input_methods/choice.rs @@ -294,11 +294,6 @@ where fn show_current_choice(&mut self, area: Rect) { self.get_current_item() .paint_center(area, self.inverse_selected_item); - - // Color inversion is just one-time thing. - if self.inverse_selected_item { - self.inverse_selected_item = false; - } } /// Display all the choices fitting on the left side. @@ -491,6 +486,13 @@ where } fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { + // Cancel highlighting of the current choice. + // The Highlighting is started by pressing the middle button and + // canceled immediately when any other event is processed + if self.inverse_selected_item { + self.inverse_selected_item = false; + } + // Possible animation movement when setting (randomizing) the page counter. if let Some(animation_direction) = self.animation_event(ctx, event) { match animation_direction { diff --git a/core/embed/rust/src/ui/model_tt/component/fido.rs b/core/embed/rust/src/ui/model_tt/component/fido.rs index 50ce240c0..3b7f64caa 100644 --- a/core/embed/rust/src/ui/model_tt/component/fido.rs +++ b/core/embed/rust/src/ui/model_tt/component/fido.rs @@ -10,6 +10,7 @@ use crate::ui::{ }; use super::CancelConfirmMsg; +use core::cell::Cell; const ICON_HEIGHT: i16 = 70; const SCROLLBAR_INSET_TOP: i16 = 5; @@ -30,7 +31,7 @@ pub struct FidoConfirm T, T, U> { /// Function/closure that will return appropriate page on demand. get_account: F, scrollbar: ScrollBar, - fade: bool, + fade: Cell, controls: U, } @@ -59,14 +60,27 @@ where page_swipe.allow_right = scrollbar.has_previous_page(); page_swipe.allow_left = scrollbar.has_next_page(); + // NOTE: This is an ugly hotfix for the erroneous behavior of + // TextLayout used in the account_name Label. In this + // particular case, TextLayout calculates the wrong height of + // fitted text that's higher than the TextLayout bound itself. + // + // The following two lines should be swapped when the problem with + // TextLayout is fixed. + // + // See also, continuation of this hotfix in the place() function. + + // let current_account = get_account(scrollbar.active_page); + let current_account = "".into(); + Self { app_name: Label::centered(app_name, theme::TEXT_DEMIBOLD), - account_name: Label::centered("".into(), theme::TEXT_DEMIBOLD), + account_name: Label::centered(current_account, theme::TEXT_DEMIBOLD), page_swipe, icon: Child::new(Image::new(icon_data)), get_account, scrollbar, - fade: false, + fade: Cell::new(false), controls, } } @@ -87,11 +101,14 @@ where self.page_swipe.allow_right = self.scrollbar.has_previous_page(); self.page_swipe.allow_left = self.scrollbar.has_next_page(); + let current_account = (self.get_account)(self.active_page()); + self.account_name.set_text(current_account); + // Redraw the page. ctx.request_paint(); // Reset backlight to normal level on next paint. - self.fade = true; + self.fade.set(true); } fn active_page(&self) -> usize { @@ -139,6 +156,12 @@ where self.app_name.place(app_name_area); self.account_name.place(account_name_area); + // NOTE: This is a hotfix used due to the erroneous behavior of TextLayout. + // This line should be removed when the problem with TextLayout is fixed. + // See also the code for FidoConfirm::new(). + self.account_name + .set_text((self.get_account)(self.scrollbar.active_page)); + bounds } @@ -166,8 +189,6 @@ where self.scrollbar.paint(); } - let current_account = (self.get_account)(self.active_page()); - // Erasing the old text content before writing the new one. let account_name_area = self.account_name.area(); let real_area = account_name_area @@ -177,15 +198,13 @@ where // Account name is optional. // Showing it only if it differs from app name. // (Dummy requests usually have some text as both app_name and account_name.) - if !current_account.as_ref().is_empty() - && current_account.as_ref() != self.app_name.text().as_ref() - { - self.account_name.set_text(current_account); + let account_name = self.account_name.text().as_ref(); + let app_name = self.app_name.text().as_ref(); + if !account_name.is_empty() && account_name != app_name { self.account_name.paint(); } - if self.fade { - self.fade = false; + if self.fade.take() { // Note that this is blocking and takes some time. display::fade_backlight(theme::BACKLIGHT_NORMAL); } diff --git a/core/embed/rust/src/ui/model_tt/component/keyboard/passphrase.rs b/core/embed/rust/src/ui/model_tt/component/keyboard/passphrase.rs index 133c55b92..ab74d7a69 100644 --- a/core/embed/rust/src/ui/model_tt/component/keyboard/passphrase.rs +++ b/core/embed/rust/src/ui/model_tt/component/keyboard/passphrase.rs @@ -13,6 +13,8 @@ use crate::ui::{ util::long_line_content_with_ellipsis, }; +use core::cell::Cell; + pub enum PassphraseKeyboardMsg { Confirmed, Cancelled, @@ -25,7 +27,7 @@ pub struct PassphraseKeyboard { confirm: Child>, keys: [Child>; KEY_COUNT], scrollbar: ScrollBar, - fade: bool, + fade: Cell, } const STARTING_PAGE: usize = 1; @@ -63,7 +65,7 @@ impl PassphraseKeyboard { Child::new(Button::new(Self::key_content(text)).styled(theme::button_pin())) }), scrollbar: ScrollBar::horizontal(), - fade: false, + fade: Cell::new(false), } } @@ -99,7 +101,7 @@ impl PassphraseKeyboard { // Update buttons. self.replace_button_content(ctx, key_page); // Reset backlight to normal level on next paint. - self.fade = true; + self.fade.set(true); // So that swipe does not visually enable the input buttons when max length // reached self.update_input_btns_state(ctx); @@ -288,8 +290,7 @@ impl Component for PassphraseKeyboard { for btn in &mut self.keys { btn.paint(); } - if self.fade { - self.fade = false; + if self.fade.take() { // Note that this is blocking and takes some time. display::fade_backlight(theme::BACKLIGHT_NORMAL); } diff --git a/core/embed/rust/src/ui/model_tt/component/simple_page.rs b/core/embed/rust/src/ui/model_tt/component/simple_page.rs index 73ed3048f..92bb4c53a 100644 --- a/core/embed/rust/src/ui/model_tt/component/simple_page.rs +++ b/core/embed/rust/src/ui/model_tt/component/simple_page.rs @@ -5,6 +5,7 @@ use crate::ui::{ }; use super::{theme, ScrollBar, Swipe, SwipeDirection}; +use core::cell::Cell; const SCROLLBAR_HEIGHT: i16 = 18; const SCROLLBAR_BORDER: i16 = 4; @@ -16,7 +17,7 @@ pub struct SimplePage { scrollbar: ScrollBar, axis: Axis, swipe_right_to_go_back: bool, - fade: Option, + fade: Cell>, } impl SimplePage @@ -32,7 +33,7 @@ where scrollbar: ScrollBar::new(axis), axis, swipe_right_to_go_back: false, - fade: None, + fade: Cell::new(None), } } @@ -79,7 +80,7 @@ where // Swipe has dimmed the screen, so fade back to normal backlight after the next // paint. - self.fade = Some(theme::BACKLIGHT_NORMAL); + self.fade.set(Some(theme::BACKLIGHT_NORMAL)); } fn is_horizontal(&self) -> bool {