Bug 77437 - freetype port: font fallback mechanism can try harder
Summary: freetype port: font fallback mechanism can try harder
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-31 07:28 PST by Martin Jackson
Modified: 2015-07-19 09:53 PDT (History)
8 users (show)

See Also:


Attachments
Allow fontconfig to choose an appropriate font if all else fails (4.30 KB, patch)
2012-02-07 06:25 PST, Martin Jackson
no flags Details | Formatted Diff | Diff
Allow fontconfig to choose an appropriate font if all else fails (4.27 KB, patch)
2012-02-08 05:42 PST, Martin Jackson
no flags Details | Formatted Diff | Diff
Allow fontconfig to choose an appropriate font if all else fails (5.79 KB, patch)
2012-02-09 02:17 PST, Martin Jackson
mrobinson: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Jackson 2012-01-31 07:28:40 PST
In the CSS rules for font matching (http://www.w3.org/TR/css3-fonts/#font-matching-algorithm), if all else fails, the system is allowed to choose an appropriate font ("system font fallback" procedure). In the case of free type, we can use fontconfig matching to find the closest font to what was asked for if none of the developer's choices could be used.

Currently, getSimilarFontPlatformData is an empty stub, but here fontconfig could be consulted for the best match for the requested font.
Comment 1 Martin Jackson 2012-02-07 06:25:20 PST
Created attachment 125835 [details]
Allow fontconfig to choose an appropriate font if all else fails

This patch allows the system to attempt to match the requested font if nothing
appropriate was found in the CSS font family list, using fontconfig. This will
only happen if none of the fonts specified in the CSS list could be used. This
is in accordance with the w3c rules:

http://www.w3.org/TR/css3-fonts/#font-matching-algorithm

"... If there are no more font families to be evaluated and no matching face has
been found, then the user agent performs a system font fallback procedure to
find the best match for the character to be rendered. The result of this
procedure may vary across user agents."
Comment 2 WebKit Review Bot 2012-02-07 13:57:07 PST
Attachment 125835 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Martin Jackson 2012-02-08 05:42:50 PST
Created attachment 126065 [details]
Allow fontconfig to choose an appropriate font if all else fails
Comment 4 Martin Jackson 2012-02-09 02:17:15 PST
Created attachment 126260 [details]
Allow fontconfig to choose an appropriate font if all else fails
Comment 5 Martin Robinson 2012-03-05 16:44:50 PST
I appreciate this patch, but I don't quite understand it. How does FontCache::getSimilarFontPlatformData interact with the changes you made in the platform-independent code. 

Is this change testable. For GTK+ we have a very particular fontconfig environment that we've set up for testing changes like this. Is there a page that this change affects?
Comment 6 Martin Jackson 2012-03-06 03:17:35 PST
(In reply to comment #5)

Hi Martin,

Thanks for getting back to me on this.

> I appreciate this patch, but I don't quite understand it. How does FontCache::getSimilarFontPlatformData interact with the changes you made in the platform-independent code. 

The patch was created in response to a bug that we had internally: Arial was requested by a web page, but since we didn't have this font in$talled, we ended up getting a serif font which was dissimilar to Arial (while we had perfectly good open alternatives installed, of which fontconfig was aware).

If none of the font families in the font fallback list, as defined in the web page, could be found, getSimilarFontPlatformData is called. Previously, webkit just give up at this point and fell through to getLastResortFallbackFont, which just substitutes 'serif' (in the freetype port at least)

I added an extra flag, m_exactFamily, to the FontDescription which indicates whether the system is allowed to substitute what it considers to be 'similar' font families for what was requested, or whether *only* what was requested will be accepted.

In general, m_exactFamily == true (i.e. we want the *exact* font family only). Then the CSS font family fallback list can be iterated (http://www.w3.org/TR/css3-fonts/#font-matching-algorithm) correctly. In getSimilarFontPlatformData, the system has failed to find any of the fonts that were requested, and so relaxes the exact matching condition (i.e. m_exactFamily -> 0) and then tries to use fontconfig to locate an appropriate replacement font for the first choice of font family as specified in the web page.

> 
> Is this change testable. For GTK+ we have a very particular fontconfig environment that we've set up for testing changes like this. Is there a page that this change affects?

The change is of course testable - however because it is highly installation dependent, I didn't really know what to do. I'm not familiar with how you go about this in the GTK+ port - if you can give me some pointers on how to reproduce your test setup, I'll do my best to provide a sensible test case.

Regards

Martin Jackson
Comment 7 Martin Jackson 2012-03-06 03:53:22 PST
I realise that I did not update the comment in FontCacheFreeType.cpp:211 ("If Fontconfig gave use a different font family ...") - I'll address this in an update to the patch once further feedback has been provided.
Comment 8 Martin Robinson 2012-03-06 07:13:24 PST
Comment on attachment 126260 [details]
Allow fontconfig to choose an appropriate font if all else fails

View in context: https://bugs.webkit.org/attachment.cgi?id=126260&action=review

Looks good, but I think a slightly different design may be in order here.

> Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:115
> +    const FontFamily* family = &fontDescription.family();
> +    if (family->family().length())
> +        result = getCachedFontData(fontDescription, family->family(), false, Retain);
> +

Instead of adding the idea of an in-exact match to the platform-independent code, I think it's better to first ask Fontconfig what the proper fallback font is and then get that font explicitly from the font cache.
Comment 9 Martin Robinson 2013-02-08 10:19:11 PST
(In reply to comment #8)

> Instead of adding the idea of an in-exact match to the platform-independent code, I think it's better to first ask Fontconfig what the proper fallback font is and then get that font explicitly from the font cache.

I'm also a bit surprised that the page didn't have a font selection order like: Arial, sans-serif.
Comment 10 Jeff Fortin 2015-07-19 09:12:37 PDT
Hi there, it is a bit unclear to me what the status of this bug/patch is.
I came here from a fairly interesting discussion on https://bugzilla.gnome.org/show_bug.cgi?id=752533

The problem is if you have this:  font-family: Arial,Helvetica,sans-serif;

Epiphany+WebKit goes through that list, and as Arial and Helvetica are not found on the system, when it hits "sans-serif" it then asks for GNOME's default "sans" font, which happens to be DejaVu Sans Book; the problem with that is that on Linux systems, there are often much better matches available, such as Liberation Sans (metrically equivalent to Arial, thus preserving the page's intended layout). So what I'm suggesting is that when you hit the generic "sans-serif" CSS rule, you ask fc-match for the font corresponding to the first non-generic font of the list (so, in a way, a "second pass"). Is this what this bug report & patch is about?
Comment 11 Martin Robinson 2015-07-19 09:25:44 PDT
(In reply to comment #10)
> Hi there, it is a bit unclear to me what the status of this bug/patch is.
> I came here from a fairly interesting discussion on
> https://bugzilla.gnome.org/show_bug.cgi?id=752533
> 
> The problem is if you have this:  font-family: Arial,Helvetica,sans-serif;
> 
> Epiphany+WebKit goes through that list, and as Arial and Helvetica are not
> found on the system, when it hits "sans-serif" it then asks for GNOME's
> default "sans" font, which happens to be DejaVu Sans Book; the problem with
> that is that on Linux systems, there are often much better matches
> available, such as Liberation Sans (metrically equivalent to Arial, thus
> preserving the page's intended layout). So what I'm suggesting is that when
> you hit the generic "sans-serif" CSS rule, you ask fc-match for the font
> corresponding to the first non-generic font of the list (so, in a way, a
> "second pass"). Is this what this bug report & patch is about? 

I believe it doesn't ask for GNOME's default first, but WebKit's default. This can be configured using the API for GTK+ http://webkitgtk.org/reference/webkit2gtk/stable/WebKitSettings.html#webkit-settings-get-sans-serif-font-family and there is also a setting in Epiphany. I think that if WebKit isn't using the font from the API settings than that is a bug.
Comment 12 Michael Catanzaro 2015-07-19 09:44:23 PDT
Epiphany sets the WebKit default to GNOME's default... by default.

Anyway, this bug only covers the case where no generic fallback (sans-serif) is in the font-family list. The case of font-family: Arial,Helvetica,sans-serif; is bug #147057, where I am pretty sure we have agreed on a solution (though the discussion occurred before I filed that bug).

In the case where sans-serif is not in the list (this bug)... after processing the entire font-family list and finding no match, there are two possibilities for what we should do: (a) allow fontconfig to match on the first font, which might return a font that's better than the browser's default -- that's what the patch in this bug implements -- or (b) use the default font always.

I've been going back and forth on whether (a) is a good idea. Yesterday I thought "if there is no generic fallback listed, we might as well perform unrestricted fontconfig matching to try to find the best font." Today I think it's better to fix bug #147057 and leave it at that: never perform fontconfig matching except for fonts that are extremely similar to each other (strongly-aliased fonts), and use our default font otherwise. That would be sufficient to fix both of these particular issues, since Arial would get matched to Liberation Sans. So I am inclined to say WONTFIX for this bug, but I will leave that choice to Martin Robinson (although maybe Martin Jackson has an opinion on this too?).

Actually, there is one more thing we should change here: the lastResortFallbackFont should be sans. It seems we use serif currently, but that's silly, it should be sans. Bug #147088.

BTW the patch status was that the general idea was approved, but not the implementation (r- means "review denied"). Also, I think there needs to be layout tests for this, since they would be easy to write. We have a jhbuild environment for running tests to make sure that our font settings are always the same.
Comment 13 Michael Catanzaro 2015-07-19 09:47:30 PDT
(In reply to comment #11)
> I think that if WebKit isn't using the font from the API settings than that
> is a bug.

That's not happening; the problem is that the default font is being used when ideally it wouldn't be.