Bug 16792

Summary: [GTK] Fails to render Japanese/Chinese text with simple path
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aike47, alp, fonts-bugs, martin.sourada, mh+webkit, pclouds, peter
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Font selection and rendering fixes
hyatt: review+
Gecko vs. WebKit japanese text rendering none

Description Xan Lopez 2008-01-08 19:56:24 PST
One example: http://job.japantimes.com/index_e.php

Not sure if it should use complex path or if there's a bug in the simple path.
Comment 1 Alp Toker 2008-02-07 23:57:54 PST
*** Bug 16937 has been marked as a duplicate of this bug. ***
Comment 2 Alp Toker 2008-02-08 00:02:44 PST
Bin Chen tracked down the issue. The problem is font fallback. A very simple workaround (substitute "Arial Unicode MS" with any full-Unicode font you have on your system):


Index: WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp
===================================================================
--- WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp	(revision 30082)
+++ WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp	(working copy)
@@ -81,6 +81,8 @@ FontPlatformData::FontPlatformData(const
 
     if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const FcChar8*>(fcfamily)))
         goto freePattern;
+    if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const FcChar8*>("Arial Unicode MS")))
+        goto freePattern;
     if (!FcPatternAddInteger(pattern, FC_SLANT, fcslant))
         goto freePattern;
     if (!FcPatternAddInteger(pattern, FC_WEIGHT, fcweight))




We haven't figured out what the correct fix for this is yet. (Perhaps the Pango font selection patch would have helped here, but it has not been re-submitted.)

Comment 3 Martin Sourada 2008-06-02 12:22:19 PDT
Hm... perhaps the problem is with webkit looking for sans font while the corespoding japaneses/chinese fonts have mincho style? Wouldn't it be better to leave all the font selection work to pango? IFAIK it is designed for that and it should work properly. As for the patch you are talking about, may I ask where can I find it? I am not much familiar either with C++ (although I am familiar with plain C) or with WebKit, but I might give it look to see if I can come up with something.

Also I noticed that bug 18546 seems to be duplicate of this one.
Comment 4 Martin Sourada 2008-06-02 12:24:10 PDT
(In reply to comment #3)
> corespoding japaneses/chinese fonts have mincho style? Wouldn't it be better
err... I actually meant gothic style, mincho style is coresponding to latin serif family...
Comment 5 Joshua Chia 2008-06-17 17:15:05 PDT
I'm not familiar with the font-related code, but my initial impression of FontFallbackList based on its name and interface suggests that it's a list of FontDatas from which you can pick one that has the glyphs for the string you want to display, by calling FontFallbackList::fontDataForCharacters().  However, it seems that the only time anything is added to the list is when Font::Font() calls FontFallbackList::setPlatformFont().  If FontFallbackList is really a fallback list, then, shouldn't more fonts be added to it at other places?
Comment 6 Joshua Chia 2008-06-18 17:50:15 PDT
I think this workaround doesn't actually work, or works with a major flaw--it just causes the font matching to always match "Arial Unicode MS", even when I specify <font face="Serif">.

(In reply to comment #2)
> Bin Chen tracked down the issue. The problem is font fallback. A very simple
> workaround (substitute "Arial Unicode MS" with any full-Unicode font you have
> on your system):
> 
> 
> Index: WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp
> ===================================================================
> --- WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp       (revision
> 30082)
> +++ WebCore/platform/graphics/gtk/FontPlatformDataGtk.cpp       (working copy)
> @@ -81,6 +81,8 @@ FontPlatformData::FontPlatformData(const
> 
>      if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const
> FcChar8*>(fcfamily)))
>          goto freePattern;
> +    if (!FcPatternAddString(pattern, FC_FAMILY, reinterpret_cast<const
> FcChar8*>("Arial Unicode MS")))
> +        goto freePattern;
>      if (!FcPatternAddInteger(pattern, FC_SLANT, fcslant))
>          goto freePattern;
>      if (!FcPatternAddInteger(pattern, FC_WEIGHT, fcweight))
> 
> 
> 
> 
> We haven't figured out what the correct fix for this is yet. (Perhaps the Pango
> font selection patch would have helped here, but it has not been re-submitted.)
> 

Comment 7 Alp Toker 2008-08-17 08:02:23 PDT
When the environment variable LC_ALL="zh" is set, Chinese and Japanese text works, at least with the FreeType backend. This might help some users in the meantime.
Comment 8 Martin Sourada 2008-08-17 08:42:56 PDT
I can confirm, that what Alp suggested fixes the issue for Japanese/Chinese (in my case I run LC_ALL="zh" midori), but it instead breaks some other skripts, e.g. some latin extensions in Slavic languages (on wikipedia I noticed missing š and &#269;). Also, during writing this text I noticed that in the input boxes both š and &#269; can be writen and is displayed correctly.
Comment 9 Martin Sourada 2008-08-25 02:49:38 PDT
Just a food for thought, but would it be possible to separate CSS font selection from glyphs font selection? I mean, we should select a font to be used for the current rendered element as specified in CSS, e.g. if there's 

font-family = arial, "Liberation Sans", sans-serif

then try arial, if not present try Liberation Sans and if still not present then use sans-serif family, system default, and let pango/freetype substitution functions handle the rest? I.e. the font selection for HTML elements would be done traditional way (as it is done now) and font selection for the glyphs themselves would be left for pango/freetype to solve.

Does that sound sane? It seems to me like a good way to have both the CSS font selection working and this bug fixed...
Comment 10 Alp Toker 2008-09-03 19:53:07 PDT
Created attachment 23155 [details]
Font selection and rendering fixes

        GTK+ font fixes and enhancements.

        Implement font fallback for the simple FontConfig-based text path and
        improve the Pango-based complex text path to make use of requested
        font properties and available font selection.

        Add text shadow support to the complex path.
Comment 11 Alp Toker 2008-09-03 20:01:55 PDT
(In reply to comment #10)
> Created an attachment (id=23155) [edit]
> Font selection and rendering fixes
> 
>         GTK+ font fixes and enhancements.
> 
>         Implement font fallback for the simple FontConfig-based text path and
>         improve the Pango-based complex text path to make use of requested
>         font properties and available font selection.
> 
>         Add text shadow support to the complex path.
> 

Some notes on this patch:

1) These changes push Pango's high level (layout) API as far as it can go. If we want to go further we'll probably want to use the low-level Pango API directly as Mozilla does, but at least the complex path is now feature-complete.

2) FontPlatformDataGtk's comparison operator and copy constructor need to be fixed some time to avoid leaking FontPlatformDatas. The leaks are pretty minor (per-font) and the fix was too involved to be ready for this patch.

3) Our FontConfig-based fallback font selection doesn't always match what Pango would select since we don't have that kind of advanced coverage analysis, but with these changes the simple path does successfully select fonts with the necessary glyphs available for the first time.

I hope this improves the experience for users of non-Latin scripts and would be interested to get some feedback. You need to build with the "freetype" backend to see most of the improvements since they haven't been implemented in the "pango" backend yet.
Comment 12 Duy Nguyen 2008-09-04 08:48:06 PDT
Still does not work for me. Pango backend, test site: http://identi.ca/kosugi, expected to see japanese glyphs
Comment 13 Alp Toker 2008-09-04 10:07:53 PDT
(In reply to comment #12)
> Still does not work for me. Pango backend, test site: http://identi.ca/kosugi,
> expected to see japanese glyphs
> 

The patch adds fallback for the FreeType backend, not the Pango backend.
Comment 14 Duy Nguyen 2008-09-04 10:11:29 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > Still does not work for me. Pango backend, test site: http://identi.ca/kosugi,
> > expected to see japanese glyphs
> > 
> 
> The patch adds fallback for the FreeType backend, not the Pango backend.
> 

Ah.. sorry I misread Pango part.
Comment 15 Martin Sourada 2008-09-04 12:56:00 PDT
Just tested the patch against r35913 with freetype backend on Fedora 9 and it works! Only glyphs hinting seems to be turned off for the previously not displayed text. I'll attach a screenshot to show the difference between gecko and webkit. I've tested on japanese wikipedia.

Just a question, do you plan to fix this bug for the pango backend as well or should I file a seperate bug for that?
Comment 16 Martin Sourada 2008-09-04 13:07:52 PDT
Created attachment 23175 [details]
Gecko vs. WebKit japanese text rendering

Here's the promissed attachment. On a closer look it seems like gecko and webkit selects different fallback fonts for Japanese, I think that might be the cause.
Comment 17 Mike Hommey 2008-09-04 22:14:08 PDT
(In reply to comment #16)
> Created an attachment (id=23175) [edit]
> Gecko vs. WebKit japanese text rendering
> 
> Here's the promissed attachment. On a closer look it seems like gecko and
> webkit selects different fallback fonts for Japanese, I think that might be the
> cause.

I have had this kind of hinting being off problem for some fonts for a while with fontconfig ; this is not webkit related.
Comment 18 Dave Hyatt 2008-09-09 18:15:19 PDT
Comment on attachment 23155 [details]
Font selection and rendering fixes

r=me
Comment 19 Alp Toker 2008-09-09 21:34:29 PDT
Landed in r36309.
Comment 20 Martin Sourada 2008-09-11 13:22:26 PDT
(In reply to comment #15)
> Just a question, do you plan to fix this bug for the pango backend as well or
> should I file a seperate bug for that?
> 

Please ignore this comment, I just noticed I am already CC'ed on duplicate of this bug for pango backed: bug #18546

Anyway, thanks for fixing this at least for the FreeType backend :-)