Bug 83279 – Line spacing in BlackBerry is too large when using VDMX metrics

• Component: Text
• Status: RESOLVED INVALID

[Eli Fidler]

Chromium Linux and BlackBerry both calculate font metrics in WebCore/platform/graphics/skia/SimpleFontDataSkia.cpp. When the font has a VDMX table, the yMin and yMax are taken as the descent and ascent, but the lineGap from the HHEA table is still used as is. This results in a larger line gap than basically any other browser in many cases. For the Microsoft core web fonts, it happens to work out the same(I tried Arial, Verdana, Trebuchet, and Times New Roman), but for other fonts this is often several pixels larger.

Microsoft has published recommended guidelines (http://www.microsoft.com/typography/otspec/recom.htm (see "Baseline to Baseline Distances:Windows")) that suggest MAX(0, LineGap - ((usWinAscent + usWinDescent) - (Ascender - Descender))) for the line gap. This works out much better.

There is an accompanying change to Skia needed to expose the WinAscent and WinDescent metrics.

[Eli Fidler]

Created attachment 135827 [details]
Skia patch to expose windows metrics

[Adam Langley]

Comment on attachment 135827 [details]
Skia patch to expose windows metrics

Skia people who have to review, but LGTM.

[Eli Fidler]

Created attachment 135829 [details]
Patch

[WebKit Review Bot]

Attachment 135829 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/graphics/skia/SimpleFontDataSkia.cpp:95:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Langley]

Comment on attachment 135829 [details]
Patch

If this comes with no rebaselines, then LGTM.

[WebKit Review Bot]

Comment on attachment 135829 [details]
Patch

Attachment 135829 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12336369

[Tony Chang]

Comment on attachment 135829 [details]
Patch

The change seems fine to me. Please fix the style error.  We have to wait for a change in Skia first, right?

[Eli Fidler]

Created attachment 135857 [details]
Fixed style

[WebKit Review Bot]

Comment on attachment 135857 [details]
Fixed style

Attachment 135857 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12335459

[Eli Fidler]

*** Bug 67998 has been marked as a duplicate of this bug. ***

[Tony Chang]

Sorry, I was on vacation.

Did the skia changes ever get landed?

[Eli Fidler]

No, the Skia team doesn't want to explicitly add the winAscent/Descent metrics to the FontMetrics object. Instead, they want to expose the HEAD, HHEA, and OS/2 OpenType tables and let the client (WebKit) pull the data. See https://codereview.appspot.com/5985052/

I haven't had time to port all that code over to WebKit.

[Tony Chang]

Comment on attachment 135857 [details]
Fixed style

Ok, removing the patch from the review queue since it'll have to be restructured.

I've been investigating this area because of another bug [1]. Since I was one of the ones in Skia who didn't like the Skia changes in this patch, I wanted to comment here now that I understood this a bit better.

This started a long time ago, with a change which added VDMX parsing to the Chromium Skia fork in order to get the right ascent/descent for hinted fonts [2]. It was then made worse when the VDMX parsing was moved out to WebKit [3]. This was a move in the wrong direction as, opposed to being moved up the stack, it should have been moved down the stack into FreeType. In fact, I'm currently investigating how to do this, and support for hdmx/VDMX in FreeType has been discussed before [4].

In the other bug [1], the issue is that Chromium is always using the VDMX values if they are present, even in the absence of a request for bytecode hinting. Since VDMX values exist solely as a cache of font-wide black box ascent/descent values for given pel sizes after hinting [5], these are the wrong values to use when not hinting. As a side effect of this Chromium overwrites parts of some low descenders like '_' and 'g' in some cases. The intent was to measure like GDI, however GDI always bytecode hints.

As to the patch proposed above [6] the issue is that it is assumed that metrics.fAscender and fDescender are sTypoAscender and sTypoDescender. However, this is not the case. These are simply the values returned from FreeType. These are actually often hhea.Ascender and hhea.Descender and can, in fact, be any one of the three ascender/descender pairs. There is also a long comment in FreeType about how the actual GDI implementation does not match what the specification says [7]. This is why the suggestion was made at the time that the tables should be used directly, as to do something like this correctly, it is necessary to know which values one is actually working with.

Also recently, FreeType clarified that the font.height is always the baseline to baseline measurement [8] and cleaned up the height measurement to match [9]. As a result, the external leading is always "abs(height) - (abs(ascent) + abs(descent))" when height, ascent, and descent are from FreeType. The only shortfall here is that FreeType needs to also look over in the VDMX tables for ascent/descent if we're going to be hinting, and then find the height using the line gap as described in the bug description. If anyone were doubting that this needs to be in FreeType, note that in addition to this VDMX parsing in WebKit, wine does the same [10], and there are probably others.

Fixing this in FreeType fixes both of these issues for everyone (particularly this bug and [1]). However, even if changes can be made to FreeType they may not be generally available for some time. As a result, the best immediate measure to take is for Skia to once again own the VDMX parsing. When asked for font metrics Skia knows if it's for hinting or not, so it can look in the VDMX table as necessary. It may need to do some 'OS/2' table sniffing as well to properly calculate the correct height. This is effectively what needed to be done in any event and, as mentioned above, is why the suggestion was made to use the tables directly. However, it is a much cleaner solution to have Skia simply report the correct ascender/descender/height values based on the hinting method as well as other settings.

[1] http://crbug.com/41457
[2] http://src.chromium.org/viewvc/chrome?view=revision&revision=5868
[3] http://src.chromium.org/viewvc/chrome?view=revision&revision=13781
[4] http://lists.gnu.org/archive/html/freetype-devel/2011-12/msg00005.html
[5] https://www.microsoft.com/typography/otspec/vdmx.htm
[6] 135827, 135857
[7] http://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/sfnt/sfobjs.c?id=04e547bd2cfaf05498ad98ade10a4ed3a168484e#n993
[8] http://lists.gnu.org/archive/html/freetype-devel/2013-01/msg00002.html
[9] http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=e0469372be3870a5ad60b2c4586e9c281357bd28
[10] http://source.winehq.org/git/wine.git/blob/e1a42bba54734dfcab59e22852405ad1030335e4:/dlls/gdi32/freetype.c#l4246

[Stephen Chenney]

As this is a Skia issue it will only be resolved on the Chromium side without more work from some on Blackberry.