From 9a263cb8716bf99e868c946f8bc674c172101961 Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Wed, 22 Apr 2026 20:50:18 +0300 Subject: [PATCH 01/19] feat: add clipping functionality to the reader - Introduced `ClipSelectionActivity` for selecting and saving text clips. - Added `ClippingsManager` to handle saving clips to a file. - Updated `EpubReaderActivity` to initiate clipping and save selected text. - Enhanced `EpubReaderMenuActivity` to include an option for saving clips. - Added new translations for "Save Clipping" in multiple languages. - Modified `TextBlock` to expose word positions and styles for clipping. - Implemented windowed display updates in `GfxRenderer` and `HalDisplay`. --- lib/Epub/Epub/blocks/TextBlock.h | 2 + lib/GfxRenderer/GfxRenderer.cpp | 4 + lib/GfxRenderer/GfxRenderer.h | 4 +- lib/I18n/translations/belarusian.yaml | 1 + lib/I18n/translations/catalan.yaml | 1 + lib/I18n/translations/czech.yaml | 1 + lib/I18n/translations/danish.yaml | 1 + lib/I18n/translations/dutch.yaml | 1 + lib/I18n/translations/english.yaml | 1 + lib/I18n/translations/finnish.yaml | 1 + lib/I18n/translations/french.yaml | 1 + lib/I18n/translations/german.yaml | 1 + lib/I18n/translations/hungarian.yaml | 1 + lib/I18n/translations/italian.yaml | 1 + lib/I18n/translations/kazakh.yaml | 1 + lib/I18n/translations/lithuanian.yaml | 1 + lib/I18n/translations/polish.yaml | 1 + lib/I18n/translations/portuguese.yaml | 1 + lib/I18n/translations/romanian.yaml | 1 + lib/I18n/translations/russian.yaml | 1 + lib/I18n/translations/slovenian.yaml | 1 + lib/I18n/translations/spanish.yaml | 1 + lib/I18n/translations/swedish.yaml | 1 + lib/I18n/translations/turkish.yaml | 1 + lib/I18n/translations/ukrainian.yaml | 1 + lib/hal/HalDisplay.cpp | 5 + lib/hal/HalDisplay.h | 1 + src/activities/ActivityResult.h | 6 +- .../reader/ClipSelectionActivity.cpp | 239 ++++++++++++++++++ src/activities/reader/ClipSelectionActivity.h | 63 +++++ src/activities/reader/EpubReaderActivity.cpp | 73 ++++++ .../reader/EpubReaderMenuActivity.cpp | 3 +- .../reader/EpubReaderMenuActivity.h | 3 +- src/clippings/ClippingsManager.cpp | 47 ++++ src/clippings/ClippingsManager.h | 14 + 35 files changed, 481 insertions(+), 5 deletions(-) create mode 100644 src/activities/reader/ClipSelectionActivity.cpp create mode 100644 src/activities/reader/ClipSelectionActivity.h create mode 100644 src/clippings/ClippingsManager.cpp create mode 100644 src/clippings/ClippingsManager.h diff --git a/lib/Epub/Epub/blocks/TextBlock.h b/lib/Epub/Epub/blocks/TextBlock.h index 85fdd55a39..0da365ce09 100644 --- a/lib/Epub/Epub/blocks/TextBlock.h +++ b/lib/Epub/Epub/blocks/TextBlock.h @@ -28,6 +28,8 @@ class TextBlock final : public Block { void setBlockStyle(const BlockStyle& blockStyle) { this->blockStyle = blockStyle; } const BlockStyle& getBlockStyle() const { return blockStyle; } const std::vector& getWords() const { return words; } + const std::vector& getWordXpos() const { return wordXpos; } + const std::vector& getWordStyles() const { return wordStyles; } bool isEmpty() override { return words.empty(); } size_t wordCount() const { return words.size(); } // given a renderer works out where to break the words into lines diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index 0fa56b7753..091188c520 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -926,6 +926,10 @@ void GfxRenderer::displayBuffer(const HalDisplay::RefreshMode refreshMode) const display.displayBuffer(refreshMode, fadingFix); } +void GfxRenderer::displayWindow(int x, int y, int width, int height) const { + display.displayWindow(x, y, width, height); +} + std::string GfxRenderer::truncatedText(const int fontId, const char* text, const int maxWidth, const EpdFontFamily::Style style) const { if (!text || maxWidth <= 0) return ""; diff --git a/lib/GfxRenderer/GfxRenderer.h b/lib/GfxRenderer/GfxRenderer.h index 9e24b39e54..3d9fe1fb31 100644 --- a/lib/GfxRenderer/GfxRenderer.h +++ b/lib/GfxRenderer/GfxRenderer.h @@ -84,8 +84,8 @@ class GfxRenderer { int getScreenWidth() const; int getScreenHeight() const; void displayBuffer(HalDisplay::RefreshMode refreshMode = HalDisplay::FAST_REFRESH) const; - // EXPERIMENTAL: Windowed update - display only a rectangular region - // void displayWindow(int x, int y, int width, int height) const; + // EXPERIMENTAL: Windowed update - display only a rectangular region (x and w must be multiples of 8) + void displayWindow(int x, int y, int width, int height) const; void invertScreen() const; void clearScreen(uint8_t color = 0xFF) const; void getOrientedViewableTRBL(int* outTop, int* outRight, int* outBottom, int* outLeft) const; diff --git a/lib/I18n/translations/belarusian.yaml b/lib/I18n/translations/belarusian.yaml index 6ce34536c2..539ea258c5 100644 --- a/lib/I18n/translations/belarusian.yaml +++ b/lib/I18n/translations/belarusian.yaml @@ -230,6 +230,7 @@ STR_GO_TO_PERCENT: "Перайсці да %" STR_GO_HOME_BUTTON: "На галоўную" STR_SYNC_PROGRESS: "Сінхранізаваць прагрэс" STR_DELETE_CACHE: "Выдаліць кэш кнігі" +STR_SAVE_CLIPPING: "Захаваць урывак" STR_CHAPTER_PREFIX: "Раздзел:" STR_PAGES_SEPARATOR: "стар. |" STR_BOOK_PREFIX: "Кніга:" diff --git a/lib/I18n/translations/catalan.yaml b/lib/I18n/translations/catalan.yaml index 5bc09c1068..fda8bfb1f5 100644 --- a/lib/I18n/translations/catalan.yaml +++ b/lib/I18n/translations/catalan.yaml @@ -256,6 +256,7 @@ STR_GO_TO_PERCENT: "Ves al %" STR_GO_HOME_BUTTON: "Ves a l'inici" STR_SYNC_PROGRESS: "Sincronitza el progrés" STR_DELETE_CACHE: "Esborra la memòria cau del llibre" +STR_SAVE_CLIPPING: "Desa la selecció" STR_DELETE: "Esborra" STR_DISPLAY_QR: "Mostra la pàgina com a QR" STR_CHAPTER_PREFIX: "Capítol: " diff --git a/lib/I18n/translations/czech.yaml b/lib/I18n/translations/czech.yaml index e2b0ef51e8..e0443b0ae8 100644 --- a/lib/I18n/translations/czech.yaml +++ b/lib/I18n/translations/czech.yaml @@ -234,6 +234,7 @@ STR_GO_TO_PERCENT: "Přejít na %" STR_GO_HOME_BUTTON: "Přejít Domů" STR_SYNC_PROGRESS: "Průběh synchronizace" STR_DELETE_CACHE: "Smazat mezipaměť knihy" +STR_SAVE_CLIPPING: "Uložit výběr" STR_DELETE: "Smazat" STR_CHAPTER_PREFIX: "Kapitola:" STR_PAGES_SEPARATOR: "stránek |" diff --git a/lib/I18n/translations/danish.yaml b/lib/I18n/translations/danish.yaml index 36ebc506ea..4ea9fa9686 100644 --- a/lib/I18n/translations/danish.yaml +++ b/lib/I18n/translations/danish.yaml @@ -256,6 +256,7 @@ STR_GO_TO_PERCENT: "Gå til %" STR_GO_HOME_BUTTON: "Gå til start" STR_SYNC_PROGRESS: "Synkroniser fremskridt" STR_DELETE_CACHE: "Slet bogcache" +STR_SAVE_CLIPPING: "Gem klip" STR_DELETE: "Slet" STR_DISPLAY_QR: "Vis side som QR" STR_CHAPTER_PREFIX: "Kapitel: " diff --git a/lib/I18n/translations/dutch.yaml b/lib/I18n/translations/dutch.yaml index 63a37be9ff..887b5fff5f 100644 --- a/lib/I18n/translations/dutch.yaml +++ b/lib/I18n/translations/dutch.yaml @@ -256,6 +256,7 @@ STR_GO_TO_PERCENT: "Ga naar %" STR_GO_HOME_BUTTON: "Naar Home" STR_SYNC_PROGRESS: "Voortgang synchroniseren" STR_DELETE_CACHE: "Boekcache verwijderen" +STR_SAVE_CLIPPING: "Selectie opslaan" STR_DELETE: "Verwijder" STR_DISPLAY_QR: "Pagina als QR tonen" STR_CHAPTER_PREFIX: "Hoofdstuk: " diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index b662389992..9062e90257 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -262,6 +262,7 @@ STR_GO_TO_PERCENT: "Go to %" STR_GO_HOME_BUTTON: "Go Home" STR_SYNC_PROGRESS: "Sync Progress" STR_DELETE_CACHE: "Delete Book Cache" +STR_SAVE_CLIPPING: "Save Clipping" STR_DELETE: "Delete" STR_DISPLAY_QR: "Show page as QR" STR_CHAPTER_PREFIX: "Chapter: " diff --git a/lib/I18n/translations/finnish.yaml b/lib/I18n/translations/finnish.yaml index 6832dd269c..8e0b515941 100644 --- a/lib/I18n/translations/finnish.yaml +++ b/lib/I18n/translations/finnish.yaml @@ -233,6 +233,7 @@ STR_GO_TO_PERCENT: "Siirry kohtaan %" STR_GO_HOME_BUTTON: "Siirry kotiin" STR_SYNC_PROGRESS: "Synkronoi edistyminen" STR_DELETE_CACHE: "Poista kirjan välimuisti" +STR_SAVE_CLIPPING: "Tallenna leike" STR_CHAPTER_PREFIX: "Luku: " STR_PAGES_SEPARATOR: " sivua | " STR_BOOK_PREFIX: "Kirja: " diff --git a/lib/I18n/translations/french.yaml b/lib/I18n/translations/french.yaml index fe5e062a9e..cf2474eb43 100644 --- a/lib/I18n/translations/french.yaml +++ b/lib/I18n/translations/french.yaml @@ -257,6 +257,7 @@ STR_GO_TO_PERCENT: "Aller à %" STR_GO_HOME_BUTTON: "Retour Accueil" STR_SYNC_PROGRESS: "Synchro progression" STR_DELETE_CACHE: "Supprimer cache livre" +STR_SAVE_CLIPPING: "Enregistrer la sélection" STR_DELETE: "Supprimer" STR_DISPLAY_QR: "Afficher la page en QR" STR_CHAPTER_PREFIX: "Chapitre : " diff --git a/lib/I18n/translations/german.yaml b/lib/I18n/translations/german.yaml index 61d36a1c39..76477f8cf3 100644 --- a/lib/I18n/translations/german.yaml +++ b/lib/I18n/translations/german.yaml @@ -257,6 +257,7 @@ STR_GO_TO_PERCENT: "Gehe zu %" STR_GO_HOME_BUTTON: "Zum Anfang" STR_SYNC_PROGRESS: "Fortschritt synchronisieren" STR_DELETE_CACHE: "Buch-Cache leeren" +STR_SAVE_CLIPPING: "Auswahl speichern" STR_DISPLAY_QR: "Seite als QR anzeigen" STR_DELETE: "Löschen" STR_REMOVE: "Entfernen" diff --git a/lib/I18n/translations/hungarian.yaml b/lib/I18n/translations/hungarian.yaml index 0343c1976b..84970acc22 100644 --- a/lib/I18n/translations/hungarian.yaml +++ b/lib/I18n/translations/hungarian.yaml @@ -253,6 +253,7 @@ STR_GO_TO_PERCENT: "Ugrás %-ra" STR_GO_HOME_BUTTON: "Főoldalra" STR_SYNC_PROGRESS: "Haladás szinkronizálása" STR_DELETE_CACHE: "Könyv gyorsítótár törlése" +STR_SAVE_CLIPPING: "Kijelölés mentése" STR_DELETE: "Törlés" STR_DISPLAY_QR: "Oldal megjelenítése QR-ként" STR_CHAPTER_PREFIX: "Fejezet: " diff --git a/lib/I18n/translations/italian.yaml b/lib/I18n/translations/italian.yaml index 25de985305..86fd581994 100644 --- a/lib/I18n/translations/italian.yaml +++ b/lib/I18n/translations/italian.yaml @@ -259,6 +259,7 @@ STR_SYNC_PROGRESS: "Sincronizza avanzamento" STR_DELETE_CACHE: "Elimina cache libro" STR_DELETE: "Elimina" STR_DISPLAY_QR: "Mostra pagina come QR" +STR_SAVE_CLIPPING: "Salva selezione" STR_CHAPTER_PREFIX: "Capitolo: " STR_PAGES_SEPARATOR: " pagine | " STR_BOOK_PREFIX: "Libro: " diff --git a/lib/I18n/translations/kazakh.yaml b/lib/I18n/translations/kazakh.yaml index 8ff2c99da0..53c989b5cb 100644 --- a/lib/I18n/translations/kazakh.yaml +++ b/lib/I18n/translations/kazakh.yaml @@ -229,6 +229,7 @@ STR_GO_TO_PERCENT: "%-ке өту" STR_GO_HOME_BUTTON: "Басты бетке өту" STR_SYNC_PROGRESS: "Үлгерімді синхрондау" STR_DELETE_CACHE: "Кітап кэшін жою" +STR_SAVE_CLIPPING: "Үзінді сақтау" STR_CHAPTER_PREFIX: "Тарау: " STR_PAGES_SEPARATOR: " бет | " STR_BOOK_PREFIX: "Кітап: " diff --git a/lib/I18n/translations/lithuanian.yaml b/lib/I18n/translations/lithuanian.yaml index c004c49e72..6fb447907a 100644 --- a/lib/I18n/translations/lithuanian.yaml +++ b/lib/I18n/translations/lithuanian.yaml @@ -253,6 +253,7 @@ STR_GO_TO_PERCENT: "Eiti į %" STR_GO_HOME_BUTTON: "Pradžia" STR_SYNC_PROGRESS: "Sinchr. progresą" STR_DELETE_CACHE: "Trinti talpyklą" +STR_SAVE_CLIPPING: "Išsaugoti ištrauką" STR_DELETE: "Trinti" STR_DISPLAY_QR: "QR kodas" STR_CHAPTER_PREFIX: "Sk: " diff --git a/lib/I18n/translations/polish.yaml b/lib/I18n/translations/polish.yaml index 4a5c0af754..3b3a0715a9 100644 --- a/lib/I18n/translations/polish.yaml +++ b/lib/I18n/translations/polish.yaml @@ -256,6 +256,7 @@ STR_GO_TO_PERCENT: "Idź do %" STR_GO_HOME_BUTTON: "Wróć do głównego ekranu" STR_SYNC_PROGRESS: "Postęp synchronizacji" STR_DELETE_CACHE: "Usuń pamięć podręczną książek" +STR_SAVE_CLIPPING: "Zapisz fragment" STR_DELETE: "Usuń" STR_DISPLAY_QR: "Pokaż stronę jako kod QR" STR_CHAPTER_PREFIX: "Rozdział: " diff --git a/lib/I18n/translations/portuguese.yaml b/lib/I18n/translations/portuguese.yaml index f7379c1649..8fe5095986 100644 --- a/lib/I18n/translations/portuguese.yaml +++ b/lib/I18n/translations/portuguese.yaml @@ -234,6 +234,7 @@ STR_GO_TO_PERCENT: "Ir para %" STR_GO_HOME_BUTTON: "Ir para o início" STR_SYNC_PROGRESS: "Sincronizar progresso" STR_DELETE_CACHE: "Excluir cache do livro" +STR_SAVE_CLIPPING: "Salvar trecho" STR_DELETE: "Excluir" STR_CHAPTER_PREFIX: "Capítulo:" STR_PAGES_SEPARATOR: "páginas |" diff --git a/lib/I18n/translations/romanian.yaml b/lib/I18n/translations/romanian.yaml index 2a4856a351..68969c8f16 100644 --- a/lib/I18n/translations/romanian.yaml +++ b/lib/I18n/translations/romanian.yaml @@ -256,6 +256,7 @@ STR_GO_TO_PERCENT: "Săriţi la %" STR_GO_HOME_BUTTON: "Acasă" STR_SYNC_PROGRESS: "Progres sincronizare" STR_DELETE_CACHE: "Ştergere cache cărţi" +STR_SAVE_CLIPPING: "Salvează selecţia" STR_DELETE: "Ştergeți" STR_DISPLAY_QR: "Afişați pagina ca cod QR" STR_CHAPTER_PREFIX: "Capitol: " diff --git a/lib/I18n/translations/russian.yaml b/lib/I18n/translations/russian.yaml index dda0acc02c..6174b88c95 100644 --- a/lib/I18n/translations/russian.yaml +++ b/lib/I18n/translations/russian.yaml @@ -260,6 +260,7 @@ STR_GO_TO_PERCENT: "Перейти к %" STR_GO_HOME_BUTTON: "На главную" STR_SYNC_PROGRESS: "Синхронизировать прогресс" STR_DELETE_CACHE: "Удалить кэш книги" +STR_SAVE_CLIPPING: "Сохранить фрагмент" STR_DELETE: "Удалить" STR_CHAPTER_PREFIX: "Глава: " STR_DISPLAY_QR: "Показать страницу в виде QR-кода" diff --git a/lib/I18n/translations/slovenian.yaml b/lib/I18n/translations/slovenian.yaml index 19dcf7843c..b8a52d036c 100644 --- a/lib/I18n/translations/slovenian.yaml +++ b/lib/I18n/translations/slovenian.yaml @@ -253,6 +253,7 @@ STR_GO_TO_PERCENT: "Pojdi na %" STR_GO_HOME_BUTTON: "Pojdi domov" STR_SYNC_PROGRESS: "Sinhroniziraj napredek" STR_DELETE_CACHE: "Izbriši predpomnilnik knjige" +STR_SAVE_CLIPPING: "Shrani odlomek" STR_DELETE: "Izbriši" STR_DISPLAY_QR: "Prikaži stran kot QR" STR_CHAPTER_PREFIX: "Poglavje: " diff --git a/lib/I18n/translations/spanish.yaml b/lib/I18n/translations/spanish.yaml index 5cb76a23ab..a0516b9719 100644 --- a/lib/I18n/translations/spanish.yaml +++ b/lib/I18n/translations/spanish.yaml @@ -257,6 +257,7 @@ STR_GO_TO_PERCENT: "Ir a %" STR_GO_HOME_BUTTON: "Volver al menú Inicio" STR_SYNC_PROGRESS: "Sincronizar progreso de lectura" STR_DELETE_CACHE: "Borrar caché del libro" +STR_SAVE_CLIPPING: "Guardar selección" STR_DELETE: "Borrar" STR_DISPLAY_QR: "Mostrar página como QR" STR_CHAPTER_PREFIX: "Cap.: " diff --git a/lib/I18n/translations/swedish.yaml b/lib/I18n/translations/swedish.yaml index 73444fad04..ecaf99a2f7 100644 --- a/lib/I18n/translations/swedish.yaml +++ b/lib/I18n/translations/swedish.yaml @@ -262,6 +262,7 @@ STR_GO_TO_PERCENT: "Gå till %" STR_GO_HOME_BUTTON: "Gå Hem" STR_SYNC_PROGRESS: "Synkroniseringsframsteg" STR_DELETE_CACHE: "Radera bokcache" +STR_SAVE_CLIPPING: "Spara klipp" STR_DELETE: "Radera" STR_DISPLAY_QR: "Visa sida som QR-kod" STR_CHAPTER_PREFIX: "Kapitel:" diff --git a/lib/I18n/translations/turkish.yaml b/lib/I18n/translations/turkish.yaml index 3c8c679a3f..71ce2c7130 100644 --- a/lib/I18n/translations/turkish.yaml +++ b/lib/I18n/translations/turkish.yaml @@ -233,6 +233,7 @@ STR_GO_TO_PERCENT: "%'ye git" STR_GO_HOME_BUTTON: "Ana Sayfaya Git" STR_SYNC_PROGRESS: "Okuma İlerlemesini Senkronize Et" STR_DELETE_CACHE: "Kitap Önbelleğini Sil" +STR_SAVE_CLIPPING: "Kırpıyı Kaydet" STR_CHAPTER_PREFIX: "Bölüm: " STR_PAGES_SEPARATOR: " sayfa | " STR_BOOK_PREFIX: "Kitap: " diff --git a/lib/I18n/translations/ukrainian.yaml b/lib/I18n/translations/ukrainian.yaml index c556aef398..02126482a1 100644 --- a/lib/I18n/translations/ukrainian.yaml +++ b/lib/I18n/translations/ukrainian.yaml @@ -261,6 +261,7 @@ STR_GO_TO_PERCENT: "Перейти до %" STR_GO_HOME_BUTTON: "На головну" STR_SYNC_PROGRESS: "Прогрес синхронізації" STR_DELETE_CACHE: "Видалити кеш книги" +STR_SAVE_CLIPPING: "Зберегти уривок" STR_DELETE: "Видалити" STR_DISPLAY_QR: "Показати сторінку як QR-код" STR_CHAPTER_PREFIX: "Розділ: " diff --git a/lib/hal/HalDisplay.cpp b/lib/hal/HalDisplay.cpp index 665df5356b..3f721658e1 100644 --- a/lib/hal/HalDisplay.cpp +++ b/lib/hal/HalDisplay.cpp @@ -58,6 +58,11 @@ void HalDisplay::displayBuffer(HalDisplay::RefreshMode mode, bool turnOffScreen) einkDisplay.displayBuffer(convertRefreshMode(mode), turnOffScreen); } +void HalDisplay::displayWindow(int x, int y, int w, int h) { + einkDisplay.displayWindow(static_cast(x), static_cast(y), + static_cast(w), static_cast(h)); +} + void HalDisplay::refreshDisplay(HalDisplay::RefreshMode mode, bool turnOffScreen) { if (gpio.deviceIsX3() && mode == RefreshMode::HALF_REFRESH) { einkDisplay.requestResync(1); diff --git a/lib/hal/HalDisplay.h b/lib/hal/HalDisplay.h index a0a7f92083..01ee7e0687 100644 --- a/lib/hal/HalDisplay.h +++ b/lib/hal/HalDisplay.h @@ -34,6 +34,7 @@ class HalDisplay { bool fromProgmem = false) const; void displayBuffer(RefreshMode mode = RefreshMode::FAST_REFRESH, bool turnOffScreen = false); + void displayWindow(int x, int y, int w, int h); void refreshDisplay(RefreshMode mode = RefreshMode::FAST_REFRESH, bool turnOffScreen = false); // Power management diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index d25df09042..5ce9271edc 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -54,8 +54,12 @@ struct FilePathResult { std::string path; }; +struct ClippingResult { + std::string text; +}; + using ResultVariant = std::variant; + PageResult, SyncResult, NetworkModeResult, FootnoteResult, FilePathResult, ClippingResult>; struct ActivityResult { bool isCancelled = false; diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp new file mode 100644 index 0000000000..823d3a3fd9 --- /dev/null +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -0,0 +1,239 @@ +#include "ClipSelectionActivity.h" + +#include +#include +#include +#include + +#include +#include + +#include "../ActivityResult.h" +#include "MappedInputManager.h" +#include "components/UITheme.h" + +ClipSelectionActivity::ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, + std::vector words, std::string bookTitle, std::string author, + std::string chapterTitle, int pageNumber, int fontId, Section& section, + int startPageInSection, int marginTop, int marginLeft) + : Activity("ClipSelection", renderer, mappedInput), + words(std::move(words)), + bookTitle(std::move(bookTitle)), + author(std::move(author)), + chapterTitle(std::move(chapterTitle)), + pageNumber(pageNumber), + fontId(fontId), + section(section), + startPageInSection(startPageInSection), + marginTop(marginTop), + marginLeft(marginLeft) {} + +void ClipSelectionActivity::onEnter() { + Activity::onEnter(); + + if (words.empty()) { + LOG_ERR("CLIP", "No words available for selection"); + ActivityResult result; + result.isCancelled = true; + setResult(std::move(result)); + finish(); + return; + } + + savedBufferSize = renderer.getBufferSize(); + savedBuffer = static_cast(malloc(savedBufferSize)); + if (!savedBuffer) { + LOG_ERR("CLIP", "malloc failed: %u bytes", savedBufferSize); + ActivityResult result; + result.isCancelled = true; + setResult(std::move(result)); + finish(); + return; + } + + // Re-render page 0 to get a clean framebuffer — the previous activity (menu) + // may still be painted on screen when onEnter() runs. + switchToPage(0); + requestUpdate(); +} + +void ClipSelectionActivity::onExit() { + free(savedBuffer); + savedBuffer = nullptr; + Activity::onExit(); +} + +void ClipSelectionActivity::loop() { + const int total = static_cast(words.size()); + + buttonNavigator.onNextRelease([this, total] { + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = ButtonNavigator::nextIndex(cursorIdx, total); + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + buttonNavigator.onNextContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = lineEndForward(cursorIdx); + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + buttonNavigator.onPreviousRelease([this, total] { + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = ButtonNavigator::previousIndex(cursorIdx, total); + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + buttonNavigator.onPreviousContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = lineEndBackward(cursorIdx); + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { + if (startMarkIdx == -1) { + startMarkIdx = cursorIdx; + requestUpdate(); + } else { + const int from = std::min(startMarkIdx, cursorIdx); + const int to = std::max(startMarkIdx, cursorIdx); + std::string text; + for (int i = from; i <= to; ++i) { + if (!text.empty()) text += ' '; + text += words[i].text; + } + ActivityResult result; + result.data = ClippingResult{std::move(text)}; + setResult(std::move(result)); + finish(); + } + return; + } + + if (mappedInput.wasReleased(MappedInputManager::Button::Back)) { + if (startMarkIdx != -1) { + startMarkIdx = -1; + requestUpdate(); + } else { + ActivityResult result; + result.isCancelled = true; + setResult(std::move(result)); + finish(); + } + } +} + +void ClipSelectionActivity::render(RenderLock&&) { + if (!savedBuffer) return; + + if (needsPageSwitch) { + switchToPage(words[cursorIdx].pageIdx); + needsPageSwitch = false; + } + + // Restore the saved page framebuffer, then draw highlights on top + memcpy(renderer.getFrameBuffer(), savedBuffer, savedBufferSize); + drawHighlights(); + + const auto labels = mappedInput.mapLabels(tr(STR_BACK), tr(STR_SELECT), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); + GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); + + renderer.displayBuffer(HalDisplay::FAST_REFRESH); +} + +void ClipSelectionActivity::switchToPage(int pageIdx) { + section.currentPage = startPageInSection + pageIdx; + auto page = section.loadPageFromSectionFile(); + if (!page) { + LOG_ERR("CLIP", "Failed to load page %d for display", pageIdx); + return; + } + + renderer.clearScreen(); + page->render(renderer, fontId, marginLeft, marginTop); + // displayBuffer is intentionally omitted here — render() always controls the final display call + memcpy(savedBuffer, renderer.getFrameBuffer(), savedBufferSize); + currentDisplayPage = pageIdx; +} + +void ClipSelectionActivity::drawHighlights() { + // Draw selection range (words on the currently displayed page only) + if (startMarkIdx != -1) { + const int from = std::min(startMarkIdx, cursorIdx); + const int to = std::max(startMarkIdx, cursorIdx); + for (int i = from; i <= to; ++i) { + if (i == cursorIdx) continue; + if (words[i].pageIdx != currentDisplayPage) continue; + renderer.fillRectDither(words[i].x, words[i].y, words[i].w, words[i].h, Color::LightGray); + renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); + } + } + + // Draw cursor highlight (always on top) + const auto& cw = words[cursorIdx]; + if (cw.pageIdx == currentDisplayPage) { + renderer.fillRectDither(cw.x, cw.y, cw.w, cw.h, Color::LightGray); + renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); + } +} + +ClipSelectionActivity::Rect ClipSelectionActivity::alignedRect(int x, int y, int w, int h) const { + const int alignedX = (x / 8) * 8; + const int alignedW = ((x + w + 7) / 8) * 8 - alignedX; + return {alignedX, y, alignedW, h}; +} + +int ClipSelectionActivity::lineEndForward(int idx) const { + const int total = static_cast(words.size()); + const int lineY = words[idx].y; + const int page = words[idx].pageIdx; + + // Find last word on the same line + int last = idx; + for (int i = idx + 1; i < total; ++i) { + if (words[i].pageIdx != page || words[i].y != lineY) break; + last = i; + } + + // Already at line end — jump to end of next line + if (last == idx && idx + 1 < total) { + const int nextY = words[idx + 1].y; + const int nextPage = words[idx + 1].pageIdx; + last = idx + 1; + for (int i = idx + 2; i < total; ++i) { + if (words[i].pageIdx != nextPage || words[i].y != nextY) break; + last = i; + } + } + + return last; +} + +int ClipSelectionActivity::lineEndBackward(int idx) const { + const int lineY = words[idx].y; + const int page = words[idx].pageIdx; + + // Find first word on the same line + int first = idx; + for (int i = idx - 1; i >= 0; --i) { + if (words[i].pageIdx != page || words[i].y != lineY) break; + first = i; + } + + // Already at line start — jump to start of previous line + if (first == idx && idx - 1 >= 0) { + const int prevY = words[idx - 1].y; + const int prevPage = words[idx - 1].pageIdx; + first = idx - 1; + for (int i = idx - 2; i >= 0; --i) { + if (words[i].pageIdx != prevPage || words[i].y != prevY) break; + first = i; + } + } + + return first; +} diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h new file mode 100644 index 0000000000..29848a53ed --- /dev/null +++ b/src/activities/reader/ClipSelectionActivity.h @@ -0,0 +1,63 @@ +#pragma once + +#include +#include + +#include +#include + +#include "../Activity.h" +#include "util/ButtonNavigator.h" + +class ClipSelectionActivity final : public Activity { + public: + struct WordRef { + int x, y, w, h; + int pageIdx; // 0 = current, 1 = next, 2 = page after next + std::string text; + }; + + ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector words, + std::string bookTitle, std::string author, std::string chapterTitle, int pageNumber, + int fontId, Section& section, int startPageInSection, int marginTop, int marginLeft); + + void onEnter() override; + void onExit() override; + void loop() override; + void render(RenderLock&&) override; + bool isReaderActivity() const override { return true; } + + private: + struct Rect { + int x, y, w, h; + }; + + std::vector words; + std::string bookTitle; + std::string author; + std::string chapterTitle; + int pageNumber; + int fontId; + + Section& section; + int startPageInSection; + int marginTop; + int marginLeft; + + uint8_t* savedBuffer = nullptr; + size_t savedBufferSize = 0; + int currentDisplayPage = 0; + + int cursorIdx = 0; + int startMarkIdx = -1; + bool needsPageSwitch = false; + bool initialRender = true; + + ButtonNavigator buttonNavigator; + + Rect alignedRect(int x, int y, int w, int h) const; + void switchToPage(int pageIdx); + void drawHighlights(); + int lineEndForward(int idx) const; + int lineEndBackward(int idx) const; +}; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 1426544bf6..1e0e5064f8 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -26,6 +26,8 @@ #include "RecentBooksStore.h" #include "components/UITheme.h" #include "fontIds.h" +#include "ClipSelectionActivity.h" +#include "clippings/ClippingsManager.h" #include "util/ScreenshotUtil.h" namespace { @@ -406,6 +408,77 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction requestUpdate(); break; } + case EpubReaderMenuActivity::MenuAction::SAVE_CLIPPING: { + if (section && epub) { + // Compute margins matching the reader render pass + int mTop, mRight, mBottom, mLeft; + renderer.getOrientedViewableTRBL(&mTop, &mRight, &mBottom, &mLeft); + mTop += SETTINGS.screenMargin; + mLeft += SETTINGS.screenMargin; + + const int readerFontId = SETTINGS.getReaderFontId(); + const int lineH = renderer.getLineHeight(readerFontId); + const int startPage = section->currentPage; + const int pagesToLoad = std::min(3, section->pageCount - startPage); + + std::vector words; + words.reserve(pagesToLoad * 60); // rough estimate + + for (int pi = 0; pi < pagesToLoad; ++pi) { + section->currentPage = startPage + pi; + auto page = section->loadPageFromSectionFile(); + if (!page) break; + + for (const auto& el : page->elements) { + if (el->getTag() != TAG_PageLine) continue; + const auto& line = static_cast(*el); + if (!line.getBlock()) continue; + const auto& block = *line.getBlock(); + const auto& xpos = block.getWordXpos(); + const auto& wlist = block.getWords(); + + for (int i = 0; i < static_cast(wlist.size()); ++i) { + const int wx = mLeft + line.xPos + xpos[i]; + const int wy = mTop + line.yPos; + const int ww = (i + 1 < static_cast(xpos.size())) + ? static_cast(xpos[i + 1]) - static_cast(xpos[i]) + : renderer.getTextWidth(readerFontId, wlist[i].c_str()); + if (ww > 0) { + words.push_back({wx, wy, ww, lineH, pi, wlist[i]}); + } + } + } + } + section->currentPage = startPage; + + if (!words.empty()) { + std::string chapterTitle; + const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); + if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; + + startActivityForResult( + std::make_unique(renderer, mappedInput, std::move(words), epub->getTitle(), + epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, + *section, startPage, mTop, mLeft), + [this](const ActivityResult& result) { + if (!result.isCancelled) { + const auto& clip = std::get(result.data); + if (!clip.text.empty()) { + std::string chapterTitle; + const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); + if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, + section ? section->currentPage + 1 : 0, clip.text); + } + } + requestUpdate(); + }); + } else { + requestUpdate(); + } + } + break; + } case EpubReaderMenuActivity::MenuAction::SYNC: { if (KOREADER_STORE.hasCredentials()) { const int currentPage = section ? section->currentPage : nextPageNumber; diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 1d95d9b7a1..1dc329973b 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -21,7 +21,7 @@ EpubReaderMenuActivity::EpubReaderMenuActivity(GfxRenderer& renderer, MappedInpu std::vector EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes) { std::vector items; - items.reserve(10); + items.reserve(11); items.push_back({MenuAction::SELECT_CHAPTER, StrId::STR_SELECT_CHAPTER}); if (hasFootnotes) { items.push_back({MenuAction::FOOTNOTES, StrId::STR_FOOTNOTES}); @@ -34,6 +34,7 @@ std::vector EpubReaderMenuActivity::buildMenuI items.push_back({MenuAction::GO_HOME, StrId::STR_GO_HOME_BUTTON}); items.push_back({MenuAction::SYNC, StrId::STR_SYNC_PROGRESS}); items.push_back({MenuAction::DELETE_CACHE, StrId::STR_DELETE_CACHE}); + items.push_back({MenuAction::SAVE_CLIPPING, StrId::STR_SAVE_CLIPPING}); return items; } diff --git a/src/activities/reader/EpubReaderMenuActivity.h b/src/activities/reader/EpubReaderMenuActivity.h index 9ddba93db8..808c2d279b 100644 --- a/src/activities/reader/EpubReaderMenuActivity.h +++ b/src/activities/reader/EpubReaderMenuActivity.h @@ -21,7 +21,8 @@ class EpubReaderMenuActivity final : public Activity { DISPLAY_QR, GO_HOME, SYNC, - DELETE_CACHE + DELETE_CACHE, + SAVE_CLIPPING }; explicit EpubReaderMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::string& title, diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp new file mode 100644 index 0000000000..aa8bdae722 --- /dev/null +++ b/src/clippings/ClippingsManager.cpp @@ -0,0 +1,47 @@ +#include "ClippingsManager.h" + +#include +#include +#include + +bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::string& author, + const std::string& chapterTitle, int pageNumber, + const std::string& selectedText) { + HalFile file = Storage.open(CLIPPINGS_PATH, O_RDWR | O_CREAT | O_AT_END); + if (!file) { + LOG_ERR("CLIP", "Failed to open %s for append", CLIPPINGS_PATH); + return false; + } + + // Header line: "Title / Author" + char header[128]; + snprintf(header, sizeof(header), "%s / %s\n", bookTitle.c_str(), author.c_str()); + + // Location line: "Chapter: X | Page N" + char location[128]; + if (!chapterTitle.empty()) { + snprintf(location, sizeof(location), "Chapter: %s | Page %d\n", chapterTitle.c_str(), pageNumber); + } else { + snprintf(location, sizeof(location), "Page %d\n", pageNumber); + } + + // Body: quoted text, trimmed to 2000 chars to avoid writing huge pages + static constexpr size_t MAX_TEXT = 2000; + const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; + + char quote[8]; + snprintf(quote, sizeof(quote), "\n\""); + + char separator[] = "\"\n\n==========\n\n"; + + file.write(header, strlen(header)); + file.write(location, strlen(location)); + file.write(quote, strlen(quote)); + file.write(selectedText.c_str(), textLen); + file.write(separator, strlen(separator)); + file.flush(); + file.close(); + + LOG_DBG("CLIP", "Saved clipping to %s (%zu chars)", CLIPPINGS_PATH, textLen); + return true; +} diff --git a/src/clippings/ClippingsManager.h b/src/clippings/ClippingsManager.h new file mode 100644 index 0000000000..fc10fff22f --- /dev/null +++ b/src/clippings/ClippingsManager.h @@ -0,0 +1,14 @@ +#pragma once + +#include + +class ClippingsManager { + public: + // Appends a clipping entry to /clippings.txt on the SD card. + // Returns false if the SD write fails. + static bool saveClipping(const std::string& bookTitle, const std::string& author, + const std::string& chapterTitle, int pageNumber, + const std::string& selectedText); + + static constexpr const char* CLIPPINGS_PATH = "/clippings.txt"; +}; From 0f7247e93f86d8686131c55986a5e78e437d4d97 Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Wed, 22 Apr 2026 21:15:45 +0300 Subject: [PATCH 02/19] fix: resolve CI failures for clang-format and cppcheck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Apply clang-format-21 formatting to all modified files - Fix variable shadowing in EpubReaderActivity lambda (chapterTitle/tocIdx → clipChapterTitle/clipTocIdx) to pass cppcheck style check - Replace snprintf with constexpr initialization in ClippingsManager for the quote/separator constants --- lib/hal/HalDisplay.cpp | 4 ++-- src/activities/reader/ClipSelectionActivity.cpp | 6 ++++-- src/activities/reader/ClipSelectionActivity.h | 4 ++-- src/activities/reader/EpubReaderActivity.cpp | 16 ++++++++-------- src/clippings/ClippingsManager.cpp | 9 +++------ src/clippings/ClippingsManager.h | 5 ++--- 6 files changed, 21 insertions(+), 23 deletions(-) diff --git a/lib/hal/HalDisplay.cpp b/lib/hal/HalDisplay.cpp index 3f721658e1..079f90eacb 100644 --- a/lib/hal/HalDisplay.cpp +++ b/lib/hal/HalDisplay.cpp @@ -59,8 +59,8 @@ void HalDisplay::displayBuffer(HalDisplay::RefreshMode mode, bool turnOffScreen) } void HalDisplay::displayWindow(int x, int y, int w, int h) { - einkDisplay.displayWindow(static_cast(x), static_cast(y), - static_cast(w), static_cast(h)); + einkDisplay.displayWindow(static_cast(x), static_cast(y), static_cast(w), + static_cast(h)); } void HalDisplay::refreshDisplay(HalDisplay::RefreshMode mode, bool turnOffScreen) { diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 823d3a3fd9..737b5b992c 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -168,7 +168,8 @@ void ClipSelectionActivity::drawHighlights() { for (int i = from; i <= to; ++i) { if (i == cursorIdx) continue; if (words[i].pageIdx != currentDisplayPage) continue; - renderer.fillRectDither(words[i].x, words[i].y, words[i].w, words[i].h, Color::LightGray); + const auto r = alignedRect(words[i].x, words[i].y, words[i].w, words[i].h); + renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); } } @@ -176,7 +177,8 @@ void ClipSelectionActivity::drawHighlights() { // Draw cursor highlight (always on top) const auto& cw = words[cursorIdx]; if (cw.pageIdx == currentDisplayPage) { - renderer.fillRectDither(cw.x, cw.y, cw.w, cw.h, Color::LightGray); + const auto r = alignedRect(cw.x, cw.y, cw.w, cw.h); + renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); } } diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h index 29848a53ed..b86e69b2fd 100644 --- a/src/activities/reader/ClipSelectionActivity.h +++ b/src/activities/reader/ClipSelectionActivity.h @@ -18,8 +18,8 @@ class ClipSelectionActivity final : public Activity { }; ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector words, - std::string bookTitle, std::string author, std::string chapterTitle, int pageNumber, - int fontId, Section& section, int startPageInSection, int marginTop, int marginLeft); + std::string bookTitle, std::string author, std::string chapterTitle, int pageNumber, int fontId, + Section& section, int startPageInSection, int marginTop, int marginLeft); void onEnter() override; void onExit() override; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 1e0e5064f8..bd210f2fa3 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -13,6 +13,7 @@ #include #include +#include "ClipSelectionActivity.h" #include "CrossPointSettings.h" #include "CrossPointState.h" #include "EpubReaderChapterSelectionActivity.h" @@ -24,10 +25,9 @@ #include "QrDisplayActivity.h" #include "ReaderUtils.h" #include "RecentBooksStore.h" +#include "clippings/ClippingsManager.h" #include "components/UITheme.h" #include "fontIds.h" -#include "ClipSelectionActivity.h" -#include "clippings/ClippingsManager.h" #include "util/ScreenshotUtil.h" namespace { @@ -458,16 +458,16 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction startActivityForResult( std::make_unique(renderer, mappedInput, std::move(words), epub->getTitle(), - epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, - *section, startPage, mTop, mLeft), + epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, + *section, startPage, mTop, mLeft), [this](const ActivityResult& result) { if (!result.isCancelled) { const auto& clip = std::get(result.data); if (!clip.text.empty()) { - std::string chapterTitle; - const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); - if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; - ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, + std::string clipChapterTitle; + const int clipTocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); + if (clipTocIdx >= 0) clipChapterTitle = epub->getTocItem(clipTocIdx).title; + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), clipChapterTitle, section ? section->currentPage + 1 : 0, clip.text); } } diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index aa8bdae722..a1a454993e 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -5,8 +5,7 @@ #include bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::string& author, - const std::string& chapterTitle, int pageNumber, - const std::string& selectedText) { + const std::string& chapterTitle, int pageNumber, const std::string& selectedText) { HalFile file = Storage.open(CLIPPINGS_PATH, O_RDWR | O_CREAT | O_AT_END); if (!file) { LOG_ERR("CLIP", "Failed to open %s for append", CLIPPINGS_PATH); @@ -29,10 +28,8 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str static constexpr size_t MAX_TEXT = 2000; const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; - char quote[8]; - snprintf(quote, sizeof(quote), "\n\""); - - char separator[] = "\"\n\n==========\n\n"; + static constexpr char quote[] = "\n\""; + static constexpr char separator[] = "\"\n\n==========\n\n"; file.write(header, strlen(header)); file.write(location, strlen(location)); diff --git a/src/clippings/ClippingsManager.h b/src/clippings/ClippingsManager.h index fc10fff22f..dbea0a7bf0 100644 --- a/src/clippings/ClippingsManager.h +++ b/src/clippings/ClippingsManager.h @@ -6,9 +6,8 @@ class ClippingsManager { public: // Appends a clipping entry to /clippings.txt on the SD card. // Returns false if the SD write fails. - static bool saveClipping(const std::string& bookTitle, const std::string& author, - const std::string& chapterTitle, int pageNumber, - const std::string& selectedText); + static bool saveClipping(const std::string& bookTitle, const std::string& author, const std::string& chapterTitle, + int pageNumber, const std::string& selectedText); static constexpr const char* CLIPPINGS_PATH = "/clippings.txt"; }; From 71f61a2037ddc9b92d2a15b77286ff991241790a Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Wed, 22 Apr 2026 21:21:32 +0300 Subject: [PATCH 03/19] fix: address coderabbitai review for clipping feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ClipSelectionActivity: save and restore section.currentPage in onEnter/onExit to prevent stale page index in parent activity - EpubReaderActivity: capture chapterTitle/startPage by value in lambda, eliminating duplicated TOC lookup and unreliable section->currentPage read - GfxRenderer::displayWindow: transform logical→physical coordinates per orientation and align to 8-pixel e-ink byte boundaries - HalDisplay::displayWindow: clamp negative/out-of-bounds inputs before casting to uint16_t to prevent wrap-around values reaching the driver - ClippingsManager: check write() return values; save to /My Clippings.txt for Kindle-compatible filename - Turkish i18n: fix STR_SAVE_CLIPPING to "Seçimi Kaydet" (save selection) --- lib/GfxRenderer/GfxRenderer.cpp | 33 ++++++++++++++++++- lib/I18n/translations/turkish.yaml | 2 +- lib/hal/HalDisplay.cpp | 5 +++ .../reader/ClipSelectionActivity.cpp | 2 ++ src/activities/reader/ClipSelectionActivity.h | 1 + src/activities/reader/EpubReaderActivity.cpp | 30 ++++++++--------- src/clippings/ClippingsManager.cpp | 22 +++++++++---- src/clippings/ClippingsManager.h | 4 +-- 8 files changed, 72 insertions(+), 27 deletions(-) diff --git a/lib/GfxRenderer/GfxRenderer.cpp b/lib/GfxRenderer/GfxRenderer.cpp index 091188c520..79a02fc986 100644 --- a/lib/GfxRenderer/GfxRenderer.cpp +++ b/lib/GfxRenderer/GfxRenderer.cpp @@ -927,7 +927,38 @@ void GfxRenderer::displayBuffer(const HalDisplay::RefreshMode refreshMode) const } void GfxRenderer::displayWindow(int x, int y, int width, int height) const { - display.displayWindow(x, y, width, height); + int phyX, phyY, phyW, phyH; + switch (orientation) { + case Portrait: + phyX = x; + phyY = panelHeight - y - height; + phyW = width; + phyH = height; + break; + case LandscapeClockwise: + phyX = panelWidth - x - width; + phyY = panelHeight - y - height; + phyW = width; + phyH = height; + break; + case PortraitInverted: + phyX = panelWidth - x - width; + phyY = y; + phyW = width; + phyH = height; + break; + case LandscapeCounterClockwise: + default: + phyX = x; + phyY = y; + phyW = width; + phyH = height; + break; + } + // Align to 8-pixel (byte) boundaries required by e-ink panel DMA + const int alignedX = (phyX / 8) * 8; + const int alignedW = ((phyX + phyW + 7) / 8) * 8 - alignedX; + display.displayWindow(alignedX, phyY, alignedW, phyH); } std::string GfxRenderer::truncatedText(const int fontId, const char* text, const int maxWidth, diff --git a/lib/I18n/translations/turkish.yaml b/lib/I18n/translations/turkish.yaml index 71ce2c7130..24bc6ee886 100644 --- a/lib/I18n/translations/turkish.yaml +++ b/lib/I18n/translations/turkish.yaml @@ -233,7 +233,7 @@ STR_GO_TO_PERCENT: "%'ye git" STR_GO_HOME_BUTTON: "Ana Sayfaya Git" STR_SYNC_PROGRESS: "Okuma İlerlemesini Senkronize Et" STR_DELETE_CACHE: "Kitap Önbelleğini Sil" -STR_SAVE_CLIPPING: "Kırpıyı Kaydet" +STR_SAVE_CLIPPING: "Seçimi Kaydet" STR_CHAPTER_PREFIX: "Bölüm: " STR_PAGES_SEPARATOR: " sayfa | " STR_BOOK_PREFIX: "Kitap: " diff --git a/lib/hal/HalDisplay.cpp b/lib/hal/HalDisplay.cpp index 079f90eacb..17635b4943 100644 --- a/lib/hal/HalDisplay.cpp +++ b/lib/hal/HalDisplay.cpp @@ -59,6 +59,11 @@ void HalDisplay::displayBuffer(HalDisplay::RefreshMode mode, bool turnOffScreen) } void HalDisplay::displayWindow(int x, int y, int w, int h) { + if (x < 0) x = 0; + if (y < 0) y = 0; + if (w <= 0 || h <= 0 || x >= DISPLAY_WIDTH || y >= DISPLAY_HEIGHT) return; + if (x + w > DISPLAY_WIDTH) w = DISPLAY_WIDTH - x; + if (y + h > DISPLAY_HEIGHT) h = DISPLAY_HEIGHT - y; einkDisplay.displayWindow(static_cast(x), static_cast(y), static_cast(w), static_cast(h)); } diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 737b5b992c..25a87d7449 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -40,6 +40,7 @@ void ClipSelectionActivity::onEnter() { return; } + savedSectionPage = section.currentPage; savedBufferSize = renderer.getBufferSize(); savedBuffer = static_cast(malloc(savedBufferSize)); if (!savedBuffer) { @@ -58,6 +59,7 @@ void ClipSelectionActivity::onEnter() { } void ClipSelectionActivity::onExit() { + section.currentPage = savedSectionPage; free(savedBuffer); savedBuffer = nullptr; Activity::onExit(); diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h index b86e69b2fd..4b4b63e4ab 100644 --- a/src/activities/reader/ClipSelectionActivity.h +++ b/src/activities/reader/ClipSelectionActivity.h @@ -47,6 +47,7 @@ class ClipSelectionActivity final : public Activity { uint8_t* savedBuffer = nullptr; size_t savedBufferSize = 0; int currentDisplayPage = 0; + int savedSectionPage = 0; int cursorIdx = 0; int startMarkIdx = -1; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index bd210f2fa3..67083d7268 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -456,23 +456,19 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; - startActivityForResult( - std::make_unique(renderer, mappedInput, std::move(words), epub->getTitle(), - epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, - *section, startPage, mTop, mLeft), - [this](const ActivityResult& result) { - if (!result.isCancelled) { - const auto& clip = std::get(result.data); - if (!clip.text.empty()) { - std::string clipChapterTitle; - const int clipTocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); - if (clipTocIdx >= 0) clipChapterTitle = epub->getTocItem(clipTocIdx).title; - ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), clipChapterTitle, - section ? section->currentPage + 1 : 0, clip.text); - } - } - requestUpdate(); - }); + startActivityForResult(std::make_unique( + renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), + chapterTitle, startPage + 1, readerFontId, *section, startPage, mTop, mLeft), + [this, chapterTitle, startPage](const ActivityResult& result) { + if (!result.isCancelled) { + const auto& clip = std::get(result.data); + if (!clip.text.empty()) { + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, + startPage + 1, clip.text); + } + } + requestUpdate(); + }); } else { requestUpdate(); } diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index a1a454993e..77b4349fe1 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -24,21 +24,31 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str snprintf(location, sizeof(location), "Page %d\n", pageNumber); } - // Body: quoted text, trimmed to 2000 chars to avoid writing huge pages + // Body: quoted text, trimmed to 2000 chars to avoid writing huge clippings static constexpr size_t MAX_TEXT = 2000; const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; static constexpr char quote[] = "\n\""; static constexpr char separator[] = "\"\n\n==========\n\n"; - file.write(header, strlen(header)); - file.write(location, strlen(location)); - file.write(quote, strlen(quote)); - file.write(selectedText.c_str(), textLen); - file.write(separator, strlen(separator)); + const size_t headerLen = strlen(header); + const size_t locationLen = strlen(location); + const size_t quoteLen = strlen(quote); + const size_t separatorLen = strlen(separator); + + bool ok = file.write(header, headerLen) == headerLen; + ok = ok && file.write(location, locationLen) == locationLen; + ok = ok && file.write(quote, quoteLen) == quoteLen; + ok = ok && file.write(selectedText.c_str(), textLen) == textLen; + ok = ok && file.write(separator, separatorLen) == separatorLen; file.flush(); file.close(); + if (!ok) { + LOG_ERR("CLIP", "Failed to write clipping to %s (SD full or removed?)", CLIPPINGS_PATH); + return false; + } + LOG_DBG("CLIP", "Saved clipping to %s (%zu chars)", CLIPPINGS_PATH, textLen); return true; } diff --git a/src/clippings/ClippingsManager.h b/src/clippings/ClippingsManager.h index dbea0a7bf0..54715b1f8a 100644 --- a/src/clippings/ClippingsManager.h +++ b/src/clippings/ClippingsManager.h @@ -4,10 +4,10 @@ class ClippingsManager { public: - // Appends a clipping entry to /clippings.txt on the SD card. + // Appends a clipping entry to /My Clippings.txt on the SD card (Kindle-compatible filename). // Returns false if the SD write fails. static bool saveClipping(const std::string& bookTitle, const std::string& author, const std::string& chapterTitle, int pageNumber, const std::string& selectedText); - static constexpr const char* CLIPPINGS_PATH = "/clippings.txt"; + static constexpr const char* CLIPPINGS_PATH = "/My Clippings.txt"; }; From dda66635043fe83f9426208e5602bdd19a18df95 Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Tue, 28 Apr 2026 17:37:45 +0300 Subject: [PATCH 04/19] feat: enhance logging for mutex operations in HalStorage and ActivityManager --- lib/hal/HalStorage.cpp | 15 ++++++-- src/activities/ActivityManager.cpp | 12 +++++++ .../reader/ClipSelectionActivity.cpp | 36 ++++++++----------- src/activities/reader/EpubReaderActivity.cpp | 8 +++-- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/lib/hal/HalStorage.cpp b/lib/hal/HalStorage.cpp index 154e2c343a..6a4c96191c 100644 --- a/lib/hal/HalStorage.cpp +++ b/lib/hal/HalStorage.cpp @@ -4,6 +4,7 @@ #include // need to be included before SdFat.h for compatibility with FS.h's File class #include #include +#include #include @@ -26,8 +27,18 @@ bool HalStorage::ready() const { return SDCard.ready(); } class HalStorage::StorageLock { public: - StorageLock() { xSemaphoreTake(HalStorage::getInstance().storageMutex, portMAX_DELAY); } - ~StorageLock() { xSemaphoreGive(HalStorage::getInstance().storageMutex); } + StorageLock() { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "SL take from %s", pcTaskGetName(nullptr)); +#endif + xSemaphoreTake(HalStorage::getInstance().storageMutex, portMAX_DELAY); + } + ~StorageLock() { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "SL give from %s", pcTaskGetName(nullptr)); +#endif + xSemaphoreGive(HalStorage::getInstance().storageMutex); + } }; #define HAL_STORAGE_WRAPPED_CALL(method, ...) \ diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 96c3c7d0a4..dd3e4e89fb 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -291,17 +291,26 @@ void ActivityManager::requestUpdateAndWait() { // RenderLock RenderLock::RenderLock() { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "RL take from %s", pcTaskGetName(nullptr)); +#endif xSemaphoreTake(activityManager.renderingMutex, portMAX_DELAY); isLocked = true; } RenderLock::RenderLock([[maybe_unused]] Activity&) { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "RL take from %s", pcTaskGetName(nullptr)); +#endif xSemaphoreTake(activityManager.renderingMutex, portMAX_DELAY); isLocked = true; } RenderLock::~RenderLock() { if (isLocked) { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "RL give from %s", pcTaskGetName(nullptr)); +#endif xSemaphoreGive(activityManager.renderingMutex); isLocked = false; } @@ -309,6 +318,9 @@ RenderLock::~RenderLock() { void RenderLock::unlock() { if (isLocked) { +#if LOG_LEVEL >= 2 + LOG_DBG("LOCK", "RL unlock from %s", pcTaskGetName(nullptr)); +#endif xSemaphoreGive(activityManager.renderingMutex); isLocked = false; } diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 25a87d7449..3ee8b3eeef 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -69,29 +69,35 @@ void ClipSelectionActivity::loop() { const int total = static_cast(words.size()); buttonNavigator.onNextRelease([this, total] { + if (cursorIdx + 1 >= total) return; const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = ButtonNavigator::nextIndex(cursorIdx, total); + cursorIdx = cursorIdx + 1; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); buttonNavigator.onNextContinuous([this] { const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = lineEndForward(cursorIdx); + const int next = lineEndForward(cursorIdx); + if (next == cursorIdx) return; + cursorIdx = next; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); - buttonNavigator.onPreviousRelease([this, total] { + buttonNavigator.onPreviousRelease([this] { + if (cursorIdx == 0) return; const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = ButtonNavigator::previousIndex(cursorIdx, total); + cursorIdx = cursorIdx - 1; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); buttonNavigator.onPreviousContinuous([this] { const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = lineEndBackward(cursorIdx); + const int prev = lineEndBackward(cursorIdx); + if (prev == cursorIdx) return; + cursorIdx = prev; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); @@ -203,15 +209,9 @@ int ClipSelectionActivity::lineEndForward(int idx) const { last = i; } - // Already at line end — jump to end of next line + // Already at line end — jump to first word of next line if (last == idx && idx + 1 < total) { - const int nextY = words[idx + 1].y; - const int nextPage = words[idx + 1].pageIdx; - last = idx + 1; - for (int i = idx + 2; i < total; ++i) { - if (words[i].pageIdx != nextPage || words[i].y != nextY) break; - last = i; - } + return idx + 1; } return last; @@ -228,15 +228,9 @@ int ClipSelectionActivity::lineEndBackward(int idx) const { first = i; } - // Already at line start — jump to start of previous line + // Already at line start — jump to last word of previous line if (first == idx && idx - 1 >= 0) { - const int prevY = words[idx - 1].y; - const int prevPage = words[idx - 1].pageIdx; - first = idx - 1; - for (int i = idx - 2; i >= 0; --i) { - if (words[i].pageIdx != prevPage || words[i].y != prevY) break; - first = i; - } + return idx - 1; } return first; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 67083d7268..f7f739d132 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -440,9 +441,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction for (int i = 0; i < static_cast(wlist.size()); ++i) { const int wx = mLeft + line.xPos + xpos[i]; const int wy = mTop + line.yPos; - const int ww = (i + 1 < static_cast(xpos.size())) - ? static_cast(xpos[i + 1]) - static_cast(xpos[i]) - : renderer.getTextWidth(readerFontId, wlist[i].c_str()); + const int ww = renderer.getTextWidth(readerFontId, wlist[i].c_str()); if (ww > 0) { words.push_back({wx, wy, ww, lineH, pi, wlist[i]}); } @@ -755,6 +754,9 @@ void EpubReaderActivity::render(RenderLock&& lock) { } silentIndexNextChapterIfNeeded(viewportWidth, viewportHeight); saveProgress(currentSpineIndex, section->currentPage, section->pageCount); +#if LOG_LEVEL >= 2 + LOG_DBG("ERS", "render stack hwm=%u", uxTaskGetStackHighWaterMark(nullptr)); +#endif if (pendingScreenshot) { pendingScreenshot = false; From 5ab19727ace48a7b8f98e30cc8da52009d1f9b54 Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Tue, 28 Apr 2026 17:55:15 +0300 Subject: [PATCH 05/19] fix: update clipping format to match Kindle-compatible standards --- src/clippings/ClippingsManager.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index 77b4349fe1..aa018cc334 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -12,24 +12,24 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str return false; } - // Header line: "Title / Author" + // Header line: "Title (Author)" — Kindle-compatible format char header[128]; - snprintf(header, sizeof(header), "%s / %s\n", bookTitle.c_str(), author.c_str()); + snprintf(header, sizeof(header), "%s (%s)\n", bookTitle.c_str(), author.c_str()); - // Location line: "Chapter: X | Page N" + // Location line: "- Highlight on Page N | Chapter X" — Kindle-compatible format char location[128]; if (!chapterTitle.empty()) { - snprintf(location, sizeof(location), "Chapter: %s | Page %d\n", chapterTitle.c_str(), pageNumber); + snprintf(location, sizeof(location), "- Highlight on Page %d | %s\n", pageNumber, chapterTitle.c_str()); } else { - snprintf(location, sizeof(location), "Page %d\n", pageNumber); + snprintf(location, sizeof(location), "- Highlight on Page %d\n", pageNumber); } - // Body: quoted text, trimmed to 2000 chars to avoid writing huge clippings + // Body: text trimmed to 2000 chars to avoid writing huge clippings static constexpr size_t MAX_TEXT = 2000; const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; - static constexpr char quote[] = "\n\""; - static constexpr char separator[] = "\"\n\n==========\n\n"; + static constexpr char quote[] = "\n"; + static constexpr char separator[] = "\n==========\n"; const size_t headerLen = strlen(header); const size_t locationLen = strlen(location); From cc149fabb8c31f5b9c0352da9c2155eff1fab2fd Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Tue, 28 Apr 2026 18:15:04 +0300 Subject: [PATCH 06/19] fix: improve error logging and revert page on load failure in ClipSelectionActivity --- src/activities/reader/ClipSelectionActivity.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 3ee8b3eeef..139242ff57 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -154,10 +154,13 @@ void ClipSelectionActivity::render(RenderLock&&) { } void ClipSelectionActivity::switchToPage(int pageIdx) { + const int oldPage = section.currentPage; section.currentPage = startPageInSection + pageIdx; auto page = section.loadPageFromSectionFile(); if (!page) { - LOG_ERR("CLIP", "Failed to load page %d for display", pageIdx); + section.currentPage = oldPage; + LOG_ERR("CLIP", "Failed to load page %d (section.currentPage=%d, currentDisplayPage=%d) — reverted", + pageIdx, section.currentPage, currentDisplayPage); return; } From ed5366146744b78f6048b27a14b2166712098ec8 Mon Sep 17 00:00:00 2001 From: Pavl Zubenko Date: Tue, 28 Apr 2026 18:20:11 +0300 Subject: [PATCH 07/19] fix: use canonical Kindle 'Your Highlight' phrasing and apply clang-format --- src/activities/reader/ClipSelectionActivity.cpp | 4 ++-- src/clippings/ClippingsManager.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 139242ff57..414e5756af 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -159,8 +159,8 @@ void ClipSelectionActivity::switchToPage(int pageIdx) { auto page = section.loadPageFromSectionFile(); if (!page) { section.currentPage = oldPage; - LOG_ERR("CLIP", "Failed to load page %d (section.currentPage=%d, currentDisplayPage=%d) — reverted", - pageIdx, section.currentPage, currentDisplayPage); + LOG_ERR("CLIP", "Failed to load page %d (section.currentPage=%d, currentDisplayPage=%d) — reverted", pageIdx, + section.currentPage, currentDisplayPage); return; } diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index aa018cc334..ffe2d31b89 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -16,12 +16,12 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str char header[128]; snprintf(header, sizeof(header), "%s (%s)\n", bookTitle.c_str(), author.c_str()); - // Location line: "- Highlight on Page N | Chapter X" — Kindle-compatible format + // Location line: "- Your Highlight on Page N | Chapter X" — Kindle-compatible format char location[128]; if (!chapterTitle.empty()) { - snprintf(location, sizeof(location), "- Highlight on Page %d | %s\n", pageNumber, chapterTitle.c_str()); + snprintf(location, sizeof(location), "- Your Highlight on Page %d | %s\n", pageNumber, chapterTitle.c_str()); } else { - snprintf(location, sizeof(location), "- Highlight on Page %d\n", pageNumber); + snprintf(location, sizeof(location), "- Your Highlight on Page %d\n", pageNumber); } // Body: text trimmed to 2000 chars to avoid writing huge clippings From 01ab20890df15b2945a0b6f8a67700fe86056d0f Mon Sep 17 00:00:00 2001 From: Pavlo Zubenko Date: Tue, 28 Apr 2026 18:25:43 +0300 Subject: [PATCH 08/19] fix: use std::string for header/location and single-buffer write in ClippingsManager --- src/clippings/ClippingsManager.cpp | 46 +++++++++++++++++------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index ffe2d31b89..b9aeb414a7 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -12,35 +12,41 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str return false; } - // Header line: "Title (Author)" — Kindle-compatible format - char header[128]; - snprintf(header, sizeof(header), "%s (%s)\n", bookTitle.c_str(), author.c_str()); - - // Location line: "- Your Highlight on Page N | Chapter X" — Kindle-compatible format - char location[128]; + // Build header and location as strings to avoid truncation of long titles/authors + const std::string header = bookTitle + " (" + author + ")\n"; + std::string location = "- Your Highlight on Page " + std::to_string(pageNumber); if (!chapterTitle.empty()) { - snprintf(location, sizeof(location), "- Your Highlight on Page %d | %s\n", pageNumber, chapterTitle.c_str()); - } else { - snprintf(location, sizeof(location), "- Your Highlight on Page %d\n", pageNumber); + location += " | " + chapterTitle; } + location += "\n"; - // Body: text trimmed to 2000 chars to avoid writing huge clippings static constexpr size_t MAX_TEXT = 2000; const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; - static constexpr char quote[] = "\n"; + // Concatenate into a single buffer and perform one write to avoid partial records on SD failure static constexpr char separator[] = "\n==========\n"; + static constexpr size_t separatorLen = sizeof(separator) - 1; + const size_t totalLen = header.size() + location.size() + 1 + textLen + separatorLen; - const size_t headerLen = strlen(header); - const size_t locationLen = strlen(location); - const size_t quoteLen = strlen(quote); - const size_t separatorLen = strlen(separator); + auto* buf = static_cast(malloc(totalLen)); + if (!buf) { + LOG_ERR("CLIP", "malloc failed: %zu bytes", totalLen); + file.close(); + return false; + } - bool ok = file.write(header, headerLen) == headerLen; - ok = ok && file.write(location, locationLen) == locationLen; - ok = ok && file.write(quote, quoteLen) == quoteLen; - ok = ok && file.write(selectedText.c_str(), textLen) == textLen; - ok = ok && file.write(separator, separatorLen) == separatorLen; + size_t pos = 0; + memcpy(buf + pos, header.c_str(), header.size()); + pos += header.size(); + memcpy(buf + pos, location.c_str(), location.size()); + pos += location.size(); + buf[pos++] = '\n'; + memcpy(buf + pos, selectedText.c_str(), textLen); + pos += textLen; + memcpy(buf + pos, separator, separatorLen); + + const bool ok = file.write(buf, totalLen) == totalLen; + free(buf); file.flush(); file.close(); From 128e71b134cd89898d164a5bf4f82cc065d0200e Mon Sep 17 00:00:00 2001 From: Pavlo Zubenko Date: Wed, 29 Apr 2026 11:31:30 +0300 Subject: [PATCH 09/19] feat: add clipping refinements and annotation underlines - Rename menu item to "Create Clipping" and move after Screenshot - Show "Done" button label after first word selection in ClipSelector - Add Clippings settings group (Storage, Navigation, Show Annotations, Delete Mode) under Reader category - ClippingsManager: per-book storage in /clippings/.txt with dynamic path resolution from settings - ClipSelectionActivity: Word-by-word vs Line-aware navigation modes - AnnotationsManager: binary cache (annotations.bin) storing pixel rects + text preview per annotation record - Draw underlines on annotated text during reading (filtered by section page to avoid rendering rects on wrong pages) - Long press Confirm in reader (700ms) shows annotation overlay with file path and Delete/Cancel when annotations exist on current page - Permanent delete streams through clipping file without full RAM read - Annotation only delete removes metadata without touching the txt file --- lib/I18n/translations/english.yaml | 13 +- src/CrossPointSettings.h | 13 ++ src/SettingsList.h | 49 ++--- src/activities/ActivityResult.h | 2 + .../reader/ClipSelectionActivity.cpp | 41 ++-- src/activities/reader/EpubReaderActivity.cpp | 103 ++++++++- src/activities/reader/EpubReaderActivity.h | 11 + .../reader/EpubReaderMenuActivity.cpp | 2 +- src/annotations/AnnotationsManager.cpp | 206 ++++++++++++++++++ src/annotations/AnnotationsManager.h | 48 ++++ src/clippings/ClippingsManager.cpp | 44 +++- src/clippings/ClippingsManager.h | 8 +- 12 files changed, 477 insertions(+), 63 deletions(-) create mode 100644 src/annotations/AnnotationsManager.cpp create mode 100644 src/annotations/AnnotationsManager.h diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index 9062e90257..6fd471a0bf 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -262,7 +262,18 @@ STR_GO_TO_PERCENT: "Go to %" STR_GO_HOME_BUTTON: "Go Home" STR_SYNC_PROGRESS: "Sync Progress" STR_DELETE_CACHE: "Delete Book Cache" -STR_SAVE_CLIPPING: "Save Clipping" +STR_SAVE_CLIPPING: "Create Clipping" +STR_CAT_CLIPPINGS: "Clippings" +STR_CLIPPING_STORAGE: "Storage" +STR_CLIPPING_SINGLE_FILE: "Single File" +STR_CLIPPING_PER_BOOK: "Per Book" +STR_CLIP_NAV_MODE: "Navigation" +STR_CLIP_NAV_LINE: "Line Aware" +STR_CLIP_NAV_WORD: "Word by Word" +STR_ANNOT_SHOW: "Show Annotations" +STR_CLIPPING_DELETE_MODE: "Delete Mode" +STR_DELETE_PERMANENT: "Permanent" +STR_DELETE_META_ONLY: "Annotation Only" STR_DELETE: "Delete" STR_DISPLAY_QR: "Show page as QR" STR_CHAPTER_PREFIX: "Chapter: " diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index 01d7cc5618..a0256e1af6 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -146,6 +146,14 @@ class CrossPointSettings { enum IMAGE_RENDERING { IMAGES_DISPLAY = 0, IMAGES_PLACEHOLDER = 1, IMAGES_SUPPRESS = 2, IMAGE_RENDERING_COUNT }; enum TILT_PAGE_TURN { TILT_OFF = 0, TILT_NORMAL = 1, TILT_NVERTED = 2, TILT_PAGE_TURN_COUNT }; + // Clipping storage mode + enum CLIPPING_STORAGE : uint8_t { SINGLE_FILE = 0, PER_BOOK = 1, CLIPPING_STORAGE_COUNT }; + // Clip selector navigation scheme + enum CLIP_NAV_MODE : uint8_t { LINE_AWARE = 0, WORD_BY_WORD = 1, CLIP_NAV_MODE_COUNT }; + // Annotation underline visibility + enum ANNOTATION_VISIBILITY : uint8_t { ANNOT_VISIBLE = 0, ANNOT_HIDDEN = 1, ANNOTATION_VISIBILITY_COUNT }; + // Clipping delete behaviour + enum CLIPPING_DELETE_MODE : uint8_t { DELETE_PERMANENT = 0, DELETE_META_ONLY = 1, CLIPPING_DELETE_MODE_COUNT }; // Sleep screen settings uint8_t sleepScreen = DARK; @@ -213,6 +221,11 @@ class CrossPointSettings { uint8_t tiltPageTurn = TILT_OFF; // Language setting (Language enum index, default 0 = EN) uint8_t language = 0; + // Clippings settings + uint8_t clippingStorage = SINGLE_FILE; + uint8_t clipNavMode = LINE_AWARE; + uint8_t annotationVisibility = ANNOT_VISIBLE; + uint8_t clippingDeleteMode = DELETE_PERMANENT; ~CrossPointSettings() = default; diff --git a/src/SettingsList.h b/src/SettingsList.h index bbe9a77458..62b256e8d9 100644 --- a/src/SettingsList.h +++ b/src/SettingsList.h @@ -85,43 +85,20 @@ inline const std::vector<SettingInfo>& getSettingsList() { SettingInfo::Toggle(StrId::STR_SHOW_HIDDEN_FILES, &CrossPointSettings::showHiddenFiles, "showHiddenFiles", StrId::STR_CAT_SYSTEM), + // --- Clippings --- + SettingInfo::Enum(StrId::STR_CLIPPING_STORAGE, &CrossPointSettings::clippingStorage, + {StrId::STR_CLIPPING_SINGLE_FILE, StrId::STR_CLIPPING_PER_BOOK}, "clippingStorage", + StrId::STR_CAT_CLIPPINGS), + SettingInfo::Enum(StrId::STR_CLIP_NAV_MODE, &CrossPointSettings::clipNavMode, + {StrId::STR_CLIP_NAV_LINE, StrId::STR_CLIP_NAV_WORD}, "clipNavMode", + StrId::STR_CAT_CLIPPINGS), + SettingInfo::Toggle(StrId::STR_ANNOT_SHOW, &CrossPointSettings::annotationVisibility, "annotationVisibility", + StrId::STR_CAT_CLIPPINGS), + SettingInfo::Enum(StrId::STR_CLIPPING_DELETE_MODE, &CrossPointSettings::clippingDeleteMode, + {StrId::STR_DELETE_PERMANENT, StrId::STR_DELETE_META_ONLY}, "clippingDeleteMode", + StrId::STR_CAT_CLIPPINGS), + // --- KOReader Sync (web-only, uses KOReaderCredentialStore) --- - SettingInfo::DynamicString( - StrId::STR_KOREADER_USERNAME, [] { return KOREADER_STORE.getUsername(); }, - [](const std::string& v) { - KOREADER_STORE.setCredentials(v, KOREADER_STORE.getPassword()); - KOREADER_STORE.saveToFile(); - }, - "koUsername", StrId::STR_KOREADER_SYNC), - SettingInfo::DynamicString( - StrId::STR_KOREADER_PASSWORD, [] { return KOREADER_STORE.getPassword(); }, - [](const std::string& v) { - KOREADER_STORE.setCredentials(KOREADER_STORE.getUsername(), v); - KOREADER_STORE.saveToFile(); - }, - "koPassword", StrId::STR_KOREADER_SYNC), - SettingInfo::DynamicString( - StrId::STR_SYNC_SERVER_URL, [] { return KOREADER_STORE.getServerUrl(); }, - [](const std::string& v) { - KOREADER_STORE.setServerUrl(v); - KOREADER_STORE.saveToFile(); - }, - "koServerUrl", StrId::STR_KOREADER_SYNC), - SettingInfo::DynamicEnum( - StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, - [] { return static_cast<uint8_t>(KOREADER_STORE.getMatchMethod()); }, - [](uint8_t v) { - KOREADER_STORE.setMatchMethod(static_cast<DocumentMatchMethod>(v)); - KOREADER_STORE.saveToFile(); - }, - "koMatchMethod", StrId::STR_KOREADER_SYNC), - // --- Status Bar Settings (web-only, uses StatusBarSettingsActivity) --- - SettingInfo::Toggle(StrId::STR_CHAPTER_PAGE_COUNT, &CrossPointSettings::statusBarChapterPageCount, - "statusBarChapterPageCount", StrId::STR_CUSTOMISE_STATUS_BAR), - SettingInfo::Toggle(StrId::STR_BOOK_PROGRESS_PERCENTAGE, &CrossPointSettings::statusBarBookProgressPercentage, - "statusBarBookProgressPercentage", StrId::STR_CUSTOMISE_STATUS_BAR), - SettingInfo::Enum(StrId::STR_PROGRESS_BAR, &CrossPointSettings::statusBarProgressBar, - {StrId::STR_BOOK, StrId::STR_CHAPTER, StrId::STR_HIDE}, "statusBarProgressBar", StrId::STR_CUSTOMISE_STATUS_BAR), SettingInfo::Enum(StrId::STR_PROGRESS_BAR_THICKNESS, &CrossPointSettings::statusBarProgressBarThickness, {StrId::STR_PROGRESS_BAR_THIN, StrId::STR_PROGRESS_BAR_MEDIUM, StrId::STR_PROGRESS_BAR_THICK}, diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index 5ce9271edc..748d52970c 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -56,6 +56,8 @@ struct FilePathResult { struct ClippingResult { std::string text; + int fromWordIdx = -1; + int toWordIdx = -1; }; using ResultVariant = std::variant<std::monostate, WifiResult, KeyboardResult, MenuResult, ChapterResult, PercentResult, diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 414e5756af..d9bb62ff99 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -76,14 +76,16 @@ void ClipSelectionActivity::loop() { requestUpdate(); }); - buttonNavigator.onNextContinuous([this] { - const int prevPage = words[cursorIdx].pageIdx; - const int next = lineEndForward(cursorIdx); - if (next == cursorIdx) return; - cursorIdx = next; - if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; - requestUpdate(); - }); + if (SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE) { + buttonNavigator.onNextContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + const int next = lineEndForward(cursorIdx); + if (next == cursorIdx) return; + cursorIdx = next; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + } buttonNavigator.onPreviousRelease([this] { if (cursorIdx == 0) return; @@ -93,14 +95,16 @@ void ClipSelectionActivity::loop() { requestUpdate(); }); - buttonNavigator.onPreviousContinuous([this] { - const int prevPage = words[cursorIdx].pageIdx; - const int prev = lineEndBackward(cursorIdx); - if (prev == cursorIdx) return; - cursorIdx = prev; - if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; - requestUpdate(); - }); + if (SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE) { + buttonNavigator.onPreviousContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + const int prev = lineEndBackward(cursorIdx); + if (prev == cursorIdx) return; + cursorIdx = prev; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + } if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { if (startMarkIdx == -1) { @@ -115,7 +119,7 @@ void ClipSelectionActivity::loop() { text += words[i].text; } ActivityResult result; - result.data = ClippingResult{std::move(text)}; + result.data = ClippingResult{std::move(text), from, to}; setResult(std::move(result)); finish(); } @@ -147,7 +151,8 @@ void ClipSelectionActivity::render(RenderLock&&) { memcpy(renderer.getFrameBuffer(), savedBuffer, savedBufferSize); drawHighlights(); - const auto labels = mappedInput.mapLabels(tr(STR_BACK), tr(STR_SELECT), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); + const auto confirmLabel = startMarkIdx == -1 ? tr(STR_SELECT) : tr(STR_DONE); + const auto labels = mappedInput.mapLabels(tr(STR_BACK), confirmLabel, tr(STR_DIR_UP), tr(STR_DIR_DOWN)); GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); renderer.displayBuffer(HalDisplay::FAST_REFRESH); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index f7f739d132..11b8052eb3 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -60,6 +60,7 @@ void EpubReaderActivity::onEnter() { ReaderUtils::applyOrientation(renderer, SETTINGS.orientation); epub->setupCacheDir(); + annotations.load(epub->getCachePath().c_str()); FsFile f; if (Storage.openFileForRead("ERS", epub->getCachePath() + "/progress.bin", f)) { @@ -104,6 +105,11 @@ void EpubReaderActivity::onEnter() { void EpubReaderActivity::onExit() { Activity::onExit(); + if (epub && annotationsDirty) { + annotations.save(epub->getCachePath().c_str()); + annotationsDirty = false; + } + // Reset orientation back to portrait for the rest of the UI renderer.setOrientation(GfxRenderer::Orientation::Portrait); @@ -146,6 +152,44 @@ void EpubReaderActivity::loop() { } } + // Annotation overlay: handle Back (close) and Confirm (delete) when overlay is visible + if (showAnnotationOverlay) { + if (mappedInput.wasReleased(MappedInputManager::Button::Back)) { + showAnnotationOverlay = false; + requestUpdate(); + } else if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { + if (overlayAnnotationIdx < annotations.size() && epub) { + const std::string clippingPath = ClippingsManager::resolveClippingPath(epub->getTitle()); + if (SETTINGS.clippingDeleteMode == CrossPointSettings::DELETE_PERMANENT) { + annotations.permanentDelete(overlayAnnotationIdx, clippingPath.c_str()); + } else { + annotations.removeMeta(overlayAnnotationIdx); + annotations.save(epub->getCachePath().c_str()); + } + } + showAnnotationOverlay = false; + annotationLongPressConsumed = true; + requestUpdate(); + } + return; + } + + // Long press Confirm: show annotation overlay if annotations exist on current page + if (mappedInput.isPressed(MappedInputManager::Button::Confirm) && + mappedInput.getHeldTime() >= ANNOTATION_HOLD_MS && !annotationLongPressConsumed) { + const size_t annotIdx = annotations.firstIndexForSection(static_cast<uint16_t>(currentSpineIndex)); + if (annotIdx != SIZE_MAX) { + overlayAnnotationIdx = annotIdx; + showAnnotationOverlay = true; + annotationLongPressConsumed = true; + requestUpdate(); + return; + } + } + if (!mappedInput.isPressed(MappedInputManager::Button::Confirm)) { + annotationLongPressConsumed = false; + } + // Enter reader menu activity. if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { const int currentPage = section ? section->currentPage + 1 : 0; @@ -455,15 +499,35 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; + // Capture words by value before moving into ClipSelectionActivity + auto wordsCopy = words; startActivityForResult(std::make_unique<ClipSelectionActivity>( renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, *section, startPage, mTop, mLeft), - [this, chapterTitle, startPage](const ActivityResult& result) { + [this, chapterTitle, startPage, + wordsCopy = std::move(wordsCopy)](const ActivityResult& result) mutable { if (!result.isCancelled) { const auto& clip = std::get<ClippingResult>(result.data); if (!clip.text.empty()) { ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, clip.text); + // Build annotation record from selected word pixel positions + if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0 && epub) { + AnnotationsManager::AnnotationRecord rec; + rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); + const int to = std::min(clip.toWordIdx, static_cast<int>(wordsCopy.size()) - 1); + for (int i = clip.fromWordIdx; i <= to; ++i) { + const auto absPage = static_cast<uint16_t>(startPage + wordsCopy[i].pageIdx); + rec.rects.push_back( + {static_cast<int16_t>(wordsCopy[i].x), static_cast<int16_t>(wordsCopy[i].y), + static_cast<int16_t>(wordsCopy[i].w), static_cast<int16_t>(wordsCopy[i].h), + absPage}); + } + const size_t previewLen = clip.text.size() < 100 ? clip.text.size() : 100; + rec.textPreview = clip.text.substr(0, previewLen); + annotations.add(std::move(rec)); + annotations.save(epub->getCachePath().c_str()); + } } } requestUpdate(); @@ -835,7 +899,44 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or bool imagePageWithAA = page->hasImages() && SETTINGS.textAntiAliasing; page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); + + // Draw annotation underlines on top of rendered text (only for current section page) + if (SETTINGS.annotationVisibility == CrossPointSettings::ANNOT_VISIBLE && section) { + const auto currentSectionPage = static_cast<uint16_t>(section->currentPage); + const auto sectionAnnotations = annotations.forSection(static_cast<uint16_t>(currentSpineIndex)); + for (const auto& rec : sectionAnnotations) { + for (const auto& r : rec.rects) { + if (r.sectionPage != currentSectionPage) continue; + const int underlineY = r.y + r.h - 1; + renderer.drawLine(r.x, underlineY, r.x + r.w, underlineY, 2, true); + } + } + } + renderStatusBar(); + + // Draw annotation management overlay on top of page + status bar + if (showAnnotationOverlay && epub) { + const int sw = renderer.getScreenWidth(); + const int sh = renderer.getScreenHeight(); + const int boxW = sw - 60; + const int boxH = 70; + const int boxX = 30; + const int boxY = (sh - boxH) / 2; + + renderer.fillRect(boxX, boxY, boxW, boxH, false); + renderer.drawRect(boxX, boxY, boxW, boxH, 2, true); + + const std::string clippingPath = ClippingsManager::resolveClippingPath(epub->getTitle()); + char pathBuf[80]; + snprintf(pathBuf, sizeof(pathBuf), "%s", clippingPath.c_str()); + renderer.drawText(UI_10_FONT_ID, boxX + 10, boxY + 6, tr(STR_SAVE_CLIPPING), true); + renderer.drawText(UI_10_FONT_ID, boxX + 10, boxY + 28, pathBuf, true); + + const auto labels = mappedInput.mapLabels(tr(STR_CANCEL), tr(STR_DELETE), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); + GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); + } + fcm->logStats("bw_render"); const auto tBwRender = millis(); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 76a6df2603..bc10e1a35a 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -7,6 +7,7 @@ #include "EpubReaderMenuActivity.h" #include "activities/Activity.h" +#include "annotations/AnnotationsManager.h" class EpubReaderActivity final : public Activity { std::shared_ptr<Epub> epub; @@ -31,6 +32,16 @@ class EpubReaderActivity final : public Activity { bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit bool automaticPageTurnActive = false; + AnnotationsManager annotations; + bool annotationsDirty = false; + + // Annotation management overlay state + bool showAnnotationOverlay = false; + size_t overlayAnnotationIdx = 0; + bool annotationLongPressConsumed = false; + + static constexpr unsigned long ANNOTATION_HOLD_MS = 700; + // Footnote support std::vector<FootnoteEntry> currentPageFootnotes; struct SavedPosition { diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 1dc329973b..ec1471d2c3 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -30,11 +30,11 @@ std::vector<EpubReaderMenuActivity::MenuItem> EpubReaderMenuActivity::buildMenuI items.push_back({MenuAction::AUTO_PAGE_TURN, StrId::STR_AUTO_TURN_PAGES_PER_MIN}); items.push_back({MenuAction::GO_TO_PERCENT, StrId::STR_GO_TO_PERCENT}); items.push_back({MenuAction::SCREENSHOT, StrId::STR_SCREENSHOT_BUTTON}); + items.push_back({MenuAction::SAVE_CLIPPING, StrId::STR_SAVE_CLIPPING}); items.push_back({MenuAction::DISPLAY_QR, StrId::STR_DISPLAY_QR}); items.push_back({MenuAction::GO_HOME, StrId::STR_GO_HOME_BUTTON}); items.push_back({MenuAction::SYNC, StrId::STR_SYNC_PROGRESS}); items.push_back({MenuAction::DELETE_CACHE, StrId::STR_DELETE_CACHE}); - items.push_back({MenuAction::SAVE_CLIPPING, StrId::STR_SAVE_CLIPPING}); return items; } diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp new file mode 100644 index 0000000000..dc28f7d3d0 --- /dev/null +++ b/src/annotations/AnnotationsManager.cpp @@ -0,0 +1,206 @@ +#include "AnnotationsManager.h" + +#include <HalStorage.h> +#include <Logging.h> +#include <common/FsApiConstants.h> + +#include <algorithm> +#include <cstring> + +static constexpr const char* ANNOT_FILENAME = "/annotations.bin"; + +std::string AnnotationsManager::annotationsPath(const char* bookCachePath) { + return std::string(bookCachePath) + ANNOT_FILENAME; +} + +bool AnnotationsManager::load(const char* bookCachePath) { + records.clear(); + const std::string path = annotationsPath(bookCachePath); + + HalFile file; + if (!Storage.openFileForRead("ANNOT", path.c_str(), file)) { + return true; // No annotations file yet — not an error + } + + uint8_t version = 0; + uint16_t count = 0; + if (file.read(&version, 1) != 1 || version != FILE_VERSION) { + file.close(); + return true; // Unknown version — treat as empty + } + if (file.read(&count, sizeof(count)) != sizeof(count)) { + file.close(); + return false; + } + + records.reserve(count); + for (uint16_t i = 0; i < count; ++i) { + AnnotationRecord rec; + if (file.read(&rec.sectionIdx, sizeof(rec.sectionIdx)) != sizeof(rec.sectionIdx)) break; + + uint16_t rectCount = 0; + if (file.read(&rectCount, sizeof(rectCount)) != sizeof(rectCount)) break; + rec.rects.resize(rectCount); + for (uint16_t r = 0; r < rectCount; ++r) { + if (file.read(&rec.rects[r], sizeof(Rect)) != sizeof(Rect)) goto done; + } + + uint8_t previewLen = 0; + if (file.read(&previewLen, 1) != 1) break; + if (previewLen > 0) { + rec.textPreview.resize(previewLen); + if (file.read(rec.textPreview.data(), previewLen) != previewLen) break; + } + + records.push_back(std::move(rec)); + } + +done: + file.close(); + LOG_DBG("ANNOT", "Loaded %zu annotations from %s", records.size(), path.c_str()); + return true; +} + +bool AnnotationsManager::save(const char* bookCachePath) const { + const std::string path = annotationsPath(bookCachePath); + + HalFile file = Storage.open(path.c_str(), O_RDWR | O_CREAT | O_TRUNC); + if (!file) { + LOG_ERR("ANNOT", "Failed to open %s for write", path.c_str()); + return false; + } + + const uint8_t version = FILE_VERSION; + const uint16_t count = static_cast<uint16_t>(records.size()); + file.write(&version, 1); + file.write(&count, sizeof(count)); + + for (const auto& rec : records) { + file.write(&rec.sectionIdx, sizeof(rec.sectionIdx)); + const uint16_t rectCount = static_cast<uint16_t>(rec.rects.size()); + file.write(&rectCount, sizeof(rectCount)); + for (const auto& r : rec.rects) { + file.write(&r, sizeof(Rect)); + } + const uint8_t previewLen = static_cast<uint8_t>( + rec.textPreview.size() < MAX_PREVIEW_LEN ? rec.textPreview.size() : MAX_PREVIEW_LEN); + file.write(&previewLen, 1); + if (previewLen > 0) { + file.write(rec.textPreview.c_str(), previewLen); + } + } + + file.flush(); + file.close(); + LOG_DBG("ANNOT", "Saved %zu annotations to %s", records.size(), path.c_str()); + return true; +} + +void AnnotationsManager::add(AnnotationRecord record) { + records.push_back(std::move(record)); +} + +void AnnotationsManager::removeMeta(size_t idx) { + if (idx < records.size()) { + records.erase(records.begin() + static_cast<ptrdiff_t>(idx)); + } +} + +bool AnnotationsManager::permanentDelete(size_t idx, const char* clippingFilePath) { + if (idx >= records.size()) return false; + const std::string preview = records[idx].textPreview; + + // Streaming copy of clipping file, skipping the matching entry + // Each entry in the Kindle format ends with "\n==========\n" + static constexpr char SEPARATOR[] = "\n==========\n"; + static constexpr size_t SEP_LEN = sizeof(SEPARATOR) - 1; + + // Build temp path alongside the source file + const std::string tempPath = std::string(clippingFilePath) + ".tmp"; + + HalFile src = Storage.open(clippingFilePath, O_RDONLY); + if (!src) { + LOG_ERR("ANNOT", "Failed to open %s for reading", clippingFilePath); + removeMeta(idx); + return false; + } + + HalFile dst = Storage.open(tempPath.c_str(), O_RDWR | O_CREAT | O_TRUNC); + if (!dst) { + src.close(); + LOG_ERR("ANNOT", "Failed to open temp file %s", tempPath.c_str()); + return false; + } + + // Read file entry-by-entry using separator as delimiter + std::string entry; + entry.reserve(STREAM_BLOCK); + char buf[STREAM_BLOCK]; + bool skipped = false; + + // We accumulate until we find a separator, then decide to copy or skip + while (true) { + int32_t n = src.read(buf, sizeof(buf)); + if (n <= 0) break; + entry.append(buf, static_cast<size_t>(n)); + + // Process all complete entries in the accumulated buffer + size_t pos = 0; + while (true) { + size_t sepPos = entry.find(SEPARATOR, pos); + if (sepPos == std::string::npos) break; + + const std::string chunk = entry.substr(pos, sepPos + SEP_LEN - pos); + + // Check if this entry contains the preview text we want to delete + if (!skipped && !preview.empty() && chunk.find(preview) != std::string::npos) { + skipped = true; // Skip this entry + } else { + dst.write(chunk.c_str(), chunk.size()); + } + pos = sepPos + SEP_LEN; + } + entry = entry.substr(pos); // Keep leftover (incomplete entry) + } + + // Write any trailing content that doesn't end with a separator + if (!entry.empty()) { + dst.write(entry.c_str(), entry.size()); + } + + src.close(); + dst.flush(); + dst.close(); + + // Replace original with temp + Storage.remove(clippingFilePath); + Storage.rename(tempPath.c_str(), clippingFilePath); + + removeMeta(idx); + LOG_DBG("ANNOT", "Permanently deleted annotation %zu from %s (skipped=%d)", idx, clippingFilePath, skipped); + return true; +} + +std::vector<AnnotationsManager::AnnotationRecord> AnnotationsManager::forSection(uint16_t sectionIdx) const { + std::vector<AnnotationRecord> result; + for (const auto& rec : records) { + if (rec.sectionIdx == sectionIdx) { + result.push_back(rec); + } + } + return result; +} + +bool AnnotationsManager::hasAnnotationsForSection(uint16_t sectionIdx) const { + for (const auto& rec : records) { + if (rec.sectionIdx == sectionIdx) return true; + } + return false; +} + +size_t AnnotationsManager::firstIndexForSection(uint16_t sectionIdx) const { + for (size_t i = 0; i < records.size(); ++i) { + if (records[i].sectionIdx == sectionIdx) return i; + } + return SIZE_MAX; +} diff --git a/src/annotations/AnnotationsManager.h b/src/annotations/AnnotationsManager.h new file mode 100644 index 0000000000..95d5e99e5b --- /dev/null +++ b/src/annotations/AnnotationsManager.h @@ -0,0 +1,48 @@ +#pragma once + +#include <cstdint> +#include <string> +#include <vector> + +class AnnotationsManager { + public: + struct Rect { + int16_t x, y, w, h; + uint16_t sectionPage; // absolute page index within section + }; + + struct AnnotationRecord { + uint16_t sectionIdx; + std::vector<Rect> rects; // pixel rects for drawing underlines + std::string textPreview; // first 100 chars for matching on permanent delete + }; + + // Load annotations from .crosspoint/epub_<hash>/annotations.bin + bool load(const char* bookCachePath); + // Save current annotations to disk + bool save(const char* bookCachePath) const; + + void add(AnnotationRecord record); + + // Remove only the metadata (annotations.bin entry) — text file untouched + void removeMeta(size_t idx); + + // Remove metadata AND delete matching entry from the clipping txt file (streaming, no full-file RAM read) + bool permanentDelete(size_t idx, const char* clippingFilePath); + + std::vector<AnnotationRecord> forSection(uint16_t sectionIdx) const; + bool hasAnnotationsForSection(uint16_t sectionIdx) const; + // Returns global index of first annotation for section, or SIZE_MAX if none + size_t firstIndexForSection(uint16_t sectionIdx) const; + bool empty() const { return records.empty(); } + size_t size() const { return records.size(); } + + private: + static constexpr uint8_t FILE_VERSION = 2; + static constexpr size_t MAX_PREVIEW_LEN = 100; + static constexpr size_t STREAM_BLOCK = 512; + + std::vector<AnnotationRecord> records; + + static std::string annotationsPath(const char* bookCachePath); +}; diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index b9aeb414a7..c70b751e71 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -1,14 +1,50 @@ #include "ClippingsManager.h" +#include <CrossPointSettings.h> #include <HalStorage.h> #include <Logging.h> #include <common/FsApiConstants.h> +#include <algorithm> +#include <cctype> + +static std::string sanitizeForFilename(const std::string& title) { + std::string result; + result.reserve(title.size()); + for (unsigned char c : title) { + if (std::isalnum(c) || c == '-') { + result += static_cast<char>(c); + } else if (std::isspace(c)) { + result += '_'; + } + } + // Limit filename length to avoid SD card path issues + if (result.size() > 60) result.resize(60); + if (result.empty()) result = "Unknown"; + return result; +} + +std::string ClippingsManager::resolveClippingPath(const std::string& bookTitle) { + if (SETTINGS.clippingStorage == CrossPointSettings::PER_BOOK) { + return std::string(CLIPPINGS_DIR) + "/" + sanitizeForFilename(bookTitle) + ".txt"; + } + return CLIPPINGS_PATH; +} + bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::string& author, const std::string& chapterTitle, int pageNumber, const std::string& selectedText) { - HalFile file = Storage.open(CLIPPINGS_PATH, O_RDWR | O_CREAT | O_AT_END); + const std::string path = resolveClippingPath(bookTitle); + + if (SETTINGS.clippingStorage == CrossPointSettings::PER_BOOK) { + if (!Storage.mkdir(CLIPPINGS_DIR)) { + LOG_ERR("CLIP", "Failed to create %s directory", CLIPPINGS_DIR); + return false; + } + } + + HalFile file = Storage.open(path.c_str(), O_RDWR | O_CREAT | O_AT_END); if (!file) { - LOG_ERR("CLIP", "Failed to open %s for append", CLIPPINGS_PATH); + LOG_ERR("CLIP", "Failed to open %s for append", path.c_str()); return false; } @@ -51,10 +87,10 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str file.close(); if (!ok) { - LOG_ERR("CLIP", "Failed to write clipping to %s (SD full or removed?)", CLIPPINGS_PATH); + LOG_ERR("CLIP", "Failed to write clipping to %s (SD full or removed?)", path.c_str()); return false; } - LOG_DBG("CLIP", "Saved clipping to %s (%zu chars)", CLIPPINGS_PATH, textLen); + LOG_DBG("CLIP", "Saved clipping to %s (%zu chars)", path.c_str(), textLen); return true; } diff --git a/src/clippings/ClippingsManager.h b/src/clippings/ClippingsManager.h index 54715b1f8a..42d44d9bd8 100644 --- a/src/clippings/ClippingsManager.h +++ b/src/clippings/ClippingsManager.h @@ -4,10 +4,14 @@ class ClippingsManager { public: - // Appends a clipping entry to /My Clippings.txt on the SD card (Kindle-compatible filename). - // Returns false if the SD write fails. + // Appends a clipping entry to the configured clipping file. + // File path is resolved dynamically from SETTINGS (single file or per-book). static bool saveClipping(const std::string& bookTitle, const std::string& author, const std::string& chapterTitle, int pageNumber, const std::string& selectedText); + // Returns the clipping file path for the given book title based on current SETTINGS. + static std::string resolveClippingPath(const std::string& bookTitle); + static constexpr const char* CLIPPINGS_PATH = "/My Clippings.txt"; + static constexpr const char* CLIPPINGS_DIR = "/clippings"; }; From 1dc51f3a77f7119974fc1e0d5f9030d5c15bfc07 Mon Sep 17 00:00:00 2001 From: Pavlo Zubenko <pashazubenko@gmail.com> Date: Wed, 29 Apr 2026 11:50:30 +0300 Subject: [PATCH 10/19] refactor: remove unused annotation overlay logic from EpubReaderActivity fix: skip whitespace-only words in highlight rendering in ClipSelectionActivity --- .../reader/ClipSelectionActivity.cpp | 5 +- src/activities/reader/EpubReaderActivity.cpp | 73 ++----------------- src/activities/reader/EpubReaderActivity.h | 7 -- 3 files changed, 11 insertions(+), 74 deletions(-) diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index d9bb62ff99..2d6802ac6e 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -184,15 +184,16 @@ void ClipSelectionActivity::drawHighlights() { for (int i = from; i <= to; ++i) { if (i == cursorIdx) continue; if (words[i].pageIdx != currentDisplayPage) continue; + if (words[i].text.find_first_not_of(" \t") == std::string::npos) continue; const auto r = alignedRect(words[i].x, words[i].y, words[i].w, words[i].h); renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); } } - // Draw cursor highlight (always on top) + // Draw cursor highlight (always on top) — skip whitespace-only words const auto& cw = words[cursorIdx]; - if (cw.pageIdx == currentDisplayPage) { + if (cw.pageIdx == currentDisplayPage && cw.text.find_first_not_of(" \t") != std::string::npos) { const auto r = alignedRect(cw.x, cw.y, cw.w, cw.h); renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 11b8052eb3..031a39ef3c 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -152,44 +152,6 @@ void EpubReaderActivity::loop() { } } - // Annotation overlay: handle Back (close) and Confirm (delete) when overlay is visible - if (showAnnotationOverlay) { - if (mappedInput.wasReleased(MappedInputManager::Button::Back)) { - showAnnotationOverlay = false; - requestUpdate(); - } else if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - if (overlayAnnotationIdx < annotations.size() && epub) { - const std::string clippingPath = ClippingsManager::resolveClippingPath(epub->getTitle()); - if (SETTINGS.clippingDeleteMode == CrossPointSettings::DELETE_PERMANENT) { - annotations.permanentDelete(overlayAnnotationIdx, clippingPath.c_str()); - } else { - annotations.removeMeta(overlayAnnotationIdx); - annotations.save(epub->getCachePath().c_str()); - } - } - showAnnotationOverlay = false; - annotationLongPressConsumed = true; - requestUpdate(); - } - return; - } - - // Long press Confirm: show annotation overlay if annotations exist on current page - if (mappedInput.isPressed(MappedInputManager::Button::Confirm) && - mappedInput.getHeldTime() >= ANNOTATION_HOLD_MS && !annotationLongPressConsumed) { - const size_t annotIdx = annotations.firstIndexForSection(static_cast<uint16_t>(currentSpineIndex)); - if (annotIdx != SIZE_MAX) { - overlayAnnotationIdx = annotIdx; - showAnnotationOverlay = true; - annotationLongPressConsumed = true; - requestUpdate(); - return; - } - } - if (!mappedInput.isPressed(MappedInputManager::Button::Confirm)) { - annotationLongPressConsumed = false; - } - // Enter reader menu activity. if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { const int currentPage = section ? section->currentPage + 1 : 0; @@ -900,43 +862,24 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop); - // Draw annotation underlines on top of rendered text (only for current section page) + // Draw annotation underlines on top of rendered text if (SETTINGS.annotationVisibility == CrossPointSettings::ANNOT_VISIBLE && section) { - const auto currentSectionPage = static_cast<uint16_t>(section->currentPage); + const int screenH = renderer.getScreenHeight(); + const int screenW = renderer.getScreenWidth(); const auto sectionAnnotations = annotations.forSection(static_cast<uint16_t>(currentSpineIndex)); for (const auto& rec : sectionAnnotations) { for (const auto& r : rec.rects) { - if (r.sectionPage != currentSectionPage) continue; - const int underlineY = r.y + r.h - 1; - renderer.drawLine(r.x, underlineY, r.x + r.w, underlineY, 2, true); + if (r.sectionPage != static_cast<uint16_t>(section->currentPage)) continue; + const int underlineY = r.y + r.h; + if (underlineY < 0 || underlineY >= screenH) continue; + if (r.x < 0 || r.x >= screenW) continue; + renderer.drawLine(r.x, underlineY, r.x + r.w - 1, underlineY, 2, true); } } } renderStatusBar(); - // Draw annotation management overlay on top of page + status bar - if (showAnnotationOverlay && epub) { - const int sw = renderer.getScreenWidth(); - const int sh = renderer.getScreenHeight(); - const int boxW = sw - 60; - const int boxH = 70; - const int boxX = 30; - const int boxY = (sh - boxH) / 2; - - renderer.fillRect(boxX, boxY, boxW, boxH, false); - renderer.drawRect(boxX, boxY, boxW, boxH, 2, true); - - const std::string clippingPath = ClippingsManager::resolveClippingPath(epub->getTitle()); - char pathBuf[80]; - snprintf(pathBuf, sizeof(pathBuf), "%s", clippingPath.c_str()); - renderer.drawText(UI_10_FONT_ID, boxX + 10, boxY + 6, tr(STR_SAVE_CLIPPING), true); - renderer.drawText(UI_10_FONT_ID, boxX + 10, boxY + 28, pathBuf, true); - - const auto labels = mappedInput.mapLabels(tr(STR_CANCEL), tr(STR_DELETE), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); - GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); - } - fcm->logStats("bw_render"); const auto tBwRender = millis(); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index bc10e1a35a..9c57588c8e 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -35,13 +35,6 @@ class EpubReaderActivity final : public Activity { AnnotationsManager annotations; bool annotationsDirty = false; - // Annotation management overlay state - bool showAnnotationOverlay = false; - size_t overlayAnnotationIdx = 0; - bool annotationLongPressConsumed = false; - - static constexpr unsigned long ANNOTATION_HOLD_MS = 700; - // Footnote support std::vector<FootnoteEntry> currentPageFootnotes; struct SavedPosition { From 2513008e74ddc1f0d642b22475b4d49a7abdeda3 Mon Sep 17 00:00:00 2001 From: Pavlo Zubenko <pashazubenko@gmail.com> Date: Wed, 29 Apr 2026 15:19:34 +0300 Subject: [PATCH 11/19] feat: implement ClippingsMenuActivity and integrate into settings refactor: remove unused clipping delete mode and related logic fix: improve whitespace handling in highlight rendering --- lib/I18n/translations/english.yaml | 3 - src/CrossPointSettings.h | 5 - src/SettingsList.h | 3 - .../reader/ClipSelectionActivity.cpp | 13 +- .../reader/ClippingsMenuActivity.cpp | 118 ++++++++++++++++++ src/activities/reader/ClippingsMenuActivity.h | 24 ++++ src/activities/reader/EpubReaderActivity.cpp | 87 +++++++------ src/activities/settings/SettingsActivity.cpp | 31 ++++- src/activities/settings/SettingsActivity.h | 5 +- src/annotations/AnnotationsManager.cpp | 108 +--------------- src/annotations/AnnotationsManager.h | 15 +-- 11 files changed, 230 insertions(+), 182 deletions(-) create mode 100644 src/activities/reader/ClippingsMenuActivity.cpp create mode 100644 src/activities/reader/ClippingsMenuActivity.h diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index 6fd471a0bf..8417cbb81e 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -271,9 +271,6 @@ STR_CLIP_NAV_MODE: "Navigation" STR_CLIP_NAV_LINE: "Line Aware" STR_CLIP_NAV_WORD: "Word by Word" STR_ANNOT_SHOW: "Show Annotations" -STR_CLIPPING_DELETE_MODE: "Delete Mode" -STR_DELETE_PERMANENT: "Permanent" -STR_DELETE_META_ONLY: "Annotation Only" STR_DELETE: "Delete" STR_DISPLAY_QR: "Show page as QR" STR_CHAPTER_PREFIX: "Chapter: " diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index a0256e1af6..a7b757c009 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -152,9 +152,6 @@ class CrossPointSettings { enum CLIP_NAV_MODE : uint8_t { LINE_AWARE = 0, WORD_BY_WORD = 1, CLIP_NAV_MODE_COUNT }; // Annotation underline visibility enum ANNOTATION_VISIBILITY : uint8_t { ANNOT_VISIBLE = 0, ANNOT_HIDDEN = 1, ANNOTATION_VISIBILITY_COUNT }; - // Clipping delete behaviour - enum CLIPPING_DELETE_MODE : uint8_t { DELETE_PERMANENT = 0, DELETE_META_ONLY = 1, CLIPPING_DELETE_MODE_COUNT }; - // Sleep screen settings uint8_t sleepScreen = DARK; // Sleep screen cover mode settings @@ -225,8 +222,6 @@ class CrossPointSettings { uint8_t clippingStorage = SINGLE_FILE; uint8_t clipNavMode = LINE_AWARE; uint8_t annotationVisibility = ANNOT_VISIBLE; - uint8_t clippingDeleteMode = DELETE_PERMANENT; - ~CrossPointSettings() = default; // Get singleton instance diff --git a/src/SettingsList.h b/src/SettingsList.h index 62b256e8d9..5d466cd4b2 100644 --- a/src/SettingsList.h +++ b/src/SettingsList.h @@ -94,9 +94,6 @@ inline const std::vector<SettingInfo>& getSettingsList() { StrId::STR_CAT_CLIPPINGS), SettingInfo::Toggle(StrId::STR_ANNOT_SHOW, &CrossPointSettings::annotationVisibility, "annotationVisibility", StrId::STR_CAT_CLIPPINGS), - SettingInfo::Enum(StrId::STR_CLIPPING_DELETE_MODE, &CrossPointSettings::clippingDeleteMode, - {StrId::STR_DELETE_PERMANENT, StrId::STR_DELETE_META_ONLY}, "clippingDeleteMode", - StrId::STR_CAT_CLIPPINGS), // --- KOReader Sync (web-only, uses KOReaderCredentialStore) --- StrId::STR_CUSTOMISE_STATUS_BAR), diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 2d6802ac6e..c8962b0201 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -184,19 +184,22 @@ void ClipSelectionActivity::drawHighlights() { for (int i = from; i <= to; ++i) { if (i == cursorIdx) continue; if (words[i].pageIdx != currentDisplayPage) continue; - if (words[i].text.find_first_not_of(" \t") == std::string::npos) continue; const auto r = alignedRect(words[i].x, words[i].y, words[i].w, words[i].h); renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); - renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); + if (words[i].text.find_first_not_of(" \t") != std::string::npos) { + renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); + } } } - // Draw cursor highlight (always on top) — skip whitespace-only words + // Draw cursor highlight (always on top) const auto& cw = words[cursorIdx]; - if (cw.pageIdx == currentDisplayPage && cw.text.find_first_not_of(" \t") != std::string::npos) { + if (cw.pageIdx == currentDisplayPage) { const auto r = alignedRect(cw.x, cw.y, cw.w, cw.h); renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); - renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); + if (cw.text.find_first_not_of(" \t") != std::string::npos) { + renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); + } } } diff --git a/src/activities/reader/ClippingsMenuActivity.cpp b/src/activities/reader/ClippingsMenuActivity.cpp new file mode 100644 index 0000000000..7a49c55bd5 --- /dev/null +++ b/src/activities/reader/ClippingsMenuActivity.cpp @@ -0,0 +1,118 @@ +#include "ClippingsMenuActivity.h" + +#include <GfxRenderer.h> +#include <I18n.h> + +#include "CrossPointSettings.h" +#include "MappedInputManager.h" +#include "components/UITheme.h" +#include "fontIds.h" + +void ClippingsMenuActivity::onEnter() { + Activity::onEnter(); + requestUpdate(); +} + +void ClippingsMenuActivity::onExit() { Activity::onExit(); } + +// Item 0: Navigation (clipNavMode) +// Item 1: Show Annotations (annotationVisibility) +// Item 2: Storage (clippingStorage) + +void ClippingsMenuActivity::toggleSetting(int idx) { + switch (idx) { + case 0: + SETTINGS.clipNavMode = (SETTINGS.clipNavMode + 1) % CrossPointSettings::CLIP_NAV_MODE_COUNT; + break; + case 1: + SETTINGS.annotationVisibility = (SETTINGS.annotationVisibility + 1) % 2; + break; + case 2: + SETTINGS.clippingStorage = (SETTINGS.clippingStorage + 1) % CrossPointSettings::CLIPPING_STORAGE_COUNT; + break; + default: + break; + } + SETTINGS.saveToFile(); +} + +const char* ClippingsMenuActivity::settingValue(int idx) const { + switch (idx) { + case 0: + return SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE ? tr(STR_CLIP_NAV_LINE) : tr(STR_CLIP_NAV_WORD); + case 1: + return SETTINGS.annotationVisibility ? tr(STR_STATE_ON) : tr(STR_STATE_OFF); + case 2: + return SETTINGS.clippingStorage == CrossPointSettings::SINGLE_FILE ? tr(STR_CLIPPING_SINGLE_FILE) + : tr(STR_CLIPPING_PER_BOOK); + default: + return ""; + } +} + +void ClippingsMenuActivity::loop() { + buttonNavigator.onNextRelease([this] { + selectedIndex = ButtonNavigator::nextIndex(selectedIndex, ITEM_COUNT); + requestUpdate(); + }); + + buttonNavigator.onPreviousRelease([this] { + selectedIndex = ButtonNavigator::previousIndex(selectedIndex, ITEM_COUNT); + requestUpdate(); + }); + + if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { + toggleSetting(selectedIndex); + requestUpdate(); + return; + } + + if (mappedInput.wasReleased(MappedInputManager::Button::Back)) { + finish(); + } +} + +void ClippingsMenuActivity::render(RenderLock&&) { + renderer.clearScreen(); + + const auto pageWidth = renderer.getScreenWidth(); + const auto orientation = renderer.getOrientation(); + const bool isLandscapeCw = orientation == GfxRenderer::Orientation::LandscapeClockwise; + const bool isLandscapeCcw = orientation == GfxRenderer::Orientation::LandscapeCounterClockwise; + const bool isPortraitInverted = orientation == GfxRenderer::Orientation::PortraitInverted; + const int hintGutterWidth = (isLandscapeCw || isLandscapeCcw) ? 30 : 0; + const int contentX = isLandscapeCw ? hintGutterWidth : 0; + const int contentWidth = pageWidth - hintGutterWidth; + const int hintGutterHeight = isPortraitInverted ? 50 : 0; + const int contentY = hintGutterHeight; + + const int titleX = + contentX + (contentWidth - renderer.getTextWidth(UI_12_FONT_ID, tr(STR_CAT_CLIPPINGS), EpdFontFamily::BOLD)) / 2; + renderer.drawText(UI_12_FONT_ID, titleX, 15 + contentY, tr(STR_CAT_CLIPPINGS), true, EpdFontFamily::BOLD); + + static const StrId labels[ITEM_COUNT] = {StrId::STR_CLIP_NAV_MODE, StrId::STR_ANNOT_SHOW, + StrId::STR_CLIPPING_STORAGE}; + + const int startY = 60 + contentY; + constexpr int lineHeight = 32; + + for (int i = 0; i < ITEM_COUNT; ++i) { + const int displayY = startY + i * lineHeight; + const bool isSelected = (i == selectedIndex); + + if (isSelected) { + renderer.fillRect(contentX, displayY, contentWidth - 1, lineHeight, true); + } + + renderer.drawText(UI_10_FONT_ID, contentX + 20, displayY + 6, I18N.get(labels[i]), !isSelected); + + const char* val = settingValue(i); + const int valW = renderer.getTextWidth(UI_10_FONT_ID, val); + renderer.drawText(UI_10_FONT_ID, contentX + contentWidth - 20 - valW, displayY + 6, val, !isSelected); + } + + const auto hints = mappedInput.mapLabels(tr(STR_BACK), tr(STR_TOGGLE), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); + GUI.drawButtonHints(renderer, hints.btn1, hints.btn2, hints.btn3, hints.btn4); + + renderer.displayBuffer(); +} diff --git a/src/activities/reader/ClippingsMenuActivity.h b/src/activities/reader/ClippingsMenuActivity.h new file mode 100644 index 0000000000..45b29fcee4 --- /dev/null +++ b/src/activities/reader/ClippingsMenuActivity.h @@ -0,0 +1,24 @@ +#pragma once +#include <I18n.h> + +#include "../Activity.h" +#include "util/ButtonNavigator.h" + +class ClippingsMenuActivity final : public Activity { + ButtonNavigator buttonNavigator; + int selectedIndex = 0; + static constexpr int ITEM_COUNT = 3; + + void toggleSetting(int idx); + const char* settingValue(int idx) const; + + public: + explicit ClippingsMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) + : Activity("ClippingsMenu", renderer, mappedInput) {} + + void onEnter() override; + void onExit() override; + void loop() override; + void render(RenderLock&&) override; + bool isReaderActivity() const override { return true; } +}; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 031a39ef3c..eb39c17656 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -417,7 +417,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction } case EpubReaderMenuActivity::MenuAction::SAVE_CLIPPING: { if (section && epub) { - // Compute margins matching the reader render pass int mTop, mRight, mBottom, mLeft; renderer.getOrientedViewableTRBL(&mTop, &mRight, &mBottom, &mLeft); mTop += SETTINGS.screenMargin; @@ -429,7 +428,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int pagesToLoad = std::min(3, section->pageCount - startPage); std::vector<ClipSelectionActivity::WordRef> words; - words.reserve(pagesToLoad * 60); // rough estimate + words.reserve(pagesToLoad * 60); for (int pi = 0; pi < pagesToLoad; ++pi) { section->currentPage = startPage + pi; @@ -448,9 +447,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int wx = mLeft + line.xPos + xpos[i]; const int wy = mTop + line.yPos; const int ww = renderer.getTextWidth(readerFontId, wlist[i].c_str()); - if (ww > 0) { - words.push_back({wx, wy, ww, lineH, pi, wlist[i]}); - } + if (ww > 0) words.push_back({wx, wy, ww, lineH, pi, wlist[i]}); } } } @@ -461,39 +458,34 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; - // Capture words by value before moving into ClipSelectionActivity auto wordsCopy = words; - startActivityForResult(std::make_unique<ClipSelectionActivity>( - renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), - chapterTitle, startPage + 1, readerFontId, *section, startPage, mTop, mLeft), - [this, chapterTitle, startPage, - wordsCopy = std::move(wordsCopy)](const ActivityResult& result) mutable { - if (!result.isCancelled) { - const auto& clip = std::get<ClippingResult>(result.data); - if (!clip.text.empty()) { - ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, - startPage + 1, clip.text); - // Build annotation record from selected word pixel positions - if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0 && epub) { - AnnotationsManager::AnnotationRecord rec; - rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); - const int to = std::min(clip.toWordIdx, static_cast<int>(wordsCopy.size()) - 1); - for (int i = clip.fromWordIdx; i <= to; ++i) { - const auto absPage = static_cast<uint16_t>(startPage + wordsCopy[i].pageIdx); - rec.rects.push_back( - {static_cast<int16_t>(wordsCopy[i].x), static_cast<int16_t>(wordsCopy[i].y), - static_cast<int16_t>(wordsCopy[i].w), static_cast<int16_t>(wordsCopy[i].h), - absPage}); - } - const size_t previewLen = clip.text.size() < 100 ? clip.text.size() : 100; - rec.textPreview = clip.text.substr(0, previewLen); - annotations.add(std::move(rec)); - annotations.save(epub->getCachePath().c_str()); - } - } - } - requestUpdate(); - }); + startActivityForResult( + std::make_unique<ClipSelectionActivity>(renderer, mappedInput, std::move(words), epub->getTitle(), + epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, + *section, startPage, mTop, mLeft), + [this, chapterTitle, startPage, wordsCopy = std::move(wordsCopy)](const ActivityResult& result) mutable { + if (!result.isCancelled) { + const auto& clip = std::get<ClippingResult>(result.data); + if (!clip.text.empty()) { + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, + clip.text); + if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0 && epub) { + AnnotationsManager::AnnotationRecord rec; + rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); + const int to = std::min(clip.toWordIdx, static_cast<int>(wordsCopy.size()) - 1); + for (int i = clip.fromWordIdx; i <= to; ++i) { + const auto absPage = static_cast<uint16_t>(startPage + wordsCopy[i].pageIdx); + rec.rects.push_back({static_cast<int16_t>(wordsCopy[i].x), static_cast<int16_t>(wordsCopy[i].y), + static_cast<int16_t>(wordsCopy[i].w), static_cast<int16_t>(wordsCopy[i].h), + absPage}); + } + annotations.add(std::move(rec)); + annotations.save(epub->getCachePath().c_str()); + } + } + } + requestUpdate(); + }); } else { requestUpdate(); } @@ -868,12 +860,31 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or const int screenW = renderer.getScreenWidth(); const auto sectionAnnotations = annotations.forSection(static_cast<uint16_t>(currentSpineIndex)); for (const auto& rec : sectionAnnotations) { + // Group rects by row (underlineY) and draw one continuous line per row + // to avoid gaps between words caused by per-word drawing + struct RowSpan { + int underlineY, xMin, xMax; + }; + std::vector<RowSpan> spans; + spans.reserve(rec.rects.size()); for (const auto& r : rec.rects) { if (r.sectionPage != static_cast<uint16_t>(section->currentPage)) continue; const int underlineY = r.y + r.h; if (underlineY < 0 || underlineY >= screenH) continue; if (r.x < 0 || r.x >= screenW) continue; - renderer.drawLine(r.x, underlineY, r.x + r.w - 1, underlineY, 2, true); + bool merged = false; + for (auto& span : spans) { + if (span.underlineY == underlineY) { + if (r.x < span.xMin) span.xMin = r.x; + if (r.x + r.w - 1 > span.xMax) span.xMax = r.x + r.w - 1; + merged = true; + break; + } + } + if (!merged) spans.push_back({underlineY, r.x, r.x + r.w - 1}); + } + for (const auto& span : spans) { + renderer.drawLine(span.xMin, span.underlineY, span.xMax, span.underlineY, 2, true); } } } diff --git a/src/activities/settings/SettingsActivity.cpp b/src/activities/settings/SettingsActivity.cpp index b9bc04657c..a4529537ce 100644 --- a/src/activities/settings/SettingsActivity.cpp +++ b/src/activities/settings/SettingsActivity.cpp @@ -3,6 +3,7 @@ #include <GfxRenderer.h> #include <Logging.h> +#include "../reader/ClippingsMenuActivity.h" #include "ButtonRemapActivity.h" #include "ClearCacheActivity.h" #include "CrossPointSettings.h" @@ -41,7 +42,6 @@ void SettingsActivity::onEnter() { } else if (setting.category == StrId::STR_CAT_SYSTEM) { systemSettings.push_back(setting); } - // Web-only categories (KOReader Sync, OPDS Browser) are skipped for device UI } // Append device-only ACTION items @@ -55,14 +55,30 @@ void SettingsActivity::onEnter() { systemSettings.push_back(SettingInfo::Action(StrId::STR_SD_FIRMWARE_UPDATE, SettingAction::SdFirmwareUpdate)); systemSettings.push_back(SettingInfo::Action(StrId::STR_LANGUAGE, SettingAction::Language)); readerSettings.push_back(SettingInfo::Action(StrId::STR_CUSTOMISE_STATUS_BAR, SettingAction::CustomiseStatusBar)); + readerSettings.push_back(SettingInfo::Action(StrId::STR_CAT_CLIPPINGS, SettingAction::ClippingSettings)); - // Reset selection to first category - selectedCategoryIndex = 0; + // Clamp initialCategoryIndex in case it was set out of range + if (selectedCategoryIndex < 0 || selectedCategoryIndex >= categoryCount) { + selectedCategoryIndex = 0; + } selectedSettingIndex = 0; - // Initialize with first category (Display) - currentSettings = &displaySettings; - settingsCount = static_cast<int>(displaySettings.size()); + // Initialize with the requested category + switch (selectedCategoryIndex) { + case 1: + currentSettings = &readerSettings; + break; + case 2: + currentSettings = &controlsSettings; + break; + case 3: + currentSettings = &systemSettings; + break; + default: + currentSettings = &displaySettings; + break; + } + settingsCount = static_cast<int>(currentSettings->size()); // Trigger first update requestUpdate(); @@ -197,6 +213,9 @@ void SettingsActivity::toggleCurrentSetting() { case SettingAction::Language: startActivityForResult(std::make_unique<LanguageSelectActivity>(renderer, mappedInput), resultHandler); break; + case SettingAction::ClippingSettings: + startActivityForResult(std::make_unique<ClippingsMenuActivity>(renderer, mappedInput), resultHandler); + break; case SettingAction::None: // Do nothing break; diff --git a/src/activities/settings/SettingsActivity.h b/src/activities/settings/SettingsActivity.h index fb2d311e27..df2e8a163d 100644 --- a/src/activities/settings/SettingsActivity.h +++ b/src/activities/settings/SettingsActivity.h @@ -22,6 +22,7 @@ enum class SettingAction { CheckForUpdates, SdFirmwareUpdate, Language, + ClippingSettings, }; struct SettingInfo { @@ -161,8 +162,8 @@ class SettingsActivity final : public Activity { void toggleCurrentSetting(); public: - explicit SettingsActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) - : Activity("Settings", renderer, mappedInput) {} + explicit SettingsActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, int initialCategoryIndex = 0) + : Activity("Settings", renderer, mappedInput), selectedCategoryIndex(initialCategoryIndex) {} void onEnter() override; void onExit() override; void loop() override; diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index dc28f7d3d0..2891553011 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -4,9 +4,6 @@ #include <Logging.h> #include <common/FsApiConstants.h> -#include <algorithm> -#include <cstring> - static constexpr const char* ANNOT_FILENAME = "/annotations.bin"; std::string AnnotationsManager::annotationsPath(const char* bookCachePath) { @@ -45,13 +42,6 @@ bool AnnotationsManager::load(const char* bookCachePath) { if (file.read(&rec.rects[r], sizeof(Rect)) != sizeof(Rect)) goto done; } - uint8_t previewLen = 0; - if (file.read(&previewLen, 1) != 1) break; - if (previewLen > 0) { - rec.textPreview.resize(previewLen); - if (file.read(rec.textPreview.data(), previewLen) != previewLen) break; - } - records.push_back(std::move(rec)); } @@ -82,12 +72,6 @@ bool AnnotationsManager::save(const char* bookCachePath) const { for (const auto& r : rec.rects) { file.write(&r, sizeof(Rect)); } - const uint8_t previewLen = static_cast<uint8_t>( - rec.textPreview.size() < MAX_PREVIEW_LEN ? rec.textPreview.size() : MAX_PREVIEW_LEN); - file.write(&previewLen, 1); - if (previewLen > 0) { - file.write(rec.textPreview.c_str(), previewLen); - } } file.flush(); @@ -96,90 +80,7 @@ bool AnnotationsManager::save(const char* bookCachePath) const { return true; } -void AnnotationsManager::add(AnnotationRecord record) { - records.push_back(std::move(record)); -} - -void AnnotationsManager::removeMeta(size_t idx) { - if (idx < records.size()) { - records.erase(records.begin() + static_cast<ptrdiff_t>(idx)); - } -} - -bool AnnotationsManager::permanentDelete(size_t idx, const char* clippingFilePath) { - if (idx >= records.size()) return false; - const std::string preview = records[idx].textPreview; - - // Streaming copy of clipping file, skipping the matching entry - // Each entry in the Kindle format ends with "\n==========\n" - static constexpr char SEPARATOR[] = "\n==========\n"; - static constexpr size_t SEP_LEN = sizeof(SEPARATOR) - 1; - - // Build temp path alongside the source file - const std::string tempPath = std::string(clippingFilePath) + ".tmp"; - - HalFile src = Storage.open(clippingFilePath, O_RDONLY); - if (!src) { - LOG_ERR("ANNOT", "Failed to open %s for reading", clippingFilePath); - removeMeta(idx); - return false; - } - - HalFile dst = Storage.open(tempPath.c_str(), O_RDWR | O_CREAT | O_TRUNC); - if (!dst) { - src.close(); - LOG_ERR("ANNOT", "Failed to open temp file %s", tempPath.c_str()); - return false; - } - - // Read file entry-by-entry using separator as delimiter - std::string entry; - entry.reserve(STREAM_BLOCK); - char buf[STREAM_BLOCK]; - bool skipped = false; - - // We accumulate until we find a separator, then decide to copy or skip - while (true) { - int32_t n = src.read(buf, sizeof(buf)); - if (n <= 0) break; - entry.append(buf, static_cast<size_t>(n)); - - // Process all complete entries in the accumulated buffer - size_t pos = 0; - while (true) { - size_t sepPos = entry.find(SEPARATOR, pos); - if (sepPos == std::string::npos) break; - - const std::string chunk = entry.substr(pos, sepPos + SEP_LEN - pos); - - // Check if this entry contains the preview text we want to delete - if (!skipped && !preview.empty() && chunk.find(preview) != std::string::npos) { - skipped = true; // Skip this entry - } else { - dst.write(chunk.c_str(), chunk.size()); - } - pos = sepPos + SEP_LEN; - } - entry = entry.substr(pos); // Keep leftover (incomplete entry) - } - - // Write any trailing content that doesn't end with a separator - if (!entry.empty()) { - dst.write(entry.c_str(), entry.size()); - } - - src.close(); - dst.flush(); - dst.close(); - - // Replace original with temp - Storage.remove(clippingFilePath); - Storage.rename(tempPath.c_str(), clippingFilePath); - - removeMeta(idx); - LOG_DBG("ANNOT", "Permanently deleted annotation %zu from %s (skipped=%d)", idx, clippingFilePath, skipped); - return true; -} +void AnnotationsManager::add(AnnotationRecord record) { records.push_back(std::move(record)); } std::vector<AnnotationsManager::AnnotationRecord> AnnotationsManager::forSection(uint16_t sectionIdx) const { std::vector<AnnotationRecord> result; @@ -197,10 +98,3 @@ bool AnnotationsManager::hasAnnotationsForSection(uint16_t sectionIdx) const { } return false; } - -size_t AnnotationsManager::firstIndexForSection(uint16_t sectionIdx) const { - for (size_t i = 0; i < records.size(); ++i) { - if (records[i].sectionIdx == sectionIdx) return i; - } - return SIZE_MAX; -} diff --git a/src/annotations/AnnotationsManager.h b/src/annotations/AnnotationsManager.h index 95d5e99e5b..3f59d39853 100644 --- a/src/annotations/AnnotationsManager.h +++ b/src/annotations/AnnotationsManager.h @@ -13,8 +13,7 @@ class AnnotationsManager { struct AnnotationRecord { uint16_t sectionIdx; - std::vector<Rect> rects; // pixel rects for drawing underlines - std::string textPreview; // first 100 chars for matching on permanent delete + std::vector<Rect> rects; }; // Load annotations from .crosspoint/epub_<hash>/annotations.bin @@ -24,23 +23,13 @@ class AnnotationsManager { void add(AnnotationRecord record); - // Remove only the metadata (annotations.bin entry) — text file untouched - void removeMeta(size_t idx); - - // Remove metadata AND delete matching entry from the clipping txt file (streaming, no full-file RAM read) - bool permanentDelete(size_t idx, const char* clippingFilePath); - std::vector<AnnotationRecord> forSection(uint16_t sectionIdx) const; bool hasAnnotationsForSection(uint16_t sectionIdx) const; - // Returns global index of first annotation for section, or SIZE_MAX if none - size_t firstIndexForSection(uint16_t sectionIdx) const; bool empty() const { return records.empty(); } size_t size() const { return records.size(); } private: - static constexpr uint8_t FILE_VERSION = 2; - static constexpr size_t MAX_PREVIEW_LEN = 100; - static constexpr size_t STREAM_BLOCK = 512; + static constexpr uint8_t FILE_VERSION = 3; std::vector<AnnotationRecord> records; From c41a659b19cee302534f488b9f57e71f9d3ca6db Mon Sep 17 00:00:00 2001 From: Pavlo Zubenko <pashazubenko@gmail.com> Date: Wed, 29 Apr 2026 22:19:14 +0300 Subject: [PATCH 12/19] refactor: optimize annotation retrieval using STL algorithms --- open-x4-sdk | 2 +- src/activities/ActivityResult.h | 5 +++-- src/activities/reader/EpubReaderActivity.cpp | 19 +++++++++---------- src/annotations/AnnotationsManager.cpp | 15 ++++++--------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/open-x4-sdk b/open-x4-sdk index 7d86603ad2..a64a3c29be 160000 --- a/open-x4-sdk +++ b/open-x4-sdk @@ -1 +1 @@ -Subproject commit 7d86603ad27709a9a766bb5ad893cfc39e60777e +Subproject commit a64a3c29bebc59b2ccdfe15492cfc4b5e4c26360 diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index 748d52970c..304f660076 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -60,8 +60,9 @@ struct ClippingResult { int toWordIdx = -1; }; -using ResultVariant = std::variant<std::monostate, WifiResult, KeyboardResult, MenuResult, ChapterResult, PercentResult, - PageResult, SyncResult, NetworkModeResult, FootnoteResult, FilePathResult, ClippingResult>; +using ResultVariant = + std::variant<std::monostate, WifiResult, KeyboardResult, MenuResult, ChapterResult, PercentResult, PageResult, + SyncResult, NetworkModeResult, FootnoteResult, FilePathResult, ClippingResult>; struct ActivityResult { bool isCancelled = false; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index eb39c17656..18464754b3 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -11,6 +11,7 @@ #include <esp_system.h> #include <freertos/task.h> +#include <algorithm> #include <iterator> #include <limits> @@ -469,7 +470,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction if (!clip.text.empty()) { ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, clip.text); - if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0 && epub) { + if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0) { AnnotationsManager::AnnotationRecord rec; rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); const int to = std::min(clip.toWordIdx, static_cast<int>(wordsCopy.size()) - 1); @@ -872,16 +873,14 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or const int underlineY = r.y + r.h; if (underlineY < 0 || underlineY >= screenH) continue; if (r.x < 0 || r.x >= screenW) continue; - bool merged = false; - for (auto& span : spans) { - if (span.underlineY == underlineY) { - if (r.x < span.xMin) span.xMin = r.x; - if (r.x + r.w - 1 > span.xMax) span.xMax = r.x + r.w - 1; - merged = true; - break; - } + auto it = std::find_if(spans.begin(), spans.end(), + [underlineY](const RowSpan& s) { return s.underlineY == underlineY; }); + if (it != spans.end()) { + if (r.x < it->xMin) it->xMin = r.x; + if (r.x + r.w - 1 > it->xMax) it->xMax = r.x + r.w - 1; + } else { + spans.push_back({underlineY, r.x, r.x + r.w - 1}); } - if (!merged) spans.push_back({underlineY, r.x, r.x + r.w - 1}); } for (const auto& span : spans) { renderer.drawLine(span.xMin, span.underlineY, span.xMax, span.underlineY, 2, true); diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index 2891553011..a15e09cc38 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -4,6 +4,8 @@ #include <Logging.h> #include <common/FsApiConstants.h> +#include <algorithm> + static constexpr const char* ANNOT_FILENAME = "/annotations.bin"; std::string AnnotationsManager::annotationsPath(const char* bookCachePath) { @@ -84,17 +86,12 @@ void AnnotationsManager::add(AnnotationRecord record) { records.push_back(std::m std::vector<AnnotationsManager::AnnotationRecord> AnnotationsManager::forSection(uint16_t sectionIdx) const { std::vector<AnnotationRecord> result; - for (const auto& rec : records) { - if (rec.sectionIdx == sectionIdx) { - result.push_back(rec); - } - } + std::copy_if(records.begin(), records.end(), std::back_inserter(result), + [sectionIdx](const AnnotationRecord& rec) { return rec.sectionIdx == sectionIdx; }); return result; } bool AnnotationsManager::hasAnnotationsForSection(uint16_t sectionIdx) const { - for (const auto& rec : records) { - if (rec.sectionIdx == sectionIdx) return true; - } - return false; + return std::any_of(records.begin(), records.end(), + [sectionIdx](const AnnotationRecord& rec) { return rec.sectionIdx == sectionIdx; }); } From 0fdc86b9d5a4d0b83b057ebf0150503248dec318 Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Sat, 2 May 2026 23:05:53 +0200 Subject: [PATCH 13/19] feat: enhance clipping with paragraph detection, hyphen merge, and power button shortcut - Add paragraph detection via em-space prefix and Y-gap for proper newline insertion in clipping text - Merge hyphenated words across line breaks (blocked across paragraph boundaries) - Strip em-space from clipping text output - Add power button shortcut (CLIPPING mode) for quick clipping access - Implement WORD_BY_WORD continuous scroll on button hold - Rewrite ClippingsMenuActivity with wasPressed pattern and continuous navigation - Extract startClipSelection() from EpubReaderActivity for reuse - Add AnnotationRect to ClippingResult for em-space-aware rect coordinates - Add word style to WordRef for correct text width calculation - Fix annotation underline rendering with RowSpan grouping - Restore verbose SL lock logging for debugging --- .gitignore | 1 + lib/hal/HalStorage.cpp | 6 +- src/CrossPointSettings.h | 2 +- src/SettingsList.h | 39 +++- src/activities/ActivityResult.h | 5 + .../reader/ClipSelectionActivity.cpp | 177 ++++++++++++++---- src/activities/reader/ClipSelectionActivity.h | 4 +- .../reader/ClippingsMenuActivity.cpp | 136 ++++++-------- src/activities/reader/ClippingsMenuActivity.h | 1 - src/activities/reader/EpubReaderActivity.cpp | 160 ++++++++-------- src/activities/reader/EpubReaderActivity.h | 1 + src/annotations/AnnotationsManager.cpp | 2 +- src/clippings/ClippingsManager.cpp | 5 +- 13 files changed, 335 insertions(+), 204 deletions(-) diff --git a/.gitignore b/.gitignore index f4bbcdfce5..31d7c7bb54 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ build .history/ /.venv *.local* +.claude.md diff --git a/lib/hal/HalStorage.cpp b/lib/hal/HalStorage.cpp index 6a4c96191c..61f4e56b53 100644 --- a/lib/hal/HalStorage.cpp +++ b/lib/hal/HalStorage.cpp @@ -28,16 +28,12 @@ bool HalStorage::ready() const { return SDCard.ready(); } class HalStorage::StorageLock { public: StorageLock() { -#if LOG_LEVEL >= 2 LOG_DBG("LOCK", "SL take from %s", pcTaskGetName(nullptr)); -#endif xSemaphoreTake(HalStorage::getInstance().storageMutex, portMAX_DELAY); } ~StorageLock() { -#if LOG_LEVEL >= 2 - LOG_DBG("LOCK", "SL give from %s", pcTaskGetName(nullptr)); -#endif xSemaphoreGive(HalStorage::getInstance().storageMutex); + LOG_DBG("LOCK", "SL give from %s", pcTaskGetName(nullptr)); } }; diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index a7b757c009..b9e3c5686f 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -126,7 +126,7 @@ class CrossPointSettings { }; // Short power button press actions - enum SHORT_PWRBTN { IGNORE = 0, SLEEP = 1, PAGE_TURN = 2, FORCE_REFRESH = 3, SHORT_PWRBTN_COUNT }; + enum SHORT_PWRBTN { IGNORE = 0, SLEEP = 1, PAGE_TURN = 2, FORCE_REFRESH = 3, CLIPPING = 4, SHORT_PWRBTN_COUNT }; // Hide battery percentage enum HIDE_BATTERY_PERCENTAGE { HIDE_NEVER = 0, HIDE_READER = 1, HIDE_ALWAYS = 2, HIDE_BATTERY_PERCENTAGE_COUNT }; diff --git a/src/SettingsList.h b/src/SettingsList.h index 5d466cd4b2..7acd75aad9 100644 --- a/src/SettingsList.h +++ b/src/SettingsList.h @@ -76,7 +76,8 @@ inline const std::vector<SettingInfo>& getSettingsList() { StrId::STR_LONG_PRESS_BEHAVIOR_ORIENTATION}, "longPressButtonBehavior", StrId::STR_CAT_CONTROLS), SettingInfo::Enum(StrId::STR_SHORT_PWR_BTN, &CrossPointSettings::shortPwrBtn, - {StrId::STR_IGNORE, StrId::STR_SLEEP, StrId::STR_PAGE_TURN, StrId::STR_FORCE_REFRESH}, + {StrId::STR_IGNORE, StrId::STR_SLEEP, StrId::STR_PAGE_TURN, StrId::STR_FORCE_REFRESH, + StrId::STR_SAVE_CLIPPING}, "shortPwrBtn", StrId::STR_CAT_CONTROLS), // --- System --- SettingInfo::Enum(StrId::STR_TIME_TO_SLEEP, &CrossPointSettings::sleepTimeout, @@ -96,6 +97,42 @@ inline const std::vector<SettingInfo>& getSettingsList() { StrId::STR_CAT_CLIPPINGS), // --- KOReader Sync (web-only, uses KOReaderCredentialStore) --- + SettingInfo::DynamicString( + StrId::STR_KOREADER_USERNAME, [] { return KOREADER_STORE.getUsername(); }, + [](const std::string& v) { + KOREADER_STORE.setCredentials(v, KOREADER_STORE.getPassword()); + KOREADER_STORE.saveToFile(); + }, + "koUsername", StrId::STR_KOREADER_SYNC), + SettingInfo::DynamicString( + StrId::STR_KOREADER_PASSWORD, [] { return KOREADER_STORE.getPassword(); }, + [](const std::string& v) { + KOREADER_STORE.setCredentials(KOREADER_STORE.getUsername(), v); + KOREADER_STORE.saveToFile(); + }, + "koPassword", StrId::STR_KOREADER_SYNC), + SettingInfo::DynamicString( + StrId::STR_SYNC_SERVER_URL, [] { return KOREADER_STORE.getServerUrl(); }, + [](const std::string& v) { + KOREADER_STORE.setServerUrl(v); + KOREADER_STORE.saveToFile(); + }, + "koServerUrl", StrId::STR_KOREADER_SYNC), + SettingInfo::DynamicEnum( + StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, + [] { return static_cast<uint8_t>(KOREADER_STORE.getMatchMethod()); }, + [](uint8_t v) { + KOREADER_STORE.setMatchMethod(static_cast<DocumentMatchMethod>(v)); + KOREADER_STORE.saveToFile(); + }, + "koMatchMethod", StrId::STR_KOREADER_SYNC), + // --- Status Bar Settings (web-only, uses StatusBarSettingsActivity) --- + SettingInfo::Toggle(StrId::STR_CHAPTER_PAGE_COUNT, &CrossPointSettings::statusBarChapterPageCount, + "statusBarChapterPageCount", StrId::STR_CUSTOMISE_STATUS_BAR), + SettingInfo::Toggle(StrId::STR_BOOK_PROGRESS_PERCENTAGE, &CrossPointSettings::statusBarBookProgressPercentage, + "statusBarBookProgressPercentage", StrId::STR_CUSTOMISE_STATUS_BAR), + SettingInfo::Enum(StrId::STR_PROGRESS_BAR, &CrossPointSettings::statusBarProgressBar, + {StrId::STR_BOOK, StrId::STR_CHAPTER, StrId::STR_HIDE}, "statusBarProgressBar", StrId::STR_CUSTOMISE_STATUS_BAR), SettingInfo::Enum(StrId::STR_PROGRESS_BAR_THICKNESS, &CrossPointSettings::statusBarProgressBarThickness, {StrId::STR_PROGRESS_BAR_THIN, StrId::STR_PROGRESS_BAR_MEDIUM, StrId::STR_PROGRESS_BAR_THICK}, diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index 304f660076..0bf1549e18 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -58,6 +58,11 @@ struct ClippingResult { std::string text; int fromWordIdx = -1; int toWordIdx = -1; + struct AnnotationRect { + int16_t x, y, w, h; + uint16_t sectionPage; + }; + std::vector<AnnotationRect> rects; }; using ResultVariant = diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index c8962b0201..0efcb2db4c 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -85,6 +85,14 @@ void ClipSelectionActivity::loop() { if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); + } else { + buttonNavigator.onNextContinuous([this, total] { + if (cursorIdx + 1 >= total) return; + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = cursorIdx + 1; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); } buttonNavigator.onPreviousRelease([this] { @@ -104,6 +112,14 @@ void ClipSelectionActivity::loop() { if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); + } else { + buttonNavigator.onPreviousContinuous([this] { + if (cursorIdx == 0) return; + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = cursorIdx - 1; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); } if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { @@ -114,12 +130,64 @@ void ClipSelectionActivity::loop() { const int from = std::min(startMarkIdx, cursorIdx); const int to = std::max(startMarkIdx, cursorIdx); std::string text; + std::vector<ClippingResult::AnnotationRect> rects; + rects.reserve(to - from + 1); + auto pushWordRect = [this](int i, std::vector<ClippingResult::AnnotationRect>& out) { + int rx = words[i].x; + int rw = words[i].w; + const auto& t = words[i].text; + if (t.size() >= 3 && static_cast<unsigned char>(t[0]) == 0xE2 && static_cast<unsigned char>(t[1]) == 0x80 && + static_cast<unsigned char>(t[2]) == 0x83) { + const auto s = static_cast<EpdFontFamily::Style>(words[i].style & ~EpdFontFamily::UNDERLINE); + const int skip = renderer.getTextAdvanceX(fontId, "\xe2\x80\x83", s); + rx += skip; + rw -= skip; + } + out.push_back({static_cast<int16_t>(rx), static_cast<int16_t>(words[i].y), static_cast<int16_t>(rw), + static_cast<int16_t>(words[i].h), static_cast<uint16_t>(startPageInSection + words[i].pageIdx)}); + }; + auto stripEmSpace = [](const std::string& w) -> const std::string& { + static thread_local std::string buf; + if (w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && + static_cast<unsigned char>(w[2]) == 0x83) { + buf = w.substr(3); + return buf; + } + return w; + }; + auto hasEmSpace = [](const std::string& w) -> bool { + return w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && + static_cast<unsigned char>(w[2]) == 0x83; + }; for (int i = from; i <= to; ++i) { - if (!text.empty()) text += ' '; - text += words[i].text; + const auto& wtext = stripEmSpace(words[i].text); + const bool emSpaceStart = hasEmSpace(words[i].text) && i > from; + const bool yGapBreak = + i > from && words[i].pageIdx == words[i - 1].pageIdx && words[i].y > words[i - 1].y + words[i - 1].h; + const bool paragraphStart = emSpaceStart || yGapBreak; + if (i > from && !text.empty() && !paragraphStart) { + const auto& prev = words[i - 1].text; + const auto& prevStripped = stripEmSpace(prev); + if (prevStripped.size() >= 1 && prevStripped.back() == '-' && !wtext.empty() && + static_cast<unsigned char>(wtext[0]) >= 'a' && static_cast<unsigned char>(wtext[0]) <= 'z' && + prevStripped.find('-') == prevStripped.size() - 1) { + text.pop_back(); + text += wtext; + pushWordRect(i, rects); + continue; + } + } + if (paragraphStart) { + text += '\n'; + } else if (!text.empty()) { + text += ' '; + } + text += wtext; + pushWordRect(i, rects); } + ActivityResult result; - result.data = ClippingResult{std::move(text), from, to}; + result.data = ClippingResult{std::move(text), from, to, std::move(rects)}; setResult(std::move(result)); finish(); } @@ -177,28 +245,74 @@ void ClipSelectionActivity::switchToPage(int pageIdx) { } void ClipSelectionActivity::drawHighlights() { - // Draw selection range (words on the currently displayed page only) + auto hasEmSpace = [](const std::string& t) { + return t.size() >= 3 && static_cast<unsigned char>(t[0]) == 0xE2 && static_cast<unsigned char>(t[1]) == 0x80 && + static_cast<unsigned char>(t[2]) == 0x83; + }; + + auto emSpaceSkip = [this, &hasEmSpace](const std::string& t, EpdFontFamily::Style s) -> int { + if (hasEmSpace(t)) return renderer.getTextAdvanceX(fontId, "\xe2\x80\x83", s); + return 0; + }; + + auto drawContinuousHighlight = [this, &emSpaceSkip, &hasEmSpace](int first, int last) { + const auto& fw = words[first]; + const auto& lw = words[last]; + const auto fs = static_cast<EpdFontFamily::Style>(fw.style & ~EpdFontFamily::UNDERLINE); + const int skip = emSpaceSkip(fw.text, fs); + const int startX = fw.x + skip; + const int spanW = (lw.x + lw.w) - startX; + renderer.fillRectDither(startX, fw.y, spanW, fw.h, Color::LightGray); + for (int i = first; i <= last; ++i) { + if (words[i].text.find_first_not_of(" \t") != std::string::npos) { + const auto s = static_cast<EpdFontFamily::Style>(words[i].style & ~EpdFontFamily::UNDERLINE); + if (i == first && hasEmSpace(words[i].text)) { + renderer.drawText(fontId, startX, words[i].y, words[i].text.c_str() + 3, true, s); + } else { + renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true, s); + } + } + } + }; + if (startMarkIdx != -1) { const int from = std::min(startMarkIdx, cursorIdx); const int to = std::max(startMarkIdx, cursorIdx); + int runStart = -1; for (int i = from; i <= to; ++i) { - if (i == cursorIdx) continue; - if (words[i].pageIdx != currentDisplayPage) continue; - const auto r = alignedRect(words[i].x, words[i].y, words[i].w, words[i].h); - renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); - if (words[i].text.find_first_not_of(" \t") != std::string::npos) { - renderer.drawText(fontId, words[i].x, words[i].y, words[i].text.c_str(), true); + bool skipWord = (words[i].pageIdx != currentDisplayPage); + if (skipWord) { + if (runStart >= 0) { + drawContinuousHighlight(runStart, i - 1); + runStart = -1; + } + } else if (runStart < 0 || words[i].y != words[runStart].y) { + if (runStart >= 0) { + drawContinuousHighlight(runStart, i - 1); + } + runStart = i; } } + if (runStart >= 0) { + drawContinuousHighlight(runStart, to); + } } - // Draw cursor highlight (always on top) const auto& cw = words[cursorIdx]; if (cw.pageIdx == currentDisplayPage) { - const auto r = alignedRect(cw.x, cw.y, cw.w, cw.h); - renderer.fillRectDither(r.x, r.y, r.w, r.h, Color::LightGray); - if (cw.text.find_first_not_of(" \t") != std::string::npos) { - renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true); + const auto cs = static_cast<EpdFontFamily::Style>(cw.style & ~EpdFontFamily::UNDERLINE); + const int skip = emSpaceSkip(cw.text, cs); + const int cx = cw.x + skip; + const int cWidth = cw.w - skip; + if (cWidth > 0) { + renderer.fillRectDither(cx, cw.y, cWidth, cw.h, Color::LightGray); + if (cw.text.find_first_not_of(" \t") != std::string::npos) { + if (hasEmSpace(cw.text)) { + renderer.drawText(fontId, cx, cw.y, cw.text.c_str() + 3, true, cs); + } else { + renderer.drawText(fontId, cw.x, cw.y, cw.text.c_str(), true, cs); + } + } } } } @@ -213,37 +327,26 @@ int ClipSelectionActivity::lineEndForward(int idx) const { const int total = static_cast<int>(words.size()); const int lineY = words[idx].y; const int page = words[idx].pageIdx; - - // Find last word on the same line - int last = idx; for (int i = idx + 1; i < total; ++i) { - if (words[i].pageIdx != page || words[i].y != lineY) break; - last = i; - } - - // Already at line end — jump to first word of next line - if (last == idx && idx + 1 < total) { - return idx + 1; + if (words[i].pageIdx != page || words[i].y != lineY) return i; } - - return last; + return idx; } int ClipSelectionActivity::lineEndBackward(int idx) const { const int lineY = words[idx].y; const int page = words[idx].pageIdx; - - // Find first word on the same line - int first = idx; - for (int i = idx - 1; i >= 0; --i) { + int i; + for (i = idx - 1; i >= 0; --i) { if (words[i].pageIdx != page || words[i].y != lineY) break; - first = i; } - - // Already at line start — jump to last word of previous line - if (first == idx && idx - 1 >= 0) { - return idx - 1; + if (i < 0) return idx; + const int prevY = words[i].y; + const int prevPage = words[i].pageIdx; + int first = i; + for (; i >= 0; --i) { + if (words[i].pageIdx != prevPage || words[i].y != prevY) break; + first = i; } - return first; } diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h index 4b4b63e4ab..17e6d35ab5 100644 --- a/src/activities/reader/ClipSelectionActivity.h +++ b/src/activities/reader/ClipSelectionActivity.h @@ -1,5 +1,6 @@ #pragma once +#include <EpdFontFamily.h> #include <Epub/Page.h> #include <Epub/Section.h> @@ -13,8 +14,9 @@ class ClipSelectionActivity final : public Activity { public: struct WordRef { int x, y, w, h; - int pageIdx; // 0 = current, 1 = next, 2 = page after next + int pageIdx; std::string text; + EpdFontFamily::Style style = EpdFontFamily::REGULAR; }; ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector<WordRef> words, diff --git a/src/activities/reader/ClippingsMenuActivity.cpp b/src/activities/reader/ClippingsMenuActivity.cpp index 7a49c55bd5..81c0d0c572 100644 --- a/src/activities/reader/ClippingsMenuActivity.cpp +++ b/src/activities/reader/ClippingsMenuActivity.cpp @@ -8,111 +8,93 @@ #include "components/UITheme.h" #include "fontIds.h" +namespace { +constexpr int MENU_ITEMS = 3; +const StrId menuNames[MENU_ITEMS] = {StrId::STR_CLIP_NAV_MODE, StrId::STR_ANNOT_SHOW, StrId::STR_CLIPPING_STORAGE}; +} // namespace + void ClippingsMenuActivity::onEnter() { Activity::onEnter(); + selectedIndex = 0; requestUpdate(); } void ClippingsMenuActivity::onExit() { Activity::onExit(); } -// Item 0: Navigation (clipNavMode) -// Item 1: Show Annotations (annotationVisibility) -// Item 2: Storage (clippingStorage) - -void ClippingsMenuActivity::toggleSetting(int idx) { - switch (idx) { - case 0: - SETTINGS.clipNavMode = (SETTINGS.clipNavMode + 1) % CrossPointSettings::CLIP_NAV_MODE_COUNT; - break; - case 1: - SETTINGS.annotationVisibility = (SETTINGS.annotationVisibility + 1) % 2; - break; - case 2: - SETTINGS.clippingStorage = (SETTINGS.clippingStorage + 1) % CrossPointSettings::CLIPPING_STORAGE_COUNT; - break; - default: - break; +void ClippingsMenuActivity::loop() { + if (mappedInput.wasPressed(MappedInputManager::Button::Back)) { + finish(); + return; } - SETTINGS.saveToFile(); -} -const char* ClippingsMenuActivity::settingValue(int idx) const { - switch (idx) { - case 0: - return SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE ? tr(STR_CLIP_NAV_LINE) : tr(STR_CLIP_NAV_WORD); - case 1: - return SETTINGS.annotationVisibility ? tr(STR_STATE_ON) : tr(STR_STATE_OFF); - case 2: - return SETTINGS.clippingStorage == CrossPointSettings::SINGLE_FILE ? tr(STR_CLIPPING_SINGLE_FILE) - : tr(STR_CLIPPING_PER_BOOK); - default: - return ""; + if (mappedInput.wasPressed(MappedInputManager::Button::Confirm)) { + toggleSetting(selectedIndex); + requestUpdate(); + return; } -} -void ClippingsMenuActivity::loop() { buttonNavigator.onNextRelease([this] { - selectedIndex = ButtonNavigator::nextIndex(selectedIndex, ITEM_COUNT); + selectedIndex = ButtonNavigator::nextIndex(selectedIndex, MENU_ITEMS); requestUpdate(); }); buttonNavigator.onPreviousRelease([this] { - selectedIndex = ButtonNavigator::previousIndex(selectedIndex, ITEM_COUNT); + selectedIndex = ButtonNavigator::previousIndex(selectedIndex, MENU_ITEMS); requestUpdate(); }); - if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - toggleSetting(selectedIndex); + buttonNavigator.onNextContinuous([this] { + selectedIndex = ButtonNavigator::nextIndex(selectedIndex, MENU_ITEMS); requestUpdate(); - return; - } + }); - if (mappedInput.wasReleased(MappedInputManager::Button::Back)) { - finish(); + buttonNavigator.onPreviousContinuous([this] { + selectedIndex = ButtonNavigator::previousIndex(selectedIndex, MENU_ITEMS); + requestUpdate(); + }); +} + +void ClippingsMenuActivity::toggleSetting(int idx) { + if (idx == 0) { + SETTINGS.clipNavMode = (SETTINGS.clipNavMode + 1) % CrossPointSettings::CLIP_NAV_MODE_COUNT; + } else if (idx == 1) { + SETTINGS.annotationVisibility = (SETTINGS.annotationVisibility + 1) % 2; + } else if (idx == 2) { + SETTINGS.clippingStorage = (SETTINGS.clippingStorage + 1) % CrossPointSettings::CLIPPING_STORAGE_COUNT; } + SETTINGS.saveToFile(); } void ClippingsMenuActivity::render(RenderLock&&) { renderer.clearScreen(); + auto metrics = UITheme::getInstance().getMetrics(); const auto pageWidth = renderer.getScreenWidth(); - const auto orientation = renderer.getOrientation(); - const bool isLandscapeCw = orientation == GfxRenderer::Orientation::LandscapeClockwise; - const bool isLandscapeCcw = orientation == GfxRenderer::Orientation::LandscapeCounterClockwise; - const bool isPortraitInverted = orientation == GfxRenderer::Orientation::PortraitInverted; - const int hintGutterWidth = (isLandscapeCw || isLandscapeCcw) ? 30 : 0; - const int contentX = isLandscapeCw ? hintGutterWidth : 0; - const int contentWidth = pageWidth - hintGutterWidth; - const int hintGutterHeight = isPortraitInverted ? 50 : 0; - const int contentY = hintGutterHeight; - - const int titleX = - contentX + (contentWidth - renderer.getTextWidth(UI_12_FONT_ID, tr(STR_CAT_CLIPPINGS), EpdFontFamily::BOLD)) / 2; - renderer.drawText(UI_12_FONT_ID, titleX, 15 + contentY, tr(STR_CAT_CLIPPINGS), true, EpdFontFamily::BOLD); - - static const StrId labels[ITEM_COUNT] = {StrId::STR_CLIP_NAV_MODE, StrId::STR_ANNOT_SHOW, - StrId::STR_CLIPPING_STORAGE}; - - const int startY = 60 + contentY; - constexpr int lineHeight = 32; - - for (int i = 0; i < ITEM_COUNT; ++i) { - const int displayY = startY + i * lineHeight; - const bool isSelected = (i == selectedIndex); - - if (isSelected) { - renderer.fillRect(contentX, displayY, contentWidth - 1, lineHeight, true); - } - - renderer.drawText(UI_10_FONT_ID, contentX + 20, displayY + 6, I18N.get(labels[i]), !isSelected); - - const char* val = settingValue(i); - const int valW = renderer.getTextWidth(UI_10_FONT_ID, val); - renderer.drawText(UI_10_FONT_ID, contentX + contentWidth - 20 - valW, displayY + 6, val, !isSelected); - } - - const auto hints = mappedInput.mapLabels(tr(STR_BACK), tr(STR_TOGGLE), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); - GUI.drawButtonHints(renderer, hints.btn1, hints.btn2, hints.btn3, hints.btn4); + const auto pageHeight = renderer.getScreenHeight(); + + GUI.drawHeader(renderer, Rect{0, metrics.topPadding, pageWidth, metrics.headerHeight}, tr(STR_CAT_CLIPPINGS)); + + const int contentTop = metrics.topPadding + metrics.headerHeight + metrics.verticalSpacing; + const int contentHeight = pageHeight - contentTop - metrics.buttonHintsHeight - metrics.verticalSpacing * 2; + GUI.drawList( + renderer, Rect{0, contentTop, pageWidth, contentHeight}, MENU_ITEMS, selectedIndex, + [](int index) { return std::string(I18N.get(menuNames[index])); }, nullptr, nullptr, + [this](int index) { + if (index == 0) { + return std::string(SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE ? tr(STR_CLIP_NAV_LINE) + : tr(STR_CLIP_NAV_WORD)); + } else if (index == 1) { + return std::string(SETTINGS.annotationVisibility ? tr(STR_STATE_OFF) : tr(STR_STATE_ON)); + } else if (index == 2) { + return std::string(SETTINGS.clippingStorage == CrossPointSettings::SINGLE_FILE ? tr(STR_CLIPPING_SINGLE_FILE) + : tr(STR_CLIPPING_PER_BOOK)); + } + return std::string(""); + }, + true); + + const auto labels = mappedInput.mapLabels(tr(STR_BACK), tr(STR_TOGGLE), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); + GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); renderer.displayBuffer(); } diff --git a/src/activities/reader/ClippingsMenuActivity.h b/src/activities/reader/ClippingsMenuActivity.h index 45b29fcee4..43ccbdeb7a 100644 --- a/src/activities/reader/ClippingsMenuActivity.h +++ b/src/activities/reader/ClippingsMenuActivity.h @@ -10,7 +10,6 @@ class ClippingsMenuActivity final : public Activity { static constexpr int ITEM_COUNT = 3; void toggleSetting(int idx); - const char* settingValue(int idx) const; public: explicit ClippingsMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 18464754b3..b471102558 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -194,6 +194,12 @@ void EpubReaderActivity::loop() { return; } + if (SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::CLIPPING && + mappedInput.wasReleased(MappedInputManager::Button::Power)) { + startClipSelection(); + return; + } + const auto [prevTriggered, nextTriggered, fromTilt] = ReaderUtils::detectPageTurn(mappedInput); if (!prevTriggered && !nextTriggered) { return; @@ -417,80 +423,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction break; } case EpubReaderMenuActivity::MenuAction::SAVE_CLIPPING: { - if (section && epub) { - int mTop, mRight, mBottom, mLeft; - renderer.getOrientedViewableTRBL(&mTop, &mRight, &mBottom, &mLeft); - mTop += SETTINGS.screenMargin; - mLeft += SETTINGS.screenMargin; - - const int readerFontId = SETTINGS.getReaderFontId(); - const int lineH = renderer.getLineHeight(readerFontId); - const int startPage = section->currentPage; - const int pagesToLoad = std::min(3, section->pageCount - startPage); - - std::vector<ClipSelectionActivity::WordRef> words; - words.reserve(pagesToLoad * 60); - - for (int pi = 0; pi < pagesToLoad; ++pi) { - section->currentPage = startPage + pi; - auto page = section->loadPageFromSectionFile(); - if (!page) break; - - for (const auto& el : page->elements) { - if (el->getTag() != TAG_PageLine) continue; - const auto& line = static_cast<const PageLine&>(*el); - if (!line.getBlock()) continue; - const auto& block = *line.getBlock(); - const auto& xpos = block.getWordXpos(); - const auto& wlist = block.getWords(); - - for (int i = 0; i < static_cast<int>(wlist.size()); ++i) { - const int wx = mLeft + line.xPos + xpos[i]; - const int wy = mTop + line.yPos; - const int ww = renderer.getTextWidth(readerFontId, wlist[i].c_str()); - if (ww > 0) words.push_back({wx, wy, ww, lineH, pi, wlist[i]}); - } - } - } - section->currentPage = startPage; - - if (!words.empty()) { - std::string chapterTitle; - const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); - if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; - - auto wordsCopy = words; - startActivityForResult( - std::make_unique<ClipSelectionActivity>(renderer, mappedInput, std::move(words), epub->getTitle(), - epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, - *section, startPage, mTop, mLeft), - [this, chapterTitle, startPage, wordsCopy = std::move(wordsCopy)](const ActivityResult& result) mutable { - if (!result.isCancelled) { - const auto& clip = std::get<ClippingResult>(result.data); - if (!clip.text.empty()) { - ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, - clip.text); - if (clip.fromWordIdx >= 0 && clip.toWordIdx >= 0) { - AnnotationsManager::AnnotationRecord rec; - rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); - const int to = std::min(clip.toWordIdx, static_cast<int>(wordsCopy.size()) - 1); - for (int i = clip.fromWordIdx; i <= to; ++i) { - const auto absPage = static_cast<uint16_t>(startPage + wordsCopy[i].pageIdx); - rec.rects.push_back({static_cast<int16_t>(wordsCopy[i].x), static_cast<int16_t>(wordsCopy[i].y), - static_cast<int16_t>(wordsCopy[i].w), static_cast<int16_t>(wordsCopy[i].h), - absPage}); - } - annotations.add(std::move(rec)); - annotations.save(epub->getCachePath().c_str()); - } - } - } - requestUpdate(); - }); - } else { - requestUpdate(); - } - } + startClipSelection(); break; } case EpubReaderMenuActivity::MenuAction::SYNC: { @@ -528,6 +461,83 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction } } +void EpubReaderActivity::startClipSelection() { + if (!section || !epub) { + requestUpdate(); + return; + } + + int mTop, mRight, mBottom, mLeft; + renderer.getOrientedViewableTRBL(&mTop, &mRight, &mBottom, &mLeft); + mTop += SETTINGS.screenMargin; + mLeft += SETTINGS.screenMargin; + + const int readerFontId = SETTINGS.getReaderFontId(); + const int lineH = renderer.getLineHeight(readerFontId); + const int startPage = section->currentPage; + const int pagesToLoad = std::min(3, section->pageCount - startPage); + + std::vector<ClipSelectionActivity::WordRef> words; + words.reserve(pagesToLoad * 60); + + for (int pi = 0; pi < pagesToLoad; ++pi) { + section->currentPage = startPage + pi; + auto page = section->loadPageFromSectionFile(); + if (!page) break; + + for (const auto& el : page->elements) { + if (el->getTag() != TAG_PageLine) continue; + const auto& line = static_cast<const PageLine&>(*el); + if (!line.getBlock()) continue; + const auto& block = *line.getBlock(); + const auto& xpos = block.getWordXpos(); + const auto& wlist = block.getWords(); + const auto& styles = block.getWordStyles(); + + for (int i = 0; i < static_cast<int>(wlist.size()); ++i) { + const int wx = mLeft + line.xPos + xpos[i]; + const int wy = mTop + line.yPos; + const auto s = i < static_cast<int>(styles.size()) ? styles[i] : EpdFontFamily::REGULAR; + const int ww = renderer.getTextWidth(readerFontId, wlist[i].c_str(), s); + if (ww > 0) words.push_back({wx, wy, ww, lineH, pi, wlist[i], s}); + } + } + } + section->currentPage = startPage; + + if (!words.empty()) { + std::string chapterTitle; + const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); + if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; + + startActivityForResult(std::make_unique<ClipSelectionActivity>( + renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), + chapterTitle, startPage + 1, readerFontId, *section, startPage, mTop, mLeft), + [this, chapterTitle, startPage](const ActivityResult& result) { + if (!result.isCancelled) { + const auto& clip = std::get<ClippingResult>(result.data); + if (!clip.text.empty()) { + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, + startPage + 1, clip.text); + if (!clip.rects.empty()) { + AnnotationsManager::AnnotationRecord rec; + rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); + for (const auto& r : clip.rects) { + rec.rects.push_back({r.x, r.y, r.w, r.h, r.sectionPage}); + } + annotations.add(std::move(rec)); + annotationsDirty = true; + annotations.save(epub->getCachePath().c_str()); + } + } + } + requestUpdate(); + }); + } else { + requestUpdate(); + } +} + void EpubReaderActivity::applyOrientation(const uint8_t orientation) { // No-op if the selected orientation matches current settings. if (SETTINGS.orientation == orientation) { @@ -861,8 +871,6 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or const int screenW = renderer.getScreenWidth(); const auto sectionAnnotations = annotations.forSection(static_cast<uint16_t>(currentSpineIndex)); for (const auto& rec : sectionAnnotations) { - // Group rects by row (underlineY) and draw one continuous line per row - // to avoid gaps between words caused by per-word drawing struct RowSpan { int underlineY, xMin, xMax; }; diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 9c57588c8e..4313628dff 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -53,6 +53,7 @@ class EpubReaderActivity final : public Activity { // Jump to a percentage of the book (0-100), mapping it to spine and page. void jumpToPercent(int percent); void onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction action); + void startClipSelection(); void applyOrientation(uint8_t orientation); void toggleAutoPageTurn(uint8_t selectedPageTurnOption); void pageTurn(bool isForwardTurn); diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index a15e09cc38..5ee6428b8d 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -25,7 +25,7 @@ bool AnnotationsManager::load(const char* bookCachePath) { uint16_t count = 0; if (file.read(&version, 1) != 1 || version != FILE_VERSION) { file.close(); - return true; // Unknown version — treat as empty + return true; } if (file.read(&count, sizeof(count)) != sizeof(count)) { file.close(); diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index c70b751e71..792882121e 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -36,10 +36,7 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str const std::string path = resolveClippingPath(bookTitle); if (SETTINGS.clippingStorage == CrossPointSettings::PER_BOOK) { - if (!Storage.mkdir(CLIPPINGS_DIR)) { - LOG_ERR("CLIP", "Failed to create %s directory", CLIPPINGS_DIR); - return false; - } + Storage.mkdir(CLIPPINGS_DIR); } HalFile file = Storage.open(path.c_str(), O_RDWR | O_CREAT | O_AT_END); From a1ca7d5577a935e039693f4c2ee315c4d116370f Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Sun, 3 May 2026 02:58:10 +0200 Subject: [PATCH 14/19] feat: add TextBlock.paragraphStart for reliable clipping paragraph detection - Add bool paragraphStart to TextBlock, set in extractLine() when breakIndex==0 - Serialize/deserialize paragraphStart in TextBlock binary format - Bump SECTION_FILE_VERSION 21->22 to invalidate old caches - Use block.getParagraphStart() in startClipSelection() as primary signal - Keep em-space/Y-gap/xpos-indent as fallback for cross-page boundaries - Fix punctuation spacing: use X-position proximity (gap<=2px) to detect visually attached words split by HTML tag boundaries (e.g. Title: vs Title :) - Rate-limit SL/RL lock logs to every 10000 ops - Remove CLIP diagnostic logs --- lib/Epub/Epub/ParsedText.cpp | 4 +- lib/Epub/Epub/blocks/TextBlock.cpp | 12 ++-- lib/Epub/Epub/blocks/TextBlock.h | 8 ++- lib/hal/HalStorage.cpp | 10 ++- src/activities/ActivityManager.cpp | 18 ++++-- .../reader/ClipSelectionActivity.cpp | 10 +-- src/activities/reader/ClipSelectionActivity.h | 1 + src/activities/reader/EpubReaderActivity.cpp | 62 ++++++++++--------- 8 files changed, 77 insertions(+), 48 deletions(-) diff --git a/lib/Epub/Epub/ParsedText.cpp b/lib/Epub/Epub/ParsedText.cpp index 043020d7b2..841467246f 100644 --- a/lib/Epub/Epub/ParsedText.cpp +++ b/lib/Epub/Epub/ParsedText.cpp @@ -536,6 +536,6 @@ void ParsedText::extractLine(const size_t breakIndex, const int pageWidth, const } } - processLine( - std::make_shared<TextBlock>(std::move(lineWords), std::move(lineXPos), std::move(lineWordStyles), blockStyle)); + processLine(std::make_shared<TextBlock>(std::move(lineWords), std::move(lineXPos), std::move(lineWordStyles), + blockStyle, breakIndex == 0)); } diff --git a/lib/Epub/Epub/blocks/TextBlock.cpp b/lib/Epub/Epub/blocks/TextBlock.cpp index e3e35d4282..ef9023e67b 100644 --- a/lib/Epub/Epub/blocks/TextBlock.cpp +++ b/lib/Epub/Epub/blocks/TextBlock.cpp @@ -7,8 +7,8 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int x, const int y) const { // Validate iterator bounds before rendering if (words.size() != wordXpos.size() || words.size() != wordStyles.size()) { - LOG_ERR("TXB", "Render skipped: size mismatch (words=%u, xpos=%u, styles=%u)\n", (uint32_t)words.size(), - (uint32_t)wordXpos.size(), (uint32_t)wordStyles.size()); + LOG_ERR("TXB", "Render skipped: size mismatch (words=%zu, xpos=%zu, styles=%zu)\n", words.size(), wordXpos.size(), + wordStyles.size()); return; } @@ -43,7 +43,7 @@ void TextBlock::render(const GfxRenderer& renderer, const int fontId, const int bool TextBlock::serialize(FsFile& file) const { if (words.size() != wordXpos.size() || words.size() != wordStyles.size()) { - LOG_ERR("TXB", "Serialization failed: size mismatch (words=%u, xpos=%u, styles=%u)\n", words.size(), + LOG_ERR("TXB", "Serialization failed: size mismatch (words=%zu, xpos=%zu, styles=%zu)\n", words.size(), wordXpos.size(), wordStyles.size()); return false; } @@ -68,6 +68,8 @@ bool TextBlock::serialize(FsFile& file) const { serialization::writePod(file, blockStyle.textIndent); serialization::writePod(file, blockStyle.textIndentDefined); + serialization::writePod(file, paragraphStart); + return true; } @@ -77,6 +79,7 @@ std::unique_ptr<TextBlock> TextBlock::deserialize(FsFile& file) { std::vector<int16_t> wordXpos; std::vector<EpdFontFamily::Style> wordStyles; BlockStyle blockStyle; + bool paragraphStart = false; // Word count serialization::readPod(file, wc); @@ -108,7 +111,8 @@ std::unique_ptr<TextBlock> TextBlock::deserialize(FsFile& file) { serialization::readPod(file, blockStyle.paddingRight); serialization::readPod(file, blockStyle.textIndent); serialization::readPod(file, blockStyle.textIndentDefined); + serialization::readPod(file, paragraphStart); return std::unique_ptr<TextBlock>( - new TextBlock(std::move(words), std::move(wordXpos), std::move(wordStyles), blockStyle)); + new TextBlock(std::move(words), std::move(wordXpos), std::move(wordStyles), blockStyle, paragraphStart)); } diff --git a/lib/Epub/Epub/blocks/TextBlock.h b/lib/Epub/Epub/blocks/TextBlock.h index 0da365ce09..13795730c7 100644 --- a/lib/Epub/Epub/blocks/TextBlock.h +++ b/lib/Epub/Epub/blocks/TextBlock.h @@ -16,20 +16,24 @@ class TextBlock final : public Block { std::vector<int16_t> wordXpos; std::vector<EpdFontFamily::Style> wordStyles; BlockStyle blockStyle; + bool paragraphStart = false; public: explicit TextBlock(std::vector<std::string> words, std::vector<int16_t> word_xpos, - std::vector<EpdFontFamily::Style> word_styles, const BlockStyle& blockStyle = BlockStyle()) + std::vector<EpdFontFamily::Style> word_styles, const BlockStyle& blockStyle = BlockStyle(), + bool paragraphStart = false) : words(std::move(words)), wordXpos(std::move(word_xpos)), wordStyles(std::move(word_styles)), - blockStyle(blockStyle) {} + blockStyle(blockStyle), + paragraphStart(paragraphStart) {} ~TextBlock() override = default; void setBlockStyle(const BlockStyle& blockStyle) { this->blockStyle = blockStyle; } const BlockStyle& getBlockStyle() const { return blockStyle; } const std::vector<std::string>& getWords() const { return words; } const std::vector<int16_t>& getWordXpos() const { return wordXpos; } const std::vector<EpdFontFamily::Style>& getWordStyles() const { return wordStyles; } + bool getParagraphStart() const { return paragraphStart; } bool isEmpty() override { return words.empty(); } size_t wordCount() const { return words.size(); } // given a renderer works out where to break the words into lines diff --git a/lib/hal/HalStorage.cpp b/lib/hal/HalStorage.cpp index 61f4e56b53..2c93702c8e 100644 --- a/lib/hal/HalStorage.cpp +++ b/lib/hal/HalStorage.cpp @@ -25,15 +25,21 @@ bool HalStorage::ready() const { return SDCard.ready(); } // For the rest of the methods, we acquire the mutex to ensure thread safety +static uint32_t lockLogCount = 0; + class HalStorage::StorageLock { public: StorageLock() { - LOG_DBG("LOCK", "SL take from %s", pcTaskGetName(nullptr)); + if (lockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "SL take from %s (#%u)", pcTaskGetName(nullptr), lockLogCount); + } xSemaphoreTake(HalStorage::getInstance().storageMutex, portMAX_DELAY); } ~StorageLock() { + if (lockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "SL give from %s (#%u)", pcTaskGetName(nullptr), lockLogCount); + } xSemaphoreGive(HalStorage::getInstance().storageMutex); - LOG_DBG("LOCK", "SL give from %s", pcTaskGetName(nullptr)); } }; diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index dd3e4e89fb..8101ddd3f5 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -290,9 +290,13 @@ void ActivityManager::requestUpdateAndWait() { // RenderLock +static uint32_t rlLockLogCount = 0; + RenderLock::RenderLock() { #if LOG_LEVEL >= 2 - LOG_DBG("LOCK", "RL take from %s", pcTaskGetName(nullptr)); + if (rlLockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "RL take from %s (#%u)", pcTaskGetName(nullptr), rlLockLogCount); + } #endif xSemaphoreTake(activityManager.renderingMutex, portMAX_DELAY); isLocked = true; @@ -300,7 +304,9 @@ RenderLock::RenderLock() { RenderLock::RenderLock([[maybe_unused]] Activity&) { #if LOG_LEVEL >= 2 - LOG_DBG("LOCK", "RL take from %s", pcTaskGetName(nullptr)); + if (rlLockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "RL take from %s (#%u)", pcTaskGetName(nullptr), rlLockLogCount); + } #endif xSemaphoreTake(activityManager.renderingMutex, portMAX_DELAY); isLocked = true; @@ -309,7 +315,9 @@ RenderLock::RenderLock([[maybe_unused]] Activity&) { RenderLock::~RenderLock() { if (isLocked) { #if LOG_LEVEL >= 2 - LOG_DBG("LOCK", "RL give from %s", pcTaskGetName(nullptr)); + if (rlLockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "RL give from %s (#%u)", pcTaskGetName(nullptr), rlLockLogCount); + } #endif xSemaphoreGive(activityManager.renderingMutex); isLocked = false; @@ -319,7 +327,9 @@ RenderLock::~RenderLock() { void RenderLock::unlock() { if (isLocked) { #if LOG_LEVEL >= 2 - LOG_DBG("LOCK", "RL unlock from %s", pcTaskGetName(nullptr)); + if (rlLockLogCount++ % 10000 == 0) { + LOG_DBG("LOCK", "RL unlock from %s (#%u)", pcTaskGetName(nullptr), rlLockLogCount); + } #endif xSemaphoreGive(activityManager.renderingMutex); isLocked = false; diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 0efcb2db4c..425577f294 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -161,10 +161,9 @@ void ClipSelectionActivity::loop() { }; for (int i = from; i <= to; ++i) { const auto& wtext = stripEmSpace(words[i].text); - const bool emSpaceStart = hasEmSpace(words[i].text) && i > from; - const bool yGapBreak = + const bool yGap = i > from && words[i].pageIdx == words[i - 1].pageIdx && words[i].y > words[i - 1].y + words[i - 1].h; - const bool paragraphStart = emSpaceStart || yGapBreak; + const bool paragraphStart = (i > from) && (hasEmSpace(words[i].text) || words[i].paragraphStart || yGap); if (i > from && !text.empty() && !paragraphStart) { const auto& prev = words[i - 1].text; const auto& prevStripped = stripEmSpace(prev); @@ -180,7 +179,10 @@ void ClipSelectionActivity::loop() { if (paragraphStart) { text += '\n'; } else if (!text.empty()) { - text += ' '; + const bool attached = (words[i].y == words[i - 1].y) && (words[i].x <= words[i - 1].x + words[i - 1].w + 2); + if (!attached) { + text += ' '; + } } text += wtext; pushWordRect(i, rects); diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h index 17e6d35ab5..3a2157f00c 100644 --- a/src/activities/reader/ClipSelectionActivity.h +++ b/src/activities/reader/ClipSelectionActivity.h @@ -17,6 +17,7 @@ class ClipSelectionActivity final : public Activity { int pageIdx; std::string text; EpdFontFamily::Style style = EpdFontFamily::REGULAR; + bool paragraphStart = false; }; ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector<WordRef> words, diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index b471102558..36447e72f6 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -499,43 +499,45 @@ void EpubReaderActivity::startClipSelection() { const int wy = mTop + line.yPos; const auto s = i < static_cast<int>(styles.size()) ? styles[i] : EpdFontFamily::REGULAR; const int ww = renderer.getTextWidth(readerFontId, wlist[i].c_str(), s); - if (ww > 0) words.push_back({wx, wy, ww, lineH, pi, wlist[i], s}); + if (ww > 0) { + words.push_back({wx, wy, ww, lineH, pi, wlist[i], s}); + if (i == 0 && block.getParagraphStart()) { + words.back().paragraphStart = true; + } + } } } } section->currentPage = startPage; - if (!words.empty()) { - std::string chapterTitle; - const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); - if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; - - startActivityForResult(std::make_unique<ClipSelectionActivity>( - renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), - chapterTitle, startPage + 1, readerFontId, *section, startPage, mTop, mLeft), - [this, chapterTitle, startPage](const ActivityResult& result) { - if (!result.isCancelled) { - const auto& clip = std::get<ClippingResult>(result.data); - if (!clip.text.empty()) { - ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, - startPage + 1, clip.text); - if (!clip.rects.empty()) { - AnnotationsManager::AnnotationRecord rec; - rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); - for (const auto& r : clip.rects) { - rec.rects.push_back({r.x, r.y, r.w, r.h, r.sectionPage}); - } - annotations.add(std::move(rec)); - annotationsDirty = true; - annotations.save(epub->getCachePath().c_str()); - } - } - } - requestUpdate(); - }); - } else { + if (words.empty()) { requestUpdate(); + return; } + + startActivityForResult( + std::make_unique<ClipSelectionActivity>(renderer, mappedInput, std::move(words), epub->getTitle(), + epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, *section, + startPage, mTop, mLeft), + [this, chapterTitle, startPage](const ActivityResult& result) { + if (!result.isCancelled) { + const auto& clip = std::get<ClippingResult>(result.data); + if (!clip.text.empty()) { + ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, clip.text); + if (!clip.rects.empty()) { + AnnotationsManager::AnnotationRecord rec; + rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); + for (const auto& r : clip.rects) { + rec.rects.push_back({r.x, r.y, r.w, r.h, r.sectionPage}); + } + annotations.add(std::move(rec)); + annotationsDirty = true; + annotations.save(epub->getCachePath().c_str()); + } + } + } + requestUpdate(); + }); } void EpubReaderActivity::applyOrientation(const uint8_t orientation) { From 0f34bfd9b71ea1ea96c2ec65638884efd0d28f02 Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Sun, 3 May 2026 19:38:32 +0200 Subject: [PATCH 15/19] debug: add CLIP diagnostic logs for intensive testing --- src/activities/reader/ClipSelectionActivity.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 425577f294..6542ed80a4 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -164,12 +164,17 @@ void ClipSelectionActivity::loop() { const bool yGap = i > from && words[i].pageIdx == words[i - 1].pageIdx && words[i].y > words[i - 1].y + words[i - 1].h; const bool paragraphStart = (i > from) && (hasEmSpace(words[i].text) || words[i].paragraphStart || yGap); + if (paragraphStart) { + LOG_DBG("CLIP", "NL w[%d] em=%d ps=%d yGap=%d text=%.30s", i, hasEmSpace(words[i].text), + words[i].paragraphStart, yGap, wtext.c_str()); + } if (i > from && !text.empty() && !paragraphStart) { const auto& prev = words[i - 1].text; const auto& prevStripped = stripEmSpace(prev); if (prevStripped.size() >= 1 && prevStripped.back() == '-' && !wtext.empty() && static_cast<unsigned char>(wtext[0]) >= 'a' && static_cast<unsigned char>(wtext[0]) <= 'z' && prevStripped.find('-') == prevStripped.size() - 1) { + LOG_DBG("CLIP", "MERGE w[%d] \"%.15s\"+\"%.15s\"", i - 1, prevStripped.c_str(), wtext.c_str()); text.pop_back(); text += wtext; pushWordRect(i, rects); From 6ad4df331be456ba45c1acdfe02548632b74f443 Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Thu, 7 May 2026 01:23:19 +0200 Subject: [PATCH 16/19] feat: add directional clip navigation mode (Left/Right=word, Up/Down=line) --- lib/I18n/translations/english.yaml | 4 +- src/CrossPointSettings.h | 4 +- .../reader/ClipSelectionActivity.cpp | 100 +++++++++++++----- .../reader/ClippingsMenuActivity.cpp | 4 +- src/activities/reader/EpubReaderActivity.cpp | 37 +++++++ 5 files changed, 117 insertions(+), 32 deletions(-) diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index 8417cbb81e..7232b6cfd3 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -268,8 +268,8 @@ STR_CLIPPING_STORAGE: "Storage" STR_CLIPPING_SINGLE_FILE: "Single File" STR_CLIPPING_PER_BOOK: "Per Book" STR_CLIP_NAV_MODE: "Navigation" -STR_CLIP_NAV_LINE: "Line Aware" -STR_CLIP_NAV_WORD: "Word by Word" +STR_CLIP_NAV_LINE: "Directional" +STR_CLIP_NAV_WORD: "Continuous" STR_ANNOT_SHOW: "Show Annotations" STR_DELETE: "Delete" STR_DISPLAY_QR: "Show page as QR" diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index b9e3c5686f..74f18ee91a 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -149,7 +149,7 @@ class CrossPointSettings { // Clipping storage mode enum CLIPPING_STORAGE : uint8_t { SINGLE_FILE = 0, PER_BOOK = 1, CLIPPING_STORAGE_COUNT }; // Clip selector navigation scheme - enum CLIP_NAV_MODE : uint8_t { LINE_AWARE = 0, WORD_BY_WORD = 1, CLIP_NAV_MODE_COUNT }; + enum CLIP_NAV_MODE : uint8_t { CLIP_NAV_DIRECTIONAL = 0, CLIP_NAV_CONTINUOUS = 1, CLIP_NAV_MODE_COUNT }; // Annotation underline visibility enum ANNOTATION_VISIBILITY : uint8_t { ANNOT_VISIBLE = 0, ANNOT_HIDDEN = 1, ANNOTATION_VISIBILITY_COUNT }; // Sleep screen settings @@ -220,7 +220,7 @@ class CrossPointSettings { uint8_t language = 0; // Clippings settings uint8_t clippingStorage = SINGLE_FILE; - uint8_t clipNavMode = LINE_AWARE; + uint8_t clipNavMode = CLIP_NAV_DIRECTIONAL; uint8_t annotationVisibility = ANNOT_VISIBLE; ~CrossPointSettings() = default; diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 6542ed80a4..3c7d28caae 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -68,43 +68,65 @@ void ClipSelectionActivity::onExit() { void ClipSelectionActivity::loop() { const int total = static_cast<int>(words.size()); - buttonNavigator.onNextRelease([this, total] { - if (cursorIdx + 1 >= total) return; - const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = cursorIdx + 1; - if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; - requestUpdate(); - }); - - if (SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE) { - buttonNavigator.onNextContinuous([this] { + if (SETTINGS.clipNavMode == CrossPointSettings::CLIP_NAV_DIRECTIONAL) { + using Btn = MappedInputManager::Button; + + buttonNavigator.onRelease({Btn::Left}, [this] { + if (cursorIdx == 0) return; const int prevPage = words[cursorIdx].pageIdx; - const int next = lineEndForward(cursorIdx); - if (next == cursorIdx) return; - cursorIdx = next; + cursorIdx = cursorIdx - 1; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); - } else { - buttonNavigator.onNextContinuous([this, total] { + buttonNavigator.onContinuous({Btn::Left}, [this] { + if (cursorIdx == 0) return; + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = cursorIdx - 1; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + buttonNavigator.onRelease({Btn::Right}, [this, total] { + if (cursorIdx + 1 >= total) return; + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = cursorIdx + 1; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + buttonNavigator.onContinuous({Btn::Right}, [this, total] { if (cursorIdx + 1 >= total) return; const int prevPage = words[cursorIdx].pageIdx; cursorIdx = cursorIdx + 1; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); - } - buttonNavigator.onPreviousRelease([this] { - if (cursorIdx == 0) return; - const int prevPage = words[cursorIdx].pageIdx; - cursorIdx = cursorIdx - 1; - if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; - requestUpdate(); - }); + buttonNavigator.onRelease({Btn::Down}, [this] { + const int prevPage = words[cursorIdx].pageIdx; + const int next = lineEndForward(cursorIdx); + if (next == cursorIdx) return; + cursorIdx = next; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + buttonNavigator.onContinuous({Btn::Down}, [this] { + const int prevPage = words[cursorIdx].pageIdx; + const int next = lineEndForward(cursorIdx); + if (next == cursorIdx) return; + cursorIdx = next; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); - if (SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE) { - buttonNavigator.onPreviousContinuous([this] { + buttonNavigator.onRelease({Btn::Up}, [this] { + const int prevPage = words[cursorIdx].pageIdx; + const int prev = lineEndBackward(cursorIdx); + if (prev == cursorIdx) return; + cursorIdx = prev; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + buttonNavigator.onContinuous({Btn::Up}, [this] { const int prevPage = words[cursorIdx].pageIdx; const int prev = lineEndBackward(cursorIdx); if (prev == cursorIdx) return; @@ -113,13 +135,37 @@ void ClipSelectionActivity::loop() { requestUpdate(); }); } else { - buttonNavigator.onPreviousContinuous([this] { + buttonNavigator.onNextRelease([this, total] { + if (cursorIdx + 1 >= total) return; + const int prevPage = words[cursorIdx].pageIdx; + cursorIdx = cursorIdx + 1; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + buttonNavigator.onNextContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + const int next = lineEndForward(cursorIdx); + if (next == cursorIdx) return; + cursorIdx = next; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); + + buttonNavigator.onPreviousRelease([this] { if (cursorIdx == 0) return; const int prevPage = words[cursorIdx].pageIdx; cursorIdx = cursorIdx - 1; if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; requestUpdate(); }); + buttonNavigator.onPreviousContinuous([this] { + const int prevPage = words[cursorIdx].pageIdx; + const int prev = lineEndBackward(cursorIdx); + if (prev == cursorIdx) return; + cursorIdx = prev; + if (words[cursorIdx].pageIdx != prevPage) needsPageSwitch = true; + requestUpdate(); + }); } if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { @@ -185,6 +231,8 @@ void ClipSelectionActivity::loop() { text += '\n'; } else if (!text.empty()) { const bool attached = (words[i].y == words[i - 1].y) && (words[i].x <= words[i - 1].x + words[i - 1].w + 2); + LOG_DBG("CLIP", "%s w[%d] gap=%d text=%.30s", attached ? "ATTACH" : "SEP", i, + words[i].x - (words[i - 1].x + words[i - 1].w), words[i].text.c_str()); if (!attached) { text += ' '; } diff --git a/src/activities/reader/ClippingsMenuActivity.cpp b/src/activities/reader/ClippingsMenuActivity.cpp index 81c0d0c572..4619db666c 100644 --- a/src/activities/reader/ClippingsMenuActivity.cpp +++ b/src/activities/reader/ClippingsMenuActivity.cpp @@ -81,8 +81,8 @@ void ClippingsMenuActivity::render(RenderLock&&) { [](int index) { return std::string(I18N.get(menuNames[index])); }, nullptr, nullptr, [this](int index) { if (index == 0) { - return std::string(SETTINGS.clipNavMode == CrossPointSettings::LINE_AWARE ? tr(STR_CLIP_NAV_LINE) - : tr(STR_CLIP_NAV_WORD)); + return std::string(SETTINGS.clipNavMode == CrossPointSettings::CLIP_NAV_DIRECTIONAL ? tr(STR_CLIP_NAV_LINE) + : tr(STR_CLIP_NAV_WORD)); } else if (index == 1) { return std::string(SETTINGS.annotationVisibility ? tr(STR_STATE_OFF) : tr(STR_STATE_ON)); } else if (index == 2) { diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 36447e72f6..9c7e9e2f4d 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -515,6 +515,43 @@ void EpubReaderActivity::startClipSelection() { return; } + // Mark paragraph-start words so ClipSelectionActivity can insert newlines in clipping text. + // Detection methods: + // 1. Em-space prefix (U+2003) — used when no CSS text-indent is defined + // 2. Xpos indent — first word of a line is more indented than previous line's first word + // (CSS text-indent path: indent is a pixel offset in wordXpos, not in the word text) + // Note: TextBlock pointers can't be used because TextBlock = rendered line, not paragraph. + { + auto hasEmSpace = [](const std::string& w) -> bool { + return w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && + static_cast<unsigned char>(w[2]) == 0x83; + }; + auto endsWithHyphen = [](const std::string& w) -> bool { return !w.empty() && w.back() == '-'; }; + const int indentThreshold = renderer.getLineHeight(readerFontId) / 2; + LOG_DBG("CLIP", "Words: %d, indentThreshold: %d", words.size(), indentThreshold); + int prevLineFirstIdx = -1; + for (int i = 0; i < static_cast<int>(words.size()); ++i) { + const bool isNewLine = (i == 0) || (words[i].pageIdx != words[i - 1].pageIdx) || (words[i].y != words[i - 1].y); + if (isNewLine) { + const bool byEm = hasEmSpace(words[i].text); + const bool byXpos = !byEm && prevLineFirstIdx >= 0 && + words[i].x > words[prevLineFirstIdx].x + indentThreshold && + !endsWithHyphen(words[i - 1].text); + if (byEm || byXpos) { + words[i].paragraphStart = true; + LOG_DBG("CLIP", "PS w[%d] x=%d prevX=%d reason=%s text=%.20s", i, words[i].x, + prevLineFirstIdx >= 0 ? words[prevLineFirstIdx].x : -1, byEm ? "em" : "xpos", words[i].text.c_str()); + } + prevLineFirstIdx = i; + } + LOG_DBG("CLIP", "W[%d] x=%d y=%d w=%d pg=%d ps=%d text=%.30s", i, words[i].x, words[i].y, words[i].w, + words[i].pageIdx, words[i].paragraphStart, words[i].text.c_str()); + } + } + + std::string chapterTitle; + const int tocIdx = epub->getTocIndexForSpineIndex(currentSpineIndex); + if (tocIdx >= 0) chapterTitle = epub->getTocItem(tocIdx).title; startActivityForResult( std::make_unique<ClipSelectionActivity>(renderer, mappedInput, std::move(words), epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, readerFontId, *section, From 5a472ec6fa8e3889bd68a19b0ad1fa8a77747d88 Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Thu, 7 May 2026 11:21:12 +0200 Subject: [PATCH 17/19] feat: switch annotations from rect-based to text-anchor matching Replace pixel-rect annotation storage with text anchors (start/end text, context before/after, word count, page range). This makes annotations resilient to font size changes and page reflows. Hyphenated words are merged across line breaks for accurate clip text. Add backward-compatible serialization (v4-v6) in AnnotationsManager. --- src/activities/ActivityResult.h | 12 +- .../reader/ClipSelectionActivity.cpp | 84 ++++-- src/activities/reader/EpubReaderActivity.cpp | 262 ++++++++++++++++-- src/annotations/AnnotationsManager.cpp | 59 +++- src/annotations/AnnotationsManager.h | 17 +- 5 files changed, 359 insertions(+), 75 deletions(-) diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index 0bf1549e18..50d84b4251 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -58,11 +58,13 @@ struct ClippingResult { std::string text; int fromWordIdx = -1; int toWordIdx = -1; - struct AnnotationRect { - int16_t x, y, w, h; - uint16_t sectionPage; - }; - std::vector<AnnotationRect> rects; + uint16_t sectionPage = 0; + uint16_t endSectionPage = 0; + std::string startText; + std::string endText; + std::string beforeStartText; + std::string afterEndText; + uint16_t wordCount = 0; }; using ResultVariant = diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 3c7d28caae..3fbd2b0936 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -176,28 +176,10 @@ void ClipSelectionActivity::loop() { const int from = std::min(startMarkIdx, cursorIdx); const int to = std::max(startMarkIdx, cursorIdx); std::string text; - std::vector<ClippingResult::AnnotationRect> rects; - rects.reserve(to - from + 1); - auto pushWordRect = [this](int i, std::vector<ClippingResult::AnnotationRect>& out) { - int rx = words[i].x; - int rw = words[i].w; - const auto& t = words[i].text; - if (t.size() >= 3 && static_cast<unsigned char>(t[0]) == 0xE2 && static_cast<unsigned char>(t[1]) == 0x80 && - static_cast<unsigned char>(t[2]) == 0x83) { - const auto s = static_cast<EpdFontFamily::Style>(words[i].style & ~EpdFontFamily::UNDERLINE); - const int skip = renderer.getTextAdvanceX(fontId, "\xe2\x80\x83", s); - rx += skip; - rw -= skip; - } - out.push_back({static_cast<int16_t>(rx), static_cast<int16_t>(words[i].y), static_cast<int16_t>(rw), - static_cast<int16_t>(words[i].h), static_cast<uint16_t>(startPageInSection + words[i].pageIdx)}); - }; - auto stripEmSpace = [](const std::string& w) -> const std::string& { - static thread_local std::string buf; + auto stripEmSpace = [](const std::string& w) -> std::string { if (w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && static_cast<unsigned char>(w[2]) == 0x83) { - buf = w.substr(3); - return buf; + return w.substr(3); } return w; }; @@ -205,6 +187,15 @@ void ClipSelectionActivity::loop() { return w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && static_cast<unsigned char>(w[2]) == 0x83; }; + auto stripTrailingHyphen = [](std::string w) -> std::string { + while (!w.empty() && w.back() == '-') w.pop_back(); + return w; + }; + + constexpr int ANCHOR_WORDS = 4; + std::string startAnchor; + int anchorCount = 0; + for (int i = from; i <= to; ++i) { const auto& wtext = stripEmSpace(words[i].text); const bool yGap = @@ -223,7 +214,6 @@ void ClipSelectionActivity::loop() { LOG_DBG("CLIP", "MERGE w[%d] \"%.15s\"+\"%.15s\"", i - 1, prevStripped.c_str(), wtext.c_str()); text.pop_back(); text += wtext; - pushWordRect(i, rects); continue; } } @@ -238,11 +228,59 @@ void ClipSelectionActivity::loop() { } } text += wtext; - pushWordRect(i, rects); + + if (anchorCount < ANCHOR_WORDS) { + if (!startAnchor.empty()) startAnchor += ' '; + startAnchor += stripTrailingHyphen(wtext); + anchorCount++; + } + } + + std::string endAnchorFull; + anchorCount = 0; + for (int i = to; i >= from && anchorCount < ANCHOR_WORDS; --i) { + const auto wtext = stripTrailingHyphen(stripEmSpace(words[i].text)); + if (!endAnchorFull.empty()) + endAnchorFull = wtext + ' ' + endAnchorFull; + else + endAnchorFull = wtext; + anchorCount++; } + constexpr int CONTEXT_WORDS = 3; + std::string beforeStart; + for (int i = from - 1; i >= 0 && (from - i) <= CONTEXT_WORDS; --i) { + const auto stripped = stripTrailingHyphen(stripEmSpace(words[i].text)); + if (stripped.find_first_not_of(' ') == std::string::npos) continue; + if (!beforeStart.empty()) + beforeStart = stripped + ' ' + beforeStart; + else + beforeStart = stripped; + } + std::string afterEnd; + for (int i = to + 1; i < total && (i - to) <= CONTEXT_WORDS; ++i) { + const auto stripped = stripTrailingHyphen(stripEmSpace(words[i].text)); + if (stripped.find_first_not_of(' ') == std::string::npos) continue; + if (!afterEnd.empty()) + afterEnd += ' ' + stripped; + else + afterEnd = stripped; + } + + LOG_DBG("CLIP", "Anchors: start=\"%.40s\" end=\"%.40s\" ctx=[\"%.20s\"] [\"%.20s\"] wc=%d", startAnchor.c_str(), + endAnchorFull.c_str(), beforeStart.c_str(), afterEnd.c_str(), to - from + 1); + ActivityResult result; - result.data = ClippingResult{std::move(text), from, to, std::move(rects)}; + result.data = ClippingResult{std::move(text), + from, + to, + static_cast<uint16_t>(startPageInSection + words[from].pageIdx), + static_cast<uint16_t>(startPageInSection + words[to].pageIdx), + std::move(startAnchor), + std::move(endAnchorFull), + std::move(beforeStart), + std::move(afterEnd), + static_cast<uint16_t>(to - from + 1)}; setResult(std::move(result)); finish(); } diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 9c7e9e2f4d..ea6921894e 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -14,6 +14,7 @@ #include <algorithm> #include <iterator> #include <limits> +#include <sstream> #include "ClipSelectionActivity.h" #include "CrossPointSettings.h" @@ -480,6 +481,14 @@ void EpubReaderActivity::startClipSelection() { std::vector<ClipSelectionActivity::WordRef> words; words.reserve(pagesToLoad * 60); + auto stripEmSpace = [](const std::string& w) -> std::string { + if (w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && + static_cast<unsigned char>(w[2]) == 0x83) { + return w.substr(3); + } + return w; + }; + for (int pi = 0; pi < pagesToLoad; ++pi) { section->currentPage = startPage + pi; auto page = section->loadPageFromSectionFile(); @@ -495,6 +504,7 @@ void EpubReaderActivity::startClipSelection() { const auto& styles = block.getWordStyles(); for (int i = 0; i < static_cast<int>(wlist.size()); ++i) { + if (stripEmSpace(wlist[i]).find_first_not_of(' ') == std::string::npos) continue; const int wx = mLeft + line.xPos + xpos[i]; const int wy = mTop + line.yPos; const auto s = i < static_cast<int>(styles.size()) ? styles[i] : EpdFontFamily::REGULAR; @@ -559,14 +569,23 @@ void EpubReaderActivity::startClipSelection() { [this, chapterTitle, startPage](const ActivityResult& result) { if (!result.isCancelled) { const auto& clip = std::get<ClippingResult>(result.data); + LOG_DBG( + "ANNOT", + "Clip result: text=%.40s start=\"%.20s\" end=\"%.20s\" sp=%d esp=%d wc=%d ctx=[\"%.20s\"] [\"%.20s\"]", + clip.text.c_str(), clip.startText.c_str(), clip.endText.c_str(), clip.sectionPage, clip.endSectionPage, + clip.wordCount, clip.beforeStartText.c_str(), clip.afterEndText.c_str()); if (!clip.text.empty()) { ClippingsManager::saveClipping(epub->getTitle(), epub->getAuthor(), chapterTitle, startPage + 1, clip.text); - if (!clip.rects.empty()) { + if (!clip.startText.empty() && !clip.endText.empty()) { AnnotationsManager::AnnotationRecord rec; rec.sectionIdx = static_cast<uint16_t>(currentSpineIndex); - for (const auto& r : clip.rects) { - rec.rects.push_back({r.x, r.y, r.w, r.h, r.sectionPage}); - } + rec.sectionPage = clip.sectionPage; + rec.endSectionPage = clip.endSectionPage; + rec.wordCount = clip.wordCount; + rec.startText = clip.startText; + rec.endText = clip.endText; + rec.beforeStartText = clip.beforeStartText; + rec.afterEndText = clip.afterEndText; annotations.add(std::move(rec)); annotationsDirty = true; annotations.save(epub->getCachePath().c_str()); @@ -909,28 +928,225 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or const int screenH = renderer.getScreenHeight(); const int screenW = renderer.getScreenWidth(); const auto sectionAnnotations = annotations.forSection(static_cast<uint16_t>(currentSpineIndex)); - for (const auto& rec : sectionAnnotations) { - struct RowSpan { - int underlineY, xMin, xMax; + + if (!sectionAnnotations.empty()) { + auto stripEmSpace = [](const std::string& w) -> std::string { + if (w.size() >= 3 && static_cast<unsigned char>(w[0]) == 0xE2 && static_cast<unsigned char>(w[1]) == 0x80 && + static_cast<unsigned char>(w[2]) == 0x83) { + return w.substr(3); + } + return w; }; - std::vector<RowSpan> spans; - spans.reserve(rec.rects.size()); - for (const auto& r : rec.rects) { - if (r.sectionPage != static_cast<uint16_t>(section->currentPage)) continue; - const int underlineY = r.y + r.h; - if (underlineY < 0 || underlineY >= screenH) continue; - if (r.x < 0 || r.x >= screenW) continue; - auto it = std::find_if(spans.begin(), spans.end(), - [underlineY](const RowSpan& s) { return s.underlineY == underlineY; }); - if (it != spans.end()) { - if (r.x < it->xMin) it->xMin = r.x; - if (r.x + r.w - 1 > it->xMax) it->xMax = r.x + r.w - 1; - } else { - spans.push_back({underlineY, r.x, r.x + r.w - 1}); + auto stripTrailingHyphen = [](std::string w) -> std::string { + while (!w.empty() && w.back() == '-') w.pop_back(); + return w; + }; + + struct PageWord { + int x, w, y, h; + std::string text; + }; + std::vector<PageWord> pageWords; + const int fontId = SETTINGS.getReaderFontId(); + + for (const auto& el : page->elements) { + if (el->getTag() != TAG_PageLine) continue; + const auto& line = static_cast<const PageLine&>(*el); + if (!line.getBlock()) continue; + const auto& block = *line.getBlock(); + const auto& wlist = block.getWords(); + const auto& xpos = block.getWordXpos(); + const int lineH = renderer.getLineHeight(fontId); + const int wy = orientedMarginTop + line.yPos; + for (int i = 0; i < static_cast<int>(wlist.size()); ++i) { + const auto& rawText = wlist[i]; + const auto s = + i < static_cast<int>(block.getWordStyles().size()) ? block.getWordStyles()[i] : EpdFontFamily::REGULAR; + const bool hasEm = rawText.size() >= 3 && static_cast<unsigned char>(rawText[0]) == 0xE2 && + static_cast<unsigned char>(rawText[1]) == 0x80 && + static_cast<unsigned char>(rawText[2]) == 0x83; + const int emSkip = hasEm ? renderer.getTextAdvanceX(fontId, "\xe2\x80\x83", s) : 0; + const int wordW = renderer.getTextWidth(fontId, rawText.c_str(), s) - emSkip; + const auto stripped = stripTrailingHyphen(stripEmSpace(rawText)); + if (stripped.find_first_not_of(' ') == std::string::npos) continue; + pageWords.push_back({orientedMarginLeft + line.xPos + xpos[i] + emSkip, wordW, wy, lineH, stripped}); } } - for (const auto& span : spans) { - renderer.drawLine(span.xMin, span.underlineY, span.xMax, span.underlineY, 2, true); + + auto findAnchorStart = [&pageWords](const std::string& anchor) -> int { + if (anchor.empty() || pageWords.empty()) return -1; + std::vector<std::string> tokens; + std::istringstream iss(anchor); + std::string tok; + while (iss >> tok) tokens.push_back(tok); + if (tokens.empty()) return -1; + + for (int start = 0; start <= static_cast<int>(pageWords.size()) - static_cast<int>(tokens.size()); ++start) { + bool match = true; + for (int t = 0; t < static_cast<int>(tokens.size()); ++t) { + if (pageWords[start + t].text != tokens[t]) { + match = false; + break; + } + } + if (match) return start; + } + return -1; + }; + + auto findAnchorEnd = [&pageWords](const std::string& anchor) -> int { + if (anchor.empty() || pageWords.empty()) return -1; + std::vector<std::string> tokens; + std::istringstream iss(anchor); + std::string tok; + while (iss >> tok) tokens.push_back(tok); + if (tokens.empty()) return -1; + const int tsize = static_cast<int>(tokens.size()); + + for (int end = pageWords.size() - 1; end >= tsize - 1; --end) { + bool match = true; + for (int t = 0; t < tsize; ++t) { + if (pageWords[end - tsize + 1 + t].text != tokens[t]) { + match = false; + break; + } + } + if (match) return end; + } + return -1; + }; + + auto findContextStart = [&pageWords](const std::string& context) -> int { + if (context.empty() || pageWords.empty()) return -1; + std::vector<std::string> tokens; + std::istringstream iss(context); + std::string tok; + while (iss >> tok) tokens.push_back(tok); + if (tokens.empty()) return -1; + const int tsize = static_cast<int>(tokens.size()); + for (int i = 0; i <= static_cast<int>(pageWords.size()) - tsize; ++i) { + bool match = true; + for (int t = 0; t < tsize; ++t) { + if (pageWords[i + t].text != tokens[t]) { + match = false; + break; + } + } + if (match) return i + tsize; + } + return -1; + }; + + auto findContextEnd = [&pageWords](const std::string& context) -> int { + if (context.empty() || pageWords.empty()) return -1; + std::vector<std::string> tokens; + std::istringstream iss(context); + std::string tok; + while (iss >> tok) tokens.push_back(tok); + if (tokens.empty()) return -1; + const int tsize = static_cast<int>(tokens.size()); + for (int i = 0; i <= static_cast<int>(pageWords.size()) - tsize; ++i) { + bool match = true; + for (int t = 0; t < tsize; ++t) { + if (pageWords[i + t].text != tokens[t]) { + match = false; + break; + } + } + if (match) return i - 1; + } + return -1; + }; + + LOG_DBG("ANNOT", "Page %d: %zu annotations, %zu pageWords", section->currentPage, sectionAnnotations.size(), + pageWords.size()); + + for (const auto& rec : sectionAnnotations) { + const auto curPage = static_cast<uint16_t>(section->currentPage); + if (curPage < rec.sectionPage || curPage > rec.endSectionPage) continue; + + const bool isStartPage = (curPage == rec.sectionPage); + const bool isEndPage = (curPage == rec.endSectionPage); + const bool isSinglePage = (isStartPage && isEndPage); + + int startIdx = -1; + int endIdx = -1; + + if (isSinglePage) { + startIdx = findContextStart(rec.beforeStartText); + if (startIdx >= 0 && rec.wordCount > 0) { + endIdx = startIdx + rec.wordCount - 1; + if (endIdx >= static_cast<int>(pageWords.size())) endIdx = static_cast<int>(pageWords.size()) - 1; + } + if (startIdx < 0) startIdx = findAnchorStart(rec.startText); + if (startIdx >= 0 && endIdx < 0) endIdx = findAnchorEnd(rec.endText); + if (startIdx < 0) continue; + if (endIdx < 0 || endIdx < startIdx) continue; + LOG_DBG("ANNOT", " rec sp=%d-%d ctx=[%.15s/%.15s] wc=%d -> %d..%d", rec.sectionPage, rec.endSectionPage, + rec.beforeStartText.c_str(), rec.afterEndText.c_str(), rec.wordCount, startIdx, endIdx); + } else if (isStartPage) { + startIdx = findContextStart(rec.beforeStartText); + if (startIdx < 0) { + std::string anchor = rec.startText; + while (!anchor.empty()) { + startIdx = findAnchorStart(anchor); + if (startIdx >= 0) break; + auto pos = anchor.rfind(' '); + if (pos == std::string::npos) break; + anchor.erase(pos); + } + } + LOG_DBG("ANNOT", " rec sp=%d-%d (start) ctx=[%.15s] -> idx=%d", rec.sectionPage, rec.endSectionPage, + rec.beforeStartText.c_str(), startIdx); + if (startIdx < 0) continue; + endIdx = static_cast<int>(pageWords.size()) - 1; + } else if (isEndPage) { + endIdx = findContextEnd(rec.afterEndText); + if (endIdx < 0) { + std::string anchor = rec.endText; + while (!anchor.empty()) { + endIdx = findAnchorEnd(anchor); + if (endIdx >= 0) break; + auto pos = anchor.find(' '); + if (pos == std::string::npos) break; + anchor.erase(0, pos + 1); + } + } + LOG_DBG("ANNOT", " rec sp=%d-%d (end) ctx=[%.15s] -> idx=%d", rec.sectionPage, rec.endSectionPage, + rec.afterEndText.c_str(), endIdx); + if (endIdx < 0) continue; + startIdx = 0; + } else { + startIdx = 0; + endIdx = static_cast<int>(pageWords.size()) - 1; + LOG_DBG("ANNOT", " rec sp=%d-%d (middle) full page", rec.sectionPage, rec.endSectionPage); + } + + struct RowSpan { + int underlineY, xMin, xMax; + }; + std::vector<RowSpan> spans; + spans.reserve(endIdx - startIdx + 1); + + for (int i = startIdx; i <= endIdx; ++i) { + const auto& pw = pageWords[i]; + if (pw.text.empty()) continue; + const int underlineY = pw.y + pw.h; + if (underlineY < 0 || underlineY >= screenH) continue; + if (pw.x < 0 || pw.x >= screenW) continue; + auto it = std::find_if(spans.begin(), spans.end(), + [underlineY](const RowSpan& s) { return s.underlineY == underlineY; }); + if (it != spans.end()) { + if (pw.x < it->xMin) it->xMin = pw.x; + if (pw.x + pw.w - 1 > it->xMax) it->xMax = pw.x + pw.w - 1; + } else { + spans.push_back({underlineY, pw.x, pw.x + pw.w - 1}); + } + } + + for (const auto& span : spans) { + renderer.drawLine(span.xMin, span.underlineY, span.xMax, span.underlineY, 2, true); + } } } } diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index 5ee6428b8d..3b6a9e4d64 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -12,18 +12,37 @@ std::string AnnotationsManager::annotationsPath(const char* bookCachePath) { return std::string(bookCachePath) + ANNOT_FILENAME; } +static bool readString(HalFile& file, std::string& out) { + uint16_t len = 0; + if (file.read(&len, sizeof(len)) != sizeof(len)) return false; + if (len == 0) { + out.clear(); + return true; + } + out.resize(len); + return file.read(&out[0], len) == len; +} + +static void writeString(HalFile& file, const std::string& s) { + const uint16_t len = static_cast<uint16_t>(s.size()); + file.write(&len, sizeof(len)); + if (len > 0) { + file.write(s.c_str(), len); + } +} + bool AnnotationsManager::load(const char* bookCachePath) { records.clear(); const std::string path = annotationsPath(bookCachePath); HalFile file; if (!Storage.openFileForRead("ANNOT", path.c_str(), file)) { - return true; // No annotations file yet — not an error + return true; } uint8_t version = 0; uint16_t count = 0; - if (file.read(&version, 1) != 1 || version != FILE_VERSION) { + if (file.read(&version, 1) != 1 || version < 4 || version > FILE_VERSION) { file.close(); return true; } @@ -36,18 +55,26 @@ bool AnnotationsManager::load(const char* bookCachePath) { for (uint16_t i = 0; i < count; ++i) { AnnotationRecord rec; if (file.read(&rec.sectionIdx, sizeof(rec.sectionIdx)) != sizeof(rec.sectionIdx)) break; - - uint16_t rectCount = 0; - if (file.read(&rectCount, sizeof(rectCount)) != sizeof(rectCount)) break; - rec.rects.resize(rectCount); - for (uint16_t r = 0; r < rectCount; ++r) { - if (file.read(&rec.rects[r], sizeof(Rect)) != sizeof(Rect)) goto done; + if (file.read(&rec.sectionPage, sizeof(rec.sectionPage)) != sizeof(rec.sectionPage)) break; + if (version >= 5) { + if (file.read(&rec.endSectionPage, sizeof(rec.endSectionPage)) != sizeof(rec.endSectionPage)) break; + } else { + rec.endSectionPage = rec.sectionPage; + } + if (version >= 6) { + if (file.read(&rec.wordCount, sizeof(rec.wordCount)) != sizeof(rec.wordCount)) break; + } else { + rec.wordCount = 0; + } + if (!readString(file, rec.startText)) break; + if (!readString(file, rec.endText)) break; + if (version >= 6) { + if (!readString(file, rec.beforeStartText)) break; + if (!readString(file, rec.afterEndText)) break; } - records.push_back(std::move(rec)); } -done: file.close(); LOG_DBG("ANNOT", "Loaded %zu annotations from %s", records.size(), path.c_str()); return true; @@ -69,11 +96,13 @@ bool AnnotationsManager::save(const char* bookCachePath) const { for (const auto& rec : records) { file.write(&rec.sectionIdx, sizeof(rec.sectionIdx)); - const uint16_t rectCount = static_cast<uint16_t>(rec.rects.size()); - file.write(&rectCount, sizeof(rectCount)); - for (const auto& r : rec.rects) { - file.write(&r, sizeof(Rect)); - } + file.write(&rec.sectionPage, sizeof(rec.sectionPage)); + file.write(&rec.endSectionPage, sizeof(rec.endSectionPage)); + file.write(&rec.wordCount, sizeof(rec.wordCount)); + writeString(file, rec.startText); + writeString(file, rec.endText); + writeString(file, rec.beforeStartText); + writeString(file, rec.afterEndText); } file.flush(); diff --git a/src/annotations/AnnotationsManager.h b/src/annotations/AnnotationsManager.h index 3f59d39853..421b557c13 100644 --- a/src/annotations/AnnotationsManager.h +++ b/src/annotations/AnnotationsManager.h @@ -6,19 +6,18 @@ class AnnotationsManager { public: - struct Rect { - int16_t x, y, w, h; - uint16_t sectionPage; // absolute page index within section - }; - struct AnnotationRecord { uint16_t sectionIdx; - std::vector<Rect> rects; + uint16_t sectionPage; + uint16_t endSectionPage; + uint16_t wordCount; + std::string startText; + std::string endText; + std::string beforeStartText; + std::string afterEndText; }; - // Load annotations from .crosspoint/epub_<hash>/annotations.bin bool load(const char* bookCachePath); - // Save current annotations to disk bool save(const char* bookCachePath) const; void add(AnnotationRecord record); @@ -29,7 +28,7 @@ class AnnotationsManager { size_t size() const { return records.size(); } private: - static constexpr uint8_t FILE_VERSION = 3; + static constexpr uint8_t FILE_VERSION = 6; std::vector<AnnotationRecord> records; From 593d106e2835c9fe2f52a0e6fdf779ef99071a01 Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Thu, 7 May 2026 16:56:43 +0200 Subject: [PATCH 18/19] feat: add midText anchor for multi-page annotations and fix wordCount font-size issue - Add midText field to ClippingResult and AnnotationRecord (captures 4 words from center of selection) for reliable matching on intermediate pages - Add midText fallback in annotation rendering when start/end anchors don't match on intermediate pages of multi-page selections - Remove wordCount-based endIdx calculation that caused extra words to be highlighted when font size changed (hyphenation alters word count) - Bump AnnotationsManager serialization to v7 with midText (backward compatible) - Simplify annotation rendering: unified logic replaces per-page-type branches --- src/activities/ActivityResult.h | 1 + .../reader/ClipSelectionActivity.cpp | 14 ++++ src/activities/reader/EpubReaderActivity.cpp | 82 +++++++------------ src/annotations/AnnotationsManager.cpp | 4 + src/annotations/AnnotationsManager.h | 3 +- 5 files changed, 51 insertions(+), 53 deletions(-) diff --git a/src/activities/ActivityResult.h b/src/activities/ActivityResult.h index 50d84b4251..d879110d4b 100644 --- a/src/activities/ActivityResult.h +++ b/src/activities/ActivityResult.h @@ -64,6 +64,7 @@ struct ClippingResult { std::string endText; std::string beforeStartText; std::string afterEndText; + std::string midText; uint16_t wordCount = 0; }; diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 3fbd2b0936..71ad6ad628 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -266,6 +266,19 @@ void ClipSelectionActivity::loop() { else afterEnd = stripped; } + std::string midText; + { + constexpr int MID_WORDS = 4; + int midStart = (from + to) / 2 - (MID_WORDS / 2); + int midEnd = midStart + MID_WORDS; + if (midStart < from) midStart = from; + if (midEnd > to) midEnd = to; + for (int i = midStart; i <= midEnd; ++i) { + const auto wtext = stripTrailingHyphen(stripEmSpace(words[i].text)); + if (!midText.empty()) midText += ' '; + midText += wtext; + } + } LOG_DBG("CLIP", "Anchors: start=\"%.40s\" end=\"%.40s\" ctx=[\"%.20s\"] [\"%.20s\"] wc=%d", startAnchor.c_str(), endAnchorFull.c_str(), beforeStart.c_str(), afterEnd.c_str(), to - from + 1); @@ -280,6 +293,7 @@ void ClipSelectionActivity::loop() { std::move(endAnchorFull), std::move(beforeStart), std::move(afterEnd), + std::move(midText), static_cast<uint16_t>(to - from + 1)}; setResult(std::move(result)); finish(); diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index ea6921894e..f273f1ec4a 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -586,6 +586,7 @@ void EpubReaderActivity::startClipSelection() { rec.endText = clip.endText; rec.beforeStartText = clip.beforeStartText; rec.afterEndText = clip.afterEndText; + rec.midText = clip.midText; annotations.add(std::move(rec)); annotationsDirty = true; annotations.save(epub->getCachePath().c_str()); @@ -1062,65 +1063,42 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or pageWords.size()); for (const auto& rec : sectionAnnotations) { - const auto curPage = static_cast<uint16_t>(section->currentPage); - if (curPage < rec.sectionPage || curPage > rec.endSectionPage) continue; - - const bool isStartPage = (curPage == rec.sectionPage); - const bool isEndPage = (curPage == rec.endSectionPage); - const bool isSinglePage = (isStartPage && isEndPage); - int startIdx = -1; int endIdx = -1; - if (isSinglePage) { - startIdx = findContextStart(rec.beforeStartText); - if (startIdx >= 0 && rec.wordCount > 0) { - endIdx = startIdx + rec.wordCount - 1; - if (endIdx >= static_cast<int>(pageWords.size())) endIdx = static_cast<int>(pageWords.size()) - 1; - } - if (startIdx < 0) startIdx = findAnchorStart(rec.startText); - if (startIdx >= 0 && endIdx < 0) endIdx = findAnchorEnd(rec.endText); - if (startIdx < 0) continue; - if (endIdx < 0 || endIdx < startIdx) continue; - LOG_DBG("ANNOT", " rec sp=%d-%d ctx=[%.15s/%.15s] wc=%d -> %d..%d", rec.sectionPage, rec.endSectionPage, - rec.beforeStartText.c_str(), rec.afterEndText.c_str(), rec.wordCount, startIdx, endIdx); - } else if (isStartPage) { - startIdx = findContextStart(rec.beforeStartText); - if (startIdx < 0) { - std::string anchor = rec.startText; - while (!anchor.empty()) { - startIdx = findAnchorStart(anchor); - if (startIdx >= 0) break; - auto pos = anchor.rfind(' '); - if (pos == std::string::npos) break; - anchor.erase(pos); - } - } - LOG_DBG("ANNOT", " rec sp=%d-%d (start) ctx=[%.15s] -> idx=%d", rec.sectionPage, rec.endSectionPage, - rec.beforeStartText.c_str(), startIdx); - if (startIdx < 0) continue; - endIdx = static_cast<int>(pageWords.size()) - 1; - } else if (isEndPage) { - endIdx = findContextEnd(rec.afterEndText); - if (endIdx < 0) { - std::string anchor = rec.endText; - while (!anchor.empty()) { - endIdx = findAnchorEnd(anchor); - if (endIdx >= 0) break; - auto pos = anchor.find(' '); - if (pos == std::string::npos) break; - anchor.erase(0, pos + 1); - } + startIdx = findContextStart(rec.beforeStartText); + if (startIdx < 0) startIdx = findAnchorStart(rec.startText); + if (endIdx < 0) endIdx = findAnchorEnd(rec.endText); + + if (endIdx < 0) { + std::string suffix = rec.endText; + while (!suffix.empty()) { + endIdx = findAnchorEnd(suffix); + if (endIdx >= 0) break; + auto pos = suffix.find(' '); + if (pos == std::string::npos) break; + suffix.erase(0, pos + 1); } - LOG_DBG("ANNOT", " rec sp=%d-%d (end) ctx=[%.15s] -> idx=%d", rec.sectionPage, rec.endSectionPage, - rec.afterEndText.c_str(), endIdx); - if (endIdx < 0) continue; - startIdx = 0; - } else { + } + if (endIdx < 0) { + int ctxEnd = findContextEnd(rec.afterEndText); + if (ctxEnd >= 0) endIdx = ctxEnd; + } + + if (startIdx < 0 && endIdx >= 0) { startIdx = 0; + } else if (startIdx >= 0 && endIdx < 0) { endIdx = static_cast<int>(pageWords.size()) - 1; - LOG_DBG("ANNOT", " rec sp=%d-%d (middle) full page", rec.sectionPage, rec.endSectionPage); + } else if (startIdx < 0 && endIdx < 0 && !rec.midText.empty()) { + int midIdx = findAnchorStart(rec.midText); + if (midIdx >= 0) { + startIdx = 0; + endIdx = static_cast<int>(pageWords.size()) - 1; + } } + if (startIdx < 0) continue; + LOG_DBG("ANNOT", " rec sp=%d-%d ctx=[%.15s/%.15s] wc=%d -> %d..%d", rec.sectionPage, rec.endSectionPage, + rec.beforeStartText.c_str(), rec.afterEndText.c_str(), rec.wordCount, startIdx, endIdx); struct RowSpan { int underlineY, xMin, xMax; diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index 3b6a9e4d64..933ce29059 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -72,6 +72,9 @@ bool AnnotationsManager::load(const char* bookCachePath) { if (!readString(file, rec.beforeStartText)) break; if (!readString(file, rec.afterEndText)) break; } + if (version >= 7) { + if (!readString(file, rec.midText)) break; + } records.push_back(std::move(rec)); } @@ -103,6 +106,7 @@ bool AnnotationsManager::save(const char* bookCachePath) const { writeString(file, rec.endText); writeString(file, rec.beforeStartText); writeString(file, rec.afterEndText); + writeString(file, rec.midText); } file.flush(); diff --git a/src/annotations/AnnotationsManager.h b/src/annotations/AnnotationsManager.h index 421b557c13..4d2f7797eb 100644 --- a/src/annotations/AnnotationsManager.h +++ b/src/annotations/AnnotationsManager.h @@ -15,6 +15,7 @@ class AnnotationsManager { std::string endText; std::string beforeStartText; std::string afterEndText; + std::string midText; }; bool load(const char* bookCachePath); @@ -28,7 +29,7 @@ class AnnotationsManager { size_t size() const { return records.size(); } private: - static constexpr uint8_t FILE_VERSION = 6; + static constexpr uint8_t FILE_VERSION = 7; std::vector<AnnotationRecord> records; From e02bc4cce2b1f7fa3027a4de56a4c833a20d1a6e Mon Sep 17 00:00:00 2001 From: pablohc <pablonoviello@outlook.com> Date: Thu, 7 May 2026 20:45:08 +0200 Subject: [PATCH 19/19] fix: CodeRabbit review fixes and page-range filter for annotations 8 fixes from CodeRabbit review: init uint16_t fields to 0, remove unused alignedRect/struct Rect/initialRender, remove dead endIdx guard, fix midText off-by-one, reset annotationsDirty after eager save, replace malloc with std::string in ClippingsManager, consistent load() return, accept accents/uppercase in hyphen merge. Also re-introduce page-range filter to prevent false positives from context matching on wrong pages. --- .../reader/ClipSelectionActivity.cpp | 11 ++----- src/activities/reader/ClipSelectionActivity.h | 6 ---- src/activities/reader/EpubReaderActivity.cpp | 6 +++- src/annotations/AnnotationsManager.cpp | 2 +- src/annotations/AnnotationsManager.h | 8 ++--- src/clippings/ClippingsManager.cpp | 30 +++++-------------- 6 files changed, 21 insertions(+), 42 deletions(-) diff --git a/src/activities/reader/ClipSelectionActivity.cpp b/src/activities/reader/ClipSelectionActivity.cpp index 71ad6ad628..e7e6ac6f9e 100644 --- a/src/activities/reader/ClipSelectionActivity.cpp +++ b/src/activities/reader/ClipSelectionActivity.cpp @@ -209,7 +209,8 @@ void ClipSelectionActivity::loop() { const auto& prev = words[i - 1].text; const auto& prevStripped = stripEmSpace(prev); if (prevStripped.size() >= 1 && prevStripped.back() == '-' && !wtext.empty() && - static_cast<unsigned char>(wtext[0]) >= 'a' && static_cast<unsigned char>(wtext[0]) <= 'z' && + !std::isspace(static_cast<unsigned char>(wtext[0])) && + !std::ispunct(static_cast<unsigned char>(wtext[0])) && prevStripped.find('-') == prevStripped.size() - 1) { LOG_DBG("CLIP", "MERGE w[%d] \"%.15s\"+\"%.15s\"", i - 1, prevStripped.c_str(), wtext.c_str()); text.pop_back(); @@ -270,7 +271,7 @@ void ClipSelectionActivity::loop() { { constexpr int MID_WORDS = 4; int midStart = (from + to) / 2 - (MID_WORDS / 2); - int midEnd = midStart + MID_WORDS; + int midEnd = midStart + MID_WORDS - 1; if (midStart < from) midStart = from; if (midEnd > to) midEnd = to; for (int i = midStart; i <= midEnd; ++i) { @@ -424,12 +425,6 @@ void ClipSelectionActivity::drawHighlights() { } } -ClipSelectionActivity::Rect ClipSelectionActivity::alignedRect(int x, int y, int w, int h) const { - const int alignedX = (x / 8) * 8; - const int alignedW = ((x + w + 7) / 8) * 8 - alignedX; - return {alignedX, y, alignedW, h}; -} - int ClipSelectionActivity::lineEndForward(int idx) const { const int total = static_cast<int>(words.size()); const int lineY = words[idx].y; diff --git a/src/activities/reader/ClipSelectionActivity.h b/src/activities/reader/ClipSelectionActivity.h index 3a2157f00c..392d84ca1e 100644 --- a/src/activities/reader/ClipSelectionActivity.h +++ b/src/activities/reader/ClipSelectionActivity.h @@ -31,10 +31,6 @@ class ClipSelectionActivity final : public Activity { bool isReaderActivity() const override { return true; } private: - struct Rect { - int x, y, w, h; - }; - std::vector<WordRef> words; std::string bookTitle; std::string author; @@ -55,11 +51,9 @@ class ClipSelectionActivity final : public Activity { int cursorIdx = 0; int startMarkIdx = -1; bool needsPageSwitch = false; - bool initialRender = true; ButtonNavigator buttonNavigator; - Rect alignedRect(int x, int y, int w, int h) const; void switchToPage(int pageIdx); void drawHighlights(); int lineEndForward(int idx) const; diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index f273f1ec4a..a037be3498 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -590,6 +590,7 @@ void EpubReaderActivity::startClipSelection() { annotations.add(std::move(rec)); annotationsDirty = true; annotations.save(epub->getCachePath().c_str()); + annotationsDirty = false; } } } @@ -1063,12 +1064,15 @@ void EpubReaderActivity::renderContents(std::unique_ptr<Page> page, const int or pageWords.size()); for (const auto& rec : sectionAnnotations) { + const auto curPage = static_cast<uint16_t>(section->currentPage); + if (curPage < rec.sectionPage || curPage > rec.endSectionPage) continue; + int startIdx = -1; int endIdx = -1; startIdx = findContextStart(rec.beforeStartText); if (startIdx < 0) startIdx = findAnchorStart(rec.startText); - if (endIdx < 0) endIdx = findAnchorEnd(rec.endText); + endIdx = findAnchorEnd(rec.endText); if (endIdx < 0) { std::string suffix = rec.endText; diff --git a/src/annotations/AnnotationsManager.cpp b/src/annotations/AnnotationsManager.cpp index 933ce29059..f394952a92 100644 --- a/src/annotations/AnnotationsManager.cpp +++ b/src/annotations/AnnotationsManager.cpp @@ -48,7 +48,7 @@ bool AnnotationsManager::load(const char* bookCachePath) { } if (file.read(&count, sizeof(count)) != sizeof(count)) { file.close(); - return false; + return true; } records.reserve(count); diff --git a/src/annotations/AnnotationsManager.h b/src/annotations/AnnotationsManager.h index 4d2f7797eb..1e1d452d8b 100644 --- a/src/annotations/AnnotationsManager.h +++ b/src/annotations/AnnotationsManager.h @@ -7,10 +7,10 @@ class AnnotationsManager { public: struct AnnotationRecord { - uint16_t sectionIdx; - uint16_t sectionPage; - uint16_t endSectionPage; - uint16_t wordCount; + uint16_t sectionIdx = 0; + uint16_t sectionPage = 0; + uint16_t endSectionPage = 0; + uint16_t wordCount = 0; std::string startText; std::string endText; std::string beforeStartText; diff --git a/src/clippings/ClippingsManager.cpp b/src/clippings/ClippingsManager.cpp index 792882121e..f9ae3103e6 100644 --- a/src/clippings/ClippingsManager.cpp +++ b/src/clippings/ClippingsManager.cpp @@ -56,30 +56,16 @@ bool ClippingsManager::saveClipping(const std::string& bookTitle, const std::str static constexpr size_t MAX_TEXT = 2000; const size_t textLen = selectedText.size() < MAX_TEXT ? selectedText.size() : MAX_TEXT; - // Concatenate into a single buffer and perform one write to avoid partial records on SD failure static constexpr char separator[] = "\n==========\n"; - static constexpr size_t separatorLen = sizeof(separator) - 1; - const size_t totalLen = header.size() + location.size() + 1 + textLen + separatorLen; + std::string buf; + buf.reserve(header.size() + location.size() + 1 + textLen + sizeof(separator) - 1); + buf += header; + buf += location; + buf += '\n'; + buf.append(selectedText.c_str(), textLen); + buf += separator; - auto* buf = static_cast<char*>(malloc(totalLen)); - if (!buf) { - LOG_ERR("CLIP", "malloc failed: %zu bytes", totalLen); - file.close(); - return false; - } - - size_t pos = 0; - memcpy(buf + pos, header.c_str(), header.size()); - pos += header.size(); - memcpy(buf + pos, location.c_str(), location.size()); - pos += location.size(); - buf[pos++] = '\n'; - memcpy(buf + pos, selectedText.c_str(), textLen); - pos += textLen; - memcpy(buf + pos, separator, separatorLen); - - const bool ok = file.write(buf, totalLen) == totalLen; - free(buf); + const bool ok = file.write(buf.data(), buf.size()) == buf.size(); file.flush(); file.close();