feat: Auto Page Turn for Epub Reader#723
feat: Auto Page Turn for Epub Reader#723GenesiaW wants to merge 18 commits intocrosspoint-reader:masterfrom
Conversation
lukestein
left a comment
lukestein
left a comment
There was a problem hiding this comment.
I like this but worried about ad hoc status bar overloading
| showBatteryPercentage); | ||
| } | ||
|
|
||
| // uses chapter name space to indicate auto page turn is enabled |
There was a problem hiding this comment.
I am slightly concerned about baking pixel-level assumptions about the status bar layout into this feature's implementation.
If we are "allowing" features to use status bar space (and this is not the only current PR trying to), I suspect we want some abstraction. Also, this will work better with current requests for fuller status bar customization.
There was a problem hiding this comment.
If we are "allowing" features to use status bar space (and this is not the only current PR trying to), I suspect we want some abstraction. Also, this will work better with current requests for fuller status bar customization.
I copied the code from the display of chapter name. I can probably clean up my code and reduce code duplication by changing the initialization of text in center of the status bar
Yes, I have been thinking of ways that could be used to indicate that auto page turn is enabled and it feels like the status bar is one of the least intrusive location to indicate it. Would love to get additional ideas/feedback on it can be better implemented |
GenesiaW
force-pushed
the
feat/auto-page-turn
branch
from
bf3f93d to
87ffb43
Compare
What if status bar is disabled? |
Here's a image of it enabled with a reader margin of 20. However, at reader margin of 5, the text will be cut off at the bottom, I will have to edit the code to reserve some space if auto page turn is enabled. Great catch! |
|
Seems like a nice addition to feature parity with the stock firmware. |
|
Thanks for the contribution! IMO, this is a great feature to add and agreed with @osteotek that it brings us more in line with the stock firmware. One problem I'm noticing is that the text is slightly more pixelated/has more jagged edges when this is enabled. I see a distinct difference in text rendering between having this off and turned on. For me, it's more noticeable in styled text, like italics and bolded text, but I also see it in normal text. I'm using "Noto Sans" for my font if that's helpful for seeing what I'm seeing it. Also, I did merge |
Actually, false alarm, ignore this! After testing this out more, I don't think this has anything to do with this PR. However, interestingly enough, if I enable auto turn, then press the back button to disable it, the text looks better to me (I believe it's the anti-aliasing) but only for that page that I was on when I turn off auto turn. As soon as I change the page, it goes back to the slightly more pixelated/jagged rendering. I wonder if there's some second-pass on the anti-aliasing we're doing after disabling auto-turn that makes text look briefly better. 😅 I'm gonna dig into this more... |
|
Wow, this is both awesome and so funny to me. @GenesiaW I think you inadvertently uncovered a way of making anti-aliased text look even smoother on the e-reader! I even tested in 0.16.0 and 0.15.0 to see if maybe we had this before and some other code changes along the way reverted it, but I don't think that's the case. This seems new 😄 From what I can gather (and to be clear I am NOT an expert in this at all, just trying to read the code as best I can and piece together what I'm seeing), toggling auto turn off does indeed run a second pass in calling the This second To force this behavior, I just simply duplicated the 
I doubt this is the best way to get this improvement, but as a simple test case, it's worth trying it out! @crosspoint-reader/firmware-maintainers Can anyone else test this and verify this behavior (and that I'm not seeing things/don't need to go to an eye doctor 😄)? |
|
And finally, here's the best visual comparison I can show Single-pass: Double-pass: I notice it most in capital M's and lowercase g's on this page (look for "Mongo" in the text above). On the left edge of the capital M, in the "single-pass" image, I can see what appears to me as slight textual bleeding, as if there's a single dark line and then a slightly lighter, grayer, shorter line to the right of that line to create the left edge of the M. In the "double-pass" image, that left edge of the capital M is a single, perfectly straight line, no second line next to it. Lowercase g's also have a smoother curve in the circle at the top of the character and the curve on the lower end. |
|
I can confirm that I get the same behavior. Indeed, you can archive the same behavior just by duplicating this code inside the render function: // Render and copy to MSB buffer
renderer.clearScreen(0x00);
renderer.setRenderMode(GfxRenderer::GRAYSCALE_MSB);
page->render(renderer, SETTINGS.getReaderFontId(), orientedMarginLeft, orientedMarginTop);
renderer.copyGrayscaleMsbBuffers();
// display grayscale part
renderer.displayGrayBuffer();
renderer.setRenderMode(GfxRenderer::BW);Note that it doesn't work if I only duplicate this line: I suspect that's because |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an automatic page-turn feature to the EPUB reader: configurable speeds selectable from the reader menu, integrated timing/state management, UI indicators (status/title), and updated APIs to enable/disable and propagate the selected page-turn option. Changes
Sequence DiagramsequenceDiagram
participant User
participant Menu as EpubReaderMenuActivity
participant Reader as EpubReaderActivity
participant Timer as RenderLoop
User->>Menu: Open menu / change AUTO_PAGE_TURN option
Menu->>Reader: onReaderMenuBack(pendingOrientation, selectedPageTurnOption)
Reader->>Reader: toggleAutoPageTurn(selectedPageTurnOption)
rect rgba(100,150,200,0.5)
Timer->>Reader: periodic tick
alt automaticPageTurnActive and elapsed >= pageTurnDuration
Reader->>Reader: advance page or spine, update lastPageTurnTime
Reader->>Reader: requestUpdate (render page + indicator)
else manual navigation input (Confirm/Back)
Reader->>Reader: disable auto-turn, reset lastPageTurnTime, requestUpdate
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/activities/reader/EpubReaderMenuActivity.h (1)
15-47:⚠️ Potential issue | 🟠 MajorAUTO_PAGE_TURN isn’t reachable from the menu list.
menuItemsnever includesMenuAction::AUTO_PAGE_TURN, so the new action can’t be selected or rendered. Please add it to the fixed menu layout (ordering as desired).🛠️ Suggested update
- const std::vector<MenuItem> menuItems = {{MenuAction::SELECT_CHAPTER, StrId::STR_SELECT_CHAPTER}, - {MenuAction::ROTATE_SCREEN, StrId::STR_ORIENTATION}, - {MenuAction::GO_TO_PERCENT, StrId::STR_GO_TO_PERCENT}, - {MenuAction::GO_HOME, StrId::STR_GO_HOME_BUTTON}, - {MenuAction::SYNC, StrId::STR_SYNC_PROGRESS}, - {MenuAction::DELETE_CACHE, StrId::STR_DELETE_CACHE}}; + const std::vector<MenuItem> menuItems = {{MenuAction::SELECT_CHAPTER, StrId::STR_SELECT_CHAPTER}, + {MenuAction::ROTATE_SCREEN, StrId::STR_ORIENTATION}, + {MenuAction::AUTO_PAGE_TURN, StrId::STR_AUTO_PAGE_TURN}, + {MenuAction::GO_TO_PERCENT, StrId::STR_GO_TO_PERCENT}, + {MenuAction::GO_HOME, StrId::STR_GO_HOME_BUTTON}, + {MenuAction::SYNC, StrId::STR_SYNC_PROGRESS}, + {MenuAction::DELETE_CACHE, StrId::STR_DELETE_CACHE}};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderMenuActivity.h` around lines 15 - 47, The fixed menu layout in EpubReaderMenuActivity declares MenuAction::AUTO_PAGE_TURN but menuItems (the vector used to render and navigate the menu) does not include it; update the const std::vector<MenuItem> menuItems initializer to include a MenuItem with action MenuAction::AUTO_PAGE_TURN and an appropriate label StrId (e.g., StrId::STR_AUTO_PAGE_TURN) in the desired order so AUTO_PAGE_TURN becomes selectable and rendered by the existing onEnter/loop/render logic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-16T20:43:12.609Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:12.609Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderActivity.cpp
📚 Learning: 2026-02-16T22:58:58.974Z
Learnt from: whyte-j
Repo: crosspoint-reader/crosspoint-reader PR: 733
File: src/components/StatusBar.cpp:63-78
Timestamp: 2026-02-16T22:58:58.974Z
Learning: In the CrossPoint Reader codebase, screen margin values are preset in settings with constrained options, so calculations involving margins (like `rendererableScreenWidth` or `availableTitleSpace` in StatusBar rendering) are unlikely to produce negative values in practice given these bounds.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
🧬 Code graph analysis (3)
src/activities/reader/EpubReaderMenuActivity.cpp (1)
src/main.cpp (1)
renderer(37-37)
src/activities/reader/EpubReaderActivity.h (1)
src/activities/reader/EpubReaderActivity.cpp (8)
onReaderMenuBack(317-324)onReaderMenuBack(317-317)onReaderMenuConfirm(389-506)onReaderMenuConfirm(389-389)applyOrientation(508-533)applyOrientation(508-508)toggleAutoPageTurn(535-555)toggleAutoPageTurn(535-535)
src/activities/reader/EpubReaderActivity.cpp (3)
src/activities/ActivityWithSubactivity.cpp (4)
requestUpdate(39-44)requestUpdate(39-39)exitActivity(17-24)exitActivity(17-17)src/activities/Activity.cpp (2)
requestUpdate(39-45)requestUpdate(39-39)src/main.cpp (3)
exitActivity(133-139)exitActivity(133-133)renderer(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cppcheck
🔇 Additional comments (11)
src/activities/reader/EpubReaderActivity.h (1)
16-39: Auto page-turn state plumbing looks good.The added fields and method signatures align with the new behavior in the .cpp and appear consistent.
src/activities/reader/EpubReaderMenuActivity.cpp (2)
122-127: Rendering of the page-turn value looks good.The right-aligned value mirrors the rotate-screen UI and is clear.
44-61: Remove the concern about AUTO_PAGE_TURN not updating the UI.The render method already includes rendering code for AUTO_PAGE_TURN (lines 122–126) that directly reads
selectedPageTurnOption. Changes to this state are reflected immediately on the next render call, so the UI will update correctly. TheupdateRequired = true;assignment appears to be dead code (the variable is not declared as a class member), but it does not prevent rendering.Likely an incorrect or invalid review comment.
src/activities/reader/EpubReaderActivity.cpp (8)
27-28: PPM label table wiring looks fine.The constant aligns with the OFF/1/3/6/12 PPM options and avoids divide-by-zero.
167-196: Auto-turn loop behavior looks consistent.The disable-on-input and timed advance paths are straightforward and guarded.
258-301: Timer reset on manual navigation is a good touch.Resetting
lastPageTurnTimeon manual skips/turns keeps the cadence stable.
317-323: Menu back now applies auto-turn selection as expected.The orientation apply + auto-turn toggle sequencing is consistent with menu intent.
535-555: Auto-turn toggle logic looks solid.The enable/disable flow and reflow trigger for status-bar-none are clear.
577-579: End-of-book auto-turn shutdown is correct.Disabling auto-turn when the last spine is reached prevents runaway refresh.
601-604: Margin reservation for indicator is a good UX safeguard.Ensures the indicator won’t collide with content when the status bar is hidden.
855-862: Title truncation logic is reasonable.The fallback centering and truncation path handles tight layouts gracefully.
🤖 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 823-839: Replace the hard-coded string "Auto Turn Enabled: …" with
a localized string lookup and formatted insertion: create or use a string
resource ID (e.g., STR_AUTO_TURN_ENABLED) and build title via
tr(STR_AUTO_TURN_ENABLED) combined with the computed PPM value (using
std::to_string or a formatting helper), so the automaticPageTurnActive branch
uses tr(...) instead of a literal; update the code paths around
automaticPageTurnActive, pageTurnDuration, title, and
renderer.getTextWidth(SMALL_FONT_ID, ...) to use the localized title.
---
Outside diff comments:
In `@src/activities/reader/EpubReaderMenuActivity.h`:
- Around line 15-47: The fixed menu layout in EpubReaderMenuActivity declares
MenuAction::AUTO_PAGE_TURN but menuItems (the vector used to render and navigate
the menu) does not include it; update the const std::vector<MenuItem> menuItems
initializer to include a MenuItem with action MenuAction::AUTO_PAGE_TURN and an
appropriate label StrId (e.g., StrId::STR_AUTO_PAGE_TURN) in the desired order
so AUTO_PAGE_TURN becomes selectable and rendered by the existing
onEnter/loop/render logic.
|
Set to draft while cleaning up merge conflict |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
lib/I18n/translations/french.yaml (1)
318-323: Translate these labels or omit them to rely on the fallback.These are English in the French locale. If translations aren’t ready, consider leaving the keys out so fallback and missing-key logs remain visible.
Based on learnings: “missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script...”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/french.yaml` around lines 318 - 323, The French locale contains English labels STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and numeric options STR_1, STR_3, STR_6, STR_12; either provide French translations for these keys (e.g., translate STR_AUTO_TURN_ENABLED and STR_AUTO_TURN_PAGES_PER_MIN into proper French strings and localize numeric labels if needed) or remove these keys from french.yaml so the build-time fallback will supply the English strings and missing-key logging remains accurate; update the entries for STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, and STR_12 accordingly.lib/I18n/translations/swedish.yaml (1)
318-323: Translate these labels or omit them to rely on the fallback.These are still English in Swedish. If translations aren’t ready, consider omitting the keys to allow fallback and missing-key logging.
Based on learnings: “missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script...”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/swedish.yaml` around lines 318 - 323, The Swedish translation file contains English labels for keys STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and value keys STR_1, STR_3, STR_6, STR_12; either provide Swedish translations for these keys (replace the English strings with translated Swedish text) or remove these keys from lib/I18n/translations/swedish.yaml so the gen_i18n.py fallback will supply the English defaults and missing-key logging will trigger; ensure you update STR_AUTO_TURN_ENABLED and STR_AUTO_TURN_PAGES_PER_MIN with proper Swedish phrases and either translate the numeric label strings or omit them as described.lib/I18n/translations/portuguese.yaml (1)
318-323: Consider localizing these new strings.These entries are still English in the Portuguese locale; translating them would avoid mixed-language UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/portuguese.yaml` around lines 318 - 323, The Portuguese locale file contains English values for the keys STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, and STR_12; update these entries in lib/I18n/translations/portuguese.yaml to proper Portuguese strings (e.g., STR_AUTO_TURN_ENABLED -> "Virada automática ativada: ", STR_AUTO_TURN_PAGES_PER_MIN -> "Virada automática (páginas por minuto)" and keep the numeric labels localized or as digits as appropriate), preserving existing key names and punctuation/spacing.lib/I18n/translations/spanish.yaml (1)
318-323: Translate these labels or omit them to rely on the fallback.Right now the Spanish locale will display English. If translations aren’t ready, consider leaving these keys out so the generator falls back and logs the missing entries.
Based on learnings: “missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script...”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/spanish.yaml` around lines 318 - 323, The Spanish locale currently contains English strings for the auto-turn labels (STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN) and numeric labels (STR_1, STR_3, STR_6, STR_12); either provide proper Spanish translations for those keys (e.g., translate STR_AUTO_TURN_ENABLED and STR_AUTO_TURN_PAGES_PER_MIN to Spanish and localize numeric labels if desired) or remove these keys entirely so the i18n generator falls back to the default and logs them as missing; update the entries for STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, and STR_12 accordingly.lib/I18n/translations/german.yaml (1)
318-323: Translate these labels or omit them to rely on the fallback.Currently the German locale will show English. If translations aren’t ready, consider removing these entries so the generator uses the English fallback and logs the missing keys.
Based on learnings: “missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script...”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/german.yaml` around lines 318 - 323, The German locale currently contains English strings for STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and numeric labels STR_1, STR_3, STR_6, STR_12; either provide proper German translations for these keys (e.g., translate the phrases for "Auto Turn Enabled" and "Auto Turn (Pages Per Minute)" and localize labels if needed) or remove those keys so the build-time gen_i18n.py fallback will supply the English values and log the missing keys; update the german.yaml by editing the entries for STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, STR_12 accordingly.lib/I18n/translations/czech.yaml (1)
318-323: Translate these labels or omit them to rely on the fallback.They’re currently English in Czech. If translations aren’t ready, omitting the keys keeps the fallback behavior and missing-key logging.
Based on learnings: “missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script...”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/I18n/translations/czech.yaml` around lines 318 - 323, The Czech file contains English labels for STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and the numeric labels STR_1, STR_3, STR_6, STR_12; either provide Czech translations for those keys (replace the English strings with the localized Czech text) or remove those keys from lib/I18n/translations/czech.yaml so the build-time gen_i18n.py fallback to English and missing-key logging are used instead; update only those keys (STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, STR_12) to avoid introducing new keys or changing other translations.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/I18n/I18nKeys.hlib/I18n/translations/czech.yamllib/I18n/translations/english.yamllib/I18n/translations/french.yamllib/I18n/translations/german.yamllib/I18n/translations/portuguese.yamllib/I18n/translations/russian.yamllib/I18n/translations/spanish.yamllib/I18n/translations/swedish.yamlsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-02-16T22:25:35.674Z
Learnt from: whyte-j
Repo: crosspoint-reader/crosspoint-reader PR: 733
File: lib/I18n/I18nKeys.h:271-272
Timestamp: 2026-02-16T22:25:35.674Z
Learning: In the crosspoint-reader i18n system (lib/I18n/), missing translation keys in non-English YAML files are automatically filled with English strings at build time by the gen_i18n.py script. All generated string arrays (STRINGS_EN, STRINGS_ES, etc.) have identical lengths, so runtime array indexing is safe without per-string fallback logic. The system prints "INFO: '{key}' missing in {lang_code}, using English fallback" during code generation when this occurs.
Applied to files:
lib/I18n/translations/czech.yamllib/I18n/translations/german.yamllib/I18n/translations/russian.yamllib/I18n/translations/english.yamllib/I18n/translations/swedish.yamllib/I18n/translations/french.yamllib/I18n/translations/spanish.yaml
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderActivity.cpp
📚 Learning: 2026-02-16T22:59:07.687Z
Learnt from: whyte-j
Repo: crosspoint-reader/crosspoint-reader PR: 733
File: src/components/StatusBar.cpp:63-78
Timestamp: 2026-02-16T22:59:07.687Z
Learning: In the CrossPoint Reader codebase, screen margin values are preset in settings with constrained options, so calculations involving margins (like `rendererableScreenWidth` or `availableTitleSpace` in StatusBar rendering) are unlikely to produce negative values in practice given these bounds.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
🧬 Code graph analysis (2)
src/activities/reader/EpubReaderMenuActivity.cpp (3)
src/activities/Activity.cpp (2)
requestUpdate(39-45)requestUpdate(39-39)src/activities/ActivityWithSubactivity.cpp (2)
requestUpdate(39-44)requestUpdate(39-39)src/main.cpp (1)
renderer(37-37)
src/activities/reader/EpubReaderActivity.cpp (3)
src/activities/Activity.cpp (2)
requestUpdate(39-45)requestUpdate(39-39)src/activities/ActivityWithSubactivity.cpp (4)
requestUpdate(39-44)requestUpdate(39-39)exitActivity(17-24)exitActivity(17-17)src/main.cpp (2)
exitActivity(133-139)exitActivity(133-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (12)
lib/I18n/I18nKeys.h (1)
352-357: LGTM — enum extension is straightforward.The new IDs are inserted before
_COUNT, keeping the sentinel intact.lib/I18n/translations/english.yaml (1)
318-323: LGTM — base English strings added.These align with the new auto page turn UI labels.
src/activities/reader/EpubReaderMenuActivity.cpp (2)
44-48: Auto‑page‑turn option cycling and return propagation look good.
Line 44‑48 cleanly cycles the option and Line 60 returns it alongside orientation for the parent to apply.Also applies to: 60-60
121-127: Auto‑turn value rendering is consistent with other menu items.
The right‑edge value mirrors the rotate screen pattern and uses localized labels.src/activities/reader/EpubReaderMenuActivity.h (2)
15-20: Enum + onBack signature update is consistent.
The new AUTO_PAGE_TURN action and two‑parameter callback align with the menu flow.Also applies to: 64-64
41-59: Menu items and labels for auto‑turn are wired cleanly.
The fixed ordering and label list match the new option set.src/activities/reader/EpubReaderActivity.cpp (6)
27-28: PAGE_TURN_LABELS mapping is clear and safe.
Keeping an OFF placeholder aligned with menu indices makes the duration math straightforward.
263-267: Resetting lastPageTurnTime on manual navigation prevents drift.
The timer reset on skip and manual page turns keeps auto‑turn cadence consistent.Also applies to: 280-300
316-323: Menu back now applies auto‑turn selection correctly.
The updated callback wiring cleanly applies both orientation and auto‑turn settings.
534-554: toggleAutoPageTurn centralizes enable/disable cleanly.
The duration calculation and conditional reflow keep state changes in one place.
576-578: Auto‑turn safely disabled at end‑of‑book.
Prevents unintended page churn once the end screen is shown.
600-603: Indicator layout and truncation logic integrate cleanly with the status bar.
Reserving space when needed and truncating the auto‑turn title keeps the footer stable across margins.Also applies to: 822-858
🤖 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/I18n/translations/russian.yaml`:
- Around line 318-323: The Russian locale contains English values for the
auto‑turn keys (STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1,
STR_3, STR_6, STR_12); either replace those English strings with their Russian
translations or remove these keys so the build-time fallback supplies
English—update the YAML entries for those exact keys (or delete them) to resolve
the issue.
In `@src/activities/reader/EpubReaderActivity.cpp`:
- Around line 167-195: When automaticPageTurnActive is turned off in the branch
handling mappedInput.wasReleased(MappedInputManager::Button::Confirm) ||
mappedInput.wasReleased(MappedInputManager::Button::Back), also force an
immediate reflow by resetting the current section while holding the RenderLock
(or call the same reflow path used by toggleAutoPageTurn). Specifically, inside
that branch where you set automaticPageTurnActive = false and call
requestUpdate(), acquire a RenderLock(*this) and either reset section
(section.reset()) or invoke the existing reflow helper used by
toggleAutoPageTurn so the reserved status-bar indicator space is removed
immediately rather than waiting for the next layout cycle.
---
Nitpick comments:
In `@lib/I18n/translations/czech.yaml`:
- Around line 318-323: The Czech file contains English labels for
STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and the numeric labels STR_1,
STR_3, STR_6, STR_12; either provide Czech translations for those keys (replace
the English strings with the localized Czech text) or remove those keys from
lib/I18n/translations/czech.yaml so the build-time gen_i18n.py fallback to
English and missing-key logging are used instead; update only those keys
(STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6,
STR_12) to avoid introducing new keys or changing other translations.
In `@lib/I18n/translations/french.yaml`:
- Around line 318-323: The French locale contains English labels
STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and numeric options STR_1,
STR_3, STR_6, STR_12; either provide French translations for these keys (e.g.,
translate STR_AUTO_TURN_ENABLED and STR_AUTO_TURN_PAGES_PER_MIN into proper
French strings and localize numeric labels if needed) or remove these keys from
french.yaml so the build-time fallback will supply the English strings and
missing-key logging remains accurate; update the entries for
STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, and
STR_12 accordingly.
In `@lib/I18n/translations/german.yaml`:
- Around line 318-323: The German locale currently contains English strings for
STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and numeric labels STR_1,
STR_3, STR_6, STR_12; either provide proper German translations for these keys
(e.g., translate the phrases for "Auto Turn Enabled" and "Auto Turn (Pages Per
Minute)" and localize labels if needed) or remove those keys so the build-time
gen_i18n.py fallback will supply the English values and log the missing keys;
update the german.yaml by editing the entries for STR_AUTO_TURN_ENABLED,
STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, STR_12 accordingly.
In `@lib/I18n/translations/portuguese.yaml`:
- Around line 318-323: The Portuguese locale file contains English values for
the keys STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3,
STR_6, and STR_12; update these entries in lib/I18n/translations/portuguese.yaml
to proper Portuguese strings (e.g., STR_AUTO_TURN_ENABLED -> "Virada automática
ativada: ", STR_AUTO_TURN_PAGES_PER_MIN -> "Virada automática (páginas por
minuto)" and keep the numeric labels localized or as digits as appropriate),
preserving existing key names and punctuation/spacing.
In `@lib/I18n/translations/spanish.yaml`:
- Around line 318-323: The Spanish locale currently contains English strings for
the auto-turn labels (STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN) and
numeric labels (STR_1, STR_3, STR_6, STR_12); either provide proper Spanish
translations for those keys (e.g., translate STR_AUTO_TURN_ENABLED and
STR_AUTO_TURN_PAGES_PER_MIN to Spanish and localize numeric labels if desired)
or remove these keys entirely so the i18n generator falls back to the default
and logs them as missing; update the entries for STR_AUTO_TURN_ENABLED,
STR_AUTO_TURN_PAGES_PER_MIN, STR_1, STR_3, STR_6, and STR_12 accordingly.
In `@lib/I18n/translations/swedish.yaml`:
- Around line 318-323: The Swedish translation file contains English labels for
keys STR_AUTO_TURN_ENABLED, STR_AUTO_TURN_PAGES_PER_MIN and value keys STR_1,
STR_3, STR_6, STR_12; either provide Swedish translations for these keys
(replace the English strings with translated Swedish text) or remove these keys
from lib/I18n/translations/swedish.yaml so the gen_i18n.py fallback will supply
the English defaults and missing-key logging will trigger; ensure you update
STR_AUTO_TURN_ENABLED and STR_AUTO_TURN_PAGES_PER_MIN with proper Swedish
phrases and either translate the numeric label strings or omit them as
described.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/activities/reader/EpubReaderMenuActivity.h (1)
17-28:⚠️ Potential issue | 🟠 MajorConstructor missing
currentPageTurnOptionparameter.The constructor accepts
currentOrientationand uses it to initializependingOrientation, but there's no equivalent parameter for the page turn setting. This meansselectedPageTurnOptionalways starts at 0 (OFF), regardless of the actual current state. Users won't see the correct auto page turn value when opening the menu.Proposed fix
explicit EpubReaderMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::string& title, const int currentPage, const int totalPages, const int bookProgressPercent, - const uint8_t currentOrientation, const std::function<void(uint8_t, uint8_t)>& onBack, + const uint8_t currentOrientation, const uint8_t currentPageTurnOption, + const std::function<void(uint8_t, uint8_t)>& onBack, const std::function<void(MenuAction)>& onAction) : ActivityWithSubactivity("EpubReaderMenu", renderer, mappedInput), title(title), pendingOrientation(currentOrientation), + selectedPageTurnOption(currentPageTurnOption), currentPage(currentPage), totalPages(totalPages), bookProgressPercent(bookProgressPercent), onBack(onBack), onAction(onAction) {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderMenuActivity.h` around lines 17 - 28, The constructor EpubReaderMenuActivity currently takes currentOrientation and initializes pendingOrientation but lacks a parameter for the page-turn setting, causing selectedPageTurnOption to default to 0; add a new parameter (e.g., const uint8_t currentPageTurnOption) to the constructor signature and initializer list and set selectedPageTurnOption(currentPageTurnOption) alongside pendingOrientation, then update any callers to pass the current page-turn value so the menu reflects the real setting on open.
🧹 Nitpick comments (1)
src/activities/reader/EpubReaderMenuActivity.h (1)
56-58: Inconsistent i18n pattern forpageTurnLabelsvsorientationLabels.
orientationLabelsstoresStrIdvalues and defers lookup to render time viaI18N.get(), butpageTurnLabelsstoresconst char*withI18N.get(StrId::STR_STATE_OFF)evaluated at construction. If the language changes after construction, the "OFF" label could be stale.Consider storing
StrIdvalues consistently and looking them up at render time, similar toorientationLabels.Proposed fix
In the header, change to store StrId values (or a mix with numeric string literals handled separately):
- const std::vector<const char*> pageTurnLabels = {I18N.get(StrId::STR_STATE_OFF), "1", "3", "6", "12"}; + const std::vector<StrId> pageTurnLabelIds = {StrId::STR_STATE_OFF}; + const std::vector<const char*> pageTurnNumericLabels = {"1", "3", "6", "12"};Or if you add STR_1, STR_3, etc. to your i18n keys:
- const std::vector<const char*> pageTurnLabels = {I18N.get(StrId::STR_STATE_OFF), "1", "3", "6", "12"}; + const std::vector<StrId> pageTurnLabels = {StrId::STR_STATE_OFF, StrId::STR_1, StrId::STR_3, StrId::STR_6, StrId::STR_12};Then in the render method, look up via
I18N.get(pageTurnLabels[selectedPageTurnOption]).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/activities/reader/EpubReaderMenuActivity.h` around lines 56 - 58, The pageTurnLabels vector evaluates I18N.get(StrId::STR_STATE_OFF) at construction, causing stale text if the language changes; change pageTurnLabels to store StrId values (e.g., StrId::STR_STATE_OFF and either new StrId keys for "1","3","6","12" or keep numeric options as a separate enum/consts) instead of const char*, and update the render code to call I18N.get(pageTurnLabels[selectedPageTurnOption]) (or resolve numeric labels at render time) so lookup happens at render time like orientationLabels; update any code that indexes pageTurnLabels (e.g., selectedPageTurnOption) to expect StrId entries.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/I18n/I18nKeys.hlib/I18n/translations/english.yamlsrc/activities/reader/EpubReaderMenuActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/I18n/translations/english.yaml
- lib/I18n/I18nKeys.h
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderMenuActivity.cpp
📚 Learning: 2026-02-18T15:43:07.515Z
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:07.515Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Applied to files:
src/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderMenuActivity.cpp
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In src/activities/reader/EpubReaderActivity.cpp's renderContents() method, when uncached images exist, Phase 1 intentionally calls displayBuffer(forceFullRefresh) to perform a HALF_REFRESH (if needed), while Phase 2 intentionally calls renderer.displayBuffer() directly without forceFullRefresh. This is by design: Phase 1's refresh clears the screen properly to prevent ghosting, so Phase 2 can use a faster refresh mode for better performance. The grayscale anti-aliasing is handled separately after renderContents() via displayGrayBuffer().
Applied to files:
src/activities/reader/EpubReaderMenuActivity.hsrc/activities/reader/EpubReaderMenuActivity.cpp
🧬 Code graph analysis (1)
src/activities/reader/EpubReaderMenuActivity.cpp (3)
src/activities/Activity.cpp (2)
requestUpdate(39-45)requestUpdate(39-39)src/activities/ActivityWithSubactivity.cpp (2)
requestUpdate(39-44)requestUpdate(39-39)src/main.cpp (1)
renderer(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: cppcheck
🔇 Additional comments (2)
src/activities/reader/EpubReaderMenuActivity.cpp (2)
44-48: LGTM!The cycling logic for
selectedPageTurnOptioncorrectly uses modulo to wrap around the available options, consistent with theROTATE_SCREENhandling above.
58-61: LGTM!The
onBackcallback now correctly passes bothpendingOrientationandselectedPageTurnOptionto the parent activity.
🤖 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/EpubReaderMenuActivity.cpp`:
- Around line 122-127: The inline comment above the AUTO_PAGE_TURN branch is
misleadingly referring to an "orientation value"; update that comment to
accurately describe what's being rendered (the page turn value/option) in the
block where MenuAction::AUTO_PAGE_TURN uses
pageTurnLabels[selectedPageTurnOption] and renderer.drawText to draw the value;
keep the rest of the code unchanged.
---
Outside diff comments:
In `@src/activities/reader/EpubReaderMenuActivity.h`:
- Around line 17-28: The constructor EpubReaderMenuActivity currently takes
currentOrientation and initializes pendingOrientation but lacks a parameter for
the page-turn setting, causing selectedPageTurnOption to default to 0; add a new
parameter (e.g., const uint8_t currentPageTurnOption) to the constructor
signature and initializer list and set
selectedPageTurnOption(currentPageTurnOption) alongside pendingOrientation, then
update any callers to pass the current page-turn value so the menu reflects the
real setting on open.
---
Nitpick comments:
In `@src/activities/reader/EpubReaderMenuActivity.h`:
- Around line 56-58: The pageTurnLabels vector evaluates
I18N.get(StrId::STR_STATE_OFF) at construction, causing stale text if the
language changes; change pageTurnLabels to store StrId values (e.g.,
StrId::STR_STATE_OFF and either new StrId keys for "1","3","6","12" or keep
numeric options as a separate enum/consts) instead of const char*, and update
the render code to call I18N.get(pageTurnLabels[selectedPageTurnOption]) (or
resolve numeric labels at render time) so lookup happens at render time like
orientationLabels; update any code that indexes pageTurnLabels (e.g.,
selectedPageTurnOption) to expect StrId entries.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/I18n/I18nKeys.hlib/I18n/translations/english.yamlsrc/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderActivity.hsrc/activities/reader/EpubReaderMenuActivity.h
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/I18n/I18nKeys.h
- src/activities/reader/EpubReaderActivity.h
- lib/I18n/translations/english.yaml
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-02-22T19:13:14.049Z
Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1016
File: src/activities/reader/EpubReaderActivity.cpp:138-146
Timestamp: 2026-02-22T19:13:14.049Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, the EpubReaderMenuActivity result callback intentionally applies orientation changes (via applyOrientation) before checking result.isCancelled. This differs from other callbacks in the file because orientation is treated as an immediate setting that should persist even when the user cancels the menu, rather than a deferred action requiring confirmation.
Applied to files:
src/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
📚 Learning: 2026-02-16T20:43:19.339Z
Learnt from: Levrk
Repo: crosspoint-reader/crosspoint-reader PR: 909
File: src/activities/reader/EpubReaderActivity.cpp:461-461
Timestamp: 2026-02-16T20:43:19.339Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, when using ConfirmationActivity with enterNewActivity(), it's not necessary to call exitActivity() beforehand. The ConfirmationActivity operates as a modal dialog and cleanup is handled via the pendingSubactivityExit flag in the callback lambda. This differs from other activities like EpubReaderChapterSelectionActivity or KOReaderSyncActivity which do require exitActivity() before enterNewActivity().
Applied to files:
src/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
📚 Learning: 2026-02-21T16:47:45.578Z
Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1031
File: src/activities/reader/EpubReaderActivity.cpp:816-853
Timestamp: 2026-02-21T16:47:45.578Z
Learning: In src/activities/reader/EpubReaderActivity.cpp, the navigateToHref() function uses a fixed-size mini-stack (MAX_FOOTNOTE_DEPTH = 3) for saved positions rather than a dynamic stack. This is intentional to minimize RAM usage on ESP32-C3, which has limited memory. The design accepts that failed navigation at max depth may decrement footnoteDepth even when no push occurred, as a tradeoff for memory efficiency. Users exit by pressing Back three times maximum.
Applied to files:
src/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
📚 Learning: 2026-02-19T17:46:26.871Z
Learnt from: Tritlo
Repo: crosspoint-reader/crosspoint-reader PR: 1003
File: src/activities/reader/EpubReaderActivity.cpp:657-674
Timestamp: 2026-02-19T17:46:26.871Z
Learning: In EpubReaderActivity.cpp, renderContents() behavior is intentional: if uncached images exist, Phase 1 should call displayBuffer(forceFullRefresh) to perform a HALF_REFRESH and clear ghosting; Phase 2 should call renderer.displayBuffer() without forceFullRefresh for a faster refresh. Grayscale anti-aliasing is applied separately after renderContents() via displayGrayBuffer(). This guidance applies specifically to this file and situation.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
📚 Learning: 2026-02-18T15:43:12.258Z
Learnt from: GavinHigham
Repo: crosspoint-reader/crosspoint-reader PR: 966
File: src/activities/ActivityWithSubactivity.h:20-21
Timestamp: 2026-02-18T15:43:12.258Z
Learning: In the crosspoint-reader activity architecture, when using ActivityWithSubactivity, the currentActivity global variable remains at the top-level activity (e.g., ReaderActivity) even when nested subactivities are active. The supportsOrientation() delegation chain starts from the top-level activity and stops when it reaches a subactivity that overrides the method (e.g., EpubReaderActivity returns true). Deeper nested subactivities (like EpubReaderChapterSelectionActivity or EpubReaderPercentSelectionActivity) do not need to override supportsOrientation() because they never become the currentActivity and the delegation chain doesn't reach them.
Applied to files:
src/activities/reader/EpubReaderActivity.cppsrc/activities/reader/EpubReaderMenuActivity.h
📚 Learning: 2026-02-16T22:59:07.687Z
Learnt from: whyte-j
Repo: crosspoint-reader/crosspoint-reader PR: 733
File: src/components/StatusBar.cpp:63-78
Timestamp: 2026-02-16T22:59:07.687Z
Learning: In the CrossPoint Reader codebase, screen margin values are preset in settings with constrained options, so calculations involving margins (like `rendererableScreenWidth` or `availableTitleSpace` in StatusBar rendering) are unlikely to produce negative values in practice given these bounds.
Applied to files:
src/activities/reader/EpubReaderActivity.cpp
🧬 Code graph analysis (1)
src/activities/reader/EpubReaderActivity.cpp (3)
src/activities/Activity.cpp (2)
requestUpdate(42-48)requestUpdate(42-42)src/activities/ActivityWithSubactivity.cpp (4)
requestUpdate(42-47)requestUpdate(42-42)exitActivity(20-27)exitActivity(20-20)src/main.cpp (2)
exitActivity(136-142)exitActivity(136-136)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: cppcheck
🔇 Additional comments (12)
src/activities/reader/EpubReaderMenuActivity.h (4)
15-23: AUTO_PAGE_TURN action fits cleanly into the menu actions list.
26-29: Two-parameteronBacksignature is consistently reflected in the API.Also applies to: 73-73
50-58: Menu layout update properly exposes the auto page turn entry.
65-68: Page-turn option state and labels are straightforward and clear.src/activities/reader/EpubReaderActivity.cpp (8)
28-29: Centralized PAGE_TURN_LABELS mapping is clear and readable.
168-196: Auto‑turn loop handling reads correctly (disable on input, advance on timeout).
210-214: Menu callback now forwards the selected auto‑turn option as expected.
264-267: ResettinglastPageTurnTimeon manual navigation avoids immediate auto‑advance.Also applies to: 281-284, 299-301
317-323: Menu back handler now wires auto‑turn toggling alongside orientation apply.
586-588: Auto‑turn disabling at end‑of‑book is a good safety guard.
610-612: Reserving space for the indicator when the status bar is hidden makes sense.
856-892: Auto‑turn status title rendering + truncation logic looks solid.
🤖 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 544-564: Clamp or validate selectedPageTurnOption before using it
to index PAGE_TURN_LABELS and perform the section.reset() while holding the
render lock to avoid races: in EpubReaderActivity::toggleAutoPageTurn, if
selectedPageTurnOption != 0 first clamp it to the valid range (e.g. between 1
and PAGE_TURN_LABELS_COUNT-1 or use std::min/std::max) and use the clamped value
when computing pageTurnDuration; additionally wrap the block that accesses and
resets section (the cachedSpineIndex/cachedChapterTotalPageCount/nextPageNumber
assignments and section.reset()) with a RenderLock (use the existing RenderLock
RAII type) so section.reset() cannot race with rendering.
|
Closed due to messy commit history. Opened #1219 as replacement |
## Summary * **What is the goal of this PR?** (e.g., Implements the new feature for file uploading.) - Implements auto page turn feature for epub reader in the reader submenu * **What changes are included?** - added auto page turn feature in epub reader in the submenu - currently there are 5 settings, `OFF, 1, 3, 6, 12` pages per minute ## Additional Context * Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks, specific areas to focus on). - Replacement PR for #723 - when auto turn is enabled, space reserved for chapter title will be used to indicate auto page turn being active - Back and Confirm button is used to disable it --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**Partially (mainly code reviews)**_
6 participants
Summary
What is the goal of this PR? (e.g., Implements the new feature for file uploading.)
What changes are included?
OFF, 1, 3, 6, 12pages per minuteAdditional Context
specific areas to focus on).
AI Usage
While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.
Did you use AI tools to help write this code? NO