cottongin/crosspoint-reader-mod
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: move render() to Activity super class, use freeRTOS notification (#774)

Browse Source 
Currently, each activity has to manage their own `displayTaskLoop` which
adds redundant boilerplate code. The loop is a wait loop which is also
not the best practice, as the `updateRequested` boolean is not protected
by a mutex.

In this PR:
- Move `displayTaskLoop` to the super `Activity` class
- Replace `updateRequested` with freeRTOS's [direct to task
notification](https://www.freertos.org/Documentation/02-Kernel/02-Kernel-features/03-Direct-to-task-notifications/01-Task-notifications)
- For `ActivityWithSubactivity`, whenever a sub-activity is present, the
parent's `render()` automatically goes inactive

With this change, activities now only need to expose `render()`
function, and anywhere in the code base can call `requestUpdate()` to
request a new rendering pass.

In theory, this change may also make the battery life a bit better,
since one wait loop is removed. Although the equipment in my home lab
wasn't been able to verify it (the electric current is too noisy and
small). Would appreciate if anyone has any insights on this subject.

Update: I managed to hack [a small piece of
code](https://github.com/ngxson/crosspoint-reader/tree/xsn/measure_cpu_usage)
that allow tracking CPU idle time.

The CPU load does decrease a bit (1.47% down to 1.39%), which make
sense, because the display task is now sleeping most of the time unless
notified. This should translate to a slightly increase in battery life
in the long run.

```
PR:
[40012] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[40012] [IDLE] Idle time: 98.61% (CPU load: 1.39%)
[50017] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[50017] [IDLE] Idle time: 98.61% (CPU load: 1.39%)
[60022] [MEM] Free: 185856 bytes, Total: 231004 bytes, Min Free: 123316 bytes
[60022] [IDLE] Idle time: 98.61% (CPU load: 1.39%)

master:
[20012] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[20012] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
[30017] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[30017] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
[40022] [MEM] Free: 195016 bytes, Total: 231532 bytes, Min Free: 132460 bytes
[40022] [IDLE] Idle time: 98.53% (CPU load: 1.47%)
```

---

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**

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

* **Refactor**
* Streamlined rendering architecture by consolidating update mechanisms
across all activities, improving efficiency and consistency.
* Modernized synchronization patterns for display updates to ensure
reliable, conflict-free rendering.

* **Bug Fixes**
* Enhanced rendering stability through improved locking mechanisms and
explicit update requests.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: znelson <znelson@users.noreply.github.com>
This commit is contained in:
Xuan-Son Nguyen
2026-02-16 11:11:15 +01:00
committed by cottongin
parent 12cc7de49e
commit ed8a0feac1
53 changed files with 511 additions and 1462 deletions
Download Patch File Download Diff File Expand all files Collapse all files

182
src/activities/reader/EpubReaderActivity.cpp
View File

@@ -67,11 +67,6 @@ void applyReaderOrientation(GfxRenderer& renderer, const uint8_t orientation) {
} // namespace
void EpubReaderActivity::taskTrampoline(void* param) {
auto* self = static_cast<EpubReaderActivity*>(param);
self->displayTaskLoop();
}
void EpubReaderActivity::onEnter() {
ActivityWithSubactivity::onEnter();
@@ -83,8 +78,6 @@ void EpubReaderActivity::onEnter() {
// NOTE: This affects layout math and must be applied before any render calls.
applyReaderOrientation(renderer, SETTINGS.orientation);
renderingMutex = xSemaphoreCreateMutex();
epub->setupCacheDir();
FsFile f;
@@ -179,14 +172,7 @@ void EpubReaderActivity::onEnter() {
RECENT_BOOKS.addBook(epub->getPath(), epub->getTitle(), epub->getAuthor(), epub->getThumbBmpPath());
// Trigger first update
updateRequired = true;
xTaskCreate(&EpubReaderActivity::taskTrampoline, "EpubReaderActivityTask",
8192, // Stack size
this, // Parameters
1, // Priority
&displayTaskHandle // Task handle
);
requestUpdate();
}
void EpubReaderActivity::onExit() {
@@ -195,14 +181,6 @@ void EpubReaderActivity::onExit() {
// Reset orientation back to portrait for the rest of the UI
renderer.setOrientation(GfxRenderer::Orientation::Portrait);
// Wait until not rendering to delete task to avoid killing mid-instruction to EPD
xSemaphoreTake(renderingMutex, portMAX_DELAY);
if (displayTaskHandle) {
vTaskDelete(displayTaskHandle);
displayTaskHandle = nullptr;
}
vSemaphoreDelete(renderingMutex);
renderingMutex = nullptr;
APP_STATE.readerActivityLoadCount = 0;
APP_STATE.saveToFile();
section.reset();
@@ -217,7 +195,7 @@ void EpubReaderActivity::loop() {
if (pendingSubactivityExit) {
pendingSubactivityExit = false;
exitActivity();
updateRequired = true;
requestUpdate();
skipNextButtonCheck = true; // Skip button processing to ignore stale events
}
// Deferred go home: process after subActivity->loop() returns to avoid race condition
@@ -257,8 +235,6 @@ void EpubReaderActivity::loop() {
// Enter reader menu activity.
if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) {
// Don't start activity transition while rendering
xSemaphoreTake(renderingMutex, portMAX_DELAY);
const int currentPage = section ? section->currentPage + 1 : 0;
const int totalPages = section ? section->pageCount : 0;
float bookProgress = 0.0f;
@@ -276,7 +252,6 @@ void EpubReaderActivity::loop() {
SETTINGS.orientation, hasDictionary, isBookmarked, epub->getCachePath(),
[this](const uint8_t orientation) { onReaderMenuBack(orientation); },
[this](EpubReaderMenuActivity::MenuAction action) { onReaderMenuConfirm(action); }));
xSemaphoreGive(renderingMutex);
}
// Long press BACK (1s+) goes to file selection
@@ -313,7 +288,7 @@ void EpubReaderActivity::loop() {
if (currentSpineIndex > 0 && currentSpineIndex >= epub->getSpineItemsCount()) {
currentSpineIndex = epub->getSpineItemsCount() - 1;
nextPageNumber = UINT16_MAX;
updateRequired = true;
requestUpdate();
return;
}
@@ -326,13 +301,13 @@ void EpubReaderActivity::loop() {
currentSpineIndex = nextTriggered ? currentSpineIndex + 1 : currentSpineIndex - 1;
section.reset();
xSemaphoreGive(renderingMutex);
updateRequired = true;
requestUpdate();
return;
}
// No current section, attempt to rerender the book
if (!section) {
updateRequired = true;
requestUpdate();
return;
}
@@ -347,7 +322,7 @@ void EpubReaderActivity::loop() {
section.reset();
xSemaphoreGive(renderingMutex);
}
updateRequired = true;
requestUpdate();
} else {
if (section->currentPage < section->pageCount - 1) {
section->currentPage++;
@@ -359,7 +334,7 @@ void EpubReaderActivity::loop() {
section.reset();
xSemaphoreGive(renderingMutex);
}
updateRequired = true;
requestUpdate();
}
}
@@ -370,7 +345,7 @@ void EpubReaderActivity::onReaderMenuBack(const uint8_t orientation) {
applyOrientation(orientation);
// Force a half refresh on the next render to clear menu/popup artifacts
pagesUntilFullRefresh = 1;
updateRequired = true;
requestUpdate();
}
// Translate an absolute percent into a spine index plus a normalized position
@@ -426,7 +401,7 @@ void EpubReaderActivity::jumpToPercent(int percent) {
pendingSpineProgress = 1.0f;
}
// Reset state so renderScreen() reloads and repositions on the target spine.
// Reset state so render() reloads and repositions on the target spine.
xSemaphoreTake(renderingMutex, portMAX_DELAY);
currentSpineIndex = targetSpineIndex;
nextPageNumber = 0;
@@ -506,7 +481,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
// and next menu open will reflect the updated state.
exitActivity();
pagesUntilFullRefresh = 1;
updateRequired = true;
requestUpdate();
break;
}
case EpubReaderMenuActivity::MenuAction::REMOVE_BOOKMARK: {
@@ -519,7 +494,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
vTaskDelay(750 / portTICK_PERIOD_MS);
exitActivity();
pagesUntilFullRefresh = 1;
updateRequired = true;
requestUpdate();
break;
}
case EpubReaderMenuActivity::MenuAction::GO_TO_BOOKMARK: {
@@ -533,13 +508,12 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
const int spineIdx = currentSpineIndex;
const std::string path = epub->getPath();
xSemaphoreTake(renderingMutex, portMAX_DELAY);
exitActivity();
enterNewActivity(new EpubReaderChapterSelectionActivity(
this->renderer, this->mappedInput, epub, path, spineIdx, currentP, totalP,
[this] {
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex) {
if (currentSpineIndex != newSpineIndex) {
@@ -548,7 +522,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex, const int newPage) {
if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) {
@@ -557,21 +531,19 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
}
// If no TOC either, just return to reader (menu already closed by callback)
break;
}
xSemaphoreTake(renderingMutex, portMAX_DELAY);
exitActivity();
enterNewActivity(new EpubReaderBookmarkSelectionActivity(
this->renderer, this->mappedInput, epub, std::move(bookmarks), epub->getCachePath(),
[this] {
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex, const int newPage) {
if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) {
@@ -580,9 +552,8 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::DELETE_DICT_CACHE: {
@@ -608,8 +579,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
const int spineIdx = currentSpineIndex;
const std::string path = epub->getPath();
xSemaphoreTake(renderingMutex, portMAX_DELAY);
// 1. Close the menu
exitActivity();
@@ -618,7 +587,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
this->renderer, this->mappedInput, epub, path, spineIdx, currentP, totalP,
[this] {
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex) {
if (currentSpineIndex != newSpineIndex) {
@@ -627,7 +596,7 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
},
[this](const int newSpineIndex, const int newPage) {
if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) {
@@ -636,10 +605,9 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
section.reset();
}
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::GO_TO_PERCENT: {
@@ -650,7 +618,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
bookProgress = epub->calculateProgress(currentSpineIndex, chapterProgress) * 100.0f;
}
const int initialPercent = clampPercent(static_cast<int>(bookProgress + 0.5f));
xSemaphoreTake(renderingMutex, portMAX_DELAY);
exitActivity();
enterNewActivity(new EpubReaderPercentSelectionActivity(
renderer, mappedInput, initialPercent,
@@ -658,59 +625,65 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
// Apply the new position and exit back to the reader.
jumpToPercent(percent);
exitActivity();
updateRequired = true;
requestUpdate();
},
[this]() {
// Cancel selection and return to the reader.
exitActivity();
updateRequired = true;
requestUpdate();
}));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::LOOKUP: {
xSemaphoreTake(renderingMutex, portMAX_DELAY);
// Compute margins (same logic as renderScreen)
// Gather data we need while holding the render lock
int orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft;
renderer.getOrientedViewableTRBL(&orientedMarginTop, &orientedMarginRight, &orientedMarginBottom,
&orientedMarginLeft);
orientedMarginTop += SETTINGS.screenMargin;
orientedMarginLeft += SETTINGS.screenMargin;
orientedMarginRight += SETTINGS.screenMargin;
orientedMarginBottom += SETTINGS.screenMargin;
if (SETTINGS.statusBar != CrossPointSettings::STATUS_BAR_MODE::NONE) {
auto metrics = UITheme::getInstance().getMetrics();
const bool showProgressBar =
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::BOOK_PROGRESS_BAR ||
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::ONLY_BOOK_PROGRESS_BAR ||
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::CHAPTER_PROGRESS_BAR;
orientedMarginBottom += statusBarMargin - SETTINGS.screenMargin +
(showProgressBar ? (metrics.bookProgressBarHeight + progressBarMarginTop) : 0);
}
// Load the current page
auto pageForLookup = section ? section->loadPageFromSectionFile() : nullptr;
const int readerFontId = SETTINGS.getReaderFontId();
const std::string bookCachePath = epub->getCachePath();
const uint8_t currentOrientation = SETTINGS.orientation;
// Get first word of next page for cross-page hyphenation
std::unique_ptr<Page> pageForLookup;
int readerFontId;
std::string bookCachePath;
uint8_t currentOrientation;
std::string nextPageFirstWord;
if (section && section->currentPage < section->pageCount - 1) {
int savedPage = section->currentPage;
section->currentPage = savedPage + 1;
auto nextPage = section->loadPageFromSectionFile();
section->currentPage = savedPage;
if (nextPage && !nextPage->elements.empty()) {
const auto* firstLine = static_cast<const PageLine*>(nextPage->elements[0].get());
if (firstLine->getBlock() && !firstLine->getBlock()->getWords().empty()) {
nextPageFirstWord = firstLine->getBlock()->getWords().front();
{
RenderLock lock(*this);
// Compute margins (same logic as render)
renderer.getOrientedViewableTRBL(&orientedMarginTop, &orientedMarginRight, &orientedMarginBottom,
&orientedMarginLeft);
orientedMarginTop += SETTINGS.screenMargin;
orientedMarginLeft += SETTINGS.screenMargin;
orientedMarginRight += SETTINGS.screenMargin;
orientedMarginBottom += SETTINGS.screenMargin;
if (SETTINGS.statusBar != CrossPointSettings::STATUS_BAR_MODE::NONE) {
auto metrics = UITheme::getInstance().getMetrics();
const bool showProgressBar =
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::BOOK_PROGRESS_BAR ||
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::ONLY_BOOK_PROGRESS_BAR ||
SETTINGS.statusBar == CrossPointSettings::STATUS_BAR_MODE::CHAPTER_PROGRESS_BAR;
orientedMarginBottom += statusBarMargin - SETTINGS.screenMargin +
(showProgressBar ? (metrics.bookProgressBarHeight + progressBarMarginTop) : 0);
}
// Load the current page
pageForLookup = section ? section->loadPageFromSectionFile() : nullptr;
readerFontId = SETTINGS.getReaderFontId();
bookCachePath = epub->getCachePath();
currentOrientation = SETTINGS.orientation;
// Get first word of next page for cross-page hyphenation
if (section && section->currentPage < section->pageCount - 1) {
int savedPage = section->currentPage;
section->currentPage = savedPage + 1;
auto nextPage = section->loadPageFromSectionFile();
section->currentPage = savedPage;
if (nextPage && !nextPage->elements.empty()) {
const auto* firstLine = static_cast<const PageLine*>(nextPage->elements[0].get());
if (firstLine->getBlock() && !firstLine->getBlock()->getWords().empty()) {
nextPageFirstWord = firstLine->getBlock()->getWords().front();
}
}
}
}
// Lock released — safe to call enterNewActivity which takes its own lock
exitActivity();
if (pageForLookup) {
@@ -718,18 +691,13 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
renderer, mappedInput, std::move(pageForLookup), readerFontId, orientedMarginLeft, orientedMarginTop,
bookCachePath, currentOrientation, [this]() { pendingSubactivityExit = true; }, nextPageFirstWord));
}
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::LOOKED_UP_WORDS: {
xSemaphoreTake(renderingMutex, portMAX_DELAY);
exitActivity();
enterNewActivity(new LookedUpWordsActivity(
renderer, mappedInput, epub->getCachePath(), SETTINGS.getReaderFontId(), SETTINGS.orientation,
[this]() { pendingSubactivityExit = true; }, [this]() { pendingSubactivityExit = true; }));
xSemaphoreGive(renderingMutex);
break;
}
case EpubReaderMenuActivity::MenuAction::GO_HOME: {
@@ -765,7 +733,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
}
case EpubReaderMenuActivity::MenuAction::SYNC: {
if (KOREADER_STORE.hasCredentials()) {
xSemaphoreTake(renderingMutex, portMAX_DELAY);
const int currentPage = section ? section->currentPage : 0;
const int totalPages = section ? section->pageCount : 0;
exitActivity();
@@ -784,7 +751,6 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
}
pendingSubactivityExit = true;
}));
xSemaphoreGive(renderingMutex);
}
break;
}
@@ -821,20 +787,8 @@ void EpubReaderActivity::applyOrientation(const uint8_t orientation) {
xSemaphoreGive(renderingMutex);
}
void EpubReaderActivity::displayTaskLoop() {
while (true) {
if (updateRequired) {
updateRequired = false;
xSemaphoreTake(renderingMutex, portMAX_DELAY);
renderScreen();
xSemaphoreGive(renderingMutex);
}
vTaskDelay(10 / portTICK_PERIOD_MS);
}
}
// TODO: Failure handling
void EpubReaderActivity::renderScreen() {
void EpubReaderActivity::render(Activity::RenderLock&& lock) {
if (!epub) {
return;
}
@@ -960,7 +914,9 @@ void EpubReaderActivity::renderScreen() {
LOG_ERR("ERS", "Failed to load page from SD - clearing section cache");
section->clearCache();
section.reset();
return renderScreen();
requestUpdate(); // Try again after clearing cache
// TODO: prevent infinite loop if the page keeps failing to load for some reason
return;
}
const auto start = millis();
renderContents(std::move(p), orientedMarginTop, orientedMarginRight, orientedMarginBottom, orientedMarginLeft);