More review comments

This commit is contained in:
jpirnay
2026-04-28 16:06:02 +02:00
parent 8a2bc9f425
commit 5a5728af3e
6 changed files with 102 additions and 54 deletions
+30 -38
View File
@@ -264,52 +264,44 @@ bool CrossPointSettings::loadFromBinaryFile() {
}
float CrossPointSettings::getReaderLineCompression() const {
// SD card fonts inherit the Bookerly-style line compression (the most neutral
// values) since we have no per-family metadata for SD fonts.
if (sdFontFamilyName[0] != '\0') {
const int effectiveFontId = getReaderFontId();
const int bookerlyId = getBuiltinReaderFontId(BOOKERLY, fontSize);
const int notosansId = getBuiltinReaderFontId(NOTOSANS, fontSize);
const int opendyslexicId = getBuiltinReaderFontId(OPENDYSLEXIC, fontSize);
if (effectiveFontId == notosansId) {
switch (lineSpacing) {
case TIGHT:
return 0.95f;
return 0.90f;
case NORMAL:
default:
return 1.0f;
return 0.95f;
case WIDE:
return 1.1f;
return 1.0f;
}
}
switch (fontFamily) {
case BOOKERLY:
if (effectiveFontId == opendyslexicId) {
switch (lineSpacing) {
case TIGHT:
return 0.90f;
case NORMAL:
default:
return 0.95f;
case WIDE:
return 1.0f;
}
}
// Bookerly or any SD card font: use the Bookerly-style neutral values.
switch (lineSpacing) {
case TIGHT:
return 0.95f;
case NORMAL:
default:
switch (lineSpacing) {
case TIGHT:
return 0.95f;
case NORMAL:
default:
return 1.0f;
case WIDE:
return 1.1f;
}
case NOTOSANS:
switch (lineSpacing) {
case TIGHT:
return 0.90f;
case NORMAL:
default:
return 0.95f;
case WIDE:
return 1.0f;
}
case OPENDYSLEXIC:
switch (lineSpacing) {
case TIGHT:
return 0.90f;
case NORMAL:
default:
return 0.95f;
case WIDE:
return 1.0f;
}
return 1.0f;
case WIDE:
return 1.1f;
}
}
@@ -392,7 +384,7 @@ int CrossPointSettings::getReaderFontId() const {
// resolveSdCardFontId() returns 0 if the named family isn't loaded
// (e.g. SD card removed since selection) — fall through to built-in.
if (sdFontFamilyName[0] != '\0') {
int id = resolveSdCardFontId(sdFontFamilyName);
int id = resolveSdCardFontId(sdFontFamilyName, fontSize);
if (id != 0) return id;
}
return getBuiltinReaderFontId(fontFamily, fontSize);
+3 -3
View File
@@ -11,11 +11,11 @@ extern SdCardFontSystem sdFontSystem;
// Defined in main.cpp; call before entering the reader or after settings change.
extern void ensureSdFontLoaded();
// Resolve the SD card font ID for the given family name.
// Returns 0 if no SD font with that family name is currently loaded.
// Resolve the SD card font ID for the given family name and font size enum.
// Returns 0 if no SD font with that family name and size is currently loaded.
// Free function (not stored as a callback in CrossPointSettings) so the linker
// can resolve it directly without runtime indirection.
int resolveSdCardFontId(const char* familyName);
int resolveSdCardFontId(const char* familyName, uint8_t fontSizeEnum);
// Trampolines used by the dynamic font-family SettingInfo. They walk
// sdFontSystem's registry on each call to translate between
+15 -5
View File
@@ -13,7 +13,9 @@
// Free-function resolver used by CrossPointSettings::getReaderFontId().
// Resolved by the linker — no callback indirection stored in settings.
int resolveSdCardFontId(const char* familyName) { return sdFontSystem.resolveFontId(familyName, 0); }
int resolveSdCardFontId(const char* familyName, uint8_t fontSizeEnum) {
return sdFontSystem.resolveFontId(familyName, fontSizeEnum);
}
// --- Font-family dynamic SettingInfo trampolines ---
//
@@ -145,9 +147,17 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) {
}
}
int SdCardFontSystem::resolveFontId(const char* familyName, uint8_t /*fontSizeEnum*/) const {
// The manager loads exactly one size (closest to SETTINGS.fontSize), so the
// enum is implicit — always return the single loaded font ID for this family.
// ensureLoaded() must have been called with the current settings before this.
static uint8_t targetPtSizeFromEnum(uint8_t fontSizeEnum) {
if (fontSizeEnum >= sizeof(FONT_SIZE_TO_PT)) fontSizeEnum = 1; // default to MEDIUM
return FONT_SIZE_TO_PT[fontSizeEnum];
}
int SdCardFontSystem::resolveFontId(const char* familyName, uint8_t fontSizeEnum) const {
// The manager loads exactly one size for the active SD family. Resolve only
// if the requested family matches the loaded family and the requested size
// matches the loaded size. otherwise return 0 so callers can fall back.
if (!familyName || familyName[0] == '\0') return 0;
if (manager_.currentFamilyName() != familyName) return 0;
if (manager_.currentPointSize() != targetPtSizeFromEnum(fontSizeEnum)) return 0;
return manager_.getFontId(familyName);
}
+2
View File
@@ -283,6 +283,7 @@ void ActivityManager::goToBrowser() {
}
void ActivityManager::goToReader(std::string path) {
RenderLock lock;
ensureSdFontLoaded();
replaceActivity(std::make_unique<ReaderActivity>(renderer, mappedInput, std::move(path)));
}
@@ -303,6 +304,7 @@ void ActivityManager::goToKOReaderSync() {
void ActivityManager::replaceWithReader(std::string path, ReturnHint hint) {
returnHint = std::move(hint);
hasReturnHint = true;
RenderLock lock;
ensureSdFontLoaded();
replaceActivity(std::make_unique<ReaderActivity>(renderer, mappedInput, std::move(path)));
}
+51 -8
View File
@@ -30,6 +30,7 @@
#include "QrDisplayActivity.h"
#include "ReaderUtils.h"
#include "RecentBooksStore.h"
#include "SdCardFontGlobals.h"
#include "StarredPagesActivity.h"
#include "components/UITheme.h"
#include "fontIds.h"
@@ -1072,6 +1073,48 @@ uint8_t EpubReaderActivity::getEffectiveImageRendering() const {
return SETTINGS.imageRendering;
}
float EpubReaderActivity::getEffectiveReaderLineCompression() const {
const uint8_t fontSize = (bookFontSizeOverride >= 0) ? static_cast<uint8_t>(bookFontSizeOverride) : SETTINGS.fontSize;
const int effectiveFontId = getEffectiveReaderFontId();
const int bookerlyId = CrossPointSettings::getBuiltinReaderFontId(CrossPointSettings::BOOKERLY, fontSize);
const int notosansId = CrossPointSettings::getBuiltinReaderFontId(CrossPointSettings::NOTOSANS, fontSize);
const int opendyslexicId = CrossPointSettings::getBuiltinReaderFontId(CrossPointSettings::OPENDYSLEXIC, fontSize);
if (effectiveFontId == notosansId) {
switch (SETTINGS.lineSpacing) {
case CrossPointSettings::TIGHT:
return 0.90f;
case CrossPointSettings::NORMAL:
default:
return 0.95f;
case CrossPointSettings::WIDE:
return 1.0f;
}
}
if (effectiveFontId == opendyslexicId) {
switch (SETTINGS.lineSpacing) {
case CrossPointSettings::TIGHT:
return 0.90f;
case CrossPointSettings::NORMAL:
default:
return 0.95f;
case CrossPointSettings::WIDE:
return 1.0f;
}
}
switch (SETTINGS.lineSpacing) {
case CrossPointSettings::TIGHT:
return 0.95f;
case CrossPointSettings::NORMAL:
default:
return 1.0f;
case CrossPointSettings::WIDE:
return 1.1f;
}
}
int EpubReaderActivity::getEffectiveReaderFontId() const {
// Per-book font override: when set, force a specific BUILT-IN family even if
// an SD card font is the global default. This makes the override predictable
@@ -1086,8 +1129,8 @@ int EpubReaderActivity::getEffectiveReaderFontId() const {
// SETTINGS.getReaderFontId() is the canonical answer.
if (bookFontSizeOverride >= 0) {
if (SETTINGS.sdFontFamilyName[0] != '\0') {
// SD font selected globally — size override doesn't change which family resolves.
return SETTINGS.getReaderFontId();
const int id = resolveSdCardFontId(SETTINGS.sdFontFamilyName, fontSize);
if (id != 0) return id;
}
return CrossPointSettings::getBuiltinReaderFontId(SETTINGS.fontFamily, fontSize);
}
@@ -1206,7 +1249,7 @@ void EpubReaderActivity::render(RenderLock&& lock) {
section = std::make_unique<Section>(epub, currentSpineIndex, renderer);
const unsigned long sectionStart = millis();
if (!section->loadSectionFile(getEffectiveReaderFontId(), SETTINGS.getReaderLineCompression(),
if (!section->loadSectionFile(getEffectiveReaderFontId(), getEffectiveReaderLineCompression(),
SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
viewportHeight, SETTINGS.hyphenationEnabled, embeddedStyle, imageRendering)) {
LOG_DBG("ERS", "Cache not found, building...");
@@ -1227,7 +1270,7 @@ void EpubReaderActivity::render(RenderLock&& lock) {
// Pagination will rebuild only the cps it actually encounters, bounded
// by MAX_PAGE_GLYPHS per style.
renderer.clearSdCardFontAccumulation();
if (!section->createSectionFile(getEffectiveReaderFontId(), SETTINGS.getReaderLineCompression(),
if (!section->createSectionFile(getEffectiveReaderFontId(), getEffectiveReaderLineCompression(),
SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
viewportHeight, SETTINGS.hyphenationEnabled, embeddedStyle, imageRendering,
progressFn)) {
@@ -1382,7 +1425,7 @@ void EpubReaderActivity::silentIndexNextChapterIfNeeded(const uint16_t viewportW
const uint8_t imageRendering = getEffectiveImageRendering();
Section nextSection(epub, nextSpineIndex, renderer);
if (nextSection.loadSectionFile(getEffectiveReaderFontId(), SETTINGS.getReaderLineCompression(),
if (nextSection.loadSectionFile(getEffectiveReaderFontId(), getEffectiveReaderLineCompression(),
SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
viewportHeight, SETTINGS.hyphenationEnabled, embeddedStyle, imageRendering)) {
return;
@@ -1391,7 +1434,7 @@ void EpubReaderActivity::silentIndexNextChapterIfNeeded(const uint16_t viewportW
LOG_DBG("ERS", "Silently indexing next chapter: %d", nextSpineIndex);
// Reset cumulative SD font metadata cache for the new section.
renderer.clearSdCardFontAccumulation();
if (!nextSection.createSectionFile(getEffectiveReaderFontId(), SETTINGS.getReaderLineCompression(),
if (!nextSection.createSectionFile(getEffectiveReaderFontId(), getEffectiveReaderLineCompression(),
SETTINGS.extraParagraphSpacing, SETTINGS.paragraphAlignment, viewportWidth,
viewportHeight, SETTINGS.hyphenationEnabled, embeddedStyle, imageRendering)) {
LOG_ERR("ERS", "Failed silent indexing for chapter: %d", nextSpineIndex);
@@ -1744,12 +1787,12 @@ bool EpubReaderActivity::drawCurrentPageToBuffer(const std::string& filePath, Gf
auto section = std::make_unique<Section>(epub, spineIndex, renderer);
if (!section->loadSectionFile(getEffectiveFontId(effectiveFontFamily, effectiveFontSize),
SETTINGS.getReaderLineCompression(), SETTINGS.extraParagraphSpacing,
getEffectiveReaderLineCompression(), SETTINGS.extraParagraphSpacing,
SETTINGS.paragraphAlignment, viewportWidth, viewportHeight, SETTINGS.hyphenationEnabled,
SETTINGS.embeddedStyle, SETTINGS.imageRendering)) {
LOG_DBG("SLP", "EPUB: section cache not found for spine %d, rebuilding", spineIndex);
if (!section->createSectionFile(getEffectiveFontId(effectiveFontFamily, effectiveFontSize),
SETTINGS.getReaderLineCompression(), SETTINGS.extraParagraphSpacing,
getEffectiveReaderLineCompression(), SETTINGS.extraParagraphSpacing,
SETTINGS.paragraphAlignment, viewportWidth, viewportHeight,
SETTINGS.hyphenationEnabled, SETTINGS.embeddedStyle, SETTINGS.imageRendering)) {
LOG_ERR("SLP", "EPUB: failed to rebuild section cache for spine %d", spineIndex);
@@ -181,6 +181,7 @@ class EpubReaderActivity final : public Activity {
bool getEffectiveEmbeddedStyle() const;
uint8_t getEffectiveImageRendering() const;
int getEffectiveReaderFontId() const;
float getEffectiveReaderLineCompression() const;
bool stepPageState(bool isForwardTurn);
void pageTurn(bool isForwardTurn);
void runRenderBenchmark();