Font features are not a Graphite-only feature

We parse features appended to font names in CommonSalLayout as well, so
code that takes care of their presence (striping them when searching,
hashing, etc.) should not be dependent on the old Graphite support or it
will not work when old Graphite is disabled (e.g. on macOS).

Change-Id: If040782a86ec76d3743baf4d2b1d7a194e8e13f2

10 files changed, 32 insertions, 58 deletions

diff --git a/vcl/inc/fontselect.hxx b/vcl/inc/fontselect.hxx
index 57f1815c3ee2..dec59905455b 100644
--- a/vcl/inc/fontselect.hxx
+++ b/vcl/inc/fontselect.hxx
@@ -49,6 +49,9 @@ public:
         return !(*this == rOther);
     }
 
+    static const char FEAT_PREFIX;
+    static const char FEAT_SEPARATOR;
+
 public:
     OUString        maTargetName;               // name of the font name token that is chosen
     OUString        maSearchName;               // name of the font that matches best
diff --git a/vcl/inc/graphite_features.hxx b/vcl/inc/graphite_features.hxx
index 1ce5cb08e608..3caa3ec2789c 100644
--- a/vcl/inc/graphite_features.hxx
+++ b/vcl/inc/graphite_features.hxx
@@ -42,8 +42,6 @@ namespace grutils
     {
     public:
         enum { MAX_FEATURES = 64 };
-        static const char FEAT_PREFIX;
-        static const char FEAT_SEPARATOR;
         static const char FEAT_ID_VALUE_SEPARATOR;
         GrFeatureParser(const gr_face * face, const OString& features, const OString& lang);
         GrFeatureParser(const gr_face * face, const OString& lang);
diff --git a/vcl/source/font/PhysicalFontCollection.cxx b/vcl/source/font/PhysicalFontCollection.cxx
index 0f7449719fc9..716811b5af21 100644
--- a/vcl/source/font/PhysicalFontCollection.cxx
+++ b/vcl/source/font/PhysicalFontCollection.cxx
@@ -18,9 +18,6 @@
  */
 
 #include <config_graphite.h>
-#if ENABLE_GRAPHITE
-#include "graphite_features.hxx"
-#endif
 #include <i18nlangtag/mslangid.hxx>
 #include <unotools/configmgr.hxx>
 #include <unotools/fontdefs.hxx>
@@ -989,22 +986,18 @@ PhysicalFontFamily* PhysicalFontCollection::FindFontFamily( FontSelectPattern& r
         rFSD.maTargetName = GetNextFontToken( rFSD.GetFamilyName(), nTokenPos );
         aSearchName = rFSD.maTargetName;
 
-#if ENABLE_GRAPHITE
         // Until features are properly supported, they are appended to the
         // font name, so we need to strip them off so the font is found.
-        sal_Int32 nFeat = aSearchName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX);
+        sal_Int32 nFeat = aSearchName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX);
         OUString aOrigName = rFSD.maTargetName;
         OUString aBaseFontName = aSearchName.copy( 0, (nFeat != -1) ? nFeat : aSearchName.getLength() );
 
-        if (nFeat != -1 &&
-            -1 != aSearchName.indexOf(grutils::GrFeatureParser::FEAT_ID_VALUE_SEPARATOR, nFeat))
+        if (nFeat != -1)
         {
             aSearchName = aBaseFontName;
             rFSD.maTargetName = aBaseFontName;
         }
 
-#endif
-
         aSearchName = GetEnglishSearchFontName( aSearchName );
         ImplFontSubstitute( aSearchName );
         // #114999# special emboldening for Ricoh fonts
@@ -1035,10 +1028,9 @@ PhysicalFontFamily* PhysicalFontCollection::FindFontFamily( FontSelectPattern& r
             }
         }
 
-#if ENABLE_GRAPHITE
         // restore the features to make the font selection data unique
         rFSD.maTargetName = aOrigName;
-#endif
+
         // check if the current font name token or its substitute is valid
         PhysicalFontFamily* pFoundData = ImplFindFontFamilyBySearchName( aSearchName );
         if( pFoundData )
@@ -1047,10 +1039,9 @@ PhysicalFontFamily* PhysicalFontCollection::FindFontFamily( FontSelectPattern& r
         // some systems provide special customization
         // e.g. they suggest "serif" as UI-font, but this name cannot be used directly
         //      because the system wants to map it to another font first, e.g. "Helvetica"
-#if ENABLE_GRAPHITE
+
         // use the target name to search in the prematch hook
         rFSD.maTargetName = aBaseFontName;
-#endif
 
         // Related: fdo#49271 RTF files often contain weird-ass
         // Win 3.1/Win95 style fontnames which attempt to put the
@@ -1071,11 +1062,11 @@ PhysicalFontFamily* PhysicalFontCollection::FindFontFamily( FontSelectPattern& r
             if( mpPreMatchHook->FindFontSubstitute( rFSD ) )
                 aSearchName = GetEnglishSearchFontName( aSearchName );
         }
-#if ENABLE_GRAPHITE
+
         // the prematch hook uses the target name to search, but we now need
         // to restore the features to make the font selection data unique
         rFSD.maTargetName = aOrigName;
-#endif
+
         pFoundData = ImplFindFontFamilyBySearchName( aSearchName );
         if( pFoundData )
             return pFoundData;
diff --git a/vcl/source/font/fontcache.cxx b/vcl/source/font/fontcache.cxx
index 4991b6d0d389..216283263931 100644
--- a/vcl/source/font/fontcache.cxx
+++ b/vcl/source/font/fontcache.cxx
@@ -24,11 +24,6 @@
 #include "PhysicalFontFace.hxx"
 #include "PhysicalFontFamily.hxx"
 
-#include <config_graphite.h>
-#if ENABLE_GRAPHITE
-#include "graphite_features.hxx"
-#endif
-
 size_t ImplFontCache::IFSD_Hash::operator()( const FontSelectPattern& rFSD ) const
 {
     return rFSD.hashCode();
@@ -73,14 +68,12 @@ bool ImplFontCache::IFSD_Equal::operator()(const FontSelectPattern& rA, const Fo
             return false;
     }
 
-#if ENABLE_GRAPHITE
     // check for features
-    if ((rA.maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+    if ((rA.maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
          != -1 ||
-         rB.maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+         rB.maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
          != -1) && rA.maTargetName != rB.maTargetName)
         return false;
-#endif
 
     if (rA.mbEmbolden != rB.mbEmbolden)
         return false;
diff --git a/vcl/source/font/fontselect.cxx b/vcl/source/font/fontselect.cxx
index 3745f041b5dd..f4b4f77ca3a5 100644
--- a/vcl/source/font/fontselect.cxx
+++ b/vcl/source/font/fontselect.cxx
@@ -22,10 +22,9 @@
 #include <PhysicalFontFace.hxx>
 #include "svdata.hxx"
 
-#include <config_graphite.h>
-#if ENABLE_GRAPHITE
-#include "graphite_features.hxx"
-#endif
+// These mustn't conflict with font name lists which use ; and ,
+const char FontSelectPatternAttributes::FEAT_PREFIX = ':';
+const char FontSelectPatternAttributes::FEAT_SEPARATOR = '&';
 
 FontSelectPattern::FontSelectPattern( const vcl::Font& rFont,
     const OUString& rSearchName, const Size& rSize, float fExactHeight)
@@ -105,15 +104,13 @@ size_t FontSelectPatternAttributes::hashCode() const
 {
     // TODO: does it pay off to improve this hash function?
     size_t nHash;
-#if ENABLE_GRAPHITE
     // check for features and generate a unique hash if necessary
-    if (maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+    if (maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
         != -1)
     {
         nHash = maTargetName.hashCode();
     }
     else
-#endif
     {
         nHash = maSearchName.hashCode();
     }
diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx
index 5e25186257c2..bcd8c78284cf 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -125,15 +125,15 @@ static hb_unicode_funcs_t* getUnicodeFuncs()
 void CommonSalLayout::ParseFeatures(const OUString& name)
 {
     int nFeatures = 0;
-    int nStart = name.indexOf(':');
+    int nStart = name.indexOf(FontSelectPatternAttributes::FEAT_PREFIX);
     if (nStart < 0)
         return;
     OString oName = OUStringToOString(name, RTL_TEXTENCODING_ASCII_US);
-    for (int nNext = nStart; nNext > 0; nNext = name.indexOf('&', nNext + 1))
+    for (int nNext = nStart; nNext > 0; nNext = name.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, nNext + 1))
     {
         if (name.match("lang=", nNext + 1))
         {
-            int endamp = name.indexOf('&', nNext+1);
+            int endamp = name.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, nNext+1);
             int enddelim = name.indexOf(' ', nNext+1);
             int end = name.getLength();
             if (endamp < 0)
@@ -154,7 +154,9 @@ void CommonSalLayout::ParseFeatures(const OUString& name)
         return;
 
     maFeatures.reserve(nFeatures);
-    for (int nThis = nStart, nNext = name.indexOf('&', nStart + 1); nThis > 0; nThis = nNext, nNext = name.indexOf('&', nNext + 1))
+    for (int nThis = nStart, nNext = name.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, nStart + 1);
+         nThis > 0;
+         nThis = nNext, nNext = name.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, nNext + 1))
     {
         if (!name.match("lang=", nThis + 1))
         {
diff --git a/vcl/source/glyphs/graphite_features.cxx b/vcl/source/glyphs/graphite_features.cxx
index fab008ce174a..6464e3fa8e14 100644
--- a/vcl/source/glyphs/graphite_features.cxx
+++ b/vcl/source/glyphs/graphite_features.cxx
@@ -29,6 +29,7 @@
 #include <windows.h>
 #endif
 
+#include <fontselect.hxx>
 #include <graphite_features.hxx>
 
 using namespace grutils;
@@ -74,9 +75,6 @@ short getIntValue(const OString & id, size_t offset, size_t length)
 
 }
 
-// These mustn't conflict with font name lists which use ; and ,
-const char GrFeatureParser::FEAT_PREFIX = ':';
-const char GrFeatureParser::FEAT_SEPARATOR = '&';
 const char GrFeatureParser::FEAT_ID_VALUE_SEPARATOR = '=';
 
 GrFeatureParser::GrFeatureParser(const gr_face * pFace, const OString& lang)
@@ -106,7 +104,7 @@ GrFeatureParser::GrFeatureParser(const gr_face * pFace, const OString& features,
         if (features.match(aLangPrefix, pos ))
         {
             pos = nEquals + 1;
-            nFeatEnd = features.indexOf(FEAT_SEPARATOR, pos);
+            nFeatEnd = features.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, pos);
             if (nFeatEnd == -1)
             {
                 nFeatEnd = features.getLength();
@@ -164,7 +162,7 @@ GrFeatureParser::GrFeatureParser(const gr_face * pFace, const OString& features,
             }
             const gr_feature_ref * pFref = gr_face_find_fref(pFace, featId);
             pos = nEquals + 1;
-            nFeatEnd = features.indexOf(FEAT_SEPARATOR, pos);
+            nFeatEnd = features.indexOf(FontSelectPatternAttributes::FEAT_SEPARATOR, pos);
             if (nFeatEnd == -1)
             {
                 nFeatEnd = features.getLength();
diff --git a/vcl/unx/generic/glyphs/glyphcache.cxx b/vcl/unx/generic/glyphs/glyphcache.cxx
index fc3fa5f0e3ee..c1b4a4e80c63 100644
--- a/vcl/unx/generic/glyphs/glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/glyphcache.cxx
@@ -26,11 +26,6 @@
 #include <fontinstance.hxx>
 #include <fontattributes.hxx>
 
-#include <config_graphite.h>
-#if ENABLE_GRAPHITE
-#include <graphite_features.hxx>
-#endif
-
 #include <rtl/ustring.hxx>
 #include <osl/file.hxx>
 #include <tools/debug.hxx>
@@ -74,23 +69,22 @@ size_t GlyphCache::IFSD_Hash::operator()( const FontSelectPattern& rFontSelData
 {
     // TODO: is it worth to improve this hash function?
     sal_IntPtr nFontId = reinterpret_cast<sal_IntPtr>( rFontSelData.mpFontData );
-#if ENABLE_GRAPHITE
-    if (rFontSelData.maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+
+    if (rFontSelData.maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
         != -1)
     {
         OString aFeatName = OUStringToOString( rFontSelData.maTargetName, RTL_TEXTENCODING_UTF8 );
         nFontId ^= aFeatName.hashCode();
     }
-#endif
+
     size_t nHash = nFontId << 8;
     nHash   += rFontSelData.mnHeight;
     nHash   += rFontSelData.mnOrientation;
     nHash   += size_t(rFontSelData.mbVertical);
     nHash   += rFontSelData.GetItalic();
     nHash   += rFontSelData.GetWeight();
-#if ENABLE_GRAPHITE
     nHash   += rFontSelData.meLanguage;
-#endif
+
     return nHash;
 }
 
@@ -121,16 +115,14 @@ bool GlyphCache::IFSD_Equal::operator()( const FontSelectPattern& rA, const Font
     if( nAWidth != nBWidth )
         return false;
 
-#if ENABLE_GRAPHITE
    if (rA.meLanguage != rB.meLanguage)
         return false;
    // check for features
-   if ((rA.maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+   if ((rA.maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
         != -1 ||
-        rB.maTargetName.indexOf(grutils::GrFeatureParser::FEAT_PREFIX)
+        rB.maTargetName.indexOf(FontSelectPatternAttributes::FEAT_PREFIX)
         != -1) && rA.maTargetName != rB.maTargetName)
         return false;
-#endif
 
     if (rA.mbEmbolden != rB.mbEmbolden)
         return false;
diff --git a/vcl/unx/generic/glyphs/graphite_serverfont.cxx b/vcl/unx/generic/glyphs/graphite_serverfont.cxx
index 6102165ce2ee..af57ded6d861 100644
--- a/vcl/unx/generic/glyphs/graphite_serverfont.cxx
+++ b/vcl/unx/generic/glyphs/graphite_serverfont.cxx
@@ -72,7 +72,7 @@ GraphiteServerFontLayout::GraphiteServerFontLayout(FreetypeFont& rFreetypeFont)
         rFreetypeFont.GetFtFace()->size->metrics.x_ppem,
         rFreetypeFont.GetFontSelData().mfExactHeight);
 #endif
-    sal_Int32 nFeat = name.indexOf(grutils::GrFeatureParser::FEAT_PREFIX) + 1;
+    sal_Int32 nFeat = name.indexOf(FontSelectPatternAttributes::FEAT_PREFIX) + 1;
     if (nFeat > 0)
     {
         OString aFeat = name.copy(nFeat, name.getLength() - nFeat);
diff --git a/vcl/win/gdi/winlayout.cxx b/vcl/win/gdi/winlayout.cxx
index 3f051d706053..468bc1ccbc1a 100644
--- a/vcl/win/gdi/winlayout.cxx
+++ b/vcl/win/gdi/winlayout.cxx
@@ -3735,7 +3735,7 @@ GraphiteWinLayout::GraphiteWinLayout(HDC hDC, const WinFontFace& rWFD, WinFontIn
             RTL_TEXTENCODING_ASCII_US);
     OString name = OUStringToOString(
         rWFE.maFontSelData.maTargetName, RTL_TEXTENCODING_UTF8 );
-    sal_Int32 nFeat = name.indexOf(grutils::GrFeatureParser::FEAT_PREFIX) + 1;
+    sal_Int32 nFeat = name.indexOf(FontSelectPatternAttributes::FEAT_PREFIX) + 1;
     if (nFeat > 0)
     {
         OString aFeat = name.copy(nFeat, name.getLength() - nFeat);