Issue 54603 - using fontconfig for font fallback
Summary: using fontconfig for font fallback
Status: CLOSED FIXED
Alias: None
Product: gsl
Classification: Code
Component: code (show other issues)
Version: 680m129
Hardware: All Unix, all
: P3 Trivial (vote)
Target Milestone: OOo 2.4
Assignee: eric.savary
QA Contact: issues@gsl
URL:
Keywords:
: 46085 63272 66321 (view as issue list)
Depends on: 58663 61835
Blocks:
  Show dependency tree
 
Reported: 2005-09-14 15:07 UTC by caolanm
Modified: 2008-09-19 12:09 UTC (History)
8 users (show)

See Also:
Issue Type: ENHANCEMENT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
patch to implement (42.81 KB, patch)
2005-09-14 15:08 UTC, caolanm
no flags Details | Diff
updated patch for 2.0.1 (44.33 KB, patch)
2005-11-22 12:07 UTC, caolanm
no flags Details | Diff
for CJK improve (48.19 KB, patch)
2006-08-21 17:24 UTC, jianhuajiao
no flags Details | Diff
update for cmc's sugestion (47.29 KB, patch)
2006-08-22 03:03 UTC, jianhuajiao
no flags Details | Diff
additional patch to show localized font name (5.84 KB, patch)
2006-11-06 09:29 UTC, caolanm
no flags Details | Diff
for cjk improvement (8.90 KB, text/plain)
2006-12-29 03:48 UTC, pflin
no flags Details
resynced current patch to OOG680 (59.19 KB, patch)
2007-08-21 14:02 UTC, caolanm
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description caolanm 2005-09-14 15:07:06 UTC
Attached is a patch to extend the use of fontconfig to determine the fallback
font when the desired font is missing or has missing glyphs. Been using a
version of this for some time. Hopefully a varient of this will make it's way
into a future OOo release.
Comment 1 caolanm 2005-09-14 15:08:30 UTC
Created attachment 29557 [details]
patch to implement
Comment 2 caolanm 2005-11-22 12:07:50 UTC
Created attachment 31710 [details]
updated patch for 2.0.1
Comment 3 hdu@apache.org 2005-11-30 11:30:17 UTC
Thanks for the patch!

Am I misunderstanding something or are you slaughtering the "glyph fallback"
feature at least on WIN32 and also on platforms without fontconfig? AFAIK all
UNX baselines don't require fontconfig yet...
Comment 4 caolanm 2005-11-30 11:34:25 UTC
might need a bit of re-work to support fully falling back to existing system for
non-fontconfig available case :-)
Comment 5 philipp.lohmann 2006-03-21 18:25:20 UTC
*** Issue 63272 has been marked as a duplicate of this issue. ***
Comment 6 jianhuajiao 2006-08-21 17:22:33 UTC
updat the patch for cjk compatibility
1. check cjk charactor one by one.
2. set the target font name as empty when the font not in the font list.
3. cjk font name convertion localized <-> english font name

for 3, it need font name array correct. issue relate with 68839
Comment 7 jianhuajiao 2006-08-21 17:24:09 UTC
Created attachment 38684 [details]
for CJK improve
Comment 8 caolanm 2006-08-21 19:50:58 UTC
-    while ( pWin && !pWin->IsSystemWindow() )
+    while ( !pWin->IsSystemWindow() )

doesn't look quite right
Comment 9 jianhuajiao 2006-08-22 03:02:19 UTC
jianhuajiao->cmc thanks 
I am a idiot for create a patch.
Comment 10 jianhuajiao 2006-08-22 03:03:00 UTC
Created attachment 38690 [details]
update for cmc's sugestion
Comment 11 jianhuajiao 2006-11-03 09:44:21 UTC
This patch will cause all fonts name are English name, no localized name. I
reversed some code to solve this problem.

--- psprint/source/fontmanager/fontconfig.cxx   2006-11-03 17:19:11.000000000 +0800
+++ psprint/source/fontmanager/fontconfig.cxx   2006-11-02 18:19:13.000000000 +0800
@@ -417,8 +417,21 @@ bool PrintFontManager::initFontconfig()
     FontCfgWrapper& rWrapper = FontCfgWrapper::get();
     if( ! rWrapper.isValid() )
         return false;
-
-    FcFontSet* pFSet = rWrapper.getFontSet();
+    FcConfig* pConfig = rWrapper.getDefConfig();
+    FcObjectSet* pOSet = rWrapper.FcObjectSetBuild( FC_FAMILY,
+                                                    FC_STYLE,
+                                                    FC_SLANT,
+                                                    FC_WEIGHT,
+                                                    FC_SPACING,
+                                                    FC_FILE,
+                                                    FC_OUTLINE,
+                                                    FC_INDEX,
+                                                    FC_EMBEDDED_BITMAP,
+                                                    FC_ANTIALIAS,
+                                                    (void *) NULL );
+    FcPattern* pPattern = rWrapper.FcPatternCreate();
+    FcFontSet* pFSet = rWrapper.FcFontList( pConfig, pPattern, pOSet );
+//    FcFontSet* pFSet = rWrapper.getFontSet();

     if( pFSet )
     {


Any suggestion?
Comment 12 caolanm 2006-11-06 09:27:19 UTC
well, that patch on it's own leaks some resources, the pFset and so on.

FWIW, the reason I create a fontset to make the rest of this work out of,
instead of the default fontconfig list is so as to restrict the fonts that
fontconfig works with for font and glyph fallback to the set of fonts that are
outline fonts, so that we don't consider non-outline fonts for fallback.

How about this alternative patch instead to use the localized fonts, does it
work for you. It also sketches out the area in which we could in the future pull
the fontnames to use for the UI from the UILocale instead of the process locale
so that font names follow that setting of OOo
Comment 13 caolanm 2006-11-06 09:29:52 UTC
Created attachment 40359 [details]
additional patch to show localized font name
Comment 14 hdu@apache.org 2006-11-07 15:17:05 UTC
I'd apply the patches if only glyph fallback wasn't damaged for the case that
fontconfig is not available (e.g. for the few people still running Win32).

@PL: please review the psprint part of the patches.
Comment 15 philipp.lohmann 2006-11-07 15:39:15 UTC
The psprint part so far looks good to me (it's to complex to say definitely ok
or not). however i stumbled over this minor part that makes no sense to me.

--- psprint/source/fontmanager/fontcache.cxx	2006-08-21 14:19:19.000000000 +0800
+++ psprint/source/fontmanager/fontcache.cxx	2006-08-11 09:42:31.000000000 +0800
@@ -679,9 +679,9 @@
         FontDirMap::const_iterator entry = dir->second.m_aEntries.find( rFile );
         if( entry != dir->second.m_aEntries.end() )
         {
-            bSuccess = true;
             for( FontCacheEntry::const_iterator font =
entry->second.m_aEntry.begin(); font != entry->second.m_aEntry.end(); ++font )
             {
+                bSuccess = true;
                 PrintFontManager::PrintFont* pFont = clonePrintFont( *font );
                 rNewFonts.push_back( pFont );
             }
Comment 16 philipp.lohmann 2006-11-27 17:24:46 UTC
*** Issue 46085 has been marked as a duplicate of this issue. ***
Comment 17 pflin 2006-12-29 03:44:53 UTC
for CJK improvement:
1) The font attributes should not be applied to CJK characters when the selected
font doesn't support CJK lang.
2) the Asian font listbox in character property dialog only list CJK fonts
Comment 18 pflin 2006-12-29 03:48:05 UTC
Created attachment 41744 [details]
for cjk improvement
Comment 19 caolanm 2006-12-29 10:32:35 UTC
I'd consider that last attachment a separate issue to the main task of using
fontconfig for font and glyph replacement, So I suggest it would perhaps be best
to split it off as a separate issue which depends on this one.

Though personally I'd be a little dubious of a "special-handling for CJK" patch,
listing just CJK fonts in the cjk drop down font lists seems fairly reasonable,
but it does lead to the obvious question if the same sort of thing should then
be done for CTL fonts and western to avoid listing fonts in e.g. the "western"
dropdown if it don't support characters in an equivalent "western" range

And not applying CJK attributes to CJK text because there isn't a proper CJK
font in use for them seems dangerous (and again inconsistent as there isn't
similar CTL/western changes), i.e. Assuming I'm not missing something, which is
quite possible :-), but if I load a doc which has bold CJK text and the user has
for some crazy reason already selected e.g. a non CJK font Timmons for that text
then the text won't be bold due to the 2nd part of the patch ?
Comment 20 pflin 2006-12-30 09:04:25 UTC
-->cmc, Thanks for your comments.
create a new issue for this. #73003

1) I am not familiar with CTL langs. and I think almost CJK fonts support
western langs, at least english. 

2) The patch doesn't has the problem you mentioned. The patch just checks font
name, not check other font attributes.
Comment 21 hdu@apache.org 2007-05-15 14:49:16 UTC
I hope to get it all into OOo3.
Comment 22 pflin 2007-08-06 03:20:40 UTC
pflin --> cmc. I'd apply the patch. But I stumbled over this following minor
part that may cause the font replacement table (tools --> options --> fonts)
doesn't work if the replaced font is missing.

 ImplFontEntry* ImplFontCache::Get( ImplDevFontList* pFontList,
@@ -2693,8 +2814,12 @@ ImplFontEntry* ImplFontCache::Get( ImplD
 
     if( !pEntry ) // no direct cache hit
     {
-        // find the best matching logical font family and update font selector
accordingly
-        pFontFamily = pFontList->ImplFindByFont( aFontSelData, mbPrinter,
pDevSpecific );
+        pFontFamily = pFontList->ImplGetFontconfigSubstitute( aFontSelData,
pDevSpecific );
+        if (!pFontFamily)
+        {
+            // find the best matching logical font family and update font
selector accordingly
+            pFontFamily = pFontList->ImplFindByFont( aFontSelData, mbPrinter,
pDevSpecific );
+        }
Comment 23 caolanm 2007-08-21 14:02:06 UTC
Created attachment 47680 [details]
resynced current patch to OOG680
Comment 24 hdu@apache.org 2007-09-12 18:53:30 UTC
Since the patch was so invasive that it
- removed code in the independent layer for generic font follback and glyph fallback
   => removed this functionality for all other platforms
- introduced very platform specific code into the independent layer
  => broke the compile on all platforms except the ones with fontconfig >2.2.x
- causes massive regressions for non-baseplane unicodes and EUDC users
- introduced new behaviour that causes regressions for monospaced fonts (if their notdef glyph is ok)
it had to be reworked. The result got into CWS gfbfcfg.
Comment 25 hdu@apache.org 2007-09-18 11:05:21 UTC
Testing hints for CWS gfbfcfg (fontconfig based font- and glyphfallback):

- watch out for font- and glyph-fallback regressions on
   - platforms that should not be affected by the changes: Windows, Solaris8
   - platforms that now behave according to FC's font- and glyph-fallback: Solaris10, Linux (if not too 
old)
- check that application specific fonts which are not installed system-wide are still used properly
- watch out for unicode surrogate regressions (see issue 40391 or issue 45983 for background info)

Since the changes introduced by the changes are often very subtle I suggest to emphasize the 
differences by modifying FC's configuration to show more effect, e.g. by aliasing a font "XXXTest" to 
"NSimSun". See the extensive documentation for users at http://www.fontconfig.org/fontconfig-
user.html Especially the "<alias>" element is important for testing this CWS.

The regression for the layout of strings in monospaced fonts which fall back to the NotDef glyphs is 
expected and desired by the patch, so it should be considered a feature.

The regression for the EUDC font (issue 33947) in case fontconfig is active is expected too, since 
normally FC is not configured to prefer EUDC fonts for glyph fallback. Respecting the results of the FC 
query should be considered a feature.
Comment 26 hdu@apache.org 2007-09-18 11:07:14 UTC
Adjusting target
Comment 27 Martin Hollmichel 2007-10-08 13:48:27 UTC
what the status of verfication right now ?
Comment 28 eric.savary 2007-10-08 13:51:03 UTC
In progress...
Comment 29 hdu@apache.org 2007-10-23 14:57:20 UTC
Adjusting target milestone to CWS milestone.
Comment 30 eric.savary 2007-10-24 16:46:13 UTC
Verified in CWS gfbfcfg
Comment 31 hdu@apache.org 2007-12-04 13:50:08 UTC
Adjusting target to CWS.
Comment 32 stefan.baltzer 2008-03-10 16:48:36 UTC
*** Issue 66321 has been marked as a duplicate of this issue. ***
Comment 33 hdu@apache.org 2008-09-19 12:09:06 UTC
Was in the MWS for a while, closing the resolved and verified issue.