From 5a5728af3e644c7e65631621c026b923922c05ca Mon Sep 17 00:00:00 2001 From: jpirnay Date: Tue, 28 Apr 2026 16:06:02 +0200 Subject: [PATCH] More review comments --- src/CrossPointSettings.cpp | 68 +++++++++----------- src/SdCardFontGlobals.h | 6 +- src/SdCardFontSystem.cpp | 20 ++++-- src/activities/ActivityManager.cpp | 2 + src/activities/reader/EpubReaderActivity.cpp | 59 ++++++++++++++--- src/activities/reader/EpubReaderActivity.h | 1 + 6 files changed, 102 insertions(+), 54 deletions(-) diff --git a/src/CrossPointSettings.cpp b/src/CrossPointSettings.cpp index 7d81e247..880b42fd 100644 --- a/src/CrossPointSettings.cpp +++ b/src/CrossPointSettings.cpp @@ -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); diff --git a/src/SdCardFontGlobals.h b/src/SdCardFontGlobals.h index 03d8517d..f10e2cd2 100644 --- a/src/SdCardFontGlobals.h +++ b/src/SdCardFontGlobals.h @@ -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 diff --git a/src/SdCardFontSystem.cpp b/src/SdCardFontSystem.cpp index 35265b09..da00eade 100644 --- a/src/SdCardFontSystem.cpp +++ b/src/SdCardFontSystem.cpp @@ -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); } diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 6d9ccbb9..2d04efa1 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -283,6 +283,7 @@ void ActivityManager::goToBrowser() { } void ActivityManager::goToReader(std::string path) { + RenderLock lock; ensureSdFontLoaded(); replaceActivity(std::make_unique(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(renderer, mappedInput, std::move(path))); } diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index b30e4a4e..dfab4fa7 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -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(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
(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
(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); diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index 2ff6d562..1fa1789b 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -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();