fix: BookInfo performance — Y-culling, newline normalization, cover clamping
Addressed critical render performance issues identified via device debug log: - Add Y-culling in render() to skip off-screen draw calls (was causing 337K LOG_ERR calls per frame, 7-13s render times) - Normalize description whitespace (strip embedded \n/\r/\t) to prevent "No glyph for codepoint 10" errors - Clamp cover bitmap maxHeight to prevent drawing beyond screen edge - Pre-compute layout in onEnter() with InfoField struct (wrappedText called once, not per frame) - Add cover image display via generateThumbBmp + drawBitmap1Bit Made-with: Cursor
This commit is contained in:
38
chat-summaries/2026-03-09_00-44-summary.md
Normal file
38
chat-summaries/2026-03-09_00-44-summary.md
Normal file
@@ -0,0 +1,38 @@
|
||||
# BookInfoActivity: Performance Fix and Cover Image
|
||||
|
||||
## Task
|
||||
Fix BookInfoActivity sluggishness (slow open, unresponsive scrolling) and add book cover display.
|
||||
|
||||
## Root Cause (from device debug log)
|
||||
1. **No Y-culling in render()**: All text lines drawn even when off-screen. Content extending ~1162px on 800px screen caused hundreds of thousands of `LOG_ERR("Outside range")` calls per frame, each doing serial I/O. Render times: 7-13 seconds per frame.
|
||||
2. **Description text contained literal newlines**: `stripHtml()` and `trim()` in ContentOpfParser don't replace interior `\n` characters. These got passed to `drawText()`, triggering "No glyph for codepoint 10" errors.
|
||||
3. **`wrappedText()` recomputed every frame**: Original render called it for every field on every scroll -- now pre-computed once.
|
||||
4. **No cover image**: Activity never loaded or displayed any cover.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### Committed first: PR #1342 port (commit 4cf395a)
|
||||
- Staged and committed all prior working state before making further changes
|
||||
|
||||
### BookInfoActivity refactor (2 files)
|
||||
|
||||
**`src/activities/home/BookInfoActivity.h`**:
|
||||
- Replaced individual metadata string members with `InfoField` struct + `std::vector<InfoField> fields`
|
||||
- Added `coverBmpPath`, `coverDisplayHeight`, `coverDisplayWidth` members
|
||||
- Added `buildLayout()` method for pre-computation
|
||||
|
||||
**`src/activities/home/BookInfoActivity.cpp`**:
|
||||
- **Y-culling**: `render()` skips draw calls for items entirely above or below the visible screen area (`y + height > 0 && y < pageH`); breaks out of field loop when `y >= pageH`
|
||||
- **Newline normalization**: Added `normalizeWhitespace()` helper that collapses `\n`, `\r`, `\t` sequences into single spaces; applied to description text before word-wrapping
|
||||
- **Cover height clamping**: `drawBitmap1Bit` maxHeight capped to `std::min(coverDisplayHeight, pageH - y)` to prevent drawing beyond screen
|
||||
- **Pre-computed layout**: All `wrappedText()` calls moved to `onEnter()` via `buildLayout()`; `render()` only iterates pre-computed lines
|
||||
- Cover thumbnail generated via `epub.generateThumbBmp()` / `xtc.generateThumbBmp()`; fallback to `PlaceholderCoverGenerator`
|
||||
- Cover rendered centered at top using `renderer.drawBitmap1Bit()`
|
||||
|
||||
## Build Verification
|
||||
- `pio run` SUCCESS (19s incremental, RAM 30.3%, Flash 95.7%)
|
||||
|
||||
## Follow-up Items
|
||||
- Hardware test: verify render times dropped from 7-13s to <100ms with Y-culling
|
||||
- Hardware test: verify cover image renders correctly
|
||||
- Hardware test: verify scroll responsiveness on device
|
||||
@@ -1,15 +1,46 @@
|
||||
#include "BookInfoActivity.h"
|
||||
|
||||
#include <algorithm>
|
||||
|
||||
#include <Bitmap.h>
|
||||
#include <Epub.h>
|
||||
#include <FsHelpers.h>
|
||||
#include <GfxRenderer.h>
|
||||
#include <HalStorage.h>
|
||||
#include <I18n.h>
|
||||
#include <Logging.h>
|
||||
#include <PlaceholderCoverGenerator.h>
|
||||
#include <Xtc.h>
|
||||
|
||||
#include "components/UITheme.h"
|
||||
#include "fontIds.h"
|
||||
|
||||
namespace {
|
||||
constexpr int MARGIN = 20;
|
||||
constexpr int LABEL_VALUE_GAP = 4;
|
||||
constexpr int SECTION_GAP = 14;
|
||||
constexpr int MAX_WRAPPED_LINES = 60;
|
||||
constexpr int COVER_GAP = 16;
|
||||
|
||||
std::string normalizeWhitespace(const std::string& s) {
|
||||
std::string out;
|
||||
out.reserve(s.size());
|
||||
bool prevSpace = false;
|
||||
for (const char c : s) {
|
||||
if (c == '\n' || c == '\r' || c == '\t') {
|
||||
if (!prevSpace) {
|
||||
out += ' ';
|
||||
prevSpace = true;
|
||||
}
|
||||
} else {
|
||||
out += c;
|
||||
prevSpace = (c == ' ');
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
void BookInfoActivity::onEnter() {
|
||||
Activity::onEnter();
|
||||
|
||||
@@ -19,12 +50,17 @@ void BookInfoActivity::onEnter() {
|
||||
fileName = filePath.substr(lastSlash + 1);
|
||||
}
|
||||
|
||||
FsFile file;
|
||||
if (Storage.openFileForRead("BIF", filePath, file)) {
|
||||
fileSize = file.fileSize();
|
||||
file.close();
|
||||
size_t fileSize = 0;
|
||||
{
|
||||
FsFile file;
|
||||
if (Storage.openFileForRead("BIF", filePath, file)) {
|
||||
fileSize = file.fileSize();
|
||||
file.close();
|
||||
}
|
||||
}
|
||||
|
||||
std::string title, author, series, seriesIndex, description, language;
|
||||
|
||||
if (FsHelpers::hasEpubExtension(fileName)) {
|
||||
Epub epub(filePath, "/.crosspoint");
|
||||
if (epub.load(false, true)) {
|
||||
@@ -32,8 +68,38 @@ void BookInfoActivity::onEnter() {
|
||||
author = epub.getAuthor();
|
||||
series = epub.getSeries();
|
||||
seriesIndex = epub.getSeriesIndex();
|
||||
description = epub.getDescription();
|
||||
description = normalizeWhitespace(epub.getDescription());
|
||||
language = epub.getLanguage();
|
||||
|
||||
const int coverH = renderer.getScreenHeight() * 2 / 5;
|
||||
if (epub.generateThumbBmp(coverH)) {
|
||||
coverBmpPath = epub.getThumbBmpPath(coverH);
|
||||
} else {
|
||||
const int thumbW = static_cast<int>(coverH * 0.6);
|
||||
const std::string placeholderPath = epub.getCachePath() + "/placeholder_" + std::to_string(coverH) + ".bmp";
|
||||
if (PlaceholderCoverGenerator::generate(placeholderPath, title.empty() ? fileName : title, author, thumbW,
|
||||
coverH)) {
|
||||
coverBmpPath = placeholderPath;
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if (FsHelpers::hasXtcExtension(fileName)) {
|
||||
Xtc xtc(filePath, "/.crosspoint");
|
||||
if (xtc.load()) {
|
||||
title = xtc.getTitle();
|
||||
author = xtc.getAuthor();
|
||||
|
||||
const int coverH = renderer.getScreenHeight() * 2 / 5;
|
||||
if (xtc.generateThumbBmp(coverH)) {
|
||||
coverBmpPath = xtc.getThumbBmpPath(coverH);
|
||||
} else {
|
||||
const int thumbW = static_cast<int>(coverH * 0.6);
|
||||
const std::string placeholderPath = xtc.getCachePath() + "/placeholder_" + std::to_string(coverH) + ".bmp";
|
||||
if (PlaceholderCoverGenerator::generate(placeholderPath, title.empty() ? fileName : title, author, thumbW,
|
||||
coverH)) {
|
||||
coverBmpPath = placeholderPath;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,11 +107,75 @@ void BookInfoActivity::onEnter() {
|
||||
title = fileName;
|
||||
}
|
||||
|
||||
buildLayout(title, author, series, seriesIndex, description, language, fileSize);
|
||||
requestUpdate();
|
||||
}
|
||||
|
||||
void BookInfoActivity::onExit() { Activity::onExit(); }
|
||||
|
||||
void BookInfoActivity::buildLayout(const std::string& title, const std::string& author, const std::string& series,
|
||||
const std::string& seriesIndex, const std::string& description,
|
||||
const std::string& language, size_t fileSize) {
|
||||
const int contentW = renderer.getScreenWidth() - MARGIN * 2;
|
||||
fields.reserve(6);
|
||||
|
||||
auto addField = [&](const char* label, const std::string& text, bool bold, EpdFontFamily::Style style) {
|
||||
if (text.empty()) return;
|
||||
InfoField field;
|
||||
field.label = label;
|
||||
field.bold = bold;
|
||||
field.lines = renderer.wrappedText(UI_12_FONT_ID, text.c_str(), contentW, MAX_WRAPPED_LINES, style);
|
||||
fields.push_back(std::move(field));
|
||||
};
|
||||
|
||||
addField(nullptr, title, true, EpdFontFamily::BOLD);
|
||||
addField(tr(STR_AUTHOR), author, false, EpdFontFamily::REGULAR);
|
||||
|
||||
if (!series.empty()) {
|
||||
std::string seriesStr = series;
|
||||
if (!seriesIndex.empty()) {
|
||||
seriesStr += " #" + seriesIndex;
|
||||
}
|
||||
addField(tr(STR_SERIES), seriesStr, false, EpdFontFamily::REGULAR);
|
||||
}
|
||||
|
||||
addField(tr(STR_LANGUAGE), language, false, EpdFontFamily::REGULAR);
|
||||
|
||||
if (fileSize > 0) {
|
||||
addField(tr(STR_FILE_SIZE), formatFileSize(fileSize), false, EpdFontFamily::REGULAR);
|
||||
}
|
||||
|
||||
addField(tr(STR_DESCRIPTION), description, false, EpdFontFamily::REGULAR);
|
||||
|
||||
const int lineH10 = renderer.getLineHeight(UI_10_FONT_ID);
|
||||
const int lineH12 = renderer.getLineHeight(UI_12_FONT_ID);
|
||||
int h = MARGIN;
|
||||
|
||||
if (!coverBmpPath.empty()) {
|
||||
FsFile file;
|
||||
if (Storage.openFileForRead("BIF", coverBmpPath, file)) {
|
||||
Bitmap bitmap(file);
|
||||
if (bitmap.parseHeaders() == BmpReaderError::Ok) {
|
||||
coverDisplayHeight = bitmap.getHeight();
|
||||
coverDisplayWidth = bitmap.getWidth();
|
||||
}
|
||||
file.close();
|
||||
}
|
||||
if (coverDisplayHeight > 0) {
|
||||
h += coverDisplayHeight + COVER_GAP;
|
||||
}
|
||||
}
|
||||
|
||||
for (const auto& field : fields) {
|
||||
if (field.label) {
|
||||
h += lineH10 + LABEL_VALUE_GAP;
|
||||
}
|
||||
h += static_cast<int>(field.lines.size()) * lineH12;
|
||||
h += SECTION_GAP;
|
||||
}
|
||||
contentHeight = h;
|
||||
}
|
||||
|
||||
void BookInfoActivity::loop() {
|
||||
if (mappedInput.wasReleased(MappedInputManager::Button::Back) ||
|
||||
mappedInput.wasReleased(MappedInputManager::Button::Confirm)) {
|
||||
@@ -56,16 +186,19 @@ void BookInfoActivity::loop() {
|
||||
return;
|
||||
}
|
||||
|
||||
const int pageH = renderer.getScreenHeight();
|
||||
const int scrollStep = pageH / 3;
|
||||
|
||||
if (mappedInput.wasReleased(MappedInputManager::Button::Down)) {
|
||||
if (scrollOffset + renderer.getScreenHeight() < contentHeight) {
|
||||
scrollOffset += renderer.getScreenHeight() / 3;
|
||||
if (scrollOffset + pageH < contentHeight) {
|
||||
scrollOffset += scrollStep;
|
||||
requestUpdate();
|
||||
}
|
||||
}
|
||||
|
||||
if (mappedInput.wasReleased(MappedInputManager::Button::Up)) {
|
||||
if (scrollOffset > 0) {
|
||||
scrollOffset -= renderer.getScreenHeight() / 3;
|
||||
scrollOffset -= scrollStep;
|
||||
if (scrollOffset < 0) scrollOffset = 0;
|
||||
requestUpdate();
|
||||
}
|
||||
@@ -75,72 +208,51 @@ void BookInfoActivity::loop() {
|
||||
void BookInfoActivity::render(RenderLock&&) {
|
||||
renderer.clearScreen();
|
||||
|
||||
const int pageW = renderer.getScreenWidth();
|
||||
const int pageH = renderer.getScreenHeight();
|
||||
constexpr int margin = 20;
|
||||
constexpr int labelValueGap = 4;
|
||||
constexpr int sectionGap = 14;
|
||||
constexpr int maxWrappedLines = 30;
|
||||
const int contentW = pageW - margin * 2;
|
||||
const int lineH10 = renderer.getLineHeight(UI_10_FONT_ID);
|
||||
const int lineH12 = renderer.getLineHeight(UI_12_FONT_ID);
|
||||
int y = margin - scrollOffset;
|
||||
int y = MARGIN - scrollOffset;
|
||||
|
||||
auto drawLabel = [&](const char* label) {
|
||||
renderer.drawText(UI_10_FONT_ID, margin, y, label, true, EpdFontFamily::BOLD);
|
||||
y += lineH10 + labelValueGap;
|
||||
};
|
||||
|
||||
auto drawWrapped = [&](int fontId, const std::string& text, int lineH, EpdFontFamily::Style style) {
|
||||
auto lines = renderer.wrappedText(fontId, text.c_str(), contentW, maxWrappedLines, style);
|
||||
for (const auto& line : lines) {
|
||||
renderer.drawText(fontId, margin, y, line.c_str(), true, style);
|
||||
y += lineH;
|
||||
// Cover image — only draw if at least partially visible
|
||||
if (!coverBmpPath.empty() && coverDisplayHeight > 0) {
|
||||
if (y + coverDisplayHeight > 0 && y < pageH) {
|
||||
FsFile file;
|
||||
if (Storage.openFileForRead("BIF", coverBmpPath, file)) {
|
||||
Bitmap bitmap(file);
|
||||
if (bitmap.parseHeaders() == BmpReaderError::Ok) {
|
||||
const int coverX = (renderer.getScreenWidth() - coverDisplayWidth) / 2;
|
||||
renderer.drawBitmap1Bit(bitmap, coverX, y, coverDisplayWidth, std::min(coverDisplayHeight, pageH - y));
|
||||
}
|
||||
file.close();
|
||||
}
|
||||
}
|
||||
y += sectionGap;
|
||||
};
|
||||
|
||||
// Title
|
||||
drawWrapped(UI_12_FONT_ID, title, lineH12, EpdFontFamily::BOLD);
|
||||
|
||||
// Author
|
||||
if (!author.empty()) {
|
||||
drawLabel(tr(STR_AUTHOR));
|
||||
drawWrapped(UI_12_FONT_ID, author, lineH12, EpdFontFamily::REGULAR);
|
||||
y += coverDisplayHeight + COVER_GAP;
|
||||
}
|
||||
|
||||
// Series
|
||||
if (!series.empty()) {
|
||||
drawLabel(tr(STR_SERIES));
|
||||
std::string seriesStr = series;
|
||||
if (!seriesIndex.empty()) {
|
||||
seriesStr += " #" + seriesIndex;
|
||||
for (const auto& field : fields) {
|
||||
if (y >= pageH) break;
|
||||
|
||||
if (field.label) {
|
||||
if (y + lineH10 > 0 && y < pageH) {
|
||||
renderer.drawText(UI_10_FONT_ID, MARGIN, y, field.label, true, EpdFontFamily::BOLD);
|
||||
}
|
||||
y += lineH10 + LABEL_VALUE_GAP;
|
||||
}
|
||||
drawWrapped(UI_12_FONT_ID, seriesStr, lineH12, EpdFontFamily::REGULAR);
|
||||
|
||||
const auto style = field.bold ? EpdFontFamily::BOLD : EpdFontFamily::REGULAR;
|
||||
for (const auto& line : field.lines) {
|
||||
if (y >= pageH) break;
|
||||
if (y + lineH12 > 0) {
|
||||
renderer.drawText(UI_12_FONT_ID, MARGIN, y, line.c_str(), true, style);
|
||||
}
|
||||
y += lineH12;
|
||||
}
|
||||
y += SECTION_GAP;
|
||||
}
|
||||
|
||||
// Language
|
||||
if (!language.empty()) {
|
||||
drawLabel(tr(STR_LANGUAGE));
|
||||
drawWrapped(UI_12_FONT_ID, language, lineH12, EpdFontFamily::REGULAR);
|
||||
}
|
||||
|
||||
// File size
|
||||
if (fileSize > 0) {
|
||||
drawLabel(tr(STR_FILE_SIZE));
|
||||
drawWrapped(UI_12_FONT_ID, formatFileSize(fileSize), lineH12, EpdFontFamily::REGULAR);
|
||||
}
|
||||
|
||||
// Description
|
||||
if (!description.empty()) {
|
||||
drawLabel(tr(STR_DESCRIPTION));
|
||||
drawWrapped(UI_12_FONT_ID, description, lineH12, EpdFontFamily::REGULAR);
|
||||
}
|
||||
|
||||
contentHeight = y + scrollOffset;
|
||||
|
||||
// Button hints
|
||||
const char* scrollHint = contentHeight > pageH ? tr(STR_DIR_DOWN) : "";
|
||||
const bool canScrollDown = scrollOffset + pageH < contentHeight;
|
||||
const bool canScrollUp = scrollOffset > 0;
|
||||
const char* scrollHint = canScrollDown ? tr(STR_DIR_DOWN) : (canScrollUp ? tr(STR_DIR_UP) : "");
|
||||
const auto labels = mappedInput.mapLabels(tr(STR_BACK), "", scrollHint, "");
|
||||
GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4);
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
#pragma once
|
||||
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
#include "../Activity.h"
|
||||
|
||||
@@ -15,18 +16,23 @@ class BookInfoActivity final : public Activity {
|
||||
void render(RenderLock&&) override;
|
||||
|
||||
private:
|
||||
std::string filePath;
|
||||
struct InfoField {
|
||||
const char* label;
|
||||
std::vector<std::string> lines;
|
||||
bool bold;
|
||||
};
|
||||
|
||||
std::string title;
|
||||
std::string author;
|
||||
std::string series;
|
||||
std::string seriesIndex;
|
||||
std::string description;
|
||||
std::string language;
|
||||
size_t fileSize = 0;
|
||||
std::string filePath;
|
||||
std::string coverBmpPath;
|
||||
std::vector<InfoField> fields;
|
||||
|
||||
int scrollOffset = 0;
|
||||
int contentHeight = 0;
|
||||
int coverDisplayHeight = 0;
|
||||
int coverDisplayWidth = 0;
|
||||
|
||||
void buildLayout(const std::string& title, const std::string& author, const std::string& series,
|
||||
const std::string& seriesIndex, const std::string& description, const std::string& language,
|
||||
size_t fileSize);
|
||||
static std::string formatFileSize(size_t bytes);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user