1
0
mirror of https://github.com/trezor/trezor-firmware.git synced 2024-11-26 01:18:28 +00:00

refactor(core): prepare for non mutable paint function

[no changelog]
This commit is contained in:
cepetr 2024-03-08 13:00:13 +01:00 committed by cepetr
parent 60333ca56c
commit dc715d7c0d
7 changed files with 68 additions and 57 deletions

View File

@ -34,7 +34,7 @@ impl<T: StringType + Clone> FormattedText<T> {
self self
} }
fn layout_content(&mut self, sink: &mut dyn LayoutSink) -> LayoutFit { pub(crate) fn layout_content(&self, sink: &mut dyn LayoutSink) -> LayoutFit {
self.op_layout self.op_layout
.layout_ops(self.char_offset, Offset::y(self.y_offset), sink) .layout_ops(self.char_offset, Offset::y(self.y_offset), sink)
} }
@ -144,20 +144,6 @@ impl<T: StringType + Clone> Component for FormattedText<T> {
// DEBUG-ONLY SECTION BELOW // DEBUG-ONLY SECTION BELOW
#[cfg(feature = "ui_debug")]
impl<T: StringType + Clone> FormattedText<T> {
/// Is the same as layout_content, but does not use `&mut self`
/// to be compatible with `trace`.
/// Therefore it has to do the `clone` of `op_layout`.
pub fn layout_content_debug(&self, sink: &mut dyn LayoutSink) -> LayoutFit {
// TODO: how to solve it "properly", without the `clone`?
// (changing `trace` to `&mut self` had some other isses...)
self.op_layout
.clone()
.layout_content(self.char_offset, sink)
}
}
#[cfg(feature = "ui_debug")] #[cfg(feature = "ui_debug")]
impl<T: StringType + Clone> crate::trace::Trace for FormattedText<T> { impl<T: StringType + Clone> crate::trace::Trace for FormattedText<T> {
fn trace(&self, t: &mut dyn crate::trace::Tracer) { fn trace(&self, t: &mut dyn crate::trace::Tracer) {
@ -166,7 +152,7 @@ impl<T: StringType + Clone> crate::trace::Trace for FormattedText<T> {
let fit: Cell<Option<LayoutFit>> = Cell::new(None); let fit: Cell<Option<LayoutFit>> = Cell::new(None);
t.component("FormattedText"); t.component("FormattedText");
t.in_list("text", &|l| { t.in_list("text", &|l| {
let result = self.layout_content_debug(&mut TraceSink(l)); let result = self.layout_content(&mut TraceSink(l));
fit.set(Some(result)); fit.set(Some(result));
}); });
t.bool("fits", matches!(fit.get(), Some(LayoutFit::Fitting { .. }))); t.bool("fits", matches!(fit.get(), Some(LayoutFit::Fitting { .. })));

View File

@ -56,7 +56,7 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
/// Perform some operations defined on `Op` for a list of those `Op`s /// Perform some operations defined on `Op` for a list of those `Op`s
/// - e.g. changing the color, changing the font or rendering the text. /// - e.g. changing the color, changing the font or rendering the text.
pub fn layout_ops( pub fn layout_ops(
&mut self, &self,
skip_bytes: usize, skip_bytes: usize,
offset: Offset, offset: Offset,
sink: &mut dyn LayoutSink, sink: &mut dyn LayoutSink,
@ -65,25 +65,26 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
let cursor = &mut (self.layout.initial_cursor() + offset); let cursor = &mut (self.layout.initial_cursor() + offset);
let init_cursor = *cursor; let init_cursor = *cursor;
let mut total_processed_chars = 0; let mut total_processed_chars = 0;
let mut layout = self.layout;
// Do something when it was not skipped // Do something when it was not skipped
for op in Self::filter_skipped_ops(self.ops.iter(), skip_bytes) { for op in Self::filter_skipped_ops(self.ops.iter(), skip_bytes) {
match op { match op {
// Changing color // Changing color
Op::Color(color) => { Op::Color(color) => {
self.layout.style.text_color = color; layout.style.text_color = color;
} }
// Changing font // Changing font
Op::Font(font) => { Op::Font(font) => {
self.layout.style.text_font = font; layout.style.text_font = font;
} }
// Changing line/text alignment // Changing line/text alignment
Op::Alignment(line_alignment) => { Op::Alignment(line_alignment) => {
self.layout.align = line_alignment; layout.align = line_alignment;
} }
// Changing line breaking // Changing line breaking
Op::LineBreaking(line_breaking) => { Op::LineBreaking(line_breaking) => {
self.layout.style.line_breaking = line_breaking; layout.style.line_breaking = line_breaking;
} }
// Moving the cursor // Moving the cursor
Op::CursorOffset(offset) => { Op::CursorOffset(offset) => {
@ -91,10 +92,10 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
cursor.y += offset.y; cursor.y += offset.y;
} }
Op::Chunkify(chunks) => { Op::Chunkify(chunks) => {
self.layout.style.chunks = chunks; layout.style.chunks = chunks;
} }
Op::LineSpacing(line_spacing) => { Op::LineSpacing(line_spacing) => {
self.layout.style.line_spacing = line_spacing; layout.style.line_spacing = line_spacing;
} }
// Moving to the next page // Moving to the next page
Op::NextPage => { Op::NextPage => {
@ -103,7 +104,7 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
total_processed_chars += PROCESSED_CHARS_ONE; total_processed_chars += PROCESSED_CHARS_ONE;
return LayoutFit::OutOfBounds { return LayoutFit::OutOfBounds {
processed_chars: total_processed_chars, processed_chars: total_processed_chars,
height: self.layout.layout_height(init_cursor, *cursor), height: layout.layout_height(init_cursor, *cursor),
}; };
} }
// Drawing text // Drawing text
@ -113,9 +114,9 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
// Inserting the ellipsis at the very beginning of the text if needed // Inserting the ellipsis at the very beginning of the text if needed
// (just for incomplete texts that were separated) // (just for incomplete texts that were separated)
self.layout.continues_from_prev_page = continued; layout.continues_from_prev_page = continued;
let fit = self.layout.layout_text(text.as_ref(), cursor, sink); let fit = layout.layout_text(text.as_ref(), cursor, sink);
match fit { match fit {
LayoutFit::Fitting { LayoutFit::Fitting {
@ -130,7 +131,7 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
return LayoutFit::OutOfBounds { return LayoutFit::OutOfBounds {
processed_chars: total_processed_chars, processed_chars: total_processed_chars,
height: self.layout.layout_height(init_cursor, *cursor), height: layout.layout_height(init_cursor, *cursor),
}; };
} }
} }
@ -140,7 +141,7 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout<T> {
LayoutFit::Fitting { LayoutFit::Fitting {
processed_chars: total_processed_chars, processed_chars: total_processed_chars,
height: self.layout.layout_height(init_cursor, *cursor), height: layout.layout_height(init_cursor, *cursor),
} }
} }

View File

@ -97,7 +97,7 @@ where
btn_actions: ButtonActions, btn_actions: ButtonActions,
formatted: FormattedText<T>, formatted: FormattedText<T>,
) -> Self { ) -> Self {
Self { let mut page = Self {
formatted, formatted,
btn_layout, btn_layout,
btn_actions, btn_actions,
@ -105,7 +105,9 @@ where
page_count: 1, page_count: 1,
title: None, title: None,
slim_arrows: false, slim_arrows: false,
} };
page.change_page(page.current_page);
page
} }
} }
@ -127,7 +129,6 @@ where
} }
pub fn paint(&mut self) { pub fn paint(&mut self) {
self.change_page(self.current_page);
self.formatted.paint(); self.formatted.paint();
} }
@ -193,10 +194,12 @@ where
pub fn go_to_prev_page(&mut self) { pub fn go_to_prev_page(&mut self) {
self.current_page -= 1; self.current_page -= 1;
self.change_page(self.current_page);
} }
pub fn go_to_next_page(&mut self) { pub fn go_to_next_page(&mut self) {
self.current_page += 1; self.current_page += 1;
self.change_page(self.current_page);
} }
pub fn get_current_page(&self) -> usize { pub fn get_current_page(&self) -> usize {
@ -240,7 +243,7 @@ where
t.int("active_page", self.current_page as i64); t.int("active_page", self.current_page as i64);
t.int("page_count", self.page_count as i64); t.int("page_count", self.page_count as i64);
t.in_list("text", &|l| { t.in_list("text", &|l| {
let result = self.formatted.layout_content_debug(&mut TraceSink(l)); let result = self.formatted.layout_content(&mut TraceSink(l));
fit.set(Some(result)); fit.set(Some(result));
}); });
} }

View File

@ -294,11 +294,6 @@ where
fn show_current_choice(&mut self, area: Rect) { fn show_current_choice(&mut self, area: Rect) {
self.get_current_item() self.get_current_item()
.paint_center(area, self.inverse_selected_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. /// Display all the choices fitting on the left side.
@ -491,6 +486,11 @@ where
} }
fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option<Self::Msg> { fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option<Self::Msg> {
// Cancel highlighting of the current choice.
// The Highlighting is started by pressing the middle button and
// canceled immediately when any other event is processed
self.inverse_selected_item = false;
// Possible animation movement when setting (randomizing) the page counter. // Possible animation movement when setting (randomizing) the page counter.
if let Some(animation_direction) = self.animation_event(ctx, event) { if let Some(animation_direction) = self.animation_event(ctx, event) {
match animation_direction { match animation_direction {

View File

@ -10,6 +10,7 @@ use crate::ui::{
}; };
use super::CancelConfirmMsg; use super::CancelConfirmMsg;
use core::cell::Cell;
const ICON_HEIGHT: i16 = 70; const ICON_HEIGHT: i16 = 70;
const SCROLLBAR_INSET_TOP: i16 = 5; const SCROLLBAR_INSET_TOP: i16 = 5;
@ -30,7 +31,7 @@ pub struct FidoConfirm<F: Fn(usize) -> T, T, U> {
/// Function/closure that will return appropriate page on demand. /// Function/closure that will return appropriate page on demand.
get_account: F, get_account: F,
scrollbar: ScrollBar, scrollbar: ScrollBar,
fade: bool, fade: Cell<bool>,
controls: U, controls: U,
} }
@ -59,14 +60,27 @@ where
page_swipe.allow_right = scrollbar.has_previous_page(); page_swipe.allow_right = scrollbar.has_previous_page();
page_swipe.allow_left = scrollbar.has_next_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 { Self {
app_name: Label::centered(app_name, theme::TEXT_DEMIBOLD), 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, page_swipe,
icon: Child::new(Image::new(icon_data)), icon: Child::new(Image::new(icon_data)),
get_account, get_account,
scrollbar, scrollbar,
fade: false, fade: Cell::new(false),
controls, controls,
} }
} }
@ -87,11 +101,14 @@ where
self.page_swipe.allow_right = self.scrollbar.has_previous_page(); self.page_swipe.allow_right = self.scrollbar.has_previous_page();
self.page_swipe.allow_left = self.scrollbar.has_next_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. // Redraw the page.
ctx.request_paint(); ctx.request_paint();
// Reset backlight to normal level on next paint. // Reset backlight to normal level on next paint.
self.fade = true; self.fade.set(true);
} }
fn active_page(&self) -> usize { fn active_page(&self) -> usize {
@ -139,6 +156,12 @@ where
self.app_name.place(app_name_area); self.app_name.place(app_name_area);
self.account_name.place(account_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 bounds
} }
@ -166,8 +189,6 @@ where
self.scrollbar.paint(); self.scrollbar.paint();
} }
let current_account = (self.get_account)(self.active_page());
// Erasing the old text content before writing the new one. // Erasing the old text content before writing the new one.
let account_name_area = self.account_name.area(); let account_name_area = self.account_name.area();
let real_area = account_name_area let real_area = account_name_area
@ -177,15 +198,13 @@ where
// Account name is optional. // Account name is optional.
// Showing it only if it differs from app name. // Showing it only if it differs from app name.
// (Dummy requests usually have some text as both app_name and account_name.) // (Dummy requests usually have some text as both app_name and account_name.)
if !current_account.as_ref().is_empty() let account_name = self.account_name.text().as_ref();
&& current_account.as_ref() != self.app_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.set_text(current_account);
self.account_name.paint(); self.account_name.paint();
} }
if self.fade { if self.fade.take() {
self.fade = false;
// Note that this is blocking and takes some time. // Note that this is blocking and takes some time.
display::fade_backlight(theme::BACKLIGHT_NORMAL); display::fade_backlight(theme::BACKLIGHT_NORMAL);
} }

View File

@ -13,6 +13,8 @@ use crate::ui::{
util::long_line_content_with_ellipsis, util::long_line_content_with_ellipsis,
}; };
use core::cell::Cell;
pub enum PassphraseKeyboardMsg { pub enum PassphraseKeyboardMsg {
Confirmed, Confirmed,
Cancelled, Cancelled,
@ -25,7 +27,7 @@ pub struct PassphraseKeyboard {
confirm: Child<Button<&'static str>>, confirm: Child<Button<&'static str>>,
keys: [Child<Button<&'static str>>; KEY_COUNT], keys: [Child<Button<&'static str>>; KEY_COUNT],
scrollbar: ScrollBar, scrollbar: ScrollBar,
fade: bool, fade: Cell<bool>,
} }
const STARTING_PAGE: usize = 1; const STARTING_PAGE: usize = 1;
@ -63,7 +65,7 @@ impl PassphraseKeyboard {
Child::new(Button::new(Self::key_content(text)).styled(theme::button_pin())) Child::new(Button::new(Self::key_content(text)).styled(theme::button_pin()))
}), }),
scrollbar: ScrollBar::horizontal(), scrollbar: ScrollBar::horizontal(),
fade: false, fade: Cell::new(false),
} }
} }
@ -99,7 +101,7 @@ impl PassphraseKeyboard {
// Update buttons. // Update buttons.
self.replace_button_content(ctx, key_page); self.replace_button_content(ctx, key_page);
// Reset backlight to normal level on next paint. // 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 // So that swipe does not visually enable the input buttons when max length
// reached // reached
self.update_input_btns_state(ctx); self.update_input_btns_state(ctx);
@ -288,8 +290,7 @@ impl Component for PassphraseKeyboard {
for btn in &mut self.keys { for btn in &mut self.keys {
btn.paint(); btn.paint();
} }
if self.fade { if self.fade.take() {
self.fade = false;
// Note that this is blocking and takes some time. // Note that this is blocking and takes some time.
display::fade_backlight(theme::BACKLIGHT_NORMAL); display::fade_backlight(theme::BACKLIGHT_NORMAL);
} }

View File

@ -5,6 +5,7 @@ use crate::ui::{
}; };
use super::{theme, ScrollBar, Swipe, SwipeDirection}; use super::{theme, ScrollBar, Swipe, SwipeDirection};
use core::cell::Cell;
const SCROLLBAR_HEIGHT: i16 = 18; const SCROLLBAR_HEIGHT: i16 = 18;
const SCROLLBAR_BORDER: i16 = 4; const SCROLLBAR_BORDER: i16 = 4;
@ -16,7 +17,7 @@ pub struct SimplePage<T> {
scrollbar: ScrollBar, scrollbar: ScrollBar,
axis: Axis, axis: Axis,
swipe_right_to_go_back: bool, swipe_right_to_go_back: bool,
fade: Option<u16>, fade: Cell<Option<u16>>,
} }
impl<T> SimplePage<T> impl<T> SimplePage<T>
@ -32,7 +33,7 @@ where
scrollbar: ScrollBar::new(axis), scrollbar: ScrollBar::new(axis),
axis, axis,
swipe_right_to_go_back: false, 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 // Swipe has dimmed the screen, so fade back to normal backlight after the next
// paint. // paint.
self.fade = Some(theme::BACKLIGHT_NORMAL); self.fade.set(Some(theme::BACKLIGHT_NORMAL));
} }
fn is_horizontal(&self) -> bool { fn is_horizontal(&self) -> bool {