diff --git a/docs/activity-manager.md b/docs/activity-manager.md index c1c3adc5..2f0756a0 100644 --- a/docs/activity-manager.md +++ b/docs/activity-manager.md @@ -11,6 +11,7 @@ This document explains the refactoring from the original per-activity render tas | `RenderLock` | Inner class of `Activity` | Standalone class, acquires global mutex | | Subactivities | `ActivityWithSubactivity` base class | Activity stack managed by `ActivityManager` | | Navigation | Free functions in `main.cpp` | `activityManager.goHome()`, `goToReader()`, etc. | +| Forward flow | Parent stays on stack (`pushActivity`) | Parent destroyed on forward flow (`replaceWith*` + `ReturnHint`) | | Subactivity results | Callback lambdas stored in parent | `startActivityForResult()` / `setResult()` / `finish()` | | `requestUpdate()` | Notifies activity's own render task | Delegates to `ActivityManager` (immediate or deferred) | @@ -99,7 +100,7 @@ class MyActivity final : public Activity { }; ``` -Note that navigation callbacks like `goBack` are no longer stored — use `finish()` or `activityManager.goHome()` instead. +Note that navigation callbacks like `goBack` are no longer stored — use `finish()`, `onGoHome()`, or a direct `activityManager.goHome()` / `goTo*()` / `replaceWith*()` call instead. ### 2. Replace Navigation Functions @@ -116,7 +117,7 @@ activityManager.goToSettings(); activityManager.replaceActivity(std::make_unique(renderer, mappedInput)); ``` -`replaceActivity()` destroys the current activity and clears the stack. Use it for top-level navigation (home, reader, settings, etc.). +`replaceActivity()` destroys the current activity and clears the stack. Use it for top-level navigation (home, reader, settings, etc.). See the "Navigation Flow" section after the checklist for when to use replace vs push, and how the `ReturnHint` mechanism restores a parent's prior state without keeping it resident. ### 3. Replace Subactivity Pattern @@ -234,12 +235,82 @@ class SettingsActivity : public Activity { public: SettingsActivity(GfxRenderer& r, MappedInputManager& m) : Activity("Settings", r, m) {} - // Use finish() to go back, activityManager.goHome() to go home + // Use finish() to go back (pops the stack, or returns via ReturnHint if empty), + // onGoHome() to exit the flow ("up and out"), or activityManager.goHome() for a hard reset. }; ``` This removes `std::function` overhead (~2-4KB per unique signature) and eliminates lifetime risks from captured `this` pointers. +## Navigation Flow + +### Push vs Replace: Memory Matters + +On an ESP32-C3, heap fragmentation is a real constraint — especially when the next activity is heavy (EPUB reader, TLS sync). There are two ways to launch a new activity, and the choice matters: + +| Call | Current activity | Stack | When to use | +|----------------------------------------------------|---------------------------------|------------------|-------------------------------------------------------------| +| `replaceActivity()` / `goTo*()` / `replaceWith*()` | **destroyed** (onExit + delete) | cleared | Forward flow: the caller has no reason to stay resident | +| `pushActivity()` / `startActivityForResult()` | kept alive on stack | parent preserved | Modal result flow: the caller needs to resume with a result | + +**Default to replace.** Push is only correct when you need to deliver a result back to a still-living parent (keyboard entry, confirmation dialog, chapter picker, etc.). For plain one-way transitions — opening a book, going to settings, switching tabs — use replace so the parent's memory is freed before the next activity runs. + +### The `ReturnHint` Pattern + +Replace destroys the parent, but the user still expects Back to return "where they came from" — not always Home. To bridge that, `ActivityManager` holds a small `ReturnHint`: + +```cpp +enum class ReturnTo : uint8_t { Home, FileBrowser, RecentBooks }; + +struct ReturnHint { + ReturnTo target = ReturnTo::Home; + std::string path; // FileBrowser directory to restore + std::string selectName; // item to re-focus (file name, book path) + int selectIndex = -1; // combined-list index (Home selector, Recents row) +}; +``` + +A parent records a hint before launching a forward flow. When the launched activity (or anything it chains to) eventually exits with an empty stack, `ActivityManager::returnFromChild()` consumes the hint and routes to the correct parent — restoring its prior selection. If no hint is set, it falls back to `goHome()`. + +Two ways to set a hint: + +**1. Dedicated wrappers** — for the common book-open paths: + +```cpp +// FileBrowserActivity::onFileOpen +ReturnHint hint; +hint.target = ReturnTo::FileBrowser; +hint.path = basepath; // "/books/fiction" +hint.selectName = entry; // "war_and_peace.epub" +activityManager.replaceWithReader(fullPath, std::move(hint)); + +// RecentBooksActivity::onSelect +ReturnHint hint; +hint.target = ReturnTo::RecentBooks; +hint.selectIndex = selectorIndex; +activityManager.replaceWithReader(path, std::move(hint)); +``` + +**2. `setReturnHint()` + any `goTo*()`** — for arbitrary transitions where a dedicated wrapper would be overkill: + +```cpp +// HomeActivity::dispatchMenuAction +ReturnHint hint; +hint.target = ReturnTo::Home; +hint.selectIndex = selectorIndex; // restore focus on the same menu entry +activityManager.setReturnHint(std::move(hint)); +activityManager.goToSettings(); // parent destroyed; hint survives the round trip +``` + +`goTo*()` helpers do **not** clear the hint — only `goHome()` (explicit hard-reset) and the `replaceWith*()` helpers (which overwrite it with their own hint) do. This lets a hint survive chained transitions: Home → Reader → KOReaderSync → Reader → back to Home, hint intact. + +How `finish()` interacts with the hint: + +- **Non-empty stack**: `finish()` pops to the parent on the stack (classic modal result flow). Hint is untouched. +- **Empty stack**: `finish()` falls through to `returnFromChild()` automatically. An activity launched via a `replaceWith*()` helper has no stack — so its Back-button `finish()` naturally routes via the hint. + +`onGoHome()` is now semantically "up and out" — it calls `returnFromChild()`, so long-press Back in a reader returns to whichever view opened the book, not always Home. For an explicit hard-reset, call `activityManager.goHome()` directly. + ## Technical Details ### FreeRTOS Task Model @@ -444,6 +515,10 @@ Child calls: setResult(MyResult{...}); finish(); **Creating background tasks that outlive the activity**: Any FreeRTOS task created in `onEnter()` must be deleted in `onExit()` before the activity is destroyed. The `ActivityManager` does not track or clean up background tasks. +**Using push for forward navigation**: `pushActivity()` / `startActivityForResult()` keeps the parent alive on the stack. For a heavy child (EPUB reader, TLS sync) on a fragmented heap, the parent's resident allocations can be the difference between a successful launch and OOM. Only push when you need the parent to receive a result — otherwise use `replaceActivity()` / `goTo*()` / `replaceWith*()` and let the parent be freed first. If you do need "back to where I came from" semantics, record a `ReturnHint` before the replace instead of pushing. + +**Stale `ReturnHint`**: A hint set by one activity persists until either `returnFromChild()` / `goHome()` clears it, or a `replaceWith*()` helper overwrites it. If you record a hint but the flow aborts down an unusual path (error screen, boot transition), the next unrelated `finish()` could consume it. Prefer setting the hint immediately before the transition, and call `activityManager.clearReturnHint()` if you abort the flow without launching the intended target. + **Holding `RenderLock` across blocking calls**: The render task is blocked on the mutex while you hold the lock. Keep critical sections short — acquire, mutate state, release, then do blocking work. ```cpp diff --git a/src/SettingsList.h b/src/SettingsList.h index 1e3be86d..e09fa64d 100644 --- a/src/SettingsList.h +++ b/src/SettingsList.h @@ -34,21 +34,13 @@ // deep inside the heap allocator chain — enough stack to overflow the 8 KB loop task stack // when called from inside SETTINGS.loadFromFile() at boot time. namespace SettingsListDetail { -inline uint8_t getKoReaderMatchMethod(const void*) { - return static_cast(KOREADER_STORE.getMatchMethod()); -} +inline uint8_t getKoReaderMatchMethod(const void*) { return static_cast(KOREADER_STORE.getMatchMethod()); } -inline std::string getKoReaderServerUrl(void*) { - return KOREADER_STORE.getServerUrl(); -} +inline std::string getKoReaderServerUrl(void*) { return KOREADER_STORE.getServerUrl(); } -inline std::string getKoReaderUsername(void*) { - return KOREADER_STORE.getUsername(); -} +inline std::string getKoReaderUsername(void*) { return KOREADER_STORE.getUsername(); } -inline std::string getKoReaderPassword(void*) { - return KOREADER_STORE.getPassword(); -} +inline std::string getKoReaderPassword(void*) { return KOREADER_STORE.getPassword(); } inline const std::vector list = { // --- Display --- @@ -167,24 +159,21 @@ inline const std::vector list = { // --- KOReader Sync (web-only, uses KOReaderCredentialStore) --- SettingInfo::DynamicString( - StrId::STR_SYNC_SERVER_URL, - static_cast(getKoReaderServerUrl), + StrId::STR_SYNC_SERVER_URL, static_cast(getKoReaderServerUrl), [](void*, const std::string& v) { KOREADER_STORE.setServerUrl(v); KOREADER_STORE.saveToFile(); }, "koServerUrl", StrId::STR_KOREADER_SYNC), SettingInfo::DynamicString( - StrId::STR_KOREADER_USERNAME, - static_cast(getKoReaderUsername), + StrId::STR_KOREADER_USERNAME, static_cast(getKoReaderUsername), [](void*, const std::string& v) { KOREADER_STORE.setCredentials(v, KOREADER_STORE.getPassword()); KOREADER_STORE.saveToFile(); }, "koUsername", StrId::STR_KOREADER_SYNC), SettingInfo::DynamicString( - StrId::STR_KOREADER_PASSWORD, - static_cast(getKoReaderPassword), + StrId::STR_KOREADER_PASSWORD, static_cast(getKoReaderPassword), [](void*, const std::string& v) { KOREADER_STORE.setCredentials(KOREADER_STORE.getUsername(), v); KOREADER_STORE.saveToFile(); @@ -192,8 +181,7 @@ inline const std::vector list = { "koPassword", StrId::STR_KOREADER_SYNC) .withObfuscated(), SettingInfo::DynamicEnum( - StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, - getKoReaderMatchMethod, + StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, getKoReaderMatchMethod, [](void*, uint8_t v) { KOREADER_STORE.setMatchMethod(static_cast(v)); KOREADER_STORE.saveToFile(); diff --git a/src/activities/Activity.cpp b/src/activities/Activity.cpp index 21aaa1b2..91122af1 100644 --- a/src/activities/Activity.cpp +++ b/src/activities/Activity.cpp @@ -10,9 +10,9 @@ void Activity::requestUpdate(bool immediate) { activityManager.requestUpdate(imm void Activity::requestUpdateAndWait() { activityManager.requestUpdateAndWait(); } -void Activity::onGoHome() { activityManager.goHome(); } - -void Activity::onSelectBook(const std::string& path) { activityManager.pushReader(path); } +// "Up and out" — return to whichever parent launched this flow. If no return hint +// is set (typical for activities launched via a plain goTo*()), falls back to Home. +void Activity::onGoHome() { activityManager.returnFromChild(); } void Activity::startActivityForResult(std::unique_ptr&& activity, ActivityResultHandler resultHandler) { this->resultHandler = std::move(resultHandler); diff --git a/src/activities/Activity.h b/src/activities/Activity.h index cc3fe443..c5e4949e 100644 --- a/src/activities/Activity.h +++ b/src/activities/Activity.h @@ -27,6 +27,7 @@ class Activity { explicit Activity(std::string name, GfxRenderer& renderer, MappedInputManager& mappedInput) : name(std::move(name)), renderer(renderer), mappedInput(mappedInput) {} virtual ~Activity() = default; + const std::string& getName() const { return name; } virtual void onEnter(); virtual void onExit(); virtual void loop() {} @@ -57,5 +58,4 @@ class Activity { // Convenience method to facilitate API transition to ActivityManager // TODO: remove this in near future void onGoHome(); - void onSelectBook(const std::string& path); }; diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index d1c0da40..c2490a8a 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -3,6 +3,9 @@ #include #include #include +#include +#include +#include #include "CrossPointState.h" #include "boot_sleep/BootActivity.h" @@ -19,6 +22,10 @@ #include "util/FullScreenMessageActivity.h" #include "weather/WeatherActivity.h" +#ifndef DEBUG_MEMORY_CONSUMPTION +#define DEBUG_MEMORY_CONSUMPTION 0 +#endif + void ActivityManager::begin() { xTaskCreate(&renderTaskTrampoline, "ActivityManagerRender", 8192, // Stack size @@ -29,6 +36,17 @@ void ActivityManager::begin() { assert(renderTaskHandle != nullptr && "Failed to create render task"); } +#if DEBUG_MEMORY_CONSUMPTION +static void logActivityStackState(const char* stage, Activity* currentActivity, size_t stackSize) { + const uint32_t freeHeap = esp_get_free_heap_size(); + const uint32_t contigHeap = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); + LOG_DBG("ACT", "%s: current=%s stackSize=%zu free=%lu contig=%lu", stage, + currentActivity ? currentActivity->getName().c_str() : "", stackSize, freeHeap, contigHeap); +} +#else +static inline void logActivityStackState(const char*, Activity*, size_t) {} +#endif + void ActivityManager::renderTaskTrampoline(void* param) { auto* self = static_cast(param); self->renderTaskLoop(); @@ -111,10 +129,10 @@ void ActivityManager::loop() { pendingAction = PendingAction::None; if (stackActivities.empty()) { - LOG_DBG("ACT", "No more activities on stack, going home"); - lock.unlock(); // goHome may acquire its own lock - goHome(); - continue; // Will launch goHome immediately + LOG_DBG("ACT", "No more activities on stack, returning from child"); + lock.unlock(); // returnFromChild may acquire its own lock via replaceActivity + returnFromChild(); + continue; // Will launch the target activity immediately } else { currentActivity = std::move(stackActivities.back()); @@ -149,6 +167,9 @@ void ActivityManager::loop() { RenderLock lock; if (pendingAction == PendingAction::Replace) { +#if DEBUG_MEMORY_CONSUMPTION + logActivityStackState("replace_before", currentActivity.get(), stackActivities.size()); +#endif // Destroy the current activity exitActivity(lock); // Clear the stack @@ -156,10 +177,21 @@ void ActivityManager::loop() { stackActivities.back()->onExit(); stackActivities.pop_back(); } +#if DEBUG_MEMORY_CONSUMPTION + logActivityStackState("replace_after_clear", nullptr, stackActivities.size()); +#endif } else if (pendingAction == PendingAction::Push) { +#if DEBUG_MEMORY_CONSUMPTION + logActivityStackState("push_before", currentActivity.get(), stackActivities.size()); +#endif // Move current activity to stack stackActivities.push_back(std::move(currentActivity)); +#if DEBUG_MEMORY_CONSUMPTION LOG_DBG("ACT", "Pushed to activity stack, new size = %zu", stackActivities.size()); + logActivityStackState("push_after", currentActivity.get(), stackActivities.size()); +#else + LOG_DBG("ACT", "Pushed to activity stack, new size = %zu", stackActivities.size()); +#endif } pendingAction = PendingAction::None; currentActivity = std::move(pendingActivity); @@ -201,6 +233,10 @@ void ActivityManager::replaceActivity(std::unique_ptr&& newActivity) { if (currentActivity) { // Defer launch if we're currently in an activity, to avoid deleting the current activity // leading to the "delete this" problem +#if DEBUG_MEMORY_CONSUMPTION + LOG_DBG("ACT", "replaceActivity requested: current=%s stackSize=%zu", currentActivity->getName().c_str(), + stackActivities.size()); +#endif pendingActivity = std::move(newActivity); pendingAction = PendingAction::Replace; } else { @@ -216,16 +252,19 @@ void ActivityManager::goToFileTransfer() { void ActivityManager::goToSettings() { replaceActivity(std::make_unique(renderer, mappedInput)); } -void ActivityManager::goToFileBrowser(std::string path) { - replaceActivity(std::make_unique(renderer, mappedInput, std::move(path))); +void ActivityManager::goToFileBrowser(std::string path, std::string focusName) { + replaceActivity(std::make_unique(renderer, mappedInput, std::move(path), std::move(focusName))); } -void ActivityManager::goToRecentBooks() { - replaceActivity(std::make_unique(renderer, mappedInput)); +void ActivityManager::goToRecentBooks(int focusIndex) { + replaceActivity(std::make_unique(renderer, mappedInput, focusIndex)); } -void ActivityManager::goToGlobalBookmarks() { - replaceActivity(std::make_unique(renderer, mappedInput)); +void ActivityManager::goToGlobalBookmarks() { goToGlobalBookmarks({}); } + +void ActivityManager::goToGlobalBookmarks(ReturnHint hint) { + hasReturnHint = false; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(hint))); } void ActivityManager::goToBrowser() { @@ -249,8 +288,48 @@ void ActivityManager::goToKOReaderSync() { sync.hasParagraphIndex, sync.intent)); } -void ActivityManager::pushReader(std::string path) { - pushActivity(std::make_unique(renderer, mappedInput, std::move(path))); +void ActivityManager::replaceWithReader(std::string path, ReturnHint hint) { + returnHint = std::move(hint); + hasReturnHint = true; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(path))); +} + +void ActivityManager::replaceWithFileBrowser(std::string path, ReturnHint hint, std::string focusName) { + returnHint = std::move(hint); + hasReturnHint = true; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(path), std::move(focusName))); +} + +void ActivityManager::replaceWithRecentBooks(ReturnHint hint) { + returnHint = std::move(hint); + hasReturnHint = true; + replaceActivity(std::make_unique(renderer, mappedInput, -1)); +} + +void ActivityManager::returnFromChild() { + if (!hasReturnHint) { + goHome(); + return; + } + ReturnHint hint = std::move(returnHint); + returnHint = {}; + hasReturnHint = false; + + switch (hint.target) { + case ReturnTo::FileBrowser: + goToFileBrowser(std::move(hint.path), std::move(hint.selectName)); + break; + case ReturnTo::RecentBooks: + goToRecentBooks(hint.selectIndex); + break; + case ReturnTo::GlobalBookmarks: + goToGlobalBookmarks(std::move(hint)); + break; + case ReturnTo::Home: + default: + goHome(std::move(hint.selectName), hint.selectIndex); + break; + } } void ActivityManager::goToSleep() { @@ -266,7 +345,10 @@ void ActivityManager::goToFullScreenMessage(std::string message, EpdFontFamily:: void ActivityManager::goToWeather() { replaceActivity(std::make_unique(renderer, mappedInput)); } -void ActivityManager::goHome() { replaceActivity(std::make_unique(renderer, mappedInput)); } +void ActivityManager::goHome(std::string focusBookPath, int focusSelectorIndex) { + hasReturnHint = false; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(focusBookPath), focusSelectorIndex)); +} void ActivityManager::pushActivity(std::unique_ptr&& activity) { if (pendingActivity) { @@ -274,6 +356,10 @@ void ActivityManager::pushActivity(std::unique_ptr&& activity) { LOG_ERR("ACT", "pendingActivity while pushActivity is not expected"); pendingActivity.reset(); } +#if DEBUG_MEMORY_CONSUMPTION + LOG_DBG("ACT", "pushActivity requested: current=%s stackSize=%zu", + currentActivity ? currentActivity->getName().c_str() : "", stackActivities.size()); +#endif pendingActivity = std::move(activity); pendingAction = PendingAction::Push; } diff --git a/src/activities/ActivityManager.h b/src/activities/ActivityManager.h index b59f6e96..5ab9d04f 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -15,6 +15,23 @@ class Activity; // forward declaration class RenderLock; // forward declaration +// Where a "child" activity (launched via one of the replaceWith* helpers) should route +// control when it exits successfully. See ActivityManager::returnFromChild(). +enum class ReturnTo : uint8_t { Home, FileBrowser, RecentBooks, GlobalBookmarks }; + +// Minimal state the returning parent needs to restore its previous view (directory, +// focused item, list index, or bookmark selection). Kept as a plain struct stored by +// value on the ActivityManager — single instance, overwritten per transition, no heap +// churn beyond the small strings. +struct ReturnHint { + ReturnTo target = ReturnTo::Home; + std::string path; // FileBrowser directory to restore + std::string selectName; // item to re-focus in a list (file name, book title) + int selectIndex = -1; // e.g. Recents index + std::string selectionContext; // optional activity-specific restore key + int selectBookmarkIndex = -1; // optional bookmark index for GlobalBookmarks +}; + /** * ActivityManager * @@ -68,6 +85,15 @@ class ActivityManager { // into the next one. bool drainInput = false; + // Where returnFromChild() should route to. Set by replaceWith*() helpers and + // preserved across plain goTo*() chains so a chained navigation flow can still + // restore its original parent state. Cleared only by returnFromChild() or by + // explicit goHome()/replaceWith*() calls, not by ordinary goTo*() transitions. + // Relevant symbols: ReturnHint, returnHint, hasReturnHint, returnFromChild(), + // goHome(), goTo*(), replaceWith*(). + ReturnHint returnHint; + bool hasReturnHint = false; + public: explicit ActivityManager(GfxRenderer& renderer, MappedInputManager& mappedInput) : renderer(renderer), mappedInput(mappedInput), renderingMutex(xSemaphoreCreateMutex()) { @@ -85,18 +111,43 @@ class ActivityManager { // goTo... functions are convenient wrapper for replaceActivity() void goToFileTransfer(); void goToSettings(); - void goToFileBrowser(std::string path = {}); - void goToRecentBooks(); + void goToFileBrowser(std::string path = {}, std::string focusName = {}); + void goToRecentBooks(int focusIndex = -1); void goToGlobalBookmarks(); + void goToGlobalBookmarks(ReturnHint hint); void goToBrowser(); void goToReader(std::string path); void goToKOReaderSync(); - void pushReader(std::string path); void goToSleep(); void goToBoot(); void goToFullScreenMessage(std::string message, EpdFontFamily::Style style = EpdFontFamily::REGULAR); void goToWeather(); - void goHome(); + void goHome(std::string focusBookPath = {}, int focusSelectorIndex = -1); + + // Replace-with-hint helpers: destroy the current activity before launching the new + // one (freeing its memory) and record where to route control when the new activity + // exits. Consumed by returnFromChild(). + void replaceWithReader(std::string path, ReturnHint hint); + void replaceWithFileBrowser(std::string path, ReturnHint hint, std::string focusName = {}); + void replaceWithRecentBooks(ReturnHint hint); + + // Called by a "child" activity on successful exit. Consults the stored ReturnHint, + // clears it, and dispatches to the corresponding parent with restoration args. If + // no hint is set, defaults to goHome(). + void returnFromChild(); + + // Record a ReturnHint before calling any plain goTo*() helper. Allows an activity + // (e.g. Home) to declare "when this flow ends, come back here with this state" for + // transitions where we don't want a dedicated replaceWith*() wrapper. + // Cleared by returnFromChild() or by an explicit goHome()/replaceWith*() call. + void setReturnHint(ReturnHint hint) { + returnHint = std::move(hint); + hasReturnHint = true; + } + void clearReturnHint() { + returnHint = {}; + hasReturnHint = false; + } // This will move current activity to stack instead of deleting it void pushActivity(std::unique_ptr&& activity); diff --git a/src/activities/home/FileBrowserActivity.cpp b/src/activities/home/FileBrowserActivity.cpp index a4e771df..feb43aea 100644 --- a/src/activities/home/FileBrowserActivity.cpp +++ b/src/activities/home/FileBrowserActivity.cpp @@ -8,6 +8,7 @@ #include +#include "../ActivityManager.h" #include "../util/ConfirmationActivity.h" #include "BookInfoActivity.h" #include "CrossPointSettings.h" @@ -113,6 +114,14 @@ void FileBrowserActivity::onEnter() { loadFiles(); selectorIndex = 0; + if (!focusName.empty()) { + const size_t idx = findEntry(focusName); + if (idx < files.size()) { + selectorIndex = idx; + } + focusName.clear(); + } + requestUpdate(); } @@ -147,7 +156,8 @@ void FileBrowserActivity::loop() { loadFiles(); const auto pos = oldPath.find_last_of('/'); const std::string dirName = oldPath.substr(pos + 1) + "/"; - selectorIndex = findEntry(dirName); + const size_t idx = findEntry(dirName); + selectorIndex = (idx < files.size()) ? idx : 0; requestUpdate(); } else { onGoHome(); @@ -171,7 +181,12 @@ void FileBrowserActivity::loop() { } else { std::string fullPath = basepath; if (fullPath.back() != '/') fullPath += "/"; - onSelectBook(fullPath + entry); + fullPath += entry; + ReturnHint hint; + hint.target = ReturnTo::FileBrowser; + hint.path = basepath; + hint.selectName = entry; + activityManager.replaceWithReader(std::move(fullPath), std::move(hint)); } return; } @@ -309,5 +324,5 @@ void FileBrowserActivity::render(RenderLock&&) { size_t FileBrowserActivity::findEntry(const std::string& name) const { for (size_t i = 0; i < files.size(); i++) if (files[i] == name) return i; - return 0; + return files.size(); } \ No newline at end of file diff --git a/src/activities/home/FileBrowserActivity.h b/src/activities/home/FileBrowserActivity.h index c4f359dd..991e1c55 100644 --- a/src/activities/home/FileBrowserActivity.h +++ b/src/activities/home/FileBrowserActivity.h @@ -19,6 +19,7 @@ class FileBrowserActivity final : public Activity { // Files state std::string basepath = "/"; + std::string focusName; // entry to select on first load (e.g. the file just returned from) std::vector files; // Data loading @@ -26,8 +27,11 @@ class FileBrowserActivity final : public Activity { size_t findEntry(const std::string& name) const; public: - explicit FileBrowserActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::string initialPath = "/") - : Activity("FileBrowser", renderer, mappedInput), basepath(initialPath.empty() ? "/" : std::move(initialPath)) {} + explicit FileBrowserActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::string initialPath = "/", + std::string focusName = {}) + : Activity("FileBrowser", renderer, mappedInput), + basepath(initialPath.empty() ? "/" : std::move(initialPath)), + focusName(std::move(focusName)) {} void onEnter() override; void onExit() override; void loop() override; diff --git a/src/activities/home/GlobalBookmarksActivity.cpp b/src/activities/home/GlobalBookmarksActivity.cpp index 09912068..5e50c6c9 100644 --- a/src/activities/home/GlobalBookmarksActivity.cpp +++ b/src/activities/home/GlobalBookmarksActivity.cpp @@ -9,6 +9,7 @@ #include #include +#include "../ActivityManager.h" #include "BookmarkStore.h" #include "CrossPointState.h" #include "GlobalBookmarkIndex.h" @@ -26,6 +27,26 @@ void GlobalBookmarksActivity::onEnter() { const int first = firstSelectableIndex(); selectorIndex = first >= 0 ? first : 0; + if (restoreHint.target == ReturnTo::GlobalBookmarks) { + const auto& entries = GLOBAL_BOOKMARKS.getEntries(); + if (!restoreHint.selectionContext.empty() && restoreHint.selectBookmarkIndex >= 0) { + for (size_t i = 0; i < rows.size(); ++i) { + const auto& row = rows[i]; + if (row.isSeparator) continue; + if (row.bookmarkIndex == static_cast(restoreHint.selectBookmarkIndex) && + row.bookIndex < entries.size() && entries[row.bookIndex].sourcePath == restoreHint.selectionContext) { + selectorIndex = static_cast(i); + break; + } + } + } + if (selectorIndex < 0 || selectorIndex >= static_cast(rows.size()) || isSeparatorRow(selectorIndex)) { + const int fallback = firstSelectableIndex(); + selectorIndex = fallback >= 0 ? fallback : 0; + } + restoreHint = {}; + } + const auto total = static_cast(rows.size()); buttonNavigator.setSelectablePredicate([this](int index) { return !isSeparatorRow(index); }, total); @@ -124,7 +145,11 @@ void GlobalBookmarksActivity::openSelected() { APP_STATE.saveToFile(); LOG_DBG("GBA", "Jumping to bookmark in %s at %u/%u", entry.sourcePath.c_str(), bm.spineIndex, bm.pageNumber); - onSelectBook(entry.sourcePath); + ReturnHint hint; + hint.target = ReturnTo::GlobalBookmarks; + hint.selectionContext = entry.sourcePath; + hint.selectBookmarkIndex = static_cast(row.bookmarkIndex); + activityManager.replaceWithReader(entry.sourcePath, std::move(hint)); } template diff --git a/src/activities/home/GlobalBookmarksActivity.h b/src/activities/home/GlobalBookmarksActivity.h index 3f4c9f75..77e505c6 100644 --- a/src/activities/home/GlobalBookmarksActivity.h +++ b/src/activities/home/GlobalBookmarksActivity.h @@ -20,8 +20,8 @@ struct Rect; // header separator or a bookmark entry belonging to the preceding header. class GlobalBookmarksActivity final : public Activity { public: - explicit GlobalBookmarksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) - : Activity("GlobalBookmarks", renderer, mappedInput) {} + explicit GlobalBookmarksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, ReturnHint restoreHint = {}) + : Activity("GlobalBookmarks", renderer, mappedInput), restoreHint(std::move(restoreHint)) {} void onEnter() override; void onExit() override; @@ -38,6 +38,7 @@ class GlobalBookmarksActivity final : public Activity { ButtonNavigator buttonNavigator; std::vector rows; int selectorIndex = 0; + ReturnHint restoreHint; void rebuildRows(); std::string getRowTitle(int index) const; diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index 52ec7658..a41b349c 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -211,6 +212,28 @@ void HomeActivity::onEnter() { recentsLoaded = true; } + // Apply focus: book path takes priority, else combined selector index (covers + // "return to the menu entry I was on"). + bool focused = false; + if (!focusBookPath.empty()) { + for (size_t i = 0; i < recentBooks.size(); ++i) { + if (recentBooks[i].path == focusBookPath) { + selectorIndex = static_cast(i); + focused = true; + break; + } + } + focusBookPath.clear(); + } + if (!focused && focusSelectorIndex >= 0) { + rebuildMenuEntries(); // need menu count to clamp; rebuild is idempotent + const int combinedSize = static_cast(recentBooks.size() + menuEntries.size()); + if (combinedSize > 0) { + selectorIndex = std::min(focusSelectorIndex, combinedSize - 1); + } + } + focusSelectorIndex = -1; + // Trigger first update menuEntriesDirty = true; requestUpdate(); @@ -348,9 +371,21 @@ void HomeActivity::render(RenderLock&&) { } } -void HomeActivity::onSelectBook(const std::string& path) { activityManager.pushReader(path); } +void HomeActivity::onSelectBook(const std::string& path) { + ReturnHint hint; + hint.target = ReturnTo::Home; + hint.selectName = path; // used to re-focus the book in the recents strip after return + activityManager.replaceWithReader(path, std::move(hint)); +} void HomeActivity::dispatchMenuAction(MenuAction action) { + // Record where the menu entry was focused so that when the launched activity exits + // (via returnFromChild() or an empty-stack finish()), we come back to the same row. + ReturnHint hint; + hint.target = ReturnTo::Home; + hint.selectIndex = selectorIndex; + activityManager.setReturnHint(std::move(hint)); + switch (action) { case MenuAction::FileBrowser: activityManager.goToFileBrowser(); diff --git a/src/activities/home/HomeActivity.h b/src/activities/home/HomeActivity.h index c4fad72a..dbf29f17 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -44,6 +44,9 @@ class HomeActivity final : public Activity { std::vector menuEntries; bool menuEntriesDirty = true; + std::string focusBookPath; // book path to re-select on first render, if present in recents + int focusSelectorIndex = -1; // fallback combined-selector index when focusBookPath doesn't match + void onSelectBook(const std::string& path); void dispatchMenuAction(MenuAction action); @@ -55,8 +58,11 @@ class HomeActivity final : public Activity { void loadRecentCovers(int coverHeight); public: - explicit HomeActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) - : Activity("Home", renderer, mappedInput) {} + explicit HomeActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::string focusBookPath = {}, + int focusSelectorIndex = -1) + : Activity("Home", renderer, mappedInput), + focusBookPath(std::move(focusBookPath)), + focusSelectorIndex(focusSelectorIndex) {} void onEnter() override; void onExit() override; void loop() override; diff --git a/src/activities/home/RecentBooksActivity.cpp b/src/activities/home/RecentBooksActivity.cpp index 1966cd3a..750546ad 100644 --- a/src/activities/home/RecentBooksActivity.cpp +++ b/src/activities/home/RecentBooksActivity.cpp @@ -7,6 +7,7 @@ #include +#include "../ActivityManager.h" #include "../util/ConfirmationActivity.h" #include "BookInfoActivity.h" #include "MappedInputManager.h" @@ -35,6 +36,10 @@ void RecentBooksActivity::onEnter() { loadRecentBooks(); selectorIndex = 0; + if (initialFocusIndex >= 0 && static_cast(initialFocusIndex) < recentBooks.size()) { + selectorIndex = static_cast(initialFocusIndex); + } + initialFocusIndex = -1; requestUpdate(); } @@ -49,7 +54,10 @@ void RecentBooksActivity::loop() { if (mappedInput.wasReleased(MappedInputManager::Button::Confirm) && !recentBooks.empty() && selectorIndex < static_cast(recentBooks.size())) { LOG_DBG("RBA", "Selected recent book: %s", recentBooks[selectorIndex].path.c_str()); - onSelectBook(recentBooks[selectorIndex].path); + ReturnHint hint; + hint.target = ReturnTo::RecentBooks; + hint.selectIndex = static_cast(selectorIndex); + activityManager.replaceWithReader(recentBooks[selectorIndex].path, std::move(hint)); return; } diff --git a/src/activities/home/RecentBooksActivity.h b/src/activities/home/RecentBooksActivity.h index b1103f04..8c028e19 100644 --- a/src/activities/home/RecentBooksActivity.h +++ b/src/activities/home/RecentBooksActivity.h @@ -14,6 +14,7 @@ class RecentBooksActivity final : public Activity { ButtonNavigator buttonNavigator; size_t selectorIndex = 0; + int initialFocusIndex = -1; // applied once in onEnter(), then cleared // Recent tab state std::vector recentBooks; @@ -22,8 +23,8 @@ class RecentBooksActivity final : public Activity { void loadRecentBooks(); public: - explicit RecentBooksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) - : Activity("RecentBooks", renderer, mappedInput) {} + explicit RecentBooksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, int focusIndex = -1) + : Activity("RecentBooks", renderer, mappedInput), initialFocusIndex(focusIndex) {} void onEnter() override; void onExit() override; void loop() override; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 30133953..4421cc6b 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -1,3 +1,7 @@ +#ifndef DEBUG_MEMORY_CONSUMPTION +#define DEBUG_MEMORY_CONSUMPTION 0 +#endif + #include "EpubReaderActivity.h" #include @@ -35,11 +39,15 @@ constexpr unsigned long skipChapterMs = 700; // pages per minute, first item is 1 to prevent division by zero if accessed constexpr int PAGE_TURN_LABELS[] = {1, 1, 3, 6, 12}; +#if DEBUG_MEMORY_CONSUMPTION void logReaderMemSnapshot(const char* stage) { const uint32_t freeHeap = esp_get_free_heap_size(); const uint32_t contigHeap = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); LOG_DBG("ERS", "Reader mem[%s]: free=%lu contig=%lu", stage, freeHeap, contigHeap); } +#else +inline void logReaderMemSnapshot(const char*) {} +#endif bool writeReaderProgressCache(const std::string& cachePath, const int spineIndex, const int currentPage, const int pageCount) { @@ -91,10 +99,13 @@ void EpubReaderActivity::onEnter() { RenderLock lock(*this); ReaderUtils::applyOrientation(renderer, SETTINGS.orientation); } + logReaderMemSnapshot("onEnter_after_orientation"); epub->setupCacheDir(); + logReaderMemSnapshot("onEnter_after_setupCacheDir"); applyPendingSyncSession(); applyPendingBookmarkJump(); + logReaderMemSnapshot("onEnter_after_pending_sync"); FsFile f; if (Storage.openFileForRead("ERS", epub->getCachePath() + "/progress.bin", f)) { @@ -120,9 +131,11 @@ void EpubReaderActivity::onEnter() { LOG_DBG("ERS", "Opened for first time, navigating to text reference at index %d", textSpineIndex); } } + logReaderMemSnapshot("onEnter_after_progress_load"); // Load bookmarks for this book bookmarkStore.load(epub->getCachePath()); + logReaderMemSnapshot("onEnter_after_bookmarks_loaded"); // Save current epub as last opened epub and add to recent books APP_STATE.openEpubPath = epub->getPath(); @@ -135,8 +148,10 @@ void EpubReaderActivity::onEnter() { const RecentBook currentBook = RECENT_BOOKS.getBookByPath(epub->getPath()); bookEmbeddedStyleOverride = currentBook.embeddedStyleOverride; bookImageRenderingOverride = currentBook.imageRenderingOverride; + logReaderMemSnapshot("onEnter_after_recent_books"); // Trigger first update + logReaderMemSnapshot("onEnter_before_request_update"); requestUpdate(); logReaderMemSnapshot("onEnter_ready"); } @@ -1092,8 +1107,10 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or const int orientedMarginRight, const int orientedMarginBottom, const int orientedMarginLeft) { const auto t0 = millis(); + logReaderMemSnapshot("render_start"); auto* fcm = renderer.getFontCacheManager(); fcm->resetStats(); + logReaderMemSnapshot("prewarm_begin"); // Font prewarm: scan pass accumulates text, then prewarm, then real render const uint32_t heapBefore = esp_get_free_heap_size(); @@ -1106,14 +1123,17 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or LOG_DBG("ERS", "Heap: before=%lu after=%lu delta=%ld", heapBefore, heapAfter, (int32_t)heapAfter - (int32_t)heapBefore); + logReaderMemSnapshot("prewarm_end"); // Force special handling for pages with images when anti-aliasing is on bool imagePageWithAA = page->hasImages() && SETTINGS.textAntiAliasing; + logReaderMemSnapshot("before_bw_render"); page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); renderStatusBar(); fcm->logStats("bw_render"); const auto tBwRender = millis(); + logReaderMemSnapshot("after_bw_render"); if (imagePageWithAA) { // Double FAST_REFRESH with selective image blanking (pablohc's technique): @@ -1140,34 +1160,44 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or const auto tDisplay = millis(); // Save bw buffer to reset buffer state after grayscale data sync + logReaderMemSnapshot("bw_store_begin"); renderer.storeBwBuffer(); const auto tBwStore = millis(); + logReaderMemSnapshot("bw_store_end"); // grayscale rendering // TODO: Only do this if font supports it if (SETTINGS.textAntiAliasing) { + logReaderMemSnapshot("gray_lsb_begin"); renderer.clearScreen(0x00); renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); renderer.copyGrayscaleLsbBuffers(); const auto tGrayLsb = millis(); + logReaderMemSnapshot("gray_lsb_end"); // Render and copy to MSB buffer + logReaderMemSnapshot("gray_msb_begin"); renderer.clearScreen(0x00); renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); renderer.copyGrayscaleMsbBuffers(); const auto tGrayMsb = millis(); + logReaderMemSnapshot("gray_msb_end"); // display grayscale part + logReaderMemSnapshot("gray_display_begin"); renderer.displayGrayBuffer(); const auto tGrayDisplay = millis(); renderer.setRenderMode(GfxRenderer::BW); fcm->logStats("gray"); + logReaderMemSnapshot("gray_display_end"); // restore the bw data + logReaderMemSnapshot("bw_restore_begin"); renderer.restoreBwBuffer(); const auto tBwRestore = millis(); + logReaderMemSnapshot("bw_restore_end"); const auto tEnd = millis(); LOG_DBG("ERS", @@ -1177,8 +1207,10 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or tGrayMsb - tGrayLsb, tGrayDisplay - tGrayMsb, tBwRestore - tGrayDisplay, tEnd - t0); } else { // restore the bw data + logReaderMemSnapshot("bw_restore_begin"); renderer.restoreBwBuffer(); const auto tBwRestore = millis(); + logReaderMemSnapshot("bw_restore_end"); const auto tEnd = millis(); LOG_DBG("ERS", diff --git a/src/activities/reader/ReaderActivity.cpp b/src/activities/reader/ReaderActivity.cpp index 4c293b8e..69f572a0 100644 --- a/src/activities/reader/ReaderActivity.cpp +++ b/src/activities/reader/ReaderActivity.cpp @@ -20,12 +20,20 @@ #include "components/UITheme.h" #include "fontIds.h" +#ifndef DEBUG_MEMORY_CONSUMPTION +#define DEBUG_MEMORY_CONSUMPTION 0 +#endif + namespace { +#if DEBUG_MEMORY_CONSUMPTION void logReaderLaunchMemSnapshot(const char* stage) { const uint32_t freeHeap = esp_get_free_heap_size(); const uint32_t contigHeap = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); LOG_DBG("READER", "Reader mem[%s]: free=%lu contig=%lu", stage, freeHeap, contigHeap); } +#else +inline void logReaderLaunchMemSnapshot(const char*) {} +#endif } // namespace std::string ReaderActivity::extractFolderPath(const std::string& filePath) { @@ -102,28 +110,24 @@ void ReaderActivity::goToLibrary(const std::string& fromBookPath) { void ReaderActivity::onGoToEpubReader(std::unique_ptr epub) { const auto epubPath = epub->getPath(); currentBookPath = epubPath; - logReaderLaunchMemSnapshot("before_push_epub_reader"); - startActivityForResult(std::make_unique(renderer, mappedInput, std::move(epub)), - [this](const ActivityResult&) { finish(); }); + logReaderLaunchMemSnapshot("before_replace_epub_reader"); + activityManager.replaceActivity(std::make_unique(renderer, mappedInput, std::move(epub))); } void ReaderActivity::onGoToBmpViewer(const std::string& path) { - startActivityForResult(std::make_unique(renderer, mappedInput, path), - [this](const ActivityResult&) { finish(); }); + activityManager.replaceActivity(std::make_unique(renderer, mappedInput, path)); } void ReaderActivity::onGoToXtcReader(std::unique_ptr xtc) { const auto xtcPath = xtc->getPath(); currentBookPath = xtcPath; - startActivityForResult(std::make_unique(renderer, mappedInput, std::move(xtc)), - [this](const ActivityResult&) { finish(); }); + activityManager.replaceActivity(std::make_unique(renderer, mappedInput, std::move(xtc))); } void ReaderActivity::onGoToTxtReader(std::unique_ptr txt) { const auto txtPath = txt->getPath(); currentBookPath = txtPath; - startActivityForResult(std::make_unique(renderer, mappedInput, std::move(txt)), - [this](const ActivityResult&) { finish(); }); + activityManager.replaceActivity(std::make_unique(renderer, mappedInput, std::move(txt))); } void ReaderActivity::onEnter() { diff --git a/src/activities/settings/KOReaderSettingsActivity.cpp b/src/activities/settings/KOReaderSettingsActivity.cpp index 0c82424b..2681be27 100644 --- a/src/activities/settings/KOReaderSettingsActivity.cpp +++ b/src/activities/settings/KOReaderSettingsActivity.cpp @@ -24,13 +24,14 @@ void KOReaderSettingsActivity::buildMenuItems() { menuItems.push_back(SettingInfo::Action(StrId::STR_PASSWORD, SettingAction::None)); // Document matching: DynamicEnum toggling between Filename and Binary - menuItems.push_back(SettingInfo::DynamicEnum( - StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, - static_cast([](const void*) -> uint8_t { return static_cast(KOREADER_STORE.getMatchMethod()); }), - [](void*, uint8_t v) { - KOREADER_STORE.setMatchMethod(static_cast(v)); - KOREADER_STORE.saveToFile(); - })); + menuItems.push_back(SettingInfo::DynamicEnum(StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, + static_cast([](const void*) -> uint8_t { + return static_cast(KOREADER_STORE.getMatchMethod()); + }), + [](void*, uint8_t v) { + KOREADER_STORE.setMatchMethod(static_cast(v)); + KOREADER_STORE.saveToFile(); + })); // Authenticate and Register: ACTION items menuItems.push_back(