diff --git a/lib/KOReaderSync/ChapterXPathForwardMapper.cpp b/lib/KOReaderSync/ChapterXPathForwardMapper.cpp index b68f4b1a..ca6f62d1 100644 --- a/lib/KOReaderSync/ChapterXPathForwardMapper.cpp +++ b/lib/KOReaderSync/ChapterXPathForwardMapper.cpp @@ -19,14 +19,24 @@ namespace { // Strategy: // 1) Count total visible text bytes in chapter. // 2) Stream parse again and stop when target byte offset is reached. -// 3) Emit /text()[N].M relative to the deepest open element so KOReader can -// place the cursor at character precision regardless of nesting depth. +// 3) Emit either /text()[N].M when the cursor is at a direct text child of +// , or the bare element path otherwise. // -// Text-node counting matches KOReader/crengine: the Nth XML text node within -// an element, including whitespace-only nodes (those are still real DOM text -// nodes). Empty (len=0) text isn't emitted by expat at all, which mirrors -// KOReader's behavior of skipping the empty text nodes that bare -// elements would otherwise produce. +// Why body-level only (and not deep nested /p[i]/span[j]/text()[k].M): +// KOReader's crengine normalises the DOM differently than expat — it merges +// adjacent inline elements, drops empty wrappers, and renumbers text nodes +// inside

//. A deep XPath we emit (e.g. /p[17]/span[1]/text()[1].26) +// often fails to match crengine's tree, and KOReader stores a degraded +// fallback position (start-of-wrapper-div or off-by-N text node) that +// round-trips back to the wrong page on pull. Body-level text-point XPaths +// have a much higher round-trip success rate even though they sacrifice +// character-precision inside paragraphs. The Section paragraph LUT then +// snaps the pulled position to the correct page anyway, so the precision +// loss is invisible to users. +// +// This matches the 1.42 behavior. The pre-1.43 forward mapper only emitted +// text-point XPaths when the cursor was a direct text child of ; the +// 1.43 change to deep emission is the regression we're undoing here. struct ForwardState : StackState { int spineIndex; @@ -35,32 +45,24 @@ struct ForwardState : StackState { bool found = false; XML_Parser parser = nullptr; - // Per-element text-node bookkeeping. Mirrors `stack` 1:1 — every push/pop - // appends/removes a counter so the top of the stack always refers to the - // currently open element. `pendingTextNode` is set after every element - // boundary so the next char data starts a fresh text node within whatever - // element is currently on top. - std::vector textNodeIndexStack; - std::vector codepointsInTextNodeStack; - bool pendingTextNode = true; + // Body-level text-node bookkeeping: only counts text nodes that are direct + // children of . Inline-element text contributes to totalTextBytes via + // the StackState base, but does not advance bodyTextNodeCount because + // KOReader can't round-trip a deep text-node XPath reliably. + int bodyTextNodeCount = 0; + size_t codepointsInBodyTextNode = 0; + bool inBodyTextNode = false; - ForwardState(const int spineIndex, const size_t targetOffset) : spineIndex(spineIndex), targetOffset(targetOffset) { - textNodeIndexStack.reserve(32); - codepointsInTextNodeStack.reserve(32); - } + ForwardState(const int spineIndex, const size_t targetOffset) : spineIndex(spineIndex), targetOffset(targetOffset) {} void onStartElement(const XML_Char* rawName) { + inBodyTextNode = false; pushElement(rawName); - textNodeIndexStack.push_back(0); - codepointsInTextNodeStack.push_back(0); - pendingTextNode = true; } void onEndElement() { + inBodyTextNode = false; popElement(); - if (!textNodeIndexStack.empty()) textNodeIndexStack.pop_back(); - if (!codepointsInTextNodeStack.empty()) codepointsInTextNodeStack.pop_back(); - pendingTextNode = true; } void onCharData(const XML_Char* text, const int len) { @@ -68,30 +70,34 @@ struct ForwardState : StackState { return; } - if (pendingTextNode) { - if (!textNodeIndexStack.empty()) textNodeIndexStack.back()++; - if (!codepointsInTextNodeStack.empty()) codepointsInTextNodeStack.back() = 0; - pendingTextNode = false; + const bool atBodyLevel = bodyIdx() + 1 == static_cast(stack.size()); + if (atBodyLevel && !inBodyTextNode) { + inBodyTextNode = true; + bodyTextNodeCount++; + codepointsInBodyTextNode = 0; } - const size_t cpCount = countUtf8Codepoints(text, len); - if (isWhitespaceOnly(text, len)) { - if (!codepointsInTextNodeStack.empty()) codepointsInTextNodeStack.back() += cpCount; + if (atBodyLevel) { + codepointsInBodyTextNode += countUtf8Codepoints(text, len); + } return; } const size_t visible = countVisibleBytes(text, len); if (totalTextBytes + visible >= targetOffset) { - const int textNode = textNodeIndexStack.empty() ? 0 : textNodeIndexStack.back(); - const size_t cpsInNode = codepointsInTextNodeStack.empty() ? 0 : codepointsInTextNodeStack.back(); - // KOReader/crengine text-point semantics use codepoint offsets. - const size_t targetVisibleByteInChunk = targetOffset - totalTextBytes; - const size_t cpInChunk = codepointAtVisibleByte(text, len, targetVisibleByteInChunk); - const size_t charOff = cpsInNode + cpInChunk; - if (textNode > 0) { - result = currentXPath(spineIndex) + "/text()[" + std::to_string(textNode) + "]." + std::to_string(charOff); + if (atBodyLevel && bodyTextNodeCount > 0) { + // KOReader/crengine text-point semantics use codepoint offsets. + const size_t targetVisibleByteInChunk = targetOffset - totalTextBytes; + const size_t cpInChunk = codepointAtVisibleByte(text, len, targetVisibleByteInChunk); + const size_t charOff = codepointsInBodyTextNode + cpInChunk; + result = + currentXPath(spineIndex) + "/text()[" + std::to_string(bodyTextNodeCount) + "]." + std::to_string(charOff); } else { + // Cursor is inside a nested element. Emit the element path without a + // text-point suffix — KOReader will treat this as a position at the + // start of the named element, which is good enough for paragraph-level + // accuracy. Don't emit a deep text() index here: see header comment. result = currentXPath(spineIndex); } found = true; @@ -102,7 +108,9 @@ struct ForwardState : StackState { } totalTextBytes += visible; - if (!codepointsInTextNodeStack.empty()) codepointsInTextNodeStack.back() += cpCount; + if (atBodyLevel) { + codepointsInBodyTextNode += countUtf8Codepoints(text, len); + } } }; diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index d1696642..80988b3f 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -43,6 +43,60 @@ bool resolveFromPercentage(const std::shared_ptr& epub, const float percen return true; } + +// Compute intra-spine progress from KOReader's book percentage, assuming the target +// spine is known. This is the constrained version of resolveFromPercentage that +// honors an XPath-derived spine index even when the heavy XPath resolver couldn't +// run (typically because heap was too fragmented to inflate the chapter at sync time). +// +// The math is identical to the per-spine portion of resolveFromPercentage. Returns 0 +// when the percentage maps to bytes before the spine's start (the position lives +// inside the spine by assumption, so clamp to 0) and 1 when it overshoots the end. +float intraSpineFromPercentage(const std::shared_ptr& epub, const int spineIndex, const float percentage) { + if (!epub || spineIndex < 0 || spineIndex >= epub->getSpineItemsCount() || !std::isfinite(percentage)) { + return 0.0f; + } + const size_t bookSize = epub->getBookSize(); + if (bookSize == 0) { + return 0.0f; + } + const float sanitized = std::clamp(percentage, 0.0f, 1.0f); + const size_t targetBytes = static_cast(bookSize * sanitized); + const size_t prevCumSize = (spineIndex > 0) ? epub->getCumulativeSpineItemSize(spineIndex - 1) : 0; + const size_t currentCumSize = epub->getCumulativeSpineItemSize(spineIndex); + const size_t spineSize = currentCumSize - prevCumSize; + if (spineSize == 0) { + return 0.0f; + } + if (targetBytes <= prevCumSize) { + return 0.0f; + } + const size_t bytesIntoSpine = targetBytes - prevCumSize; + return std::clamp(static_cast(bytesIntoSpine) / static_cast(spineSize), 0.0f, 1.0f); +} + +// KOReader emits chapter-start XPaths as ".../body/.0" or just +// ".../body/text()[1].0" — there's no paragraph segment, and the character offset is 0. +// These unambiguously denote "the start of the spine"; we can pin intra=0 without +// inflating the chapter. Catches the common case of starting a new chapter on +// another device, which previously round-tripped through book-percentage byte math +// and landed several pages into the chapter due to byte-vs-page-density skew. +bool isChapterStartXPath(const std::string& xpath) { + // Reject anything with a paragraph or list-item predicate — those carry real + // position information that can't be flattened to "start of spine". + if (xpath.find("/p[") != std::string::npos) return false; + if (xpath.find("/li[") != std::string::npos) return false; + // The path must end with a ".0" text-point segment. The reverse mapper already + // strips text() suffixes for matching, but here we look at the raw form: either + // ".0" (cursor at start of element) or "text()[1].0" / similar (cursor at + // start of the first text node) with no following character offset. + const size_t dotPos = xpath.rfind('.'); + if (dotPos == std::string::npos || dotPos + 1 >= xpath.size()) return false; + for (size_t i = dotPos + 1; i < xpath.size(); i++) { + if (xpath[i] != '0') return false; + } + return true; +} } // namespace KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr& epub, const CrossPointPosition& pos) { @@ -101,9 +155,14 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu bool usedXPathMapping = false; bool usedPercentageReconcile = false; + // Mapping source used for the final log line; updated as we narrow down the path + // actually taken (xpath / xpath+percentage / xpath-spine+percentage / percentage). + const char* mappingSource = "percentage"; + int xpathSpineIndex = -1; - if (ChapterXPathIndexer::tryExtractSpineIndexFromXPath(koPos.xpath, xpathSpineIndex) && xpathSpineIndex >= 0 && - xpathSpineIndex < spineCount) { + const bool haveXPathSpine = ChapterXPathIndexer::tryExtractSpineIndexFromXPath(koPos.xpath, xpathSpineIndex) && + xpathSpineIndex >= 0 && xpathSpineIndex < spineCount; + if (haveXPathSpine) { float intraFromXPath = 0.0f; uint16_t liIndexFromXPath = 0; if (ChapterXPathIndexer::findProgressForXPath(epub, xpathSpineIndex, koPos.xpath, intraFromXPath, xpathExactMatch, @@ -139,8 +198,12 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu } } } + mappingSource = usedPercentageReconcile ? "xpath+percentage" : "xpath"; } - // Extract paragraph index from XPath for direct page lookup via section cache + // Extract paragraph index from XPath for direct page lookup via section cache. + // Done regardless of whether the heavy XPath resolver succeeded — the paragraph + // LUT lookup later (in EpubReaderActivity::NavigationTarget::resolveInto) snaps + // to the precise page, so even without intra resolution we get an exact landing. uint16_t pIndex = 0; if (ChapterXPathIndexer::tryExtractParagraphIndexFromXPath(koPos.xpath, pIndex)) { result.paragraphIndex = pIndex; @@ -149,14 +212,39 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu } if (!usedXPathMapping) { - int percentageSpineIndex = -1; - float percentageIntraSpine = -1.0f; - if (!resolveFromPercentage(epub, koPos.percentage, spineCount, percentageSpineIndex, percentageIntraSpine)) { - return result; + // Heavy XPath resolution failed (typically because heap was too fragmented to + // inflate the spine at sync time). Salvage as much as we can: + // 1) Trust the spine index extracted from the XPath itself — it's purely + // string-derived and always correct when present. Using it preserves + // cross-chapter syncs even when chapter content can't be re-parsed. + // 2) For chapter-start XPaths (ending in ".0" with no paragraph predicate), + // pin intra=0. KOReader's percentage carries small per-DOM rounding that + // would otherwise leak into a spurious intra > 0 via byte-fraction math. + // 3) Otherwise compute intra-spine from KOReader's percentage relative to + // the XPath-derived spine. Falls back to global percentage spine selection + // only when no XPath spine is available. + if (haveXPathSpine) { + result.spineIndex = xpathSpineIndex; + if (isChapterStartXPath(koPos.xpath)) { + resolvedIntraSpineProgress = 0.0f; + mappingSource = "xpath-spine+chapter-start"; + LOG_DBG("ProgressMapper", "Chapter-start XPath '%s' on spine=%d, pinning intra=0", koPos.xpath.c_str(), + xpathSpineIndex); + } else { + resolvedIntraSpineProgress = intraSpineFromPercentage(epub, xpathSpineIndex, koPos.percentage); + mappingSource = "xpath-spine+percentage"; + LOG_DBG("ProgressMapper", "XPath resolve unavailable for spine=%d; intra from pct=%.3f -> %.3f", + xpathSpineIndex, koPos.percentage, resolvedIntraSpineProgress); + } + } else { + int percentageSpineIndex = -1; + float percentageIntraSpine = -1.0f; + if (!resolveFromPercentage(epub, koPos.percentage, spineCount, percentageSpineIndex, percentageIntraSpine)) { + return result; + } + result.spineIndex = percentageSpineIndex; + resolvedIntraSpineProgress = percentageIntraSpine; } - - result.spineIndex = percentageSpineIndex; - resolvedIntraSpineProgress = percentageIntraSpine; } // Estimate page number within the selected spine item @@ -207,8 +295,6 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu result.spineIndex, resolvedIntraSpineProgress, result.hasParagraphIndex ? "yes" : "no", result.paragraphIndex, result.hasListItemIndex ? "yes" : "no", result.listItemIndex); - const char* mappingSource = - usedXPathMapping ? (usedPercentageReconcile ? "xpath+percentage" : "xpath") : "percentage"; LOG_DBG("ProgressMapper", "KOReader -> CrossPoint: %.2f%% at %s -> spine=%d, page=%d (%s, exact=%s)", koPos.percentage * 100, koPos.xpath.c_str(), result.spineIndex, result.pageNumber, mappingSource, xpathExactMatch ? "yes" : "no"); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index b6afdc34..bb69366f 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -104,6 +104,30 @@ void logReaderMemSnapshot(const char* stage) { inline void logReaderMemSnapshot(const char*) {} #endif +// Integrity bisector. Logs at every probe site (unconditional, not gated) and +// fires an ERR when integrity transitions from ok -> fail so we can pinpoint +// which render phase corrupts the heap. Free/contig included so we can see if +// the corruption coincides with a specific allocation pattern. Calling +// heap_caps_check_integrity_all is ~O(blocks) — not free but fine at phase +// boundaries during onEnter / first render. +void logIntegrityProbe(const char* stage) { + static bool sLastOk = true; + const bool ok = heap_caps_check_integrity_all(true); + 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); + if (ok != sLastOk) { + if (ok) { + LOG_DBG("INTG", "[%s] integrity recovered (free=%lu contig=%lu)", stage, freeHeap, contigHeap); + } else { + LOG_ERR("INTG", "[%s] integrity FAIL — corruption introduced here (free=%lu contig=%lu)", stage, freeHeap, + contigHeap); + } + sLastOk = ok; + } else { + LOG_DBG("INTG", "[%s] %s free=%lu contig=%lu", stage, ok ? "ok" : "fail", freeHeap, contigHeap); + } +} + // Tiled grayscale: render each plane band-by-band into a small scratch and // stream straight to the controller, leaving the BW framebuffer intact so no // storeBwBuffer / restoreBwBuffer is needed. Controller RAM is re-synced from @@ -156,15 +180,20 @@ bool runTiledGrayscalePass(GfxRenderer& renderer, const Page& page, int fontId, } }; + logIntegrityProbe("tiledGray_after_scratchAlloc"); renderPlane(GfxRenderer::GRAYSCALE_LSB, true); + logIntegrityProbe("tiledGray_after_lsbPlane"); renderPlane(GfxRenderer::GRAYSCALE_MSB, false); + logIntegrityProbe("tiledGray_after_msbPlane"); renderer.setRenderMode(GfxRenderer::BW); renderer.displayGrayBuffer(); + logIntegrityProbe("tiledGray_after_displayGrayBuffer"); // BW framebuffer is intact; re-sync controller RAM for the next differential // page turn directly from it. renderer.cleanupGrayscaleWithFrameBuffer(); + logIntegrityProbe("tiledGray_after_cleanup"); return true; } @@ -253,6 +282,7 @@ int getImageOnlyPageYOffset(const Page& page, const int viewportHeight) { void EpubReaderActivity::onEnter() { Activity::onEnter(); logReaderMemSnapshot("onEnter_begin"); + logIntegrityProbe("onEnter_begin"); // Drop any input events that arrived from the activity that launched us (e.g. a wake-up power // button hold) before they reach detectPageTurn() — see ReaderUtils::InputDrainGuard. @@ -272,10 +302,13 @@ void EpubReaderActivity::onEnter() { epub->setupCacheDir(); logReaderMemSnapshot("onEnter_after_setupCacheDir"); - applyPendingSyncSession(); - applyPendingBookmarkJump(); - logReaderMemSnapshot("onEnter_after_pending_sync"); + // Load the persistent baseline (progress.bin) first. Pending session state + // (sync result, bookmark jump) is then overlaid on top — this is the only order + // that lets a Kind::Paragraph / Kind::ListItem navTarget set by applyPendingSyncSession + // survive into render(). The previous order (apply then load) clobbered the LUT + // target with Kind::Page from progress.bin, which is why XPath-precision sync + // silently degraded to the rough page estimate. FsFile f; if (Storage.openFileForRead("ERS", epub->getCachePath() + "/progress.bin", f)) { uint8_t data[6]; @@ -300,6 +333,10 @@ void EpubReaderActivity::onEnter() { navTarget = NavigationTarget::makePage(0); } + applyPendingSyncSession(); + applyPendingBookmarkJump(); + logReaderMemSnapshot("onEnter_after_pending_sync"); + if (currentSpineIndex == 0) { int textSpineIndex = epub->getSpineIndexForTextReference(); if (textSpineIndex != 0) { @@ -1185,7 +1222,9 @@ void EpubReaderActivity::applyPendingSyncSession() { restorePage = 0; } - // Build the navigation target from the sync result. + // Build the navigation target from the sync result. For LUT-anchored targets the + // estimated restorePage is plumbed through as fallbackPage so a LUT miss in the + // target spine still lands the user on a sensible page rather than page 0. NavigationTarget restoreTarget; if (sync.outcome == KOReaderSyncOutcomeState::APPLIED_REMOTE) { const int spineCount = epub->getSpineItemsCount(); @@ -1198,11 +1237,11 @@ void EpubReaderActivity::applyPendingSyncSession() { restorePage = sync.resultPage; } if (sync.resultHasListItemIndex) { - restoreTarget = NavigationTarget::makeListItem(sync.resultListItemIndex); + restoreTarget = NavigationTarget::makeListItem(sync.resultListItemIndex, restorePage); 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); + restoreTarget = NavigationTarget::makeParagraph(sync.resultParagraphIndex, restorePage); LOG_DBG("ERS", "Applied synced remote position: spine=%d page=%d p[%u]", restoreSpineIndex, restorePage, sync.resultParagraphIndex); } else { @@ -1216,21 +1255,26 @@ void EpubReaderActivity::applyPendingSyncSession() { // 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 - // rescaling. Store 0 to disable rescaling; the LUT lookup handles precise positioning. + // rescaling the fallbackPage estimate (which was estimated from cross-spine + // density anyway). Store 0 to disable rescaling — the LUT lookup is the precise + // path, and the cross-spine fallback can't usefully be rescaled here. 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)) { - navTarget = restoreTarget; + // Seed live state directly — the previous write-then-reload-from-disk pattern relied + // on progress.bin being read after this function ran, which clobbered the LUT target. + // Live-state seeding is authoritative; the persistent write below is just for crash + // recovery so a power loss before the next saveProgress() doesn't lose the synced + // spine/page. The next render's saveProgress() supplies the real percent before + // the user can return to the home screen. + currentSpineIndex = restoreSpineIndex; + navTarget = restoreTarget; + if (!writeReaderProgressCache(epub->getCachePath(), restoreSpineIndex, restorePage, restorePageCount, 0)) { + LOG_ERR("ERS", "Failed to persist sync restore to progress.bin; live state still seeded"); + } else { 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; - navTarget = restoreTarget; } sync.clear(); @@ -1249,14 +1293,13 @@ void EpubReaderActivity::applyPendingBookmarkJump() { jump.spineIndex = 0; jump.pageNumber = 0; } - // Transient write before initializeReader; saveProgress() overwrites with the real percent. - if (writeReaderProgressCache(epub->getCachePath(), jump.spineIndex, jump.pageNumber, 0, 0)) { - navTarget = NavigationTarget::makePage(jump.pageNumber); - navTarget.cachedSpineIdx = jump.spineIndex; - } else { - currentSpineIndex = jump.spineIndex; - navTarget = NavigationTarget::makePage(jump.pageNumber); - navTarget.cachedSpineIdx = jump.spineIndex; + // Seed live state directly; the persistent write is for crash recovery only. + // saveProgress() on the next render overwrites with the real percent. + currentSpineIndex = jump.spineIndex; + navTarget = NavigationTarget::makePage(jump.pageNumber); + navTarget.cachedSpineIdx = jump.spineIndex; + if (!writeReaderProgressCache(epub->getCachePath(), jump.spineIndex, jump.pageNumber, 0, 0)) { + LOG_ERR("ERS", "Failed to persist bookmark jump to progress.bin; live state still seeded"); } jump.clear(); APP_STATE.saveToFile(); @@ -1467,58 +1510,95 @@ int EpubReaderActivity::getEffectiveReaderFontId() const { } 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()); + // Resolve to a baseline page first. Each branch records whether it produced a + // precise page (LUT/anchor hit, percent jump, explicit page) or only an estimate. + // The estimate path runs cross-spine rescale + clamp at the end; the precise path + // skips both because LUT pages are already in the target spine's coordinate system. + bool isEstimate = false; + + switch (kind) { + case Kind::LastPage: { + sec.currentPage = (sec.pageCount > 0) ? sec.pageCount - 1 : 0; + break; } - 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); + + case Kind::TocIndex: { + if (const auto p = sec.getPageForTocIndex(tocIndex)) { + sec.currentPage = *p; + } + break; } - 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); + + case 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; using fallback page %d", anchorStr.c_str(), fallbackPage); + sec.currentPage = fallbackPage; + isEstimate = true; + } + break; } - 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; + + case Kind::ListItem: { + if (const auto p = sec.getPageForListItemIndex(lutIndex)) { + sec.currentPage = *p; + LOG_DBG("ERS", "Resolved li[%u] -> page %d", lutIndex, *p); + } else if (const auto pp = sec.getPageForParagraphIndex(lutIndex)) { + // Some

  • -anchored XPaths land in books where the LI LUT is empty (no
  • + // inside 's direct children, or all
  • s skipped). Fall back to the + // paragraph LUT — the running indices coincide often enough to help, and + // it's strictly better than dropping back to the estimate. + sec.currentPage = *pp; + LOG_DBG("ERS", "Li LUT miss for li[%u]; paragraph LUT -> page %d", lutIndex, *pp); + } else { + LOG_DBG("ERS", "Li[%u] not in LUT; using fallback page %d", lutIndex, fallbackPage); + sec.currentPage = fallbackPage; + isEstimate = true; + } + break; } - 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)); + + case 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 for p[%u]; using fallback page %d", lutIndex, fallbackPage); + sec.currentPage = fallbackPage; + isEstimate = true; + } + break; + } + + case 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; + } + break; + } + + case Kind::Page: { + sec.currentPage = page; + isEstimate = true; + break; } } - // Safety clamp. + + // Cross-font / cross-spine rescaling: only for estimated pages. cachedPageCount + // is the page count at the time the estimate was made — when it disagrees with + // the section's current page count (reflow / different spine entirely), rescale + // the estimate proportionally before clamping. + if (isEstimate && cachedPageCount > 0 && cachedSpineIdx == spineIndex && sec.pageCount != cachedPageCount) { + const float progress = static_cast(sec.currentPage) / static_cast(cachedPageCount); + sec.currentPage = static_cast(progress * static_cast(sec.pageCount)); + } + + // Safety clamp for all paths — a LUT-derived page is also defensively clamped in + // case the cache is somehow stale. if (sec.currentPage < 0) { LOG_DBG("ERS", "Clamping negative page %d to 0 (spine=%d cachedPageCount=%d)", sec.currentPage, spineIndex, cachedPageCount); @@ -1600,6 +1680,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { if (!epub) { return; } + logIntegrityProbe("render_entry"); const int spineCount = epub->getSpineItemsCount(); if (spineCount <= 0) { @@ -1744,11 +1825,13 @@ void EpubReaderActivity::render(RenderLock&& lock) { auto p = section->loadPageFromSectionFile(); section->currentPage = savedPage; if (p && !p->hasImages()) { + logIntegrityProbe("preRender_before_renderPageContentOnly"); section->currentPage = nextPage; renderPageContentOnly(*p, orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft); section->currentPage = savedPage; preRenderedPage = {true, currentSpineIndex, nextPage}; LOG_DBG("ERS", "Pre-rendered page %d/%d", nextPage, section->pageCount - 1); + logIntegrityProbe("preRender_after_renderPageContentOnly"); } } } @@ -1831,6 +1914,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { LOG_DBG("ERS", "Cache found, skipping build..."); } lastRenderStats.sectionLoadMs = millis() - sectionStart; + logIntegrityProbe("render_after_sectionLoad"); if (section->isTruncatedCache() && currentSpineIndex != lastWarnedTruncatedSpineIndex) { lastWarnedTruncatedSpineIndex = currentSpineIndex; @@ -1868,6 +1952,7 @@ void EpubReaderActivity::render(RenderLock&& lock) { const unsigned long pageLoadStart = millis(); auto p = section->loadPageFromSectionFile(); lastRenderStats.pageLoadMs = millis() - pageLoadStart; + logIntegrityProbe("render_after_pageLoad"); if (!p) { LOG_ERR("ERS", "Failed to load page from SD - clearing section cache"); section->clearCache(); @@ -1894,8 +1979,10 @@ void EpubReaderActivity::render(RenderLock&& lock) { truncatedSectionHintRendersRemaining--; } LOG_DBG("ERS", "Rendered page in %dms", lastRenderStats.requestRenderMs); + logIntegrityProbe("render_after_renderContents"); } silentIndexNextChapterIfNeeded(viewportWidth, viewportHeight); + logIntegrityProbe("render_after_silentIndex"); pendingProgressSave.spineIndex = currentSpineIndex; pendingProgressSave.page = section->currentPage; pendingProgressSave.pageCount = section->pageCount; @@ -1975,6 +2062,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or const int orientedMarginLeft) { const auto t0 = millis(); logReaderMemSnapshot("render_start"); + logIntegrityProbe("renderContents_entry"); auto* fcm = renderer.getFontCacheManager(); fcm->resetStats(); @@ -1990,6 +2078,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or const bool warmForceLoad = forceLoadLargeImages || !SETTINGS.largeImagePlaceholder; page->warmImageCaches(renderer, orientedMarginLeft, contentTop, warmForceLoad); renderer.clearScreen(); + logIntegrityProbe("renderContents_after_warmImages"); logReaderMemSnapshot("prewarm_begin"); @@ -2009,6 +2098,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or LOG_DBG("ERS", "Heap: before=%lu (contig=%lu) after=%lu (contig=%lu) delta=%ld", heapBefore, contigBefore, heapAfter, contigAfter, (int32_t)heapAfter - (int32_t)heapBefore); logReaderMemSnapshot("prewarm_end"); + logIntegrityProbe("renderContents_after_fontPrewarm"); const bool aaConfigured = SETTINGS.textAntiAliasing; bool aaEnabledForThisRender = aaConfigured; @@ -2060,6 +2150,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or fcm->logStats("bw_render"); const auto tBwRender = millis(); logReaderMemSnapshot("after_bw_render"); + logIntegrityProbe("renderContents_after_bwRender"); if (imagePageWithAA) { // Double FAST_REFRESH with selective image blanking (pablohc's technique): @@ -2107,6 +2198,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or uint32_t tiledGrayMs = 0; if (aaEnabledForThisRender) { logReaderMemSnapshot("tiled_gray_begin"); + logIntegrityProbe("renderContents_before_tiledGray"); const auto tTiledBegin = millis(); grayscaleDone = runTiledGrayscalePass(renderer, *page, getEffectiveReaderFontId(), orientedMarginLeft, contentTop, SETTINGS.fastAntiAliasing); @@ -2114,6 +2206,7 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or tiledGrayMs = millis() - tTiledBegin; fcm->logStats("tiled_gray"); logReaderMemSnapshot("tiled_gray_end"); + logIntegrityProbe("renderContents_after_tiledGray"); } } diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 6bcafb1d..d69e9932 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -46,9 +46,15 @@ class EpubReaderActivity final : public Activity { }; 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. + // Non-zero for Kind::Page when loaded from progress.bin or written during reflow. + // Also set for Kind::Paragraph / Kind::ListItem / Kind::Anchor so a LUT miss + // still rescales the estimated fallbackPage instead of stranding at 0. int cachedPageCount = 0; int cachedSpineIdx = 0; + // Estimated page used as a baseline before LUT/anchor lookup, and as a fallback + // when the lookup misses. Only meaningful for Kind::Paragraph / Kind::ListItem / + // Kind::Anchor — for Kind::Page the `page` field is the baseline. + int fallbackPage = 0; NavigationTarget() : kind(Kind::Page), page(0) {} @@ -64,11 +70,12 @@ class EpubReaderActivity final : public Activity { t.page = 0; return t; } - static NavigationTarget makeAnchor(std::string a) { + static NavigationTarget makeAnchor(std::string a, int fallback = 0) { NavigationTarget t; t.kind = Kind::Anchor; t.page = 0; t.anchorStr = std::move(a); + t.fallbackPage = fallback; return t; } static NavigationTarget makeTocIndex(int idx) { @@ -83,16 +90,18 @@ class EpubReaderActivity final : public Activity { t.spineProgress = sp; return t; } - static NavigationTarget makeParagraph(uint16_t i) { + static NavigationTarget makeParagraph(uint16_t i, int fallback = 0) { NavigationTarget t; t.kind = Kind::Paragraph; t.lutIndex = i; + t.fallbackPage = fallback; return t; } - static NavigationTarget makeListItem(uint16_t i) { + static NavigationTarget makeListItem(uint16_t i, int fallback = 0) { NavigationTarget t; t.kind = Kind::ListItem; t.lutIndex = i; + t.fallbackPage = fallback; return t; } diff --git a/src/activities/reader/KOReaderSyncActivity.cpp b/src/activities/reader/KOReaderSyncActivity.cpp index 8f3d439b..c23efded 100644 --- a/src/activities/reader/KOReaderSyncActivity.cpp +++ b/src/activities/reader/KOReaderSyncActivity.cpp @@ -163,8 +163,10 @@ void KOReaderSyncActivity::performFetchAndCompare() { // avoid a second TLS handshake under fragmented heap. KOReaderSyncClient::beginPersistentSession(); + logSyncMemSnapshot("before_getProgress"); // Fetch remote progress const auto result = KOReaderSyncClient::getProgress(documentHash, remoteProgress); + logSyncMemSnapshot("after_getProgress"); if (result == KOReaderSyncClient::NOT_FOUND) { if (syncIntent == KOReaderSyncIntentState::PULL_REMOTE) { @@ -278,7 +280,16 @@ void KOReaderSyncActivity::performFetchAndCompare() { // still useful for manual conflict decisions. // Pre-map remote progress now so compare UI always shows concrete chapter/ // page data. The mapped result is cached and reused if Apply is chosen. - if (!ensureRemotePositionMapped(false)) { + // closeSessionBeforeMapping=true tears down the warmed TLS session before + // reverse XPath mapping so the 32 KB inflate ring buffer can allocate. + // Trade-off: if the user later picks Upload, we eat one extra TLS handshake + // (~1.7s). That's the less-common choice — Apply is what users usually want — + // and silent inflate failures here previously caused syncs to land on the + // wrong page. See logSyncMemSnapshot("after_getProgress") for the heap drop + // a held-open session causes (~36 KB contig consumed by esp_http_client + // state and response buffer that aren't released until cleanup). + logSyncMemSnapshot("before_compare_map"); + if (!ensureRemotePositionMapped(true)) { { RenderLock lock(*this); state = SYNC_FAILED; @@ -704,11 +715,19 @@ bool KOReaderSyncActivity::ensureRemotePositionMapped(const bool closeSessionBef return true; } + // Diagnostic snapshots around each phase of remote->local mapping. The reverse + // XPath mapper needs a 32 KB contiguous block for the inflate ring buffer; if + // that allocation fails we silently degrade to percentage-only mapping and + // round-trip accuracy suffers. Snapshots here let us see exactly which phase + // fragments the heap so the fix can target the actual culprit. + logSyncMemSnapshot("ensureRemoteMap_entry"); + // Mapping remote->local can trigger EPUB inflate work. For apply/pull paths, // release HTTP/TLS first to maximize heap headroom. Compare pre-map keeps // the warmed session alive so Upload can reuse it without a fresh handshake. if (closeSessionBeforeMapping) { KOReaderSyncClient::endPersistentSession(); + logSyncMemSnapshot("ensureRemoteMap_after_endSession"); } { @@ -716,12 +735,17 @@ bool KOReaderSyncActivity::ensureRemotePositionMapped(const bool closeSessionBef statusMessage = tr(STR_MAPPING_REMOTE); } requestUpdateAndWait(); + logSyncMemSnapshot("ensureRemoteMap_after_statusUpdate"); KOReaderPosition koPos = {remoteProgress.progress, remoteProgress.percentage}; if (!ensureEpubLoadedForMapping()) { return false; } + logSyncMemSnapshot("ensureRemoteMap_after_epubLoad"); + remotePosition = ProgressMapper::toCrossPoint(epub, koPos, currentSpineIndex, totalPagesInSpine); + logSyncMemSnapshot("ensureRemoteMap_after_toCrossPoint"); + computeRemoteChapter(); releaseEpubForMapping(); hasRemoteProgress = true; diff --git a/src/main.cpp b/src/main.cpp index eb550138..2d5a975e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -153,6 +154,33 @@ enum class BootResume : uint8_t { // startDeepSleep() does not return, so a set latch only ends at the wakeup reset. static bool deepSleepInProgress = false; +// Heap-integrity probe. Scoped to MALLOC_CAP_8BIT|MALLOC_CAP_DEFAULT so we +// only inspect the user-app heap, not ROM/BLE/WiFi reserved DRAM regions that +// `heap_caps_check_integrity_all` would also walk (those report spurious +// canary mismatches because the user-heap allocator never stamped canaries +// there — the address 0x3fcdc710 we kept seeing FAIL on is outside our +// dram0_0_seg, in a system-reserved area). A fail here genuinely means user +// code overwrote a heap canary. Transition is loud (ERR), steady-state is DBG. +void runHeapIntegrityProbe(const char* stage) { + const bool integrityOk = heap_caps_check_integrity(MALLOC_CAP_8BIT | MALLOC_CAP_DEFAULT, true); + static bool lastIntegrityOk = true; + static bool firstIntegrityProbe = true; + 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); + if (firstIntegrityProbe || integrityOk != lastIntegrityOk) { + if (integrityOk) { + LOG_INF("MEM", "[%s] integrity ok (uptime %lu ms, free=%lu contig=%lu)", stage, millis(), freeHeap, contigHeap); + } else { + LOG_ERR("MEM", "[%s] integrity FAIL (uptime %lu ms, free=%lu contig=%lu) — corruption introduced here", stage, + millis(), freeHeap, contigHeap); + } + lastIntegrityOk = integrityOk; + firstIntegrityProbe = false; + } else { + LOG_DBG("MEM", "[%s] integrity %s (free=%lu contig=%lu)", stage, integrityOk ? "ok" : "fail", freeHeap, contigHeap); + } +} + void silentRestart() { if (deepSleepInProgress) return; // sleeping supersedes the heap-defrag reboot // ESP.restart() bypasses activity onExit(), so flush any in-flight reading @@ -347,6 +375,7 @@ void ensureSdFontLoadedForPath(const char* path) { } void setup() { + runHeapIntegrityProbe("setup_entry"); { esp_ota_img_states_t otaState; const esp_partition_t* running = esp_ota_get_running_partition(); @@ -354,6 +383,7 @@ void setup() { esp_ota_mark_app_valid_cancel_rollback(); } } + runHeapIntegrityProbe("setup_after_otaCheck"); // Read-and-clear so a panic later in setup() doesn't loop into silent reboot. // Bound the target range too — RTC_NOINIT memory is uninitialized on cold boot. @@ -363,13 +393,20 @@ void setup() { silentRebootMagic = 0; silentRebootTarget = 0; + runHeapIntegrityProbe("setup_before_HalSystem_begin"); HalSystem::begin(); + runHeapIntegrityProbe("setup_after_HalSystem_begin"); gpio.begin(); + runHeapIntegrityProbe("setup_after_gpio_begin"); powerManager.begin(); + runHeapIntegrityProbe("setup_after_powerManager_begin"); halTiltSensor.begin(); + runHeapIntegrityProbe("setup_after_halTiltSensor_begin"); gpio_deep_sleep_hold_dis(); // Release deep sleep GPIO hold state from previous sleep cycle + runHeapIntegrityProbe("setup_after_deepSleepHoldDis"); const auto wakeupReason = gpio.getWakeupReason(); + runHeapIntegrityProbe("setup_after_getWakeupReason"); if (wakeupReason == HalGPIO::WakeupReason::AfterUSBPower) { // If USB power caused a cold boot, go back to sleep immediately without initializing subsystems @@ -392,6 +429,7 @@ void setup() { LOG_INF("MAIN", "Hardware detect: %s", gpio.deviceIsX3() ? "X3" : "X4"); LOG_DBG("MAIN", "Wakeup reason: %d, millis=%lu, rawPowerPin=%d", static_cast(wakeupReason), millis(), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW); + runHeapIntegrityProbe("setup_after_hwInit"); // Load just the settings we need *before* initializing the SD card to speed up and reduce power on unverified wakes SETTINGS.loadStartupFromNvs(); @@ -469,7 +507,9 @@ void setup() { : !APP_STATE.showBootScreen ? BootResume::QuickResume : BootResume::Splash; + runHeapIntegrityProbe("setup_before_displayAndFonts"); setupDisplayAndFonts(resume != BootResume::Splash); + runHeapIntegrityProbe("setup_after_displayAndFonts"); switch (resume) { case BootResume::Silent: @@ -500,10 +540,12 @@ void setup() { break; } + runHeapIntegrityProbe("setup_after_initialPaint"); HalClock::restore(); RECENT_BOOKS.loadFromFile(); GLOBAL_BOOKMARKS.load(); READING_STATS.loadFromFile(); + runHeapIntegrityProbe("setup_after_userStoresLoaded"); if (recoveryFirmwareMode) { // Skip normal home/reader routing: jump straight into the SD firmware picker. @@ -563,6 +605,7 @@ void loop() { if (Serial && millis() - lastMemPrint >= 10000) { LOG_INF("MEM", "Free: %d bytes, Total: %d bytes, Min Free: %d bytes, MaxAlloc: %d bytes", ESP.getFreeHeap(), ESP.getHeapSize(), ESP.getMinFreeHeap(), ESP.getMaxAllocHeap()); + runHeapIntegrityProbe("MEM_periodic"); lastMemPrint = millis(); }