Merge pull request #3855 from brdvd/feat/resources

Improve performance by optimizing glyph lookup
rism-digital · Nov 11, 2024 · c87b3d6 · c87b3d6
2 parents d985867 + ffc5e13
commit c87b3d6
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 36 deletions.
diff --git a/include/vrv/resources.h b/include/vrv/resources.h
@@ -8,6 +8,7 @@
 #ifndef __VRV_RESOURCES_H__
 #define __VRV_RESOURCES_H__
 
+#include <optional>
 #include <unordered_map>
 
 //----------------------------------------------------------------------------
@@ -65,11 +66,9 @@ class Resources {
     /** Load all music fonts available in the resource directory */
     bool LoadAll();
     /** Set the fallback font (Leipzig or Bravura) when some glyphs are missing in the current font */
-    bool SetFallback(const std::string &fontName);
+    void SetFallbackFont(const std::string &fontName);
     /** Get the fallback font name */
-    std::string GetFallbackFont() const { return m_defaultFontName; }
-    /** Init the text font (bounding boxes and ASCII only) */
-    bool InitTextFont(const std::string &fontName, const StyleAttributes &style);
+    std::string GetFallbackFont() const { return m_fallbackFontName; }
 
     /** Select a particular font */
     bool SetCurrentFont(const std::string &fontName, bool allowLoading = false);
@@ -160,6 +159,9 @@ class Resources {
 
     bool LoadFont(const std::string &fontName, ZipFileReader *zipFile = NULL);
 
+    /** Init the text font (bounding boxes and ASCII only) */
+    bool InitTextFont(const std::string &fontName, const StyleAttributes &style);
+
     const GlyphTable &GetCurrentGlyphTable() const { return m_loadedFonts.at(m_currentFontName).GetGlyphTable(); };
     const GlyphTable &GetFallbackGlyphTable() const { return m_loadedFonts.at(m_fallbackFontName).GetGlyphTable(); };
 
@@ -177,6 +179,9 @@ class Resources {
      */
     GlyphNameTable m_glyphNameTable;
 
+    /** Cache of the last glyph that was looked up in loaded fonts */
+    mutable std::optional<std::pair<char32_t, const Glyph *>> m_cachedGlyph;
+
     //----------------//
     // Static members //
     //----------------//

diff --git a/src/resources.cpp b/src/resources.cpp
@@ -59,6 +59,7 @@ Resources::Resources()
 
 bool Resources::InitFonts()
 {
+    m_cachedGlyph.reset();
     m_loadedFonts.clear();
 
     // Font Bravura first. As it is expected to have always all symbols we build the code -> name table from it
@@ -95,7 +96,9 @@ bool Resources::InitFonts()
 
 bool Resources::SetFont(const std::string &fontName)
 {
-    // and the default font provided in options, if it is not one of the previous
+    m_cachedGlyph.reset();
+
+    // add the default font provided in options, if it is not one of the previous
     if (!fontName.empty() && !IsFontLoaded(fontName)) {
         if (!LoadFont(fontName)) {
             LogError("%s font could not be loaded.", fontName.c_str());
@@ -132,28 +135,30 @@ bool Resources::AddCustom(const std::vector<std::string> &extraFonts)
 
 bool Resources::LoadAll()
 {
-    bool success = true;
     std::string path = Resources::GetPath() + "/";
-    for (const std::filesystem::directory_entry &entry : std::filesystem::directory_iterator(path)) {
-        const std::filesystem::path path = entry.path();
-        if (path.has_extension() && path.has_stem() && path.extension() == ".xml") {
-            const std::string fontName = path.stem().string();
-            if (!IsFontLoaded(fontName)) {
-                success = success && LoadFont(fontName);
+    return std::ranges::all_of(
+        std::filesystem::directory_iterator(path), [this](const std::filesystem::directory_entry &entry) {
+            const std::filesystem::path &path = entry.path();
+            if (path.has_extension() && path.has_stem() && path.extension() == ".xml") {
+                const std::string fontName = path.stem().string();
+                if (!this->IsFontLoaded(fontName) && !this->LoadFont(fontName)) {
+                    return false;
+                }
             }
-        }
-    }
-    return success;
+            return true;
+        });
 }
 
-bool Resources::SetFallback(const std::string &fontName)
+void Resources::SetFallbackFont(const std::string &fontName)
 {
+    m_cachedGlyph.reset();
     m_fallbackFontName = fontName;
-    return true;
 }
 
 bool Resources::SetCurrentFont(const std::string &fontName, bool allowLoading)
 {
+    m_cachedGlyph.reset();
+
     if (IsFontLoaded(fontName)) {
         m_currentFontName = fontName;
         return true;
@@ -162,33 +167,47 @@ bool Resources::SetCurrentFont(const std::string &fontName, bool allowLoading)
         m_currentFontName = fontName;
         return true;
     }
-    else {
-        return false;
-    }
+
+    return false;
 }
 
 const Glyph *Resources::GetGlyph(char32_t smuflCode) const
 {
-    if (GetCurrentGlyphTable().contains(smuflCode)) {
-        return &GetCurrentGlyphTable().at(smuflCode);
+    if (m_cachedGlyph && m_cachedGlyph->first == smuflCode) {
+        return m_cachedGlyph->second;
+    }
+
+    const GlyphTable &currentTable = this->GetCurrentGlyphTable();
+    if (auto glyphIter = currentTable.find(smuflCode); glyphIter != currentTable.end()) {
+        const Glyph *glyph = &glyphIter->second;
+        m_cachedGlyph = std::make_pair(glyphIter->first, glyph);
+        return glyph;
     }
     else if (!this->IsCurrentFontFallback()) {
         const GlyphTable &fallbackTable = this->GetFallbackGlyphTable();
-        return (fallbackTable.contains(smuflCode)) ? &fallbackTable.at(smuflCode) : NULL;
-    }
-    else {
-        return NULL;
+        if (auto glyphIter = fallbackTable.find(smuflCode); glyphIter != fallbackTable.end()) {
+            const Glyph *glyph = &glyphIter->second;
+            m_cachedGlyph = std::make_pair(glyphIter->first, glyph);
+            return glyph;
+        }
     }
+    return NULL;
 }
 
 const Glyph *Resources::GetGlyph(const std::string &smuflName) const
 {
-    return (this->GetGlyphCode(smuflName)) ? &GetCurrentGlyphTable().at(this->GetGlyphCode(smuflName)) : NULL;
+    if (const char32_t code = this->GetGlyphCode(smuflName); code) {
+        return this->GetGlyph(code);
+    }
+    return NULL;
 }
 
 char32_t Resources::GetGlyphCode(const std::string &smuflName) const
 {
-    return m_glyphNameTable.contains(smuflName) ? m_glyphNameTable.at(smuflName) : 0;
+    if (auto glyphNameIter = m_glyphNameTable.find(smuflName); glyphNameIter != m_glyphNameTable.end()) {
+        return glyphNameIter->second;
+    }
+    return 0;
 }
 
 bool Resources::IsSmuflFallbackNeeded(const std::u32string &text) const
@@ -214,12 +233,7 @@ bool Resources::FontHasGlyphAvailable(const std::string &fontName, char32_t smuf
     }
 
     const GlyphTable &table = m_loadedFonts.at(fontName).GetGlyphTable();
-    if (table.find(smuflCode) != table.end()) {
-        return true;
-    }
-    else {
-        return false;
-    }
+    return (table.find(smuflCode) != table.end());
 }
 
 std::string Resources::GetCSSFontFor(const std::string &fontName) const

diff --git a/src/toolkit.cpp b/src/toolkit.cpp
@@ -127,7 +127,7 @@ bool Toolkit::SetResourcePath(const std::string &path)
         success = success && this->SetFont(m_options->m_font.GetValue());
     }
     if (m_options->m_fontFallback.IsSet()) {
-        success = success && resources.SetFallback(m_options->m_fontFallback.GetStrValue());
+        resources.SetFallbackFont(m_options->m_fontFallback.GetStrValue());
     }
     if (m_options->m_fontLoadAll.IsSet()) {
         success = success && resources.LoadAll();
@@ -1224,7 +1224,7 @@ bool Toolkit::SetOptions(const std::string &jsonOptions)
     }
     if (json.has<jsonxx::String>("fontFallback")) {
         Resources &resources = m_doc.GetResourcesForModification();
-        resources.SetFallback(m_options->m_fontFallback.GetStrValue());
+        resources.SetFallbackFont(m_options->m_fontFallback.GetStrValue());
     }
     if (json.has<jsonxx::Boolean>("fontLoadAll")) {
         Resources &resources = m_doc.GetResourcesForModification();