From e4170e2272cb112816f27d0d7682b284bdfc41f4 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 22 Apr 2026 07:33:39 +0200 Subject: [PATCH 1/9] Font rendering performance improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - FontDecompressor: replace heap-allocated std::vector hot-group and glyph scratch buffers with static BSS arrays, eliminating per-page malloc/free and the heap fragmentation it causes during rendering - FontDecompressor: add bounds checks on group/glyph buffer sizes; sort prewarm group list by ascending index for sequential flash reads - ParsedText: upgrade line-break cost function to Knuth-Plass cubic badness (strongly penalises very loose lines, lenient on moderate looseness) - ParsedText: exclude gaps before closing punctuation (. , ) » etc.) from justification distribution so they stay at natural space width Co-Authored-By: Claude Sonnet 4.6 --- lib/EpdFont/FontDecompressor.cpp | 80 +++++++++++++++++--------------- lib/EpdFont/FontDecompressor.h | 18 ++++--- lib/Epub/Epub/ParsedText.cpp | 54 +++++++++++++++++---- 3 files changed, 101 insertions(+), 51 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index 10af14df..fda79fad 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -33,12 +33,9 @@ void FontDecompressor::freePageBuffer() { } void FontDecompressor::freeHotGroup() { - hotGroup.clear(); - hotGroup.shrink_to_fit(); hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; - hotGlyphBuf.clear(); - hotGlyphBuf.shrink_to_fit(); + _hotGroupBufUsed = 0; } uint16_t FontDecompressor::getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex) { @@ -61,9 +58,17 @@ bool FontDecompressor::decompressGroup(const EpdFontData* fontData, uint16_t gro uint32_t outSize) { const EpdFontGroup& group = fontData->groups[groupIndex]; + if (outSize > HOT_GROUP_BUF_SIZE) { + LOG_ERR("FDC", "Group %u uncompressed size %lu exceeds HOT_GROUP_BUF_SIZE %lu", groupIndex, outSize, + HOT_GROUP_BUF_SIZE); + return false; + } + const uint32_t tDecomp = millis(); inflateReader.init(false); + inflateReader.setSource(&fontData->bitmap[group.compressedOffset], group.compressedSize); + if (!inflateReader.read(outBuf, outSize)) { stats.decompressTimeMs += millis() - tDecomp; LOG_ERR("FDC", "Decompression failed for group %u", groupIndex); @@ -170,48 +175,37 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep } // Check if hot group already has this group decompressed — if not, decompress it - if (!(!hotGroup.empty() && hotGroupFont == fontData && hotGroupIndex == groupIndex)) { + if (!(hotGroupFont == fontData && hotGroupIndex == groupIndex && _hotGroupBufUsed > 0)) { stats.cacheMisses++; const EpdFontGroup& group = fontData->groups[groupIndex]; - hotGroup.resize(group.uncompressedSize); - if (hotGroup.empty()) { - LOG_ERR("FDC", "Failed to allocate %u bytes for hot group %u", group.uncompressedSize, groupIndex); - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; - stats.getBitmapTimeUs += micros() - tStart; - return nullptr; - } - - if (!decompressGroup(fontData, groupIndex, hotGroup.data(), group.uncompressedSize)) { - hotGroup.clear(); - hotGroup.shrink_to_fit(); + if (!decompressGroup(fontData, groupIndex, _hotGroupBuf, group.uncompressedSize)) { hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; + _hotGroupBufUsed = 0; stats.getBitmapTimeUs += micros() - tStart; return nullptr; } hotGroupFont = fontData; hotGroupIndex = groupIndex; + _hotGroupBufUsed = group.uncompressedSize; stats.hotGroupBytes = group.uncompressedSize; } else { stats.cacheHits++; } - // Compact just the requested glyph from byte-aligned data into scratch buffer - if (glyph->dataLength > hotGlyphBuf.size()) { - hotGlyphBuf.resize(glyph->dataLength); - } - if (hotGlyphBuf.empty()) { + if (glyph->dataLength > HOT_GLYPH_BUF_SIZE) { + LOG_ERR("FDC", "Glyph dataLength %u exceeds HOT_GLYPH_BUF_SIZE %u", glyph->dataLength, HOT_GLYPH_BUF_SIZE); stats.getBitmapTimeUs += micros() - tStart; return nullptr; } uint32_t alignedOff = getAlignedOffset(fontData, groupIndex, glyphIndex); - compactSingleGlyph(&hotGroup[alignedOff], hotGlyphBuf.data(), glyph->width, glyph->height); + compactSingleGlyph(&_hotGroupBuf[alignedOff], _hotGlyphBuf, glyph->width, glyph->height); + stats.getBitmapTimeUs += micros() - tStart; - return hotGlyphBuf.data(); + return _hotGlyphBuf; } // --- Prewarm: pre-decompress glyph bitmaps for a page of text --- @@ -314,6 +308,18 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 stats.uniqueGroupsAccessed = groupCount; + // Sort neededGroups by ascending group index so flash reads are sequential. + // Uses insertion sort — groupCount is bounded at 128, typically <14 for Latin fonts. + for (uint8_t i = 1; i < groupCount; i++) { + uint16_t key = neededGroups[i]; + int j = i - 1; + while (j >= 0 && neededGroups[j] > key) { + neededGroups[j + 1] = neededGroups[j]; + j--; + } + neededGroups[j + 1] = key; + } + // Step 3: Allocate page buffer and lookup table for this slot slot.buffer = static_cast(malloc(totalBytes)); slot.glyphs = static_cast(malloc(glyphCount * sizeof(PageGlyphEntry))); @@ -417,7 +423,9 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 } } - // Step 4: For each unique group, decompress to temp buffer and extract needed glyphs + // Step 4: For each unique group, decompress into the static _hotGroupBuf and extract needed glyphs. + // No heap allocation — _hotGroupBuf is reused for each group in turn. + // After prewarm, _hotGroupBuf is invalidated (hotGroupFont reset) since its contents are transient. uint32_t writeOffset = 0; int missed = 0; @@ -425,37 +433,35 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 uint16_t groupIdx = neededGroups[g]; const EpdFontGroup& group = fontData->groups[groupIdx]; - auto* tempBuf = static_cast(malloc(group.uncompressedSize)); - if (!tempBuf) { - LOG_ERR("FDC", "Failed to allocate temp buffer (%u bytes) for group %u", group.uncompressedSize, groupIdx); - missed++; - continue; - } if (group.uncompressedSize > stats.peakTempBytes) { stats.peakTempBytes = group.uncompressedSize; } - if (!decompressGroup(fontData, groupIdx, tempBuf, group.uncompressedSize)) { - free(tempBuf); + if (!decompressGroup(fontData, groupIdx, _hotGroupBuf, group.uncompressedSize)) { missed++; continue; } - // Extract needed glyphs directly from the byte-aligned temp buffer, compacting on the fly. + // Extract needed glyphs directly from the byte-aligned buffer, compacting on the fly. // alignedOffset was pre-computed in step 3b — no full-group compact scan needed. for (uint16_t i = 0; i < slot.glyphCount; i++) { if (slot.glyphs[i].bufferOffset != UINT32_MAX) continue; // already extracted if (getGroupIndex(fontData, slot.glyphs[i].glyphIndex) != groupIdx) continue; const EpdGlyph& glyph = fontData->glyph[slot.glyphs[i].glyphIndex]; - compactSingleGlyph(&tempBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, glyph.height); + compactSingleGlyph(&_hotGroupBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, + glyph.height); slot.glyphs[i].bufferOffset = writeOffset; writeOffset += glyph.dataLength; } - - free(tempBuf); } + // Prewarm reused _hotGroupBuf transiently — invalidate hot group state so getBitmap() + // doesn't treat stale contents as a valid cache entry for a different glyph request. + hotGroupFont = nullptr; + hotGroupIndex = UINT16_MAX; + _hotGroupBufUsed = 0; + LOG_DBG("FDC", "Prewarm: %u glyphs in %u bytes from %u groups (%d missed)", glyphCount, writeOffset, groupCount, missed); diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index 54e75a86..bd1c25ac 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -2,8 +2,6 @@ #include -#include - #include "EpdFontData.h" class FontDecompressor { @@ -65,15 +63,23 @@ class FontDecompressor { PageSlot pageSlots[MAX_PAGE_SLOTS] = {}; uint8_t pageSlotCount = 0; + // Measured maxima across all built-in fonts: + // uncompressedSize: 50 KB (notosans_18 / bookerly_18) + // glyph dataLength: 500 B (bookerly_18) + // Static BSS arrays eliminate per-page heap alloc/free and the fragmentation it causes. + static constexpr uint32_t HOT_GROUP_BUF_SIZE = 51200; // 50 KB uncompressed group + static constexpr uint16_t HOT_GLYPH_BUF_SIZE = 512; // largest packed single glyph + // Hot group: last decompressed group (byte-aligned) for non-prewarmed fallback path. - // Kept in byte-aligned format; individual glyphs are compacted on demand into hotGlyphBuf. + // Kept in byte-aligned format; individual glyphs are compacted on demand into _hotGlyphBuf. const EpdFontData* hotGroupFont = nullptr; uint16_t hotGroupIndex = UINT16_MAX; - std::vector hotGroup; + uint32_t _hotGroupBufUsed = 0; + uint8_t _hotGroupBuf[HOT_GROUP_BUF_SIZE]; - // Scratch buffer for compacting a single glyph from the hot group. + // Scratch buffer for compacting a single glyph out of the byte-aligned hot group. // Valid until the next getBitmap() call. - std::vector hotGlyphBuf; + uint8_t _hotGlyphBuf[HOT_GLYPH_BUF_SIZE]; void freePageBuffer(); void freeHotGroup(); diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 40a125e4..aad424cf 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -17,6 +17,30 @@ constexpr int MAX_COST = std::numeric_limits::max(); namespace { +// Closing punctuation that should not have extra space inserted before it during justification. +// Includes common closing brackets/quotes and sentence-ending marks. +bool isClosingPunctuation(const uint32_t cp) { + switch (cp) { + case '.': + case ',': + case '!': + case '?': + case ':': + case ';': + case ')': + case ']': + case '}': + case 0x00BB: // » + case 0x203A: // › + case 0x2019: // ' right single quotation mark + case 0x201D: // " right double quotation mark + case 0x2026: // … ellipsis + return true; + default: + return false; + } +} + // Soft hyphen byte pattern used throughout EPUBs (UTF-8 for U+00AD). constexpr char SOFT_HYPHEN_UTF8[] = "\xC2\xAD"; constexpr size_t SOFT_HYPHEN_BYTES = 2; @@ -330,11 +354,19 @@ std::vector ParsedText::computeLineBreaks(const GfxRenderer& renderer, c int cost; if (j == totalWordCount - 1) { - cost = 0; // Last line + cost = 0; // Last line — no penalty regardless of looseness } else { const int remainingSpace = effectivePageWidth - currlen; - // Use long long for the square to prevent overflow - const long long cost_ll = static_cast(remainingSpace) * remainingSpace + dp[j + 1]; + // Knuth-Plass style demerits: + // badness = (gap/lineWidth)³ × 10000, clamped to [0, 10000] + // demerits = (1 + badness)² + // Cubic badness strongly penalises very loose lines while being + // lenient on moderately loose ones, producing visually balanced paragraphs. + const long long b_num = static_cast(remainingSpace) * remainingSpace * remainingSpace; + const long long b_den = static_cast(effectivePageWidth) * effectivePageWidth * effectivePageWidth; + const int badness = (b_den > 0) ? static_cast(std::min(b_num * 10000LL / b_den, 10000LL)) : 10000; + const long long demerits = static_cast(1 + badness) * (1 + badness); + const long long cost_ll = demerits + dp[j + 1]; if (cost_ll > MAX_COST) { cost = MAX_COST; @@ -767,9 +799,12 @@ ParsedText::LineProcessResult ParsedText::extractLine( for (size_t wordIdx = 0; wordIdx < lineWordCount; wordIdx++) { lineWordWidthSum += wordWidths[lastBreakAt + wordIdx]; - // Count gaps: each word after the first creates a gap, unless it's a continuation + // Count gaps: each word after the first creates a gap, unless it's a continuation. + // Gaps before closing punctuation (. , ) » etc.) are excluded from justification + // distribution so they stay at natural space width. if (wordIdx > 0 && !continuesVec[lastBreakAt + wordIdx]) { - actualGapCount++; + const bool beforeClosing = isClosingPunctuation(firstCodepoint(words[lastBreakAt + wordIdx])); + if (!beforeClosing) actualGapCount++; totalNaturalGaps += renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]), firstCodepoint(words[lastBreakAt + wordIdx]), wordStyles[lastBreakAt + wordIdx - 1]); @@ -825,9 +860,12 @@ ParsedText::LineProcessResult ParsedText::extractLine( gap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx]), firstCodepoint(words[lastBreakAt + wordIdx + 1]), wordStyles[lastBreakAt + wordIdx]); - } - if (blockStyle.alignment == CssTextAlign::Justify && !isLastLine) { - gap += justifyExtra; + // Don't stretch the gap before closing punctuation — it looks wrong with + // extra space before ".", ")", "»" etc. + const bool nextIsClosing = isClosingPunctuation(firstCodepoint(words[lastBreakAt + wordIdx + 1])); + if (blockStyle.alignment == CssTextAlign::Justify && !isLastLine && !nextIsClosing) { + gap += justifyExtra; + } } xpos += wordWidths[lastBreakAt + wordIdx] + gap; } From 80722118a2bffc78421e8a9cb85b944e2040f6af Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 22 Apr 2026 08:14:04 +0200 Subject: [PATCH 2/9] FontDecompressor: heap-allocate hot group buffer sized to active font MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the 50 KB static BSS _hotGroupBuf array with a heap pointer allocated once per font (ensureHotGroupBuf). The buffer is sized to the largest group in the active font — 6 KB for small fonts, ~50 KB for 18pt — so no memory is wasted when rendering body text at typical sizes. The buffer persists across pages (freed only in freeHotGroup/deinit), so there is exactly one malloc per font session rather than one per page. Co-Authored-By: Claude Sonnet 4.6 --- lib/EpdFont/FontDecompressor.cpp | 56 +++++++++++++++++++++++++------- lib/EpdFont/FontDecompressor.h | 16 ++++----- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index fda79fad..6dea3c01 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -36,6 +36,34 @@ void FontDecompressor::freeHotGroup() { hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; _hotGroupBufUsed = 0; + free(_hotGroupBuf); + _hotGroupBuf = nullptr; + _hotGroupBufSize = 0; +} + +bool FontDecompressor::ensureHotGroupBuf(const EpdFontData* fontData) { + // Find largest uncompressed group size for this font + uint32_t maxSize = 0; + for (uint16_t i = 0; i < fontData->groupCount; i++) { + if (fontData->groups[i].uncompressedSize > maxSize) maxSize = fontData->groups[i].uncompressedSize; + } + if (maxSize == 0) return false; + + if (_hotGroupBufSize >= maxSize) return true; // existing allocation is large enough + + free(_hotGroupBuf); + _hotGroupBuf = static_cast(malloc(maxSize)); + if (!_hotGroupBuf) { + _hotGroupBufSize = 0; + LOG_ERR("FDC", "OOM: cannot allocate %lu bytes for hot group buf", maxSize); + return false; + } + _hotGroupBufSize = maxSize; + // Switching fonts invalidates any cached group + hotGroupFont = nullptr; + hotGroupIndex = UINT16_MAX; + _hotGroupBufUsed = 0; + return true; } uint16_t FontDecompressor::getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex) { @@ -57,16 +85,8 @@ uint16_t FontDecompressor::getGroupIndex(const EpdFontData* fontData, uint32_t g bool FontDecompressor::decompressGroup(const EpdFontData* fontData, uint16_t groupIndex, uint8_t* outBuf, uint32_t outSize) { const EpdFontGroup& group = fontData->groups[groupIndex]; - - if (outSize > HOT_GROUP_BUF_SIZE) { - LOG_ERR("FDC", "Group %u uncompressed size %lu exceeds HOT_GROUP_BUF_SIZE %lu", groupIndex, outSize, - HOT_GROUP_BUF_SIZE); - return false; - } - const uint32_t tDecomp = millis(); inflateReader.init(false); - inflateReader.setSource(&fontData->bitmap[group.compressedOffset], group.compressedSize); if (!inflateReader.read(outBuf, outSize)) { @@ -179,6 +199,11 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep stats.cacheMisses++; const EpdFontGroup& group = fontData->groups[groupIndex]; + if (!ensureHotGroupBuf(fontData)) { + stats.getBitmapTimeUs += micros() - tStart; + return nullptr; + } + if (!decompressGroup(fontData, groupIndex, _hotGroupBuf, group.uncompressedSize)) { hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; @@ -423,9 +448,17 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 } } - // Step 4: For each unique group, decompress into the static _hotGroupBuf and extract needed glyphs. - // No heap allocation — _hotGroupBuf is reused for each group in turn. - // After prewarm, _hotGroupBuf is invalidated (hotGroupFont reset) since its contents are transient. + // Step 4: Ensure hot group buffer is sized for this font, then decompress each group and extract. + // ensureHotGroupBuf() allocates once for the largest group — reused across all groups in this prewarm. + if (!ensureHotGroupBuf(fontData)) { + LOG_ERR("FDC", "Failed to allocate hot group buf during prewarm"); + free(slot.buffer); + free(slot.glyphs); + slot = {}; + pageSlotCount--; + return glyphCount; + } + uint32_t writeOffset = 0; int missed = 0; @@ -458,6 +491,7 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 // Prewarm reused _hotGroupBuf transiently — invalidate hot group state so getBitmap() // doesn't treat stale contents as a valid cache entry for a different glyph request. + // The buffer itself is kept allocated (freed in freeHotGroup/deinit) — no churn between pages. hotGroupFont = nullptr; hotGroupIndex = UINT16_MAX; _hotGroupBufUsed = 0; diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index bd1c25ac..88b552d2 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -63,19 +63,16 @@ class FontDecompressor { PageSlot pageSlots[MAX_PAGE_SLOTS] = {}; uint8_t pageSlotCount = 0; - // Measured maxima across all built-in fonts: - // uncompressedSize: 50 KB (notosans_18 / bookerly_18) - // glyph dataLength: 500 B (bookerly_18) - // Static BSS arrays eliminate per-page heap alloc/free and the fragmentation it causes. - static constexpr uint32_t HOT_GROUP_BUF_SIZE = 51200; // 50 KB uncompressed group - static constexpr uint16_t HOT_GLYPH_BUF_SIZE = 512; // largest packed single glyph + static constexpr uint16_t HOT_GLYPH_BUF_SIZE = 512; // largest packed single glyph - // Hot group: last decompressed group (byte-aligned) for non-prewarmed fallback path. - // Kept in byte-aligned format; individual glyphs are compacted on demand into _hotGlyphBuf. + // Hot group: single heap buffer sized to the largest group of the active font. + // Allocated once per font (lazily in ensureHotGroupBuf) and freed in freeHotGroup(). + // Single malloc per font session — no repeated alloc/free per page. const EpdFontData* hotGroupFont = nullptr; uint16_t hotGroupIndex = UINT16_MAX; uint32_t _hotGroupBufUsed = 0; - uint8_t _hotGroupBuf[HOT_GROUP_BUF_SIZE]; + uint8_t* _hotGroupBuf = nullptr; + uint32_t _hotGroupBufSize = 0; // Scratch buffer for compacting a single glyph out of the byte-aligned hot group. // Valid until the next getBitmap() call. @@ -83,6 +80,7 @@ class FontDecompressor { void freePageBuffer(); void freeHotGroup(); + bool ensureHotGroupBuf(const EpdFontData* fontData); uint16_t getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex); uint32_t getAlignedOffset(const EpdFontData* fontData, uint16_t groupIndex, uint32_t glyphIndex); bool decompressGroup(const EpdFontData* fontData, uint16_t groupIndex, uint8_t* outBuf, uint32_t outSize); From e76118dff0d42261e1392643787de7683a44f689 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 22 Apr 2026 08:31:44 +0200 Subject: [PATCH 3/9] FontDecompressor: eliminate persistent hot group buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hot group buffer (formerly up to 50 KB BSS / heap) is removed. During prewarm, each group is now decompressed into a transient malloc that is freed immediately after its glyphs are extracted — only one group buffer and the page buffer coexist at a time. For getBitmap() cache misses (rare: only hit when a glyph wasn't covered by prewarm), the group is also decompressed transiently and freed after the single glyph is compacted into _hotGlyphBuf. Peak heap during prewarm is now: page buffer + one group buffer. Outside of prewarm, heap usage is only the page buffer itself. Works correctly for large future fonts (Vietnamese, CJK) regardless of group size, with no permanent allocation overhead. Co-Authored-By: Claude Sonnet 4.6 --- lib/EpdFont/FontDecompressor.cpp | 126 +++++++++---------------------- lib/EpdFont/FontDecompressor.h | 18 +---- 2 files changed, 40 insertions(+), 104 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index 6dea3c01..38fe095c 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -13,15 +13,9 @@ bool FontDecompressor::init() { return true; } -void FontDecompressor::deinit() { - freePageBuffer(); - freeHotGroup(); -} +void FontDecompressor::deinit() { freePageBuffer(); } -void FontDecompressor::clearCache() { - freePageBuffer(); - freeHotGroup(); -} +void FontDecompressor::clearCache() { freePageBuffer(); } void FontDecompressor::freePageBuffer() { for (uint8_t s = 0; s < pageSlotCount; s++) { @@ -32,40 +26,6 @@ void FontDecompressor::freePageBuffer() { pageSlotCount = 0; } -void FontDecompressor::freeHotGroup() { - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; - _hotGroupBufUsed = 0; - free(_hotGroupBuf); - _hotGroupBuf = nullptr; - _hotGroupBufSize = 0; -} - -bool FontDecompressor::ensureHotGroupBuf(const EpdFontData* fontData) { - // Find largest uncompressed group size for this font - uint32_t maxSize = 0; - for (uint16_t i = 0; i < fontData->groupCount; i++) { - if (fontData->groups[i].uncompressedSize > maxSize) maxSize = fontData->groups[i].uncompressedSize; - } - if (maxSize == 0) return false; - - if (_hotGroupBufSize >= maxSize) return true; // existing allocation is large enough - - free(_hotGroupBuf); - _hotGroupBuf = static_cast(malloc(maxSize)); - if (!_hotGroupBuf) { - _hotGroupBufSize = 0; - LOG_ERR("FDC", "OOM: cannot allocate %lu bytes for hot group buf", maxSize); - return false; - } - _hotGroupBufSize = maxSize; - // Switching fonts invalidates any cached group - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; - _hotGroupBufUsed = 0; - return true; -} - uint16_t FontDecompressor::getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex) { // O(1) path for frequency-grouped fonts with glyphToGroup mapping if (fontData->glyphToGroup != nullptr) { @@ -186,7 +146,8 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep break; // Found the right slot but glyph wasn't in it; don't check other slots } - // Fallback: hot group slot + // Fallback: glyph wasn't in the page buffer — decompress its group transiently. + // This is the rare path (prewarm should cover all glyphs on a normal page). uint16_t groupIndex = getGroupIndex(fontData, glyphIndex); if (groupIndex >= fontData->groupCount) { LOG_ERR("FDC", "Glyph %u not found in any group", glyphIndex); @@ -194,40 +155,34 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep return nullptr; } - // Check if hot group already has this group decompressed — if not, decompress it - if (!(hotGroupFont == fontData && hotGroupIndex == groupIndex && _hotGroupBufUsed > 0)) { - stats.cacheMisses++; - const EpdFontGroup& group = fontData->groups[groupIndex]; + stats.cacheMisses++; + const EpdFontGroup& group = fontData->groups[groupIndex]; - if (!ensureHotGroupBuf(fontData)) { - stats.getBitmapTimeUs += micros() - tStart; - return nullptr; - } + if (group.uncompressedSize > stats.peakTempBytes) stats.peakTempBytes = group.uncompressedSize; - if (!decompressGroup(fontData, groupIndex, _hotGroupBuf, group.uncompressedSize)) { - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; - _hotGroupBufUsed = 0; - stats.getBitmapTimeUs += micros() - tStart; - return nullptr; - } + uint8_t* groupBuf = static_cast(malloc(group.uncompressedSize)); + if (!groupBuf) { + LOG_ERR("FDC", "OOM: cannot allocate %lu bytes for group %u fallback", group.uncompressedSize, groupIndex); + stats.getBitmapTimeUs += micros() - tStart; + return nullptr; + } - hotGroupFont = fontData; - hotGroupIndex = groupIndex; - _hotGroupBufUsed = group.uncompressedSize; - stats.hotGroupBytes = group.uncompressedSize; - } else { - stats.cacheHits++; + if (!decompressGroup(fontData, groupIndex, groupBuf, group.uncompressedSize)) { + free(groupBuf); + stats.getBitmapTimeUs += micros() - tStart; + return nullptr; } if (glyph->dataLength > HOT_GLYPH_BUF_SIZE) { LOG_ERR("FDC", "Glyph dataLength %u exceeds HOT_GLYPH_BUF_SIZE %u", glyph->dataLength, HOT_GLYPH_BUF_SIZE); + free(groupBuf); stats.getBitmapTimeUs += micros() - tStart; return nullptr; } uint32_t alignedOff = getAlignedOffset(fontData, groupIndex, glyphIndex); - compactSingleGlyph(&_hotGroupBuf[alignedOff], _hotGlyphBuf, glyph->width, glyph->height); + compactSingleGlyph(&groupBuf[alignedOff], _hotGlyphBuf, glyph->width, glyph->height); + free(groupBuf); stats.getBitmapTimeUs += micros() - tStart; return _hotGlyphBuf; @@ -448,17 +403,9 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 } } - // Step 4: Ensure hot group buffer is sized for this font, then decompress each group and extract. - // ensureHotGroupBuf() allocates once for the largest group — reused across all groups in this prewarm. - if (!ensureHotGroupBuf(fontData)) { - LOG_ERR("FDC", "Failed to allocate hot group buf during prewarm"); - free(slot.buffer); - free(slot.glyphs); - slot = {}; - pageSlotCount--; - return glyphCount; - } - + // Step 4: For each unique group, malloc a transient buffer, decompress, extract needed glyphs, free. + // One malloc/free per group per prewarm call. Groups are visited in sorted order, so + // only one group buffer is alive at a time — peak heap = page buffer + largest single group. uint32_t writeOffset = 0; int missed = 0; @@ -466,11 +413,17 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 uint16_t groupIdx = neededGroups[g]; const EpdFontGroup& group = fontData->groups[groupIdx]; - if (group.uncompressedSize > stats.peakTempBytes) { - stats.peakTempBytes = group.uncompressedSize; + if (group.uncompressedSize > stats.peakTempBytes) stats.peakTempBytes = group.uncompressedSize; + + uint8_t* groupBuf = static_cast(malloc(group.uncompressedSize)); + if (!groupBuf) { + LOG_ERR("FDC", "OOM: cannot allocate %lu bytes for group %u during prewarm", group.uncompressedSize, groupIdx); + missed++; + continue; } - if (!decompressGroup(fontData, groupIdx, _hotGroupBuf, group.uncompressedSize)) { + if (!decompressGroup(fontData, groupIdx, groupBuf, group.uncompressedSize)) { + free(groupBuf); missed++; continue; } @@ -482,19 +435,14 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 if (getGroupIndex(fontData, slot.glyphs[i].glyphIndex) != groupIdx) continue; const EpdGlyph& glyph = fontData->glyph[slot.glyphs[i].glyphIndex]; - compactSingleGlyph(&_hotGroupBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, + compactSingleGlyph(&groupBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, glyph.height); slot.glyphs[i].bufferOffset = writeOffset; writeOffset += glyph.dataLength; } - } - // Prewarm reused _hotGroupBuf transiently — invalidate hot group state so getBitmap() - // doesn't treat stale contents as a valid cache entry for a different glyph request. - // The buffer itself is kept allocated (freed in freeHotGroup/deinit) — no churn between pages. - hotGroupFont = nullptr; - hotGroupIndex = UINT16_MAX; - _hotGroupBufUsed = 0; + free(groupBuf); + } LOG_DBG("FDC", "Prewarm: %u glyphs in %u bytes from %u groups (%d missed)", glyphCount, writeOffset, groupCount, missed); @@ -511,8 +459,8 @@ void FontDecompressor::logStats(const char* label) { LOG_DBG("FDC", "[%s] hits=%lu misses=%lu (%.1f%% hit rate)", label, stats.cacheHits, stats.cacheMisses, total > 0 ? 100.0f * stats.cacheHits / total : 0.0f); LOG_DBG("FDC", "[%s] decompress=%lums groups_accessed=%u", label, stats.decompressTimeMs, stats.uniqueGroupsAccessed); - LOG_DBG("FDC", "[%s] mem: pageBuf=%lu pageGlyphs=%lu hotGroup=%lu peakTemp=%lu", label, stats.pageBufferBytes, - stats.pageGlyphsBytes, stats.hotGroupBytes, stats.peakTempBytes); + LOG_DBG("FDC", "[%s] mem: pageBuf=%lu pageGlyphs=%lu peakTemp=%lu", label, stats.pageBufferBytes, + stats.pageGlyphsBytes, stats.peakTempBytes); if (stats.getBitmapCalls > 0) { LOG_DBG("FDC", "[%s] getBitmap: %lu calls, %luus total, %luus/call avg", label, stats.getBitmapCalls, stats.getBitmapTimeUs, stats.getBitmapTimeUs / stats.getBitmapCalls); diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index 88b552d2..2894406a 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -19,7 +19,7 @@ class FontDecompressor { // Checks the page buffer (from prewarm) first, then falls back to the hot group slot. const uint8_t* getBitmap(const EpdFontData* fontData, const EpdGlyph* glyph, uint32_t glyphIndex); - // Free all cached data (page buffer + hot group). + // Free all cached data (page buffers). void clearCache(); // Pre-scan UTF-8 text and extract needed glyph bitmaps into a flat page buffer. @@ -34,8 +34,7 @@ class FontDecompressor { uint16_t uniqueGroupsAccessed = 0; uint32_t pageBufferBytes = 0; // pageBuffer allocation uint32_t pageGlyphsBytes = 0; // pageGlyphs lookup table allocation - uint32_t hotGroupBytes = 0; // current hot group allocation - uint32_t peakTempBytes = 0; // largest temp buffer in prewarm + uint32_t peakTempBytes = 0; // largest temp buffer in prewarm or getBitmap miss uint32_t getBitmapTimeUs = 0; // cumulative getBitmap time (micros) uint32_t getBitmapCalls = 0; // number of getBitmap calls }; @@ -65,22 +64,11 @@ class FontDecompressor { static constexpr uint16_t HOT_GLYPH_BUF_SIZE = 512; // largest packed single glyph - // Hot group: single heap buffer sized to the largest group of the active font. - // Allocated once per font (lazily in ensureHotGroupBuf) and freed in freeHotGroup(). - // Single malloc per font session — no repeated alloc/free per page. - const EpdFontData* hotGroupFont = nullptr; - uint16_t hotGroupIndex = UINT16_MAX; - uint32_t _hotGroupBufUsed = 0; - uint8_t* _hotGroupBuf = nullptr; - uint32_t _hotGroupBufSize = 0; - - // Scratch buffer for compacting a single glyph out of the byte-aligned hot group. + // Scratch buffer for compacting a single glyph after a getBitmap() miss. // Valid until the next getBitmap() call. uint8_t _hotGlyphBuf[HOT_GLYPH_BUF_SIZE]; void freePageBuffer(); - void freeHotGroup(); - bool ensureHotGroupBuf(const EpdFontData* fontData); uint16_t getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex); uint32_t getAlignedOffset(const EpdFontData* fontData, uint16_t groupIndex, uint32_t glyphIndex); bool decompressGroup(const EpdFontData* fontData, uint16_t groupIndex, uint8_t* outBuf, uint32_t outSize); From 23a77c6aa58279e715761c9aa3444261ae305e72 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Wed, 22 Apr 2026 08:36:36 +0200 Subject: [PATCH 4/9] fontconvert.py: cap group uncompressed size at 64 KB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Script-range boundaries alone don't bound group size — dense Unicode blocks (CJK, user fonts) can produce groups of hundreds of KB. Add a 64 KB hard cap: when adding the next glyph would exceed it, close the current group and start a new one with the same script ID. 64 KB is well above the largest current built-in group (~50 KB for notosans_18 Cyrillic) and a safe transient malloc on the ESP32-C3. The decompressor side already handles any number of groups per font. Co-Authored-By: Claude Sonnet 4.6 --- lib/EpdFont/scripts/fontconvert.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/EpdFont/scripts/fontconvert.py b/lib/EpdFont/scripts/fontconvert.py index beb7d2bb..64f6c228 100755 --- a/lib/EpdFont/scripts/fontconvert.py +++ b/lib/EpdFont/scripts/fontconvert.py @@ -726,6 +726,12 @@ if compress: # are grouped together for efficient LRU caching on the embedded target. # Since glyphs are in codepoint order, glyphs in the same Unicode block # are contiguous in the array and form natural groups. + # + # A hard size cap (GROUP_MAX_UNCOMPRESSED_BYTES) is applied on top of script + # boundaries: if adding the next glyph would push the uncompressed group size + # over the cap, the group is closed and a new one started with the same script + # ID. This keeps the embedded decompressor's transient malloc bounded regardless + # of font density (CJK, Vietnamese, user-supplied fonts with large Unicode blocks). SCRIPT_GROUP_RANGES = [ (0x0000, 0x007F), # ASCII (0x0080, 0x00FF), # Latin-1 Supplement @@ -743,6 +749,10 @@ if compress: (0xFFFD, 0xFFFD), # Replacement Character ] + # 64 KB cap: large enough to hold any single built-in font group with headroom, + # small enough to be a comfortable transient malloc on the ESP32-C3. + GROUP_MAX_UNCOMPRESSED_BYTES = 65536 + def get_script_group(code_point): for i, (start, end) in enumerate(SCRIPT_GROUP_RANGES): if start <= code_point <= end: @@ -753,17 +763,23 @@ if compress: current_group_id = None group_start = 0 group_count = 0 + group_uncompressed = 0 for i, (props, packed) in enumerate(all_glyphs): sg = get_script_group(props.code_point) - if sg != current_group_id: + glyph_aligned_size = ((props.width + 3) // 4) * props.height if props.width > 0 and props.height > 0 else 0 + size_overflow = group_uncompressed + glyph_aligned_size > GROUP_MAX_UNCOMPRESSED_BYTES + + if sg != current_group_id or size_overflow: if group_count > 0: groups.append((group_start, group_count)) current_group_id = sg group_start = i group_count = 1 + group_uncompressed = glyph_aligned_size else: group_count += 1 + group_uncompressed += glyph_aligned_size if group_count > 0: groups.append((group_start, group_count)) From 7291ccacc87abf6a0b8c5d701e1650f560210eda Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 24 Apr 2026 16:23:14 +0200 Subject: [PATCH 5/9] Further code review --- lib/EpdFont/FontDecompressor.cpp | 38 +++++++++++++++++++------------- lib/EpdFont/FontDecompressor.h | 1 + lib/Epub/Epub/ParsedText.cpp | 2 ++ 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index 38fe095c..8cdc1d12 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -5,6 +5,7 @@ #include #include +#include FontDecompressor::~FontDecompressor() { deinit(); } @@ -263,12 +264,14 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 // Step 2: Compute total buffer size and collect unique groups uint32_t totalBytes = 0; uint16_t neededGroups[128]; + uint16_t neededGlyphGroups[MAX_PAGE_GLYPHS]; // parallel to neededGlyphs; avoids re-calling getGroupIndex later uint8_t groupCount = 0; bool groupCapWarned = false; for (uint16_t i = 0; i < glyphCount; i++) { totalBytes += fontData->glyph[neededGlyphs[i]].dataLength; - uint16_t gi = getGroupIndex(fontData, neededGlyphs[i]); + const uint16_t gi = getGroupIndex(fontData, neededGlyphs[i]); + neededGlyphGroups[i] = gi; bool found = false; for (uint8_t j = 0; j < groupCount; j++) { if (neededGroups[j] == gi) { @@ -319,7 +322,7 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 // Initialize lookup entries (bufferOffset = UINT32_MAX means not yet extracted) for (uint16_t i = 0; i < glyphCount; i++) { - slot.glyphs[i] = {neededGlyphs[i], UINT32_MAX, 0}; + slot.glyphs[i] = {neededGlyphs[i], UINT32_MAX, 0, neededGlyphGroups[i]}; } // Sort by glyphIndex for binary search in getBitmap() @@ -338,21 +341,25 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 uint32_t groupAlignedTracker[128] = {}; // running byte-aligned offset for each needed group if (fontData->glyphToGroup) { - // Frequency-grouped: single O(totalGlyphs) pass through glyphToGroup + // Frequency-grouped: single O(totalGlyphs) pass through glyphToGroup. + // Reverse map (fontGroupIdx → position in neededGroups) replaces the inner + // linear scan, dropping this pass from O(totalGlyphs × groupCount) to O(totalGlyphs). + uint8_t* groupIdToPos = static_cast(malloc(fontData->groupCount)); + if (!groupIdToPos) { + LOG_ERR("FDC", "OOM: cannot allocate %u bytes for groupIdToPos map", fontData->groupCount); + freePageBuffer(); + return glyphCount; + } + memset(groupIdToPos, 0xFF, fontData->groupCount); + for (uint8_t j = 0; j < groupCount; j++) groupIdToPos[neededGroups[j]] = j; + const auto& lastInterval = fontData->intervals[fontData->intervalCount - 1]; const uint32_t totalGlyphs = lastInterval.offset + (lastInterval.last - lastInterval.first + 1); for (uint32_t i = 0; i < totalGlyphs; i++) { const uint16_t gi = fontData->glyphToGroup[i]; - // Find this glyph's group position in neededGroups - uint8_t gpPos = groupCount; - for (uint8_t j = 0; j < groupCount; j++) { - if (neededGroups[j] == gi) { - gpPos = j; - break; - } - } - if (gpPos == groupCount) continue; // not a needed group + const uint8_t gpPos = groupIdToPos[gi]; + if (gpPos == 0xFF) continue; // not a needed group const EpdGlyph& glyph = fontData->glyph[i]; @@ -374,6 +381,8 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 groupAlignedTracker[gpPos] += ((glyph.width + 3) / 4) * glyph.height; } } + + free(groupIdToPos); } else { // Contiguous-group: iterate each needed group's glyphs directly for (uint8_t g = 0; g < groupCount; g++) { @@ -432,11 +441,10 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 // alignedOffset was pre-computed in step 3b — no full-group compact scan needed. for (uint16_t i = 0; i < slot.glyphCount; i++) { if (slot.glyphs[i].bufferOffset != UINT32_MAX) continue; // already extracted - if (getGroupIndex(fontData, slot.glyphs[i].glyphIndex) != groupIdx) continue; + if (slot.glyphs[i].groupIndex != groupIdx) continue; const EpdGlyph& glyph = fontData->glyph[slot.glyphs[i].glyphIndex]; - compactSingleGlyph(&groupBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, - glyph.height); + compactSingleGlyph(&groupBuf[slot.glyphs[i].alignedOffset], &slot.buffer[writeOffset], glyph.width, glyph.height); slot.glyphs[i].bufferOffset = writeOffset; writeOffset += glyph.dataLength; } diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index 2894406a..cdf24b9a 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -52,6 +52,7 @@ class FontDecompressor { uint32_t glyphIndex; uint32_t bufferOffset; uint32_t alignedOffset; // byte-aligned offset within its decompressed group (set during prewarm pre-scan) + uint16_t groupIndex; // cached to avoid re-calling getGroupIndex in prewarm Step 4 }; struct PageSlot { uint8_t* buffer = nullptr; diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index aad424cf..ecac74c1 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -35,6 +35,8 @@ bool isClosingPunctuation(const uint32_t cp) { case 0x2019: // ' right single quotation mark case 0x201D: // " right double quotation mark case 0x2026: // … ellipsis + case 0x2013: // – en dash + case 0x2014: // — em dash return true; default: return false; From 6f312023329865ff8a312355a50075f7fc6089f1 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 24 Apr 2026 16:37:39 +0200 Subject: [PATCH 6/9] Minor adjustment --- lib/EpdFont/FontDecompressor.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index 8cdc1d12..1f34186b 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -347,7 +347,11 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 uint8_t* groupIdToPos = static_cast(malloc(fontData->groupCount)); if (!groupIdToPos) { LOG_ERR("FDC", "OOM: cannot allocate %u bytes for groupIdToPos map", fontData->groupCount); - freePageBuffer(); + // Roll back this slot only (other slots from prior prewarmCache calls stay valid) + free(slot.buffer); + free(slot.glyphs); + slot = {}; + pageSlotCount--; return glyphCount; } memset(groupIdToPos, 0xFF, fontData->groupCount); From 76bed0b381e412deed3d2edfcf44e864c8fc9d7f Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 24 Apr 2026 17:27:50 +0200 Subject: [PATCH 7/9] Amend stats --- src/activities/reader/EpubReaderActivity.cpp | 4 +--- src/activities/reader/EpubReaderActivity.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 1db9674e..a70c8308 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -824,8 +824,7 @@ std::string EpubReaderActivity::buildRenderBenchmarkReport(const LastRenderStats std::to_string(endSnapshot.fontDecompressMs) + " ms, groups " + std::to_string(endSnapshot.fontUniqueGroups)); appendLine("Font buffers: page " + std::to_string(endSnapshot.fontPageBufferBytes) + ", glyph table " + - std::to_string(endSnapshot.fontPageGlyphsBytes) + ", hot group " + - std::to_string(endSnapshot.fontHotGroupBytes) + ", peak temp " + + std::to_string(endSnapshot.fontPageGlyphsBytes) + ", peak temp " + std::to_string(endSnapshot.fontPeakTempBytes)); appendLine("Glyph lookups: " + std::to_string(endSnapshot.fontGetBitmapCalls) + " calls, " + std::to_string(endSnapshot.fontGetBitmapTimeUs) + " us total"); @@ -1575,7 +1574,6 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or lastRenderStats.fontUniqueGroups = stats.uniqueGroupsAccessed; lastRenderStats.fontPageBufferBytes = stats.pageBufferBytes; lastRenderStats.fontPageGlyphsBytes = stats.pageGlyphsBytes; - lastRenderStats.fontHotGroupBytes = stats.hotGroupBytes; lastRenderStats.fontPeakTempBytes = stats.peakTempBytes; lastRenderStats.fontGetBitmapTimeUs = stats.getBitmapTimeUs; lastRenderStats.fontGetBitmapCalls = stats.getBitmapCalls; diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 1a43f58d..8185139f 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -85,7 +85,6 @@ class EpubReaderActivity final : public Activity { uint16_t fontUniqueGroups = 0; uint32_t fontPageBufferBytes = 0; uint32_t fontPageGlyphsBytes = 0; - uint32_t fontHotGroupBytes = 0; uint32_t fontPeakTempBytes = 0; uint32_t fontGetBitmapTimeUs = 0; uint32_t fontGetBitmapCalls = 0; From a73836c5948c3526906d7cf204d40f157a5f3447 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 24 Apr 2026 18:08:37 +0200 Subject: [PATCH 8/9] cppcheck complains --- lib/EpdFont/FontDecompressor.h | 2 +- src/activities/reader/EpubReaderActivity.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index cdf24b9a..e4bf5c05 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -67,7 +67,7 @@ class FontDecompressor { // Scratch buffer for compacting a single glyph after a getBitmap() miss. // Valid until the next getBitmap() call. - uint8_t _hotGlyphBuf[HOT_GLYPH_BUF_SIZE]; + uint8_t _hotGlyphBuf[HOT_GLYPH_BUF_SIZE] = {}; void freePageBuffer(); uint16_t getGroupIndex(const EpdFontData* fontData, uint32_t glyphIndex); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index a70c8308..cca2e9b0 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -1119,7 +1119,7 @@ int EpubReaderActivity::getEffectiveReaderFontId() const { } bool EpubReaderActivity::stepPageState(const bool isForwardTurn) { - if (!epub || !section || section->pageCount <= 0) { + if (!epub || !section || section->pageCount == 0) { return false; } @@ -1565,8 +1565,8 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or tEnd - t0); } - if (auto* cacheManager = renderer.getFontCacheManager()) { - if (auto* decompressor = cacheManager->getDecompressor()) { + if (const auto* cacheManager = renderer.getFontCacheManager()) { + if (const auto* decompressor = cacheManager->getDecompressor()) { const auto& stats = decompressor->getStats(); lastRenderStats.fontCacheHits = stats.cacheHits; lastRenderStats.fontCacheMisses = stats.cacheMisses; From 37395f8e0f24e24c627af507e479f16d43bb0eb9 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 24 Apr 2026 19:42:18 +0200 Subject: [PATCH 9/9] Final review --- lib/EpdFont/FontDecompressor.cpp | 17 +++++++++-------- lib/EpdFont/FontDecompressor.h | 6 +++++- lib/EpdFont/scripts/fontconvert.py | 7 ++++++- lib/Epub/Epub/ParsedText.cpp | 23 ++++++++++++----------- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/EpdFont/FontDecompressor.cpp b/lib/EpdFont/FontDecompressor.cpp index 1f34186b..a0153155 100644 --- a/lib/EpdFont/FontDecompressor.cpp +++ b/lib/EpdFont/FontDecompressor.cpp @@ -112,7 +112,7 @@ void FontDecompressor::compactSingleGlyph(const uint8_t* alignedSrc, uint8_t* pa if (outBits > 0) packedDst[writeIdx] = outByte << (8 - outBits); } -// --- getBitmap: page buffer → hot group → decompress --- +// --- getBitmap: page buffer → transient malloc + decompress + compact --- const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const EpdGlyph* glyph, uint32_t glyphIndex) { const uint32_t tStart = micros(); @@ -159,6 +159,12 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep stats.cacheMisses++; const EpdFontGroup& group = fontData->groups[groupIndex]; + if (glyph->dataLength > HOT_GLYPH_BUF_SIZE) { + LOG_ERR("FDC", "Glyph dataLength %u exceeds HOT_GLYPH_BUF_SIZE %u", glyph->dataLength, HOT_GLYPH_BUF_SIZE); + stats.getBitmapTimeUs += micros() - tStart; + return nullptr; + } + if (group.uncompressedSize > stats.peakTempBytes) stats.peakTempBytes = group.uncompressedSize; uint8_t* groupBuf = static_cast(malloc(group.uncompressedSize)); @@ -174,13 +180,6 @@ const uint8_t* FontDecompressor::getBitmap(const EpdFontData* fontData, const Ep return nullptr; } - if (glyph->dataLength > HOT_GLYPH_BUF_SIZE) { - LOG_ERR("FDC", "Glyph dataLength %u exceeds HOT_GLYPH_BUF_SIZE %u", glyph->dataLength, HOT_GLYPH_BUF_SIZE); - free(groupBuf); - stats.getBitmapTimeUs += micros() - tStart; - return nullptr; - } - uint32_t alignedOff = getAlignedOffset(fontData, groupIndex, glyphIndex); compactSingleGlyph(&groupBuf[alignedOff], _hotGlyphBuf, glyph->width, glyph->height); free(groupBuf); @@ -348,6 +347,8 @@ int FontDecompressor::prewarmCache(const EpdFontData* fontData, const char* utf8 if (!groupIdToPos) { LOG_ERR("FDC", "OOM: cannot allocate %u bytes for groupIdToPos map", fontData->groupCount); // Roll back this slot only (other slots from prior prewarmCache calls stay valid) + stats.pageBufferBytes -= totalBytes; + stats.pageGlyphsBytes -= glyphCount * sizeof(PageGlyphEntry); free(slot.buffer); free(slot.glyphs); slot = {}; diff --git a/lib/EpdFont/FontDecompressor.h b/lib/EpdFont/FontDecompressor.h index e4bf5c05..8f574dba 100644 --- a/lib/EpdFont/FontDecompressor.h +++ b/lib/EpdFont/FontDecompressor.h @@ -16,7 +16,11 @@ class FontDecompressor { void deinit(); // Returns pointer to decompressed bitmap data for the given glyph. - // Checks the page buffer (from prewarm) first, then falls back to the hot group slot. + // Checks the page buffer (from prewarm) first and otherwise transiently + // allocates/decompresses the glyph's group into a temporary buffer and + // compacts the requested glyph. The returned pointer is valid only until the + // next getBitmap call or cache eviction; callers must copy bitmap data if a + // longer lifetime is required. const uint8_t* getBitmap(const EpdFontData* fontData, const EpdGlyph* glyph, uint32_t glyphIndex); // Free all cached data (page buffers). diff --git a/lib/EpdFont/scripts/fontconvert.py b/lib/EpdFont/scripts/fontconvert.py index 64f6c228..69b06c26 100755 --- a/lib/EpdFont/scripts/fontconvert.py +++ b/lib/EpdFont/scripts/fontconvert.py @@ -765,9 +765,14 @@ if compress: group_count = 0 group_uncompressed = 0 - for i, (props, packed) in enumerate(all_glyphs): + for i, (props, _) in enumerate(all_glyphs): sg = get_script_group(props.code_point) glyph_aligned_size = ((props.width + 3) // 4) * props.height if props.width > 0 and props.height > 0 else 0 + if glyph_aligned_size > GROUP_MAX_UNCOMPRESSED_BYTES: + raise ValueError( + f"Glyph {i} (code point U+{props.code_point:04X}) single aligned size " + f"{glyph_aligned_size} exceeds GROUP_MAX_UNCOMPRESSED_BYTES={GROUP_MAX_UNCOMPRESSED_BYTES}" + ) size_overflow = group_uncompressed + glyph_aligned_size > GROUP_MAX_UNCOMPRESSED_BYTES if sg != current_group_id or size_overflow: diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index ecac74c1..f24550c7 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -18,7 +18,9 @@ constexpr int MAX_COST = std::numeric_limits::max(); namespace { // Closing punctuation that should not have extra space inserted before it during justification. -// Includes common closing brackets/quotes and sentence-ending marks. +// Includes common closing brackets/quotes and sentence-ending marks. En/em dashes +// are also treated as inline separators here to avoid justification stretch +// immediately before them. bool isClosingPunctuation(const uint32_t cp) { switch (cp) { case '.': @@ -804,17 +806,16 @@ ParsedText::LineProcessResult ParsedText::extractLine( // Count gaps: each word after the first creates a gap, unless it's a continuation. // Gaps before closing punctuation (. , ) » etc.) are excluded from justification // distribution so they stay at natural space width. + const uint32_t firstCp = firstCodepoint(words[lastBreakAt + wordIdx]); if (wordIdx > 0 && !continuesVec[lastBreakAt + wordIdx]) { - const bool beforeClosing = isClosingPunctuation(firstCodepoint(words[lastBreakAt + wordIdx])); + const bool beforeClosing = isClosingPunctuation(firstCp); if (!beforeClosing) actualGapCount++; - totalNaturalGaps += - renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]), - firstCodepoint(words[lastBreakAt + wordIdx]), wordStyles[lastBreakAt + wordIdx - 1]); + totalNaturalGaps += renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]), firstCp, + wordStyles[lastBreakAt + wordIdx - 1]); } else if (wordIdx > 0 && continuesVec[lastBreakAt + wordIdx]) { // Cross-boundary kerning for continuation words (e.g. nonbreaking spaces, attached punctuation) - totalNaturalGaps += - renderer.getKerning(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]), - firstCodepoint(words[lastBreakAt + wordIdx]), wordStyles[lastBreakAt + wordIdx - 1]); + totalNaturalGaps += renderer.getKerning(fontId, lastCodepoint(words[lastBreakAt + wordIdx - 1]), firstCp, + wordStyles[lastBreakAt + wordIdx - 1]); } } @@ -859,12 +860,12 @@ ParsedText::LineProcessResult ParsedText::extractLine( } else { int gap = 0; if (wordIdx + 1 < lineWordCount) { - gap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx]), - firstCodepoint(words[lastBreakAt + wordIdx + 1]), + const uint32_t nextFirstCp = firstCodepoint(words[lastBreakAt + wordIdx + 1]); + gap = renderer.getSpaceAdvance(fontId, lastCodepoint(words[lastBreakAt + wordIdx]), nextFirstCp, wordStyles[lastBreakAt + wordIdx]); // Don't stretch the gap before closing punctuation — it looks wrong with // extra space before ".", ")", "»" etc. - const bool nextIsClosing = isClosingPunctuation(firstCodepoint(words[lastBreakAt + wordIdx + 1])); + const bool nextIsClosing = isClosingPunctuation(nextFirstCp); if (blockStyle.alignment == CssTextAlign::Justify && !isLastLine && !nextIsClosing) { gap += justifyExtra; }