From 55a1fef01a5b8a68b3b4290aa7baa8b3239dc57a Mon Sep 17 00:00:00 2001 From: cottongin Date: Fri, 20 Feb 2026 15:52:30 -0500 Subject: [PATCH] fix: Port upstream 1.1.0-rc PRs #1014, #1018, #990 and align #1002 Port three new upstream commits and align the existing #1002 port: - PR #1014: Strip unused CSS rules by filtering unsupported selector types (+, >, [, :, #, ~, *, descendants) in processRuleBlockWithStyle. Fix normalized() trailing whitespace to also strip newlines. - PR #1018: Add deleteCache() to CssParser, move CSS_CACHE_VERSION to static class member, remove stale cache on version mismatch, invalidate section caches (Storage.removeDir) when CSS is rebuilt. Refactor parseCssFiles() to early-return when cache exists. - PR #990: Adapt classic theme continue-reading card width to cover aspect ratio (clamped to 90% screen width), increase homeTopPadding 20->40, fix centering with rect.x offset for boxX/continueBoxX. - #1002 alignment: Add tryInterpretLength() to skip non-numeric CSS values (auto, inherit), add "both width and height set" image sizing branch in ChapterHtmlSlimParser. Co-authored-by: Cursor --- lib/Epub/Epub.cpp | 120 +++++++++--------- lib/Epub/Epub/css/CssParser.cpp | 69 +++++++--- lib/Epub/Epub/css/CssParser.h | 10 ++ .../Epub/parsers/ChapterHtmlSlimParser.cpp | 29 ++++- src/components/themes/BaseTheme.cpp | 74 +++++++---- src/components/themes/BaseTheme.h | 2 +- 6 files changed, 195 insertions(+), 109 deletions(-) diff --git a/lib/Epub/Epub.cpp b/lib/Epub/Epub.cpp index fb946ab7..bef93d17 100644 --- a/lib/Epub/Epub.cpp +++ b/lib/Epub/Epub.cpp @@ -213,74 +213,69 @@ bool Epub::parseTocNavFile() const { } void Epub::parseCssFiles() const { - // Maximum CSS file size we'll attempt to parse (uncompressed) - // Larger files risk memory exhaustion on ESP32 - constexpr size_t MAX_CSS_FILE_SIZE = 128 * 1024; // 128KB - // Minimum heap required before attempting CSS parsing - constexpr size_t MIN_HEAP_FOR_CSS_PARSING = 64 * 1024; // 64KB + constexpr size_t MAX_CSS_FILE_SIZE = 128 * 1024; + constexpr size_t MIN_HEAP_FOR_CSS_PARSING = 64 * 1024; if (cssFiles.empty()) { LOG_DBG("EBP", "No CSS files to parse, but CssParser created for inline styles"); } - // See if we have a cached version of the CSS rules - if (!cssParser->hasCache()) { - // No cache yet - parse CSS files - for (const auto& cssPath : cssFiles) { - LOG_DBG("EBP", "Parsing CSS file: %s", cssPath.c_str()); + LOG_DBG("EBP", "CSS files to parse: %zu", cssFiles.size()); - // Check heap before parsing - CSS parsing allocates heavily - const uint32_t freeHeap = ESP.getFreeHeap(); - if (freeHeap < MIN_HEAP_FOR_CSS_PARSING) { - LOG_ERR("EBP", "Insufficient heap for CSS parsing (%u bytes free, need %zu), skipping: %s", freeHeap, - MIN_HEAP_FOR_CSS_PARSING, cssPath.c_str()); + if (cssParser->hasCache()) { + LOG_DBG("EBP", "CSS cache exists, skipping parseCssFiles"); + return; + } + + for (const auto& cssPath : cssFiles) { + LOG_DBG("EBP", "Parsing CSS file: %s", cssPath.c_str()); + + const uint32_t freeHeap = ESP.getFreeHeap(); + if (freeHeap < MIN_HEAP_FOR_CSS_PARSING) { + LOG_ERR("EBP", "Insufficient heap for CSS parsing (%u bytes free, need %zu), skipping: %s", freeHeap, + MIN_HEAP_FOR_CSS_PARSING, cssPath.c_str()); + continue; + } + + size_t cssFileSize = 0; + if (getItemSize(cssPath, &cssFileSize)) { + if (cssFileSize > MAX_CSS_FILE_SIZE) { + LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s", cssFileSize, MAX_CSS_FILE_SIZE, + cssPath.c_str()); continue; } + } - // Check CSS file size before decompressing - skip files that are too large - size_t cssFileSize = 0; - if (getItemSize(cssPath, &cssFileSize)) { - if (cssFileSize > MAX_CSS_FILE_SIZE) { - LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s", cssFileSize, MAX_CSS_FILE_SIZE, - cssPath.c_str()); - continue; - } - } - - // Extract CSS file to temp location - const auto tmpCssPath = getCachePath() + "/.tmp.css"; - FsFile tempCssFile; - if (!Storage.openFileForWrite("EBP", tmpCssPath, tempCssFile)) { - LOG_ERR("EBP", "Could not create temp CSS file"); - continue; - } - if (!readItemContentsToStream(cssPath, tempCssFile, 1024)) { - LOG_ERR("EBP", "Could not read CSS file: %s", cssPath.c_str()); - tempCssFile.close(); - Storage.remove(tmpCssPath.c_str()); - continue; - } - tempCssFile.close(); - - // Parse the CSS file - if (!Storage.openFileForRead("EBP", tmpCssPath, tempCssFile)) { - LOG_ERR("EBP", "Could not open temp CSS file for reading"); - Storage.remove(tmpCssPath.c_str()); - continue; - } - cssParser->loadFromStream(tempCssFile); + const auto tmpCssPath = getCachePath() + "/.tmp.css"; + FsFile tempCssFile; + if (!Storage.openFileForWrite("EBP", tmpCssPath, tempCssFile)) { + LOG_ERR("EBP", "Could not create temp CSS file"); + continue; + } + if (!readItemContentsToStream(cssPath, tempCssFile, 1024)) { + LOG_ERR("EBP", "Could not read CSS file: %s", cssPath.c_str()); tempCssFile.close(); Storage.remove(tmpCssPath.c_str()); + continue; } + tempCssFile.close(); - // Save to cache for next time - if (!cssParser->saveToCache()) { - LOG_ERR("EBP", "Failed to save CSS rules to cache"); + if (!Storage.openFileForRead("EBP", tmpCssPath, tempCssFile)) { + LOG_ERR("EBP", "Could not open temp CSS file for reading"); + Storage.remove(tmpCssPath.c_str()); + continue; } - cssParser->clear(); - - LOG_DBG("EBP", "Loaded %zu CSS style rules from %zu files", cssParser->ruleCount(), cssFiles.size()); + cssParser->loadFromStream(tempCssFile); + tempCssFile.close(); + Storage.remove(tmpCssPath.c_str()); } + + if (!cssParser->saveToCache()) { + LOG_ERR("EBP", "Failed to save CSS rules to cache"); + } + cssParser->clear(); + + LOG_DBG("EBP", "Loaded %zu CSS style rules from %zu files", cssParser->ruleCount(), cssFiles.size()); } // load in the meta data for the epub file @@ -294,14 +289,17 @@ bool Epub::load(const bool buildIfMissing, const bool skipLoadingCss) { // Try to load existing cache first if (bookMetadataCache->load()) { - if (!skipLoadingCss && !cssParser->hasCache()) { - LOG_DBG("EBP", "Warning: CSS rules cache not found, attempting to parse CSS files"); - // to get CSS file list - if (!parseContentOpf(bookMetadataCache->coreMetadata)) { - LOG_ERR("EBP", "Could not parse content.opf from cached bookMetadata for CSS files"); - // continue anyway - book will work without CSS and we'll still load any inline style CSS + if (!skipLoadingCss) { + if (!cssParser->hasCache() || !cssParser->loadFromCache()) { + LOG_DBG("EBP", "CSS rules cache missing or stale, attempting to parse CSS files"); + cssParser->deleteCache(); + + if (!parseContentOpf(bookMetadataCache->coreMetadata)) { + LOG_ERR("EBP", "Could not parse content.opf from cached bookMetadata for CSS files"); + } + parseCssFiles(); + Storage.removeDir((cachePath + "/sections").c_str()); } - parseCssFiles(); } LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str()); return true; @@ -400,8 +398,8 @@ bool Epub::load(const bool buildIfMissing, const bool skipLoadingCss) { } if (!skipLoadingCss) { - // Parse CSS files after cache reload parseCssFiles(); + Storage.removeDir((cachePath + "/sections").c_str()); } LOG_DBG("EBP", "Loaded ePub: %s", filepath.c_str()); diff --git a/lib/Epub/Epub/css/CssParser.cpp b/lib/Epub/Epub/css/CssParser.cpp index 72f4de32..11a4f037 100644 --- a/lib/Epub/Epub/css/CssParser.cpp +++ b/lib/Epub/Epub/css/CssParser.cpp @@ -74,7 +74,7 @@ std::string CssParser::normalized(const std::string& s) { } // Remove trailing space - if (!result.empty() && result.back() == ' ') { + while (!result.empty() && (result.back() == ' ' || result.back() == '\n')) { result.pop_back(); } return result; @@ -189,10 +189,18 @@ CssTextDecoration CssParser::interpretDecoration(const std::string& val) { } CssLength CssParser::interpretLength(const std::string& val) { - const std::string v = normalized(val); - if (v.empty()) return CssLength{}; + CssLength result; + tryInterpretLength(val, result); + return result; +} + +bool CssParser::tryInterpretLength(const std::string& val, CssLength& out) { + const std::string v = normalized(val); + if (v.empty()) { + out = CssLength{}; + return false; + } - // Find where the number ends size_t unitStart = v.size(); for (size_t i = 0; i < v.size(); ++i) { const char c = v[i]; @@ -205,12 +213,13 @@ CssLength CssParser::interpretLength(const std::string& val) { const std::string numPart = v.substr(0, unitStart); const std::string unitPart = v.substr(unitStart); - // Parse numeric value char* endPtr = nullptr; const float numericValue = std::strtof(numPart.c_str(), &endPtr); - if (endPtr == numPart.c_str()) return CssLength{}; // No number parsed + if (endPtr == numPart.c_str()) { + out = CssLength{}; + return false; // No number parsed (e.g. auto, inherit, initial) + } - // Determine unit type (preserve for deferred resolution) auto unit = CssUnit::Pixels; if (unitPart == "em") { unit = CssUnit::Em; @@ -221,9 +230,9 @@ CssLength CssParser::interpretLength(const std::string& val) { } else if (unitPart == "%") { unit = CssUnit::Percent; } - // px and unitless default to Pixels - return CssLength{numericValue, unit}; + out = CssLength{numericValue, unit}; + return true; } // Declaration parsing @@ -296,11 +305,17 @@ void CssParser::parseDeclarationIntoStyle(const std::string& decl, CssStyle& sty 1; } } else if (propNameBuf == "height") { - style.imageHeight = interpretLength(propValueBuf); - style.defined.imageHeight = 1; + CssLength len; + if (tryInterpretLength(propValueBuf, len)) { + style.imageHeight = len; + style.defined.imageHeight = 1; + } } else if (propNameBuf == "width") { - style.width = interpretLength(propValueBuf); - style.defined.width = 1; + CssLength len; + if (tryInterpretLength(propValueBuf, len)) { + style.width = len; + style.defined.width = 1; + } } } @@ -349,6 +364,17 @@ void CssParser::processRuleBlockWithStyle(const std::string& selectorGroup, cons std::string key = normalized(sel); if (key.empty()) continue; + // Skip unsupported selector types to reduce memory usage. + // We only match: tag, tag.class, .class + if (key.find('+') != std::string::npos) continue; // adjacent sibling + if (key.find('>') != std::string::npos) continue; // child combinator + if (key.find('[') != std::string::npos) continue; // attribute selector + if (key.find(':') != std::string::npos) continue; // pseudo selector + if (key.find('#') != std::string::npos) continue; // ID selector + if (key.find('~') != std::string::npos) continue; // general sibling + if (key.find('*') != std::string::npos) continue; // wildcard + if (key.find(' ') != std::string::npos) continue; // descendant combinator + // Skip if this would exceed the rule limit if (rulesBySelector_.size() >= MAX_RULES) { LOG_DBG("CSS", "Reached max rules limit, stopping selector processing"); @@ -534,6 +560,7 @@ CssStyle CssParser::resolveStyle(const std::string& tagName, const std::string& result.applyOver(tagIt->second); } + // TODO: Support combinations of classes (e.g. style on .class1.class2) // 2. Apply class styles (medium priority) if (!classAttr.empty()) { const auto classes = splitWhitespace(classAttr); @@ -547,6 +574,7 @@ CssStyle CssParser::resolveStyle(const std::string& tagName, const std::string& } } + // TODO: Support combinations of classes (e.g. style on p.class1.class2) // 3. Apply element.class styles (higher priority) for (const auto& cls : classes) { std::string combinedKey = tag + "." + normalized(cls); @@ -567,12 +595,15 @@ CssStyle CssParser::parseInlineStyle(const std::string& styleValue) { return par // Cache serialization -// Cache format version - increment when format changes -constexpr uint8_t CSS_CACHE_VERSION = 3; +// Cache file name (version is CssParser::CSS_CACHE_VERSION) constexpr char rulesCache[] = "/css_rules.cache"; bool CssParser::hasCache() const { return Storage.exists((cachePath + rulesCache).c_str()); } +void CssParser::deleteCache() const { + if (hasCache()) Storage.remove((cachePath + rulesCache).c_str()); +} + bool CssParser::saveToCache() const { if (cachePath.empty()) { return false; @@ -584,7 +615,7 @@ bool CssParser::saveToCache() const { } // Write version - file.write(CSS_CACHE_VERSION); + file.write(CssParser::CSS_CACHE_VERSION); // Write rule count const auto ruleCount = static_cast(rulesBySelector_.size()); @@ -662,9 +693,11 @@ bool CssParser::loadFromCache() { // Read and verify version uint8_t version = 0; - if (file.read(&version, 1) != 1 || version != CSS_CACHE_VERSION) { - LOG_DBG("CSS", "Cache version mismatch (got %u, expected %u)", version, CSS_CACHE_VERSION); + if (file.read(&version, 1) != 1 || version != CssParser::CSS_CACHE_VERSION) { + LOG_DBG("CSS", "Cache version mismatch (got %u, expected %u), removing stale cache for rebuild", version, + CssParser::CSS_CACHE_VERSION); file.close(); + Storage.remove((cachePath + rulesCache).c_str()); return false; } diff --git a/lib/Epub/Epub/css/CssParser.h b/lib/Epub/Epub/css/CssParser.h index 60f70d23..36fbfb85 100644 --- a/lib/Epub/Epub/css/CssParser.h +++ b/lib/Epub/Epub/css/CssParser.h @@ -82,6 +82,11 @@ class CssParser { */ bool hasCache() const; + /** + * Delete CSS rules cache file if it exists + */ + void deleteCache() const; + /** * Save parsed CSS rules to a cache file. * @return true if cache was written successfully @@ -91,10 +96,14 @@ class CssParser { /** * Load CSS rules from a cache file. * Clears any existing rules before loading. + * Removes stale cache file on version mismatch. * @return true if cache was loaded successfully */ bool loadFromCache(); + // Bump when CSS cache format or rules change; section caches are invalidated when this changes + static constexpr uint8_t CSS_CACHE_VERSION = 3; + private: // Storage: maps normalized selector -> style properties std::unordered_map rulesBySelector_; @@ -113,6 +122,7 @@ class CssParser { static CssFontWeight interpretFontWeight(const std::string& val); static CssTextDecoration interpretDecoration(const std::string& val); static CssLength interpretLength(const std::string& val); + static bool tryInterpretLength(const std::string& val, CssLength& out); // String utilities static std::string normalized(const std::string& s); diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index 13a33338..9cdc2608 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -429,12 +429,39 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* const bool hasCssHeight = imgStyle.hasImageHeight(); const bool hasCssWidth = imgStyle.hasWidth(); - if (hasCssHeight && dims.width > 0 && dims.height > 0) { + if (hasCssHeight && hasCssWidth && dims.width > 0 && dims.height > 0) { + displayHeight = static_cast( + imgStyle.imageHeight.toPixels(emSize, static_cast(self->viewportHeight)) + 0.5f); + displayWidth = static_cast( + imgStyle.width.toPixels(emSize, static_cast(self->viewportWidth)) + 0.5f); + if (displayHeight < 1) displayHeight = 1; + if (displayWidth < 1) displayWidth = 1; + if (displayWidth > self->viewportWidth || displayHeight > self->viewportHeight) { + float scaleX = (displayWidth > self->viewportWidth) + ? static_cast(self->viewportWidth) / displayWidth + : 1.0f; + float scaleY = (displayHeight > self->viewportHeight) + ? static_cast(self->viewportHeight) / displayHeight + : 1.0f; + float scale = (scaleX < scaleY) ? scaleX : scaleY; + displayWidth = static_cast(displayWidth * scale + 0.5f); + displayHeight = static_cast(displayHeight * scale + 0.5f); + if (displayWidth < 1) displayWidth = 1; + if (displayHeight < 1) displayHeight = 1; + } + LOG_DBG("EHP", "Display size from CSS height+width: %dx%d", displayWidth, displayHeight); + } else if (hasCssHeight && !hasCssWidth && dims.width > 0 && dims.height > 0) { displayHeight = static_cast( imgStyle.imageHeight.toPixels(emSize, static_cast(self->viewportHeight)) + 0.5f); if (displayHeight < 1) displayHeight = 1; displayWidth = static_cast(displayHeight * (static_cast(dims.width) / dims.height) + 0.5f); + if (displayHeight > self->viewportHeight) { + displayHeight = self->viewportHeight; + displayWidth = + static_cast(displayHeight * (static_cast(dims.width) / dims.height) + 0.5f); + if (displayWidth < 1) displayWidth = 1; + } if (displayWidth > self->viewportWidth) { displayWidth = self->viewportWidth; displayHeight = diff --git a/src/components/themes/BaseTheme.cpp b/src/components/themes/BaseTheme.cpp index 0e9b4ad7..149bb28a 100644 --- a/src/components/themes/BaseTheme.cpp +++ b/src/components/themes/BaseTheme.cpp @@ -365,14 +365,52 @@ void BaseTheme::drawTabBar(const GfxRenderer& renderer, const Rect rect, const s void BaseTheme::drawRecentBookCover(GfxRenderer& renderer, Rect rect, const std::vector& recentBooks, const int selectorIndex, bool& coverRendered, bool& coverBufferStored, bool& bufferRestored, std::function storeCoverBuffer) const { - // --- Top "book" card for the current title (selectorIndex == 0) --- - const int bookWidth = rect.width / 2; - const int bookHeight = rect.height; - const int bookX = (rect.width - bookWidth) / 2; - const int bookY = rect.y; const bool hasContinueReading = !recentBooks.empty(); const bool bookSelected = hasContinueReading && selectorIndex == 0; + // --- Top "book" card for the current title (selectorIndex == 0) --- + // Adapt width to cover image aspect ratio; fall back to half screen when no cover + const int baseHeight = rect.height; + + int bookWidth; + bool hasCoverImage = false; + + if (hasContinueReading && !recentBooks[0].coverBmpPath.empty()) { + const std::string coverBmpPath = + UITheme::getCoverThumbPath(recentBooks[0].coverBmpPath, BaseMetrics::values.homeCoverHeight); + + FsFile file; + if (Storage.openFileForRead("HOME", coverBmpPath, file)) { + Bitmap bitmap(file); + if (bitmap.parseHeaders() == BmpReaderError::Ok) { + hasCoverImage = true; + const int imgWidth = bitmap.getWidth(); + const int imgHeight = bitmap.getHeight(); + + if (imgWidth > 0 && imgHeight > 0) { + const float aspectRatio = static_cast(imgWidth) / static_cast(imgHeight); + bookWidth = static_cast(baseHeight * aspectRatio); + + const int maxWidth = static_cast(rect.width * 0.9f); + if (bookWidth > maxWidth) { + bookWidth = maxWidth; + } + } else { + bookWidth = rect.width / 2; + } + } + file.close(); + } + } + + if (!hasCoverImage) { + bookWidth = rect.width / 2; + } + + const int bookX = rect.x + (rect.width - bookWidth) / 2; + const int bookY = rect.y; + const int bookHeight = baseHeight; + // Bookmark dimensions (used in multiple places) const int bookmarkWidth = bookWidth / 8; const int bookmarkHeight = bookHeight / 5; @@ -394,29 +432,9 @@ void BaseTheme::drawRecentBookCover(GfxRenderer& renderer, Rect rect, const std: Bitmap bitmap(file); if (bitmap.parseHeaders() == BmpReaderError::Ok) { LOG_DBG("THEME", "Rendering bmp"); - // Calculate position to center image within the book card - int coverX, coverY; - if (bitmap.getWidth() > bookWidth || bitmap.getHeight() > bookHeight) { - const float imgRatio = static_cast(bitmap.getWidth()) / static_cast(bitmap.getHeight()); - const float boxRatio = static_cast(bookWidth) / static_cast(bookHeight); + renderer.drawBitmap(bitmap, bookX, bookY, bookWidth, bookHeight); - if (imgRatio > boxRatio) { - coverX = bookX; - coverY = bookY + (bookHeight - static_cast(bookWidth / imgRatio)) / 2; - } else { - coverX = bookX + (bookWidth - static_cast(bookHeight * imgRatio)) / 2; - coverY = bookY; - } - } else { - coverX = bookX + (bookWidth - bitmap.getWidth()) / 2; - coverY = bookY + (bookHeight - bitmap.getHeight()) / 2; - } - - // Draw the cover image centered within the book card - renderer.drawBitmap(bitmap, coverX, coverY, bookWidth, bookHeight); - - // Draw border around the card renderer.drawRect(bookX, bookY, bookWidth, bookHeight); // No bookmark ribbon when cover is shown - it would just cover the art @@ -597,7 +615,7 @@ void BaseTheme::drawRecentBookCover(GfxRenderer& renderer, Rect rect, const std: const int boxWidth = maxTextWidth + boxPadding * 2; const int boxHeight = totalTextHeight + boxPadding * 2; - const int boxX = (rect.width - boxWidth) / 2; + const int boxX = rect.x + (rect.width - boxWidth) / 2; const int boxY = titleYStart - boxPadding; // Draw box (inverted when selected: black box instead of white) @@ -640,7 +658,7 @@ void BaseTheme::drawRecentBookCover(GfxRenderer& renderer, Rect rect, const std: constexpr int continuePadding = 6; const int continueBoxWidth = continueTextWidth + continuePadding * 2; const int continueBoxHeight = renderer.getLineHeight(UI_10_FONT_ID) + continuePadding; - const int continueBoxX = (rect.width - continueBoxWidth) / 2; + const int continueBoxX = rect.x + (rect.width - continueBoxWidth) / 2; const int continueBoxY = continueY - continuePadding / 2; renderer.fillRect(continueBoxX, continueBoxY, continueBoxWidth, continueBoxHeight, bookSelected); renderer.drawRect(continueBoxX, continueBoxY, continueBoxWidth, continueBoxHeight, !bookSelected); diff --git a/src/components/themes/BaseTheme.h b/src/components/themes/BaseTheme.h index 91193d39..92397f5b 100644 --- a/src/components/themes/BaseTheme.h +++ b/src/components/themes/BaseTheme.h @@ -82,7 +82,7 @@ constexpr ThemeMetrics values = {.batteryWidth = 15, .tabBarHeight = 50, .scrollBarWidth = 4, .scrollBarRightOffset = 5, - .homeTopPadding = 20, + .homeTopPadding = 40, .homeCoverHeight = 400, .homeCoverTileHeight = 400, .homeRecentBooksCount = 1,