fix: Fix kosync repositioning issue (#783)
## Summary * Original implementation had inconsistent positioning logic: - When XPath parsing succeeded: incorrectly set pageNumber = 0 (always beginning of chapter) - When XPath parsing failed: used percentage for positioning (worked correctly) - Result: Positions restored to wrong locations depending on XPath parsing success - Mentioned in Issue #581 * Solution - Unified ProgressMapper::toCrossPoint() to use percentage-based positioning exclusively for both spine identification and intra-chapter page calculation, eliminating unreliable XPath parsing entirely. ## Additional Context * ProgressMapper.cpp: Simplified toCrossPoint() to always use percentage for positioning, removed parseDocFragmentIndex() function * ProgressMapper.h: Updated comments and removed unused function declaration * Tests confirmed appropriate positioning * __Notabene: the syncing to another device will (most probably) end up at the current chapter of crosspoints reading position. There is not much we can do about it, as KOReader needs to have the correct XPath information - we can only provide an apporximate position (plus percentage) - the percentage information is not used in KOReaders current implementation__ --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? YES
This commit is contained in:
@@ -30,49 +30,73 @@ KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr<Epub>& epub, c
|
||||
}
|
||||
|
||||
CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr<Epub>& epub, const KOReaderPosition& koPos,
|
||||
int totalPagesInSpine) {
|
||||
int currentSpineIndex, int totalPagesInCurrentSpine) {
|
||||
CrossPointPosition result;
|
||||
result.spineIndex = 0;
|
||||
result.pageNumber = 0;
|
||||
result.totalPages = totalPagesInSpine;
|
||||
result.totalPages = 0;
|
||||
|
||||
const size_t bookSize = epub->getBookSize();
|
||||
if (bookSize == 0) {
|
||||
return result;
|
||||
}
|
||||
|
||||
// First, try to get spine index from XPath (DocFragment)
|
||||
int xpathSpineIndex = parseDocFragmentIndex(koPos.xpath);
|
||||
if (xpathSpineIndex >= 0 && xpathSpineIndex < epub->getSpineItemsCount()) {
|
||||
result.spineIndex = xpathSpineIndex;
|
||||
// When we have XPath, go to page 0 of the spine - byte-based page calculation is unreliable
|
||||
result.pageNumber = 0;
|
||||
} else {
|
||||
// Fall back to percentage-based lookup for both spine and page
|
||||
const size_t targetBytes = static_cast<size_t>(bookSize * koPos.percentage);
|
||||
// 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<size_t>(bookSize * koPos.percentage);
|
||||
|
||||
// Find the spine item that contains this byte position
|
||||
for (int i = 0; i < epub->getSpineItemsCount(); i++) {
|
||||
const size_t cumulativeSize = epub->getCumulativeSpineItemSize(i);
|
||||
if (cumulativeSize >= targetBytes) {
|
||||
result.spineIndex = i;
|
||||
break;
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
// Estimate page number within the spine item using percentage
|
||||
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;
|
||||
|
||||
int estimatedTotalPages = 0;
|
||||
|
||||
// If we are in the same spine, use the known total pages
|
||||
if (result.spineIndex == currentSpineIndex && totalPagesInCurrentSpine > 0) {
|
||||
estimatedTotalPages = totalPagesInCurrentSpine;
|
||||
}
|
||||
// Otherwise try to estimate based on density from current spine
|
||||
else if (currentSpineIndex >= 0 && currentSpineIndex < epub->getSpineItemsCount() && totalPagesInCurrentSpine > 0) {
|
||||
const size_t prevCurrCumSize =
|
||||
(currentSpineIndex > 0) ? epub->getCumulativeSpineItemSize(currentSpineIndex - 1) : 0;
|
||||
const size_t currCumSize = epub->getCumulativeSpineItemSize(currentSpineIndex);
|
||||
const size_t currSpineSize = currCumSize - prevCurrCumSize;
|
||||
|
||||
if (currSpineSize > 0) {
|
||||
float ratio = static_cast<float>(spineSize) / static_cast<float>(currSpineSize);
|
||||
estimatedTotalPages = static_cast<int>(totalPagesInCurrentSpine * ratio);
|
||||
if (estimatedTotalPages < 1) estimatedTotalPages = 1;
|
||||
}
|
||||
}
|
||||
|
||||
// Estimate page number within the spine item using percentage (only when no XPath)
|
||||
if (totalPagesInSpine > 0 && 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;
|
||||
result.totalPages = estimatedTotalPages;
|
||||
|
||||
if (spineSize > 0) {
|
||||
const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0;
|
||||
const float intraSpineProgress = static_cast<float>(bytesIntoSpine) / static_cast<float>(spineSize);
|
||||
const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress));
|
||||
result.pageNumber = static_cast<int>(clampedProgress * totalPagesInSpine);
|
||||
result.pageNumber = std::max(0, std::min(result.pageNumber, totalPagesInSpine - 1));
|
||||
}
|
||||
if (spineSize > 0 && estimatedTotalPages > 0) {
|
||||
const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0;
|
||||
const float intraSpineProgress = static_cast<float>(bytesIntoSpine) / static_cast<float>(spineSize);
|
||||
const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress));
|
||||
result.pageNumber = static_cast<int>(clampedProgress * estimatedTotalPages);
|
||||
result.pageNumber = std::max(0, std::min(result.pageNumber, estimatedTotalPages - 1));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,30 +107,8 @@ CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr<Epub>& epu
|
||||
}
|
||||
|
||||
std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) {
|
||||
// KOReader uses 1-based DocFragment indices
|
||||
// Use a simple xpath pointing to the DocFragment - KOReader will use the percentage for fine positioning
|
||||
// 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
|
||||
return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body";
|
||||
}
|
||||
|
||||
int ProgressMapper::parseDocFragmentIndex(const std::string& xpath) {
|
||||
// Look for DocFragment[N] pattern
|
||||
const size_t start = xpath.find("DocFragment[");
|
||||
if (start == std::string::npos) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
const size_t numStart = start + 12; // Length of "DocFragment["
|
||||
const size_t numEnd = xpath.find(']', numStart);
|
||||
if (numEnd == std::string::npos) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
try {
|
||||
const int docFragmentIndex = std::stoi(xpath.substr(numStart, numEnd - numStart));
|
||||
// KOReader uses 1-based indices, we use 0-based
|
||||
return docFragmentIndex - 1;
|
||||
} catch (...) {
|
||||
return -1;
|
||||
}
|
||||
return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body";
|
||||
}
|
||||
|
||||
@@ -50,23 +50,18 @@ class ProgressMapper {
|
||||
*
|
||||
* @param epub The EPUB book
|
||||
* @param koPos KOReader position
|
||||
* @param totalPagesInSpine Total pages in the target spine item (for page estimation)
|
||||
* @param currentSpineIndex Index of the currently open spine item (for density estimation)
|
||||
* @param totalPagesInCurrentSpine Total pages in the current spine item (for density estimation)
|
||||
* @return CrossPoint position
|
||||
*/
|
||||
static CrossPointPosition toCrossPoint(const std::shared_ptr<Epub>& epub, const KOReaderPosition& koPos,
|
||||
int totalPagesInSpine = 0);
|
||||
int currentSpineIndex = -1, int totalPagesInCurrentSpine = 0);
|
||||
|
||||
private:
|
||||
/**
|
||||
* Generate XPath for KOReader compatibility.
|
||||
* Format: /body/DocFragment[spineIndex+1]/body/p[estimatedParagraph]
|
||||
* Paragraph is estimated based on page position within the chapter.
|
||||
* Format: /body/DocFragment[spineIndex+1]/body
|
||||
* Since CrossPoint doesn't preserve HTML structure, we rely on percentage for positioning.
|
||||
*/
|
||||
static std::string generateXPath(int spineIndex, int pageNumber, int totalPages);
|
||||
|
||||
/**
|
||||
* Parse DocFragment index from XPath string.
|
||||
* Returns -1 if not found.
|
||||
*/
|
||||
static int parseDocFragmentIndex(const std::string& xpath);
|
||||
};
|
||||
|
||||
@@ -95,7 +95,7 @@ void KOReaderSyncActivity::performSync() {
|
||||
// Convert remote progress to CrossPoint position
|
||||
hasRemoteProgress = true;
|
||||
KOReaderPosition koPos = {remoteProgress.progress, remoteProgress.percentage};
|
||||
remotePosition = ProgressMapper::toCrossPoint(epub, koPos, totalPagesInSpine);
|
||||
remotePosition = ProgressMapper::toCrossPoint(epub, koPos, currentSpineIndex, totalPagesInSpine);
|
||||
|
||||
// Calculate local progress in KOReader format (for display)
|
||||
CrossPointPosition localPos = {currentSpineIndex, currentPage, totalPagesInSpine};
|
||||
@@ -104,7 +104,13 @@ void KOReaderSyncActivity::performSync() {
|
||||
{
|
||||
RenderLock lock(*this);
|
||||
state = SHOWING_RESULT;
|
||||
selectedOption = 0; // Default to "Apply"
|
||||
|
||||
// Default to the option that corresponds to the furthest progress
|
||||
if (localProgress.percentage > remoteProgress.percentage) {
|
||||
selectedOption = 1; // Upload local progress
|
||||
} else {
|
||||
selectedOption = 0; // Apply remote progress
|
||||
}
|
||||
}
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user