diff --git a/include/eepp/graphics/fontservice.hpp b/include/eepp/graphics/fontservice.hpp index d211aa614..3fa9c29e5 100644 --- a/include/eepp/graphics/fontservice.hpp +++ b/include/eepp/graphics/fontservice.hpp @@ -113,9 +113,10 @@ class EE_API FontService { /** * Finds or loads a system fallback font described by @p desc. * - * Successfully loaded fonts are published into the associated scope and strongly retained by - * the service for future glyph fallback requests. The returned pointer is borrowed and remains - * valid until the font is removed from the scope or the service is destroyed. + * Successfully loaded fonts are published under an internal, path-specific resource name in the + * associated scope and strongly retained by the service for future glyph fallback requests. + * This leaves semantic family-name bindings untouched. The returned pointer is borrowed and + * remains valid until the font is removed from the scope or the service is destroyed. * * @return A borrowed pointer to the cached font, or nullptr when loading fails. */ diff --git a/src/eepp/graphics/fontservice.cpp b/src/eepp/graphics/fontservice.cpp index 250fd4dc6..cba6b0346 100644 --- a/src/eepp/graphics/fontservice.cpp +++ b/src/eepp/graphics/fontservice.cpp @@ -147,8 +147,13 @@ FontTrueType* FontService::getOrLoadSystemFallbackFont( const FontDesc& desc ) { return ttf; } + // System fallbacks are implementation resources, not family-name bindings. Publishing one + // under desc.family could replace the font whose getGlyph() call requested the fallback and + // destroy that font while it is still executing. + std::string resourceName = + "@system-fallback/" + desc.path + "#" + std::to_string( desc.faceIndex ); FontTrueTypePtr ttf = - FontTrueType::New( desc.family, desc.path, desc.faceIndex, mResourceScope ); + FontTrueType::New( resourceName, desc.path, desc.faceIndex, mResourceScope ); if ( !ttf || !ttf->loaded() ) { if ( ttf ) mResourceScope.eraseLocalFont( ttf.get() ); diff --git a/src/tests/unit_tests/resource_prerequisite_tests.cpp b/src/tests/unit_tests/resource_prerequisite_tests.cpp index fa7b5a53e..87fddb8e5 100644 --- a/src/tests/unit_tests/resource_prerequisite_tests.cpp +++ b/src/tests/unit_tests/resource_prerequisite_tests.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -664,6 +665,30 @@ UTEST( ResourcePrerequisites, distinctFallbackResourcesRemainInFallbackChain ) { Engine::destroySingleton(); } +UTEST( ResourcePrerequisites, systemFallbackDoesNotReplaceMatchingFamilyBinding ) { + EE::Window::Window* window = createLifecycleTestWindow( "System fallback ownership test" ); + ResourceScopePtr scope = ResourceScope::New(); + const std::string fontPath = Sys::getProcessPath() + "assets/fonts/NotoNaskhArabic-Regular.ttf"; + FontTrueTypePtr familyFont = FontTrueType::New( "Noto Naskh Arabic", fontPath, *scope ); + ASSERT_TRUE( familyFont && familyFont->loaded() ); + FontTrueTypeWeakPtr familyFontWeak = familyFont; + + FontDesc desc; + desc.family = "Noto Naskh Arabic"; + desc.path = fontPath; + FontTrueType* fallback = scope->getFontService().getOrLoadSystemFallbackFont( desc ); + ASSERT_TRUE( fallback != nullptr ); + EXPECT_NE( familyFont.get(), fallback ); + EXPECT_EQ( familyFont.get(), scope->findFont( desc.family ).get() ); + + familyFont.reset(); + EXPECT_FALSE( familyFontWeak.expired() ); + scope.reset(); + EXPECT_TRUE( familyFontWeak.expired() ); + window->display( false ); + Engine::destroySingleton(); +} + UTEST( ResourcePrerequisites, fontFactoriesPublishIntoExplicitScope ) { EE::Window::Window* window = createLifecycleTestWindow( "Scoped font factories test" ); auto scope = ResourceScope::New();