mirror of
https://github.com/SpartanJ/eepp.git
synced 2026-10-01 18:50:16 +03:00
Fix external font handling in UIFontPickerDialog
Preserve explicitly selected font styles and correctly reconcile external fonts with the asynchronously loaded system font list. Group same-family external fonts under the Style column when styles are visible, and expose them as distinct family entries when styles are hidden. Include filenames in localized external-font labels to distinguish alternate font versions. Ensure system-listed regular faces remain available after asynchronous loading and add regression coverage for collisions, selection, and hidden style behavior.
This commit is contained in:
Martín Lucas Golini
@@ -472,6 +472,8 @@ void UIFontPickerDialog::loadFonts() {
|
||||
|
||||
void UIFontPickerDialog::setFonts( std::vector<FontDesc> fonts ) {
|
||||
FontDesc selectedFont = mSelection.font;
|
||||
for ( const auto& font : fonts )
|
||||
mExternalFontKeys.erase( font.getFileKey() );
|
||||
mergeLoadedFonts( fonts );
|
||||
for ( const auto& font : mFonts ) {
|
||||
auto found = std::find_if( fonts.begin(), fonts.end(),
|
||||
@@ -575,21 +577,32 @@ void UIFontPickerDialog::updateFamilies() {
|
||||
FontDesc previousFont = mSelection.font;
|
||||
|
||||
const std::string query = String::toLower( mSearchInput->getText().toUtf8() );
|
||||
UnorderedSet<std::string> systemFamilies;
|
||||
for ( const auto& font : mFonts ) {
|
||||
if ( !isExternalFont( font ) )
|
||||
systemFamilies.insert( font.family );
|
||||
}
|
||||
UnorderedSet<std::string> familyKeys;
|
||||
mFamilies.clear();
|
||||
for ( const auto& font : mFonts ) {
|
||||
const bool external =
|
||||
mExternalFontKeys.find( font.getFileKey() ) != mExternalFontKeys.end();
|
||||
const bool external = isExternalFont( font );
|
||||
if ( wantsMonospaceOnly() && !font.monospace && !external )
|
||||
continue;
|
||||
const std::string label = font.family + ( external ? " [External]" : "" );
|
||||
const bool separateExternal = external && ( mFlags & ShowStyle ) == 0;
|
||||
const bool externalOnly = systemFamilies.find( font.family ) == systemFamilies.end();
|
||||
std::string label( font.family );
|
||||
if ( separateExternal ) {
|
||||
label += " " + externalTag( font, true );
|
||||
} else if ( externalOnly ) {
|
||||
label += " " + externalTag( font, false );
|
||||
}
|
||||
if ( !query.empty() && String::toLower( label ).find( query ) == std::string::npos )
|
||||
continue;
|
||||
const std::string familyKey =
|
||||
font.family + ( external ? "\n" + font.getFileKey() : std::string{} );
|
||||
font.family + ( separateExternal ? "\n" + font.getFileKey() : std::string{} );
|
||||
if ( familyKeys.insert( familyKey ).second )
|
||||
mFamilies.push_back(
|
||||
{ label, font.family, external ? font.getFileKey() : std::string{} } );
|
||||
{ label, font.family, separateExternal ? font.getFileKey() : std::string{} } );
|
||||
}
|
||||
mFamilyModel = std::make_shared<FamilyListModel>( &mFamilies );
|
||||
|
||||
@@ -609,22 +622,23 @@ void UIFontPickerDialog::updateFamilies() {
|
||||
|
||||
void UIFontPickerDialog::updateStyles() {
|
||||
mStyles.clear();
|
||||
bool selectionMatchesFamily = false;
|
||||
if ( !mFamilyList->getSelection().isEmpty() ) {
|
||||
const Int64 row = mFamilyList->getSelection().first().row();
|
||||
if ( row >= 0 && row < static_cast<Int64>( mFamilies.size() ) ) {
|
||||
const FontFamilyEntry& family = mFamilies[row];
|
||||
selectionMatchesFamily = familyEntryMatchesFont( family, mSelection.font );
|
||||
for ( const auto& font : mFonts ) {
|
||||
const bool external =
|
||||
mExternalFontKeys.find( font.getFileKey() ) != mExternalFontKeys.end();
|
||||
const bool matchesSource = family.externalFontKey.empty()
|
||||
? !external
|
||||
: family.externalFontKey == font.getFileKey();
|
||||
if ( font.family == family.family && matchesSource &&
|
||||
const bool external = isExternalFont( font );
|
||||
if ( familyEntryMatchesFont( family, font ) &&
|
||||
( !wantsMonospaceOnly() || font.monospace || external ) ) {
|
||||
std::string label( styleLabel( font ) );
|
||||
auto tagIt = mFontTags.find( font.getFileKey() );
|
||||
if ( tagIt != mFontTags.end() )
|
||||
if ( external ) {
|
||||
label += " " + externalTag( font, true );
|
||||
} else if ( tagIt != mFontTags.end() ) {
|
||||
label += " [" + tagIt->second + "]";
|
||||
}
|
||||
mStyles.push_back( { label, font } );
|
||||
}
|
||||
}
|
||||
@@ -637,8 +651,7 @@ void UIFontPickerDialog::updateStyles() {
|
||||
mUpdating = false;
|
||||
|
||||
if ( !mStyles.empty() ) {
|
||||
const std::string selectedFamily( mStyles.front().desc.family );
|
||||
if ( selectedFamily == mSelection.font.family )
|
||||
if ( selectionMatchesFamily && ( mFlags & ShowStyle ) != 0 )
|
||||
selectStyle( mSelection.font );
|
||||
else
|
||||
selectRegularStyle();
|
||||
@@ -747,12 +760,12 @@ void UIFontPickerDialog::selectInitialRows() {
|
||||
void UIFontPickerDialog::selectFamily( const FontDesc& font ) {
|
||||
if ( font.family.empty() || !mFamilyModel )
|
||||
return;
|
||||
const bool external = mExternalFontKeys.find( font.getFileKey() ) != mExternalFontKeys.end();
|
||||
const bool separateExternal = isExternalFont( font ) && ( mFlags & ShowStyle ) == 0;
|
||||
for ( size_t i = 0; i < mFamilies.size(); i++ ) {
|
||||
const FontFamilyEntry& family = mFamilies[i];
|
||||
if ( family.family == font.family &&
|
||||
( external ? family.externalFontKey == font.getFileKey()
|
||||
: family.externalFontKey.empty() ) ) {
|
||||
( separateExternal ? family.externalFontKey == font.getFileKey()
|
||||
: family.externalFontKey.empty() ) ) {
|
||||
mFamilyList->setSelection( mFamilyModel->index( i ) );
|
||||
return;
|
||||
}
|
||||
@@ -899,6 +912,30 @@ bool UIFontPickerDialog::addExternalFont( const std::string& path, Uint32 faceIn
|
||||
return true;
|
||||
}
|
||||
|
||||
bool UIFontPickerDialog::isExternalFont( const FontDesc& font ) const {
|
||||
return mExternalFontKeys.find( font.getFileKey() ) != mExternalFontKeys.end();
|
||||
}
|
||||
|
||||
std::string UIFontPickerDialog::externalTag( const FontDesc& font, bool includeFileName ) {
|
||||
const std::string fileName( FileSystem::fileNameFromPath( font.path ) );
|
||||
if ( includeFileName && !fileName.empty() ) {
|
||||
return "[" +
|
||||
String::format( i18n( "font_picker_external_file", "External: %s" ).toUtf8(),
|
||||
fileName ) +
|
||||
"]";
|
||||
}
|
||||
return "[" + i18n( "font_picker_external", "External" ).toUtf8() + "]";
|
||||
}
|
||||
|
||||
bool UIFontPickerDialog::familyEntryMatchesFont( const FontFamilyEntry& family,
|
||||
const FontDesc& font ) const {
|
||||
if ( family.family != font.family )
|
||||
return false;
|
||||
if ( family.externalFontKey.empty() )
|
||||
return ( mFlags & ShowStyle ) != 0 || !isExternalFont( font );
|
||||
return family.externalFontKey == font.getFileKey();
|
||||
}
|
||||
|
||||
void UIFontPickerDialog::clearBrowseDialog() {
|
||||
if ( !mBrowseDialog )
|
||||
return;
|
||||
@@ -930,7 +967,6 @@ void UIFontPickerDialog::setSelectedFont( const FontDesc& desc ) {
|
||||
mMonospaceOnly->setChecked( true );
|
||||
selectFamily( selection );
|
||||
updateStyles();
|
||||
selectStyle( selection );
|
||||
updateSelectionFromLists( false );
|
||||
if ( previousSelection != mSelection )
|
||||
emitSelectionChanged();
|
||||
|
||||
@@ -65,6 +65,13 @@ class TestFontPickerDialog : public UIFontPickerDialog {
|
||||
Uint32 getPreviewTextSize() const { return mPreviewText->getFontSize(); }
|
||||
Uint32 getPreviewInputSize() const { return mPreviewInput->getFontSize(); }
|
||||
String getDetails() const { return mDetailsText->getText(); }
|
||||
void setFontsForTest( std::vector<FontDesc> fonts ) { setFonts( std::move( fonts ) ); }
|
||||
void markSelectedFontExternalForTest() {
|
||||
FontDesc selectedFont = mSelection.font;
|
||||
mExternalFontKeys.insert( selectedFont.getFileKey() );
|
||||
updateFamilies();
|
||||
setSelectedFont( selectedFont );
|
||||
}
|
||||
|
||||
protected:
|
||||
TestFontPickerDialog( Uint32 flags ) : UIFontPickerDialog( flags ) {}
|
||||
@@ -97,7 +104,7 @@ UTEST( UIFontPickerDialog, PreselectsExternalFontPath ) {
|
||||
EXPECT_FALSE( selectionDialog->getSelection().font.family.empty() );
|
||||
}
|
||||
|
||||
UTEST( UIFontPickerDialog, ExternalFontKeepsSeparateFamilyEntryOnNameCollision ) {
|
||||
UTEST( UIFontPickerDialog, ExternalFontBecomesDistinctStyleOnFamilyNameCollision ) {
|
||||
const std::string managedFontPath =
|
||||
Sys::getProcessPath() + "assets/fonts/NotoSansKR-Regular.ttf";
|
||||
ASSERT_TRUE( FileSystem::fileExists( managedFontPath ) );
|
||||
@@ -126,23 +133,44 @@ UTEST( UIFontPickerDialog, ExternalFontKeepsSeparateFamilyEntryOnNameCollision )
|
||||
|
||||
EXPECT_STDSTREQ( externalPath, dialog->getSelection().font.path );
|
||||
EXPECT_FALSE( dialog->getFamilyList()->getSelection().isEmpty() );
|
||||
EXPECT_STDSTREQ( family + " [External]",
|
||||
dialog->getFamilyList()->getSelection().first().data().toString() );
|
||||
EXPECT_STDSTREQ( family, dialog->getFamilyList()->getSelection().first().data().toString() );
|
||||
EXPECT_TRUE( dialog->getStyleList()->getSelection().first().data().toString().find(
|
||||
"[External:" ) != std::string::npos );
|
||||
|
||||
bool foundSystemFamily = false;
|
||||
bool foundExternalFamily = false;
|
||||
size_t matchingFamilies = 0;
|
||||
Model* familyModel = dialog->getFamilyList()->getModel();
|
||||
ASSERT_TRUE( familyModel != nullptr );
|
||||
for ( size_t row = 0; row < familyModel->rowCount(); row++ ) {
|
||||
const std::string label = familyModel->index( row ).data().toString();
|
||||
foundSystemFamily |= label == family;
|
||||
foundExternalFamily |= label == family + " [External]";
|
||||
matchingFamilies += label == family;
|
||||
}
|
||||
EXPECT_TRUE( foundSystemFamily );
|
||||
EXPECT_TRUE( foundExternalFamily );
|
||||
EXPECT_EQ( 1u, matchingFamilies );
|
||||
|
||||
bool foundSystemStyle = false;
|
||||
bool foundExternalStyle = false;
|
||||
Model* styleModel = dialog->getStyleList()->getModel();
|
||||
ASSERT_TRUE( styleModel != nullptr );
|
||||
for ( size_t row = 0; row < styleModel->rowCount(); row++ ) {
|
||||
const std::string label = styleModel->index( row ).data().toString();
|
||||
foundSystemStyle |= label.find( "[External:" ) == std::string::npos;
|
||||
foundExternalStyle |= label.find( "[External:" ) != std::string::npos;
|
||||
}
|
||||
EXPECT_TRUE( foundSystemStyle );
|
||||
EXPECT_TRUE( foundExternalStyle );
|
||||
EXPECT_EQ( managedFont.get(), resourceScope.findFont( externalName ).get() );
|
||||
|
||||
TestFontPickerDialog* hiddenStyleDialog =
|
||||
TestFontPickerDialog::New( UIFontPickerDialog::ShowSize );
|
||||
hiddenStyleDialog->setSelectedFont( externalPath );
|
||||
pumpUntil( app.getUI(),
|
||||
[hiddenStyleDialog] { return hiddenStyleDialog->getButtonOK()->isEnabled(); } );
|
||||
EXPECT_STDSTREQ( externalPath, hiddenStyleDialog->getSelection().font.path );
|
||||
EXPECT_FALSE( hiddenStyleDialog->getStyleList()->getParent()->isVisible() );
|
||||
EXPECT_TRUE( hiddenStyleDialog->getFamilyList()->getSelection().first().data().toString().find(
|
||||
"[External:" ) != std::string::npos );
|
||||
|
||||
dialog->releasePreviewFont();
|
||||
hiddenStyleDialog->releasePreviewFont();
|
||||
FileSystem::fileRemove( externalPath );
|
||||
}
|
||||
|
||||
@@ -324,7 +352,44 @@ UTEST( UIFontPickerDialog, AsyncLoadPreservesExternalFontPreselection ) {
|
||||
EXPECT_FALSE( dialog->getFamilyList()->getSelection().isEmpty() );
|
||||
EXPECT_FALSE( dialog->getStyleList()->getSelection().isEmpty() );
|
||||
EXPECT_TRUE( dialog->getFamilyList()->getSelection().first().data().toString().find(
|
||||
"[External]" ) != std::string::npos );
|
||||
"[External" ) != std::string::npos );
|
||||
}
|
||||
|
||||
UTEST( UIFontPickerDialog, AsyncLoadPromotesEnumeratedExternalFontToSystemFamily ) {
|
||||
std::vector<FontDesc> fonts = SystemFontResolver::instance()->enumerate();
|
||||
auto regular = std::find_if( fonts.begin(), fonts.end(), []( const FontDesc& font ) {
|
||||
return font.weight == FontWeight::Normal && !font.italic && !font.family.empty() &&
|
||||
!font.path.empty();
|
||||
} );
|
||||
if ( regular == fonts.end() )
|
||||
UTEST_SKIP( "no regular system font available" );
|
||||
const FontDesc regularFont = *regular;
|
||||
|
||||
UIApplication app(
|
||||
WindowSettings( 320, 240, "eepp - UIFontPickerDialog Test", WindowStyle::Default,
|
||||
WindowBackend::Default, 32 ),
|
||||
UIApplication::Settings( Sys::getProcessPath() + ".." + FileSystem::getOSSlash(), 1 ) );
|
||||
|
||||
TestFontPickerDialog* dialog = TestFontPickerDialog::New();
|
||||
dialog->setFontsForTest( fonts );
|
||||
dialog->setSelectedFont( regularFont );
|
||||
dialog->markSelectedFontExternalForTest();
|
||||
EXPECT_STDSTREQ( regularFont.family + " [External]",
|
||||
dialog->getFamilyList()->getSelection().first().data().toString() );
|
||||
EXPECT_TRUE( dialog->getStyleList()->getSelection().first().data().toString().find(
|
||||
"[External:" ) != std::string::npos );
|
||||
|
||||
dialog->setFontsForTest( std::move( fonts ) );
|
||||
EXPECT_STDSTREQ( regularFont.path, dialog->getSelection().font.path );
|
||||
EXPECT_EQ( regularFont.faceIndex, dialog->getSelection().font.faceIndex );
|
||||
EXPECT_EQ( FontWeight::Normal, dialog->getSelection().font.weight );
|
||||
EXPECT_FALSE( dialog->getSelection().font.italic );
|
||||
EXPECT_STDSTREQ( regularFont.family,
|
||||
dialog->getFamilyList()->getSelection().first().data().toString() );
|
||||
EXPECT_TRUE( dialog->getStyleList()->getSelection().first().data().toString().find(
|
||||
"[External:" ) == std::string::npos );
|
||||
|
||||
dialog->releasePreviewFont();
|
||||
}
|
||||
|
||||
UTEST( UIFontPickerDialog, DefaultColorComesFromTheme ) {
|
||||
@@ -415,6 +480,43 @@ UTEST( UIFontPickerDialog, StyleLabelsIncludeFontStretch ) {
|
||||
EXPECT_TRUE( foundStyle );
|
||||
}
|
||||
|
||||
UTEST( UIFontPickerDialog, PreservesVisibleStyleAndPrefersRegularWhenHidden ) {
|
||||
std::vector<FontDesc> fonts = SystemFontResolver::instance()->enumerate();
|
||||
auto italic = std::find_if( fonts.begin(), fonts.end(), [&]( const FontDesc& candidate ) {
|
||||
if ( !candidate.italic )
|
||||
return false;
|
||||
return std::find_if( fonts.begin(), fonts.end(), [&]( const FontDesc& regular ) {
|
||||
return regular.family == candidate.family &&
|
||||
regular.weight == FontWeight::Normal && !regular.italic;
|
||||
} ) != fonts.end();
|
||||
} );
|
||||
if ( italic == fonts.end() )
|
||||
UTEST_SKIP( "no family with regular and italic styles available" );
|
||||
const FontDesc italicFont = *italic;
|
||||
|
||||
UIApplication app(
|
||||
WindowSettings( 320, 240, "eepp - UIFontPickerDialog Test", WindowStyle::Default,
|
||||
WindowBackend::Default, 32 ),
|
||||
UIApplication::Settings( Sys::getProcessPath() + ".." + FileSystem::getOSSlash(), 1 ) );
|
||||
|
||||
TestFontPickerDialog* visible = TestFontPickerDialog::New( UIFontPickerDialog::ShowStyle );
|
||||
visible->setFontsForTest( fonts );
|
||||
visible->setSelectedFont( italicFont );
|
||||
EXPECT_TRUE( visible->getStyleList()->getParent()->isVisible() );
|
||||
EXPECT_EQ( italicFont.weight, visible->getSelection().font.weight );
|
||||
EXPECT_TRUE( visible->getSelection().font.italic );
|
||||
|
||||
TestFontPickerDialog* hidden = TestFontPickerDialog::New( UIFontPickerDialog::ShowSize );
|
||||
hidden->setFontsForTest( std::move( fonts ) );
|
||||
hidden->setSelectedFont( italicFont );
|
||||
EXPECT_FALSE( hidden->getStyleList()->getParent()->isVisible() );
|
||||
EXPECT_EQ( FontWeight::Normal, hidden->getSelection().font.weight );
|
||||
EXPECT_FALSE( hidden->getSelection().font.italic );
|
||||
|
||||
visible->releasePreviewFont();
|
||||
hidden->releasePreviewFont();
|
||||
}
|
||||
|
||||
UTEST( UIFontPickerDialog, ApplyButtonEmitsOnApply ) {
|
||||
UIApplication app(
|
||||
WindowSettings( 320, 240, "eepp - UIFontPickerDialog Test", WindowStyle::Default,
|
||||
|
||||
@@ -105,7 +105,8 @@ void FontPickerController::openFontDialog( std::string& fontPath, bool loadingMo
|
||||
defaultResourceScope().publishLocalFont( std::move( fontName ), font );
|
||||
};
|
||||
|
||||
const Uint32 flags = ( pickFontSize ? UIFontPickerDialog::ShowSize : 0 ) |
|
||||
const Uint32 flags = UIFontPickerDialog::ShowStyle |
|
||||
( pickFontSize ? UIFontPickerDialog::ShowSize : 0 ) |
|
||||
( loadingMonoFont ? UIFontPickerDialog::MonospaceOnly : 0 );
|
||||
UIFontPickerDialog* dialog = UIFontPickerDialog::New( flags );
|
||||
dialog->setTitle( mApp->i18n( "select_font", "Select Font" ) );
|
||||
|
||||
Reference in New Issue
Block a user