Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix fading issues #4492

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/.changelog.d/4492.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[T3T1] Fixed flashing old content when fading
9 changes: 1 addition & 8 deletions core/embed/io/display/st-7789/display_fb.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ void display_fb_clear(void) {
static void bg_copy_callback(void) {
display_driver_t *drv = &g_display_driver;

drv->update_pending = 1;
drv->update_pending = 2;

fb_queue_put(&drv->empty_frames, fb_queue_take(&drv->ready_frames));
}
Expand Down Expand Up @@ -267,13 +267,6 @@ void display_ensure_refreshed(void) {
irq_unlock(irq_key);
__WFI();
} while (copy_pending);

// Wait until the display is fully refreshed
// (TE signal is low when the display is updating)
while (GPIO_PIN_RESET ==
HAL_GPIO_ReadPin(DISPLAY_TE_PORT, DISPLAY_TE_PIN)) {
__WFI();
}
}
#endif
}
Expand Down
1 change: 1 addition & 0 deletions core/embed/projects/bootloader/.changelog.d/4492.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[T3T1] Fix slow fade in/non responsiveness in bootloader UI
2 changes: 2 additions & 0 deletions core/embed/rust/librust_qstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ static void _librust_qstrs(void) {
MP_QSTR_auto_lock__change_template;
MP_QSTR_auto_lock__title;
MP_QSTR_auto_lock__turned_on;
MP_QSTR_backlight_fade;
MP_QSTR_backlight_set;
MP_QSTR_backup__can_back_up_anytime;
MP_QSTR_backup__create_backup_to_prevent_loss;
MP_QSTR_backup__info_multi_share_backup;
Expand Down
2 changes: 1 addition & 1 deletion core/embed/rust/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl Stopwatch {
pub fn elapsed(&self) -> Duration {
match *self {
Self::Stopped(duration) => duration,
Self::Running(time) => Instant::now().checked_duration_since(time).unwrap(),
Self::Running(time) => unwrap!(Instant::now().checked_duration_since(time)),
}
}

Expand Down
29 changes: 29 additions & 0 deletions core/embed/rust/src/ui/api/firmware_micropython.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ use crate::{
};
use heapless::Vec;

#[cfg(feature = "backlight")]
use crate::ui::display::{fade_backlight_duration, set_backlight};

/// Dummy implementation so that we can use `Empty` in a return type of
/// unimplemented trait function
impl ComponentMsgObj for Empty {
Expand Down Expand Up @@ -1035,6 +1038,24 @@ pub extern "C" fn upy_check_homescreen_format(data: Obj) -> Obj {
unsafe { util::try_or_raise(block) }
}

pub extern "C" fn upy_backlight_set(_level: Obj) -> Obj {
let block = || {
#[cfg(feature = "backlight")]
set_backlight(_level.try_into()?);
Ok(Obj::const_none())
};
unsafe { util::try_or_raise(block) }
}

pub extern "C" fn upy_backlight_fade(_level: Obj) -> Obj {
let block = || {
#[cfg(feature = "backlight")]
fade_backlight_duration(_level.try_into()?, 150);
Ok(Obj::const_none())
};
unsafe { util::try_or_raise(block) }
}

#[no_mangle]
pub static mp_module_trezorui_api: Module = obj_module! {
/// from trezor import utils
Expand Down Expand Up @@ -1141,6 +1162,14 @@ pub static mp_module_trezorui_api: Module = obj_module! {
/// """Disable animations, debug builds only."""
Qstr::MP_QSTR_disable_animation => obj_fn_1!(upy_disable_animation).as_obj(),

/// def backlight_set(level: int) -> None:
/// """Set backlight to desired level."""
Qstr::MP_QSTR_backlight_set => obj_fn_1!(upy_backlight_set).as_obj(),

/// def backlight_fade(level: int) -> None:
/// """Fade backlight to desired level."""
Qstr::MP_QSTR_backlight_fade => obj_fn_1!(upy_backlight_fade).as_obj(),

/// def confirm_action(
/// *,
/// title: str,
Expand Down
17 changes: 14 additions & 3 deletions core/embed/rust/src/ui/display/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ use crate::{strutil::TString, trezorhal::display};
#[cfg(feature = "backlight")]
use crate::ui::lerp::Lerp;

#[cfg(feature = "backlight")]
use crate::{time::Stopwatch, ui::util::animation_disabled};

// Reexports
pub use crate::ui::display::toif::Icon;
pub use color::Color;
Expand All @@ -39,11 +42,19 @@ pub fn fade_backlight(target: u8) {
#[cfg(feature = "backlight")]
pub fn fade_backlight_duration(target: u8, duration_ms: u32) {
let target = target as i32;
let duration_ms = duration_ms as i32;
let current = backlight() as i32;
let duration = Duration::from_millis(duration_ms);

if animation_disabled() {
set_backlight(target as u8);
return;
}

let timer = Stopwatch::new_started();

for i in 0..duration_ms {
let val = i32::lerp(current, target, i as f32 / duration_ms as f32);
while timer.elapsed() < duration {
let elapsed = timer.elapsed();
let val = i32::lerp(current, target, elapsed / duration);
set_backlight(val as u8);
time::sleep(Duration::from_millis(1));
}
Expand Down
25 changes: 0 additions & 25 deletions core/embed/upymod/modtrezorui/modtrezorui-display.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,29 +103,6 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorui_Display_orientation_obj,
1, 2,
mod_trezorui_Display_orientation);

/// def backlight(self, val: int | None = None) -> int:
/// """
/// Sets backlight intensity to the value specified in val.
/// Call without the val parameter to just perform the read of the value.
/// """
STATIC mp_obj_t mod_trezorui_Display_backlight(size_t n_args,
const mp_obj_t *args) {
mp_int_t val;
if (n_args > 1) {
val = mp_obj_get_int(args[1]);
if (val < 0 || val > 255) {
mp_raise_ValueError("Value must be between 0 and 255");
}
val = display_set_backlight(val);
} else {
val = display_get_backlight();
}
return MP_OBJ_NEW_SMALL_INT(val);
}
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorui_Display_backlight_obj,
1, 2,
mod_trezorui_Display_backlight);

/// def save(self, prefix: str) -> None:
/// """
/// Saves current display contents to PNG file with given prefix.
Expand Down Expand Up @@ -162,8 +139,6 @@ STATIC const mp_rom_map_elem_t mod_trezorui_Display_locals_dict_table[] = {
{MP_ROM_QSTR(MP_QSTR_bar), MP_ROM_PTR(&mod_trezorui_Display_bar_obj)},
{MP_ROM_QSTR(MP_QSTR_orientation),
MP_ROM_PTR(&mod_trezorui_Display_orientation_obj)},
{MP_ROM_QSTR(MP_QSTR_backlight),
MP_ROM_PTR(&mod_trezorui_Display_backlight_obj)},
{MP_ROM_QSTR(MP_QSTR_save), MP_ROM_PTR(&mod_trezorui_Display_save_obj)},
{MP_ROM_QSTR(MP_QSTR_clear_save),
MP_ROM_PTR(&mod_trezorui_Display_clear_save_obj)},
Expand Down
6 changes: 0 additions & 6 deletions core/mocks/generated/trezorui.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ class Display:
value.
"""

def backlight(self, val: int | None = None) -> int:
"""
Sets backlight intensity to the value specified in val.
Call without the val parameter to just perform the read of the value.
"""

def save(self, prefix: str) -> None:
"""
Saves current display contents to PNG file with given prefix.
Expand Down
10 changes: 10 additions & 0 deletions core/mocks/generated/trezorui_api.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ def disable_animation(disable: bool) -> None:
"""Disable animations, debug builds only."""


# rust/src/ui/api/firmware_micropython.rs
def backlight_set(level: int) -> None:
"""Set backlight to desired level."""


# rust/src/ui/api/firmware_micropython.rs
def backlight_fade(level: int) -> None:
"""Fade backlight to desired level."""


# rust/src/ui/api/firmware_micropython.rs
def confirm_action(
*,
Expand Down
34 changes: 11 additions & 23 deletions core/src/trezor/ui/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
from trezor.messages import ButtonAck, ButtonRequest
from trezor.wire import context
from trezor.wire.protocol_common import Context
from trezorui_api import AttachType, BacklightLevels, LayoutState
from trezorui_api import (
AttachType,
BacklightLevels,
LayoutState,
backlight_fade,
backlight_set,
)

if TYPE_CHECKING:
from typing import Any, Callable, Generator, Generic, Iterator, TypeVar
Expand Down Expand Up @@ -67,12 +73,12 @@ async def _alert(count: int) -> None:
long_sleep = loop.sleep(80)
for i in range(count * 2):
if i % 2 == 0:
display.backlight(BacklightLevels.MAX)
backlight_set(BacklightLevels.MAX)
await short_sleep
else:
display.backlight(BacklightLevels.DIM)
backlight_set(BacklightLevels.DIM)
await long_sleep
display.backlight(BacklightLevels.NORMAL)
backlight_set(BacklightLevels.NORMAL)
global _alert_in_progress
_alert_in_progress = False

Expand All @@ -87,24 +93,6 @@ def alert(count: int = 3) -> None:
loop.schedule(_alert(count))


def backlight_fade(val: int, delay: int = 14000, step: int = 15) -> None:
if utils.USE_BACKLIGHT:
if __debug__:
if utils.DISABLE_ANIMATION:
display.backlight(val)
return
current = display.backlight()
if current < 0:
display.backlight(val)
return
elif current > val:
step = -step
for i in range(current, val, step):
display.backlight(i)
utime.sleep_us(delay)
display.backlight(val)


class Shutdown(Exception):
pass

Expand Down Expand Up @@ -526,9 +514,9 @@ def start(self) -> None:

self.layout.request_complete_repaint()
painted = self.layout.paint()
backlight_fade(BacklightLevels.NORMAL)
if painted:
refresh()
backlight_fade(BacklightLevels.NORMAL)

def stop(self) -> None:
global CURRENT_LAYOUT
Expand Down
3 changes: 2 additions & 1 deletion core/tests/test_trezor.ui.display.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from common import * # isort:skip

from trezor.ui import display
from trezorui_api import backlight_set


class TestDisplay(unittest.TestCase):
Expand All @@ -17,7 +18,7 @@ def test_orientation(self):

def test_backlight(self):
for b in range(256):
display.backlight(b)
backlight_set(b)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if there's any point in keeping this testcase around? it's just a smoke test for existence of the function, we're not verifying any result here 🤷‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, we should either make something useful of it or remove it. I'd say its out of scope here though


def test_raw(self):
pass
Expand Down
Loading