Some review comments

This commit is contained in:
jpirnay
2026-04-28 15:47:51 +02:00
parent 4c8517cf67
commit 8a2bc9f425
7 changed files with 77 additions and 30 deletions
+21 -3
View File
@@ -432,7 +432,14 @@ bool SdCardFont::load(const char* path) {
return false;
}
// Begin content hash: accumulate global header
// Begin content hash: accumulate global header.
// KNOWN LIMITATION: hash covers global header + per-style TOC only, not the
// payload sections (intervals / glyph metrics / kern / ligature / bitmap). A
// font edit that alters payload bytes without changing any TOC count would
// produce the same contentHash and could leave stale EPUB section caches.
// Acceptable in practice because generate-sd-fonts.sh regeneration almost
// always changes interval/glyph/kern counts; revisit if we see real-world
// mismatches.
uint32_t hash = fnv1a(headerBuf, HEADER_SIZE);
bool is2Bit = (readU16(headerBuf + 10) & 1) != 0;
@@ -1214,7 +1221,12 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) {
EpdGlyph tempGlyph;
uint32_t glyphFileOff = s.glyphsFileOffset + static_cast<uint32_t>(globalIdx) * sizeof(EpdGlyph);
file.seekSet(glyphFileOff);
if (!file.seekSet(glyphFileOff)) {
LOG_ERR("SDCF", "Overflow: seek failed for glyph metadata U+%04X style %u", codepoint, styleIdx);
file.close();
if (!wasAtCapacity) self->overflowCount_--;
return nullptr;
}
if (file.read(reinterpret_cast<uint8_t*>(&tempGlyph), sizeof(EpdGlyph)) != sizeof(EpdGlyph)) {
LOG_ERR("SDCF", "Overflow: failed to read glyph metadata for U+%04X style %u", codepoint, styleIdx);
file.close();
@@ -1232,7 +1244,13 @@ const EpdGlyph* SdCardFont::onGlyphMiss(void* ctx, uint32_t codepoint) {
if (!wasAtCapacity) self->overflowCount_--;
return nullptr;
}
file.seekSet(s.bitmapFileOffset + tempGlyph.dataOffset);
if (!file.seekSet(s.bitmapFileOffset + tempGlyph.dataOffset)) {
LOG_ERR("SDCF", "Overflow: seek failed for bitmap U+%04X style %u", codepoint, styleIdx);
delete[] tempBitmap;
file.close();
if (!wasAtCapacity) self->overflowCount_--;
return nullptr;
}
if (file.read(tempBitmap, tempGlyph.dataLength) != static_cast<int>(tempGlyph.dataLength)) {
LOG_ERR("SDCF", "Overflow: failed to read bitmap for U+%04X", codepoint);
delete[] tempBitmap;
+1 -9
View File
@@ -39,15 +39,7 @@ bool SdCardFontManager::loadFamily(const SdCardFontFamilyInfo& family, GfxRender
// Pick the single file whose size is closest to targetPtSize. Loading
// only one size bounds resident memory (intervals + kern/ligature tables
// per style) to one file's worth, vs. N_sizes × per-file overhead.
const SdCardFontFileInfo* selected = nullptr;
int bestDiff = INT32_MAX;
for (const auto& fileInfo : family.files) {
int diff = std::abs(static_cast<int>(fileInfo.pointSize) - static_cast<int>(targetPtSize));
if (diff < bestDiff) {
bestDiff = diff;
selected = &fileInfo;
}
}
const SdCardFontFileInfo* selected = family.pickClosestSize(targetPtSize);
if (!selected) {
LOG_ERR("SDMGR", "Family %s has no files to load", family.name.c_str());
return false;
+15
View File
@@ -38,6 +38,21 @@ std::vector<uint8_t> SdCardFontFamilyInfo::availableSizes() const {
return sizes;
}
const SdCardFontFileInfo* SdCardFontFamilyInfo::pickClosestSize(uint8_t targetPtSize) const {
const SdCardFontFileInfo* selected = nullptr;
int bestDiff = INT32_MAX;
for (const auto& f : files) {
int diff = std::abs(static_cast<int>(f.pointSize) - static_cast<int>(targetPtSize));
// Strict < ensures the first scan wins on ties; then tie-break by smaller
// pointSize to make the choice independent of filesystem enumeration order.
if (diff < bestDiff || (diff == bestDiff && selected && f.pointSize < selected->pointSize)) {
bestDiff = diff;
selected = &f;
}
}
return selected;
}
// --- SdCardFontRegistry ---
bool SdCardFontRegistry::parseFilename(const char* filename, uint8_t& size, uint8_t& style) {
+5
View File
@@ -19,6 +19,11 @@ struct SdCardFontFamilyInfo {
const SdCardFontFileInfo* findFile(uint8_t size, uint8_t style = 0) const;
bool hasSize(uint8_t size) const;
std::vector<uint8_t> availableSizes() const;
// Pick the file whose pointSize is closest to targetPtSize. On ties (equal
// distance) prefers the smaller pointSize so behaviour is deterministic
// across SD card layouts. Returns nullptr when files is empty.
const SdCardFontFileInfo* pickClosestSize(uint8_t targetPtSize) const;
};
class SdCardFontRegistry {
+6 -3
View File
@@ -451,6 +451,12 @@ def rasterize_font_style(fontfile, size, intervals, style_id=0, force_autohint=F
style_label = style_names.get(style_id, str(style_id))
face = freetype.Face(fontfile)
# Set font size at 150 DPI (matching fontconvert.py) BEFORE any glyph load
# — load_glyph() with FT_LOAD_RENDER renders at the active size, so calling
# it before set_char_size() would waste work at the default size and risk
# Invalid_Size_Handle on some fonts.
face.set_char_size(size << 6, size << 6, 150, 150)
load_flags = freetype.FT_LOAD_RENDER
if force_autohint:
load_flags |= freetype.FT_LOAD_FORCE_AUTOHINT
@@ -480,9 +486,6 @@ def rasterize_font_style(fontfile, size, intervals, style_id=0, force_autohint=F
total_glyphs = sum(end - start + 1 for start, end in intervals)
print(f" [{style_label}] Validated: {len(intervals)} intervals, {total_glyphs} glyphs", file=sys.stderr)
# Set font size at 150 DPI (matching fontconvert.py)
face.set_char_size(size << 6, size << 6, 150, 150)
# Rasterize all glyphs
total_bitmap_size = 0
all_glyphs = []
+2 -9
View File
@@ -120,15 +120,8 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) {
SETTINGS.sdFontFamilyName[0] = '\0';
return;
}
uint8_t bestPt = 0;
int bestDiff = INT32_MAX;
for (const auto& f : family->files) {
int diff = abs(static_cast<int>(f.pointSize) - static_cast<int>(targetPt));
if (diff < bestDiff) {
bestDiff = diff;
bestPt = f.pointSize;
}
}
const auto* best = family->pickClosestSize(targetPt);
const uint8_t bestPt = best ? best->pointSize : 0;
if (bestPt == manager_.currentPointSize()) return; // already loaded with the right size
LOG_DBG("SDFS", "Reloading %s: size %u -> %u (target %u)", wantedFamily, manager_.currentPointSize(), bestPt,
targetPt);
@@ -5,10 +5,31 @@
#include "KOReaderCredentialStore.h"
#include "MappedInputManager.h"
#include "SdCardFontGlobals.h"
#include "activities/settings/SettingsSubmenuActivity.h"
#include "components/UITheme.h"
#include "fontIds.h"
namespace {
// Returns the localized name of the family currently used as the global default
// for the reader. When the user has selected an SD card font globally, the
// override menu's "Default" label should reflect that family by name even
// though the per-book override list itself is built-in only.
std::string defaultFontFamilyLabel(const SettingInfo& item) {
if (SETTINGS.sdFontFamilyName[0] != '\0') {
return std::string(SETTINGS.sdFontFamilyName);
}
// Built-in: enumValues[0] is STR_DEFAULT_VALUE, [1..3] are the three families
// in the same order as CrossPointSettings::FONT_FAMILY (BOOKERLY, NOTOSANS,
// OPENDYSLEXIC).
const auto idx = static_cast<size_t>(SETTINGS.fontFamily + 1);
if (idx < item.enumValues.size()) {
return I18N.get(item.enumValues[idx]);
}
return {};
}
} // namespace
EpubReaderMenuActivity::EpubReaderMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput,
const std::string& title, const int currentPage, const int totalPages,
const int bookProgressPercent, const uint8_t currentOrientation,
@@ -290,9 +311,9 @@ std::string EpubReaderMenuActivity::getItemValueString(int index) const {
}
}
if (item.nameId == StrId::STR_FONT_FAMILY && pendingFontFamilyOverride < 0) {
const auto defaultIndex = static_cast<size_t>(SETTINGS.fontFamily + 1);
if (defaultIndex < item.enumValues.size()) {
return std::string(tr(STR_DEFAULT_VALUE)) + " (" + I18N.get(item.enumValues[defaultIndex]) + ")";
const auto label = defaultFontFamilyLabel(item);
if (!label.empty()) {
return std::string(tr(STR_DEFAULT_VALUE)) + " (" + label + ")";
}
}
if (item.nameId == StrId::STR_FONT_SIZE && pendingFontSizeOverride < 0) {
@@ -324,9 +345,9 @@ void EpubReaderMenuActivity::openSubmenu(const SettingInfo& submenuEntry) {
}
}
if (item.nameId == StrId::STR_FONT_FAMILY && pendingFontFamilyOverride < 0) {
const auto valueIndex = static_cast<size_t>(SETTINGS.fontFamily + 1);
if (valueIndex < item.enumValues.size()) {
return std::string(tr(STR_DEFAULT_VALUE)) + " (" + I18N.get(item.enumValues[valueIndex]) + ")";
const auto label = defaultFontFamilyLabel(item);
if (!label.empty()) {
return std::string(tr(STR_DEFAULT_VALUE)) + " (" + label + ")";
}
}
if (item.nameId == StrId::STR_FONT_SIZE && pendingFontSizeOverride < 0) {