From 45441e078981938696cce3c94e97987059250fb8 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sun, 10 May 2026 18:01:24 +0200 Subject: [PATCH] feat: closest-pt size selection instead of ordinal slot (#1912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add SdCardFontFamilyInfo::pickClosestSize(targetPtSize) and have the manager and SdCardFontSystem drive size selection from a target point size derived from the user's font-size enum (SMALL=12, MEDIUM=14, LARGE=16, EXTRA_LARGE=18) rather than indexing the family's sorted size list by enum ordinal. The ordinal-slot approach mis-selects whenever a family doesn't ship the canonical {12,14,16,18} set: a family with only [10,14,18] would map SMALL/MEDIUM/LARGE/EXTRA_LARGE to 10/14/18/18 — fine for SMALL but arbitrary for the rest. Closest-pt always picks the on-disk file nearest to the user-intended point size, with a deterministic smaller-pt tie-break. No change for canonical-sized families. ## Summary * **What is the goal of this PR?** (e.g., Implements the new feature for file uploading.) * **What changes are included?** ## Additional Context * Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks, specific areas to focus on). --- ### AI Usage 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? _**< YES | PARTIALLY | NO >**_ --- lib/EpdFont/SdCardFontManager.cpp | 20 ++++++++----------- lib/EpdFont/SdCardFontManager.h | 14 +++++++++----- lib/EpdFont/SdCardFontRegistry.cpp | 17 ++++++++++++++++ lib/EpdFont/SdCardFontRegistry.h | 9 +++++++++ src/SdCardFontSystem.cpp | 31 ++++++++++++++++-------------- 5 files changed, 60 insertions(+), 31 deletions(-) diff --git a/lib/EpdFont/SdCardFontManager.cpp b/lib/EpdFont/SdCardFontManager.cpp index a6032336..804df07d 100644 --- a/lib/EpdFont/SdCardFontManager.cpp +++ b/lib/EpdFont/SdCardFontManager.cpp @@ -28,25 +28,21 @@ int SdCardFontManager::computeFontId(uint32_t contentHash, const char* familyNam return id != 0 ? id : 1; // 0 is reserved as "not found" sentinel } -bool SdCardFontManager::loadFamily(const SdCardFontFamilyInfo& family, GfxRenderer& renderer, uint8_t fontSizeEnum) { +bool SdCardFontManager::loadFamily(const SdCardFontFamilyInfo& family, GfxRenderer& renderer, uint8_t targetPtSize) { // Unload any previously loaded family first if (!loadedFamilyName_.empty()) { unloadAll(renderer); } - // Select by ordinal position: sort available sizes, then map the font size - // enum (SMALL=0 .. EXTRA_LARGE=3) to the corresponding slot. When the - // family has fewer sizes than 4, clamp to the last available size. - auto sizes = family.availableSizes(); - if (sizes.empty()) { + // 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 = family.pickClosestSize(targetPtSize); + if (!selected) { LOG_ERR("SDMGR", "Family %s has no files to load", family.name.c_str()); return false; } - uint8_t idx = fontSizeEnum; - if (idx >= sizes.size()) idx = sizes.size() - 1; - const SdCardFontFileInfo* selected = family.findFile(sizes[idx]); - auto* font = new (std::nothrow) SdCardFont(); if (!font) { LOG_ERR("SDMGR", "Failed to allocate SdCardFont for %s", selected->path.c_str()); @@ -70,8 +66,8 @@ bool SdCardFontManager::loadFamily(const SdCardFontFamilyInfo& family, GfxRender renderer.registerSdCardFont(fontId, font); loaded_.push_back({font, fontId, selected->pointSize}); - LOG_DBG("SDMGR", "Loaded %s size=%u id=%d styles=%u (sizeEnum=%u)", selected->path.c_str(), selected->pointSize, - fontId, font->styleCount(), fontSizeEnum); + LOG_DBG("SDMGR", "Loaded %s size=%u id=%d styles=%u (target=%u)", selected->path.c_str(), selected->pointSize, fontId, + font->styleCount(), targetPtSize); EpdFontFamily fontFamily(font->getEpdFont(0), font->getEpdFont(1), font->getEpdFont(2), font->getEpdFont(3)); renderer.insertFont(fontId, fontFamily); diff --git a/lib/EpdFont/SdCardFontManager.h b/lib/EpdFont/SdCardFontManager.h index aec07472..def66e8d 100644 --- a/lib/EpdFont/SdCardFontManager.h +++ b/lib/EpdFont/SdCardFontManager.h @@ -15,12 +15,16 @@ class SdCardFontManager { SdCardFontManager(const SdCardFontManager&) = delete; SdCardFontManager& operator=(const SdCardFontManager&) = delete; - // Load the font file matching fontSizeEnum (SMALL=0 .. EXTRA_LARGE=3) by - // ordinal position in the family's sorted size list. Only one .cpfont file - // is loaded; other sizes remain on disk. This keeps resident interval + - // kern/ligature tables to one size's worth of memory. + // Load the single size whose pointSize is closest to targetPtSize. Only one + // .cpfont file is loaded; other sizes remain on disk. This keeps resident + // interval + kern/ligature tables to one size's worth of memory. + // + // Closest-pt selection is robust against families that don't ship the + // canonical {12,14,16,18}: a family with only [10,14,18] still resolves + // any reasonable target, where ordinal slot-mapping by SMALL..EXTRA_LARGE + // would mis-select. // Returns true on success. - bool loadFamily(const SdCardFontFamilyInfo& family, GfxRenderer& renderer, uint8_t fontSizeEnum); + bool loadFamily(const SdCardFontFamilyInfo& family, GfxRenderer& renderer, uint8_t targetPtSize); // Unload everything, unregister from renderer. void unloadAll(GfxRenderer& renderer); diff --git a/lib/EpdFont/SdCardFontRegistry.cpp b/lib/EpdFont/SdCardFontRegistry.cpp index 2e0f145c..af1009b0 100644 --- a/lib/EpdFont/SdCardFontRegistry.cpp +++ b/lib/EpdFont/SdCardFontRegistry.cpp @@ -4,6 +4,8 @@ #include #include +#include +#include #include // --- SdCardFontFamilyInfo helpers --- @@ -38,6 +40,21 @@ std::vector SdCardFontFamilyInfo::availableSizes() const { return sizes; } +const SdCardFontFileInfo* SdCardFontFamilyInfo::pickClosestSize(uint8_t targetPtSize) const { + const SdCardFontFileInfo* selected = nullptr; + int bestDiff = INT_MAX; + for (const auto& f : files) { + int diff = std::abs(static_cast(f.pointSize) - static_cast(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) { diff --git a/lib/EpdFont/SdCardFontRegistry.h b/lib/EpdFont/SdCardFontRegistry.h index f96035ed..aae932d9 100644 --- a/lib/EpdFont/SdCardFontRegistry.h +++ b/lib/EpdFont/SdCardFontRegistry.h @@ -20,6 +20,15 @@ struct SdCardFontFamilyInfo { const SdCardFontFileInfo* findFile(uint8_t size, uint8_t style = 0) const; bool hasSize(uint8_t size) const; std::vector 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. + // + // Robust against families that don't ship the canonical {12,14,16,18} set: + // a family with only [10,14,18] resolves target 12 → 10 (or 14 on tie), + // target 16 → 14 or 18, etc., instead of mis-indexing by ordinal slot. + const SdCardFontFileInfo* pickClosestSize(uint8_t targetPtSize) const; }; class SdCardFontRegistry { diff --git a/src/SdCardFontSystem.cpp b/src/SdCardFontSystem.cpp index b771bede..8e59d6ed 100644 --- a/src/SdCardFontSystem.cpp +++ b/src/SdCardFontSystem.cpp @@ -5,10 +5,15 @@ #include "CrossPointSettings.h" -static uint8_t fontSizeEnumFromSettings() { +// Map fontSize enum (SMALL=0, MEDIUM=1, LARGE=2, EXTRA_LARGE=3) to the point +// sizes shipped with the built-in fonts. Used to drive closest-pt selection +// in the SD card font registry (see SdCardFontFamilyInfo::pickClosestSize). +static constexpr uint8_t FONT_SIZE_TO_PT[CrossPointSettings::FONT_SIZE_COUNT] = {12, 14, 16, 18}; + +static uint8_t targetPtSizeFromSettings() { uint8_t e = SETTINGS.fontSize; if (e >= CrossPointSettings::FONT_SIZE_COUNT) e = 1; // default to MEDIUM - return e; + return FONT_SIZE_TO_PT[e]; } void SdCardFontSystem::begin(GfxRenderer& renderer) { @@ -25,7 +30,7 @@ void SdCardFontSystem::begin(GfxRenderer& renderer) { if (SETTINGS.sdFontFamilyName[0] != '\0') { const auto* family = registry_.findFamily(SETTINGS.sdFontFamilyName); if (family) { - if (manager_.loadFamily(*family, renderer, fontSizeEnumFromSettings())) { + if (manager_.loadFamily(*family, renderer, targetPtSizeFromSettings())) { LOG_DBG("SDFS", "Loaded SD card font family: %s", SETTINGS.sdFontFamilyName); } else { LOG_ERR("SDFS", "Failed to load SD font family: %s (clearing)", SETTINGS.sdFontFamilyName); @@ -53,7 +58,7 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) { const char* wantedFamily = SETTINGS.sdFontFamilyName; const std::string& currentFamily = manager_.currentFamilyName(); - const uint8_t sizeEnum = fontSizeEnumFromSettings(); + const uint8_t targetPt = targetPtSizeFromSettings(); if (wantedFamily[0] == '\0') { if (!currentFamily.empty()) { @@ -62,8 +67,8 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) { return; } - // Reload if family changed OR if the user-selected size maps to a - // different file than what's currently loaded OR if the registry was + // Reload if family changed OR if the user-selected size now resolves to a + // different on-disk file than what's currently loaded OR if the registry was // just rediscovered (file may have been replaced on disk). bool familyMatches = (currentFamily == wantedFamily); if (familyMatches) { @@ -74,13 +79,11 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) { SETTINGS.sdFontFamilyName[0] = '\0'; return; } - auto sizes = family->availableSizes(); - uint8_t idx = sizeEnum; - if (idx >= sizes.size()) idx = sizes.size() - 1; - uint8_t wantedPt = sizes.empty() ? 0 : sizes[idx]; - if (!registryWasDirty && wantedPt == manager_.currentPointSize()) return; - LOG_DBG("SDFS", "Reloading %s: size %u -> %u (enum %u)%s", wantedFamily, manager_.currentPointSize(), wantedPt, - sizeEnum, registryWasDirty ? " [registry dirty]" : ""); + const auto* best = family->pickClosestSize(targetPt); + const uint8_t bestPt = best ? best->pointSize : 0; + if (!registryWasDirty && bestPt == manager_.currentPointSize()) return; + LOG_DBG("SDFS", "Reloading %s: size %u -> %u (target %u)%s", wantedFamily, manager_.currentPointSize(), bestPt, + targetPt, registryWasDirty ? " [registry dirty]" : ""); } if (!currentFamily.empty()) { @@ -89,7 +92,7 @@ void SdCardFontSystem::ensureLoaded(GfxRenderer& renderer) { const auto* family = registry_.findFamily(wantedFamily); if (family) { - if (manager_.loadFamily(*family, renderer, sizeEnum)) { + if (manager_.loadFamily(*family, renderer, targetPt)) { LOG_DBG("SDFS", "Loaded SD font family: %s", wantedFamily); } else { LOG_ERR("SDFS", "Failed to load SD font family: %s (clearing)", wantedFamily);