docs/font-compression.md (1)

3-35: Add measurement context for the reported metrics.

The tables are useful, but they’re hard to reproduce without stating the target (device/emulator), build profile/toolchain, measurement command, date, and what the percentages are relative to. A short context section would prevent misinterpretation.

✍️ Suggested doc addition

 ## Flash & RAM
+
+### Measurement context
+- Target: <device/emulator>
+- Build profile/toolchain: <profile + version>
+- Command/script: <how sizes were measured>
+- Date/commit: <timestamp or commit SHA>
+- Percentages: relative to <configured app partition size>
 
 | | baseline | font-compression | Difference |

lib/EpdFont/EpdFontData.h (1)

18-45: Document or widen uint16_t limits on glyph and group counts.

glyphCount, firstGlyphIndex, and groupCount use uint16_t, which caps fonts at 65,535 glyphs per group/total. Current fonts reach only 877 glyphs, well below the limit. However, no assertions or documentation enforce this constraint. For future fonts, either document this limit explicitly or widen these fields to uint32_t to avoid overflow risks.

src/activities/reader/EpubReaderActivity.cpp (1)

648-650: Add a short rationale for per‑page font‑cache clears

If the cache is intentionally page‑scoped (heap/fragmentation control), a brief comment will prevent accidental removal later.

♻️ Suggested comment

-    const auto start = millis();
-    renderer.clearFontCache();
+    const auto start = millis();
+    // Clear per page to limit heap usage and avoid stale decompressed glyphs.
+    renderer.clearFontCache();

lib/uzlib/src/tinf.h (1)

1-3: Add a header guard / pragma once

Even for a thin compatibility header, a guard prevents accidental multiple inclusion if it expands later.

♻️ Suggested update

+#pragma once
 /* Compatibility header for the original tinf lib/older versions of uzlib.
    Note: may be removed in the future, please migrate to uzlib.h. */
 `#include` "uzlib.h"

lib/uzlib/src/tinf_compat.h (1)

1-9: Add an include guard/pragma once for this new compatibility header.

Without a guard, repeated includes can re-define these macros and trigger warnings on some toolchains. Consider adding #pragma once (or a classic guard) at the top.

♻️ Proposed tweak

+#pragma once
+
 /* This header contains compatibility defines for the original tinf API
    and uzlib 2.x and below API. These defines are deprecated and going
    to be removed in the future, so applications should migrate to new
    uzlib API. */

lib/ZipFile/ZipFile.cpp (1)

9-29: Validate inflated size to catch corrupt ZIPs.

Line 21: tinfl_decompress can return DONE even if the produced byte count doesn’t match the expected inflatedSize. Checking outBytes (and optionally inBytes) avoids returning partially filled buffers.

✅ Proposed check

   const tinfl_status status = tinfl_decompress(inflator, inputBuf, &inBytes, nullptr, outputBuf, &outBytes,
                                                TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF);

   free(inflator);

-  if (status != TINFL_STATUS_DONE) {
+  if (status != TINFL_STATUS_DONE || outBytes != inflatedSize || inBytes != deflatedSize) {
     LOG_ERR("ZIP", "tinfl_decompress() failed with status %d", status);
     return false;
   }

lib/uzlib/src/defl_static.h (1)

1-37: Add an include guard/pragma once for this new header.

This prevents duplicate prototype declarations if included multiple times in a TU.

♻️ Proposed tweak

+#pragma once
+
 /*
  * Copyright (c) uzlib authors
  *

lib/EpdFont/FontDecompressor.h (1)

9-35: Make FontDecompressor non-copyable to avoid shallow pointer copies.

This class owns raw buffers in CacheEntry. Deleting copy/assign prevents accidental double-free or corruption.

♻️ Proposed safeguard

 class FontDecompressor {
  public:
+  FontDecompressor() = default;
+  FontDecompressor(const FontDecompressor&) = delete;
+  FontDecompressor& operator=(const FontDecompressor&) = delete;
   bool init();
   void deinit();

🤖 Fix all nitpicks with AI agents

Verify each finding against the current code and only fix it if needed.


In `@lib/EpdFont/FontDecompressor.cpp`:
- Around line 14-33: Extract the duplicated cleanup logic in
FontDecompressor::deinit and FontDecompressor::clearCache into a private helper
(e.g., clearCacheEntries or freeCacheEntries) that iterates over cache, frees
entry.data, sets entry.data = nullptr and entry.valid = false; then have deinit
call that helper and clearCache call the helper and also reset accessCounter =
0. Update references to cache and accessCounter in those methods accordingly so
behavior stays identical and duplication is removed.

In `@lib/GfxRenderer/GfxRenderer.cpp`:
- Around line 6-16: The getGlyphBitmap path detects compressed fonts
(fontData->groups != nullptr) and currently calls LOG_ERR("GFX", "Compressed
font but no FontDecompressor set") every glyph when fontDecompressor is null;
add a log-once guard (e.g. a static or GfxRenderer member boolean like
decompressorMissingLogged) checked and set inside GfxRenderer::getGlyphBitmap
before calling LOG_ERR so the error is emitted only once per process or renderer
instance, leaving the existing return nullptr behavior unchanged.

lib/EpdFont/FontDecompressor.cpp (1)

14-33: Consider deduplicating cache cleanup.

deinit() and clearCache() share identical cleanup logic. A small helper would reduce duplication and keep behavior consistent if you tweak it later.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/EpdFont/FontDecompressor.cpp` around lines 14 - 33, Extract the
duplicated cleanup logic in FontDecompressor::deinit and
FontDecompressor::clearCache into a private helper (e.g., clearCacheEntries or
freeCacheEntries) that iterates over cache, frees entry.data, sets entry.data =
nullptr and entry.valid = false; then have deinit call that helper and
clearCache call the helper and also reset accessCounter = 0. Update references
to cache and accessCounter in those methods accordingly so behavior stays
identical and duplication is removed.

lib/GfxRenderer/GfxRenderer.cpp (1)

6-16: Consider rate‑limiting the “no decompressor” error.

If a compressed font is rendered without a decompressor, this logs once per glyph and can flood logs / slow rendering. A log‑once guard keeps the error visible without spamming.

🔇 Log-once guard

   if (fontData->groups != nullptr) {
     if (!fontDecompressor) {
-      LOG_ERR("GFX", "Compressed font but no FontDecompressor set");
+      static bool logged = false;
+      if (!logged) {
+        LOG_ERR("GFX", "Compressed font but no FontDecompressor set");
+        logged = true;
+      }
       return nullptr;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/GfxRenderer/GfxRenderer.cpp` around lines 6 - 16, The getGlyphBitmap path
detects compressed fonts (fontData->groups != nullptr) and currently calls
LOG_ERR("GFX", "Compressed font but no FontDecompressor set") every glyph when
fontDecompressor is null; add a log-once guard (e.g. a static or GfxRenderer
member boolean like decompressorMissingLogged) checked and set inside
GfxRenderer::getGlyphBitmap before calling LOG_ERR so the error is emitted only
once per process or renderer instance, leaving the existing return nullptr
behavior unchanged.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

🤖 Fix all nitpicks with AI agents

Verify each finding against the current code and only fix it if needed.


In `@lib/EpdFont/FontDecompressor.cpp`:
- Around line 14-33: Both FontDecompressor::deinit and
FontDecompressor::clearCache duplicate the same loop that frees cache entries;
extract that logic into a private helper (e.g.,
FontDecompressor::freeAllEntries) which iterates over cache, frees entry.data
and sets entry.data = nullptr and entry.valid = false, then call
freeAllEntries() from both deinit() and clearCache(); ensure clearCache still
resets accessCounter = 0 after calling the helper.

In `@lib/EpdFont/scripts/verify_compression.py`:
- Around line 15-18: The parse_hex_array function's regex only matches '0x' with
exactly two hex digits so it will miss variants like '0X' or single-digit bytes
such as '0xA'; update the pattern used in parse_hex_array to accept either '0x'
or '0X' and 1–2 hex digits (e.g. use a pattern like r'0[xX]([0-9A-Fa-f]{1,2})'),
keep converting each capture with int(h, 16) and returning bytes(...) so
single-digit captures are parsed correctly into their numeric byte values.

lib/EpdFont/FontDecompressor.cpp (1)

14-33: Consider extracting shared cache-clearing logic.

deinit() and clearCache() share identical loops for freeing cache entries. You could extract a private helper (e.g., freeAllEntries()) to reduce duplication.

♻️ Proposed refactor

+void FontDecompressor::freeAllEntries() {
+  for (auto& entry : cache) {
+    if (entry.data) {
+      free(entry.data);
+      entry.data = nullptr;
+    }
+    entry.valid = false;
+  }
+}
+
 void FontDecompressor::deinit() {
-  for (auto& entry : cache) {
-    if (entry.data) {
-      free(entry.data);
-      entry.data = nullptr;
-    }
-    entry.valid = false;
-  }
+  freeAllEntries();
 }
 
 void FontDecompressor::clearCache() {
-  for (auto& entry : cache) {
-    if (entry.data) {
-      free(entry.data);
-      entry.data = nullptr;
-    }
-    entry.valid = false;
-  }
+  freeAllEntries();
   accessCounter = 0;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/EpdFont/FontDecompressor.cpp` around lines 14 - 33, Both
FontDecompressor::deinit and FontDecompressor::clearCache duplicate the same
loop that frees cache entries; extract that logic into a private helper (e.g.,
FontDecompressor::freeAllEntries) which iterates over cache, frees entry.data
and sets entry.data = nullptr and entry.valid = false, then call
freeAllEntries() from both deinit() and clearCache(); ensure clearCache still
resets accessCounter = 0 after calling the helper.

lib/EpdFont/scripts/verify_compression.py (1)

15-18: Make hex parsing tolerant of 0X or single‑digit bytes.

Optional robustness: if the generator ever emits 0X or 0xA, the current regex would drop bytes. Broadening the pattern avoids false negatives with minimal risk.

♻️ Suggested tweak

-    hex_vals = re.findall(r'0x([0-9A-Fa-f]{2})', text)
+    hex_vals = re.findall(r'0[xX]([0-9A-Fa-f]{1,2})', text)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/EpdFont/scripts/verify_compression.py` around lines 15 - 18, The
parse_hex_array function's regex only matches '0x' with exactly two hex digits
so it will miss variants like '0X' or single-digit bytes such as '0xA'; update
the pattern used in parse_hex_array to accept either '0x' or '0X' and 1–2 hex
digits (e.g. use a pattern like r'0[xX]([0-9A-Fa-f]{1,2})'), keep converting
each capture with int(h, 16) and returning bytes(...) so single-digit captures
are parsed correctly into their numeric byte values.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

🤖 Fix all nitpicks with AI agents

Verify each finding against the current code and only fix it if needed.


In `@lib/EpdFont/FontDecompressor.cpp`:
- Around line 110-136: In FontDecompressor::getBitmap add defensive bounds
checks for glyph->dataOffset before indexing into fontData->bitmap and
entry->data: ensure glyph->dataOffset is less than the corresponding buffer size
(for non-grouped case, compare against fontData->bitmap length; for
cached/decompressed case, compare against the decompressed entry->size or buffer
length in CacheEntry) and if out-of-range log an error (e.g., via LOG_ERR) and
return nullptr; adjust checks around the findInCache and decompressGroup paths
(functions findInCache, decompressGroup, and the CacheEntry structure) so you
never access entry->data[glyph->dataOffset] or
&fontData->bitmap[glyph->dataOffset] without validating the offset.

lib/EpdFont/FontDecompressor.cpp (1)

110-136: Consider bounds-checking glyph->dataOffset for defense-in-depth.

Lines 125 and 135 access entry->data[glyph->dataOffset] without verifying that dataOffset is within the decompressed buffer size. While the font generation tooling and verify_compression.py should ensure consistency, a bounds check would guard against corrupt or mismatched metadata.

Given this is a performance-sensitive path and the data is tooling-generated, this is optional.

🛡️ Optional defensive bounds check

   // Check cache
   CacheEntry* entry = findInCache(fontData, groupIndex);
   if (entry) {
     entry->lastUsed = ++accessCounter;
+    if (glyph->dataOffset >= entry->dataSize) {
+      LOG_ERR("FDC", "dataOffset %u out of bounds for group %u (size %u)", glyph->dataOffset, groupIndex, entry->dataSize);
+      return nullptr;
+    }
     return &entry->data[glyph->dataOffset];
   }

   // Cache miss - decompress
   entry = findEvictionCandidate();
   if (!decompressGroup(fontData, groupIndex, entry)) {
     return nullptr;
   }

   entry->lastUsed = ++accessCounter;
+  if (glyph->dataOffset >= entry->dataSize) {
+    LOG_ERR("FDC", "dataOffset %u out of bounds for group %u (size %u)", glyph->dataOffset, groupIndex, entry->dataSize);
+    return nullptr;
+  }
   return &entry->data[glyph->dataOffset];
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/EpdFont/FontDecompressor.cpp` around lines 110 - 136, In
FontDecompressor::getBitmap add defensive bounds checks for glyph->dataOffset
before indexing into fontData->bitmap and entry->data: ensure glyph->dataOffset
is less than the corresponding buffer size (for non-grouped case, compare
against fontData->bitmap length; for cached/decompressed case, compare against
the decompressed entry->size or buffer length in CacheEntry) and if out-of-range
log an error (e.g., via LOG_ERR) and return nullptr; adjust checks around the
findInCache and decompressGroup paths (functions findInCache, decompressGroup,
and the CacheEntry structure) so you never access entry->data[glyph->dataOffset]
or &fontData->bitmap[glyph->dataOffset] without validating the offset.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro