port: upstream PRs #1311 (inter-word spacing fix) and #1322 (zip early exit)

PR #1311: Replace separate spaceWidth + getSpaceKernAdjust() with a single getSpaceAdvance() that combines space glyph advance and kerning in fixed-point before snapping to pixels, eliminating +/-1 px rounding drift in text layout. PR #1322: Add early exit to fillUncompressedSizes() once all target entries are matched, avoiding unnecessary central directory traversal. Also updates tracking docs and verifies PR #1329 (reader utils refactor) matches upstream after merge. Made-with: Cursor
2026-03-08 15:53:13 -04:00
parent 0d8a3fdbdd
commit 255b98bda0
9 changed files with 167 additions and 37 deletions
--- a/chat-summaries/2026-03-08_14-00-summary.md
+++ b/chat-summaries/2026-03-08_14-00-summary.md
@@ -0,0 +1,28 @@
+# Restore Preferred Orientation Settings and Long-Press Sub-Menu
+
+**Date**: 2026-03-08
+**Branch**: mod/master
+**Commit**: 0d8a3fd
+
+## Task
+
+Restore two orientation preference features lost during the upstream PR resync:
+1. Settings UI entries for preferred portrait/landscape modes
+2. Long-press sub-menu on the reader menu's orientation toggle
+
+## Changes
+
+### Settings UI and Persistence
+- **`src/SettingsList.h`**: Added `DynamicEnum` entries for `preferredPortrait` (Portrait/Inverted) and `preferredLandscape` (Landscape CW/Landscape CCW) in the Reader settings category. Uses getter/setter lambdas to map between sequential indices and non-sequential orientation enum values.
+- **`src/JsonSettingsIO.cpp`**: Added manual JSON save/load for both fields with validation (rejects invalid orientation values, falls back to defaults).
+
+### Long-Press Orientation Sub-Menu
+- **`src/activities/reader/EpubReaderMenuActivity.h`**: Added `orientationSubMenuOpen`, `orientationSubMenuIndex`, and `ignoreNextConfirmRelease` state flags.
+- **`src/activities/reader/EpubReaderMenuActivity.cpp`**:
+  - `loop()`: Long-press (700ms) on Confirm when the orientation item is selected opens the sub-menu. Sub-menu handles its own Up/Down/Confirm/Back input. Added `ignoreNextConfirmRelease` guard to prevent the long-press release from immediately selecting.
+  - `render()`: When sub-menu is open, renders a centered list of all 4 orientations with the current one marked with `*`. Uses the same gutter/hint layout as the main menu.
+
+## Follow-up
+
+- Hardware testing needed for both features
+- Translations for `STR_PREFERRED_PORTRAIT` and `STR_PREFERRED_LANDSCAPE` only exist in English; other languages fall back automatically
--- a/chat-summaries/2026-03-08_17-30-summary.md
+++ b/chat-summaries/2026-03-08_17-30-summary.md
@@ -0,0 +1,43 @@
+# Port Upstream PRs #1311, #1322 + Verify #1329
+
+**Date:** 2026-03-08
+**Task:** Port two upstream PRs and verify alignment of a previously-ported PR that was recently merged.
+
+## Changes Made
+
+### PR #1311 -- Fix inter-word spacing rounding error (UNMERGED, ported as mod feature)
+
+Replaced `getSpaceKernAdjust()` with `getSpaceAdvance()` which combines space glyph advance and flanking kern values into a single fixed-point sum before pixel snapping, fixing +/-1 px rounding drift in inter-word spacing.
+
+**Files modified:**
+- `lib/GfxRenderer/GfxRenderer.h` -- replaced declaration
+- `lib/GfxRenderer/GfxRenderer.cpp` -- replaced implementation (single-snap pattern)
+- `lib/Epub/Epub/ParsedText.h` -- removed `spaceWidth` parameter from 3 internal functions
+- `lib/Epub/Epub/ParsedText.cpp` -- updated all 4 call sites to use `getSpaceAdvance()`
+
+### PR #1322 -- Early exit on fillUncompressedSizes (MERGED, ported for immediate use)
+
+Added `targetCount` variable and early `break` when all ZIP central-directory targets are matched.
+
+**Files modified:**
+- `lib/ZipFile/ZipFile.cpp` -- 5-line addition
+
+### PR #1329 -- Reader utils refactor (MERGED, verification only)
+
+Confirmed our existing port matches the upstream merged version (commit `cd508d2`) line-for-line. No code changes needed.
+
+### Tracking documentation updated
+
+- `mod/docs/upstream-sync.md` -- added #1311, #1322; updated #1329 status to MERGED
+- `mod/prs/MERGED.md` -- added detailed entries for #1311 and #1322; updated #1329 author and status
+
+## Build Result
+
+SUCCESS -- zero compiler errors/warnings from our changes. Only pre-existing i18n translation warnings.
+
+## Follow-up Items
+
+- #1311: Will be dropped during next sync if/when merged upstream
+- #1322: Will be dropped during next sync (already merged upstream)
+- #1329: Will be dropped during next sync (already merged upstream)
+- Hardware testing recommended: verify text layout rendering after spacing fix (#1311)
--- a/lib/Epub/Epub/ParsedText.cpp
+++ b/lib/Epub/Epub/ParsedText.cpp
@@ -176,20 +176,18 @@ void ParsedText::layoutAndExtractLines(const GfxRenderer& renderer, const int fo
  applyParagraphIndent();

  const int pageWidth = viewportWidth;
-  const int spaceWidth = renderer.getSpaceWidth(fontId, EpdFontFamily::REGULAR);
  auto wordWidths = calculateWordWidths(renderer, fontId);

  std::vector<size_t> lineBreakIndices;
  if (hyphenationEnabled) {
-    // Use greedy layout that can split words mid-loop when a hyphenated prefix fits.
-    lineBreakIndices = computeHyphenatedLineBreaks(renderer, fontId, pageWidth, spaceWidth, wordWidths, wordContinues);
+    lineBreakIndices = computeHyphenatedLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues);
  } else {
-    lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, spaceWidth, wordWidths, wordContinues);
+    lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues);
  }
  const size_t lineCount = includeLastLine ? lineBreakIndices.size() : lineBreakIndices.size() - 1;

  for (size_t i = 0; i < lineCount; ++i) {
-    extractLine(i, pageWidth, spaceWidth, wordWidths, wordContinues, lineBreakIndices, processLine, renderer, fontId);
+    extractLine(i, pageWidth, wordWidths, wordContinues, lineBreakIndices, processLine, renderer, fontId);
  }

  // Remove consumed words so size() reflects only remaining words
@@ -213,7 +211,7 @@ std::vector<uint16_t> ParsedText::calculateWordWidths(const GfxRenderer& rendere
 }

 std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, const int fontId, const int pageWidth,
-                                                  const int spaceWidth, std::vector<uint16_t>& wordWidths,
+                                                  std::vector<uint16_t>& wordWidths,
                                                  std::vector<bool>& continuesVec) {
  if (words.empty()) {
    return {};
@@ -262,9 +260,8 @@ std::vector<size_t> ParsedText::computeLineBreaks(const GfxRenderer& renderer, c
      // Add space before word j, unless it's the first word on the line or a continuation
      int gap = 0;
      if (j > static_cast<size_t>(i) && !continuesVec[j]) {
-        gap = spaceWidth;
-        gap += renderer.getSpaceKernAdjust(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]),
-                                           wordStyles[j - 1]);
+        gap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]),
+                                       wordStyles[j - 1]);
      } else if (j > static_cast<size_t>(i) && continuesVec[j]) {
        // Cross-boundary kerning for continuation words (e.g. nonbreaking spaces, attached punctuation)
        gap = renderer.getKerning(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]), wordStyles[j - 1]);
@@ -351,7 +348,7 @@ void ParsedText::applyParagraphIndent() {

 // Builds break indices while opportunistically splitting the word that would overflow the current line.
 std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& renderer, const int fontId,
-                                                            const int pageWidth, const int spaceWidth,
+                                                            const int pageWidth,
                                                            std::vector<uint16_t>& wordWidths,
                                                            std::vector<bool>& continuesVec) {
  // Calculate first line indent (only for left/justified text).
@@ -380,9 +377,8 @@ std::vector<size_t> ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& r
      const bool isFirstWord = currentIndex == lineStart;
      int spacing = 0;
      if (!isFirstWord && !continuesVec[currentIndex]) {
-        spacing = spaceWidth;
-        spacing += renderer.getSpaceKernAdjust(fontId, lastCodepoint(words[currentIndex - 1]),
-                                               firstCodepoint(words[currentIndex]), wordStyles[currentIndex - 1]);
+        spacing = renderer.getSpaceAdvance(fontId, lastCodepoint(words[currentIndex - 1]),
+                                           firstCodepoint(words[currentIndex]), wordStyles[currentIndex - 1]);
      } else if (!isFirstWord && continuesVec[currentIndex]) {
        // Cross-boundary kerning for continuation words (e.g. nonbreaking spaces, attached punctuation)
        spacing = renderer.getKerning(fontId, lastCodepoint(words[currentIndex - 1]),
@@ -523,9 +519,8 @@ bool ParsedText::hyphenateWordAtIndex(const size_t wordIndex, const int availabl
  return true;
 }

-void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const int spaceWidth,
-                             const std::vector<uint16_t>& wordWidths, const std::vector<bool>& continuesVec,
-                             const std::vector<size_t>& lineBreakIndices,
+void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const std::vector<uint16_t>& wordWidths,
+                             const std::vector<bool>& continuesVec, const std::vector<size_t>& lineBreakIndices,
                             const std::function<void(std::shared_ptr<TextBlock>)>& processLine,
                             const GfxRenderer& renderer, const int fontId) {
  const size_t lineBreak = lineBreakIndices[breakIndex];
@@ -554,8 +549,7 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
    // Count gaps: each word after the first creates a gap, unless it's a continuation
    if (wordIdx > 0 && !continuesVec[lastBreakAt + wordIdx]) {
      actualGapCount++;
-      int naturalGap = spaceWidth;
-      naturalGap += renderer.getSpaceKernAdjust(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]),
+      int naturalGap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]),
                                                firstCodepoint(words[lastBreakAt + wordIdx]),
                                                wordStyles[lastBreakAt + wordIdx - 1]);
      totalNaturalGaps += naturalGap;
@@ -603,12 +597,11 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const
                              firstCodepoint(words[lastBreakAt + wordIdx + 1]), wordStyles[lastBreakAt + wordIdx]);
      xpos += advance;
    } else {
-      int gap = spaceWidth;
-      if (wordIdx + 1 < lineWordCount) {
-        gap += renderer.getSpaceKernAdjust(fontId, lastCodepoint(words[lastBreakAt + wordIdx]),
-                                           firstCodepoint(words[lastBreakAt + wordIdx + 1]),
-                                           wordStyles[lastBreakAt + wordIdx]);
-      }
+      int gap = wordIdx + 1 < lineWordCount
+                    ? renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx]),
+                                              firstCodepoint(words[lastBreakAt + wordIdx + 1]),
+                                              wordStyles[lastBreakAt + wordIdx])
+                    : renderer.getSpaceWidth(fontId, wordStyles[lastBreakAt + wordIdx]);
      if (blockStyle.alignment == CssTextAlign::Justify && !isLastLine) {
        gap += justifyExtra;
      }
--- a/lib/Epub/Epub/ParsedText.h
+++ b/lib/Epub/Epub/ParsedText.h
@@ -21,14 +21,14 @@ class ParsedText {
  bool hyphenationEnabled;

  void applyParagraphIndent();
-  std::vector<size_t> computeLineBreaks(const GfxRenderer& renderer, int fontId, int pageWidth, int spaceWidth,
+  std::vector<size_t> computeLineBreaks(const GfxRenderer& renderer, int fontId, int pageWidth,
                                        std::vector<uint16_t>& wordWidths, std::vector<bool>& continuesVec);
  std::vector<size_t> computeHyphenatedLineBreaks(const GfxRenderer& renderer, int fontId, int pageWidth,
-                                                  int spaceWidth, std::vector<uint16_t>& wordWidths,
+                                                  std::vector<uint16_t>& wordWidths,
                                                  std::vector<bool>& continuesVec);
  bool hyphenateWordAtIndex(size_t wordIndex, int availableWidth, const GfxRenderer& renderer, int fontId,
                            std::vector<uint16_t>& wordWidths, bool allowFallbackBreaks);
-  void extractLine(size_t breakIndex, int pageWidth, int spaceWidth, const std::vector<uint16_t>& wordWidths,
+  void extractLine(size_t breakIndex, int pageWidth, const std::vector<uint16_t>& wordWidths,
                   const std::vector<bool>& continuesVec, const std::vector<size_t>& lineBreakIndices,
                   const std::function<void(std::shared_ptr<TextBlock>)>& processLine, const GfxRenderer& renderer,
                   int fontId);
--- a/lib/GfxRenderer/GfxRenderer.cpp
+++ b/lib/GfxRenderer/GfxRenderer.cpp
@@ -1123,13 +1123,15 @@ int GfxRenderer::getSpaceWidth(const int fontId, const EpdFontFamily::Style styl
  return spaceGlyph ? fp4::toPixel(spaceGlyph->advanceX) : 0;  // snap 12.4 fixed-point to nearest pixel
 }

-int GfxRenderer::getSpaceKernAdjust(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
-                                    const EpdFontFamily::Style style) const {
+int GfxRenderer::getSpaceAdvance(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
+                                 const EpdFontFamily::Style style) const {
  const auto fontIt = fontMap.find(fontId);
  if (fontIt == fontMap.end()) return 0;
  const auto& font = fontIt->second;
-  const int kernFP = font.getKerning(leftCp, ' ', style) + font.getKerning(' ', rightCp, style);  // 4.4 fixed-point
-  return fp4::toPixel(kernFP);  // snap 4.4 fixed-point to nearest pixel
+  const EpdGlyph* spaceGlyph = font.getGlyph(' ', style);
+  if (!spaceGlyph) return 0;
+  const int spaceAdvanceFP = spaceGlyph->advanceX;  // 12.4 fixed-point
+  return fp4::toPixel(spaceAdvanceFP + font.getKerning(leftCp, ' ', style) + font.getKerning(' ', rightCp, style));
 }

 int GfxRenderer::getKerning(const int fontId, const uint32_t leftCp, const uint32_t rightCp,
--- a/lib/GfxRenderer/GfxRenderer.h
+++ b/lib/GfxRenderer/GfxRenderer.h
@@ -119,9 +119,9 @@ class GfxRenderer {
  void drawText(int fontId, int x, int y, const char* text, bool black = true,
                EpdFontFamily::Style style = EpdFontFamily::REGULAR) const;
  int getSpaceWidth(int fontId, EpdFontFamily::Style style = EpdFontFamily::REGULAR) const;
-  /// Returns the kerning adjustment for a space between two codepoints:
-  /// kern(leftCp, ' ') + kern(' ', rightCp). Returns 0 if kerning is unavailable.
-  int getSpaceKernAdjust(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
+  /// Returns the full inter-word space advance: space glyph advance + kern(leftCp, ' ') + kern(' ', rightCp),
+  /// combined in fixed-point before a single pixel snap to avoid +/-1 px rounding drift.
+  int getSpaceAdvance(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
  /// Returns the kerning adjustment between two adjacent codepoints.
  int getKerning(int fontId, uint32_t leftCp, uint32_t rightCp, EpdFontFamily::Style style) const;
  int getTextAdvanceX(int fontId, const char* text, EpdFontFamily::Style style) const;
--- a/lib/ZipFile/ZipFile.cpp
+++ b/lib/ZipFile/ZipFile.cpp
@@ -332,6 +332,7 @@ int ZipFile::fillUncompressedSizes(std::vector<SizeTarget>& targets, std::vector
  file.seek(zipDetails.centralDirOffset);

  int matched = 0;
+  const int targetCount = static_cast<int>(targets.size());
  uint32_t sig;
  char itemName[256];

@@ -372,6 +373,10 @@ int ZipFile::fillUncompressedSizes(std::vector<SizeTarget>& targets, std::vector
        }
        ++it;
      }
+
+      if (matched >= targetCount) {
+        break;
+      }
    } else {
      file.seekCur(nameLen);
    }
--- a/mod/docs/upstream-sync.md
+++ b/mod/docs/upstream-sync.md
@@ -317,9 +317,11 @@ Additionally, keep the quick-reference status table below up to date during each
 | #1185 | KOReader document hash cache | OPEN | #1286 (OPEN, OPDS filename matching) | Keep until merged upstream |
 | #1209 | Multiple OPDS servers | OPEN | #1214 (OPEN, author folders) | Keep until merged upstream |
 | #1217 | KOReader sync improvements | OPEN | #946 (OPEN, sync streamlining) | Evaluate overlap with #946 |
+| #1311 | Fix inter-word spacing rounding error | OPEN | #1168 (MERGED, fixed-point layout), #873 (MERGED, kerning) | Keep until merged upstream |
 | #1320 | JPEG resource cleanup (RAII) | OPEN | None | Keep until merged upstream |
+| #1322 | Early exit on fillUncompressedSizes | MERGED | None | DROP on next sync (now in upstream) |
 | #1325 | Settings tab label | OPEN | None | Keep until merged upstream |
-| #1329 | Reader utils refactor | OPEN (draft) | #1143 (dependent) | Keep until merged upstream |
+| #1329 | Reader utils refactor | MERGED | #1143 (dependent) | DROP on next sync (now in upstream) |

 *Last updated: 2026-03-08*

--- a/mod/prs/MERGED.md
+++ b/mod/prs/MERGED.md
@@ -11,7 +11,9 @@ Tracking document for upstream PRs ported into this mod.
 - [PR #1068](#pr-1068-correct-hyphenation-of-urls) — Correct hyphenation of URLs (Uri-Tauber)
 - [PR #1329](#pr-1329-refactor-reader-utils) — Refactor shared reader utilities (upstream)
 - [PR #1143 + #1172](#pr-1143--1172-toc-fragment-navigation--multi-spine-toc) — TOC fragment navigation + multi-spine TOC (upstream)
+- [PR #1311](#pr-1311-fix-inter-word-spacing-rounding-error) — Fix inter-word spacing rounding error (znelson)
 - [PR #1320](#pr-1320-jpeg-resource-cleanup) — RAII JPEG resource cleanup (upstream)
+- [PR #1322](#pr-1322-early-exit-on-filluncompressedsizes) — Early exit on fillUncompressedSizes (jpirnay)
 - [PR #1325](#pr-1325-settings-tab-label) — Dynamic settings tab label (upstream)

 ---
@@ -265,8 +267,8 @@ Long URLs in EPUBs could not be line-wrapped because `buildExplicitBreakInfos` r
 ## PR #1329: Refactor reader utils

 - **URL:** [https://github.com/crosspoint-reader/crosspoint-reader/pull/1329](https://github.com/crosspoint-reader/crosspoint-reader/pull/1329)
- **Author:** upstream
- **Status in upstream:** Open (draft)
+- **Author:** Uri-Tauber
+- **Status in upstream:** MERGED (commit `cd508d2`, 2026-03-08). DROP on next sync.
 - **Method:** Manual port (3 files)

 ### Context
@@ -356,3 +358,58 @@ The settings screen's "Confirm" button showed a hardcoded "Toggle" label even wh
 ### Differences from upstream PR

 - None -- clean port.
+
+---
+
+## PR #1311: Fix inter-word spacing rounding error
+
+- **URL:** [https://github.com/crosspoint-reader/crosspoint-reader/pull/1311](https://github.com/crosspoint-reader/crosspoint-reader/pull/1311)
+- **Author:** znelson
+- **Status in upstream:** Open (approved by jdk2pq, awaiting second review from osteotek)
+- **Method:** Manual port (4 files)
+
+### Context
+
+Inter-word gap widths were computed as two separately-snapped integers: `fp4::toPixel(spaceAdvance) + fp4::toPixel(kernSum)`. Because `fp4::toPixel(a) + fp4::toPixel(b)` can differ from `fp4::toPixel(a + b)` by +/-1 pixel when fractional parts straddle a rounding boundary, each inter-word space could be one pixel wider or narrower than the correct value, affecting line-break width decisions and word-position accumulation.
+
+### Changes applied
+
+1. **`lib/GfxRenderer/GfxRenderer.h`**: Replaced `getSpaceKernAdjust()` declaration with `getSpaceAdvance()`, which returns the full inter-word space advance (space glyph advance + both flanking kern values) combined in fixed-point before a single pixel snap.
+2. **`lib/GfxRenderer/GfxRenderer.cpp`**: Replaced `getSpaceKernAdjust()` implementation with `getSpaceAdvance()` that retrieves the space glyph advance (12.4 FP), adds kern(leftCp, ' ') and kern(' ', rightCp) (4.4 FP), then snaps the combined sum once via `fp4::toPixel()`. `getSpaceWidth()` is retained for the single-space-word case in `measureWordWidth`.
+3. **`lib/Epub/Epub/ParsedText.h`**: Removed `spaceWidth` parameter from `computeLineBreaks`, `computeHyphenatedLineBreaks`, and `extractLine`.
+4. **`lib/Epub/Epub/ParsedText.cpp`**: Updated all four call sites (`computeLineBreaks`, `computeHyphenatedLineBreaks`, and both gap-calculation loops in `extractLine`) to use `renderer.getSpaceAdvance()` instead of `spaceWidth + renderer.getSpaceKernAdjust()`. Removed `spaceWidth` pre-computation from `layoutAndExtractLines`.
+
+### Differences from upstream PR
+
+- **Mod's vector-based ParsedText**: The mod's `ParsedText.cpp` uses `std::vector` (from #1038 port) and has additional vectors (`forceBreakAfter`, `wordContinues`). The spacing replacement is structurally identical at each call site.
+- **Word-width cache preserved**: The mod's word-width cache (from #1027 port) is unaffected by this change since it caches `getTextAdvanceX` results, not space widths.
+
+### Notable PR discussion
+
+- jdk2pq tested on device and confirmed working.
+- The single-snap pattern matches what `getTextAdvanceX` already uses for intra-word glyph advances, making inter-word spacing consistent with word-width measurement.
+
+---
+
+## PR #1322: Early exit on fillUncompressedSizes
+
+- **URL:** [https://github.com/crosspoint-reader/crosspoint-reader/pull/1322](https://github.com/crosspoint-reader/crosspoint-reader/pull/1322)
+- **Author:** jpirnay
+- **Status in upstream:** MERGED (commit `e60ba76`, 2026-03-08). DROP on next sync.
+- **Method:** Manual port (1 file)
+
+### Context
+
+`fillUncompressedSizes` scanned the entire ZIP central directory even when all requested target entries had already been matched, wasting time on large EPUB files with many entries.
+
+### Changes applied
+
+1. **`lib/ZipFile/ZipFile.cpp`**: Added `targetCount` variable before the scan loop and a `break` after the inner match loop when `matched >= targetCount`, terminating the central-directory scan early.
+
+### Differences from upstream PR
+
+- None -- identical to upstream commit `e60ba76`.
+
+### Notable PR discussion
+
+- CodeRabbit flagged a theoretical duplicate-match overcount issue, but znelson and ngxson approved as-is since duplicate central-directory filenames are not expected in valid EPUB/ZIP files.