From 4622aec0f182694d43caa7422dccf70a010814bf Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Thu, 16 Feb 2023 19:55:34 +0100 Subject: [PATCH] refactor(core): switch to Rust implementation of QR-Code-generator [no changelog] --- core/SConscript.firmware | 1 - core/SConscript.prodtest | 1 + core/SConscript.unix | 1 - core/embed/extmod/modtrezorui/display.c | 7 +- .../extmod/modtrezorui/modtrezorui-display.h | 23 --- core/embed/rust/Cargo.lock | 5 + core/embed/rust/Cargo.toml | 3 + core/embed/rust/src/trezorhal/mod.rs | 1 - core/embed/rust/src/trezorhal/qr.rs | 75 --------- core/embed/rust/src/ui/component/mod.rs | 4 +- core/embed/rust/src/ui/component/painter.rs | 11 -- core/embed/rust/src/ui/component/qr_code.rs | 156 ++++++++++++++++++ core/embed/rust/src/ui/display/mod.rs | 7 +- core/embed/rust/src/ui/model_tt/layout.rs | 13 +- core/mocks/generated/trezorui.pyi | 6 - core/tests/test_trezor.ui.display.py | 3 - vendor/QR-Code-generator | 2 +- 17 files changed, 183 insertions(+), 136 deletions(-) delete mode 100644 core/embed/rust/src/trezorhal/qr.rs create mode 100644 core/embed/rust/src/ui/component/qr_code.rs diff --git a/core/SConscript.firmware b/core/SConscript.firmware index 06022f6fe..0226fc18d 100644 --- a/core/SConscript.firmware +++ b/core/SConscript.firmware @@ -181,7 +181,6 @@ SOURCE_MOD += [ 'embed/extmod/modtrezorui/fonts/fonts.c', 'embed/extmod/modtrezorui/fonts/font_bitmap.c', 'embed/extmod/modtrezorui/modtrezorui.c', - 'embed/extmod/modtrezorui/qr-code-generator/qrcodegen.c', 'vendor/micropython/lib/uzlib/adler32.c', 'vendor/micropython/lib/uzlib/crc32.c', 'vendor/micropython/lib/uzlib/tinflate.c', diff --git a/core/SConscript.prodtest b/core/SConscript.prodtest index b5fb76879..d2d401135 100644 --- a/core/SConscript.prodtest +++ b/core/SConscript.prodtest @@ -155,6 +155,7 @@ env.Replace( 'vendor/micropython/lib/cmsis/inc', ] + CPPPATH_MOD, CPPDEFINES=[ + 'TREZOR_PRODTEST', 'TREZOR_MODEL_'+TREZOR_MODEL, CPU_MODEL, 'USE_HAL_DRIVER', diff --git a/core/SConscript.unix b/core/SConscript.unix index dad147417..09c5964af 100644 --- a/core/SConscript.unix +++ b/core/SConscript.unix @@ -175,7 +175,6 @@ SOURCE_MOD += [ 'embed/extmod/modtrezorui/fonts/fonts.c', 'embed/extmod/modtrezorui/fonts/font_bitmap.c', 'embed/extmod/modtrezorui/modtrezorui.c', - 'embed/extmod/modtrezorui/qr-code-generator/qrcodegen.c', 'vendor/micropython/lib/uzlib/adler32.c', 'vendor/micropython/lib/uzlib/crc32.c', 'vendor/micropython/lib/uzlib/tinflate.c', diff --git a/core/embed/extmod/modtrezorui/display.c b/core/embed/extmod/modtrezorui/display.c index b848fc5ba..f1203f6b0 100644 --- a/core/embed/extmod/modtrezorui/display.c +++ b/core/embed/extmod/modtrezorui/display.c @@ -19,8 +19,6 @@ #define _GNU_SOURCE -#include "qr-code-generator/qrcodegen.h" - #include "uzlib.h" #include "buffers.h" @@ -868,6 +866,9 @@ int display_text_split(const char *text, int textlen, int font, return textlen; } +#ifdef TREZOR_PRODTEST + +#include "qr-code-generator/qrcodegen.h" #define QR_MAX_VERSION 9 void display_qrcode(int x, int y, const char *data, uint8_t scale) { @@ -908,6 +909,8 @@ void display_qrcode(int x, int y, const char *data, uint8_t scale) { PIXELDATA_DIRTY(); } +#endif + void display_offset(int set_xy[2], int *get_x, int *get_y) { if (set_xy) { DISPLAY_OFFSET.x = set_xy[0]; diff --git a/core/embed/extmod/modtrezorui/modtrezorui-display.h b/core/embed/extmod/modtrezorui/modtrezorui-display.h index 9625b59d8..2c633f5ef 100644 --- a/core/embed/extmod/modtrezorui/modtrezorui-display.h +++ b/core/embed/extmod/modtrezorui/modtrezorui-display.h @@ -479,28 +479,6 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorui_Display_text_split_obj, 4, 4, mod_trezorui_Display_text_split); -/// def qrcode(self, x: int, y: int, data: str, scale: int) -> None: -/// """ -/// Renders data encoded as a QR code centered at position (x,y). -/// Scale determines a zoom factor. -/// """ -STATIC mp_obj_t mod_trezorui_Display_qrcode(size_t n_args, - const mp_obj_t *args) { - mp_int_t x = mp_obj_get_int(args[1]); - mp_int_t y = mp_obj_get_int(args[2]); - mp_int_t scale = mp_obj_get_int(args[4]); - if (scale < 1 || scale > 10) { - mp_raise_ValueError("Scale has to be between 1 and 10"); - } - const char *data = mp_obj_str_get_str(args[3]); - if (data) { - display_qrcode(x, y, data, scale); - } - return mp_const_none; -} -STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorui_Display_qrcode_obj, 5, - 5, mod_trezorui_Display_qrcode); - /// def orientation(self, degrees: int | None = None) -> int: /// """ /// Sets display orientation to 0, 90, 180 or 270 degrees. @@ -631,7 +609,6 @@ STATIC const mp_rom_map_elem_t mod_trezorui_Display_locals_dict_table[] = { MP_ROM_PTR(&mod_trezorui_Display_text_width_obj)}, {MP_ROM_QSTR(MP_QSTR_text_split), MP_ROM_PTR(&mod_trezorui_Display_text_split_obj)}, - {MP_ROM_QSTR(MP_QSTR_qrcode), MP_ROM_PTR(&mod_trezorui_Display_qrcode_obj)}, {MP_ROM_QSTR(MP_QSTR_orientation), MP_ROM_PTR(&mod_trezorui_Display_orientation_obj)}, {MP_ROM_QSTR(MP_QSTR_backlight), diff --git a/core/embed/rust/Cargo.lock b/core/embed/rust/Cargo.lock index 27965aa7e..34a4c8e76 100644 --- a/core/embed/rust/Cargo.lock +++ b/core/embed/rust/Cargo.lock @@ -207,6 +207,10 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "qrcodegen" +version = "1.8.0" + [[package]] name = "quote" version = "1.0.9" @@ -287,6 +291,7 @@ dependencies = [ "heapless", "num-derive", "num-traits", + "qrcodegen", ] [[package]] diff --git a/core/embed/rust/Cargo.toml b/core/embed/rust/Cargo.toml index c3ea2e855..9c191f76f 100644 --- a/core/embed/rust/Cargo.toml +++ b/core/embed/rust/Cargo.toml @@ -43,6 +43,9 @@ debug = 2 split-debuginfo = "off" debug = 2 +[dependencies] +qrcodegen = { version = "1.8.0", path = "../../vendor/QR-Code-generator/rust-no-heap" } + # Runtime dependencies [dependencies.cty] diff --git a/core/embed/rust/src/trezorhal/mod.rs b/core/embed/rust/src/trezorhal/mod.rs index 3a2e7002f..a3730ed73 100644 --- a/core/embed/rust/src/trezorhal/mod.rs +++ b/core/embed/rust/src/trezorhal/mod.rs @@ -8,7 +8,6 @@ pub mod display; pub mod dma2d; mod ffi; pub mod io; -pub mod qr; pub mod random; #[cfg(feature = "model_tr")] pub mod rgb_led; diff --git a/core/embed/rust/src/trezorhal/qr.rs b/core/embed/rust/src/trezorhal/qr.rs deleted file mode 100644 index c4ed961b1..000000000 --- a/core/embed/rust/src/trezorhal/qr.rs +++ /dev/null @@ -1,75 +0,0 @@ -use crate::error::Error; - -use cstr_core::CStr; - -extern "C" { - fn display_qrcode( - x: cty::c_int, - y: cty::c_int, - data: *const cty::uint16_t, - scale: cty::uint8_t, - ); -} - -const NVERSIONS: usize = 10; // range of versions (=capacities) that we support -const QR_WIDTHS: [u32; NVERSIONS] = [21, 25, 29, 33, 37, 41, 45, 49, 53, 57]; -const THRESHOLDS_BINARY: [usize; NVERSIONS] = [14, 26, 42, 62, 84, 106, 122, 152, 180, 213]; -const THRESHOLDS_ALPHANUM: [usize; NVERSIONS] = [20, 38, 61, 90, 122, 154, 178, 221, 262, 311]; -const ALPHANUM: &str = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ $*+-./:"; - -const MAX_DATA: usize = THRESHOLDS_ALPHANUM[THRESHOLDS_ALPHANUM.len() - 1] + 1; //FIXME - -fn is_alphanum_only(data: &str) -> bool { - data.chars().all(|c| ALPHANUM.contains(c)) -} - -fn qr_version_index(data: &str, thresholds: &[usize]) -> Option { - for (i, threshold) in thresholds.iter().enumerate() { - if data.len() <= *threshold { - return Some(i); - } - } - None -} - -pub fn render_qrcode( - x: i16, - y: i16, - data: &str, - max_size: u32, - case_sensitive: bool, -) -> Result<(), Error> { - let data_len = data.len(); - let version_idx; - let mut buffer = [0u8; MAX_DATA]; - assert!(data_len < buffer.len()); - buffer.as_mut_slice()[..data_len].copy_from_slice(data.as_bytes()); - - if case_sensitive && !is_alphanum_only(data) { - version_idx = match qr_version_index(data, &THRESHOLDS_BINARY) { - Some(idx) => idx, - _ => return Err(Error::OutOfRange), - } - } else if let Some(idx) = qr_version_index(data, &THRESHOLDS_ALPHANUM) { - version_idx = idx; - if data_len > THRESHOLDS_BINARY[idx] { - for c in buffer.iter_mut() { - c.make_ascii_uppercase() - } - }; - } else { - return Err(Error::OutOfRange); - } - - let size = QR_WIDTHS[version_idx]; - let scale = max_size / size; - assert!((1..=10).contains(&scale)); - - unsafe { - buffer[data_len] = 0u8; - let cstr = CStr::from_bytes_with_nul_unchecked(&buffer[..data_len + 1]); - - display_qrcode(x.into(), y.into(), cstr.as_ptr() as _, scale as u8); - Ok(()) - } -} diff --git a/core/embed/rust/src/ui/component/mod.rs b/core/embed/rust/src/ui/component/mod.rs index 96aa50b37..f4d211268 100644 --- a/core/embed/rust/src/ui/component/mod.rs +++ b/core/embed/rust/src/ui/component/mod.rs @@ -12,6 +12,7 @@ pub mod pad; pub mod paginated; pub mod painter; pub mod placed; +pub mod qr_code; pub mod text; pub mod timeout; @@ -24,8 +25,9 @@ pub use marquee::Marquee; pub use maybe::Maybe; pub use pad::Pad; pub use paginated::{PageMsg, Paginate}; -pub use painter::{qrcode_painter, Painter}; +pub use painter::Painter; pub use placed::{FixedHeightBar, GridPlaced}; +pub use qr_code::Qr; pub use text::{ formatted::FormattedText, layout::{LineBreaking, PageBreaking, TextLayout}, diff --git a/core/embed/rust/src/ui/component/painter.rs b/core/embed/rust/src/ui/component/painter.rs index 6c6569d4c..72650b385 100644 --- a/core/embed/rust/src/ui/component/painter.rs +++ b/core/embed/rust/src/ui/component/painter.rs @@ -51,17 +51,6 @@ impl crate::trace::Trace for Painter { } } -pub fn qrcode_painter(data: T, max_size: u32, case_sensitive: bool) -> Painter -where - T: AsRef, -{ - // Ignore errors as we currently can't propagate them out of paint(). - let f = move |area: Rect| { - display::qrcode(area.center(), data.as_ref(), max_size, case_sensitive).unwrap_or(()) - }; - Painter::new(f) -} - pub fn image_painter(image: Image) -> Painter { let f = move |area: Rect| image.draw(area.center(), CENTER); Painter::new(f) diff --git a/core/embed/rust/src/ui/component/qr_code.rs b/core/embed/rust/src/ui/component/qr_code.rs new file mode 100644 index 000000000..5b8d5e493 --- /dev/null +++ b/core/embed/rust/src/ui/component/qr_code.rs @@ -0,0 +1,156 @@ +use heapless::String; +use qrcodegen::{QrCode, QrCodeEcc, Version}; + +use crate::{ + error::Error, + ui::{ + component::{Component, Event, EventCtx, Never}, + constant, + display::{pixeldata, pixeldata_dirty, rect_fill_rounded, set_window, Color}, + geometry::{Insets, Offset, Rect}, + }, +}; + +const NVERSIONS: usize = 10; // range of versions (=capacities) that we support +const THRESHOLDS_BINARY: [usize; NVERSIONS] = [14, 26, 42, 62, 84, 106, 122, 152, 180, 213]; +const THRESHOLDS_ALPHANUM: [usize; NVERSIONS] = [20, 38, 61, 90, 122, 154, 178, 221, 262, 311]; +const ALPHANUMERIC_CHARSET: &str = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ $%*+-./:"; +const MAX_DATA: usize = THRESHOLDS_ALPHANUM[THRESHOLDS_ALPHANUM.len() - 1]; + +const QR_MAX_VERSION: Version = Version::new(NVERSIONS as u8 - 1); +const CORNER_RADIUS: u8 = 4; + +const DARK: Color = Color::rgb(0, 0, 0); +const LIGHT: Color = Color::rgb(0xff, 0xff, 0xff); + +pub struct Qr { + text: String, + border: i16, + area: Rect, +} + +impl Qr { + pub fn new(text: T, case_sensitive: bool) -> Result + where + T: AsRef, + { + let indata = text.as_ref(); + let mut s = String::new(); + + if !case_sensitive + && Self::is_smaller_for_alphanumeric(indata.len()) + && Self::is_alphanumeric_after_conversion(indata) + { + for c in indata.chars() { + s.push(c.to_ascii_uppercase()) + .map_err(|_| Error::OutOfRange)?; + } + } else { + s.push_str(indata).map_err(|_| Error::OutOfRange)?; + } + + Ok(Self { + text: s, + border: 0, + area: Rect::zero(), + }) + } + + pub fn with_border(mut self, border: i16) -> Self { + self.border = border; + self + } + + fn is_alphanumeric_after_conversion(data: &str) -> bool { + data.chars() + .all(|c| ALPHANUMERIC_CHARSET.contains(c.to_ascii_uppercase())) + } + + fn is_smaller_for_alphanumeric(len: usize) -> bool { + for version in 0..NVERSIONS { + if len <= THRESHOLDS_ALPHANUM[version] { + return len > THRESHOLDS_BINARY[version]; + } + } + false + } + + fn draw(qr: &QrCode, area: Rect, border: i16, scale: i16) { + if border > 0 { + rect_fill_rounded( + area.inset(Insets::uniform(-border)), + LIGHT, + DARK, + CORNER_RADIUS, + ); + } + + let window = area.clamp(constant::screen()); + set_window(window); + + for y in window.y0..window.y1 { + for x in window.x0..window.x1 { + let rx = (x - window.x0) / scale; + let ry = (y - window.y0) / scale; + if qr.get_module(rx.into(), ry.into()) { + pixeldata(DARK); + } else { + pixeldata(LIGHT); + }; + } + } + pixeldata_dirty(); + } +} + +impl Component for Qr { + type Msg = Never; + + fn place(&mut self, bounds: Rect) -> Rect { + self.area = bounds; + bounds + } + + fn event(&mut self, _ctx: &mut EventCtx, _event: Event) -> Option { + None + } + + fn paint(&mut self) { + let mut outbuffer = [0u8; QR_MAX_VERSION.buffer_len()]; + let mut tempbuffer = [0u8; QR_MAX_VERSION.buffer_len()]; + + let qr = QrCode::encode_text( + self.text.as_ref(), + &mut tempbuffer, + &mut outbuffer, + QrCodeEcc::Medium, + Version::MIN, + QR_MAX_VERSION, + None, + true, + ); + let qr = unwrap!(qr); + let size = qr.size() as i16; + + let avail_space = self.area.width().min(self.area.height()); + let avail_space = avail_space - 2 * self.border; + let scale = avail_space / size; + assert!((1..=10).contains(&scale)); + + let area = Rect::from_center_and_size(self.area.center(), Offset::uniform(size * scale)); + Self::draw(&qr, area, self.border, scale); + } + + fn bounds(&self, sink: &mut dyn FnMut(Rect)) { + sink(self.area) + } +} + +#[cfg(feature = "ui_debug")] +impl crate::trace::Trace for Qr { + fn trace(&self, t: &mut dyn crate::trace::Tracer) { + t.open("Qr"); + t.field("text", &self.text.as_str()); + t.close(); + } +} diff --git a/core/embed/rust/src/ui/display/mod.rs b/core/embed/rust/src/ui/display/mod.rs index 7ab1c26e2..f8dcc2ac9 100644 --- a/core/embed/rust/src/ui/display/mod.rs +++ b/core/embed/rust/src/ui/display/mod.rs @@ -20,9 +20,8 @@ use crate::trezorhal::{ use crate::ui::geometry::TOP_LEFT; use crate::{ - error::Error, time::Duration, - trezorhal::{buffers::get_text_buffer, display, qr, time, uzlib::UzlibContext}, + trezorhal::{buffers::get_text_buffer, display, time, uzlib::UzlibContext}, ui::lerp::Lerp, }; use core::slice; @@ -806,10 +805,6 @@ pub fn dotted_line(start: Point, width: i16, color: Color) { } } -pub fn qrcode(center: Point, data: &str, max_size: u32, case_sensitive: bool) -> Result<(), Error> { - qr::render_qrcode(center.x, center.y, data, max_size, case_sensitive) -} - pub fn text(baseline: Point, text: &str, font: Font, fg_color: Color, bg_color: Color) { display::text( baseline.x, diff --git a/core/embed/rust/src/ui/model_tt/layout.rs b/core/embed/rust/src/ui/model_tt/layout.rs index 7626c8990..7a8773cb0 100644 --- a/core/embed/rust/src/ui/model_tt/layout.rs +++ b/core/embed/rust/src/ui/model_tt/layout.rs @@ -28,7 +28,7 @@ use crate::{ }, TextStyle, }, - Border, Component, Empty, FormattedText, Timeout, TimeoutMsg, + Border, Component, Empty, FormattedText, Qr, Timeout, TimeoutMsg, }, display::{self, tjpgd::jpeg_info, toif::Icon}, geometry, @@ -332,6 +332,12 @@ where } } +impl ComponentMsgObj for Qr { + fn msg_try_into_obj(&self, _msg: Self::Msg) -> Result { + unreachable!(); + } +} + extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = move |_args: &[Obj], kwargs: &Map| { let title: StrBuffer = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; @@ -568,10 +574,7 @@ extern "C" fn new_show_qr(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Frame::left_aligned( theme::label_title(), title, - Dialog::new( - painter::qrcode_painter(address, theme::QR_SIDE_MAX, case_sensitive), - buttons, - ), + Dialog::new(Qr::new(address, case_sensitive)?.with_border(4), buttons), ) .with_border(theme::borders()), )?; diff --git a/core/mocks/generated/trezorui.pyi b/core/mocks/generated/trezorui.pyi index 233f1f915..3cd672112 100644 --- a/core/mocks/generated/trezorui.pyi +++ b/core/mocks/generated/trezorui.pyi @@ -178,12 +178,6 @@ class Display: font is used for rendering. """ - def qrcode(self, x: int, y: int, data: str, scale: int) -> None: - """ - Renders data encoded as a QR code centered at position (x,y). - Scale determines a zoom factor. - """ - def orientation(self, degrees: int | None = None) -> int: """ Sets display orientation to 0, 90, 180 or 270 degrees. diff --git a/core/tests/test_trezor.ui.display.py b/core/tests/test_trezor.ui.display.py index 1efad4aa6..3c6223b12 100644 --- a/core/tests/test_trezor.ui.display.py +++ b/core/tests/test_trezor.ui.display.py @@ -64,9 +64,6 @@ class TestDisplay(unittest.TestCase): self.assertEqual(display.text_width("ǑǑǑǑǑǑǑǑ", 9, 1), 0) self.assertEqual(display.text_width("ǑǑǑǑǑǑǑǑ", 15, 1), 0) - def test_qrcode(self): - display.qrcode(0, 0, 'Test', 4) - def test_loader(self): display.loader(333, False, 0, 0xFFFF, 0x0000) diff --git a/vendor/QR-Code-generator b/vendor/QR-Code-generator index 2fc287904..2643e824e 160000 --- a/vendor/QR-Code-generator +++ b/vendor/QR-Code-generator @@ -1 +1 @@ -Subproject commit 2fc287904a8c2f2cb504c5710ee58684dd2bf697 +Subproject commit 2643e824eb15064662e6c4d99b010740275a0be1