From ae076936aead69ce5ed91ee6cfb05a264f50e1ae Mon Sep 17 00:00:00 2001 From: jacqueline Date: Tue, 11 Jul 2023 17:28:56 +1000 Subject: [PATCH] Fix browser view jank --- src/drivers/relative_wheel.cpp | 1 - src/ui/include/lvgl_task.hpp | 2 +- src/ui/screen_track_browser.cpp | 79 ++++++++++++++++++++++++++------- src/ui/themes.cpp | 8 ++-- src/ui/ui_fsm.cpp | 2 +- 5 files changed, 70 insertions(+), 22 deletions(-) diff --git a/src/drivers/relative_wheel.cpp b/src/drivers/relative_wheel.cpp index 74b1e022..846d0e7b 100644 --- a/src/drivers/relative_wheel.cpp +++ b/src/drivers/relative_wheel.cpp @@ -58,7 +58,6 @@ auto RelativeWheel::Update() -> void { } else { ticks_ = 0; } - } auto RelativeWheel::is_clicking() -> bool { diff --git a/src/ui/include/lvgl_task.hpp b/src/ui/include/lvgl_task.hpp index c649c4b6..7e60c4b4 100644 --- a/src/ui/include/lvgl_task.hpp +++ b/src/ui/include/lvgl_task.hpp @@ -15,8 +15,8 @@ #include "display.hpp" #include "relative_wheel.hpp" -#include "touchwheel.hpp" #include "themes.hpp" +#include "touchwheel.hpp" namespace ui { diff --git a/src/ui/screen_track_browser.cpp b/src/ui/screen_track_browser.cpp index b41c129e..bf8a550c 100644 --- a/src/ui/screen_track_browser.cpp +++ b/src/ui/screen_track_browser.cpp @@ -9,11 +9,13 @@ #include "core/lv_obj.h" #include "core/lv_obj_scroll.h" +#include "core/lv_obj_tree.h" #include "database.hpp" #include "event_queue.hpp" #include "extra/layouts/flex/lv_flex.h" #include "font/lv_symbol_def.h" #include "lvgl.h" +#include "misc/lv_anim.h" #include "screen_menu.hpp" #include "core/lv_event.h" @@ -33,8 +35,8 @@ static constexpr char kTag[] = "browser"; -static constexpr int kMaxPages = 3; -static constexpr int kPageBuffer = 5; +static constexpr int kMaxPages = 4; +static constexpr int kPageBuffer = 6; namespace ui { namespace screens { @@ -70,7 +72,11 @@ TrackBrowser::TrackBrowser( lv_obj_set_flex_flow(root_, LV_FLEX_FLOW_COLUMN); lv_obj_set_flex_align(root_, LV_FLEX_ALIGN_CENTER, LV_FLEX_ALIGN_START, LV_FLEX_ALIGN_START); + + // The default scrollbar is deceptive because we load in items progressively. lv_obj_set_scrollbar_mode(root_, LV_SCROLLBAR_MODE_OFF); + // Wrapping behaves in surprising ways, again due to progressing loading. + lv_group_set_wrap(group_, false); lv_obj_t* header = lv_obj_create(root_); lv_obj_set_size(header, lv_pct(100), 15); @@ -104,6 +110,7 @@ auto TrackBrowser::Tick() -> void { } if (loading_page_->wait_for(std::chrono::seconds(0)) == std::future_status::ready) { + ESP_LOGI(kTag, "load finished. adding to page."); auto result = loading_page_->get(); AddResults(loading_pos_.value_or(END), result); @@ -118,10 +125,12 @@ auto TrackBrowser::OnItemSelected(lv_event_t* ev) -> void { return; } if (index < kPageBuffer) { + ESP_LOGI(kTag, "fetch page at start"); FetchNewPage(START); return; } if (index > GetNumRecords() - kPageBuffer) { + ESP_LOGI(kTag, "fetch page at end"); FetchNewPage(END); return; } @@ -169,12 +178,28 @@ auto TrackBrowser::AddResults(Position pos, lv_obj_t* item = lv_list_add_btn(list_, NULL, text->c_str()); lv_obj_add_event_cb(item, item_click_cb, LV_EVENT_CLICKED, this); lv_obj_add_event_cb(item, item_select_cb, LV_EVENT_FOCUSED, this); - lv_group_add_obj(group_, item); + if (pos == START) { lv_obj_move_to_index(item, 0); } }; + lv_obj_t* focused = lv_group_get_focused(group_); + + // Adding objects at the start of the list will artificially scroll the list + // up. Scroll it down by the height we're adding so that the user doesn't + // notice any jank. + if (pos == START) { + int num_to_add = results->values().size(); + // Assuming that all items are the same height, this item's y pos should be + // exactly the height of the new items. + lv_obj_t* representative_item = lv_obj_get_child(list_, num_to_add); + if (representative_item != nullptr) { + int scroll_adjustment = lv_obj_get_y(representative_item); + lv_obj_scroll_by(list_, 0, -scroll_adjustment, LV_ANIM_OFF); + } + } + switch (pos) { case START: std::for_each(results->values().rbegin(), results->values().rend(), fn); @@ -185,10 +210,27 @@ auto TrackBrowser::AddResults(Position pos, current_pages_.emplace_back(results); break; } + + lv_group_remove_all_objs(group_); + int num_children = lv_obj_get_child_cnt(list_); + for (int i = 0; i < num_children; i++) { + lv_group_add_obj(group_, lv_obj_get_child(list_, i)); + } + lv_group_focus_obj(focused); } auto TrackBrowser::DropPage(Position pos) -> void { if (pos == START) { + // Removing objects from the start of the list will artificially scroll the + // list down. Scroll it up by the height we're removing so that the user + // doesn't notice any jank. + int num_to_remove = current_pages_.front()->values().size(); + lv_obj_t* new_top_obj = lv_obj_get_child(list_, num_to_remove); + if (new_top_obj != nullptr) { + int scroll_adjustment = lv_obj_get_y(new_top_obj); + lv_obj_scroll_by(list_, 0, scroll_adjustment, LV_ANIM_OFF); + } + for (int i = 0; i < current_pages_.front()->values().size(); i++) { lv_obj_t* item = lv_obj_get_child(list_, 0); if (item == NULL) { @@ -212,8 +254,24 @@ auto TrackBrowser::DropPage(Position pos) -> void { auto TrackBrowser::FetchNewPage(Position pos) -> void { if (loading_page_) { + ESP_LOGI(kTag, "already loading; giving up"); + return; + } + + std::optional> cont; + switch (pos) { + case START: + cont = current_pages_.front()->prev_page(); + break; + case END: + cont = current_pages_.back()->next_page(); + break; + } + if (!cont) { + ESP_LOGI(kTag, "out of pages; giving up"); return; } + auto db = db_.lock(); if (!db) { return; @@ -224,27 +282,16 @@ auto TrackBrowser::FetchNewPage(Position pos) -> void { if (current_pages_.size() >= kMaxPages) { switch (pos) { case START: + ESP_LOGI(kTag, "dropping end page"); DropPage(END); break; case END: + ESP_LOGI(kTag, "dropping start page"); DropPage(START); break; } } - std::optional> cont; - switch (pos) { - case START: - cont = current_pages_.front()->prev_page(); - break; - case END: - cont = current_pages_.back()->next_page(); - break; - } - if (!cont) { - return; - } - loading_pos_ = pos; loading_page_ = db->GetPage(&cont.value()); } diff --git a/src/ui/themes.cpp b/src/ui/themes.cpp index 6ae59365..54a41607 100644 --- a/src/ui/themes.cpp +++ b/src/ui/themes.cpp @@ -1,4 +1,5 @@ #include "themes.hpp" +#include "core/lv_obj.h" namespace ui { namespace themes { @@ -29,9 +30,10 @@ void Theme::Apply(void) { } void Theme::Callback(lv_obj_t* obj) { - if (lv_obj_check_type(obj, &lv_btn_class) || lv_obj_check_type(obj, &lv_list_btn_class)) { - lv_obj_add_style(obj, &button_style_, 0); + if (lv_obj_check_type(obj, &lv_btn_class) || + lv_obj_check_type(obj, &lv_list_btn_class)) { + lv_obj_add_style(obj, &button_style_, LV_PART_MAIN | LV_STATE_FOCUSED); } } } // namespace themes -} // namespace ui \ No newline at end of file +} // namespace ui diff --git a/src/ui/ui_fsm.cpp b/src/ui/ui_fsm.cpp index 5394311c..b57f97f1 100644 --- a/src/ui/ui_fsm.cpp +++ b/src/ui/ui_fsm.cpp @@ -25,7 +25,7 @@ namespace ui { static constexpr char kTag[] = "ui_fsm"; -static const std::size_t kRecordsPerPage = 10; +static const std::size_t kRecordsPerPage = 15; drivers::IGpios* UiState::sIGpios; audio::TrackQueue* UiState::sQueue;