From 913b7e5f8b98823cacc06abdec6c0ca3aca52180 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 15:54:40 +1200 Subject: [PATCH 01/12] Avoid OOM during prewarm by loading only ligature data, not kern pairs This is particularly important when using SD fonts with many kern pairs --- lib/EpdFont/SdCardFont.cpp | 63 +++++++++++++++++++++++--------------- lib/EpdFont/SdCardFont.h | 5 +-- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/lib/EpdFont/SdCardFont.cpp b/lib/EpdFont/SdCardFont.cpp index b493cf1c..ea88e8c3 100644 --- a/lib/EpdFont/SdCardFont.cpp +++ b/lib/EpdFont/SdCardFont.cpp @@ -66,11 +66,12 @@ void SdCardFont::freeStyleMiniData(PerStyle& s) { void SdCardFont::freeStyleKernLigatureData(PerStyle& s) { delete[] s.kernLeftClasses; s.kernLeftClasses = nullptr; + s.kernClassesLoaded = false; delete[] s.kernRightClasses; s.kernRightClasses = nullptr; delete[] s.ligaturePairs; s.ligaturePairs = nullptr; - s.kernLigLoaded = false; + s.ligLoaded = false; } void SdCardFont::freeStyleMiniKern(PerStyle& s) { @@ -134,14 +135,17 @@ void SdCardFont::applyKernLigaturePointers(const PerStyle& s, EpdFontData& data) data.ligaturePairCount = s.header.ligaturePairCount; } -bool SdCardFont::loadStyleKernLigatureData(PerStyle& s) { - if (s.kernLigLoaded) return true; - bool hasKern = s.header.kernLeftEntryCount > 0; - bool hasLig = s.header.ligaturePairCount > 0; - if (!hasKern && !hasLig) { - s.kernLigLoaded = true; - return true; - } +bool SdCardFont::loadStyleKernLigatureData(PerStyle& s, bool ligatureOnly) { + // During metadata-only (layout) prewarms, skip the kern class tables: the kern + // matrix is never built at layout time so getKerning() returns 0 regardless. + // Skipping them saves ~4KB per style (~17KB total for 4 styles), preventing OOM + // on low-heap devices when long paragraphs try to grow their word vector. + const bool wantKern = !ligatureOnly && s.header.kernLeftEntryCount > 0; + const bool wantLig = s.header.ligaturePairCount > 0; + + const bool kernDone = !wantKern || s.kernClassesLoaded; + const bool ligDone = !wantLig || s.ligLoaded; + if (kernDone && ligDone) return true; FsFile file; if (!Storage.openFileForRead("SDCF", filePath_, file)) { @@ -149,7 +153,7 @@ bool SdCardFont::loadStyleKernLigatureData(PerStyle& s) { return false; } - if (hasKern) { + if (wantKern && !s.kernClassesLoaded) { // Load only the small class-lookup tables (~3KB each). The full matrix // (~36KB contiguous for Literata) is built per-page from SD in // buildMiniKernMatrix(). @@ -179,9 +183,10 @@ bool SdCardFont::loadStyleKernLigatureData(PerStyle& s) { file.close(); return false; } + s.kernClassesLoaded = true; } - if (hasLig) { + if (wantLig && !s.ligLoaded) { s.ligaturePairs = new (std::nothrow) EpdLigaturePair[s.header.ligaturePairCount]; if (!s.ligaturePairs) { LOG_ERR("SDCF", "Failed to allocate ligature pairs"); @@ -202,19 +207,21 @@ bool SdCardFont::loadStyleKernLigatureData(PerStyle& s) { file.close(); return false; } + s.ligLoaded = true; + + // Make ligatures visible to the stub (used when no mini data built yet). + // Kern stays nullptr on the stub — it is only wired in miniData via + // applyKernLigaturePointers() after buildMiniKernMatrix() runs. + s.stubData.ligaturePairs = s.ligaturePairs; + s.stubData.ligaturePairCount = s.header.ligaturePairCount; } file.close(); - s.kernLigLoaded = true; - - // Make ligatures visible to the stub (used when no mini data built yet). - // Kern stays nullptr on the stub — it is only wired in miniData via - // applyKernLigaturePointers() after buildMiniKernMatrix() runs. - s.stubData.ligaturePairs = s.ligaturePairs; - s.stubData.ligaturePairCount = s.header.ligaturePairCount; - - LOG_DBG("SDCF", "Kern classes + lig loaded: kernL=%u, kernR=%u, ligs=%u", s.header.kernLeftEntryCount, - s.header.kernRightEntryCount, s.header.ligaturePairCount); + LOG_DBG("SDCF", "Kern/lig loaded: kernL=%u kernR=%u ligs=%u ligOnly=%d", + s.kernClassesLoaded ? s.header.kernLeftEntryCount : 0u, + s.kernClassesLoaded ? s.header.kernRightEntryCount : 0u, + s.ligLoaded ? s.header.ligaturePairCount : 0u, + ligatureOnly); return true; } @@ -653,7 +660,7 @@ int SdCardFont::prewarm(const char* utf8Text, uint8_t styleMask, bool metadataOn if (!(styleMask & (1 << si)) || !styles_[si].present) continue; auto& s = styles_[si]; - loadStyleKernLigatureData(s); + loadStyleKernLigatureData(s, /*ligatureOnly=*/true); if (s.ligaturePairs && s.header.ligaturePairCount > 0) { for (uint8_t li = 0; li < s.header.ligaturePairCount && cpCount < MAX_PAGE_GLYPHS; li++) { uint32_t leftCp = s.ligaturePairs[li].pair >> 16; @@ -1143,7 +1150,7 @@ int SdCardFont::prewarmStyle(uint8_t styleIdx, const uint32_t* codepoints, uint3 kernLigOk = buildMiniKernMatrix(s, codepoints, cpCount); } } else if (loadKernLigatureData) { - loadStyleKernLigatureData(s); + loadStyleKernLigatureData(s, /*ligatureOnly=*/true); // Don't set kernLigOk → mini kern matrix stays null on miniData, but // ligatures are still resident on stubData (set in loadStyleKernLigatureData). } @@ -1161,7 +1168,7 @@ int SdCardFont::prewarmStyle(uint8_t styleIdx, const uint32_t* codepoints, uint3 if (kernLigOk) { // Full prewarm: wire mini kern matrix + class tables + ligatures. applyKernLigaturePointers(s, s.miniData); - } else if (loadKernLigatureData && s.kernLigLoaded) { + } else if (loadKernLigatureData && s.ligLoaded) { // Layout-only prewarm: wire ligatures so applyLigatures() works (e.g. "fi" // measures correctly). Skip the kern matrix — getKerning() returns 0 // cleanly when kernMatrix is null. Per-pair kern is applied at render time. @@ -1173,6 +1180,8 @@ int SdCardFont::prewarmStyle(uint8_t styleIdx, const uint32_t* codepoints, uint3 s.epdFont.data = &s.miniData; s.miniMode = metadataOnly ? PerStyle::MiniMode::METADATA : PerStyle::MiniMode::FULL; + LOG_DBG("SDCF", "prewarmStyle %u: mode→%s glyphs=%u bitmap=%p", + styleIdx, metadataOnly ? "METADATA" : "FULL", validCount, s.miniBitmap); // Accumulate stats stats_.sdReadTimeMs += sdTime; @@ -1236,6 +1245,10 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) { const auto& s = self->styles_[styleIdx]; if (!s.fullIntervals) return nullptr; + // Diagnostic: log first miss per codepoint+style to show why it bypassed prewarm + LOG_DBG("SDCF", "onGlyphMiss: U+%04X style %u miniMode=%u miniIntervals=%u bitmap=%p", + codepoint, styleIdx, (uint8_t)s.miniMode, s.miniIntervalCount, s.miniBitmap); + // Check overflow cache first (matching both codepoint and style) for (uint32_t i = 0; i < self->overflowCount_; i++) { if (self->overflow_[i].codepoint == codepoint && self->overflow_[i].styleIdx == styleIdx) { @@ -1309,6 +1322,8 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) { // All reads succeeded — commit to slot (evict old entry if at capacity) if (wasAtCapacity) { + LOG_DBG("SDCF", "Overflow: evicting U+%04X style %u from slot %u", + self->overflow_[slot].codepoint, self->overflow_[slot].styleIdx, slot); delete[] self->overflow_[slot].bitmap; } self->overflow_[slot].glyph = tempGlyph; diff --git a/lib/EpdFont/SdCardFont.h b/lib/EpdFont/SdCardFont.h index c7671a0f..597cbf06 100644 --- a/lib/EpdFont/SdCardFont.h +++ b/lib/EpdFont/SdCardFont.h @@ -118,7 +118,8 @@ class SdCardFont { EpdKernClassEntry* kernLeftClasses = nullptr; EpdKernClassEntry* kernRightClasses = nullptr; EpdLigaturePair* ligaturePairs = nullptr; - bool kernLigLoaded = false; + bool ligLoaded = false; ///< ligaturePairs resident + bool kernClassesLoaded = false; ///< kernLeft/RightClasses resident (skipped during metadata-only prewarm) // Stub EpdFontData returned when not prewarmed EpdFontData stubData{}; @@ -204,7 +205,7 @@ class SdCardFont { void freeStyleAll(PerStyle& s); void freeStyleKernLigatureData(PerStyle& s); void freeStyleMiniKern(PerStyle& s); - bool loadStyleKernLigatureData(PerStyle& s); + bool loadStyleKernLigatureData(PerStyle& s, bool ligatureOnly = false); bool buildMiniKernMatrix(PerStyle& s, const uint32_t* codepoints, uint32_t cpCount); void applyKernLigaturePointers(const PerStyle& s, EpdFontData& data) const; void applyGlyphMissCallback(uint8_t styleIdx); From 5928f32364543ee29ddac83d4175aa4b2eb16032 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 16:01:45 +1200 Subject: [PATCH 02/12] Avoid OOM diring HTML parsing by reducing max block size and freeing unneeded buffers --- lib/Epub/Epub/ParsedText.cpp | 9 ++++++++- lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp | 11 ++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 61a6a210..bc869102 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -355,12 +355,19 @@ void ParsedText::layoutAndExtractLines( } } - // Remove consumed words so size() reflects only remaining words + // Remove consumed words so size() reflects only remaining words, then + // release excess capacity. Without shrink_to_fit the vector retains a + // large allocation from before the flush; the next paragraph fills it + // back up and eventually needs an even larger contiguous realloc. if (lineCount > 0) { const size_t consumed = lineBreakIndices[lineCount - 1]; words.erase(words.begin(), words.begin() + consumed); wordStyles.erase(wordStyles.begin(), wordStyles.begin() + consumed); wordContinues.erase(wordContinues.begin(), wordContinues.begin() + consumed); + words.shrink_to_fit(); + wordStyles.shrink_to_fit(); + wordContinues.shrink_to_fit(); + isContinuation_ = !includeLastLine; } } diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index bfa5ad9a..dfe7d664 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -1207,11 +1207,12 @@ void XMLCALL ChapterHtmlSlimParser::characterData(void* userData, const XML_Char self->partWordBuffer[self->partWordBufferIndex++] = s[i]; } - // If we have > 750 words buffered up, perform the layout and consume out all but the last line - // There should be enough here to build out 1-2 full pages and doing this will free up a lot of - // memory. - // Spotted when reading Intermezzo, there are some really long text blocks in there. - if (self->currentTextBlock->size() > 750) { + // Flush when words approach the doubling boundary that would require a + // large contiguous realloc. 96 fires before capacity reaches 128 + // (the next doubling after 64), keeping the realloc below 1.5KB and + // releasing excess capacity via shrink_to_fit in layoutAndExtractLines. + // The original 750-word threshold was too late for low-heap devices. + if (self->currentTextBlock->size() > 96) { LOG_DBG("EHP", "Text block too long, splitting into multiple pages"); const int horizontalInset = self->currentTextBlock->getBlockStyle().totalHorizontalInset(); const uint16_t effectiveWidth = (horizontalInset < self->viewportWidth) From 40c3ecbada054278b376228e52a7ca20236d60c6 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 16:05:30 +1200 Subject: [PATCH 03/12] When a large paragraph is split across blocks, don't treat the continuation as a new paragraph --- lib/Epub/Epub/ParsedText.cpp | 10 +++++++--- lib/Epub/Epub/ParsedText.h | 2 ++ lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp | 16 ++++++++++------ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index bc869102..0987aa16 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -215,9 +215,13 @@ void ParsedText::layoutAndExtractLines( } // Apply fixed transforms before any per-line layout work. - applyParagraphIndent(); - if (bionicReadingEnabled) { - applyBionicReadingTransform(); + // Skip on continuation flushes: the words are mid-paragraph and have + // already been transformed by the initial layoutAndExtractLines call. + if (!isContinuation_) { + applyParagraphIndent(); + if (bionicReadingEnabled) { + applyBionicReadingTransform(); + } } // Ensure SD card font glyph metrics are loaded before measuring word widths. diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 2d94ed09..7b19db4c 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -27,6 +27,7 @@ class ParsedText { bool extraParagraphSpacing; bool hyphenationEnabled; bool bionicReadingEnabled; + bool isContinuation_ = false; ///< true after an intermediate flush; suppresses re-applying indent/transforms void applyParagraphIndent(); void applyBionicReadingTransform(); @@ -74,6 +75,7 @@ class ParsedText { BlockStyle& getBlockStyle() { return blockStyle; } size_t size() const { return words.size(); } bool isEmpty() const { return words.empty(); } + bool isContinuation() const { return isContinuation_; } void layoutAndExtractLines( const GfxRenderer& renderer, int fontId, uint16_t viewportWidth, const std::function, bool, bool)>& processLine, diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index dfe7d664..231f30fa 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -1592,13 +1592,17 @@ void ChapterHtmlSlimParser::makePages() { const int lineHeight = renderer.getLineHeight(fontId) * lineCompression; - // Apply top spacing before the paragraph (stored in pixels) + // Apply top spacing before the paragraph — skip for continuation fragments + // (words left over after an intermediate flush): the top margin was already + // applied before the first set of lines from this logical paragraph. const BlockStyle& blockStyle = currentTextBlock->getBlockStyle(); - if (blockStyle.marginTop > 0) { - currentPageNextY += blockStyle.marginTop; - } - if (blockStyle.paddingTop > 0) { - currentPageNextY += blockStyle.paddingTop; + if (!currentTextBlock->isContinuation()) { + if (blockStyle.marginTop > 0) { + currentPageNextY += blockStyle.marginTop; + } + if (blockStyle.paddingTop > 0) { + currentPageNextY += blockStyle.paddingTop; + } } // Calculate effective width accounting for horizontal margins/padding From 8152204985548f44491a96db105a2c825202ba3b Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 16:40:23 +1200 Subject: [PATCH 04/12] Fix formatting This is particularly important when using SD fonts with many kern pairs --- lib/EpdFont/SdCardFont.cpp | 15 +++++++-------- lib/EpdFont/SdCardFont.h | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/EpdFont/SdCardFont.cpp b/lib/EpdFont/SdCardFont.cpp index ea88e8c3..a07660b4 100644 --- a/lib/EpdFont/SdCardFont.cpp +++ b/lib/EpdFont/SdCardFont.cpp @@ -219,8 +219,7 @@ bool SdCardFont::loadStyleKernLigatureData(PerStyle& s, bool ligatureOnly) { file.close(); LOG_DBG("SDCF", "Kern/lig loaded: kernL=%u kernR=%u ligs=%u ligOnly=%d", s.kernClassesLoaded ? s.header.kernLeftEntryCount : 0u, - s.kernClassesLoaded ? s.header.kernRightEntryCount : 0u, - s.ligLoaded ? s.header.ligaturePairCount : 0u, + s.kernClassesLoaded ? s.header.kernRightEntryCount : 0u, s.ligLoaded ? s.header.ligaturePairCount : 0u, ligatureOnly); return true; } @@ -1180,8 +1179,8 @@ int SdCardFont::prewarmStyle(uint8_t styleIdx, const uint32_t* codepoints, uint3 s.epdFont.data = &s.miniData; s.miniMode = metadataOnly ? PerStyle::MiniMode::METADATA : PerStyle::MiniMode::FULL; - LOG_DBG("SDCF", "prewarmStyle %u: mode→%s glyphs=%u bitmap=%p", - styleIdx, metadataOnly ? "METADATA" : "FULL", validCount, s.miniBitmap); + LOG_DBG("SDCF", "prewarmStyle %u: mode→%s glyphs=%u bitmap=%p", styleIdx, metadataOnly ? "METADATA" : "FULL", + validCount, s.miniBitmap); // Accumulate stats stats_.sdReadTimeMs += sdTime; @@ -1246,8 +1245,8 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) { if (!s.fullIntervals) return nullptr; // Diagnostic: log first miss per codepoint+style to show why it bypassed prewarm - LOG_DBG("SDCF", "onGlyphMiss: U+%04X style %u miniMode=%u miniIntervals=%u bitmap=%p", - codepoint, styleIdx, (uint8_t)s.miniMode, s.miniIntervalCount, s.miniBitmap); + LOG_DBG("SDCF", "onGlyphMiss: U+%04X style %u miniMode=%u miniIntervals=%u bitmap=%p", codepoint, styleIdx, + (uint8_t)s.miniMode, s.miniIntervalCount, s.miniBitmap); // Check overflow cache first (matching both codepoint and style) for (uint32_t i = 0; i < self->overflowCount_; i++) { @@ -1322,8 +1321,8 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) { // All reads succeeded — commit to slot (evict old entry if at capacity) if (wasAtCapacity) { - LOG_DBG("SDCF", "Overflow: evicting U+%04X style %u from slot %u", - self->overflow_[slot].codepoint, self->overflow_[slot].styleIdx, slot); + LOG_DBG("SDCF", "Overflow: evicting U+%04X style %u from slot %u", self->overflow_[slot].codepoint, + self->overflow_[slot].styleIdx, slot); delete[] self->overflow_[slot].bitmap; } self->overflow_[slot].glyph = tempGlyph; diff --git a/lib/EpdFont/SdCardFont.h b/lib/EpdFont/SdCardFont.h index 597cbf06..6776ce34 100644 --- a/lib/EpdFont/SdCardFont.h +++ b/lib/EpdFont/SdCardFont.h @@ -118,8 +118,8 @@ class SdCardFont { EpdKernClassEntry* kernLeftClasses = nullptr; EpdKernClassEntry* kernRightClasses = nullptr; EpdLigaturePair* ligaturePairs = nullptr; - bool ligLoaded = false; ///< ligaturePairs resident - bool kernClassesLoaded = false; ///< kernLeft/RightClasses resident (skipped during metadata-only prewarm) + bool ligLoaded = false; ///< ligaturePairs resident + bool kernClassesLoaded = false; ///< kernLeft/RightClasses resident (skipped during metadata-only prewarm) // Stub EpdFontData returned when not prewarmed EpdFontData stubData{}; From c04ec25dc78c257c66e3c71fba0f6712305be798 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 16:50:49 +1200 Subject: [PATCH 05/12] Address review comment: also suppress CSS styling on continuation --- lib/Epub/Epub/ParsedText.cpp | 70 ++++++++++++++---------------------- lib/Epub/Epub/ParsedText.h | 11 +++--- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 0987aa16..d265f9d6 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -246,6 +246,15 @@ void ParsedText::layoutAndExtractLines( } const int pageWidth = viewportWidth; + + // Compute firstLineIndent once here so all layout helpers use the same value. + // On a continuation flush the remaining words are mid-paragraph, so no indent. + const int firstLineIndent = + !isContinuation_ && blockStyle.textIndentDefined && + (blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left) + ? std::min(std::max(static_cast(blockStyle.textIndent), -(pageWidth - 1)), pageWidth - 1) + : 0; + auto wordWidths = calculateWordWidths(renderer, fontId); std::vector lineBreakIndices; @@ -256,9 +265,9 @@ void ParsedText::layoutAndExtractLines( // Use greedy layout that can split words mid-loop when a hyphenated prefix fits. lineBreakIndices = computeHyphenatedLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues, lineEndsWithHyphenatedWord, - splitPrefixWordIndexes, splitInsertedHyphen); + splitPrefixWordIndexes, splitInsertedHyphen, firstLineIndent); } else { - lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues); + lineBreakIndices = computeLineBreaks(renderer, fontId, pageWidth, wordWidths, wordContinues, firstLineIndent); lineEndsWithHyphenatedWord.assign(lineBreakIndices.size(), false); splitPrefixWordIndexes.assign(lineBreakIndices.size(), -1); splitInsertedHyphen.assign(lineBreakIndices.size(), false); @@ -268,7 +277,7 @@ void ParsedText::layoutAndExtractLines( for (size_t i = 0; i < lineCount; ++i) { const bool lineEndedWithHyphenation = i < lineEndsWithHyphenatedWord.size() ? lineEndsWithHyphenatedWord[i] : false; const auto result = extractLine(i, pageWidth, wordWidths, wordContinues, lineBreakIndices, processLine, renderer, - fontId, lineEndedWithHyphenation, false); + fontId, lineEndedWithHyphenation, false, firstLineIndent); if (result == LineProcessResult::RetryWithoutHyphenation && lineEndedWithHyphenation) { const size_t lineStart = i > 0 ? lineBreakIndices[i - 1] : 0; @@ -314,7 +323,8 @@ void ParsedText::layoutAndExtractLines( // Keep previous lines fixed; recompute only this specific line without hyphenation. // Suppression is intentionally line-local. const size_t retryBreak = - computeSingleLineBreakNoHyphen(renderer, fontId, pageWidth, wordWidths, wordContinues, lineStart); + computeSingleLineBreakNoHyphen(renderer, fontId, pageWidth, wordWidths, wordContinues, lineStart, + firstLineIndent); lineBreakIndices.resize(i + 1); lineEndsWithHyphenatedWord.resize(i + 1); @@ -334,7 +344,7 @@ void ParsedText::layoutAndExtractLines( LOG_DBG("PTX", "Rerendering line %u with hyphenation suppressed, retry attempt: %s", static_cast(i), retryPreview.c_str()); extractLine(i, pageWidth, wordWidths, wordContinues, lineBreakIndices, processLine, renderer, fontId, false, - true); + true, firstLineIndent); // Resume regular hyphenation from the first word after the retried line. const size_t resumeIndex = lineBreakIndices[i]; @@ -387,20 +397,12 @@ std::vector ParsedText::calculateWordWidths(const GfxRenderer& rendere } std::vector ParsedText::computeLineBreaks(const GfxRenderer& renderer, const int fontId, const int pageWidth, - std::vector& wordWidths, std::vector& continuesVec) { + std::vector& wordWidths, std::vector& continuesVec, + const int firstLineIndent) { if (words.empty()) { return {}; } - // Calculate first line indent (only for left/justified text). - // Explicit CSS text-indent always applies — author intent overrides the extraParagraphSpacing - // toggle. Only the implicit EmSpace fallback in applyParagraphIndent() is gated on it. - const int firstLineIndent = - blockStyle.textIndentDefined && - (blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left) - ? std::min(std::max(static_cast(blockStyle.textIndent), -(pageWidth - 1)), pageWidth - 1) - : 0; - // Ensure any word that would overflow even as the first entry on a line is split using fallback hyphenation. for (size_t i = 0; i < wordWidths.size(); ++i) { // First word needs to fit in reduced width if there's an indent @@ -522,18 +524,13 @@ std::vector ParsedText::computeLineBreaks(const GfxRenderer& renderer, c size_t ParsedText::computeSingleLineBreakNoHyphen(const GfxRenderer& renderer, const int fontId, const int pageWidth, const std::vector& wordWidths, const std::vector& continuesVec, - const size_t lineStartIndex) const { + const size_t lineStartIndex, const int firstLineIndent) const { // One-line non-hyphenating breaker used by the page-boundary retry path. if (lineStartIndex >= wordWidths.size()) { return lineStartIndex; } - const int firstLineIndent = - lineStartIndex == 0 && blockStyle.textIndentDefined && - (blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left) - ? std::min(std::max(static_cast(blockStyle.textIndent), -(pageWidth - 1)), pageWidth - 1) - : 0; - const int effectivePageWidth = pageWidth - firstLineIndent; + const int effectivePageWidth = pageWidth - (lineStartIndex == 0 ? firstLineIndent : 0); size_t currentIndex = lineStartIndex; int lineWidth = 0; @@ -672,15 +669,8 @@ std::vector ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& r std::vector& continuesVec, std::vector& lineEndsWithHyphenatedWord, std::vector& splitPrefixWordIndexes, - std::vector& splitInsertedHyphen) { - // Calculate first line indent (only for left/justified text). - // Explicit CSS text-indent always applies — author intent overrides the extraParagraphSpacing - // toggle. Only the implicit EmSpace fallback in applyParagraphIndent() is gated on it. - const int firstLineIndent = - blockStyle.textIndentDefined && - (blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left) - ? std::min(std::max(static_cast(blockStyle.textIndent), -(pageWidth - 1)), pageWidth - 1) - : 0; + std::vector& splitInsertedHyphen, + const int firstLineIndent) { // Pre-compute inter-word gaps to avoid repeated codepoint scanning and renderer // calls in the inner loop. When hyphenateWordAtIndex inserts a new word, we insert @@ -960,20 +950,14 @@ ParsedText::LineProcessResult ParsedText::extractLine( const std::vector& continuesVec, const std::vector& lineBreakIndices, const std::function, bool, bool)>& processLine, const GfxRenderer& renderer, const int fontId, const bool lineEndsWithHyphenatedWord, - const bool suppressHyphenationRetry) { + const bool suppressHyphenationRetry, const int firstLineIndent) { const size_t lineBreak = lineBreakIndices[breakIndex]; const size_t lastBreakAt = breakIndex > 0 ? lineBreakIndices[breakIndex - 1] : 0; const size_t lineWordCount = lineBreak - lastBreakAt; - // Calculate first line indent (only for left/justified text). - // Explicit CSS text-indent always applies — author intent overrides the extraParagraphSpacing - // toggle. Only the implicit EmSpace fallback in applyParagraphIndent() is gated on it. - const bool isFirstLine = breakIndex == 0; - const int firstLineIndent = - isFirstLine && blockStyle.textIndentDefined && - (blockStyle.alignment == CssTextAlign::Justify || blockStyle.alignment == CssTextAlign::Left) - ? std::min(std::max(static_cast(blockStyle.textIndent), -(pageWidth - 1)), pageWidth - 1) - : 0; + // Apply indent only to line 0 of the layout pass; firstLineIndent is already + // 0 for continuation flushes (computed once in layoutAndExtractLines). + const int lineIndent = (breakIndex == 0) ? firstLineIndent : 0; // Calculate total word width for this line, count actual word gaps, // and accumulate total natural gap widths (including space kerning adjustments). @@ -1000,7 +984,7 @@ ParsedText::LineProcessResult ParsedText::extractLine( } // Calculate spacing (account for indent reducing effective page width on first line) - const int effectivePageWidth = pageWidth - firstLineIndent; + const int effectivePageWidth = pageWidth - lineIndent; // A line is only truly last when it consumes all paragraph words. // During single-line retry we may temporarily pass a truncated break vector, // so relying only on breakIndex would incorrectly disable justification. @@ -1014,7 +998,7 @@ ParsedText::LineProcessResult ParsedText::extractLine( // Calculate initial x position (first line starts at indent for left/justified text; // may be negative for hanging indents, e.g. margin-left:3em; text-indent:-1em). - auto xpos = static_cast(firstLineIndent); + auto xpos = static_cast(lineIndent); if (blockStyle.alignment == CssTextAlign::Right) { xpos = effectivePageWidth - lineWordWidthSum - totalNaturalGaps; } else if (blockStyle.alignment == CssTextAlign::Center) { diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 7b19db4c..2112beec 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -32,12 +32,14 @@ class ParsedText { void applyParagraphIndent(); void applyBionicReadingTransform(); std::vector computeLineBreaks(const GfxRenderer& renderer, int fontId, int pageWidth, - std::vector& wordWidths, std::vector& continuesVec); + std::vector& wordWidths, std::vector& continuesVec, + int firstLineIndent); std::vector computeHyphenatedLineBreaks(const GfxRenderer& renderer, int fontId, int pageWidth, std::vector& wordWidths, std::vector& continuesVec, std::vector& lineEndsWithHyphenatedWord, std::vector& splitPrefixWordIndexes, - std::vector& splitInsertedHyphen); + std::vector& splitInsertedHyphen, + int firstLineIndent); // Recompute hyphenated breaks for a suffix that starts at startIndex. // Used after a single-line retry so later lines keep normal hyphenation. std::vector computeHyphenatedLineBreaksFromIndex(const GfxRenderer& renderer, int fontId, int pageWidth, @@ -50,7 +52,7 @@ class ParsedText { // Used only for the page-boundary retry line. size_t computeSingleLineBreakNoHyphen(const GfxRenderer& renderer, int fontId, int pageWidth, const std::vector& wordWidths, const std::vector& continuesVec, - size_t lineStartIndex) const; + size_t lineStartIndex, int firstLineIndent) const; bool hyphenateWordAtIndex(size_t wordIndex, int availableWidth, const GfxRenderer& renderer, int fontId, std::vector& wordWidths, bool allowFallbackBreaks, bool* outInsertedHyphen = nullptr); @@ -58,7 +60,8 @@ class ParsedText { size_t breakIndex, int pageWidth, const std::vector& wordWidths, const std::vector& continuesVec, const std::vector& lineBreakIndices, const std::function, bool, bool)>& processLine, - const GfxRenderer& renderer, int fontId, bool lineEndsWithHyphenatedWord, bool suppressHyphenationRetry); + const GfxRenderer& renderer, int fontId, bool lineEndsWithHyphenatedWord, bool suppressHyphenationRetry, + int firstLineIndent); std::vector calculateWordWidths(const GfxRenderer& renderer, int fontId); public: From 0a5fee283968ad3a02fb80fa4b439966fbd2bf43 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 16:55:37 +1200 Subject: [PATCH 06/12] Address review comment: fix bionic reading on continuation --- lib/Epub/Epub/ParsedText.cpp | 67 ++++++++++++++++++++++-------------- lib/Epub/Epub/ParsedText.h | 3 +- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index d265f9d6..75553560 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -215,13 +215,16 @@ void ParsedText::layoutAndExtractLines( } // Apply fixed transforms before any per-line layout work. - // Skip on continuation flushes: the words are mid-paragraph and have - // already been transformed by the initial layoutAndExtractLines call. + // Paragraph indent only applies to the first layout pass; skip on continuations. if (!isContinuation_) { applyParagraphIndent(); - if (bionicReadingEnabled) { - applyBionicReadingTransform(); - } + } + // Bionic transform is incremental: applyBionicReadingTransform() is a no-op + // for already-transformed words (bionicTransformedUpTo_ == words.size()) and + // only processes raw words appended since the last flush, so it is always safe + // to call regardless of isContinuation_. + if (bionicReadingEnabled) { + applyBionicReadingTransform(); } // Ensure SD card font glyph metrics are loaded before measuring word widths. @@ -382,6 +385,9 @@ void ParsedText::layoutAndExtractLines( wordStyles.shrink_to_fit(); wordContinues.shrink_to_fit(); isContinuation_ = !includeLastLine; + // All remaining words were already transformed before the flush; reset the + // watermark so that words appended by addWord() are processed next time. + bionicTransformedUpTo_ = words.size(); } } @@ -585,18 +591,23 @@ void ParsedText::applyParagraphIndent() { } void ParsedText::applyBionicReadingTransform() { - if (words.empty()) { + // Only transform words that haven't been processed yet. On a fresh block + // bionicTransformedUpTo_ == 0 so all words are processed. After an + // intermediate flush, only the new raw words appended since the last flush + // (indices bionicTransformedUpTo_..words.size()-1) need transformation. + if (words.empty() || bionicTransformedUpTo_ >= words.size()) { return; } - std::vector transformedWords; - std::vector transformedStyles; - std::vector transformedContinues; - transformedWords.reserve(words.size() * 2); - transformedStyles.reserve(wordStyles.size() * 2); - transformedContinues.reserve(wordContinues.size() * 2); + const size_t suffixStart = bionicTransformedUpTo_; + std::vector transformedSuffix; + std::vector transformedSuffixStyles; + std::vector transformedSuffixContinues; + transformedSuffix.reserve((words.size() - suffixStart) * 2); + transformedSuffixStyles.reserve(transformedSuffix.capacity()); + transformedSuffixContinues.reserve(transformedSuffix.capacity()); - for (size_t i = 0; i < words.size(); ++i) { + for (size_t i = suffixStart; i < words.size(); ++i) { std::string source = std::move(words[i]); const auto originalStyle = wordStyles[i]; const bool originalAttachToPrevious = wordContinues[i]; @@ -638,29 +649,35 @@ void ParsedText::applyBionicReadingTransform() { std::string suffix(reinterpret_cast(prefixEnd), token.size() - prefixByteCount); token.resize(prefixByteCount); const auto boldStyle = static_cast(originalStyle | EpdFontFamily::BOLD); - transformedWords.push_back(std::move(token)); - transformedStyles.push_back(boldStyle); - transformedContinues.push_back(attachToPrevious); + transformedSuffix.push_back(std::move(token)); + transformedSuffixStyles.push_back(boldStyle); + transformedSuffixContinues.push_back(attachToPrevious); - transformedWords.push_back(std::move(suffix)); - transformedStyles.push_back(originalStyle); - transformedContinues.push_back(true); + transformedSuffix.push_back(std::move(suffix)); + transformedSuffixStyles.push_back(originalStyle); + transformedSuffixContinues.push_back(true); attachToPrevious = true; continue; } } } - transformedWords.push_back(std::move(token)); - transformedStyles.push_back(originalStyle); - transformedContinues.push_back(attachToPrevious); + transformedSuffix.push_back(std::move(token)); + transformedSuffixStyles.push_back(originalStyle); + transformedSuffixContinues.push_back(attachToPrevious); attachToPrevious = true; } } - words = std::move(transformedWords); - wordStyles = std::move(transformedStyles); - wordContinues = std::move(transformedContinues); + // Replace the (now move-emptied) suffix with the transformed version. + words.resize(suffixStart); + wordStyles.resize(suffixStart); + wordContinues.resize(suffixStart); + words.insert(words.end(), std::make_move_iterator(transformedSuffix.begin()), + std::make_move_iterator(transformedSuffix.end())); + wordStyles.insert(wordStyles.end(), transformedSuffixStyles.begin(), transformedSuffixStyles.end()); + wordContinues.insert(wordContinues.end(), transformedSuffixContinues.begin(), transformedSuffixContinues.end()); + bionicTransformedUpTo_ = words.size(); } // Builds break indices while opportunistically splitting the word that would overflow the current line. diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 2112beec..486054ec 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -27,7 +27,8 @@ class ParsedText { bool extraParagraphSpacing; bool hyphenationEnabled; bool bionicReadingEnabled; - bool isContinuation_ = false; ///< true after an intermediate flush; suppresses re-applying indent/transforms + bool isContinuation_ = false; ///< true after an intermediate flush; suppresses re-applying paragraph indent + size_t bionicTransformedUpTo_ = 0; ///< words[0..bionicTransformedUpTo_) have already been bionic-transformed void applyParagraphIndent(); void applyBionicReadingTransform(); From 3f4db13c29f5e320540d0d53c4c1fe8063d45df5 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 17:05:19 +1200 Subject: [PATCH 07/12] Fix formatting --- lib/Epub/Epub/ParsedText.cpp | 21 ++++++++------------- lib/Epub/Epub/ParsedText.h | 5 ++--- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 75553560..c1abcaab 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -325,9 +325,8 @@ void ParsedText::layoutAndExtractLines( // Keep previous lines fixed; recompute only this specific line without hyphenation. // Suppression is intentionally line-local. - const size_t retryBreak = - computeSingleLineBreakNoHyphen(renderer, fontId, pageWidth, wordWidths, wordContinues, lineStart, - firstLineIndent); + const size_t retryBreak = computeSingleLineBreakNoHyphen(renderer, fontId, pageWidth, wordWidths, wordContinues, + lineStart, firstLineIndent); lineBreakIndices.resize(i + 1); lineEndsWithHyphenatedWord.resize(i + 1); @@ -529,8 +528,8 @@ std::vector ParsedText::computeLineBreaks(const GfxRenderer& renderer, c size_t ParsedText::computeSingleLineBreakNoHyphen(const GfxRenderer& renderer, const int fontId, const int pageWidth, const std::vector& wordWidths, - const std::vector& continuesVec, - const size_t lineStartIndex, const int firstLineIndent) const { + const std::vector& continuesVec, const size_t lineStartIndex, + const int firstLineIndent) const { // One-line non-hyphenating breaker used by the page-boundary retry path. if (lineStartIndex >= wordWidths.size()) { return lineStartIndex; @@ -681,14 +680,10 @@ void ParsedText::applyBionicReadingTransform() { } // Builds break indices while opportunistically splitting the word that would overflow the current line. -std::vector ParsedText::computeHyphenatedLineBreaks(const GfxRenderer& renderer, const int fontId, - const int pageWidth, std::vector& wordWidths, - std::vector& continuesVec, - std::vector& lineEndsWithHyphenatedWord, - std::vector& splitPrefixWordIndexes, - std::vector& splitInsertedHyphen, - const int firstLineIndent) { - +std::vector ParsedText::computeHyphenatedLineBreaks( + const GfxRenderer& renderer, const int fontId, const int pageWidth, std::vector& wordWidths, + std::vector& continuesVec, std::vector& lineEndsWithHyphenatedWord, + std::vector& splitPrefixWordIndexes, std::vector& splitInsertedHyphen, const int firstLineIndent) { // Pre-compute inter-word gaps to avoid repeated codepoint scanning and renderer // calls in the inner loop. When hyphenateWordAtIndex inserts a new word, we insert // a placeholder gap (0) at that position to keep the vector in sync; the remainder diff --git a/lib/Epub/Epub/ParsedText.h b/lib/Epub/Epub/ParsedText.h index 486054ec..f60388bb 100644 --- a/lib/Epub/Epub/ParsedText.h +++ b/lib/Epub/Epub/ParsedText.h @@ -27,7 +27,7 @@ class ParsedText { bool extraParagraphSpacing; bool hyphenationEnabled; bool bionicReadingEnabled; - bool isContinuation_ = false; ///< true after an intermediate flush; suppresses re-applying paragraph indent + bool isContinuation_ = false; ///< true after an intermediate flush; suppresses re-applying paragraph indent size_t bionicTransformedUpTo_ = 0; ///< words[0..bionicTransformedUpTo_) have already been bionic-transformed void applyParagraphIndent(); @@ -39,8 +39,7 @@ class ParsedText { std::vector& wordWidths, std::vector& continuesVec, std::vector& lineEndsWithHyphenatedWord, std::vector& splitPrefixWordIndexes, - std::vector& splitInsertedHyphen, - int firstLineIndent); + std::vector& splitInsertedHyphen, int firstLineIndent); // Recompute hyphenated breaks for a suffix that starts at startIndex. // Used after a single-line retry so later lines keep normal hyphenation. std::vector computeHyphenatedLineBreaksFromIndex(const GfxRenderer& renderer, int fontId, int pageWidth, From 13a76597d4ace6e1f10511e15e4c59e760117400 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 19:38:38 +1200 Subject: [PATCH 08/12] Address review comment: check block size as soon as each word is added. --- .../Epub/parsers/ChapterHtmlSlimParser.cpp | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index 231f30fa..cd83b5c5 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -189,6 +189,20 @@ void ChapterHtmlSlimParser::flushPartWordBuffer() { currentTextBlock->addWord(partWordBuffer, fontStyle, false, nextWordContinues); partWordBufferIndex = 0; nextWordContinues = false; + + if (currentTextBlock->size() > 96) { + LOG_DBG("EHP", "Text block too long, splitting into multiple pages"); + const int horizontalInset = currentTextBlock->getBlockStyle().totalHorizontalInset(); + const uint16_t effectiveWidth = + (horizontalInset < viewportWidth) ? static_cast(viewportWidth - horizontalInset) : viewportWidth; + currentTextBlock->layoutAndExtractLines( + renderer, fontId, effectiveWidth, + [this](const std::shared_ptr& textBlock, const bool lineEndsWithHyphenatedWord, + const bool suppressHyphenationRetry) { + return addLineToPage(textBlock, lineEndsWithHyphenatedWord, suppressHyphenationRetry); + }, + false); + } } // Emit the current page, keeping paragraphLutPerPage and completedPageCount in lockstep. @@ -1206,26 +1220,6 @@ void XMLCALL ChapterHtmlSlimParser::characterData(void* userData, const XML_Char self->partWordBuffer[self->partWordBufferIndex++] = s[i]; } - - // Flush when words approach the doubling boundary that would require a - // large contiguous realloc. 96 fires before capacity reaches 128 - // (the next doubling after 64), keeping the realloc below 1.5KB and - // releasing excess capacity via shrink_to_fit in layoutAndExtractLines. - // The original 750-word threshold was too late for low-heap devices. - if (self->currentTextBlock->size() > 96) { - LOG_DBG("EHP", "Text block too long, splitting into multiple pages"); - const int horizontalInset = self->currentTextBlock->getBlockStyle().totalHorizontalInset(); - const uint16_t effectiveWidth = (horizontalInset < self->viewportWidth) - ? static_cast(self->viewportWidth - horizontalInset) - : self->viewportWidth; - self->currentTextBlock->layoutAndExtractLines( - self->renderer, self->fontId, effectiveWidth, - [self](const std::shared_ptr& textBlock, const bool lineEndsWithHyphenatedWord, - const bool suppressHyphenationRetry) { - return self->addLineToPage(textBlock, lineEndsWithHyphenatedWord, suppressHyphenationRetry); - }, - false); - } } void XMLCALL ChapterHtmlSlimParser::defaultHandlerExpand(void* userData, const XML_Char* s, const int len) { From 9def61aed9a0e1d1314bcd5dcf328f68d07b8932 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 19:51:14 +1200 Subject: [PATCH 09/12] Address review comment: Don't leave dangling pointers when loading ligature/kern darta fails --- lib/EpdFont/SdCardFont.cpp | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/EpdFont/SdCardFont.cpp b/lib/EpdFont/SdCardFont.cpp index a07660b4..23a90233 100644 --- a/lib/EpdFont/SdCardFont.cpp +++ b/lib/EpdFont/SdCardFont.cpp @@ -72,6 +72,11 @@ void SdCardFont::freeStyleKernLigatureData(PerStyle& s) { delete[] s.ligaturePairs; s.ligaturePairs = nullptr; s.ligLoaded = false; + // Clear dangling pointers in EpdFontData structs + s.stubData.ligaturePairs = nullptr; + s.stubData.ligaturePairCount = 0; + s.miniData.ligaturePairs = nullptr; + s.miniData.ligaturePairCount = 0; } void SdCardFont::freeStyleMiniKern(PerStyle& s) { @@ -157,56 +162,61 @@ bool SdCardFont::loadStyleKernLigatureData(PerStyle& s, bool ligatureOnly) { // Load only the small class-lookup tables (~3KB each). The full matrix // (~36KB contiguous for Literata) is built per-page from SD in // buildMiniKernMatrix(). - s.kernLeftClasses = new (std::nothrow) EpdKernClassEntry[s.header.kernLeftEntryCount]; - s.kernRightClasses = new (std::nothrow) EpdKernClassEntry[s.header.kernRightEntryCount]; + EpdKernClassEntry* newLeft = new (std::nothrow) EpdKernClassEntry[s.header.kernLeftEntryCount]; + EpdKernClassEntry* newRight = new (std::nothrow) EpdKernClassEntry[s.header.kernRightEntryCount]; - if (!s.kernLeftClasses || !s.kernRightClasses) { + if (!newLeft || !newRight) { + delete[] newLeft; + delete[] newRight; LOG_ERR("SDCF", "Failed to allocate kern classes (%u+%u bytes)", s.header.kernLeftEntryCount * 3u, s.header.kernRightEntryCount * 3u); - freeStyleKernLigatureData(s); file.close(); return false; } if (!file.seekSet(s.kernLeftFileOffset)) { + delete[] newLeft; + delete[] newRight; LOG_ERR("SDCF", "Failed to seek to kern data"); - freeStyleKernLigatureData(s); file.close(); return false; } size_t leftSz = s.header.kernLeftEntryCount * sizeof(EpdKernClassEntry); size_t rightSz = s.header.kernRightEntryCount * sizeof(EpdKernClassEntry); - if (file.read(reinterpret_cast(s.kernLeftClasses), leftSz) != static_cast(leftSz) || - file.read(reinterpret_cast(s.kernRightClasses), rightSz) != static_cast(rightSz)) { + if (file.read(reinterpret_cast(newLeft), leftSz) != static_cast(leftSz) || + file.read(reinterpret_cast(newRight), rightSz) != static_cast(rightSz)) { + delete[] newLeft; + delete[] newRight; LOG_ERR("SDCF", "Failed to read kern classes"); - freeStyleKernLigatureData(s); file.close(); return false; } + s.kernLeftClasses = newLeft; + s.kernRightClasses = newRight; s.kernClassesLoaded = true; } if (wantLig && !s.ligLoaded) { - s.ligaturePairs = new (std::nothrow) EpdLigaturePair[s.header.ligaturePairCount]; - if (!s.ligaturePairs) { + EpdLigaturePair* newLig = new (std::nothrow) EpdLigaturePair[s.header.ligaturePairCount]; + if (!newLig) { LOG_ERR("SDCF", "Failed to allocate ligature pairs"); - freeStyleKernLigatureData(s); file.close(); return false; } if (!file.seekSet(s.ligatureFileOffset)) { + delete[] newLig; LOG_ERR("SDCF", "Failed to seek to ligature data"); - freeStyleKernLigatureData(s); file.close(); return false; } size_t sz = s.header.ligaturePairCount * sizeof(EpdLigaturePair); - if (file.read(reinterpret_cast(s.ligaturePairs), sz) != static_cast(sz)) { + if (file.read(reinterpret_cast(newLig), sz) != static_cast(sz)) { + delete[] newLig; LOG_ERR("SDCF", "Failed to read ligature pairs"); - freeStyleKernLigatureData(s); file.close(); return false; } + s.ligaturePairs = newLig; s.ligLoaded = true; // Make ligatures visible to the stub (used when no mini data built yet). From acfb9e6cdc0546a95df52afb779c41c12c632976 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 20:03:35 +1200 Subject: [PATCH 10/12] Address Review Comment: clear dangling pointers when miniData freed --- lib/EpdFont/SdCardFont.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/EpdFont/SdCardFont.cpp b/lib/EpdFont/SdCardFont.cpp index 23a90233..62bd8469 100644 --- a/lib/EpdFont/SdCardFont.cpp +++ b/lib/EpdFont/SdCardFont.cpp @@ -52,6 +52,21 @@ void SdCardFont::freeStyleMiniData(PerStyle& s) { s.miniIntervalCount = 0; s.miniGlyphCount = 0; s.miniMode = PerStyle::MiniMode::NONE; + // Clear dangling pointers in miniData and stubData (kern data points to freed mini arrays) + s.miniData.kernLeftClasses = nullptr; + s.miniData.kernRightClasses = nullptr; + s.miniData.kernMatrix = nullptr; + s.miniData.kernLeftEntryCount = 0; + s.miniData.kernRightEntryCount = 0; + s.miniData.kernLeftClassCount = 0; + s.miniData.kernRightClassCount = 0; + s.stubData.kernLeftClasses = nullptr; + s.stubData.kernRightClasses = nullptr; + s.stubData.kernMatrix = nullptr; + s.stubData.kernLeftEntryCount = 0; + s.stubData.kernRightEntryCount = 0; + s.stubData.kernLeftClassCount = 0; + s.stubData.kernRightClassCount = 0; // NOTE: reportedMissCount is intentionally NOT reset here. The merge path // calls freeStyleMiniData() to swap mini buffers, and resetting the miss // tracker every paragraph would re-spam the log for the same 4 missing cps. From f07724dab431552b12b7d45e4417d4af95c7f233 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 20:09:52 +1200 Subject: [PATCH 11/12] Address review comment: Ensure ligature metadata is wired if requested but not yet wired --- lib/EpdFont/SdCardFont.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/EpdFont/SdCardFont.cpp b/lib/EpdFont/SdCardFont.cpp index 62bd8469..143e6c10 100644 --- a/lib/EpdFont/SdCardFont.cpp +++ b/lib/EpdFont/SdCardFont.cpp @@ -767,6 +767,18 @@ int SdCardFont::prewarmStyle(uint8_t styleIdx, const uint32_t* codepoints, uint3 // For metadata-only calls, METADATA or FULL cache both satisfy layout queries. // For full (bitmap) calls, only FULL satisfies — METADATA lacks bitmap data. if (metadataOnly || s.miniMode == PerStyle::MiniMode::FULL) { + // Ensure ligature metadata is wired if requested but not yet wired. + if (loadKernLigatureData && !s.ligLoaded) { + loadStyleKernLigatureData(s, /*ligatureOnly=*/true); + } + if (loadKernLigatureData && s.ligLoaded && s.miniData.ligaturePairs == nullptr) { + if (s.miniMode == PerStyle::MiniMode::FULL) { + applyKernLigaturePointers(s, s.miniData); + } else { + s.miniData.ligaturePairs = s.ligaturePairs; + s.miniData.ligaturePairCount = s.header.ligaturePairCount; + } + } // Already wired into miniData; nothing else to do. return 0; } From 383523696199cad06036d21d177d6bbf99e342f6 Mon Sep 17 00:00:00 2001 From: spfenwick Date: Sat, 2 May 2026 20:23:29 +1200 Subject: [PATCH 12/12] Address nitpick review comments --- lib/Epub/Epub/ParsedText.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index c1abcaab..7f584f32 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -383,11 +383,11 @@ void ParsedText::layoutAndExtractLines( words.shrink_to_fit(); wordStyles.shrink_to_fit(); wordContinues.shrink_to_fit(); - isContinuation_ = !includeLastLine; // All remaining words were already transformed before the flush; reset the // watermark so that words appended by addWord() are processed next time. bionicTransformedUpTo_ = words.size(); } + isContinuation_ = !includeLastLine; } std::vector ParsedText::calculateWordWidths(const GfxRenderer& renderer, const int fontId) { @@ -610,7 +610,6 @@ void ParsedText::applyBionicReadingTransform() { std::string source = std::move(words[i]); const auto originalStyle = wordStyles[i]; const bool originalAttachToPrevious = wordContinues[i]; - const char* raw = source.c_str(); const auto spans = tokenizeBionicWord(source); if (spans.empty()) { @@ -621,12 +620,7 @@ void ParsedText::applyBionicReadingTransform() { for (size_t spanIndex = 0; spanIndex < spans.size(); ++spanIndex) { const TokenSpan span = spans[spanIndex]; const size_t spanLength = span.end - span.start; - std::string token; - if (spans.size() == 1 && spanIndex == 0) { - token = std::move(source); - } else { - token.assign(raw + span.start, spanLength); - } + std::string token = source.substr(span.start, spanLength); if (span.isWord) { const unsigned char* ptr = reinterpret_cast(token.c_str());