From 7e8bd70f3c710c8c76a4709f164026bcac27f7f8 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 25 Feb 2026 22:34:11 +0100 Subject: [PATCH 1/9] First attempt for proper koreader xpath generation / resolution --- lib/KOReaderSync/ChapterXPathIndexer.cpp | 462 +++++++++++++++++++++++ lib/KOReaderSync/ChapterXPathIndexer.h | 43 +++ lib/KOReaderSync/ProgressMapper.cpp | 100 +++-- lib/KOReaderSync/ProgressMapper.h | 9 +- 4 files changed, 578 insertions(+), 36 deletions(-) create mode 100644 lib/KOReaderSync/ChapterXPathIndexer.cpp create mode 100644 lib/KOReaderSync/ChapterXPathIndexer.h diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp new file mode 100644 index 00000000..478a5e6e --- /dev/null +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -0,0 +1,462 @@ +#include "ChapterXPathIndexer.h" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace { + +struct XPathAnchor { + size_t textOffset = 0; + std::string xpath; +}; + +struct StackNode { + std::string tag; + int index = 1; + bool hasTextAnchor = false; +}; + +struct ParserState { + explicit ParserState(const int spineIndex) : spineIndex(spineIndex) { siblingCounters.emplace_back(); } + + int spineIndex = 0; + int skipDepth = -1; + size_t totalTextBytes = 0; + + std::vector stack; + std::vector> siblingCounters; + std::vector anchors; + + std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; } + + static std::string normalizeXPath(const std::string& input) { + if (input.empty()) { + return ""; + } + + std::string out; + out.reserve(input.size()); + for (char c : input) { + const unsigned char uc = static_cast(c); + if (std::isspace(uc)) { + continue; + } + out.push_back(static_cast(std::tolower(uc))); + } + + const std::string textSuffix = "/text()"; + size_t textPos = out.find(textSuffix); + if (textPos != std::string::npos) { + out.erase(textPos); + } + + while (!out.empty() && out.back() == '/') { + out.pop_back(); + } + + return out; + } + + static std::string removeIndices(const std::string& xpath) { + std::string out; + out.reserve(xpath.size()); + + bool inBracket = false; + for (char c : xpath) { + if (c == '[') { + inBracket = true; + continue; + } + if (c == ']') { + inBracket = false; + continue; + } + if (!inBracket) { + out.push_back(c); + } + } + return out; + } + + static int pathDepth(const std::string& xpath) { + int depth = 0; + for (char c : xpath) { + if (c == '/') { + depth++; + } + } + return depth; + } + + bool pickBestAnchorByPath(const std::string& targetPath, const bool ignoreIndices, size_t& outTextOffset, + bool& outExact) const { + if (targetPath.empty() || anchors.empty()) { + return false; + } + + const std::string normalizedTarget = ignoreIndices ? removeIndices(targetPath) : targetPath; + std::string probe = normalizedTarget; + bool exactProbe = true; + + while (!probe.empty()) { + int bestDepth = -1; + size_t bestOffset = 0; + bool found = false; + + for (const auto& anchor : anchors) { + const std::string anchorPath = ignoreIndices ? removeIndices(anchor.xpath) : anchor.xpath; + if (anchorPath == probe) { + const int depth = pathDepth(anchorPath); + if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset > bestOffset)) { + found = true; + bestDepth = depth; + bestOffset = anchor.textOffset; + } + } + } + + if (found) { + outTextOffset = bestOffset; + outExact = exactProbe; + return true; + } + + const size_t lastSlash = probe.find_last_of('/'); + if (lastSlash == std::string::npos || lastSlash == 0) { + break; + } + probe.erase(lastSlash); + exactProbe = false; + } + + return false; + } + + static std::string toLower(std::string value) { + for (char& c : value) { + c = static_cast(std::tolower(static_cast(c))); + } + return value; + } + + static bool isSkippableTag(const std::string& tag) { return tag == "head" || tag == "script" || tag == "style"; } + + static bool isWhitespaceOnly(const XML_Char* text, const int len) { + for (int i = 0; i < len; i++) { + if (!std::isspace(static_cast(text[i]))) { + return false; + } + } + return true; + } + + static size_t countVisibleBytes(const XML_Char* text, const int len) { + size_t count = 0; + for (int i = 0; i < len; i++) { + if (!std::isspace(static_cast(text[i]))) { + count++; + } + } + return count; + } + + int bodyDepth() const { + for (int i = static_cast(stack.size()) - 1; i >= 0; i--) { + if (stack[i].tag == "body") { + return i; + } + } + return -1; + } + + bool insideBody() const { return bodyDepth() >= 0; } + + std::string currentXPath() const { + const int bodyIdx = bodyDepth(); + if (bodyIdx < 0) { + return baseXPath(); + } + + std::string xpath = baseXPath(); + for (size_t i = static_cast(bodyIdx + 1); i < stack.size(); i++) { + xpath += "/" + stack[i].tag + "[" + std::to_string(stack[i].index) + "]"; + } + return xpath; + } + + void addAnchorIfNeeded() { + if (!insideBody() || stack.empty()) { + return; + } + + if (!stack.back().hasTextAnchor) { + anchors.push_back({totalTextBytes, currentXPath()}); + stack.back().hasTextAnchor = true; + } else if (anchors.empty() || totalTextBytes - anchors.back().textOffset >= 192) { + const std::string xpath = currentXPath(); + if (anchors.empty() || anchors.back().xpath != xpath) { + anchors.push_back({totalTextBytes, xpath}); + } + } + } + + void onStartElement(const XML_Char* rawName) { + std::string name = toLower(rawName ? rawName : ""); + const size_t depth = stack.size(); + + if (siblingCounters.size() <= depth) { + siblingCounters.resize(depth + 1); + } + const int siblingIndex = ++siblingCounters[depth][name]; + + stack.push_back({name, siblingIndex, false}); + siblingCounters.emplace_back(); + + if (skipDepth < 0 && isSkippableTag(name)) { + skipDepth = static_cast(stack.size()) - 1; + } + } + + void onEndElement() { + if (stack.empty()) { + return; + } + + if (skipDepth == static_cast(stack.size()) - 1) { + skipDepth = -1; + } + + stack.pop_back(); + if (!siblingCounters.empty()) { + siblingCounters.pop_back(); + } + } + + void onCharacterData(const XML_Char* text, const int len) { + if (skipDepth >= 0 || len <= 0 || !insideBody() || isWhitespaceOnly(text, len)) { + return; + } + + addAnchorIfNeeded(); + totalTextBytes += countVisibleBytes(text, len); + } + + std::string chooseXPath(const float intraSpineProgress) const { + if (anchors.empty()) { + return baseXPath(); + } + if (totalTextBytes == 0) { + return anchors.front().xpath; + } + + const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress)); + const size_t target = static_cast(clampedProgress * static_cast(totalTextBytes)); + + auto it = std::lower_bound(anchors.begin(), anchors.end(), target, + [](const XPathAnchor& anchor, const size_t value) { return anchor.textOffset < value; }); + + if (it == anchors.end()) { + return anchors.back().xpath; + } + return it->xpath; + } + + bool chooseProgressForXPath(const std::string& xpath, float& outIntraSpineProgress, bool& outExactMatch) const { + if (anchors.empty()) { + return false; + } + + const std::string normalized = normalizeXPath(xpath); + if (normalized.empty()) { + return false; + } + + size_t matchedOffset = 0; + bool exact = false; + bool matched = pickBestAnchorByPath(normalized, false, matchedOffset, exact); + + if (!matched) { + matched = pickBestAnchorByPath(normalized, true, matchedOffset, exact); + } + + if (!matched) { + return false; + } + + outExactMatch = exact; + if (totalTextBytes == 0) { + outIntraSpineProgress = 0.0f; + return true; + } + + outIntraSpineProgress = static_cast(matchedOffset) / static_cast(totalTextBytes); + outIntraSpineProgress = std::max(0.0f, std::min(1.0f, outIntraSpineProgress)); + return true; + } +}; + +void XMLCALL onStartElement(void* userData, const XML_Char* name, const XML_Char**) { + auto* state = static_cast(userData); + state->onStartElement(name); +} + +void XMLCALL onEndElement(void* userData, const XML_Char*) { + auto* state = static_cast(userData); + state->onEndElement(); +} + +void XMLCALL onCharacterData(void* userData, const XML_Char* text, const int len) { + auto* state = static_cast(userData); + state->onCharacterData(text, len); +} + +void XMLCALL onDefaultHandlerExpand(void* userData, const XML_Char* text, const int len) { + auto* state = static_cast(userData); + state->onCharacterData(text, len); +} + +} // namespace + +std::string ChapterXPathIndexer::findXPathForProgress(const std::shared_ptr& epub, const int spineIndex, + const float intraSpineProgress) { + if (!epub || spineIndex < 0 || spineIndex >= epub->getSpineItemsCount()) { + return ""; + } + + const auto spineItem = epub->getSpineItem(spineIndex); + if (spineItem.href.empty()) { + return ""; + } + + size_t chapterSize = 0; + uint8_t* chapterBytes = epub->readItemContentsToBytes(spineItem.href, &chapterSize, false); + if (!chapterBytes || chapterSize == 0) { + free(chapterBytes); + return ""; + } + + ParserState state(spineIndex); + + XML_Parser parser = XML_ParserCreate(nullptr); + if (!parser) { + free(chapterBytes); + LOG_ERR("KOX", "Failed to allocate XML parser for spine=%d", spineIndex); + return ""; + } + + XML_SetUserData(parser, &state); + XML_SetElementHandler(parser, onStartElement, onEndElement); + XML_SetCharacterDataHandler(parser, onCharacterData); + XML_SetDefaultHandlerExpand(parser, onDefaultHandlerExpand); + + const bool parseOk = XML_Parse(parser, reinterpret_cast(chapterBytes), static_cast(chapterSize), + XML_TRUE) != XML_STATUS_ERROR; + + if (!parseOk) { + LOG_ERR("KOX", "XPath parse failed for spine=%d at line %lu: %s", spineIndex, XML_GetCurrentLineNumber(parser), + XML_ErrorString(XML_GetErrorCode(parser))); + } + + XML_ParserFree(parser); + free(chapterBytes); + + if (!parseOk) { + return ""; + } + + return state.chooseXPath(intraSpineProgress); +} + +bool ChapterXPathIndexer::findProgressForXPath(const std::shared_ptr& epub, const int spineIndex, + const std::string& xpath, float& outIntraSpineProgress, + bool& outExactMatch) { + outIntraSpineProgress = 0.0f; + outExactMatch = false; + + if (!epub || spineIndex < 0 || spineIndex >= epub->getSpineItemsCount() || xpath.empty()) { + return false; + } + + const auto spineItem = epub->getSpineItem(spineIndex); + if (spineItem.href.empty()) { + return false; + } + + size_t chapterSize = 0; + uint8_t* chapterBytes = epub->readItemContentsToBytes(spineItem.href, &chapterSize, false); + if (!chapterBytes || chapterSize == 0) { + free(chapterBytes); + return false; + } + + ParserState state(spineIndex); + XML_Parser parser = XML_ParserCreate(nullptr); + if (!parser) { + free(chapterBytes); + LOG_ERR("KOX", "Failed to allocate XML parser for reverse lookup spine=%d", spineIndex); + return false; + } + + XML_SetUserData(parser, &state); + XML_SetElementHandler(parser, onStartElement, onEndElement); + XML_SetCharacterDataHandler(parser, onCharacterData); + XML_SetDefaultHandlerExpand(parser, onDefaultHandlerExpand); + + const bool parseOk = XML_Parse(parser, reinterpret_cast(chapterBytes), static_cast(chapterSize), + XML_TRUE) != XML_STATUS_ERROR; + + if (!parseOk) { + LOG_ERR("KOX", "Reverse XPath parse failed for spine=%d at line %lu: %s", spineIndex, + XML_GetCurrentLineNumber(parser), XML_ErrorString(XML_GetErrorCode(parser))); + } + + XML_ParserFree(parser); + free(chapterBytes); + + if (!parseOk) { + return false; + } + + return state.chooseProgressForXPath(xpath, outIntraSpineProgress, outExactMatch); +} + +bool ChapterXPathIndexer::tryExtractSpineIndexFromXPath(const std::string& xpath, int& outSpineIndex) { + outSpineIndex = -1; + if (xpath.empty()) { + return false; + } + + const std::string normalized = ParserState::normalizeXPath(xpath); + const std::string key = "/docfragment["; + const size_t pos = normalized.find(key); + if (pos == std::string::npos) { + return false; + } + + const size_t start = pos + key.size(); + size_t end = start; + while (end < normalized.size() && std::isdigit(static_cast(normalized[end]))) { + end++; + } + + if (end == start || end >= normalized.size() || normalized[end] != ']') { + return false; + } + + const std::string value = normalized.substr(start, end - start); + const long parsed = std::strtol(value.c_str(), nullptr, 10); + if (parsed < 0 || parsed > std::numeric_limits::max()) { + return false; + } + + outSpineIndex = static_cast(parsed); + return true; +} diff --git a/lib/KOReaderSync/ChapterXPathIndexer.h b/lib/KOReaderSync/ChapterXPathIndexer.h new file mode 100644 index 00000000..e728b881 --- /dev/null +++ b/lib/KOReaderSync/ChapterXPathIndexer.h @@ -0,0 +1,43 @@ +#pragma once + +#include + +#include +#include + +/** + * Builds element-level XPath anchors for a spine item and picks the best match + * for an intra-spine progress value. + */ +class ChapterXPathIndexer { + public: + /** + * @param epub Loaded EPUB instance + * @param spineIndex Current spine item index + * @param intraSpineProgress Position within the spine item [0.0, 1.0] + * @return Best matching XPath, or empty string on failure + */ + static std::string findXPathForProgress(const std::shared_ptr& epub, int spineIndex, float intraSpineProgress); + + /** + * Resolve a KOReader XPath to an intra-spine progress value. + * + * @param epub Loaded EPUB instance + * @param spineIndex Spine item index to parse + * @param xpath Incoming KOReader XPath + * @param outIntraSpineProgress Resolved position within spine [0.0, 1.0] + * @param outExactMatch True when an exact anchor match was found + * @return true if an exact or ancestor match was resolved + */ + static bool findProgressForXPath(const std::shared_ptr& epub, int spineIndex, const std::string& xpath, + float& outIntraSpineProgress, bool& outExactMatch); + + /** + * Parse the DocFragment index from a KOReader-style XPath. + * + * @param xpath KOReader XPath + * @param outSpineIndex Parsed DocFragment index (0-based) + * @return true when DocFragment[...] is present and valid + */ + static bool tryExtractSpineIndexFromXPath(const std::string& xpath, int& outSpineIndex); +}; diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index ef542ff4..11a6e6e5 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -4,6 +4,9 @@ #include +#include "ChapterXPathIndexer.h" + + KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr& epub, const CrossPointPosition& pos) { KOReaderPosition result; @@ -16,8 +19,13 @@ KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr& epub, c // Calculate overall book progress (0.0-1.0) result.percentage = epub->calculateProgress(pos.spineIndex, intraSpineProgress); - // Generate XPath with estimated paragraph position based on page - result.xpath = generateXPath(pos.spineIndex, pos.pageNumber, pos.totalPages); + // Generate the best available XPath for the current chapter position. + // Prefer element-level XPaths from a lightweight XHTML reparse; fall back + // to a synthetic chapter-level path if parsing fails. + result.xpath = ChapterXPathIndexer::findXPathForProgress(epub, pos.spineIndex, intraSpineProgress); + if (result.xpath.empty()) { + result.xpath = generateXPath(pos.spineIndex, pos.pageNumber, pos.totalPages); + } // Get chapter info for logging const int tocIndex = epub->getTocIndexForSpineIndex(pos.spineIndex); @@ -36,34 +44,64 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu result.pageNumber = 0; result.totalPages = 0; - const size_t bookSize = epub->getBookSize(); - if (bookSize == 0) { + if (!epub || epub->getSpineItemsCount() <= 0) { return result; } - // Use percentage-based lookup for both spine and page positioning - // XPath parsing is unreliable since CrossPoint doesn't preserve detailed HTML structure - const size_t targetBytes = static_cast(bookSize * koPos.percentage); - - // Find the spine item that contains this byte position const int spineCount = epub->getSpineItemsCount(); - bool spineFound = false; - for (int i = 0; i < spineCount; i++) { - const size_t cumulativeSize = epub->getCumulativeSpineItemSize(i); - if (cumulativeSize >= targetBytes) { - result.spineIndex = i; - spineFound = true; - break; + + float resolvedIntraSpineProgress = -1.0f; + bool xpathExactMatch = false; + bool usedXPathMapping = false; + + int xpathSpineIndex = -1; + if (ChapterXPathIndexer::tryExtractSpineIndexFromXPath(koPos.xpath, xpathSpineIndex) && xpathSpineIndex >= 0 && + xpathSpineIndex < spineCount) { + float intraFromXPath = 0.0f; + if (ChapterXPathIndexer::findProgressForXPath(epub, xpathSpineIndex, koPos.xpath, intraFromXPath, + xpathExactMatch)) { + result.spineIndex = xpathSpineIndex; + resolvedIntraSpineProgress = intraFromXPath; + usedXPathMapping = true; } } - // If no spine item was found (e.g., targetBytes beyond last cumulative size), - // default to the last spine item so we map to the end of the book instead of the beginning. - if (!spineFound && spineCount > 0) { - result.spineIndex = spineCount - 1; + if (!usedXPathMapping) { + const size_t bookSize = epub->getBookSize(); + if (bookSize == 0) { + return result; + } + + const size_t targetBytes = static_cast(bookSize * koPos.percentage); + + bool spineFound = false; + for (int i = 0; i < spineCount; i++) { + const size_t cumulativeSize = epub->getCumulativeSpineItemSize(i); + if (cumulativeSize >= targetBytes) { + result.spineIndex = i; + spineFound = true; + break; + } + } + + if (!spineFound && spineCount > 0) { + result.spineIndex = spineCount - 1; + } + + if (result.spineIndex < epub->getSpineItemsCount()) { + const size_t prevCumSize = (result.spineIndex > 0) ? epub->getCumulativeSpineItemSize(result.spineIndex - 1) : 0; + const size_t currentCumSize = epub->getCumulativeSpineItemSize(result.spineIndex); + const size_t spineSize = currentCumSize - prevCumSize; + + if (spineSize > 0) { + const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0; + resolvedIntraSpineProgress = static_cast(bytesIntoSpine) / static_cast(spineSize); + resolvedIntraSpineProgress = std::max(0.0f, std::min(1.0f, resolvedIntraSpineProgress)); + } + } } - // Estimate page number within the spine item using percentage + // Estimate page number within the selected spine item if (result.spineIndex < epub->getSpineItemsCount()) { const size_t prevCumSize = (result.spineIndex > 0) ? epub->getCumulativeSpineItemSize(result.spineIndex - 1) : 0; const size_t currentCumSize = epub->getCumulativeSpineItemSize(result.spineIndex); @@ -91,24 +129,24 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu result.totalPages = estimatedTotalPages; - if (spineSize > 0 && estimatedTotalPages > 0) { - const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0; - const float intraSpineProgress = static_cast(bytesIntoSpine) / static_cast(spineSize); - const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress)); - result.pageNumber = static_cast(clampedProgress * estimatedTotalPages); + if (estimatedTotalPages > 0 && resolvedIntraSpineProgress >= 0.0f) { + const float clampedProgress = std::max(0.0f, std::min(1.0f, resolvedIntraSpineProgress)); + result.pageNumber = static_cast(clampedProgress * static_cast(estimatedTotalPages)); result.pageNumber = std::max(0, std::min(result.pageNumber, estimatedTotalPages - 1)); + } else if (spineSize > 0 && estimatedTotalPages > 0) { + result.pageNumber = 0; } } - LOG_DBG("ProgressMapper", "KOReader -> CrossPoint: %.2f%% at %s -> spine=%d, page=%d", koPos.percentage * 100, - koPos.xpath.c_str(), result.spineIndex, result.pageNumber); + 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, + usedXPathMapping ? "xpath" : "percentage", xpathExactMatch ? "yes" : "no"); return result; } std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) { - // Use 0-based DocFragment indices for KOReader - // Use a simple xpath pointing to the DocFragment - KOReader will use the percentage for fine positioning within it - // Avoid specifying paragraph numbers as they may not exist in the target document + // Fallback path when element-level XPath extraction is unavailable. + // Uses 0-based DocFragment indices for KOReader compatibility. return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; } diff --git a/lib/KOReaderSync/ProgressMapper.h b/lib/KOReaderSync/ProgressMapper.h index 53ff7696..e4e056e4 100644 --- a/lib/KOReaderSync/ProgressMapper.h +++ b/lib/KOReaderSync/ProgressMapper.h @@ -27,9 +27,9 @@ struct KOReaderPosition { * CrossPoint tracks position as (spineIndex, pageNumber). * KOReader uses XPath-like strings + percentage. * - * Since CrossPoint discards HTML structure during parsing, we generate - * synthetic XPath strings based on spine index, using percentage as the - * primary sync mechanism. + * CrossPoint first tries to extract an element-level XPath by reparsing the + * current spine XHTML and mapping intra-spine progress to text anchors. + * If extraction fails, it falls back to a synthetic chapter-level XPath. */ class ProgressMapper { public: @@ -60,8 +60,7 @@ class ProgressMapper { private: /** * Generate XPath for KOReader compatibility. - * Format: /body/DocFragment[spineIndex+1]/body - * Since CrossPoint doesn't preserve HTML structure, we rely on percentage for positioning. + * Fallback format: /body/DocFragment[spineIndex]/body */ static std::string generateXPath(int spineIndex, int pageNumber, int totalPages); }; From c335e027b486cf1e9e4bb5ed51bc019206997d2b Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 25 Feb 2026 22:44:08 +0100 Subject: [PATCH 2/9] Update docs --- docs/contributing/architecture.md | 17 ++++ .../koreader-sync-xpath-mapping.md | 83 +++++++++++++++++++ lib/KOReaderSync/ChapterXPathIndexer.cpp | 23 +++++ lib/KOReaderSync/ChapterXPathIndexer.h | 36 ++++++-- lib/KOReaderSync/ProgressMapper.cpp | 1 - lib/KOReaderSync/ProgressMapper.h | 18 ++-- 6 files changed, 164 insertions(+), 14 deletions(-) create mode 100644 docs/contributing/koreader-sync-xpath-mapping.md diff --git a/docs/contributing/architecture.md b/docs/contributing/architecture.md index f5478115..cab0db95 100644 --- a/docs/contributing/architecture.md +++ b/docs/contributing/architecture.md @@ -125,6 +125,23 @@ Notes: - rendering favors reusing precomputed layout data to keep page turns responsive on constrained hardware - progress/session state is persisted so the reader can reopen at the last position after reboot/sleep +## KOReader sync position mapping + +KOReader sync integration is implemented under `lib/KOReaderSync/` and is used by +`src/activities/reader/KOReaderSyncActivity.*`. + +Position translation currently follows a dual-path strategy: + +- CrossPoint -> KOReader: prefer element-level XPath extracted from the current + spine XHTML; fallback to chapter-level `DocFragment` path when needed. +- KOReader -> CrossPoint: prefer incoming XPath resolution; fallback to + percentage-based estimation if XPath is invalid or cannot be resolved. + +Detailed algorithm and constraints (including low-memory rationale for ESP32-C3) +are documented in: + +- [KOReader Sync XPath Mapping](koreader-sync-xpath-mapping.md) + ## State and persistence Two singletons are central: diff --git a/docs/contributing/koreader-sync-xpath-mapping.md b/docs/contributing/koreader-sync-xpath-mapping.md new file mode 100644 index 00000000..570b2861 --- /dev/null +++ b/docs/contributing/koreader-sync-xpath-mapping.md @@ -0,0 +1,83 @@ +# KOReader Sync XPath Mapping + +This note documents how CrossPoint maps reading positions to and from KOReader sync payloads. + +## Problem + +CrossPoint internally stores position as: + +- `spineIndex` (chapter index) +- `pageNumber` + `totalPages` + +KOReader sync payload stores: + +- `progress` (XPath-like location) +- `percentage` (overall progress) + +A direct 1:1 mapping is not guaranteed because page layout differs between engines/devices. + +## Current Strategy + +### CrossPoint -> KOReader + +Implemented in `ProgressMapper::toKOReader`. + +1. Compute overall `percentage` from chapter/page. +2. Attempt to compute a real element-level XPath via `ChapterXPathIndexer::findXPathForProgress`. +3. If XPath extraction fails, fallback to synthetic chapter path: + - `/body/DocFragment[N]/body` + +### KOReader -> CrossPoint + +Implemented in `ProgressMapper::toCrossPoint`. + +1. Attempt to parse `DocFragment[N]` from incoming XPath. +2. If valid, attempt XPath-to-offset mapping via `ChapterXPathIndexer::findProgressForXPath`. +3. Convert resolved intra-spine progress to page estimate. +4. If XPath path is invalid/unresolvable, fallback to percentage-based chapter/page estimation. + +## ChapterXPathIndexer Design + +The module reparses **one spine XHTML** on demand using Expat and builds temporary anchors: + +- anchor: `` +- `textOffset` counts non-whitespace bytes + +Matching for reverse lookup: + +1. exact path match +2. index-insensitive path match (`div[2]` vs `div[3]` tolerated) +3. ancestor fallback + +If no match is found, caller must fallback to percentage. + +## Memory / Safety Constraints (ESP32-C3) + +The implementation intentionally avoids full DOM storage. + +- Parse one chapter only. +- Keep anchors in transient vectors only for duration of call. +- Free XML parser and chapter byte buffer on all success/failure paths. +- No persistent cache structures are introduced by this module. + +## Known Limitations + +- Page number on reverse mapping is still an estimate (renderer differences). +- Image-only/low-text chapters may yield coarse anchors. +- Extremely malformed XHTML can force fallback behavior. + +## Operational Logging + +`ProgressMapper` logs mapping source in reverse direction: + +- `xpath` when XPath mapping path was used +- `percentage` when fallback path was used + +It also logs exactness (`exact=yes/no`) for XPath matches. + +## Validation + +Use test vectors in: + +- `test/koreader_sync/roundtrip_vectors.md` +- `test/koreader_sync/memory_resource_qa.md` diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index 478a5e6e..eee2e877 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -13,6 +13,10 @@ namespace { +// Anchor used for both mapping directions. +// textOffset is counted as visible (non-whitespace) bytes from chapter start. +// xpath points to the nearest element path at/near that offset. + struct XPathAnchor { size_t textOffset = 0; std::string xpath; @@ -24,6 +28,9 @@ struct StackNode { bool hasTextAnchor = false; }; +// ParserState is intentionally ephemeral and created per lookup call. +// It holds only one spine parse worth of data to avoid retaining structures +// that would increase long-lived heap usage on the ESP32-C3. struct ParserState { explicit ParserState(const int spineIndex) : spineIndex(spineIndex) { siblingCounters.emplace_back(); } @@ -37,6 +44,11 @@ struct ParserState { std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; } + // Canonicalize incoming KOReader XPath before matching: + // - remove all whitespace + // - lowercase tags + // - strip optional trailing /text() + // - strip trailing slash static std::string normalizeXPath(const std::string& input) { if (input.empty()) { return ""; @@ -65,6 +77,8 @@ struct ParserState { return out; } + // Remove bracketed numeric predicates so paths can be compared even when + // index counters differ between parser implementations. static std::string removeIndices(const std::string& xpath) { std::string out; out.reserve(xpath.size()); @@ -96,6 +110,9 @@ struct ParserState { return depth; } + // Resolve a path to the best anchor offset. + // If exact node path is not found, progressively trim trailing segments and + // match ancestors to obtain a stable approximate location. bool pickBestAnchorByPath(const std::string& targetPath, const bool ignoreIndices, size_t& outTextOffset, bool& outExact) const { if (targetPath.empty() || anchors.empty()) { @@ -147,6 +164,7 @@ struct ParserState { return value; } + // Elements that should not contribute text position anchors. static bool isSkippableTag(const std::string& tag) { return tag == "head" || tag == "script" || tag == "style"; } static bool isWhitespaceOnly(const XML_Char* text, const int len) { @@ -158,6 +176,8 @@ struct ParserState { return true; } + // Count non-whitespace bytes to keep offsets stable against formatting-only + // differences and indentation in source XHTML. static size_t countVisibleBytes(const XML_Char* text, const int len) { size_t count = 0; for (int i = 0; i < len; i++) { @@ -192,6 +212,8 @@ struct ParserState { return xpath; } + // Adds first anchor for an element when text begins and periodic anchors in + // longer runs so matching has sufficient granularity without exploding memory. void addAnchorIfNeeded() { if (!insideBody() || stack.empty()) { return; @@ -269,6 +291,7 @@ struct ParserState { return it->xpath; } + // Convert path -> progress ratio by matching to nearest available anchor. bool chooseProgressForXPath(const std::string& xpath, float& outIntraSpineProgress, bool& outExactMatch) const { if (anchors.empty()) { return false; diff --git a/lib/KOReaderSync/ChapterXPathIndexer.h b/lib/KOReaderSync/ChapterXPathIndexer.h index e728b881..356815f2 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.h +++ b/lib/KOReaderSync/ChapterXPathIndexer.h @@ -6,38 +6,58 @@ #include /** - * Builds element-level XPath anchors for a spine item and picks the best match - * for an intra-spine progress value. + * Lightweight XPath/progress bridge for KOReader sync. + * + * Why this exists: + * - CrossPoint stores reading position as chapter/page. + * - KOReader sync uses XPath + percentage. + * + * This utility reparses exactly one spine XHTML item with Expat and builds + * transient text anchors () so we can translate in both + * directions without keeping a full DOM in memory. + * + * Design constraints (ESP32-C3): + * - No persistent full-book structures. + * - Parse-on-demand and free memory immediately. + * - Keep fallback behavior deterministic if parsing/matching fails. */ class ChapterXPathIndexer { public: /** + * Convert an intra-spine progress ratio to the nearest element-level XPath. + * * @param epub Loaded EPUB instance * @param spineIndex Current spine item index * @param intraSpineProgress Position within the spine item [0.0, 1.0] - * @return Best matching XPath, or empty string on failure + * @return Best matching XPath for KOReader, or empty string on failure */ static std::string findXPathForProgress(const std::shared_ptr& epub, int spineIndex, float intraSpineProgress); /** - * Resolve a KOReader XPath to an intra-spine progress value. + * Resolve a KOReader XPath to an intra-spine progress ratio. + * + * Matching strategy: + * 1) exact anchor path match, + * 2) index-insensitive path match, + * 3) ancestor fallback. * * @param epub Loaded EPUB instance * @param spineIndex Spine item index to parse * @param xpath Incoming KOReader XPath * @param outIntraSpineProgress Resolved position within spine [0.0, 1.0] - * @param outExactMatch True when an exact anchor match was found - * @return true if an exact or ancestor match was resolved + * @param outExactMatch True only for full exact path match + * @return true if any match was resolved; false means caller should fallback */ static bool findProgressForXPath(const std::shared_ptr& epub, int spineIndex, const std::string& xpath, float& outIntraSpineProgress, bool& outExactMatch); /** - * Parse the DocFragment index from a KOReader-style XPath. + * Parse DocFragment index from KOReader-style path segment: + * /body/DocFragment[N]/body/... * * @param xpath KOReader XPath * @param outSpineIndex Parsed DocFragment index (0-based) - * @return true when DocFragment[...] is present and valid + * @return true when DocFragment[N] exists and N is valid integer >= 0 */ static bool tryExtractSpineIndexFromXPath(const std::string& xpath, int& outSpineIndex); }; diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index 11a6e6e5..f5e58171 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -6,7 +6,6 @@ #include "ChapterXPathIndexer.h" - KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr& epub, const CrossPointPosition& pos) { KOReaderPosition result; diff --git a/lib/KOReaderSync/ProgressMapper.h b/lib/KOReaderSync/ProgressMapper.h index e4e056e4..6195fccc 100644 --- a/lib/KOReaderSync/ProgressMapper.h +++ b/lib/KOReaderSync/ProgressMapper.h @@ -27,9 +27,16 @@ struct KOReaderPosition { * CrossPoint tracks position as (spineIndex, pageNumber). * KOReader uses XPath-like strings + percentage. * - * CrossPoint first tries to extract an element-level XPath by reparsing the - * current spine XHTML and mapping intra-spine progress to text anchors. - * If extraction fails, it falls back to a synthetic chapter-level XPath. + * Forward mapping (CrossPoint -> KOReader): + * - Prefer element-level XPath extracted from current spine XHTML. + * - Fallback to synthetic chapter XPath if extraction fails. + * + * Reverse mapping (KOReader -> CrossPoint): + * - Prefer incoming XPath (DocFragment + element path) when resolvable. + * - Fallback to percentage-based approximation when XPath is missing/invalid. + * + * This keeps behavior stable on low-memory devices while improving round-trip + * sync precision when KOReader provides detailed paths. */ class ProgressMapper { public: @@ -45,8 +52,9 @@ class ProgressMapper { /** * Convert KOReader position to CrossPoint format. * - * Note: The returned pageNumber may be approximate since different - * rendering settings produce different page counts. + * Uses XPath-first resolution when possible and percentage fallback otherwise. + * Returned pageNumber can still be approximate because page counts differ + * across renderer/font/layout settings. * * @param epub The EPUB book * @param koPos KOReader position From d3178c7852f45cd71f0c597efed30acc761a4868 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Thu, 26 Feb 2026 07:36:36 +0100 Subject: [PATCH 3/9] Doc update --- docs/contributing/koreader-sync-xpath-mapping.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/contributing/koreader-sync-xpath-mapping.md b/docs/contributing/koreader-sync-xpath-mapping.md index 570b2861..9df844c8 100644 --- a/docs/contributing/koreader-sync-xpath-mapping.md +++ b/docs/contributing/koreader-sync-xpath-mapping.md @@ -40,6 +40,8 @@ Implemented in `ProgressMapper::toCrossPoint`. The module reparses **one spine XHTML** on demand using Expat and builds temporary anchors: +Source-of-truth note: XPath anchors are built from the original EPUB spine XHTML bytes (zip item contents), not from CrossPoint's distilled section render cache. This is intentional to preserve KOReader XPath compatibility. + - anchor: `` - `textOffset` counts non-whitespace bytes @@ -63,6 +65,7 @@ The implementation intentionally avoids full DOM storage. ## Known Limitations - Page number on reverse mapping is still an estimate (renderer differences). +- XPath mapping intentionally uses original spine XHTML while pagination comes from distilled renderer output, so minor roundtrip page drift is expected. - Image-only/low-text chapters may yield coarse anchors. - Extremely malformed XHTML can force fallback behavior. @@ -74,10 +77,3 @@ The implementation intentionally avoids full DOM storage. - `percentage` when fallback path was used It also logs exactness (`exact=yes/no`) for XPath matches. - -## Validation - -Use test vectors in: - -- `test/koreader_sync/roundtrip_vectors.md` -- `test/koreader_sync/memory_resource_qa.md` From 34586d001056212d07a5655f7c6a8c0fa3ebbe7d Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 27 Feb 2026 09:51:53 +0100 Subject: [PATCH 4/9] Fix faulty 0 based docfragment --- .../koreader-sync-xpath-mapping.md | 34 +++++++++++++++---- lib/KOReaderSync/ChapterXPathIndexer.cpp | 25 ++++++++------ lib/KOReaderSync/ProgressMapper.cpp | 4 +-- 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/docs/contributing/koreader-sync-xpath-mapping.md b/docs/contributing/koreader-sync-xpath-mapping.md index 9df844c8..3e80ebaf 100644 --- a/docs/contributing/koreader-sync-xpath-mapping.md +++ b/docs/contributing/koreader-sync-xpath-mapping.md @@ -6,7 +6,7 @@ This note documents how CrossPoint maps reading positions to and from KOReader s CrossPoint internally stores position as: -- `spineIndex` (chapter index) +- `spineIndex` (chapter index, 0-based) - `pageNumber` + `totalPages` KOReader sync payload stores: @@ -16,6 +16,19 @@ KOReader sync payload stores: A direct 1:1 mapping is not guaranteed because page layout differs between engines/devices. +## DocFragment Index Convention + +KOReader uses **1-based** XPath predicates throughout, following standard XPath conventions. +The first EPUB spine item is `DocFragment[1]`, the second is `DocFragment[2]`, and so on. + +CrossPoint stores spine items as 0-based indices internally. The conversion is: + +- **Generating XPath (to KOReader):** `DocFragment[spineIndex + 1]` +- **Parsing XPath (from KOReader):** `spineIndex = DocFragment[N] - 1` + +Reference: [koreader/koreader#11585](https://github.com/koreader/koreader/issues/11585) confirms this +via a KOReader contributor mapping spine items to DocFragment numbers. + ## Current Strategy ### CrossPoint -> KOReader @@ -25,13 +38,13 @@ Implemented in `ProgressMapper::toKOReader`. 1. Compute overall `percentage` from chapter/page. 2. Attempt to compute a real element-level XPath via `ChapterXPathIndexer::findXPathForProgress`. 3. If XPath extraction fails, fallback to synthetic chapter path: - - `/body/DocFragment[N]/body` + - `/body/DocFragment[spineIndex + 1]/body` ### KOReader -> CrossPoint Implemented in `ProgressMapper::toCrossPoint`. -1. Attempt to parse `DocFragment[N]` from incoming XPath. +1. Attempt to parse `DocFragment[N]` from incoming XPath; convert N to 0-based `spineIndex = N - 1`. 2. If valid, attempt XPath-to-offset mapping via `ChapterXPathIndexer::findProgressForXPath`. 3. Convert resolved intra-spine progress to page estimate. 4. If XPath path is invalid/unresolvable, fallback to percentage-based chapter/page estimation. @@ -44,12 +57,18 @@ Source-of-truth note: XPath anchors are built from the original EPUB spine XHTML - anchor: `` - `textOffset` counts non-whitespace bytes +- When multiple anchors exist for the same path, the one with the **smallest** textOffset is used + (start of element), not the latest periodic anchor. + +Forward lookup (CrossPoint → XPath): uses `upper_bound` to find the last anchor at or before the +target text offset, ensuring the returned XPath corresponds to the element the user is currently +inside rather than the next element. Matching for reverse lookup: -1. exact path match -2. index-insensitive path match (`div[2]` vs `div[3]` tolerated) -3. ancestor fallback +1. exact path match — reported as `exact=yes` +2. index-insensitive path match (`div[2]` vs `div[3]` tolerated) — reported as `exact=no` +3. ancestor fallback — reported as `exact=no` If no match is found, caller must fallback to percentage. @@ -76,4 +95,5 @@ The implementation intentionally avoids full DOM storage. - `xpath` when XPath mapping path was used - `percentage` when fallback path was used -It also logs exactness (`exact=yes/no`) for XPath matches. +It also logs exactness (`exact=yes/no`) for XPath matches. Note that `exact=yes` is only set for +a full path match with correct indices; index-insensitive and ancestor matches always log `exact=no`. diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index eee2e877..984f8d00 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -42,7 +42,7 @@ struct ParserState { std::vector> siblingCounters; std::vector anchors; - std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; } + std::string baseXPath() const { return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; } // Canonicalize incoming KOReader XPath before matching: // - remove all whitespace @@ -65,8 +65,8 @@ struct ParserState { } const std::string textSuffix = "/text()"; - size_t textPos = out.find(textSuffix); - if (textPos != std::string::npos) { + const size_t textPos = out.rfind(textSuffix); + if (textPos != std::string::npos && textPos + textSuffix.size() == out.size()) { out.erase(textPos); } @@ -132,7 +132,7 @@ struct ParserState { const std::string anchorPath = ignoreIndices ? removeIndices(anchor.xpath) : anchor.xpath; if (anchorPath == probe) { const int depth = pathDepth(anchorPath); - if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset > bestOffset)) { + if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset < bestOffset)) { found = true; bestDepth = depth; bestOffset = anchor.textOffset; @@ -282,11 +282,12 @@ struct ParserState { const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress)); const size_t target = static_cast(clampedProgress * static_cast(totalTextBytes)); - auto it = std::lower_bound(anchors.begin(), anchors.end(), target, - [](const XPathAnchor& anchor, const size_t value) { return anchor.textOffset < value; }); - - if (it == anchors.end()) { - return anchors.back().xpath; + // upper_bound returns the first anchor strictly after target; step back to get + // the last anchor at-or-before target (the element the user is currently inside). + auto it = std::upper_bound(anchors.begin(), anchors.end(), target, + [](const size_t value, const XPathAnchor& anchor) { return value < anchor.textOffset; }); + if (it != anchors.begin()) { + --it; } return it->xpath; } @@ -308,6 +309,7 @@ struct ParserState { if (!matched) { matched = pickBestAnchorByPath(normalized, true, matchedOffset, exact); + if (matched) exact = false; } if (!matched) { @@ -476,10 +478,11 @@ bool ChapterXPathIndexer::tryExtractSpineIndexFromXPath(const std::string& xpath const std::string value = normalized.substr(start, end - start); const long parsed = std::strtol(value.c_str(), nullptr, 10); - if (parsed < 0 || parsed > std::numeric_limits::max()) { + // KOReader uses 1-based DocFragment indices; convert to 0-based spine index. + if (parsed < 1 || parsed > std::numeric_limits::max()) { return false; } - outSpineIndex = static_cast(parsed); + outSpineIndex = static_cast(parsed) - 1; return true; } diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index f5e58171..b0199bf6 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -146,6 +146,6 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) { // Fallback path when element-level XPath extraction is unavailable. - // Uses 0-based DocFragment indices for KOReader compatibility. - return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body"; + // KOReader uses 1-based XPath predicates; spineIndex is 0-based internally. + return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; } From 9a4c9baf8528e6c325de86834d38d071ad9dcfb8 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 27 Feb 2026 10:20:32 +0100 Subject: [PATCH 5/9] Additional debug information --- lib/KOReaderSync/ChapterXPathIndexer.cpp | 29 ++++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index 984f8d00..ceb5c52b 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -305,25 +305,38 @@ struct ParserState { size_t matchedOffset = 0; bool exact = false; - bool matched = pickBestAnchorByPath(normalized, false, matchedOffset, exact); + const char* matchTier = nullptr; - if (!matched) { - matched = pickBestAnchorByPath(normalized, true, matchedOffset, exact); - if (matched) exact = false; + bool matched = pickBestAnchorByPath(normalized, false, matchedOffset, exact); + if (matched) { + matchTier = exact ? "exact" : "ancestor"; + } else { + bool exactRaw = false; + matched = pickBestAnchorByPath(normalized, true, matchedOffset, exactRaw); + if (matched) { + exact = false; + matchTier = exactRaw ? "index-insensitive" : "index-insensitive-ancestor"; + } } if (!matched) { + LOG_DBG("KOX", "Reverse: spine=%d no anchor match for '%s' (%zu anchors)", spineIndex, normalized.c_str(), + anchors.size()); return false; } outExactMatch = exact; if (totalTextBytes == 0) { outIntraSpineProgress = 0.0f; + LOG_DBG("KOX", "Reverse: spine=%d %s match offset=%zu -> progress=0.0 (no text)", spineIndex, matchTier, + matchedOffset); return true; } outIntraSpineProgress = static_cast(matchedOffset) / static_cast(totalTextBytes); outIntraSpineProgress = std::max(0.0f, std::min(1.0f, outIntraSpineProgress)); + LOG_DBG("KOX", "Reverse: spine=%d %s match offset=%zu/%zu -> progress=%.3f", spineIndex, matchTier, matchedOffset, + totalTextBytes, outIntraSpineProgress); return true; } }; @@ -397,7 +410,10 @@ std::string ChapterXPathIndexer::findXPathForProgress(const std::shared_ptr %s", spineIndex, intraSpineProgress, + state.anchors.size(), state.totalTextBytes, result.c_str()); + return result; } bool ChapterXPathIndexer::findProgressForXPath(const std::shared_ptr& epub, const int spineIndex, @@ -450,6 +466,8 @@ bool ChapterXPathIndexer::findProgressForXPath(const std::shared_ptr& epub return false; } + LOG_DBG("KOX", "Reverse: spine=%d anchors=%zu textBytes=%zu for '%s'", spineIndex, state.anchors.size(), + state.totalTextBytes, xpath.c_str()); return state.chooseProgressForXPath(xpath, outIntraSpineProgress, outExactMatch); } @@ -463,6 +481,7 @@ bool ChapterXPathIndexer::tryExtractSpineIndexFromXPath(const std::string& xpath const std::string key = "/docfragment["; const size_t pos = normalized.find(key); if (pos == std::string::npos) { + LOG_DBG("KOX", "No DocFragment in xpath: '%s'", xpath.c_str()); return false; } From 5e62dd79cd91d2d7048678c491c0c7c20b74f124 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 27 Feb 2026 11:09:42 +0100 Subject: [PATCH 6/9] Fix comparison --- lib/KOReaderSync/ChapterXPathIndexer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index ceb5c52b..28635486 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -129,7 +129,8 @@ struct ParserState { bool found = false; for (const auto& anchor : anchors) { - const std::string anchorPath = ignoreIndices ? removeIndices(anchor.xpath) : anchor.xpath; + const std::string normalizedAnchor = normalizeXPath(anchor.xpath); + const std::string anchorPath = ignoreIndices ? removeIndices(normalizedAnchor) : normalizedAnchor; if (anchorPath == probe) { const int depth = pathDepth(anchorPath); if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset < bestOffset)) { From 5764e22b82444403c13f317f7d4b11a4d7048ad7 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 27 Feb 2026 11:53:19 +0100 Subject: [PATCH 7/9] Review comments --- lib/KOReaderSync/ChapterXPathIndexer.h | 8 ++++++-- lib/KOReaderSync/ProgressMapper.cpp | 8 +++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexer.h b/lib/KOReaderSync/ChapterXPathIndexer.h index 356815f2..246fdecd 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.h +++ b/lib/KOReaderSync/ChapterXPathIndexer.h @@ -55,9 +55,13 @@ class ChapterXPathIndexer { * Parse DocFragment index from KOReader-style path segment: * /body/DocFragment[N]/body/... * + * KOReader uses 1-based DocFragment indices; N is converted to the 0-based + * spine index stored in outSpineIndex (i.e. outSpineIndex = N - 1). + * * @param xpath KOReader XPath - * @param outSpineIndex Parsed DocFragment index (0-based) - * @return true when DocFragment[N] exists and N is valid integer >= 0 + * @param outSpineIndex 0-based spine index derived from DocFragment[N] + * @return true when DocFragment[N] exists and N is a valid integer >= 1 + * (converted to 0-based outSpineIndex); false otherwise */ static bool tryExtractSpineIndexFromXPath(const std::string& xpath, int& outSpineIndex); }; diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index b0199bf6..d62e0608 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -2,6 +2,7 @@ #include +#include #include #include "ChapterXPathIndexer.h" @@ -71,7 +72,12 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu return result; } - const size_t targetBytes = static_cast(bookSize * koPos.percentage); + if (!std::isfinite(koPos.percentage)) { + return result; + } + + const float sanitizedPercentage = std::clamp(koPos.percentage, 0.0f, 1.0f); + const size_t targetBytes = static_cast(bookSize * sanitizedPercentage); bool spineFound = false; for (int i = 0; i < spineCount; i++) { From 8c08c19ca60f008181bfd503fdda74266090a23d Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 28 Feb 2026 05:31:12 +0100 Subject: [PATCH 8/9] Review comments --- lib/KOReaderSync/ChapterXPathIndexer.cpp | 20 ++++++++++++++++---- lib/KOReaderSync/ProgressMapper.cpp | 4 ++-- lib/KOReaderSync/ProgressMapper.h | 4 ++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index 28635486..667616b7 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -20,6 +20,7 @@ namespace { struct XPathAnchor { size_t textOffset = 0; std::string xpath; + std::string xpathNoIndex; // precomputed removeIndices(xpath) }; struct StackNode { @@ -129,8 +130,7 @@ struct ParserState { bool found = false; for (const auto& anchor : anchors) { - const std::string normalizedAnchor = normalizeXPath(anchor.xpath); - const std::string anchorPath = ignoreIndices ? removeIndices(normalizedAnchor) : normalizedAnchor; + const std::string& anchorPath = ignoreIndices ? anchor.xpathNoIndex : anchor.xpath; if (anchorPath == probe) { const int depth = pathDepth(anchorPath); if (!found || depth > bestDepth || (depth == bestDepth && anchor.textOffset < bestOffset)) { @@ -221,12 +221,13 @@ struct ParserState { } if (!stack.back().hasTextAnchor) { - anchors.push_back({totalTextBytes, currentXPath()}); + const std::string xpath = currentXPath(); + anchors.push_back({totalTextBytes, xpath, removeIndices(xpath)}); stack.back().hasTextAnchor = true; } else if (anchors.empty() || totalTextBytes - anchors.back().textOffset >= 192) { const std::string xpath = currentXPath(); if (anchors.empty() || anchors.back().xpath != xpath) { - anchors.push_back({totalTextBytes, xpath}); + anchors.push_back({totalTextBytes, xpath, removeIndices(xpath)}); } } } @@ -358,6 +359,17 @@ void XMLCALL onCharacterData(void* userData, const XML_Char* text, const int len } void XMLCALL onDefaultHandlerExpand(void* userData, const XML_Char* text, const int len) { + // The default handler fires for comments, PIs, DOCTYPE, and entity references. + // Only forward entity references (&..;) to avoid skewing text offsets with + // non-visible markup. + if (len < 3 || text[0] != '&' || text[len - 1] != ';') { + return; + } + for (int i = 1; i < len - 1; ++i) { + if (text[i] == '<' || text[i] == '>') { + return; + } + } auto* state = static_cast(userData); state->onCharacterData(text, len); } diff --git a/lib/KOReaderSync/ProgressMapper.cpp b/lib/KOReaderSync/ProgressMapper.cpp index d62e0608..f974da1b 100644 --- a/lib/KOReaderSync/ProgressMapper.cpp +++ b/lib/KOReaderSync/ProgressMapper.cpp @@ -24,7 +24,7 @@ KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr& epub, c // to a synthetic chapter-level path if parsing fails. result.xpath = ChapterXPathIndexer::findXPathForProgress(epub, pos.spineIndex, intraSpineProgress); if (result.xpath.empty()) { - result.xpath = generateXPath(pos.spineIndex, pos.pageNumber, pos.totalPages); + result.xpath = generateXPath(pos.spineIndex); } // Get chapter info for logging @@ -150,7 +150,7 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr& epu return result; } -std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) { +std::string ProgressMapper::generateXPath(int spineIndex) { // Fallback path when element-level XPath extraction is unavailable. // KOReader uses 1-based XPath predicates; spineIndex is 0-based internally. return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; diff --git a/lib/KOReaderSync/ProgressMapper.h b/lib/KOReaderSync/ProgressMapper.h index 6195fccc..6e375681 100644 --- a/lib/KOReaderSync/ProgressMapper.h +++ b/lib/KOReaderSync/ProgressMapper.h @@ -68,7 +68,7 @@ class ProgressMapper { private: /** * Generate XPath for KOReader compatibility. - * Fallback format: /body/DocFragment[spineIndex]/body + * Fallback format: /body/DocFragment[spineIndex + 1]/body */ - static std::string generateXPath(int spineIndex, int pageNumber, int totalPages); + static std::string generateXPath(int spineIndex); }; From f859169dde6aabb493a62a15ea0228072ceab414 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 28 Feb 2026 05:46:46 +0100 Subject: [PATCH 9/9] Function combination --- lib/KOReaderSync/ChapterXPathIndexer.cpp | 79 +++++++++--------------- 1 file changed, 28 insertions(+), 51 deletions(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexer.cpp b/lib/KOReaderSync/ChapterXPathIndexer.cpp index 667616b7..32909913 100644 --- a/lib/KOReaderSync/ChapterXPathIndexer.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexer.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -374,24 +375,23 @@ void XMLCALL onDefaultHandlerExpand(void* userData, const XML_Char* text, const state->onCharacterData(text, len); } -} // namespace - -std::string ChapterXPathIndexer::findXPathForProgress(const std::shared_ptr& epub, const int spineIndex, - const float intraSpineProgress) { +// Parse one spine item and return a fully populated ParserState. +// Returns std::nullopt if validation, I/O, or XML parse fails. +static std::optional parseSpineItem(const std::shared_ptr& epub, const int spineIndex) { if (!epub || spineIndex < 0 || spineIndex >= epub->getSpineItemsCount()) { - return ""; + return std::nullopt; } const auto spineItem = epub->getSpineItem(spineIndex); if (spineItem.href.empty()) { - return ""; + return std::nullopt; } size_t chapterSize = 0; uint8_t* chapterBytes = epub->readItemContentsToBytes(spineItem.href, &chapterSize, false); if (!chapterBytes || chapterSize == 0) { free(chapterBytes); - return ""; + return std::nullopt; } ParserState state(spineIndex); @@ -400,7 +400,7 @@ std::string ChapterXPathIndexer::findXPathForProgress(const std::shared_ptr& epub, const int spineIndex, + const float intraSpineProgress) { + const auto state = parseSpineItem(epub, spineIndex); + if (!state) { return ""; } - const std::string result = state.chooseXPath(intraSpineProgress); + const std::string result = state->chooseXPath(intraSpineProgress); LOG_DBG("KOX", "Forward: spine=%d progress=%.3f anchors=%zu textBytes=%zu -> %s", spineIndex, intraSpineProgress, - state.anchors.size(), state.totalTextBytes, result.c_str()); + state->anchors.size(), state->totalTextBytes, result.c_str()); return result; } @@ -435,53 +447,18 @@ bool ChapterXPathIndexer::findProgressForXPath(const std::shared_ptr& epub outIntraSpineProgress = 0.0f; outExactMatch = false; - if (!epub || spineIndex < 0 || spineIndex >= epub->getSpineItemsCount() || xpath.empty()) { + if (xpath.empty()) { return false; } - const auto spineItem = epub->getSpineItem(spineIndex); - if (spineItem.href.empty()) { + const auto state = parseSpineItem(epub, spineIndex); + if (!state) { return false; } - size_t chapterSize = 0; - uint8_t* chapterBytes = epub->readItemContentsToBytes(spineItem.href, &chapterSize, false); - if (!chapterBytes || chapterSize == 0) { - free(chapterBytes); - return false; - } - - ParserState state(spineIndex); - XML_Parser parser = XML_ParserCreate(nullptr); - if (!parser) { - free(chapterBytes); - LOG_ERR("KOX", "Failed to allocate XML parser for reverse lookup spine=%d", spineIndex); - return false; - } - - XML_SetUserData(parser, &state); - XML_SetElementHandler(parser, onStartElement, onEndElement); - XML_SetCharacterDataHandler(parser, onCharacterData); - XML_SetDefaultHandlerExpand(parser, onDefaultHandlerExpand); - - const bool parseOk = XML_Parse(parser, reinterpret_cast(chapterBytes), static_cast(chapterSize), - XML_TRUE) != XML_STATUS_ERROR; - - if (!parseOk) { - LOG_ERR("KOX", "Reverse XPath parse failed for spine=%d at line %lu: %s", spineIndex, - XML_GetCurrentLineNumber(parser), XML_ErrorString(XML_GetErrorCode(parser))); - } - - XML_ParserFree(parser); - free(chapterBytes); - - if (!parseOk) { - return false; - } - - LOG_DBG("KOX", "Reverse: spine=%d anchors=%zu textBytes=%zu for '%s'", spineIndex, state.anchors.size(), - state.totalTextBytes, xpath.c_str()); - return state.chooseProgressForXPath(xpath, outIntraSpineProgress, outExactMatch); + LOG_DBG("KOX", "Reverse: spine=%d anchors=%zu textBytes=%zu for '%s'", spineIndex, state->anchors.size(), + state->totalTextBytes, xpath.c_str()); + return state->chooseProgressForXPath(xpath, outIntraSpineProgress, outExactMatch); } bool ChapterXPathIndexer::tryExtractSpineIndexFromXPath(const std::string& xpath, int& outSpineIndex) {