Bug 150340 - [FreeType] Add support for the USE_TYPO_METRICS flag
Summary: [FreeType] Add support for the USE_TYPO_METRICS flag
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on:
Blocks: USE_TYPO_METRICS 150394
  Show dependency treegraph
 
Reported: 2015-10-19 14:06 PDT by Frédéric Wang (:fredw)
Modified: 2015-10-21 09:43 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.09 KB, patch)
2015-10-19 14:35 PDT, Frédéric Wang (:fredw)
mrobinson: review+
Details | Formatted Diff | Diff
Patch final version (8.64 KB, patch)
2015-10-20 14:42 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch final version (8.64 KB, patch)
2015-10-20 23:59 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2015-10-19 14:06:16 PDT
This is bug 131839 for Linux.
Comment 1 Frédéric Wang (:fredw) 2015-10-19 14:35:20 PDT
Created attachment 263512 [details]
Patch
Comment 2 Myles C. Maxfield 2015-10-20 10:51:22 PDT
Comment on attachment 263512 [details]
Patch

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

I can't comment on the FreeType-specific stuff, but this approach looks good to me.

> Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:72
> +    if (TT_OS2* os2 = static_cast<TT_OS2*>(FT_Get_Sfnt_Table(freeTypeFace, ft_sfnt_os2))) {
> +        const FT_Short kUseTypoMetricsMask = 1 << 7;
> +        if (os2->fsSelection & kUseTypoMetricsMask) {

There's really no FreeType API for this? :(
Comment 3 Frédéric Wang (:fredw) 2015-10-20 11:14:49 PDT
> There's really no FreeType API for this? :(

I checked the Cairo and FreeType online documentation and could not find any easier way to retrieve the typo values (the yscale parameter was actually taken from Gecko's gfx/thebes/gfxFT2Utils.cpp and they also hardcode the use_typo_metrics bit). I just downloaded the freetype-2.6 source and searched for "typo" metrics but I did not find anything either...
Comment 4 Martin Robinson 2015-10-20 13:56:29 PDT
Comment on attachment 263512 [details]
Patch

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

Looks good to me, except for a few nits.

> Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:70
> +    if (TT_OS2* os2 = static_cast<TT_OS2*>(FT_Get_Sfnt_Table(freeTypeFace, ft_sfnt_os2))) {

I think WebKit style specifies this should be called OS2Table instead of os2.

> LayoutTests/fonts/use-typo-metrics-1.html:9
> +    src: url("./lineheight5000-typolineheight2300.woff");

Is the "./" necessary here?
Comment 5 Frédéric Wang (:fredw) 2015-10-20 14:42:25 PDT
Created attachment 263619 [details]
Patch final version

Thanks!
Comment 6 Frédéric Wang (:fredw) 2015-10-20 23:59:50 PDT
Created attachment 263663 [details]
Patch final version
Comment 7 Frédéric Wang (:fredw) 2015-10-21 00:06:07 PDT
Committed r191378: <http://trac.webkit.org/changeset/191378>