From c986a274ee4a2eca5de02f3c488cc14cd4a2047b Mon Sep 17 00:00:00 2001 From: Uri Tauber <142022451+Uri-Tauber@users.noreply.github.com> Date: Sun, 8 Mar 2026 18:45:54 +0200 Subject: [PATCH 1/2] refactor: reader utils (#1329) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Extract shared reader utilities (`ReaderUtils.h`) to reduce duplication across `EpubReaderActivity`, `TxtReaderActivity`, and (upcoming) `MarkdownReaderActivity`. Utilities extracted: - `applyOrientation()` — orientation switch logic - `detectPageTurn()` — page navigation input detection - `renderAntiAliased()` — grayscale anti-aliasing pass - `displayWithRefreshCycle()` — refresh mode cadence - `GO_HOME_MS` — back button timing constant ## Impact Flash: 32 bytes saved (6006441 → 6006409 bytes). Minimal immediate gain, but meaningful once markdown reader and future reader types share these functions. Code quality: Eliminates ~100 lines of duplicated logic spread across multiple files. All readers now follow the same patterns for orientation, input handling, and rendering. ## Rationale This refactor is preparation for markdown support, which requires identical input and rendering logic. Instead of copy-pasting these patterns a third time, all readers now share a single, tested implementation. Future reader types can reuse `ReaderUtils` without duplication. --- ## AI Usage Did you use AI tools to help write this code? YES Claude extracted the code, under my guidance. Tested on my device and seems to work fine. --- src/activities/reader/EpubReaderActivity.cpp | 53 ++---------- src/activities/reader/ReaderUtils.h | 89 ++++++++++++++++++++ src/activities/reader/TxtReaderActivity.cpp | 69 ++------------- 3 files changed, 105 insertions(+), 106 deletions(-) create mode 100644 src/activities/reader/ReaderUtils.h diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index d2af6751..291d4312 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -17,6 +17,7 @@ #include "KOReaderSyncActivity.h" #include "MappedInputManager.h" #include "QrDisplayActivity.h" +#include "ReaderUtils.h" #include "RecentBooksStore.h" #include "components/UITheme.h" #include "fontIds.h" @@ -25,7 +26,6 @@ namespace { // pagesPerRefresh now comes from SETTINGS.getRefreshFrequency() constexpr unsigned long skipChapterMs = 700; -constexpr unsigned long goHomeMs = 1000; // pages per minute, first item is 1 to prevent division by zero if accessed const std::vector PAGE_TURN_LABELS = {1, 1, 3, 6, 12}; @@ -39,27 +39,6 @@ int clampPercent(int percent) { return percent; } -// Apply the logical reader orientation to the renderer. -// This centralizes orientation mapping so we don't duplicate switch logic elsewhere. -void applyReaderOrientation(GfxRenderer& renderer, const uint8_t orientation) { - switch (orientation) { - case CrossPointSettings::ORIENTATION::PORTRAIT: - renderer.setOrientation(GfxRenderer::Orientation::Portrait); - break; - case CrossPointSettings::ORIENTATION::LANDSCAPE_CW: - renderer.setOrientation(GfxRenderer::Orientation::LandscapeClockwise); - break; - case CrossPointSettings::ORIENTATION::INVERTED: - renderer.setOrientation(GfxRenderer::Orientation::PortraitInverted); - break; - case CrossPointSettings::ORIENTATION::LANDSCAPE_CCW: - renderer.setOrientation(GfxRenderer::Orientation::LandscapeCounterClockwise); - break; - default: - break; - } -} - } // namespace void EpubReaderActivity::onEnter() { @@ -71,7 +50,7 @@ void EpubReaderActivity::onEnter() { // Configure screen orientation based on settings // NOTE: This affects layout math and must be applied before any render calls. - applyReaderOrientation(renderer, SETTINGS.orientation); + ReaderUtils::applyOrientation(renderer, SETTINGS.orientation); epub->setupCacheDir(); @@ -179,13 +158,14 @@ void EpubReaderActivity::loop() { } // Long press BACK (1s+) goes to file selection - if (mappedInput.isPressed(MappedInputManager::Button::Back) && mappedInput.getHeldTime() >= goHomeMs) { + if (mappedInput.isPressed(MappedInputManager::Button::Back) && mappedInput.getHeldTime() >= ReaderUtils::GO_HOME_MS) { activityManager.goToFileBrowser(epub ? epub->getPath() : ""); return; } // Short press BACK goes directly to home (or restores position if viewing footnote) - if (mappedInput.wasReleased(MappedInputManager::Button::Back) && mappedInput.getHeldTime() < goHomeMs) { + if (mappedInput.wasReleased(MappedInputManager::Button::Back) && + mappedInput.getHeldTime() < ReaderUtils::GO_HOME_MS) { if (footnoteDepth > 0) { restoreSavedPosition(); return; @@ -194,20 +174,7 @@ void EpubReaderActivity::loop() { return; } - // When long-press chapter skip is disabled, turn pages on press instead of release. - const bool usePressForPageTurn = !SETTINGS.longPressChapterSkip; - const bool prevTriggered = usePressForPageTurn ? (mappedInput.wasPressed(MappedInputManager::Button::PageBack) || - mappedInput.wasPressed(MappedInputManager::Button::Left)) - : (mappedInput.wasReleased(MappedInputManager::Button::PageBack) || - mappedInput.wasReleased(MappedInputManager::Button::Left)); - const bool powerPageTurn = SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::PAGE_TURN && - mappedInput.wasReleased(MappedInputManager::Button::Power); - const bool nextTriggered = usePressForPageTurn - ? (mappedInput.wasPressed(MappedInputManager::Button::PageForward) || powerPageTurn || - mappedInput.wasPressed(MappedInputManager::Button::Right)) - : (mappedInput.wasReleased(MappedInputManager::Button::PageForward) || powerPageTurn || - mappedInput.wasReleased(MappedInputManager::Button::Right)); - + auto [prevTriggered, nextTriggered] = ReaderUtils::detectPageTurn(mappedInput); if (!prevTriggered && !nextTriggered) { return; } @@ -455,7 +422,7 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) { SETTINGS.saveToFile(); // Update renderer orientation to match the new logical coordinate system. - applyReaderOrientation(renderer, SETTINGS.orientation); + ReaderUtils::applyOrientation(renderer, SETTINGS.orientation); // Reset section to force re-layout in the new orientation. section.reset(); @@ -719,12 +686,8 @@ void EpubReaderActivity::renderContents(std::unique_ptr page, const int or renderer.displayBuffer(HalDisplay::HALF_REFRESH); } // Double FAST_REFRESH handles ghosting for image pages; don't count toward full refresh cadence - } else if (pagesUntilFullRefresh <= 1) { - renderer.displayBuffer(HalDisplay::HALF_REFRESH); - pagesUntilFullRefresh = SETTINGS.getRefreshFrequency(); } else { - renderer.displayBuffer(); - pagesUntilFullRefresh--; + ReaderUtils::displayWithRefreshCycle(renderer, pagesUntilFullRefresh); } // Save bw buffer to reset buffer state after grayscale data sync diff --git a/src/activities/reader/ReaderUtils.h b/src/activities/reader/ReaderUtils.h new file mode 100644 index 00000000..8694be08 --- /dev/null +++ b/src/activities/reader/ReaderUtils.h @@ -0,0 +1,89 @@ +#pragma once + +#include +#include +#include + +#include "MappedInputManager.h" + +namespace ReaderUtils { + +constexpr unsigned long GO_HOME_MS = 1000; + +inline void applyOrientation(GfxRenderer& renderer, const uint8_t orientation) { + switch (orientation) { + case CrossPointSettings::ORIENTATION::PORTRAIT: + renderer.setOrientation(GfxRenderer::Orientation::Portrait); + break; + case CrossPointSettings::ORIENTATION::LANDSCAPE_CW: + renderer.setOrientation(GfxRenderer::Orientation::LandscapeClockwise); + break; + case CrossPointSettings::ORIENTATION::INVERTED: + renderer.setOrientation(GfxRenderer::Orientation::PortraitInverted); + break; + case CrossPointSettings::ORIENTATION::LANDSCAPE_CCW: + renderer.setOrientation(GfxRenderer::Orientation::LandscapeCounterClockwise); + break; + default: + break; + } +} + +struct PageTurnResult { + bool prev; + bool next; +}; + +inline PageTurnResult detectPageTurn(const MappedInputManager& input) { + const bool usePress = !SETTINGS.longPressChapterSkip; + const bool prev = usePress ? (input.wasPressed(MappedInputManager::Button::PageBack) || + input.wasPressed(MappedInputManager::Button::Left)) + : (input.wasReleased(MappedInputManager::Button::PageBack) || + input.wasReleased(MappedInputManager::Button::Left)); + const bool powerTurn = SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::PAGE_TURN && + input.wasReleased(MappedInputManager::Button::Power); + const bool next = usePress ? (input.wasPressed(MappedInputManager::Button::PageForward) || powerTurn || + input.wasPressed(MappedInputManager::Button::Right)) + : (input.wasReleased(MappedInputManager::Button::PageForward) || powerTurn || + input.wasReleased(MappedInputManager::Button::Right)); + return {prev, next}; +} + +inline void displayWithRefreshCycle(const GfxRenderer& renderer, int& pagesUntilFullRefresh) { + if (pagesUntilFullRefresh <= 1) { + renderer.displayBuffer(HalDisplay::HALF_REFRESH); + pagesUntilFullRefresh = SETTINGS.getRefreshFrequency(); + } else { + renderer.displayBuffer(); + pagesUntilFullRefresh--; + } +} + +// Grayscale anti-aliasing pass. Renders content twice (LSB + MSB) to build +// the grayscale buffer. Only the content callback is re-rendered — status bars +// and other overlays should be drawn before calling this. +// Kept as a template to avoid std::function overhead; instantiated once per reader type. +template +void renderAntiAliased(GfxRenderer& renderer, RenderFn&& renderFn) { + if (!renderer.storeBwBuffer()) { + LOG_ERR("READER", "Failed to store BW buffer for anti-aliasing"); + return; + } + + renderer.clearScreen(0x00); + renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); + renderFn(); + renderer.copyGrayscaleLsbBuffers(); + + renderer.clearScreen(0x00); + renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); + renderFn(); + renderer.copyGrayscaleMsbBuffers(); + + renderer.displayGrayBuffer(); + renderer.setRenderMode(GfxRenderer::BW); + + renderer.restoreBwBuffer(); +} + +} // namespace ReaderUtils diff --git a/src/activities/reader/TxtReaderActivity.cpp b/src/activities/reader/TxtReaderActivity.cpp index 756b7749..381ea6a8 100644 --- a/src/activities/reader/TxtReaderActivity.cpp +++ b/src/activities/reader/TxtReaderActivity.cpp @@ -9,14 +9,13 @@ #include "CrossPointSettings.h" #include "CrossPointState.h" #include "MappedInputManager.h" +#include "ReaderUtils.h" #include "RecentBooksStore.h" #include "components/UITheme.h" #include "fontIds.h" namespace { -constexpr unsigned long goHomeMs = 1000; constexpr size_t CHUNK_SIZE = 8 * 1024; // 8KB chunk for reading - // Cache file magic and version constexpr uint32_t CACHE_MAGIC = 0x54585449; // "TXTI" constexpr uint8_t CACHE_VERSION = 2; // Increment when cache format changes @@ -29,23 +28,7 @@ void TxtReaderActivity::onEnter() { return; } - // Configure screen orientation based on settings - switch (SETTINGS.orientation) { - case CrossPointSettings::ORIENTATION::PORTRAIT: - renderer.setOrientation(GfxRenderer::Orientation::Portrait); - break; - case CrossPointSettings::ORIENTATION::LANDSCAPE_CW: - renderer.setOrientation(GfxRenderer::Orientation::LandscapeClockwise); - break; - case CrossPointSettings::ORIENTATION::INVERTED: - renderer.setOrientation(GfxRenderer::Orientation::PortraitInverted); - break; - case CrossPointSettings::ORIENTATION::LANDSCAPE_CCW: - renderer.setOrientation(GfxRenderer::Orientation::LandscapeCounterClockwise); - break; - default: - break; - } + ReaderUtils::applyOrientation(renderer, SETTINGS.orientation); txt->setupCacheDir(); @@ -75,31 +58,19 @@ void TxtReaderActivity::onExit() { void TxtReaderActivity::loop() { // Long press BACK (1s+) goes to file selection - if (mappedInput.isPressed(MappedInputManager::Button::Back) && mappedInput.getHeldTime() >= goHomeMs) { + if (mappedInput.isPressed(MappedInputManager::Button::Back) && mappedInput.getHeldTime() >= ReaderUtils::GO_HOME_MS) { activityManager.goToFileBrowser(txt ? txt->getPath() : ""); return; } // Short press BACK goes directly to home - if (mappedInput.wasReleased(MappedInputManager::Button::Back) && mappedInput.getHeldTime() < goHomeMs) { + if (mappedInput.wasReleased(MappedInputManager::Button::Back) && + mappedInput.getHeldTime() < ReaderUtils::GO_HOME_MS) { onGoHome(); return; } - // When long-press chapter skip is disabled, turn pages on press instead of release. - const bool usePressForPageTurn = !SETTINGS.longPressChapterSkip; - const bool prevTriggered = usePressForPageTurn ? (mappedInput.wasPressed(MappedInputManager::Button::PageBack) || - mappedInput.wasPressed(MappedInputManager::Button::Left)) - : (mappedInput.wasReleased(MappedInputManager::Button::PageBack) || - mappedInput.wasReleased(MappedInputManager::Button::Left)); - const bool powerPageTurn = SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::PAGE_TURN && - mappedInput.wasReleased(MappedInputManager::Button::Power); - const bool nextTriggered = usePressForPageTurn - ? (mappedInput.wasPressed(MappedInputManager::Button::PageForward) || powerPageTurn || - mappedInput.wasPressed(MappedInputManager::Button::Right)) - : (mappedInput.wasReleased(MappedInputManager::Button::PageForward) || powerPageTurn || - mappedInput.wasReleased(MappedInputManager::Button::Right)); - + auto [prevTriggered, nextTriggered] = ReaderUtils::detectPageTurn(mappedInput); if (!prevTriggered && !nextTriggered) { return; } @@ -398,34 +369,10 @@ void TxtReaderActivity::renderPage() { renderLines(); renderStatusBar(); - if (pagesUntilFullRefresh <= 1) { - renderer.displayBuffer(HalDisplay::HALF_REFRESH); - pagesUntilFullRefresh = SETTINGS.getRefreshFrequency(); - } else { - renderer.displayBuffer(); - pagesUntilFullRefresh--; - } + ReaderUtils::displayWithRefreshCycle(renderer, pagesUntilFullRefresh); - // Grayscale rendering pass (for anti-aliased fonts) if (SETTINGS.textAntiAliasing) { - // Save BW buffer for restoration after grayscale pass - renderer.storeBwBuffer(); - - renderer.clearScreen(0x00); - renderer.setRenderMode(GfxRenderer::GRAYSCALE_LSB); - renderLines(); - renderer.copyGrayscaleLsbBuffers(); - - renderer.clearScreen(0x00); - renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB); - renderLines(); - renderer.copyGrayscaleMsbBuffers(); - - renderer.displayGrayBuffer(); - renderer.setRenderMode(GfxRenderer::BW); - - // Restore BW buffer - renderer.restoreBwBuffer(); + ReaderUtils::renderAntiAliased(renderer, [&renderLines]() { renderLines(); }); } } From cde401d22b5ed05f2c5b5e5e3af1d5fa74f0c7b4 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 8 Mar 2026 20:15:51 +0100 Subject: [PATCH 2/2] chore: micro-optimisation: early exit on fillUncompressedSizes (#1322) ## Summary * **What is the goal of this PR?** Avoid repeated full central-directory scans by using early stop when all requested targets are already matched. * **What changes are included?** ## Additional Context * Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks, specific areas to focus on). --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _** PARTIALLY **_ Identified by AI --- lib/ZipFile/ZipFile.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/ZipFile/ZipFile.cpp b/lib/ZipFile/ZipFile.cpp index a740e14d..58fa64d5 100644 --- a/lib/ZipFile/ZipFile.cpp +++ b/lib/ZipFile/ZipFile.cpp @@ -332,6 +332,7 @@ int ZipFile::fillUncompressedSizes(std::vector& targets, std::vector file.seek(zipDetails.centralDirOffset); int matched = 0; + const int targetCount = static_cast(targets.size()); uint32_t sig; char itemName[256]; @@ -372,6 +373,10 @@ int ZipFile::fillUncompressedSizes(std::vector& targets, std::vector } ++it; } + + if (matched >= targetCount) { + break; + } } else { file.seekCur(nameLen); }