RESOLVED FIXED 47452
[Gtk] style="font-family: courier" makes text disappear
https://bugs.webkit.org/show_bug.cgi?id=47452
Summary [Gtk] style="font-family: courier" makes text disappear
Holger Freyther
Reported 2010-10-09 06:12:46 PDT
It appears that there is an error in finding a fallback font. If one is using the above CSS rule, no text is being displayed. Most likely we are not finding the right fallback font, or are not using it.
Attachments
Illustrate the issue. (143 bytes, text/html)
2010-10-09 06:13 PDT, Holger Freyther
no flags
Attach config showing the issue. (3.85 KB, text/plain)
2010-10-10 13:07 PDT, Holger Freyther
no flags
App to illustrate the issue (6.02 KB, text/plain)
2010-10-23 08:16 PDT, Holger Freyther
no flags
Attempt to simplify the issue (6.42 KB, text/plain)
2010-10-23 11:12 PDT, Holger Freyther
no flags
Patch for this issue (23.93 KB, patch)
2010-12-09 16:53 PST, Martin Robinson
xan.lopez: review+
Holger Freyther
Comment 1 2010-10-09 06:13:45 PDT
Created attachment 70351 [details] Illustrate the issue.
Holger Freyther
Comment 2 2010-10-09 06:24:41 PDT
IIRC WebCore maps 'courier' to 'courier new', and if I use a bogus font name it is displaying just fine. So it might be a problem with Red Hat's fontconfig configuaration.
Holger Freyther
Comment 3 2010-10-09 06:27:43 PDT
Maybe also of interest is $fc-match 'Courier' coue1255.fon: "Courier" "Regular"
Martin Robinson
Comment 4 2010-10-09 08:15:03 PDT
Do you mind running with fc-match -v? Perhaps there is some property of the font which is causing the render to fail (e.g. not being scalable).
Holger Freyther
Comment 5 2010-10-09 08:37:58 PDT
Do you want to see the result with an -a as well? I can attach it to the bugreport. $ fc-match -v 'courier' Pattern has 27 elts (size 32) family: "Courier"(s) style: "Regular"(s) slant: 0(i)(s) weight: 80(i)(s) width: 100(i)(s) size: 12(f)(s) pixelsize: 13(f)(s) spacing: 100(i)(s) foundry: "unknown"(s) antialias: FcFalse(s) hintstyle: 3(i)(s) hinting: FcTrue(s) verticallayout: FcFalse(s) autohint: FcFalse(s) globaladvance: FcTrue(s) file: "/usr/share/fonts/wine-courier-fonts/coue1255.fon"(s) index: 0(i)(s) outline: FcFalse(s) scalable: FcFalse(s) dpi: 75(f)(s) scale: 1(f)(s) charset: (s) lang: (s) fontversion: 0(i)(s) fontformat: "Windows FNT"(s) embeddedbitmap: FcTrue(s) decorative: FcFalse(s)
Martin Robinson
Comment 6 2010-10-09 08:41:38 PDT
(In reply to comment #5) > scalable: FcFalse(s) This appears to be the issue. We need to be more stringent in the types of fonts that accept from FontConfig, I think.
Holger Freyther
Comment 7 2010-10-09 08:54:20 PDT
I was moving the font away, just to have a .fon font that is not displayed either. I agree that we need to check if our display code can load the font. In theory freetype should be able to load the Windows Font. At least it is listed here: http://freetype.sourceforge.net/freetype2/index.html.
Martin Robinson
Comment 8 2010-10-09 21:38:24 PDT
I think you might be right about this. Do you mind attaching the font file and any pertitent configuration info in /etc/fonts to the bug? I'll try to have this fixed soon.
Holger Freyther
Comment 9 2010-10-10 12:49:04 PDT
It is less a issue with the font but more one with the config. In FontCacheFreeType.cpp we run into if (strcasecmp(reinterpret_cast<char*>(familyNameAfterConfiguration), reinterpret_cast<char*>(familyNameAfterMatching)) && !isFallbackFontAllowed(familyNameString)) return 0; this happens as we go from 'courier new' to 'Liberation Mono'. I am going to attach my /etc/fonts in a second, I am also trying to identify the rule responsible for that. I assume it is the following, but I will verify this. <alias> <family>Bitstream Vera Sans Mono</family> <family>DejaVu Sans Mono</family> <family>Liberation Mono</family> <family>Inconsolata</family> <family>Courier New</family> <family>Courier</family> <family>Andale Mono</family> <family>Luxi Mono</family> <family>Cumberland AMT</family> <family>Cumberland</family> <family>Nimbus Mono L</family> <default><family>monospace</family></default> </alias>
Holger Freyther
Comment 10 2010-10-10 13:07:30 PDT
Created attachment 70411 [details] Attach config showing the issue. It appears to be cause by a rule such as: <alias binding="same"> <family>Courier New</family> <accept> <family>Liberation Mono</family> <family>Cumberland</family> <family>Cumberland AMT</family> </accept> </alias>
Holger Freyther
Comment 11 2010-10-10 13:16:13 PDT
Okay this got introduced by bug #36548 which landed in r68406. The question is, why don't we fallback to anything else? How can we know that there is nothing else? I read the comment about CSS font matching rule.. but according to my fontconfig config, 'Courier New' is the same as 'Monospace'. Any idea how to resolve it?
Holger Freyther
Comment 12 2010-10-10 14:27:58 PDT
I have poked it a bit more... but I still have no conclusion. 1.) it tries 'Courier New' finds Liberation Mono... rejects it... 2.) FontCache.cpp takes the alternate path which will try 'Courier' and this works. a FontPlatformData is created with the FcPattern. PlatformRefPtr<cairo_font_face_t> fontFace = adoptPlatformRef(cairo_ft_font_face_create_for_pattern(m_pattern.get())); I have printed the cairo_font_face_status of fontFace and it is 0 (success).. I have also dumped the FcPattern.. Pattern has 24 elts (size 32) family: "Courier"(s) style: "Regular"(s) slant: 0(i)(s) weight: 80(i)(s) width: 100(i)(s) pixelsize: 13(f)(s) spacing: 100(i)(s) foundry: "unknown"(s) antialias: FcFalse(s) hintstyle: 3(i)(s) hinting: FcTrue(s) verticallayout: FcFalse(s) autohint: FcFalse(s) globaladvance: FcTrue(s) file: "/usr/share/fonts/wine-courier-fonts/coue1255.fon"(s) index: 0(i)(s) outline: FcFalse(s) scalable: FcFalse(s) charset: (s) lang: (s) fontversion: 0(i)(s) fontformat: "Windows FNT"(s) embeddedbitmap: FcTrue(s) decorative: FcFalse(s) so I wonder why something in webkit is rejecting and not attempting to draw the text.
Martin Robinson
Comment 13 2010-10-11 15:00:30 PDT
Maybe the issue here is the call to cairo_scaled_font_create. Perhaps for unscalable fonts, we should just use the original cairo_font_face_t as the target font.
Holger Freyther
Comment 14 2010-10-12 08:00:09 PDT
(In reply to comment #13) > Maybe the issue here is the call to cairo_scaled_font_create. Perhaps for unscalable fonts, we should just use the original cairo_font_face_t as the target font. it does not return an error... status is 0. I will try to build a debug version during my train trip and then single step through the code... compiling/linking/make dependencies just take too long here.
Holger Freyther
Comment 15 2010-10-23 06:12:21 PDT
I am working on a fontconfig/cairo code reduction now to isolate the issue. A debug build prints a nice runtime error.. somehow we do not get any glyphs from the font.
Holger Freyther
Comment 16 2010-10-23 08:16:45 PDT
Created attachment 71641 [details] App to illustrate the issue The code is copy and pasted from the WebCore sources, it will call abort() on all error paths of the WebCore code. In case it aborts in the GlyphPage::fill function, it will also print the Pattern. The code aborts with 'courier' but it does not abort with 'liberation sans'. The code lacks error checking on the created cairo fonts but this turned out to be okay. I assume there is an error with characterset conversion of the font... but this will be the next step...
Holger Freyther
Comment 17 2010-10-23 11:12:48 PDT
Created attachment 71649 [details] Attempt to simplify the issue This will load the coue1255.fon (md5sum 3ae6da5157fe37d0aba30b817277662b) directly via FreeType2 and then uses FcFreeTypeCharIndex on the font face. We now need to look into the implementation to figure out the real meaning of a zero return of this function (the man page does not say).
Holger Freyther
Comment 18 2010-10-23 11:27:40 PDT
(In reply to comment #17) > Created an attachment (id=71649) [details] > Attempt to simplify the issue > > This will load the coue1255.fon (md5sum 3ae6da5157fe37d0aba30b817277662b) directly via FreeType2 and then uses FcFreeTypeCharIndex on the font face. We now need to look into the implementation to figure out the real meaning of a zero return of this function (the man page does not say). I can use the freetype-demos (ftdump, ftview) on the font, calls to FT_Get_Char_Index also return sensible values... so we will need to dig into FontConfig to figure out what is going wrong... The FT_Face also has a charmap set...
Holger Freyther
Comment 19 2010-11-09 07:14:13 PST
(In reply to comment #18) > I can use the freetype-demos (ftdump, ftview) on the font, calls to FT_Get_Char_Index also return sensible values... so we will need to dig into FontConfig to figure out what is going wrong... The FT_Face also has a charmap set... My email to the fontconfig list got stuck but behdad was kind enough to respond to private emails. Right now it is not possible to query for certain encodings of the font. Behdad proposed that we will have to have proper font fallback then. So two ways to go forward: a) Before loading the font, get the FT_Face and check if it is supported by FontConfig. We could try to get this method into fontconfig itself b) Handle that we do not have any 'accessible' glyphs and have a proper fallback, or have a fallback code to just try a different font. For b) we will need to change WebCore internals, for a) we have to face the penalty on selecting the font.
Martin Robinson
Comment 20 2010-11-10 09:49:14 PST
(In reply to comment #19) > a) Before loading the font, get the FT_Face and check if it is supported by FontConfig. We could try to get this method into fontconfig itself If I understand correctly, Fontconfig should already deliver a list of fallback fonts. We could examine if that gives reasonable alternatives. I don't think the performance hit would be too bad there.
Holger Freyther
Comment 21 2010-11-10 10:10:10 PST
(In reply to comment #20) > (In reply to comment #19) > > > a) Before loading the font, get the FT_Face and check if it is supported by FontConfig. We could try to get this method into fontconfig itself > > If I understand correctly, Fontconfig should already deliver a list of fallback fonts. We could examine if that gives reasonable alternatives. I don't think the performance hit would be too bad there. True, and we don't have perf checks in place and wouldn't notice. I will try to find some time over the weekend, but if you have a pressing need then please don't stop yourself.
Martin Robinson
Comment 22 2010-11-10 10:15:39 PST
(In reply to comment #21) > True, and we don't have perf checks in place and wouldn't notice. I will try to find some time over the weekend, but if you have a pressing need then please don't stop yourself. I entirely agree that the lack of a performance testing for WebKitGTK+ is seriously bad / uncomfortable.
Martin Robinson
Comment 23 2010-12-09 16:53:32 PST
Created attachment 76136 [details] Patch for this issue
Martin Robinson
Comment 24 2010-12-09 16:59:29 PST
After Xan had this issue too and I thought about it a bit, I decided to write a patch to simply never select fonts that do not have one of the three valid Fontconfig encodings. On Ubuntu these fonts never seem to be selected, because they are bitmap fonts and Ubuntu doesn't select bitmap fonts by default.
Holger Freyther
Comment 25 2010-12-09 17:02:40 PST
Looks very reasonable. Thanks.
Xan Lopez
Comment 26 2010-12-09 17:12:25 PST
Comment on attachment 76136 [details] Patch for this issue Looks great to me.
Holger Freyther
Comment 27 2010-12-09 17:21:45 PST
Comment on attachment 76136 [details] Patch for this issue View in context: https://bugs.webkit.org/attachment.cgi?id=76136&action=review > WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:198 > + if (!platformData->hasCompatibleCharmap()) { I wonder if we could do this with a static method but I assume it will be difficult as we would have to go from FcPattern to FT_Face without cairo's help... or replicate a lot what the FontPlatformData is doing.
Martin Robinson
Comment 28 2010-12-09 23:57:41 PST
(In reply to comment #27) > I wonder if we could do this with a static method but I assume it will be difficult as we would have to go from FcPattern to FT_Face without cairo's help... or replicate a lot what the FontPlatformData is doing. Here it might be useful to replicate Cairo's logic for loading the FT_Face and then passing that to Cairo. Having the FT_Face before the cairo_scaled_font_t is also useful for vertical text rendering, I think.
Martin Robinson
Comment 29 2010-12-10 01:54:39 PST
Note You need to log in before you can comment on or make changes to this bug.