From a6cb9b74524c20508d2a3378c27949ede8a0f3fa Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 18 May 2026 18:53:52 +0200 Subject: [PATCH 1/2] Pick up some valid refactor ideas from bgd indexing --- lib/Epub/Epub/Section.cpp | 18 +- src/activities/reader/EpubReaderActivity.cpp | 285 ++++++++---------- src/activities/reader/EpubReaderActivity.h | 110 +++++-- .../reader/KOReaderSyncActivity.cpp | 14 +- 4 files changed, 231 insertions(+), 196 deletions(-) diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index bd6df71f..6fe83033 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -132,21 +132,13 @@ void Section::evictOldVariants() const { // Find all cache variants belonging to this spineIndex char prefix[16]; snprintf(prefix, sizeof(prefix), "%d_", spineIndex); - size_t prefixLen = strlen(prefix); for (const auto& file : files) { - if (file.startsWith(prefix) && file.endsWith(".bin")) { - HalFile hf = Storage.open((sectionsDir + "/" + file.c_str()).c_str(), O_RDONLY); - if (hf) { - uint16_t md, mt; - if (hf.getModifyDateTime(&md, &mt)) { - variants.push_back({file.c_str(), md, mt}); - } else { - // If we can't get modified time, assume it's very old to evict it - variants.push_back({file.c_str(), 0, 0}); - } - } - } + if (!file.startsWith(prefix) || !file.endsWith(".bin")) continue; + uint16_t md = 0, mt = 0; + HalFile hf = Storage.open((sectionsDir + "/" + file.c_str()).c_str(), O_RDONLY); + if (hf) hf.getModifyDateTime(&md, &mt); + variants.push_back({file.c_str(), md, mt}); } if (variants.size() <= MAX_VARIANTS) return; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 7ea867ca..85c142be 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -269,12 +269,12 @@ void EpubReaderActivity::onEnter() { int dataSize = f.read(data, 6); if (dataSize == 4 || dataSize == 6) { currentSpineIndex = data[0] + (data[1] << 8); - nextPageNumber = data[2] + (data[3] << 8); - cachedSpineIndex = currentSpineIndex; - LOG_DBG("ERS", "Loaded cache: %d, %d", currentSpineIndex, nextPageNumber); + navTarget = NavigationTarget::makePage(data[2] + (data[3] << 8)); + navTarget.cachedSpineIdx = currentSpineIndex; + LOG_DBG("ERS", "Loaded cache: %d, %d", currentSpineIndex, navTarget.page); } if (dataSize == 6) { - cachedChapterTotalPageCount = data[4] + (data[5] << 8); + navTarget.cachedPageCount = data[4] + (data[5] << 8); } f.close(); } @@ -284,9 +284,7 @@ void EpubReaderActivity::onEnter() { LOG_ERR("ERS", "Invalid saved spine index %d (valid 0..%d), resetting to start", currentSpineIndex, epub->getSpineItemsCount() > 0 ? epub->getSpineItemsCount() - 1 : 0); currentSpineIndex = 0; - nextPageNumber = 0; - cachedSpineIndex = 0; - cachedChapterTotalPageCount = 0; + navTarget = NavigationTarget::makePage(0); } if (currentSpineIndex == 0) { @@ -360,6 +358,11 @@ void EpubReaderActivity::loop() { return; } + if (pendingProgressSave.pending) { + pendingProgressSave.pending = false; + saveProgress(pendingProgressSave.spineIndex, pendingProgressSave.page, pendingProgressSave.pageCount); + } + if (inputDrainGuard.shouldDrain(mappedInput)) { buttonEvents.drain(); return; @@ -507,7 +510,7 @@ void EpubReaderActivity::loop() { }); } else { currentSpineIndex = epub->getSpineItemsCount() - 1; - nextPageNumber = UINT16_MAX; + navTarget = NavigationTarget::makeLastPage(); requestUpdate(); } return; @@ -570,21 +573,18 @@ void EpubReaderActivity::jumpToPercent(int percent) { const size_t cumulative = epub->getCumulativeSpineItemSize(targetSpineIndex); const size_t spineSize = (cumulative > prevCumulative) ? (cumulative - prevCumulative) : 0; - // Store a normalized position within the spine so it can be applied once loaded. - pendingSpineProgress = + float spineProgress = (spineSize == 0) ? 0.0f : static_cast(targetSize - prevCumulative) / static_cast(spineSize); - if (pendingSpineProgress < 0.0f) { - pendingSpineProgress = 0.0f; - } else if (pendingSpineProgress > 1.0f) { - pendingSpineProgress = 1.0f; - } + if (spineProgress < 0.0f) + spineProgress = 0.0f; + else if (spineProgress > 1.0f) + spineProgress = 1.0f; // Reset state so render() reloads and repositions on the target spine. { RenderLock lock(*this); currentSpineIndex = targetSpineIndex; - nextPageNumber = 0; - pendingPercentJump = true; + navTarget = NavigationTarget::makePercent(spineProgress); section.reset(); } } @@ -608,9 +608,9 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction if (resolvedPage) { section->currentPage = *resolvedPage; } else { - pendingTocIndex = chapter.tocIndex; + navTarget = + chapter.tocIndex ? NavigationTarget::makeTocIndex(*chapter.tocIndex) : NavigationTarget::makePage(0); currentSpineIndex = chapter.spineIndex; - nextPageNumber = 0; section.reset(); } }); @@ -687,7 +687,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction if (currentSpineIndex != starred.spineIndex || !section || section->currentPage != starred.pageNumber) { RenderLock lock(*this); currentSpineIndex = starred.spineIndex; - nextPageNumber = starred.pageNumber; + navTarget = NavigationTarget::makePage(starred.pageNumber); section.reset(); } } @@ -1125,55 +1125,52 @@ void EpubReaderActivity::applyPendingSyncSession() { int restoreSpineIndex = sync.spineIndex; int restorePage = sync.page; - pendingParagraphLookup = false; - pendingParagraphIndex = 0; - pendingListItemLookup = false; - pendingListItemIndex = 0; if (restoreSpineIndex < 0 || restoreSpineIndex >= epub->getSpineItemsCount()) { LOG_ERR("ERS", "Invalid sync restore spine index %d, resetting to 0", restoreSpineIndex); restoreSpineIndex = 0; restorePage = 0; - pendingParagraphLookup = false; - pendingParagraphIndex = 0; - pendingListItemLookup = false; - pendingListItemIndex = 0; } + // Build the navigation target from the sync result. + NavigationTarget restoreTarget; if (sync.outcome == KOReaderSyncOutcomeState::APPLIED_REMOTE) { restoreSpineIndex = sync.resultSpineIndex; restorePage = sync.resultPage; - pendingParagraphLookup = sync.resultHasParagraphIndex; - pendingParagraphIndex = sync.resultParagraphIndex; - pendingListItemLookup = sync.resultHasListItemIndex; - pendingListItemIndex = sync.resultListItemIndex; - LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d paragraph=%u hasParagraph=%s liIdx=%u hasLi=%s", - restoreSpineIndex, restorePage, pendingParagraphIndex, pendingParagraphLookup ? "yes" : "no", - pendingListItemIndex, pendingListItemLookup ? "yes" : "no"); + if (sync.resultHasListItemIndex) { + restoreTarget = NavigationTarget::makeListItem(sync.resultListItemIndex); + LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d li[%u]", restoreSpineIndex, restorePage, + sync.resultListItemIndex); + } else if (sync.resultHasParagraphIndex) { + restoreTarget = NavigationTarget::makeParagraph(sync.resultParagraphIndex); + LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d p[%u]", restoreSpineIndex, restorePage, + sync.resultParagraphIndex); + } else { + restoreTarget = NavigationTarget::makePage(restorePage); + LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d (no LUT)", restoreSpineIndex, restorePage); + } } else { + restoreTarget = NavigationTarget::makePage(restorePage); LOG_DBG("ERS", "Restored local pre-sync position: spine=%d page=%d", restoreSpineIndex, restorePage); } // sync.totalPagesInSpine is the page count of the local spine at launch time. - // When the restore targets a different spine, that count is meaningless for the - // rescaling logic in render() and can cause out-of-bounds pages (the estimated - // page number may exceed the local spine's count, producing progress > 1.0). - // Store 0 to disable rescaling; the paragraph lookup handles precise positioning. + // When the restore targets a different spine, that count is meaningless for + // rescaling. Store 0 to disable rescaling; the LUT lookup handles precise positioning. const int restorePageCount = (restoreSpineIndex == sync.spineIndex) ? sync.totalPagesInSpine : 0; + restoreTarget.cachedPageCount = restorePageCount; + restoreTarget.cachedSpineIdx = restoreSpineIndex; // Transient write — the next render's saveProgress() supplies the real percent before the user // can return to the home screen, so a placeholder 0 here is harmless. if (writeReaderProgressCache(epub->getCachePath(), restoreSpineIndex, restorePage, restorePageCount, 0)) { - cachedSpineIndex = restoreSpineIndex; - cachedChapterTotalPageCount = restorePageCount; + navTarget = restoreTarget; LOG_DBG("ERS", "Prepared progress.bin for sync restore: spine=%d page=%d/%d", restoreSpineIndex, restorePage, sync.totalPagesInSpine); } else { // Fall back to directly seeding live state if cache write fails. currentSpineIndex = restoreSpineIndex; - nextPageNumber = restorePage; - cachedSpineIndex = restoreSpineIndex; - cachedChapterTotalPageCount = restorePageCount; + navTarget = restoreTarget; } sync.clear(); @@ -1194,13 +1191,12 @@ void EpubReaderActivity::applyPendingBookmarkJump() { } // Transient write before initializeReader; saveProgress() overwrites with the real percent. if (writeReaderProgressCache(epub->getCachePath(), jump.spineIndex, jump.pageNumber, 0, 0)) { - cachedSpineIndex = jump.spineIndex; - cachedChapterTotalPageCount = 0; + navTarget = NavigationTarget::makePage(jump.pageNumber); + navTarget.cachedSpineIdx = jump.spineIndex; } else { currentSpineIndex = jump.spineIndex; - nextPageNumber = jump.pageNumber; - cachedSpineIndex = jump.spineIndex; - cachedChapterTotalPageCount = 0; + navTarget = NavigationTarget::makePage(jump.pageNumber); + navTarget.cachedSpineIdx = jump.spineIndex; } jump.clear(); APP_STATE.saveToFile(); @@ -1216,9 +1212,9 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) { { RenderLock lock(*this); if (section) { - cachedSpineIndex = currentSpineIndex; - cachedChapterTotalPageCount = section->pageCount; - nextPageNumber = section->currentPage; + navTarget = NavigationTarget::makePage(section->currentPage); + navTarget.cachedPageCount = section->pageCount; + navTarget.cachedSpineIdx = currentSpineIndex; } // Persist the selection so the reader keeps the new orientation on next launch. @@ -1258,9 +1254,9 @@ void EpubReaderActivity::stopAutomaticPageTurn() { // Preserve current reading position so we can restore after reflow. RenderLock lock(*this); if (section) { - cachedSpineIndex = currentSpineIndex; - cachedChapterTotalPageCount = section->pageCount; - nextPageNumber = section->currentPage; + navTarget = NavigationTarget::makePage(section->currentPage); + navTarget.cachedPageCount = section->pageCount; + navTarget.cachedSpineIdx = currentSpineIndex; } section.reset(); } @@ -1281,9 +1277,9 @@ void EpubReaderActivity::toggleAutoPageTurn(const uint8_t selectedPageTurnOption // Preserve current reading position so we can restore after reflow. RenderLock lock(*this); if (section) { - cachedSpineIndex = currentSpineIndex; - cachedChapterTotalPageCount = section->pageCount; - nextPageNumber = section->currentPage; + navTarget = NavigationTarget::makePage(section->currentPage); + navTarget.cachedPageCount = section->pageCount; + navTarget.cachedSpineIdx = currentSpineIndex; } section.reset(); } @@ -1328,9 +1324,9 @@ void EpubReaderActivity::applyBookReaderOverrides(const int8_t embeddedStyleOver RenderLock lock(*this); if (section) { - cachedSpineIndex = currentSpineIndex; - cachedChapterTotalPageCount = section->pageCount; - nextPageNumber = section->currentPage; + navTarget = NavigationTarget::makePage(section->currentPage); + navTarget.cachedPageCount = section->pageCount; + navTarget.cachedSpineIdx = currentSpineIndex; } section.reset(); } @@ -1410,6 +1406,65 @@ int EpubReaderActivity::getEffectiveReaderFontId() const { return SETTINGS.getReaderFontId(); } +void EpubReaderActivity::NavigationTarget::resolveInto(Section& sec, int spineIndex) const { + if (kind == Kind::LastPage) { + sec.currentPage = (sec.pageCount > 0) ? sec.pageCount - 1 : 0; + return; + } + if (kind == Kind::TocIndex) { + if (const auto p = sec.getPageForTocIndex(tocIndex)) sec.currentPage = *p; + return; + } + if (kind == Kind::Anchor) { + if (const auto p = sec.getPageForAnchor(anchorStr)) { + sec.currentPage = *p; + LOG_DBG("ERS", "Resolved anchor '%s' -> page %d", anchorStr.c_str(), *p); + } else { + LOG_DBG("ERS", "Anchor '%s' not found in section", anchorStr.c_str()); + } + return; + } + if (kind == Kind::ListItem) { + if (const auto p = sec.getPageForListItemIndex(lutIndex)) { + sec.currentPage = *p; + LOG_DBG("ERS", "Resolved li[%u] -> page %d", lutIndex, *p); + } else { + LOG_DBG("ERS", "Li index %u not found in section LUT", lutIndex); + } + return; + } + if (kind == Kind::Paragraph) { + if (const auto p = sec.getPageForParagraphIndex(lutIndex)) { + sec.currentPage = *p; + LOG_DBG("ERS", "Resolved p[%u] -> page %d", lutIndex, *p); + } else { + LOG_DBG("ERS", "Paragraph LUT miss, using page %d", sec.currentPage); + } + return; + } + if (kind == Kind::Percent) { + if (sec.pageCount > 0) { + int newPage = static_cast(spineProgress * static_cast(sec.pageCount)); + if (newPage >= sec.pageCount) newPage = sec.pageCount - 1; + sec.currentPage = newPage; + } + return; + } + // Kind::Page — apply baseline, then cross-font rescale if we have a cached page count. + sec.currentPage = page; + if (cachedPageCount > 0 && cachedSpineIdx == spineIndex) { + if (sec.pageCount != cachedPageCount) { + const float progress = static_cast(sec.currentPage) / static_cast(cachedPageCount); + sec.currentPage = static_cast(progress * static_cast(sec.pageCount)); + } + } + // Safety clamp. + if (sec.pageCount > 0 && sec.currentPage >= sec.pageCount) { + LOG_DBG("ERS", "Clamping page %d to last page %d", sec.currentPage, sec.pageCount - 1); + sec.currentPage = sec.pageCount - 1; + } +} + bool EpubReaderActivity::stepPageState(const bool isForwardTurn) { if (!epub || !section || section->pageCount == 0) { return false; @@ -1420,12 +1475,12 @@ bool EpubReaderActivity::stepPageState(const bool isForwardTurn) { section->currentPage++; } else if (currentSpineIndex + 1 < epub->getSpineItemsCount()) { RenderLock lock(*this); - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); currentSpineIndex++; section.reset(); } else if (currentSpineIndex + 1 == epub->getSpineItemsCount()) { RenderLock lock(*this); - nextPageNumber = UINT16_MAX; + navTarget = NavigationTarget::makeLastPage(); currentSpineIndex++; section.reset(); } else { @@ -1436,7 +1491,7 @@ bool EpubReaderActivity::stepPageState(const bool isForwardTurn) { section->currentPage--; } else if (currentSpineIndex > 0) { RenderLock lock(*this); - nextPageNumber = UINT16_MAX; + navTarget = NavigationTarget::makeLastPage(); currentSpineIndex--; section.reset(); } else { @@ -1561,7 +1616,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { if (currentSpineIndex < 0 || currentSpineIndex >= spineCount) { LOG_ERR("ERS", "Render rejected invalid spine index %d (valid 0..%d)", currentSpineIndex, spineCount - 1); currentSpineIndex = 0; - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); automaticPageTurnActive = false; requestUpdate(); return; @@ -1639,82 +1694,8 @@ void EpubReaderActivity::render(RenderLock&& lock) { LOG_INF("ERS", "Section %d is truncated; showing mitigation hint", currentSpineIndex); } - if (nextPageNumber == UINT16_MAX) { - section->currentPage = section->pageCount - 1; - } else { - section->currentPage = nextPageNumber; - } - - if (pendingTocIndex) { - if (const auto resolvedPage = section->getPageForTocIndex(*pendingTocIndex)) { - section->currentPage = *resolvedPage; - } - pendingTocIndex.reset(); - } - - if (!pendingAnchor.empty()) { - if (const auto page = section->getPageForAnchor(pendingAnchor)) { - section->currentPage = *page; - LOG_DBG("ERS", "Resolved anchor '%s' to page %d", pendingAnchor.c_str(), *page); - } else { - LOG_DBG("ERS", "Anchor '%s' not found in section %d", pendingAnchor.c_str(), currentSpineIndex); - } - pendingAnchor.clear(); - } - - // Resolve pending KOReader sync position via Section LUTs. - //
  • -anchored XPaths can't be expressed in the body-child

    LUT, so try the - // li LUT first when set; fall back to the paragraph LUT (which handles direct - //

    children of ) on miss. - bool resolvedFromLut = false; - if (pendingListItemLookup) { - if (const auto page = section->getPageForListItemIndex(pendingListItemIndex)) { - section->currentPage = *page; - LOG_DBG("ERS", "Resolved li[%u] to page %d (was %d)", pendingListItemIndex, *page, nextPageNumber); - resolvedFromLut = true; - } else { - LOG_DBG("ERS", "Li index %u not found in section LUT", pendingListItemIndex); - } - pendingListItemLookup = false; - } - if (!resolvedFromLut && pendingParagraphLookup) { - if (const auto page = section->getPageForParagraphIndex(pendingParagraphIndex)) { - section->currentPage = *page; - LOG_DBG("ERS", "Resolved p[%u] to page %d (was %d)", pendingParagraphIndex, *page, nextPageNumber); - } else { - LOG_DBG("ERS", "Paragraph LUT not available, using estimated page %d", nextPageNumber); - } - } - pendingParagraphLookup = false; - - // handles changes in reader settings and reset to approximate position based on cached progress - if (cachedChapterTotalPageCount > 0) { - // only goes to relative position if spine index matches cached value - if (currentSpineIndex == cachedSpineIndex && section->pageCount != cachedChapterTotalPageCount) { - float progress = static_cast(section->currentPage) / static_cast(cachedChapterTotalPageCount); - int newPage = static_cast(progress * section->pageCount); - section->currentPage = newPage; - } - cachedChapterTotalPageCount = 0; // resets to 0 to prevent reading cached progress again - } - - // Safety clamp: estimated page numbers from sync or progress.bin may exceed - // the actual page count when the section was built with different settings or - // the estimate was based on a different spine's density. - if (section->pageCount > 0 && section->currentPage >= section->pageCount) { - LOG_DBG("ERS", "Clamping page %d to last page %d", section->currentPage, section->pageCount - 1); - section->currentPage = section->pageCount - 1; - } - - if (pendingPercentJump && section->pageCount > 0) { - // Apply the pending percent jump now that we know the new section's page count. - int newPage = static_cast(pendingSpineProgress * static_cast(section->pageCount)); - if (newPage >= section->pageCount) { - newPage = section->pageCount - 1; - } - section->currentPage = newPage; - pendingPercentJump = false; - } + navTarget.resolveInto(*section, currentSpineIndex); + navTarget = NavigationTarget::makePage(section->currentPage); } renderer.clearScreen(); @@ -1769,7 +1750,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { LOG_DBG("ERS", "Rendered page in %dms", lastRenderStats.requestRenderMs); } silentIndexNextChapterIfNeeded(viewportWidth, viewportHeight); - saveProgress(currentSpineIndex, section->currentPage, section->pageCount); + pendingProgressSave = {true, currentSpineIndex, section->currentPage, section->pageCount}; lastRenderStats.freeHeapAfter = esp_get_free_heap_size(); lastRenderStats.largestFreeBlockAfter = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); lastRenderStats.valid = true; @@ -2123,9 +2104,8 @@ void EpubReaderActivity::navigateToHref(const std::string& hrefStr, const bool s { RenderLock lock(*this); - pendingAnchor = std::move(anchor); + navTarget = anchor.empty() ? NavigationTarget::makePage(0) : NavigationTarget::makeAnchor(std::move(anchor)); currentSpineIndex = targetSpineIndex; - nextPageNumber = 0; section.reset(); } requestUpdate(); @@ -2141,7 +2121,7 @@ void EpubReaderActivity::restoreSavedPosition() { { RenderLock lock(*this); currentSpineIndex = pos.spineIndex; - nextPageNumber = pos.pageNumber; + navTarget = NavigationTarget::makePage(pos.pageNumber); section.reset(); } requestUpdate(); @@ -2361,9 +2341,9 @@ void EpubReaderActivity::onButtonAction(const CrossPointSettings::BUTTON_ACTION if (resolvedPage) { section->currentPage = *resolvedPage; } else { - pendingTocIndex = chapter.tocIndex; + navTarget = chapter.tocIndex ? NavigationTarget::makeTocIndex(*chapter.tocIndex) + : NavigationTarget::makePage(0); currentSpineIndex = chapter.spineIndex; - nextPageNumber = 0; section.reset(); } }); @@ -2378,7 +2358,7 @@ void EpubReaderActivity::onButtonAction(const CrossPointSettings::BUTTON_ACTION const int curTocIndex = section->getTocIndexForPage(section->currentPage); const int nextTocIndex = forward ? curTocIndex + 1 : curTocIndex - 1; if (curTocIndex < 0) { - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); currentSpineIndex = forward ? currentSpineIndex + 1 : currentSpineIndex - 1; section.reset(); } else if (nextTocIndex >= 0 && nextTocIndex < epub->getTocItemsCount()) { @@ -2388,22 +2368,21 @@ void EpubReaderActivity::onButtonAction(const CrossPointSettings::BUTTON_ACTION section->currentPage = *resolvedPage; } } else { - pendingTocIndex = nextTocIndex; - nextPageNumber = 0; + navTarget = NavigationTarget::makeTocIndex(nextTocIndex); currentSpineIndex = newSpineIndex; section.reset(); } } else if (forward) { - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); currentSpineIndex = epub->getSpineItemsCount(); section.reset(); } else { - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); currentSpineIndex = epub->getTocItem(curTocIndex).spineIndex - 1; section.reset(); } } else { - nextPageNumber = 0; + navTarget = NavigationTarget::makePage(0); currentSpineIndex = forward ? currentSpineIndex + 1 : currentSpineIndex - 1; section.reset(); } diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 9d8747f1..0a9022ba 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -3,8 +3,6 @@ #include #include -#include - #include "BookmarkStore.h" #include "EpubReaderMenuActivity.h" #include "ReaderUtils.h" @@ -24,19 +22,88 @@ class EpubReaderActivity final : public Activity { AUTO_PUSH, }; + // Encodes pending navigation intent — where to land once the target section is loaded. + // Replaces the scattered nextPageNumber / pendingTocIndex / pendingAnchor / + // cachedSpineIndex / cachedChapterTotalPageCount / pendingPercent* / pendingParagraph* fields. + struct NavigationTarget { + enum class Kind : uint8_t { + Page, // go to page n (0-based) + LastPage, // go to last page of section (was UINT16_MAX sentinel) + Anchor, // href fragment (e.g. "note1") + TocIndex, // TOC entry index + Percent, // normalised 0.0–1.0 within spine + Paragraph, // KOReader paragraph LUT index + ListItem, // KOReader li-anchored LUT index + }; + Kind kind = Kind::Page; + union { + int page; // Kind::Page + int tocIndex; // Kind::TocIndex + float spineProgress; // Kind::Percent + uint16_t lutIndex; // Kind::Paragraph / Kind::ListItem + }; + std::string anchorStr; // Kind::Anchor; empty for all others + // Cross-font rescaling: page count of this spine at save time. + // Non-zero only for Kind::Page when loaded from progress.bin or written during reflow. + int cachedPageCount = 0; + int cachedSpineIdx = 0; + + NavigationTarget() : kind(Kind::Page), page(0) {} + + static NavigationTarget makePage(int n) { + NavigationTarget t; + t.kind = Kind::Page; + t.page = n; + return t; + } + static NavigationTarget makeLastPage() { + NavigationTarget t; + t.kind = Kind::LastPage; + t.page = 0; + return t; + } + static NavigationTarget makeAnchor(std::string a) { + NavigationTarget t; + t.kind = Kind::Anchor; + t.page = 0; + t.anchorStr = std::move(a); + return t; + } + static NavigationTarget makeTocIndex(int idx) { + NavigationTarget t; + t.kind = Kind::TocIndex; + t.tocIndex = idx; + return t; + } + static NavigationTarget makePercent(float sp) { + NavigationTarget t; + t.kind = Kind::Percent; + t.spineProgress = sp; + return t; + } + static NavigationTarget makeParagraph(uint16_t i) { + NavigationTarget t; + t.kind = Kind::Paragraph; + t.lutIndex = i; + return t; + } + static NavigationTarget makeListItem(uint16_t i) { + NavigationTarget t; + t.kind = Kind::ListItem; + t.lutIndex = i; + return t; + } + + // Resolves the target into section.currentPage. Must be called on the render task + // after the section has been loaded (pageCount is known). + void resolveInto(Section& section, int spineIndex) const; + }; + std::shared_ptr epub; std::unique_ptr

    section = nullptr; int currentSpineIndex = 0; - int nextPageNumber = 0; - // Set when navigating to a TOC entry in a different spine (chapter skip or chapter selector). - // Cleared on the next render after the new section loads and resolves it to a page. - std::optional pendingTocIndex; - // Set when navigating to a footnote href with a fragment (e.g. #note1). - // Cleared on the next render after the new section loads and resolves it to a page. - std::string pendingAnchor; + NavigationTarget navTarget; int pagesUntilFullRefresh = 0; - int cachedSpineIndex = 0; - int cachedChapterTotalPageCount = 0; unsigned long lastPageTurnTime = 0UL; unsigned long pageTurnDuration = 0UL; bool pendingHalfRefreshAfterImagePage = false; @@ -120,19 +187,14 @@ class EpubReaderActivity final : public Activity { uint32_t maxFreeHeapAfter = 0; }; LastRenderStats lastRenderStats; - // Signals that the next render should reposition within the newly loaded section - // based on a cross-book percentage jump. - bool pendingPercentJump = false; - // Normalized 0.0-1.0 progress within the target spine item, computed from book percentage. - float pendingSpineProgress = 0.0f; - // Pending paragraph index from KOReader sync (resolved to page via Section paragraph LUT) - bool pendingParagraphLookup = false; - uint16_t pendingParagraphIndex = 0; - // Pending list-item index for KOReader-supplied XPaths whose deepest element is /li[N]. - // Preferred over pendingParagraphLookup when set because
  • -anchored XPaths are not - // representable in the body-child

    LUT. - bool pendingListItemLookup = false; - uint16_t pendingListItemIndex = 0; + // Progress save is posted by render() and consumed by loop() to keep SD I/O off the render task. + struct PendingProgressSave { + bool pending = false; + int spineIndex = 0; + int page = 0; + int pageCount = 0; + }; + PendingProgressSave pendingProgressSave; bool pendingScreenshot = false; bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit bool finishedBookActivityStarted_ = false; diff --git a/src/activities/reader/KOReaderSyncActivity.cpp b/src/activities/reader/KOReaderSyncActivity.cpp index f00e174e..ea51caf5 100644 --- a/src/activities/reader/KOReaderSyncActivity.cpp +++ b/src/activities/reader/KOReaderSyncActivity.cpp @@ -343,12 +343,14 @@ void KOReaderSyncActivity::performSync() { // Drop EPUB state before HTTPS to maximize contiguous heap for TLS. releaseEpubForMapping(); - // Push intent skips comparison UI but still warms an HTTP/TLS session first - // so PUT can reuse the connection instead of forcing a fresh handshake. - if (syncIntent == KOReaderSyncIntentState::PUSH_LOCAL || syncIntent == KOReaderSyncIntentState::AUTO_PUSH) { - if (!handleAutoPushPreflight()) { - return; - } + // PUSH_LOCAL is an explicit user upload — go straight to PUT. + // AUTO_PUSH needs the preflight GET to bail out if the remote is already ahead. + if (syncIntent == KOReaderSyncIntentState::PUSH_LOCAL) { + performUpload(); + return; + } + if (syncIntent == KOReaderSyncIntentState::AUTO_PUSH) { + if (!handleAutoPushPreflight()) return; performUpload(); return; } From d0eb1c07f4ee5fbb7182a62db7bc55d516991407 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 18 May 2026 19:15:52 +0200 Subject: [PATCH 2/2] Review changes --- src/activities/reader/EpubReaderActivity.cpp | 25 ++++++++++++++++---- src/activities/reader/EpubReaderActivity.h | 6 ++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 85c142be..7990f6ee 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -358,8 +358,8 @@ void EpubReaderActivity::loop() { return; } - if (pendingProgressSave.pending) { - pendingProgressSave.pending = false; + if (pendingProgressSave.pending.load(std::memory_order_acquire)) { + pendingProgressSave.pending.store(false, std::memory_order_relaxed); saveProgress(pendingProgressSave.spineIndex, pendingProgressSave.page, pendingProgressSave.pageCount); } @@ -1135,8 +1135,15 @@ void EpubReaderActivity::applyPendingSyncSession() { // Build the navigation target from the sync result. NavigationTarget restoreTarget; if (sync.outcome == KOReaderSyncOutcomeState::APPLIED_REMOTE) { - restoreSpineIndex = sync.resultSpineIndex; - restorePage = sync.resultPage; + const int spineCount = epub->getSpineItemsCount(); + if (sync.resultSpineIndex < 0 || sync.resultSpineIndex >= spineCount) { + LOG_ERR("ERS", "Sync resultSpineIndex %d out of range [0,%d), clamping to previous %d", sync.resultSpineIndex, + spineCount, restoreSpineIndex); + // Keep restoreSpineIndex / restorePage from the pre-validation block above. + } else { + restoreSpineIndex = sync.resultSpineIndex; + restorePage = sync.resultPage; + } if (sync.resultHasListItemIndex) { restoreTarget = NavigationTarget::makeListItem(sync.resultListItemIndex); LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d li[%u]", restoreSpineIndex, restorePage, @@ -1459,6 +1466,11 @@ void EpubReaderActivity::NavigationTarget::resolveInto(Section& sec, int spineIn } } // Safety clamp. + if (sec.currentPage < 0) { + LOG_DBG("ERS", "Clamping negative page %d to 0 (spine=%d cachedPageCount=%d)", sec.currentPage, spineIndex, + cachedPageCount); + sec.currentPage = 0; + } if (sec.pageCount > 0 && sec.currentPage >= sec.pageCount) { LOG_DBG("ERS", "Clamping page %d to last page %d", sec.currentPage, sec.pageCount - 1); sec.currentPage = sec.pageCount - 1; @@ -1750,7 +1762,10 @@ void EpubReaderActivity::render(RenderLock&& lock) { LOG_DBG("ERS", "Rendered page in %dms", lastRenderStats.requestRenderMs); } silentIndexNextChapterIfNeeded(viewportWidth, viewportHeight); - pendingProgressSave = {true, currentSpineIndex, section->currentPage, section->pageCount}; + pendingProgressSave.spineIndex = currentSpineIndex; + pendingProgressSave.page = section->currentPage; + pendingProgressSave.pageCount = section->pageCount; + pendingProgressSave.pending.store(true, std::memory_order_release); lastRenderStats.freeHeapAfter = esp_get_free_heap_size(); lastRenderStats.largestFreeBlockAfter = heap_caps_get_largest_free_block(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT); lastRenderStats.valid = true; diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 0a9022ba..d242046a 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -3,6 +3,8 @@ #include #include +#include + #include "BookmarkStore.h" #include "EpubReaderMenuActivity.h" #include "ReaderUtils.h" @@ -188,8 +190,10 @@ class EpubReaderActivity final : public Activity { }; LastRenderStats lastRenderStats; // Progress save is posted by render() and consumed by loop() to keep SD I/O off the render task. + // render() writes spineIndex/page/pageCount then sets pending with release semantics so loop() + // sees a coherent snapshot when it observes pending==true via acquire. struct PendingProgressSave { - bool pending = false; + std::atomic pending{false}; int spineIndex = 0; int page = 0; int pageCount = 0;