From a1ac11ab51117d7c5c295e28aa6cddec53864d45 Mon Sep 17 00:00:00 2001 From: cottongin Date: Wed, 18 Feb 2026 15:45:06 -0500 Subject: [PATCH] feat: port upstream PRs #852, #965, #972, #971, #977, #975 Port 6 upstream PRs (PR #939 was already ported): - #852: Complete HalPowerManager with RAII Lock class, WiFi check in setPowerSaving, skipLoopDelay overrides for ClearCache/OtaUpdate, and power lock in Activity render task loops - #965: Fix paragraph formatting inside list items by tracking listItemUntilDepth to prevent unwanted line breaks - #972: Micro-optimizations: std::move in insertFont, const ref for getDataFromBook parameter - #971: Remove redundant hasPrintableChars pre-rendering pass from EpdFont, EpdFontFamily, and GfxRenderer - #977: Skip unsupported image formats before extraction, add PARSE_BUFFER_SIZE constant and chapter parse timing - #975: Fix UITheme memory leak by replacing raw pointer with std::unique_ptr for currentTheme Co-authored-by: Cursor --- lib/EpdFont/EpdFont.cpp | 8 ---- lib/EpdFont/EpdFont.h | 1 - lib/EpdFont/EpdFontFamily.cpp | 4 -- lib/EpdFont/EpdFontFamily.h | 1 - .../Epub/parsers/ChapterHtmlSlimParser.cpp | 36 +++++++++++++----- lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h | 1 + lib/GfxRenderer/GfxRenderer.cpp | 17 +-------- lib/hal/HalPowerManager.cpp | 37 ++++++++++++++++++- lib/hal/HalPowerManager.h | 21 +++++++++++ src/RecentBooksStore.cpp | 2 +- src/RecentBooksStore.h | 2 +- src/activities/Activity.cpp | 3 ++ src/activities/ActivityWithSubactivity.cpp | 3 ++ src/activities/settings/ClearCacheActivity.h | 1 + src/activities/settings/OtaUpdateActivity.h | 1 + src/components/UITheme.cpp | 4 +- src/components/UITheme.h | 3 +- 17 files changed, 100 insertions(+), 45 deletions(-) diff --git a/lib/EpdFont/EpdFont.cpp b/lib/EpdFont/EpdFont.cpp index 8550cba4..5b770462 100644 --- a/lib/EpdFont/EpdFont.cpp +++ b/lib/EpdFont/EpdFont.cpp @@ -47,14 +47,6 @@ void EpdFont::getTextDimensions(const char* string, int* w, int* h) const { *h = maxY - minY; } -bool EpdFont::hasPrintableChars(const char* string) const { - int w = 0, h = 0; - - getTextDimensions(string, &w, &h); - - return w > 0 || h > 0; -} - const EpdGlyph* EpdFont::getGlyph(const uint32_t cp) const { const EpdUnicodeInterval* intervals = data->intervals; const int count = data->intervalCount; diff --git a/lib/EpdFont/EpdFont.h b/lib/EpdFont/EpdFont.h index c8473fc0..5b0e2f9b 100644 --- a/lib/EpdFont/EpdFont.h +++ b/lib/EpdFont/EpdFont.h @@ -9,7 +9,6 @@ class EpdFont { explicit EpdFont(const EpdFontData* data) : data(data) {} ~EpdFont() = default; void getTextDimensions(const char* string, int* w, int* h) const; - bool hasPrintableChars(const char* string) const; const EpdGlyph* getGlyph(uint32_t cp) const; }; diff --git a/lib/EpdFont/EpdFontFamily.cpp b/lib/EpdFont/EpdFontFamily.cpp index 821153e3..5a1f8cef 100644 --- a/lib/EpdFont/EpdFontFamily.cpp +++ b/lib/EpdFont/EpdFontFamily.cpp @@ -22,10 +22,6 @@ void EpdFontFamily::getTextDimensions(const char* string, int* w, int* h, const getFont(style)->getTextDimensions(string, w, h); } -bool EpdFontFamily::hasPrintableChars(const char* string, const Style style) const { - return getFont(style)->hasPrintableChars(string); -} - const EpdFontData* EpdFontFamily::getData(const Style style) const { return getFont(style)->data; } const EpdGlyph* EpdFontFamily::getGlyph(const uint32_t cp, const Style style) const { diff --git a/lib/EpdFont/EpdFontFamily.h b/lib/EpdFont/EpdFontFamily.h index 64fd9953..746cc507 100644 --- a/lib/EpdFont/EpdFontFamily.h +++ b/lib/EpdFont/EpdFontFamily.h @@ -10,7 +10,6 @@ class EpdFontFamily { : regular(regular), bold(bold), italic(italic), boldItalic(boldItalic) {} ~EpdFontFamily() = default; void getTextDimensions(const char* string, int* w, int* h, Style style = REGULAR) const; - bool hasPrintableChars(const char* string, Style style = REGULAR) const; const EpdFontData* getData(Style style = REGULAR) const; const EpdGlyph* getGlyph(uint32_t cp, Style style = REGULAR) const; diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp index 560c730c..d7adb4fa 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp @@ -19,6 +19,7 @@ constexpr int NUM_HEADER_TAGS = sizeof(HEADER_TAGS) / sizeof(HEADER_TAGS[0]); // Minimum file size (in bytes) to show indexing popup - smaller chapters don't benefit from it constexpr size_t MIN_SIZE_FOR_POPUP = 10 * 1024; // 10KB +constexpr size_t PARSE_BUFFER_SIZE = 1024; const char* BLOCK_TAGS[] = {"p", "li", "div", "br", "blockquote"}; constexpr int NUM_BLOCK_TAGS = sizeof(BLOCK_TAGS) / sizeof(BLOCK_TAGS[0]); @@ -389,6 +390,9 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* // Resolve the image path relative to the HTML file std::string resolvedPath = FsHelpers::normalisePath(self->contentBase + src); + // Check format support before any file I/O + ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(resolvedPath); + if (decoder) { // Create a unique filename for the cached image std::string ext; size_t extPos = resolvedPath.rfind('.'); @@ -410,8 +414,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* if (extractSuccess) { // Get image dimensions ImageDimensions dims = {0, 0}; - ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(cachedImagePath); - if (decoder && decoder->getDimensions(cachedImagePath, dims)) { + if (decoder->getDimensions(cachedImagePath, dims)) { LOG_DBG("EHP", "Image dimensions: %dx%d", dims.width, dims.height); // Scale to fit viewport while maintaining aspect ratio @@ -470,6 +473,7 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* } else { LOG_ERR("EHP", "Failed to extract image"); } + } // if (decoder) } } @@ -540,18 +544,24 @@ void XMLCALL ChapterHtmlSlimParser::startElement(void* userData, const XML_Char* } else if (matches(name, BLOCK_TAGS, NUM_BLOCK_TAGS)) { if (strcmp(name, "br") == 0) { if (self->partWordBufferIndex > 0) { - // flush word preceding
to currentTextBlock before calling startNewTextBlock self->flushPartWordBuffer(); } self->startNewTextBlock(self->currentTextBlock->getBlockStyle()); + } else if (strcmp(name, "li") == 0) { + self->currentCssStyle = cssStyle; + self->startNewTextBlock(userAlignmentBlockStyle); + self->updateEffectiveInlineStyle(); + self->currentTextBlock->addWord("\xe2\x80\xa2", EpdFontFamily::REGULAR); + self->listItemUntilDepth = std::min(self->listItemUntilDepth, self->depth); + } else if (strcmp(name, "p") == 0 && self->listItemUntilDepth < self->depth) { + // Inside a
  • element - don't start a new text block for

    + // This prevents bullet points from appearing on their own line + self->currentCssStyle = cssStyle; + self->updateEffectiveInlineStyle(); } else { self->currentCssStyle = cssStyle; self->startNewTextBlock(userAlignmentBlockStyle); self->updateEffectiveInlineStyle(); - - if (strcmp(name, "li") == 0) { - self->currentTextBlock->addWord("\xe2\x80\xa2", EpdFontFamily::REGULAR); - } } } else if (matches(name, UNDERLINE_TAGS, NUM_UNDERLINE_TAGS)) { // Flush buffer before style change so preceding text gets current style @@ -807,6 +817,7 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n if (self->boldUntilDepth == self->depth) self->boldUntilDepth = INT_MAX; if (self->italicUntilDepth == self->depth) self->italicUntilDepth = INT_MAX; if (self->underlineUntilDepth == self->depth) self->underlineUntilDepth = INT_MAX; + if (self->listItemUntilDepth == self->depth) self->listItemUntilDepth = INT_MAX; if (!self->inlineStyleStack.empty() && self->inlineStyleStack.back().depth == self->depth) { self->inlineStyleStack.pop_back(); self->updateEffectiveInlineStyle(); @@ -852,6 +863,11 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n self->underlineUntilDepth = INT_MAX; } + // Leaving list item + if (self->listItemUntilDepth == self->depth) { + self->listItemUntilDepth = INT_MAX; + } + // Pop from inline style stack if we pushed an entry at this depth // This handles all inline elements: b, i, u, span, etc. if (!self->inlineStyleStack.empty() && self->inlineStyleStack.back().depth == self->depth) { @@ -867,6 +883,7 @@ void XMLCALL ChapterHtmlSlimParser::endElement(void* userData, const XML_Char* n } bool ChapterHtmlSlimParser::parseAndBuildPages() { + unsigned long chapterStartTime = millis(); auto paragraphAlignmentBlockStyle = BlockStyle(); paragraphAlignmentBlockStyle.textAlignDefined = true; // Resolve None sentinel to Justify for initial block (no CSS context yet) @@ -904,7 +921,7 @@ bool ChapterHtmlSlimParser::parseAndBuildPages() { XML_SetCharacterDataHandler(parser, characterData); do { - void* const buf = XML_GetBuffer(parser, 1024); + void* const buf = XML_GetBuffer(parser, PARSE_BUFFER_SIZE); if (!buf) { LOG_ERR("EHP", "Couldn't allocate memory for buffer"); XML_StopParser(parser, XML_FALSE); // Stop any pending processing @@ -915,7 +932,7 @@ bool ChapterHtmlSlimParser::parseAndBuildPages() { return false; } - const size_t len = file.read(buf, 1024); + const size_t len = file.read(buf, PARSE_BUFFER_SIZE); if (len == 0 && file.available() > 0) { LOG_ERR("EHP", "File read error"); @@ -955,6 +972,7 @@ bool ChapterHtmlSlimParser::parseAndBuildPages() { currentTextBlock.reset(); } + LOG_DBG("EHP", "Chapter parsed in %lu ms", millis() - chapterStartTime); return true; } diff --git a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h index 51b65592..4e146a64 100644 --- a/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h +++ b/lib/Epub/Epub/parsers/ChapterHtmlSlimParser.h @@ -31,6 +31,7 @@ class ChapterHtmlSlimParser { int boldUntilDepth = INT_MAX; int italicUntilDepth = INT_MAX; int underlineUntilDepth = INT_MAX; + int listItemUntilDepth = INT_MAX; // buffer for building up words from characters, will auto break if longer than this // leave one char at end for null pointer char partWordBuffer[MAX_WORD_SIZE + 1] = {}; diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index ade9883e..2659ec19 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -11,7 +11,7 @@ void GfxRenderer::begin() { } } -void GfxRenderer::insertFont(const int fontId, EpdFontFamily font) { fontMap.insert({fontId, font}); } +void GfxRenderer::insertFont(const int fontId, EpdFontFamily font) { fontMap.insert({fontId, std::move(font)}); } // Translate logical (x,y) coordinates to physical panel coordinates based on current orientation // This should always be inlined for better performance @@ -116,11 +116,6 @@ void GfxRenderer::drawText(const int fontId, const int x, const int y, const cha } const auto font = fontMap.at(fontId); - // no printable characters - if (!font.hasPrintableChars(text, style)) { - return; - } - uint32_t cp; while ((cp = utf8NextCodepoint(reinterpret_cast(&text)))) { renderChar(font, cp, &xpos, &yPos, black, style); @@ -853,11 +848,6 @@ void GfxRenderer::drawTextRotated90CW(const int fontId, const int x, const int y } const auto font = fontMap.at(fontId); - // No printable characters - if (!font.hasPrintableChars(text, style)) { - return; - } - // For 90° clockwise rotation: // Original (glyphX, glyphY) -> Rotated (glyphY, -glyphX) // Text reads from bottom to top @@ -936,11 +926,6 @@ void GfxRenderer::drawTextRotated90CCW(const int fontId, const int x, const int } const auto font = fontMap.at(fontId); - // No printable characters - if (!font.hasPrintableChars(text, style)) { - return; - } - // For 90° counter-clockwise rotation: // Mirror of CW: glyphY maps to -X direction, glyphX maps to +Y direction // Text reads from top to bottom diff --git a/lib/hal/HalPowerManager.cpp b/lib/hal/HalPowerManager.cpp index 31e9a7e8..6cf5a311 100644 --- a/lib/hal/HalPowerManager.cpp +++ b/lib/hal/HalPowerManager.cpp @@ -1,6 +1,7 @@ #include "HalPowerManager.h" #include +#include #include #include "HalGPIO.h" @@ -8,12 +9,27 @@ void HalPowerManager::begin() { pinMode(BAT_GPIO0, INPUT); normalFreq = getCpuFrequencyMhz(); + modeMutex = xSemaphoreCreateMutex(); + assert(modeMutex != nullptr); } void HalPowerManager::setPowerSaving(bool enabled) { if (normalFreq <= 0) { - return; // invalid state + return; } + + if (enabled) { + if (WiFi.getMode() != WIFI_MODE_NULL) { + enabled = false; + } + xSemaphoreTake(modeMutex, portMAX_DELAY); + const LockMode mode = currentLockMode; + xSemaphoreGive(modeMutex); + if (mode == NormalSpeed) { + enabled = false; + } + } + if (enabled && !isLowPower) { LOG_DBG("PWR", "Going to low-power mode"); if (!setCpuFrequencyMhz(LOW_POWER_FREQ)) { @@ -31,6 +47,25 @@ void HalPowerManager::setPowerSaving(bool enabled) { isLowPower = enabled; } +// RAII Lock implementation + +HalPowerManager::Lock::Lock() { + xSemaphoreTake(powerManager.modeMutex, portMAX_DELAY); + powerManager.currentLockMode = NormalSpeed; + valid = true; + if (powerManager.isLowPower) { + powerManager.setPowerSaving(false); + } + xSemaphoreGive(powerManager.modeMutex); +} + +HalPowerManager::Lock::~Lock() { + if (!valid) return; + xSemaphoreTake(powerManager.modeMutex, portMAX_DELAY); + powerManager.currentLockMode = None; + xSemaphoreGive(powerManager.modeMutex); +} + void HalPowerManager::startDeepSleep(HalGPIO& gpio) const { // Ensure that the power button has been released to avoid immediately turning back on if you're holding it while (gpio.isPressed(HalGPIO::BTN_POWER)) { diff --git a/lib/hal/HalPowerManager.h b/lib/hal/HalPowerManager.h index 636b2c97..ceff9287 100644 --- a/lib/hal/HalPowerManager.h +++ b/lib/hal/HalPowerManager.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "HalGPIO.h" @@ -10,6 +11,10 @@ class HalPowerManager { int normalFreq = 0; // MHz bool isLowPower = false; + enum LockMode { None, NormalSpeed }; + LockMode currentLockMode = None; + SemaphoreHandle_t modeMutex = nullptr; + public: static constexpr int LOW_POWER_FREQ = 10; // MHz static constexpr unsigned long IDLE_POWER_SAVING_MS = 3000; // ms @@ -24,4 +29,20 @@ class HalPowerManager { // Get battery percentage (range 0-100) int getBatteryPercentage() const; + + // RAII lock to prevent low-power mode during critical work (e.g. rendering) + class Lock { + friend class HalPowerManager; + bool valid = false; + + public: + Lock(); + ~Lock(); + Lock(const Lock&) = delete; + Lock& operator=(const Lock&) = delete; + Lock(Lock&&) = delete; + Lock& operator=(Lock&&) = delete; + }; }; + +extern HalPowerManager powerManager; diff --git a/src/RecentBooksStore.cpp b/src/RecentBooksStore.cpp index 5970ef77..9d7adf3e 100644 --- a/src/RecentBooksStore.cpp +++ b/src/RecentBooksStore.cpp @@ -85,7 +85,7 @@ bool RecentBooksStore::saveToFile() const { return true; } -RecentBook RecentBooksStore::getDataFromBook(std::string path) const { +RecentBook RecentBooksStore::getDataFromBook(const std::string& path) const { std::string lastBookFileName = ""; const size_t lastSlash = path.find_last_of('/'); if (lastSlash != std::string::npos) { diff --git a/src/RecentBooksStore.h b/src/RecentBooksStore.h index ec81c011..bea11d44 100644 --- a/src/RecentBooksStore.h +++ b/src/RecentBooksStore.h @@ -42,7 +42,7 @@ class RecentBooksStore { bool saveToFile() const; bool loadFromFile(); - RecentBook getDataFromBook(std::string path) const; + RecentBook getDataFromBook(const std::string& path) const; }; // Helper macro to access recent books store diff --git a/src/activities/Activity.cpp b/src/activities/Activity.cpp index 6cd8493e..9a9c5de3 100644 --- a/src/activities/Activity.cpp +++ b/src/activities/Activity.cpp @@ -1,5 +1,7 @@ #include "Activity.h" +#include + void Activity::renderTaskTrampoline(void* param) { auto* self = static_cast(param); self->renderTaskLoop(); @@ -9,6 +11,7 @@ void Activity::renderTaskLoop() { while (true) { ulTaskNotifyTake(pdTRUE, portMAX_DELAY); { + HalPowerManager::Lock powerLock; RenderLock lock(*this); render(std::move(lock)); } diff --git a/src/activities/ActivityWithSubactivity.cpp b/src/activities/ActivityWithSubactivity.cpp index 40da93f1..dd127067 100644 --- a/src/activities/ActivityWithSubactivity.cpp +++ b/src/activities/ActivityWithSubactivity.cpp @@ -1,9 +1,12 @@ #include "ActivityWithSubactivity.h" +#include + void ActivityWithSubactivity::renderTaskLoop() { while (true) { ulTaskNotifyTake(pdTRUE, portMAX_DELAY); { + HalPowerManager::Lock powerLock; RenderLock lock(*this); if (!subActivity) { render(std::move(lock)); diff --git a/src/activities/settings/ClearCacheActivity.h b/src/activities/settings/ClearCacheActivity.h index 8bfbe1f3..48ae32ff 100644 --- a/src/activities/settings/ClearCacheActivity.h +++ b/src/activities/settings/ClearCacheActivity.h @@ -14,6 +14,7 @@ class ClearCacheActivity final : public ActivityWithSubactivity { void onExit() override; void loop() override; void render(Activity::RenderLock&&) override; + bool skipLoopDelay() override { return true; } private: enum State { WARNING, CLEARING, SUCCESS, FAILED }; diff --git a/src/activities/settings/OtaUpdateActivity.h b/src/activities/settings/OtaUpdateActivity.h index 7978acf4..aa9b1d16 100644 --- a/src/activities/settings/OtaUpdateActivity.h +++ b/src/activities/settings/OtaUpdateActivity.h @@ -33,5 +33,6 @@ class OtaUpdateActivity : public ActivityWithSubactivity { void onExit() override; void loop() override; void render(Activity::RenderLock&&) override; + bool skipLoopDelay() override { return true; } bool preventAutoSleep() override { return state == CHECKING_FOR_UPDATE || state == UPDATE_IN_PROGRESS; } }; diff --git a/src/components/UITheme.cpp b/src/components/UITheme.cpp index a1ddd1f3..1ee50b98 100644 --- a/src/components/UITheme.cpp +++ b/src/components/UITheme.cpp @@ -25,12 +25,12 @@ void UITheme::setTheme(CrossPointSettings::UI_THEME type) { switch (type) { case CrossPointSettings::UI_THEME::CLASSIC: LOG_DBG("UI", "Using Classic theme"); - currentTheme = new BaseTheme(); + currentTheme = std::make_unique(); currentMetrics = &BaseMetrics::values; break; case CrossPointSettings::UI_THEME::LYRA: LOG_DBG("UI", "Using Lyra theme"); - currentTheme = new LyraTheme(); + currentTheme = std::make_unique(); currentMetrics = &LyraMetrics::values; break; } diff --git a/src/components/UITheme.h b/src/components/UITheme.h index bf8595c0..b4ca3bf3 100644 --- a/src/components/UITheme.h +++ b/src/components/UITheme.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include "CrossPointSettings.h" @@ -24,7 +25,7 @@ class UITheme { private: const ThemeMetrics* currentMetrics; - const BaseTheme* currentTheme; + std::unique_ptr currentTheme; }; // Known theme thumbnail heights to prerender when opening a book for the first time.