fix: drawer/carousel/BLE bundle + Indexing animation + new defaults - #9
Merged
Conversation
Bug 1: BookSettingsDrawer rendered Line Spacing and Orientation values as
"????" after the user toggled the Font setting. The values rendered
correctly on first open and again after re-opening the drawer, but every
font change re-triggered the corruption.
Root cause: `getSettingsList()` returns `std::vector<SettingInfo>` by
value. In `BookSettingsDrawerActivity::buildItems()` we iterated that
temporary with a range-for and captured `&info` into the lambda closures
stored on every Item:
for (const auto& info : getSettingsList()) {
...
const SettingInfo* infoPtr = &info;
item.getValueText = [infoPtr]() { return valueTextForSetting(*infoPtr); };
...
}
When the loop ended, the temporary vector was destroyed and every
captured `infoPtr` dangled. It happened to work right after onEnter
because the freed bytes were still intact, but the first allocation that
landed on top of that memory clobbered the SettingInfos for items past
Font (Line Spacing and Orientation are declared immediately after Font
in SettingsList.h). Their enumValues vector pointer/size then read as
garbage, I18N.get(garbage_StrId) returned the missing-key fallback, and
the value rendered as "????". Font load is a heap-heavy operation and
reliably triggered the corruption.
Fix: capture SettingInfo by value into each lambda closure. The copy is
owned by the std::function and lives as long as the Item does; the
pointer-to-member in valuePtr is stable regardless of where the copy
sits in memory. Also dropped the (write-only, unused) `const
SettingInfo* settingInfo` field from Item to remove the parallel
dangling pointer.
---
Bug 2: Removing a book via the home long-press menu's "Remove from
Recent Books" left the book visible in the Flow carousel until the next
selector move, which finally forced a re-layout.
Root cause: the RemoveFromRecentBooks handler called RECENT_BOOKS.remove
+ loadRecentBooks() to refresh the data layer, but didn't invalidate
any of the cached render flags. The Flow carousel paints from
`carouselFrames` (cached pre-rasterized covers, gated by
`carouselFramesReady`); the Lyra shelf paints from `shelfSnapshot`
(gated by `shelfSnapshotValid`). Neither knew the book set had changed,
so the next paint replayed the stale snapshot with the removed cover
still in place.
Fix: after the data-layer remove, flush every cache flag that onEnter
clears for the same reason: carouselFramesReady, shelfCoversLoaded,
shelfPathsCache (via invalidateShelfPathsCache), shelfSnapshotValid,
lastRenderedCoverSelectorValid. Next paint regenerates everything from
the updated recentBooks list, removed cover gone immediately.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drawer white background (Bug 3) - GfxRenderer::storeBwBuffer() used to free the existing backup before allocating the new one. Under heap pressure (NimBLE eats 58 KB) the malloc would fail and we'd be left with no backup at all, forcing the drawer's renderDrawer() into clearScreen() as a fallback -> stark white background behind the panel. Now allocate-first; only free the previous backup on success. - Exposed bool hasStoredBwBuffer() so the drawer can fall back to the reader's already-stored backup if its own storeBwBuffer() failed. - BookSettingsDrawerActivity::renderDrawer() now tiers fallbacks: our own snapshot -> reader's existing backup -> leave framebuffer as-is. Never clearScreen() -- whatever the e-ink is currently showing is a better starting point than white for the fast-refresh diff. BLE drop + reconnect around re-layouts (Bug 4) - Moved the BLE-drop trigger into render()'s cache-miss path, before createSectionFile. Catches every re-layout uniformly (drawer settings change, chapter boundary advance, percent jump, anything that resets section). Avoids the original whack-a-mole of patching individual section.reset() call sites. - Uses requestDisableLater() + return; deferred so the main loop's tryDisableIfRequested() drain runs *before* the next render. Can't disable() inline from render() -- holds RenderLock, and NimBLE teardown callbacks can call requestUpdateAndWait() which trips the lock-held assertion. Resets the freshly-constructed Section back to null on the early return so the next render iteration re-enters the construct+build path (without this, render saw a non-null section with zero pages and showed "empty chapter"). - Added requestEnableLater() / tryEnableIfRequested() companion to the existing requestDisableLater() / tryDisableIfRequested() pair. The enable drain re-initializes NimBLE *and* directly connectToDevice()s the bonded remote, bypassing checkAutoReconnect()'s local-button-press gate -- programmatic drop expects programmatic reconnect, not a user having to mash a local button to wake the bonded-remote restore. - EpubReaderActivity drains bleAutoReEnableAfterReindex on both the success and the permanent-failure paths so a build error doesn't permanently strand the user without their remote. The reactive chapter-abort retry path also opts into the same re-enable hook. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Indexing popup animation
- ChapterHtmlSlimParser ticks the popupFn callback every ~250 ms
through the parse loop, instead of only once at the start. Big
chapters used to render the popup once and freeze for 10+ seconds.
- EpubReaderActivity's popupFn lambda cycles trailing dots:
"Indexing" -> "Indexing." -> "Indexing.." -> "Indexing..." -> repeat.
- drawPopup gained two optional params on BaseTheme / LyraTheme:
minTextWidth -> floor on the box's text-width measurement so
animated frames share one stable box size (period
vs space glyph widths in Inter differ enough that
a naive padded string still pulses the box).
leftAlignText -> anchor the text at the box's left margin instead
of centering it. Used for the dots animation so
"Indexing" stays pinned and only the trailing
dots cycle in/out to its right; the word itself
doesn't shift left/right per frame.
Defaults (first-boot only; existing settings.json values still win)
- uiTheme: LYRA -> LYRA_FLOW. The 3D-perspective book carousel from
CrossInk Carousel is the visual centerpiece of the fork and what
most new users will want first.
- cycleScreensaverOnTap: 0 -> 1. The on-demand sleep-screen cycler is
one of CrumBLE's headline features; opt-out makes more sense than
opt-in. Battery cost per cycle (one boot + e-ink half-refresh) is
small enough to be worth defaulting on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The chapter parser's do-while loop pulls PARSE_BUFFER_SIZE chunks from the file and feeds each to XML_ParseBuffer, whose element/character callbacks run synchronously (page layout, image extraction, etc.). Long-running callbacks on a single chunk could: 1. Starve the FreeRTOS task watchdog -> reset (user perceives as "freeze then reboot"). 2. Starve the main task's input + activity-pop processing -> user presses Back during a heavy parse and nothing happens until the chunk finishes; if they spam Back rapidly the events queue up and the resulting cascade of section.reset / re-render / BLE drop / reconnect cycles can panic the system. Adding yield() per chunk gives the scheduler a chance to feed the watchdog and run the input/activity tasks even on chapters whose handlers grind for several seconds. Verified by reproducing both the chapter-back hang and the rapid-Back-spam crash; both go away with the yield() in place. Also added a per-chunk LOG_DBG line with file offset, read/parse ms, free heap, max alloc, and elapsed time. Compiled out at the production LOG_LEVEL=1 (no serial spam in tiny builds) but visible at debug LOG_LEVEL=2 so future hang investigations can pinpoint exactly which chunk stalls without rebuilding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
imshentastic
pushed a commit
that referenced
this pull request
May 23, 2026
imshentastic
pushed a commit
that referenced
this pull request
May 23, 2026
fix: remove unused variable
imshentastic
added a commit
that referenced
this pull request
May 23, 2026
fix: drawer/carousel/BLE bundle + Indexing animation + new defaults
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
1 participant
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
Bug-fix bundle from device testing, plus two default changes:
getSettingsList()vector; they dangled after the loop and got clobbered (most reliably by a font change's heap churn), rendering Line Spacing / Orientation values as "????". Now capturesSettingInfoby value.storeBwBuffer()freed the existing backup before a (failable) malloc; under BLE heap pressure the drawer fell back toclearScreen(). Now allocates-first and the drawer tiers fallbacks (own snapshot → reader's backup → leave framebuffer).yield()per parse chunk (fixes watchdog/starvation hangs on heavy chapters and rapid-Back spam).Test plan