lib/Epub/Epub/ParsedText.cpp (2)

168-181: Extract shared inter-word-gap precompute into a helper to avoid drift.

The gap-building logic is now duplicated in two paths. A shared helper will keep behavior aligned and reduce maintenance risk.

♻️ Suggested refactor

+namespace {
+std::vector<int> buildInterWordGaps(const GfxRenderer& renderer, int fontId,
+                                    const std::vector<std::string>& words,
+                                    const std::vector<EpdFontFamily::Style>& wordStyles,
+                                    const std::vector<bool>& continuesVec) {
+  std::vector<int> gaps(words.size(), 0);
+  for (size_t j = 1; j < words.size(); ++j) {
+    gaps[j] = !continuesVec[j]
+                  ? renderer.getSpaceAdvance(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]),
+                                             wordStyles[j - 1])
+                  : renderer.getKerning(fontId, lastCodepoint(words[j - 1]), firstCodepoint(words[j]),
+                                        wordStyles[j - 1]);
+  }
+  return gaps;
+}
+}  // namespace

Then replace both local precompute blocks with:

-std::vector<int> interWordGaps(...);
-...
+std::vector<int> interWordGaps = buildInterWordGaps(renderer, fontId, words, wordStyles, continuesVec);

Also applies to: 293-307

🤖 Prompt for AI Agents

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

In `@lib/Epub/Epub/ParsedText.cpp` around lines 168 - 181, The duplicated
inter-word-gap precompute should be extracted into a helper function to keep
behavior consistent; implement a function (e.g., computeInterWordGaps) that
takes fontId, totalWordCount, words, continuesVec, wordStyles and renderer and
returns the std::vector<int> interWordGaps by using
lastCodepoint(words[j-1])/firstCodepoint(words[j]) and calling
renderer.getSpaceAdvance or renderer.getKerning depending on continuesVec[j],
then replace both local blocks that build interWordGaps (the one using
continuesVec and renderer.getSpaceAdvance/getKerning) with calls to this helper
to avoid drift.

---

338-340: Guard the gap-vector sync invariant with assertions.

This insertion strategy is valid but depends on strict vector alignment invariants. Add explicit assert() checks after split+insert so future changes fail fast if alignment breaks.

🛡️ Suggested hardening

+#include <cassert>
...
       if (availableWidth > 0 &&
           hyphenateWordAtIndex(currentIndex, availableWidth, renderer, fontId, wordWidths, allowFallbackBreaks)) {
         // Keep interWordGaps in sync: insert placeholder for the new remainder word.
         // The remainder is always the first word on the next line so this slot is never read.
         interWordGaps.insert(interWordGaps.begin() + currentIndex + 1, 0);
+        assert(interWordGaps.size() == wordWidths.size());
+        assert(wordWidths.size() == words.size());
+        assert(wordStyles.size() == words.size());
+        assert(continuesVec.size() == words.size());

Based on learnings: In this codebase, assertions are always enabled and should be used to enforce invariants (side-effect free, inexpensive).

🤖 Prompt for AI Agents

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

In `@lib/Epub/Epub/ParsedText.cpp` around lines 338 - 340, After inserting the
placeholder gap, add lightweight assert checks to enforce the inter-word gap
alignment: immediately after interWordGaps.insert(...) assert that currentIndex
+ 1 is a valid index (currentIndex + 1 < interWordGaps.size()) and that the
newly inserted slot equals 0 (interWordGaps[currentIndex + 1] == 0); also add an
assertion that the interWordGaps vector remains aligned with the corresponding
word/token container used in this routine (e.g., the words/wordSlots vector used
by ParsedText) so sizes match after the insert.

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp (1)

297-307: Reuse the already-resolved cssStyle in the <img> path.

For img, cssStyle already contains the same stylesheet + inline-style merge, so these extra cache probes cannot change behavior. Removing them trims per-image hot-path work and gives you one place to apply the cache policy consistently.

♻️ Suggested simplification

-      if (self->cssParser) {
-        std::string imgCacheKey("img|");
-        imgCacheKey += classAttr;
-        auto imgIt = self->cssStyleCache_.find(imgCacheKey);
-        if (imgIt == self->cssStyleCache_.end())
-          imgIt = self->cssStyleCache_.emplace(imgCacheKey, self->cssParser->resolveStyle("img", classAttr)).first;
-        CssStyle imgDisplayStyle = imgIt->second;
-        if (!styleAttr.empty()) {
-          auto it = self->inlineStyleCache_.find(styleAttr);
-          if (it == self->inlineStyleCache_.end())
-            it = self->inlineStyleCache_.emplace(styleAttr, CssParser::parseInlineStyle(styleAttr)).first;
-          imgDisplayStyle.applyOver(it->second);
-        }
+      {
+        const CssStyle& imgDisplayStyle = cssStyle;
         if (imgDisplayStyle.hasDisplay() && imgDisplayStyle.display == CssDisplay::None) {
           self->skipUntilDepth = self->depth;
           self->depth += 1;
           return;
         }
       }
@@
-                std::string imgCacheKey("img|");
-                imgCacheKey += classAttr;
-                auto imgStyleIt = self->cssParser ? self->cssStyleCache_.find(imgCacheKey) : self->cssStyleCache_.end();
-                if (self->cssParser && imgStyleIt == self->cssStyleCache_.end())
-                  imgStyleIt =
-                      self->cssStyleCache_.emplace(imgCacheKey, self->cssParser->resolveStyle("img", classAttr)).first;
-                CssStyle imgStyle = self->cssParser ? imgStyleIt->second : CssStyle{};
-                // Merge inline style (e.g. style="height: 2em") so it overrides stylesheet rules
-                if (!styleAttr.empty()) {
-                  auto it = self->inlineStyleCache_.find(styleAttr);
-                  if (it == self->inlineStyleCache_.end())
-                    it = self->inlineStyleCache_.emplace(styleAttr, CssParser::parseInlineStyle(styleAttr)).first;
-                  imgStyle.applyOver(it->second);
-                }
+                CssStyle imgStyle = cssStyle;

Also applies to: 352-364

🤖 Prompt for AI Agents

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

In `@lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp` around lines 297 - 307, The
img handling duplicates work by re-resolving and re-caching styles even though
cssStyle already holds the resolved stylesheet+inline merge; update the <img>
path in ChapterHtmlSlimParser (where imgCacheKey, cssStyleCache_,
cssParser->resolveStyle("img", classAttr), inlineStyleCache_,
CssParser::parseInlineStyle and imgDisplayStyle.applyOver are used) to reuse the
existing cssStyle variable instead of creating imgCacheKey and probing
cssStyleCache_/inlineStyleCache_ again—remove the extra cache lookups and
resolve calls, and apply the same single cache/application policy already used
for other elements so style resolution for images is done in one place
consistently (also apply the same change in the similar block referenced at the
later img path).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes