A fork of https://github.com/crosspoint-reader/crosspoint-reader
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
+69
-66
+56
-54
lib/KOReaderSync/ProgressMapper.cpp
+56
-54
lib/KOReaderSync/ProgressMapper.cpp
···
30
30
}
31
31
32
32
CrossPointPosition ProgressMapper::toCrossPoint(const std::shared_ptr<Epub>& epub, const KOReaderPosition& koPos,
33
-
int totalPagesInSpine) {
33
+
int currentSpineIndex, int totalPagesInCurrentSpine) {
34
34
CrossPointPosition result;
35
35
result.spineIndex = 0;
36
36
result.pageNumber = 0;
37
-
result.totalPages = totalPagesInSpine;
37
+
result.totalPages = 0;
38
38
39
39
const size_t bookSize = epub->getBookSize();
40
40
if (bookSize == 0) {
41
41
return result;
42
42
}
43
43
44
-
// First, try to get spine index from XPath (DocFragment)
45
-
int xpathSpineIndex = parseDocFragmentIndex(koPos.xpath);
46
-
if (xpathSpineIndex >= 0 && xpathSpineIndex < epub->getSpineItemsCount()) {
47
-
result.spineIndex = xpathSpineIndex;
48
-
// When we have XPath, go to page 0 of the spine - byte-based page calculation is unreliable
49
-
result.pageNumber = 0;
50
-
} else {
51
-
// Fall back to percentage-based lookup for both spine and page
52
-
const size_t targetBytes = static_cast<size_t>(bookSize * koPos.percentage);
44
+
// Use percentage-based lookup for both spine and page positioning
45
+
// XPath parsing is unreliable since CrossPoint doesn't preserve detailed HTML structure
46
+
const size_t targetBytes = static_cast<size_t>(bookSize * koPos.percentage);
53
47
54
-
// Find the spine item that contains this byte position
55
-
for (int i = 0; i < epub->getSpineItemsCount(); i++) {
56
-
const size_t cumulativeSize = epub->getCumulativeSpineItemSize(i);
57
-
if (cumulativeSize >= targetBytes) {
58
-
result.spineIndex = i;
59
-
break;
60
-
}
48
+
// Find the spine item that contains this byte position
49
+
const int spineCount = epub->getSpineItemsCount();
50
+
bool spineFound = false;
51
+
for (int i = 0; i < spineCount; i++) {
52
+
const size_t cumulativeSize = epub->getCumulativeSpineItemSize(i);
53
+
if (cumulativeSize >= targetBytes) {
54
+
result.spineIndex = i;
55
+
spineFound = true;
56
+
break;
61
57
}
58
+
}
62
59
63
-
// Estimate page number within the spine item using percentage (only when no XPath)
64
-
if (totalPagesInSpine > 0 && result.spineIndex < epub->getSpineItemsCount()) {
65
-
const size_t prevCumSize = (result.spineIndex > 0) ? epub->getCumulativeSpineItemSize(result.spineIndex - 1) : 0;
66
-
const size_t currentCumSize = epub->getCumulativeSpineItemSize(result.spineIndex);
67
-
const size_t spineSize = currentCumSize - prevCumSize;
60
+
// If no spine item was found (e.g., targetBytes beyond last cumulative size),
61
+
// default to the last spine item so we map to the end of the book instead of the beginning.
62
+
if (!spineFound && spineCount > 0) {
63
+
result.spineIndex = spineCount - 1;
64
+
}
65
+
66
+
// Estimate page number within the spine item using percentage
67
+
if (result.spineIndex < epub->getSpineItemsCount()) {
68
+
const size_t prevCumSize = (result.spineIndex > 0) ? epub->getCumulativeSpineItemSize(result.spineIndex - 1) : 0;
69
+
const size_t currentCumSize = epub->getCumulativeSpineItemSize(result.spineIndex);
70
+
const size_t spineSize = currentCumSize - prevCumSize;
68
71
69
-
if (spineSize > 0) {
70
-
const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0;
71
-
const float intraSpineProgress = static_cast<float>(bytesIntoSpine) / static_cast<float>(spineSize);
72
-
const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress));
73
-
result.pageNumber = static_cast<int>(clampedProgress * totalPagesInSpine);
74
-
result.pageNumber = std::max(0, std::min(result.pageNumber, totalPagesInSpine - 1));
72
+
int estimatedTotalPages = 0;
73
+
74
+
// If we are in the same spine, use the known total pages
75
+
if (result.spineIndex == currentSpineIndex && totalPagesInCurrentSpine > 0) {
76
+
estimatedTotalPages = totalPagesInCurrentSpine;
77
+
}
78
+
// Otherwise try to estimate based on density from current spine
79
+
else if (currentSpineIndex >= 0 && currentSpineIndex < epub->getSpineItemsCount() && totalPagesInCurrentSpine > 0) {
80
+
const size_t prevCurrCumSize =
81
+
(currentSpineIndex > 0) ? epub->getCumulativeSpineItemSize(currentSpineIndex - 1) : 0;
82
+
const size_t currCumSize = epub->getCumulativeSpineItemSize(currentSpineIndex);
83
+
const size_t currSpineSize = currCumSize - prevCurrCumSize;
84
+
85
+
if (currSpineSize > 0) {
86
+
float ratio = static_cast<float>(spineSize) / static_cast<float>(currSpineSize);
87
+
estimatedTotalPages = static_cast<int>(totalPagesInCurrentSpine * ratio);
88
+
if (estimatedTotalPages < 1) estimatedTotalPages = 1;
75
89
}
76
90
}
91
+
92
+
result.totalPages = estimatedTotalPages;
93
+
94
+
if (spineSize > 0 && estimatedTotalPages > 0) {
95
+
const size_t bytesIntoSpine = (targetBytes > prevCumSize) ? (targetBytes - prevCumSize) : 0;
96
+
const float intraSpineProgress = static_cast<float>(bytesIntoSpine) / static_cast<float>(spineSize);
97
+
const float clampedProgress = std::max(0.0f, std::min(1.0f, intraSpineProgress));
98
+
result.pageNumber = static_cast<int>(clampedProgress * estimatedTotalPages);
99
+
result.pageNumber = std::max(0, std::min(result.pageNumber, estimatedTotalPages - 1));
100
+
}
77
101
}
78
102
79
103
LOG_DBG("ProgressMapper", "KOReader -> CrossPoint: %.2f%% at %s -> spine=%d, page=%d", koPos.percentage * 100,
···
83
107
}
84
108
85
109
std::string ProgressMapper::generateXPath(int spineIndex, int pageNumber, int totalPages) {
86
-
// KOReader uses 1-based DocFragment indices
87
-
// Use a simple xpath pointing to the DocFragment - KOReader will use the percentage for fine positioning
110
+
// Use 0-based DocFragment indices for KOReader
111
+
// Use a simple xpath pointing to the DocFragment - KOReader will use the percentage for fine positioning within it
88
112
// Avoid specifying paragraph numbers as they may not exist in the target document
89
-
return "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body";
90
-
}
91
-
92
-
int ProgressMapper::parseDocFragmentIndex(const std::string& xpath) {
93
-
// Look for DocFragment[N] pattern
94
-
const size_t start = xpath.find("DocFragment[");
95
-
if (start == std::string::npos) {
96
-
return -1;
97
-
}
98
-
99
-
const size_t numStart = start + 12; // Length of "DocFragment["
100
-
const size_t numEnd = xpath.find(']', numStart);
101
-
if (numEnd == std::string::npos) {
102
-
return -1;
103
-
}
104
-
105
-
try {
106
-
const int docFragmentIndex = std::stoi(xpath.substr(numStart, numEnd - numStart));
107
-
// KOReader uses 1-based indices, we use 0-based
108
-
return docFragmentIndex - 1;
109
-
} catch (...) {
110
-
return -1;
111
-
}
113
+
return "/body/DocFragment[" + std::to_string(spineIndex) + "]/body";
112
114
}
+5
-10
lib/KOReaderSync/ProgressMapper.h
+5
-10
lib/KOReaderSync/ProgressMapper.h
···
50
50
*
51
51
* @param epub The EPUB book
52
52
* @param koPos KOReader position
53
-
* @param totalPagesInSpine Total pages in the target spine item (for page estimation)
53
+
* @param currentSpineIndex Index of the currently open spine item (for density estimation)
54
+
* @param totalPagesInCurrentSpine Total pages in the current spine item (for density estimation)
54
55
* @return CrossPoint position
55
56
*/
56
57
static CrossPointPosition toCrossPoint(const std::shared_ptr<Epub>& epub, const KOReaderPosition& koPos,
57
-
int totalPagesInSpine = 0);
58
+
int currentSpineIndex = -1, int totalPagesInCurrentSpine = 0);
58
59
59
60
private:
60
61
/**
61
62
* Generate XPath for KOReader compatibility.
62
-
* Format: /body/DocFragment[spineIndex+1]/body/p[estimatedParagraph]
63
-
* Paragraph is estimated based on page position within the chapter.
63
+
* Format: /body/DocFragment[spineIndex+1]/body
64
+
* Since CrossPoint doesn't preserve HTML structure, we rely on percentage for positioning.
64
65
*/
65
66
static std::string generateXPath(int spineIndex, int pageNumber, int totalPages);
66
-
67
-
/**
68
-
* Parse DocFragment index from XPath string.
69
-
* Returns -1 if not found.
70
-
*/
71
-
static int parseDocFragmentIndex(const std::string& xpath);
72
67
};
+8
-2
src/activities/reader/KOReaderSyncActivity.cpp
+8
-2
src/activities/reader/KOReaderSyncActivity.cpp
···
123
123
// Convert remote progress to CrossPoint position
124
124
hasRemoteProgress = true;
125
125
KOReaderPosition koPos = {remoteProgress.progress, remoteProgress.percentage};
126
-
remotePosition = ProgressMapper::toCrossPoint(epub, koPos, totalPagesInSpine);
126
+
remotePosition = ProgressMapper::toCrossPoint(epub, koPos, currentSpineIndex, totalPagesInSpine);
127
127
128
128
// Calculate local progress in KOReader format (for display)
129
129
CrossPointPosition localPos = {currentSpineIndex, currentPage, totalPagesInSpine};
···
132
132
{
133
133
RenderLock lock(*this);
134
134
state = SHOWING_RESULT;
135
-
selectedOption = 0; // Default to "Apply"
135
+
136
+
// Default to the option that corresponds to the furthest progress
137
+
if (localProgress.percentage > remoteProgress.percentage) {
138
+
selectedOption = 1; // Upload local progress
139
+
} else {
140
+
selectedOption = 0; // Apply remote progress
141
+
}
136
142
}
137
143
requestUpdate();
138
144
}