Add clipping support with CrossPoint PR #1742 adaptations#41
Draft
Louieza23 wants to merge 24 commits into
Draft
Add clipping support with CrossPoint PR #1742 adaptations#41Louieza23 wants to merge 24 commits into
Louieza23 wants to merge 24 commits into
Conversation
- 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`.
- 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
- 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)
## Summary * Add swedish translation of strings added by "feat: Support for multiple OPDS servers (crosspoint-reader#1209)" --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**NO**_
## Summary * Includes short SHA in version string for better version tracking, so it looks like `1.1.0-dev-feat-kosync-xpath-05c6cf8` * Closes crosspoint-reader#1247 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< YES >**_
## Summary * **What is the goal of this PR?** Enables the PlatformIO build cache to speed up compilation. ## Additional Context Significant improvement when switching branches, as unchanged files won't need re-compilation. See [Docs](https://docs.platformio.org/en/latest/projectconf/sections/platformio/options/directory/build_cache_dir.html). Note: May need to manually delete .cache folder if updating toolchain or framework. --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< NO >**_
…facts (crosspoint-reader#918) ## Summary - Add new `roundedraff` theme. - Fix sleep screen artifact where book cover showed grid lines in `Cover` mode with `Crop`. ## Additional Context ## Key changes - Added `src/components/themes/roundedraff/` theme implementation. - Updated sleep cover rendering logic in: - `src/activities/boot_sleep/SleepActivity.cpp` - `src/activities/boot_sleep/SleepActivity.h` - Improved cover generation/regeneration paths in: - `lib/Epub/Epub.cpp` - `lib/Epub/Epub.h` - `lib/Txt/Txt.cpp` - `lib/Txt/Txt.h` ## Verification - Sleep screen set to Book cover - Sleep screen cover mode set to Crop - Confirmed grid artifacts no longer appear on cover sleep screen. ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< YES >**_ --------- Co-authored-by: CaptainFrito <yzq6x5zypy@privaterelay.appleid.com> Co-authored-by: Zach Nelson <zach@zdnelson.com> Co-authored-by: Uri Tauber <142022451+Uri-Tauber@users.noreply.github.com>
## Summary The Python scripts in the current repo have many requirements that are not mentioned in any requirements.txt files, so I have therefore added them to the directories that need them. Question: Should these changes maybe moved into a "root" requirements.txt file? --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**NO**_
## Summary
- **What is the goal of this PR?** Fix chapter-opener text collapsing to
1-2 words per line in EPUBs that apply large em-based horizontal CSS
insets.
- **What changes are included?** Caps `marginLeft`, `marginRight`,
`paddingLeft`, and `paddingRight` at 2em in `BlockStyle::fromCssStyle`
(`lib/Epub/Epub/blocks/BlockStyle.h`). Vertical margins/padding are
unchanged - the bug is horizontal-only.
## Additional Context
**Repro:** *Mother Night* by Kurt Vonnegut, Chapter 21 ("My best
friend..."). The chapter-opening element's embedded CSS sets a large
horizontal inset.
- Settings: Embedded Style = On, Justify alignment (default).
- Before: 1-2 words per line with a visible river between them.
- After: body text fills the usable page width like every other
paragraph.
**Why the clamp lives in `fromCssStyle`:** This is the single point
where CSS lengths resolve to pixels, so clamping here keeps both
`effectiveWidth` call sites in `ChapterHtmlSlimParser` (`:1131-1133`
makePages, `:841-844` long-block split) consistent with the
`leftInset()` xOffset. A width-only clamp at either call site would
leave text pushed right by a large xOffset.
**Test plan:**
- [x] Flashed to Xteink X4. Chapter 21 of *Mother Night* renders
normally with Embedded Style on.
- Users with cached layouts of affected books need to delete
`.crosspoint/epub_<hash>/sections/` (or the whole `.crosspoint/`) on the
SD card to pick up the fix.
**Before / After:**
<p float="left">
<img height="400" alt="IMG_0320"
src="https://github.com/user-attachments/assets/de8815ae-f2f4-4845-be1f-449c5a25d191"
/>
<img height="400" alt="IMG_0324"
src="https://github.com/user-attachments/assets/95f5f4cb-8aa1-418e-b611-18e0c342cbef"
/>
</p>
---
### AI Usage
While CrossPoint doesn't have restrictions on AI tools in contributing,
please be transparent about their usage as it helps set the right
context for reviewers.
Did you use AI tools to help write this code? _**< YES >**_
I used Claude Code (Opus 4.7) to evaluate the codebase and find the
relevant code related to this rendering. From there, it was human
designed, reviewed and tested.
Co-authored-by: rhoopr <>
📝 WalkthroughWalkthroughAdds a reader clipping feature with a ClipSelectionActivity, persistence via ClippingsManager::saveClipping, reader menu/power wiring, per-word metadata accessors, display/window and rounded-corner mask primitives, translations/settings additions, and a few conditional debug logs. ChangesClip selection and persistence
Sequence Diagram(s)sequenceDiagram
participant User
participant Reader as EpubReaderActivity
participant Menu as ReaderMenu
participant Clip as ClipSelectionActivity
participant Manager as ClippingsManager
participant Storage as SD_Card
User->>Reader: open menu or quick action (SAVE_CLIPPING)
Reader->>Reader: load up to 3 pages, extract per-word boxes
Reader->>Clip: Launch ClipSelectionActivity with WordRef list
activate Clip
User->>Clip: Navigate cursor / mark selection
Clip->>Clip: Render highlights, switch pages as needed
User->>Clip: Confirm selection
Clip-->>Reader: Return ClippingResult(text, startPageNumber)
deactivate Clip
Reader->>Manager: saveClipping(title, author, chapter, page, text)
Manager->>Storage: ensure /clippings dir, append file
Storage-->>Manager: success / error
Manager-->>Reader: bool (success/failure)
Reader->>User: show success or STR_CLIPPING_SAVE_FAILED toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing [Slack Agent](https://www.coderabbit.ai/agent): Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/GfxRenderer/GfxRenderer.cpp`:
- Around line 946-979: The displayWindow transform in GfxRenderer::displayWindow
only handles 0°/180° correctly; for 90° rotations (LandscapeClockwise and
LandscapeCounterClockwise) you must recompute the physical origin using the
opposite axis and swap width/height before alignment so partial-update rects map
to the correct panel area. Update the case handling for LandscapeClockwise and
LandscapeCounterClockwise to set phyX/phyY using panelHeight/panelWidth as
needed and set phyW = height and phyH = width (i.e., swap dims), then perform
the 8-pixel alignment using those swapped dimensions before calling
display.displayWindow; ensure the logic matches rotateCoordinates()’s rotation
conventions.
- Around line 507-540: The code in maskRoundedRectOutsideCorners can write past
the opposite edge when radius is larger than the rectangle; clamp radius first
(e.g., compute int r = std::min(radius, std::min(width, height) / 2) and use r
for the early return and subsequent math instead of the raw radius) so all
mirrored writes (the loops using rr/rr2 and the drawPixel/drawPixelDither calls)
stay inside the box; update the function to use r, rr = r-1, rr2 = rr*rr and
keep the existing color-handling logic with drawPixel and drawPixelDither.
In `@lib/hal/HalDisplay.cpp`:
- Around line 61-68: The clipping logic in HalDisplay::displayWindow incorrectly
shifts negative x/y to 0 without reducing w/h, causing the wrong region to be
drawn; update HalDisplay::displayWindow to compute the clipped origin and size
by subtracting the amount clipped on the left/top (e.g., if x<0, reduce w by -x
and set x=0; similarly for y), then clamp w/h to the remaining visible area and
return early if resulting w<=0 or h<=0 before calling einkDisplay.displayWindow
with the adjusted uint16_t values; ensure you still handle right/bottom overflow
(x+w>DISPLAY_WIDTH, y+h>DISPLAY_HEIGHT) after the left/top clipping.
In `@lib/I18n/translations/kazakh.yaml`:
- Around line 232-234: Add the missing STR_SAVE_CLIPPING key to vietnamese.yaml
so it matches other locales; use the same key name STR_SAVE_CLIPPING and provide
the Vietnamese translation (e.g., "Lưu đoạn trích" or the project's approved
phrasing), ensuring valid YAML syntax and placement alongside STR_DELETE_CACHE
and STR_CHAPTER_PREFIX entries to keep locales consistent.
In `@src/activities/reader/ClipSelectionActivity.cpp`:
- Around line 55-58: The call to switchToPage(...) currently only logs failures
but the code continues as if the page snapshot succeeded; update onEnter (the
initial switchToPage(0) + requestUpdate() path) and the later page-switching
code to check the boolean return of switchToPage and fail-fast: if
switchToPage(...) returns false, do not call requestUpdate(), do not clear
needsPageSwitch, and return/abort the transition (or set an error state) so we
never render a stale/uninitialized snapshot. Specifically modify the
switchToPage(0) usage in onEnter, and all places that clear needsPageSwitch (and
the follow-up requestUpdate() calls) so they only proceed when switchToPage
succeeded.
In `@src/activities/reader/ClipSelectionActivity.h`:
- Around line 14-18: The WordRef struct must carry per-word rendering metadata
so measurements use the actual style; add fields (e.g., int fontId, float
fontSize, and an enum/bitmask fontStyle or std::string fontFamily) to WordRef
and populate them where words are created in EpubReaderActivity.cpp (use the
exact per-run font info when calling measurement/layout routines) and then use
those new WordRef fields in ClipSelectionActivity.cpp when computing
widths/centers and drawing highlight boxes or handling up/down navigation;
update any measurement calls and highlight drawing code to accept/use the
per-word fontId/fontSize/fontStyle instead of assuming a single shared font.
In `@src/activities/reader/EpubReaderActivity.cpp`:
- Around line 755-760: The lambda callback that handles clipping results ignores
ClippingsManager::saveClipping()'s boolean return and always proceeds as if the
save succeeded; update the lambda (the [this, chapterTitle, startPage](const
ActivityResult& result) handler that extracts ClippingResult from result.data)
to check the bool returned by ClippingsManager::saveClipping(epub->getTitle(),
epub->getAuthor(), chapterTitle, startPage + 1, clip.text) and, on false,
surface the failure to the user (e.g., call the existing UI
notification/alert/toast routine or enqueue a pending save alert) instead of
silently returning to the reader so SD-full or removed-card failures are
visible.
In `@src/activities/reader/EpubReaderMenuActivity.cpp`:
- Around line 29-30: The constant baseItemCount in EpubReaderMenuActivity.cpp is
one too large and doesn't match the number of unconditional menu pushes; update
the calculation so baseItemCount reflects the actual fixed/unconditional items
(reduce it by 1 from 15 to 14) or replace baseItemCount with a computed value
derived from the unconditional push calls, then keep totalItemCount =
baseItemCount + (hasFootnotes ? 1u : 0u) + (hasBookmarks ? 2u : 0u) consistent
with those changes; ensure any references to baseItemCount/totalItemCount remain
correct (symbols: baseItemCount, totalItemCount).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69ac6f26-571e-4e2a-b9ac-6a1130ea391f
📒 Files selected for processing (40)
lib/Epub/Epub/blocks/TextBlock.hlib/GfxRenderer/GfxRenderer.cpplib/GfxRenderer/GfxRenderer.hlib/I18n/translations/belarusian.yamllib/I18n/translations/catalan.yamllib/I18n/translations/czech.yamllib/I18n/translations/danish.yamllib/I18n/translations/dutch.yamllib/I18n/translations/english.yamllib/I18n/translations/finnish.yamllib/I18n/translations/french.yamllib/I18n/translations/german.yamllib/I18n/translations/hungarian.yamllib/I18n/translations/italian.yamllib/I18n/translations/kazakh.yamllib/I18n/translations/lithuanian.yamllib/I18n/translations/polish.yamllib/I18n/translations/portuguese.yamllib/I18n/translations/romanian.yamllib/I18n/translations/russian.yamllib/I18n/translations/slovenian.yamllib/I18n/translations/spanish.yamllib/I18n/translations/swedish.yamllib/I18n/translations/turkish.yamllib/I18n/translations/ukrainian.yamllib/hal/HalDisplay.cpplib/hal/HalDisplay.hlib/hal/HalStorage.cppsrc/CrossPointSettings.hsrc/SettingsList.hsrc/activities/ActivityManager.cppsrc/activities/ActivityResult.hsrc/activities/reader/ClipSelectionActivity.cppsrc/activities/reader/ClipSelectionActivity.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderMenuActivity.hsrc/clippings/ClippingsManager.cppsrc/clippings/ClippingsManager.h
Comment on lines
+507
to
+540
| void GfxRenderer::maskRoundedRectOutsideCorners(const int x, const int y, const int width, const int height, | ||
| const int radius, const Color color) const { | ||
| if (radius <= 0 || color == Color::Clear) { | ||
| return; | ||
| } | ||
|
|
||
| const int rr = radius - 1; | ||
| const int rr2 = rr * rr; | ||
| for (int dy = 0; dy < radius; dy++) { | ||
| for (int dx = 0; dx < radius; dx++) { | ||
| const int tx = rr - dx; | ||
| const int ty = rr - dy; | ||
| if (tx * tx + ty * ty > rr2) { | ||
| if (color == Color::White || color == Color::Black) { | ||
| bool state = color == Color::Black; | ||
| drawPixel(x + dx, y + dy, state); // top-left | ||
| drawPixel(x + width - 1 - dx, y + dy, state); // top-right | ||
| drawPixel(x + dx, y + height - 1 - dy, state); // bottom-left | ||
| drawPixel(x + width - 1 - dx, y + height - 1 - dy, state); // bottom-right | ||
| } else if (color == Color::LightGray) { | ||
| drawPixelDither<Color::LightGray>(x + dx, y + dy); // top-left | ||
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + dy); // top-right | ||
| drawPixelDither<Color::LightGray>(x + dx, y + height - 1 - dy); // bottom-left | ||
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | ||
| } else if (color == Color::DarkGray) { | ||
| drawPixelDither<Color::DarkGray>(x + dx, y + dy); // top-left | ||
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + dy); // top-right | ||
| drawPixelDither<Color::DarkGray>(x + dx, y + height - 1 - dy); // bottom-left | ||
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Clamp radius before mirroring the corner mask.
This helper assumes radius fits inside the rectangle. If it doesn't, the mirrored writes cross past the opposite edge and start touching pixels outside the requested box. Since this is a public drawing helper, it should defensively cap the radius first.
Suggested fix
void GfxRenderer::maskRoundedRectOutsideCorners(const int x, const int y, const int width, const int height,
const int radius, const Color color) const {
- if (radius <= 0 || color == Color::Clear) {
+ if (width <= 0 || height <= 0 || radius <= 0 || color == Color::Clear) {
return;
}
- const int rr = radius - 1;
+ const int clippedRadius = std::min({radius, width / 2, height / 2});
+ if (clippedRadius <= 0) {
+ return;
+ }
+
+ const int rr = clippedRadius - 1;
const int rr2 = rr * rr;
- for (int dy = 0; dy < radius; dy++) {
- for (int dx = 0; dx < radius; dx++) {
+ for (int dy = 0; dy < clippedRadius; dy++) {
+ for (int dx = 0; dx < clippedRadius; dx++) {
const int tx = rr - dx;
const int ty = rr - dy;
if (tx * tx + ty * ty > rr2) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void GfxRenderer::maskRoundedRectOutsideCorners(const int x, const int y, const int width, const int height, | |
| const int radius, const Color color) const { | |
| if (radius <= 0 || color == Color::Clear) { | |
| return; | |
| } | |
| const int rr = radius - 1; | |
| const int rr2 = rr * rr; | |
| for (int dy = 0; dy < radius; dy++) { | |
| for (int dx = 0; dx < radius; dx++) { | |
| const int tx = rr - dx; | |
| const int ty = rr - dy; | |
| if (tx * tx + ty * ty > rr2) { | |
| if (color == Color::White || color == Color::Black) { | |
| bool state = color == Color::Black; | |
| drawPixel(x + dx, y + dy, state); // top-left | |
| drawPixel(x + width - 1 - dx, y + dy, state); // top-right | |
| drawPixel(x + dx, y + height - 1 - dy, state); // bottom-left | |
| drawPixel(x + width - 1 - dx, y + height - 1 - dy, state); // bottom-right | |
| } else if (color == Color::LightGray) { | |
| drawPixelDither<Color::LightGray>(x + dx, y + dy); // top-left | |
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + dy); // top-right | |
| drawPixelDither<Color::LightGray>(x + dx, y + height - 1 - dy); // bottom-left | |
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | |
| } else if (color == Color::DarkGray) { | |
| drawPixelDither<Color::DarkGray>(x + dx, y + dy); // top-left | |
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + dy); // top-right | |
| drawPixelDither<Color::DarkGray>(x + dx, y + height - 1 - dy); // bottom-left | |
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | |
| } | |
| } | |
| } | |
| } | |
| } | |
| void GfxRenderer::maskRoundedRectOutsideCorners(const int x, const int y, const int width, const int height, | |
| const int radius, const Color color) const { | |
| if (width <= 0 || height <= 0 || radius <= 0 || color == Color::Clear) { | |
| return; | |
| } | |
| const int clippedRadius = std::min({radius, width / 2, height / 2}); | |
| if (clippedRadius <= 0) { | |
| return; | |
| } | |
| const int rr = clippedRadius - 1; | |
| const int rr2 = rr * rr; | |
| for (int dy = 0; dy < clippedRadius; dy++) { | |
| for (int dx = 0; dx < clippedRadius; dx++) { | |
| const int tx = rr - dx; | |
| const int ty = rr - dy; | |
| if (tx * tx + ty * ty > rr2) { | |
| if (color == Color::White || color == Color::Black) { | |
| bool state = color == Color::Black; | |
| drawPixel(x + dx, y + dy, state); // top-left | |
| drawPixel(x + width - 1 - dx, y + dy, state); // top-right | |
| drawPixel(x + dx, y + height - 1 - dy, state); // bottom-left | |
| drawPixel(x + width - 1 - dx, y + height - 1 - dy, state); // bottom-right | |
| } else if (color == Color::LightGray) { | |
| drawPixelDither<Color::LightGray>(x + dx, y + dy); // top-left | |
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + dy); // top-right | |
| drawPixelDither<Color::LightGray>(x + dx, y + height - 1 - dy); // bottom-left | |
| drawPixelDither<Color::LightGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | |
| } else if (color == Color::DarkGray) { | |
| drawPixelDither<Color::DarkGray>(x + dx, y + dy); // top-left | |
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + dy); // top-right | |
| drawPixelDither<Color::DarkGray>(x + dx, y + height - 1 - dy); // bottom-left | |
| drawPixelDither<Color::DarkGray>(x + width - 1 - dx, y + height - 1 - dy); // bottom-right | |
| } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/GfxRenderer/GfxRenderer.cpp` around lines 507 - 540, The code in
maskRoundedRectOutsideCorners can write past the opposite edge when radius is
larger than the rectangle; clamp radius first (e.g., compute int r =
std::min(radius, std::min(width, height) / 2) and use r for the early return and
subsequent math instead of the raw radius) so all mirrored writes (the loops
using rr/rr2 and the drawPixel/drawPixelDither calls) stay inside the box;
update the function to use r, rr = r-1, rr2 = rr*rr and keep the existing
color-handling logic with drawPixel and drawPixelDither.
Comment on lines
+946
to
+979
| void GfxRenderer::displayWindow(int x, int y, int width, int height) const { | ||
| 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); | ||
| } |
There was a problem hiding this comment.
Handle 90° orientations when converting partial-update rectangles.
The current transform only works for 0°/180° cases. rotateCoordinates() rotates Portrait and PortraitInverted by 90°, so the physical origin must be recomputed from the opposite axis and width/height must swap. As written, portrait-mode partial refreshes will hit the wrong panel area and can leave stale pixels behind.
Suggested fix
void GfxRenderer::displayWindow(int x, int y, int width, int height) const {
int phyX, phyY, phyW, phyH;
switch (orientation) {
case Portrait:
- phyX = x;
- phyY = panelHeight - y - height;
- phyW = width;
- phyH = height;
+ phyX = y;
+ phyY = panelHeight - x - width;
+ phyW = height;
+ phyH = width;
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;
+ phyX = panelWidth - y - height;
+ phyY = x;
+ phyW = height;
+ phyH = width;
break;
case LandscapeCounterClockwise:
default:
phyX = x;
phyY = y;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/GfxRenderer/GfxRenderer.cpp` around lines 946 - 979, The displayWindow
transform in GfxRenderer::displayWindow only handles 0°/180° correctly; for 90°
rotations (LandscapeClockwise and LandscapeCounterClockwise) you must recompute
the physical origin using the opposite axis and swap width/height before
alignment so partial-update rects map to the correct panel area. Update the case
handling for LandscapeClockwise and LandscapeCounterClockwise to set phyX/phyY
using panelHeight/panelWidth as needed and set phyW = height and phyH = width
(i.e., swap dims), then perform the 8-pixel alignment using those swapped
dimensions before calling display.displayWindow; ensure the logic matches
rotateCoordinates()’s rotation conventions.
Comment on lines
+61
to
+68
| 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<uint16_t>(x), static_cast<uint16_t>(y), static_cast<uint16_t>(w), | ||
| static_cast<uint16_t>(h)); |
There was a problem hiding this comment.
Clip top/left overflow instead of shifting the window.
When x or y is negative, this resets the origin to 0 but leaves w/h unchanged. A request like (-10, 5, 20, 8) then refreshes 20 px from 0 instead of the visible 10 px intersection, which can redraw the wrong region during partial updates.
Suggested fix
void HalDisplay::displayWindow(int x, int y, int w, int h) {
- if (x < 0) x = 0;
- if (y < 0) y = 0;
+ if (x < 0) {
+ w += x;
+ x = 0;
+ }
+ if (y < 0) {
+ h += y;
+ 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<uint16_t>(x), static_cast<uint16_t>(y), static_cast<uint16_t>(w),
static_cast<uint16_t>(h));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/hal/HalDisplay.cpp` around lines 61 - 68, The clipping logic in
HalDisplay::displayWindow incorrectly shifts negative x/y to 0 without reducing
w/h, causing the wrong region to be drawn; update HalDisplay::displayWindow to
compute the clipped origin and size by subtracting the amount clipped on the
left/top (e.g., if x<0, reduce w by -x and set x=0; similarly for y), then clamp
w/h to the remaining visible area and return early if resulting w<=0 or h<=0
before calling einkDisplay.displayWindow with the adjusted uint16_t values;
ensure you still handle right/bottom overflow (x+w>DISPLAY_WIDTH,
y+h>DISPLAY_HEIGHT) after the left/top clipping.
Comment on lines
232
to
234
| STR_DELETE_CACHE: "Кітап кэшін жою" | ||
| STR_SAVE_CLIPPING: "Үзінді сақтау" | ||
| STR_CHAPTER_PREFIX: "Тарау: " |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find which translation files already define STR_SAVE_CLIPPING
rg -n --glob 'lib/I18n/translations/*.ya?ml' '^\s*STR_SAVE_CLIPPING\s*:' lib/I18n/translations || true
# Fail if any locale file is missing the key
missing=0
for f in lib/I18n/translations/*.ya?ml; do
if ! rg -n '^\s*STR_SAVE_CLIPPING\s*:' "$f" >/dev/null 2>&1; then
echo "MISSING: $f"
missing=1
fi
done
if [ "$missing" -ne 0 ]; then
echo "One or more locale files are missing STR_SAVE_CLIPPING."
exit 1
fiRepository: uxjulia/CrossInk
Length of output: 155
🏁 Script executed:
# List all translation files in the expected directory
find lib/I18n/translations -name '*.yaml' -o -name '*.yml' | sortRepository: uxjulia/CrossInk
Length of output: 875
🏁 Script executed:
# For each translation file, check if STR_SAVE_CLIPPING exists
for f in $(find lib/I18n/translations -name '*.yaml' -o -name '*.yml' | sort); do
echo "=== $f ==="
if rg -n 'STR_SAVE_CLIPPING' "$f"; then
echo "FOUND"
else
echo "NOT FOUND"
fi
doneRepository: uxjulia/CrossInk
Length of output: 2104
Kazakh translation is correct, but Vietnamese locale is missing STR_SAVE_CLIPPING.
STR_SAVE_CLIPPING: "Үзінді сақтау" is properly added to kazakh.yaml with valid YAML syntax. However, verification shows that vietnamese.yaml is the only locale file missing this key while all other 22 locales have it. Add the Vietnamese translation to maintain localization completeness and prevent fallback/missing-key behavior for Vietnamese users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/I18n/translations/kazakh.yaml` around lines 232 - 234, Add the missing
STR_SAVE_CLIPPING key to vietnamese.yaml so it matches other locales; use the
same key name STR_SAVE_CLIPPING and provide the Vietnamese translation (e.g.,
"Lưu đoạn trích" or the project's approved phrasing), ensuring valid YAML syntax
and placement alongside STR_DELETE_CACHE and STR_CHAPTER_PREFIX entries to keep
locales consistent.
Comment on lines
+14
to
+18
| struct WordRef { | ||
| int x, y, w, h; | ||
| int pageIdx; // 0 = current, 1 = next, 2 = page after next | ||
| std::string text; | ||
| }; |
There was a problem hiding this comment.
WordRef needs per-word style/font metadata.
The clipping selector currently treats every word as if it were rendered with one shared fontId, but EPUB lines can mix bold/italic/embedded styles. That makes the stored width/center wrong for styled runs, so the highlight box and up/down navigation drift away from the visible word. Add the rendered style/font info here and use it when measuring words in EpubReaderActivity.cpp and redrawing highlights in ClipSelectionActivity.cpp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/activities/reader/ClipSelectionActivity.h` around lines 14 - 18, The
WordRef struct must carry per-word rendering metadata so measurements use the
actual style; add fields (e.g., int fontId, float fontSize, and an enum/bitmask
fontStyle or std::string fontFamily) to WordRef and populate them where words
are created in EpubReaderActivity.cpp (use the exact per-run font info when
calling measurement/layout routines) and then use those new WordRef fields in
ClipSelectionActivity.cpp when computing widths/centers and drawing highlight
boxes or handling up/down navigation; update any measurement calls and highlight
drawing code to accept/use the per-word fontId/fontSize/fontStyle instead of
assuming a single shared font.
Owner
|
Hey, I tested this locally and this is really cool! Just a few comments to improve the UX:
|
Author
|
Updated this PR with the requested clipping UX changes:
I also cleaned up clipping output formatting so each per-book .txt file has a single title/author header and each clipping is stored as:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/activities/reader/EpubReaderActivity.cpp`:
- Around line 712-755: The loop that mutates section->currentPage and calls
section->loadPageFromSectionFile must run while holding the same RenderLock used
elsewhere to prevent race conditions; wrap the for-loop (the code that sets
section->currentPage = startPage + pi, calls loadPageFromSectionFile, and
collects words into the words vector) inside a RenderLock scope, ensuring you
save startPage before acquiring the lock and restore section->currentPage =
startPage immediately before releasing the lock; use the same render lock
class/instance used in this file (RenderLock) so render/save-progress cannot see
the temporary currentPage values while you build the
ClipSelectionActivity::WordRef list.
- Around line 764-773: The code always persists startPage + 1 when saving a
clip, which is wrong for selections that begin on page 2 or 3 of the current
window; update the flow so the actual selected page/offset from the selection is
used: extend ClippingResult (returned from ClipSelectionActivity) to carry the
selected start page or WordRef.pageIdx, have ClipSelectionActivity construct
that ClippingResult using the WordRef.pageIdx (or computed absolute page)
instead of assuming startPage, and in the callback here use that ClippingResult
page value when calling ClippingsManager::saveClipping(...) (replace startPage +
1 with the selected page value + 1 if needed) so saved clippings reflect the
real starting page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c538690c-a592-4ce0-89af-ec5d0ceba97b
📒 Files selected for processing (8)
lib/I18n/translations/english.yamlsrc/activities/reader/ClipSelectionActivity.cppsrc/activities/reader/ClipSelectionActivity.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/clippings/ClippingsManager.cppsrc/clippings/ClippingsManager.h
✅ Files skipped from review due to trivial changes (4)
- src/clippings/ClippingsManager.h
- lib/I18n/translations/english.yaml
- src/clippings/ClippingsManager.cpp
- src/activities/reader/ClipSelectionActivity.h
🚧 Files skipped from review as they are similar to previous changes (2)
- src/activities/reader/EpubReaderActivity.h
- src/activities/reader/ClipSelectionActivity.cpp
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/activities/reader/ClipSelectionActivity.h (2)
20-22: ⚡ Quick win
std::vector<WordRef>taken by value forces an unnecessary deep copy at the call site.Each
WordRefcontains a heap-allocatedstd::string; copying the whole vector allocates once per word. The constructor should accept the vector by&&and move it into the member.♻️ Proposed fix
- ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector<WordRef> words, + ClipSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, std::vector<WordRef>&& words, std::string bookTitle, std::string author, std::string chapterTitle, int fontId, Section& section, int startPageInSection, int marginTop, int marginLeft);In the constructor body, initialise the member with
std::move(words).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/ClipSelectionActivity.h` around lines 20 - 22, The ClipSelectionActivity constructor currently takes std::vector<WordRef> by value causing an expensive copy; change the parameter to std::vector<WordRef>&& words (rvalue reference) in the ClipSelectionActivity declaration and implementation, and initialise the class member that holds the words by moving it (e.g., member = std::move(words) or in the ctor initializer list) so the caller's vector is moved instead of deep-copied; reference types: ClipSelectionActivity, WordRef, and the words parameter.
17-17: ⚡ Quick win
std::string textin a per-word struct iterated duringrender()violates the hot-pathstd::stringguideline.
std::vector<WordRef>is walked every render cycle to draw highlights and build the clipping text. Eachstd::stringmember triggers a separate heap allocation, adding GC pressure and fragmentation on a constrained heap.If the page text buffer outlives the page switch (i.e., the raw
char*pointer remains valid while this activity is alive),std::string_viewis a zero-allocation drop-in. Otherwise a compact fixed-sizechar[](e.g.,char text[48]) bounds memory use per word while keeping the struct trivially copyable.As per coding guidelines: "Avoid
std::stringand ArduinoStringin hot paths. Preferstring_view,char[], andsnprintf."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/ClipSelectionActivity.h` at line 17, The per-word member "std::string text" in struct WordRef (used by ClipSelectionActivity::render()) causes allocations on every render; replace it with a zero-allocation std::string_view if the page's raw text buffer outlives the WordRef instances (update the code that populates WordRef to store views into that buffer), otherwise replace the member with a fixed-size char array (e.g., char text[48]) and copy using snprintf with bounds checking when populating WordRef to keep the struct trivially copyable and avoid heap allocations; also update any constructors/populators of WordRef and include <string_view> or <cstdio> as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/activities/reader/ClipSelectionActivity.h`:
- Around line 46-47: The class ClipSelectionActivity currently owns a raw
pointer savedBuffer (with savedBufferSize) but has no destructor, risking leaks
if onExit() is not called; fix by making ownership RAII or providing
deterministic cleanup: either replace uint8_t* savedBuffer with
std::unique_ptr<uint8_t[]> savedBuffer (update allocation sites to reset/new[]
usage and remove manual free), or declare
ClipSelectionActivity::~ClipSelectionActivity() and free/delete[] savedBuffer
there (and set savedBuffer=nullptr and savedBufferSize=0); ensure references to
savedBuffer and allocations in the .cpp (allocation and any free calls) are
updated to match the chosen approach so destruction always releases the
framebuffer even if onExit() is bypassed.
---
Nitpick comments:
In `@src/activities/reader/ClipSelectionActivity.h`:
- Around line 20-22: The ClipSelectionActivity constructor currently takes
std::vector<WordRef> by value causing an expensive copy; change the parameter to
std::vector<WordRef>&& words (rvalue reference) in the ClipSelectionActivity
declaration and implementation, and initialise the class member that holds the
words by moving it (e.g., member = std::move(words) or in the ctor initializer
list) so the caller's vector is moved instead of deep-copied; reference types:
ClipSelectionActivity, WordRef, and the words parameter.
- Line 17: The per-word member "std::string text" in struct WordRef (used by
ClipSelectionActivity::render()) causes allocations on every render; replace it
with a zero-allocation std::string_view if the page's raw text buffer outlives
the WordRef instances (update the code that populates WordRef to store views
into that buffer), otherwise replace the member with a fixed-size char array
(e.g., char text[48]) and copy using snprintf with bounds checking when
populating WordRef to keep the struct trivially copyable and avoid heap
allocations; also update any constructors/populators of WordRef and include
<string_view> or <cstdio> as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cee02d7a-ae90-4151-acb8-fc5541f3d279
📒 Files selected for processing (5)
src/activities/ActivityResult.hsrc/activities/reader/ClipSelectionActivity.cppsrc/activities/reader/ClipSelectionActivity.hsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.h
🚧 Files skipped from review as they are similar to previous changes (3)
- src/activities/ActivityResult.h
- src/activities/reader/EpubReaderActivity.cpp
- src/activities/reader/ClipSelectionActivity.cpp
Comment on lines
+46
to
+47
| uint8_t* savedBuffer = nullptr; | ||
| size_t savedBufferSize = 0; |
There was a problem hiding this comment.
Raw owning pointer savedBuffer with no destructor — potential heap leak on embedded target.
ClipSelectionActivity declares no destructor, so the compiler-generated one (from virtual ~Activity() = default) will not free savedBuffer. If the activity is ever destroyed without onExit() running (finish/unwind path, future refactoring), the framebuffer allocation is silently leaked on a heap-constrained device.
Declare a destructor (or use a std::unique_ptr<uint8_t[]>) to guarantee cleanup:
🛡️ Option A – explicit destructor (minimal change)
In the header, add:
+ ~ClipSelectionActivity();In the .cpp, implement:
+ClipSelectionActivity::~ClipSelectionActivity() {
+ delete[] savedBuffer;
+ savedBuffer = nullptr;
+}🛡️ Option B – RAII (preferred)
- uint8_t* savedBuffer = nullptr;
- size_t savedBufferSize = 0;
+ std::unique_ptr<uint8_t[]> savedBuffer;
+ size_t savedBufferSize = 0;Update allocation/free sites in the .cpp accordingly; the destructor is then implicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/activities/reader/ClipSelectionActivity.h` around lines 46 - 47, The
class ClipSelectionActivity currently owns a raw pointer savedBuffer (with
savedBufferSize) but has no destructor, risking leaks if onExit() is not called;
fix by making ownership RAII or providing deterministic cleanup: either replace
uint8_t* savedBuffer with std::unique_ptr<uint8_t[]> savedBuffer (update
allocation sites to reset/new[] usage and remove manual free), or declare
ClipSelectionActivity::~ClipSelectionActivity() and free/delete[] savedBuffer
there (and set savedBuffer=nullptr and savedBufferSize=0); ensure references to
savedBuffer and allocations in the .cpp (allocation and any free calls) are
updated to match the chosen approach so destruction always releases the
framebuffer even if onExit() is bypassed.
Owner
|
I merged this into my latest dev branch to make sure it works with all the changes I've made recently and it looks good from what I saw. I did notice that the original PR made some updates though. Can you merge those in with your changes and see that everything still works as intended? They changed the way clippings are saved/named so that it's compatible with clippings.io, we should probably keep that change. I tested the original crosspoint PR a few days ago and it seemed like the up/down buttons advance by one word now instead of a line and a line requires pressing and holding. I actually prefer the up/down advancing by a line and the left/right by a word.. it's a lot faster to clip that way, so if we can keep that behavior that'd be ideal. |
Author
Will do! |
imshentastic
referenced
this pull request
in imshentastic/CrumBLE
May 23, 2026
## Summary
- **What is the goal of this PR?**
Implements wireless EPUB file management via a built-in web server,
enabling users to upload, browse, organize, and delete EPUB files from
any device on the same WiFi network without needing a computer cable
connection.
- **What changes are included?**
- **New Web Server**
([`CrossPointWebServer.cpp`](src/CrossPointWebServer.cpp),
[`CrossPointWebServer.h`](src/CrossPointWebServer.h)):
- HTTP server on port 80 with a responsive HTML/CSS interface
- Home page showing device status (version, IP, free memory)
- File Manager with folder navigation and breadcrumb support
- EPUB file upload with progress tracking
- Folder creation and file/folder deletion
- XSS protection via HTML escaping
- Hidden system folders (`.` prefixed, "System Volume Information",
"XTCache")
- **WiFi Screen** ([`WifiScreen.cpp`](src/screens/WifiScreen.cpp),
[`WifiScreen.h`](src/screens/WifiScreen.h)):
- Network scanning with signal strength indicators
- Visual indicators for encrypted (`*`) and saved (`+`) networks
- State machine managing: scanning, network selection, password entry,
connecting, save/forget prompts
- 15-second connection timeout handling
- Integration with web server (starts on connect, stops on exit)
- **WiFi Credential Storage**
([`WifiCredentialStore.cpp`](src/WifiCredentialStore.cpp),
[`WifiCredentialStore.h`](src/WifiCredentialStore.h)):
- Persistent storage in `/sd/.crosspoint/wifi.bin`
- XOR obfuscation for stored passwords (basic protection against casual
reading)
- Up to 8 saved networks with add/remove/update operations
- **On-Screen Keyboard**
([`OnScreenKeyboard.cpp`](src/screens/OnScreenKeyboard.cpp),
[`OnScreenKeyboard.h`](src/screens/OnScreenKeyboard.h)):
- Reusable QWERTY keyboard component with shift support
- Special keys: Shift, Space, Backspace, Done
- Support for password masking mode
- **Settings Screen Integration**
([`SettingsScreen.h`](src/screens/SettingsScreen.h)):
- Added WiFi action to navigate to the new WiFi screen
- **Documentation** ([`docs/webserver.md`](docs/webserver.md)):
- Comprehensive user guide covering WiFi setup, web interface usage,
file management, troubleshooting, and security notes
- See this for more screenshots!
- Working "displays the right way in GitHub" on my repo:
https://github.com/olearycrew/crosspoint-reader/blob/feature/connect-to-wifi/docs/webserver.md
**Video demo**
https://github.com/user-attachments/assets/283e32dc-2d9f-4ae2-848e-01f41166a731
## Additional Context
- **Security considerations**: The web server has no
authentication—anyone on the same WiFi network can access files. This is
documented as a limitation, recommending use only on trusted private
networks. Password obfuscation in the credential store is XOR-based, not
cryptographically secure.
- **Memory implications**: The web server and WiFi stack consume
significant memory. The implementation properly cleans up (stops server,
disconnects WiFi, sets `WIFI_OFF` mode) when exiting the WiFi screen to
free resources.
- **Async operations**: Network scanning and connection use async
patterns with FreeRTOS tasks to prevent blocking the UI. The display
task handles rendering on a dedicated thread with mutex protection.
- **Browser compatibility**: The web interface uses standard
HTML5/CSS3/JavaScript and is tested to work with all modern browsers on
desktop and mobile.
---------
Co-authored-by: Dave Allie <dave@daveallie.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This ports the clipping feature from crosspoint-reader#1742 into CrossInk and adapts it to CrossInk's input model and settings.
Credit to the original CrossPoint work in
crosspoint-reader/crosspoint-reader#1742; this PR is based on that feature and then extends it with CrossInk-specific control and storage changes.Included changes
Save Clippingas a configurable power-button action/clippings/with one file per book instead of a single global fileWhy this is one PR
These changes are all part of one user-facing clipping workflow: starting clipping, navigating the selector, and storing the result. Splitting them would make review history cleaner in theory, but it would also force reviewers to reason about an incomplete clipping UX across multiple PRs.
Testing
xlargefirmware targetfirmware-xlarge.binSummary by CodeRabbit