From e5cd7583803239ef3eabd8f0693a27ed79ed47be Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 18:17:35 +0200 Subject: [PATCH 1/8] Add logs --- src/activities/reader/EpubReaderActivity.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 30133953..47663008 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -1092,6 +1092,7 @@ 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(); @@ -1106,14 +1107,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 +1144,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 +1191,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", From bf7869dfceecfdefdfb3d2fe6018911d64d1a074 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 18:44:50 +0200 Subject: [PATCH 2/8] More logging points --- src/activities/reader/EpubReaderActivity.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 47663008..7c3494e3 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -91,10 +91,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 +123,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 +140,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"); } @@ -1095,6 +1102,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or 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(); From ef088db789c6a3bf105ff2e4274b54e3c9bbef4b Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 19:09:16 +0200 Subject: [PATCH 3/8] Add more logging --- src/activities/Activity.h | 1 + src/activities/ActivityManager.cpp | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/activities/Activity.h b/src/activities/Activity.h index cc3fe443..f74599a2 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() {} diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index d1c0da40..86acbeb1 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" @@ -29,6 +32,17 @@ void ActivityManager::begin() { assert(renderTaskHandle != nullptr && "Failed to create render task"); } +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); +} + void ActivityManager::renderTaskTrampoline(void* param) { auto* self = static_cast(param); self->renderTaskLoop(); @@ -149,6 +163,7 @@ void ActivityManager::loop() { RenderLock lock; if (pendingAction == PendingAction::Replace) { + logActivityStackState("replace_before", currentActivity.get(), stackActivities.size()); // Destroy the current activity exitActivity(lock); // Clear the stack @@ -156,10 +171,13 @@ void ActivityManager::loop() { stackActivities.back()->onExit(); stackActivities.pop_back(); } + logActivityStackState("replace_after_clear", nullptr, stackActivities.size()); } else if (pendingAction == PendingAction::Push) { + logActivityStackState("push_before", currentActivity.get(), stackActivities.size()); // Move current activity to stack stackActivities.push_back(std::move(currentActivity)); LOG_DBG("ACT", "Pushed to activity stack, new size = %zu", stackActivities.size()); + logActivityStackState("push_after", currentActivity.get(), stackActivities.size()); } pendingAction = PendingAction::None; currentActivity = std::move(pendingActivity); @@ -201,6 +219,8 @@ 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 + LOG_DBG("ACT", "replaceActivity requested: current=%s stackSize=%zu", + currentActivity->getName().c_str(), stackActivities.size()); pendingActivity = std::move(newActivity); pendingAction = PendingAction::Replace; } else { @@ -274,6 +294,8 @@ void ActivityManager::pushActivity(std::unique_ptr&& activity) { LOG_ERR("ACT", "pendingActivity while pushActivity is not expected"); pendingActivity.reset(); } + LOG_DBG("ACT", "pushActivity requested: current=%s stackSize=%zu", + currentActivity ? currentActivity->getName().c_str() : "", stackActivities.size()); pendingActivity = std::move(activity); pendingAction = PendingAction::Push; } From b4881e1563cf00c83600450440340ffde6ddf905 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 20:23:42 +0200 Subject: [PATCH 4/8] Refactor ActivityManager calls --- src/activities/Activity.cpp | 2 - src/activities/Activity.h | 1 - src/activities/ActivityManager.cpp | 76 ++++++++++++++----- src/activities/ActivityManager.h | 40 +++++++++- src/activities/home/FileBrowserActivity.cpp | 16 +++- src/activities/home/FileBrowserActivity.h | 8 +- .../home/GlobalBookmarksActivity.cpp | 6 +- src/activities/home/HomeActivity.cpp | 17 ++++- src/activities/home/HomeActivity.h | 6 +- src/activities/home/RecentBooksActivity.cpp | 10 ++- src/activities/home/RecentBooksActivity.h | 5 +- src/activities/reader/ReaderActivity.cpp | 14 ++-- 12 files changed, 156 insertions(+), 45 deletions(-) diff --git a/src/activities/Activity.cpp b/src/activities/Activity.cpp index 21aaa1b2..17ad577d 100644 --- a/src/activities/Activity.cpp +++ b/src/activities/Activity.cpp @@ -12,8 +12,6 @@ void Activity::requestUpdateAndWait() { activityManager.requestUpdateAndWait(); void Activity::onGoHome() { activityManager.goHome(); } -void Activity::onSelectBook(const std::string& path) { activityManager.pushReader(path); } - void Activity::startActivityForResult(std::unique_ptr&& activity, ActivityResultHandler resultHandler) { this->resultHandler = std::move(resultHandler); activityManager.pushActivity(std::move(activity)); diff --git a/src/activities/Activity.h b/src/activities/Activity.h index f74599a2..c5e4949e 100644 --- a/src/activities/Activity.h +++ b/src/activities/Activity.h @@ -58,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 86acbeb1..7aa1f239 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -35,12 +35,8 @@ void ActivityManager::begin() { 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); + LOG_DBG("ACT", "%s: current=%s stackSize=%zu free=%lu contig=%lu", stage, + currentActivity ? currentActivity->getName().c_str() : "", stackSize, freeHeap, contigHeap); } void ActivityManager::renderTaskTrampoline(void* param) { @@ -125,10 +121,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()); @@ -219,8 +215,8 @@ 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 - LOG_DBG("ACT", "replaceActivity requested: current=%s stackSize=%zu", - currentActivity->getName().c_str(), stackActivities.size()); + LOG_DBG("ACT", "replaceActivity requested: current=%s stackSize=%zu", currentActivity->getName().c_str(), + stackActivities.size()); pendingActivity = std::move(newActivity); pendingAction = PendingAction::Replace; } else { @@ -236,12 +232,14 @@ 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) { + hasReturnHint = false; + 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) { + hasReturnHint = false; + replaceActivity(std::make_unique(renderer, mappedInput, focusIndex)); } void ActivityManager::goToGlobalBookmarks() { @@ -269,8 +267,45 @@ 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::Home: + default: + goHome(std::move(hint.selectName)); + break; + } } void ActivityManager::goToSleep() { @@ -286,7 +321,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) { + hasReturnHint = false; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(focusBookPath))); +} void ActivityManager::pushActivity(std::unique_ptr&& activity) { if (pendingActivity) { diff --git a/src/activities/ActivityManager.h b/src/activities/ActivityManager.h index b59f6e96..b108e988 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -15,6 +15,21 @@ 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 }; + +// Minimal state the returning parent needs to restore its previous view (directory, +// focused item, list index). Kept as a plain struct stored by value on the +// ActivityManager — single instance, overwritten per transition, no heap churn +// beyond the two small std::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 +}; + /** * ActivityManager * @@ -68,6 +83,12 @@ class ActivityManager { // into the next one. bool drainInput = false; + // Where returnFromChild() should route to. Set by replaceWith*() helpers, cleared + // in returnFromChild(). Cleared on any manual goHome()/goTo*() to avoid stale hints + // outliving the flow they were recorded for. + ReturnHint returnHint; + bool hasReturnHint = false; + public: explicit ActivityManager(GfxRenderer& renderer, MappedInputManager& mappedInput) : renderer(renderer), mappedInput(mappedInput), renderingMutex(xSemaphoreCreateMutex()) { @@ -85,18 +106,29 @@ 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 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 = {}); + + // 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(); // 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..0e5229a9 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(); } @@ -171,7 +180,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; } 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..0739f8de 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" @@ -124,7 +125,10 @@ 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::Home; + hint.selectName = entry.sourcePath; + activityManager.replaceWithReader(entry.sourcePath, std::move(hint)); } template diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index 52ec7658..fe9ad3d0 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -211,6 +211,16 @@ void HomeActivity::onEnter() { recentsLoaded = true; } + if (!focusBookPath.empty()) { + for (size_t i = 0; i < recentBooks.size(); ++i) { + if (recentBooks[i].path == focusBookPath) { + selectorIndex = static_cast(i); + break; + } + } + focusBookPath.clear(); + } + // Trigger first update menuEntriesDirty = true; requestUpdate(); @@ -348,7 +358,12 @@ 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) { switch (action) { diff --git a/src/activities/home/HomeActivity.h b/src/activities/home/HomeActivity.h index c4fad72a..32cece1f 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -44,6 +44,8 @@ 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 + void onSelectBook(const std::string& path); void dispatchMenuAction(MenuAction action); @@ -55,8 +57,8 @@ 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 = {}) + : Activity("Home", renderer, mappedInput), focusBookPath(std::move(focusBookPath)) {} 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/ReaderActivity.cpp b/src/activities/reader/ReaderActivity.cpp index 4c293b8e..031bd78b 100644 --- a/src/activities/reader/ReaderActivity.cpp +++ b/src/activities/reader/ReaderActivity.cpp @@ -102,28 +102,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() { From 6ca8de83290bee479edaf8a963134a797f5f59c2 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 20:43:36 +0200 Subject: [PATCH 5/8] Reduce debug logging --- docs/activity-manager.md | 81 +++++++++++++++++++- src/activities/Activity.cpp | 4 +- src/activities/ActivityManager.cpp | 30 ++++++-- src/activities/ActivityManager.h | 15 +++- src/activities/home/HomeActivity.cpp | 20 +++++ src/activities/home/HomeActivity.h | 10 ++- src/activities/reader/EpubReaderActivity.cpp | 8 ++ src/activities/reader/ReaderActivity.cpp | 8 ++ 8 files changed, 163 insertions(+), 13 deletions(-) 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/activities/Activity.cpp b/src/activities/Activity.cpp index 17ad577d..91122af1 100644 --- a/src/activities/Activity.cpp +++ b/src/activities/Activity.cpp @@ -10,7 +10,9 @@ void Activity::requestUpdate(bool immediate) { activityManager.requestUpdate(imm void Activity::requestUpdateAndWait() { activityManager.requestUpdateAndWait(); } -void Activity::onGoHome() { activityManager.goHome(); } +// "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/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 7aa1f239..92036049 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -22,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 @@ -32,12 +36,16 @@ 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); @@ -159,7 +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 @@ -167,13 +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); @@ -215,8 +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 { @@ -233,12 +253,10 @@ void ActivityManager::goToFileTransfer() { void ActivityManager::goToSettings() { replaceActivity(std::make_unique(renderer, mappedInput)); } void ActivityManager::goToFileBrowser(std::string path, std::string focusName) { - hasReturnHint = false; replaceActivity(std::make_unique(renderer, mappedInput, std::move(path), std::move(focusName))); } void ActivityManager::goToRecentBooks(int focusIndex) { - hasReturnHint = false; replaceActivity(std::make_unique(renderer, mappedInput, focusIndex)); } @@ -303,7 +321,7 @@ void ActivityManager::returnFromChild() { break; case ReturnTo::Home: default: - goHome(std::move(hint.selectName)); + goHome(std::move(hint.selectName), hint.selectIndex); break; } } @@ -321,9 +339,9 @@ void ActivityManager::goToFullScreenMessage(std::string message, EpdFontFamily:: void ActivityManager::goToWeather() { replaceActivity(std::make_unique(renderer, mappedInput)); } -void ActivityManager::goHome(std::string focusBookPath) { +void ActivityManager::goHome(std::string focusBookPath, int focusSelectorIndex) { hasReturnHint = false; - replaceActivity(std::make_unique(renderer, mappedInput, std::move(focusBookPath))); + replaceActivity(std::make_unique(renderer, mappedInput, std::move(focusBookPath), focusSelectorIndex)); } void ActivityManager::pushActivity(std::unique_ptr&& activity) { @@ -332,8 +350,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 b108e988..f0e6ec28 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -116,7 +116,7 @@ class ActivityManager { void goToBoot(); void goToFullScreenMessage(std::string message, EpdFontFamily::Style style = EpdFontFamily::REGULAR); void goToWeather(); - void goHome(std::string focusBookPath = {}); + 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 @@ -130,6 +130,19 @@ class ActivityManager { // 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/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index fe9ad3d0..a41b349c 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -211,15 +212,27 @@ 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; @@ -366,6 +379,13 @@ void HomeActivity::onSelectBook(const std::string& path) { } 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 32cece1f..dbf29f17 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -44,7 +44,8 @@ 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 + 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); @@ -57,8 +58,11 @@ class HomeActivity final : public Activity { void loadRecentCovers(int coverHeight); public: - explicit HomeActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::string focusBookPath = {}) - : Activity("Home", renderer, mappedInput), focusBookPath(std::move(focusBookPath)) {} + 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/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 7c3494e3..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) { diff --git a/src/activities/reader/ReaderActivity.cpp b/src/activities/reader/ReaderActivity.cpp index 031bd78b..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) { From d39246ee70b376eef49af8230e67a38ca581b4fd Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 20:54:49 +0200 Subject: [PATCH 6/8] yaclf --- src/SettingsList.h | 28 ++++++------------- .../settings/KOReaderSettingsActivity.cpp | 15 +++++----- 2 files changed, 16 insertions(+), 27 deletions(-) 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/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( From e74e52b23a0729e65bab697dca8f6c968c5e1e98 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 20:59:49 +0200 Subject: [PATCH 7/8] Review comments --- src/activities/ActivityManager.cpp | 10 +++++-- src/activities/ActivityManager.h | 26 ++++++++++++------- src/activities/home/FileBrowserActivity.cpp | 2 +- .../home/GlobalBookmarksActivity.cpp | 25 ++++++++++++++++-- src/activities/home/GlobalBookmarksActivity.h | 5 ++-- 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 92036049..c2490a8a 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -260,8 +260,11 @@ 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() { @@ -319,6 +322,9 @@ void ActivityManager::returnFromChild() { 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); diff --git a/src/activities/ActivityManager.h b/src/activities/ActivityManager.h index f0e6ec28..5ab9d04f 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -17,17 +17,19 @@ 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 }; +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). Kept as a plain struct stored by value on the -// ActivityManager — single instance, overwritten per transition, no heap churn -// beyond the two small std::strings. +// 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 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 }; /** @@ -83,9 +85,12 @@ class ActivityManager { // into the next one. bool drainInput = false; - // Where returnFromChild() should route to. Set by replaceWith*() helpers, cleared - // in returnFromChild(). Cleared on any manual goHome()/goTo*() to avoid stale hints - // outliving the flow they were recorded for. + // 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; @@ -109,6 +114,7 @@ class ActivityManager { 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(); diff --git a/src/activities/home/FileBrowserActivity.cpp b/src/activities/home/FileBrowserActivity.cpp index 0e5229a9..d3c07f66 100644 --- a/src/activities/home/FileBrowserActivity.cpp +++ b/src/activities/home/FileBrowserActivity.cpp @@ -323,5 +323,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/GlobalBookmarksActivity.cpp b/src/activities/home/GlobalBookmarksActivity.cpp index 0739f8de..5e50c6c9 100644 --- a/src/activities/home/GlobalBookmarksActivity.cpp +++ b/src/activities/home/GlobalBookmarksActivity.cpp @@ -27,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); @@ -126,8 +146,9 @@ void GlobalBookmarksActivity::openSelected() { LOG_DBG("GBA", "Jumping to bookmark in %s at %u/%u", entry.sourcePath.c_str(), bm.spineIndex, bm.pageNumber); ReturnHint hint; - hint.target = ReturnTo::Home; - hint.selectName = entry.sourcePath; + hint.target = ReturnTo::GlobalBookmarks; + hint.selectionContext = entry.sourcePath; + hint.selectBookmarkIndex = static_cast(row.bookmarkIndex); activityManager.replaceWithReader(entry.sourcePath, std::move(hint)); } 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; From 3e28b458888e40b0ca96f08491350bfc74248c7a Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 21:05:30 +0200 Subject: [PATCH 8/8] Fix according to comment --- src/activities/home/FileBrowserActivity.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/activities/home/FileBrowserActivity.cpp b/src/activities/home/FileBrowserActivity.cpp index d3c07f66..feb43aea 100644 --- a/src/activities/home/FileBrowserActivity.cpp +++ b/src/activities/home/FileBrowserActivity.cpp @@ -156,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();