Bug 75702 – [Chromium] Use SkFontHost::GetUnitsPerEm instead of advanced type metrics for Android in FontPlatformData

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Peter Beverloo]

[Chromium] Use SkFontHost::GetUnitsPerEm instead of advanced type metrics for Android in FontPlatformData

[Peter Beverloo]

Created attachment 121431 [details]
Patch

[Peter Beverloo]

Hi Tony, could you take a look please?

[Tony Chang]

Comment on attachment 121431 [details]
Patch

What's the difference between getAdvancedTypefaceMetrics and GetUnitsPerEm?  Is one faster?  On platforms other than Android, do they return the same value?  If so, should we use GetUnitsPerEm on all platforms?

[Tony Chang]

Also, feel free to loop in someone who works on skia to review.

[Peter Beverloo]

(In reply to comment #3)
> (From update of attachment 121431 [details])
> What's the difference between getAdvancedTypefaceMetrics and GetUnitsPerEm?  Is one faster?  On platforms other than Android, do they return the same value?  If so, should we use GetUnitsPerEm on all platforms?

It's an Android-specific function which solely retrieves the em size from the font header, whereas getAdvancedTypefaceMetrics retrieves other information (such as the name, font type and italic angle) as well. The latter invokes Freetype's FT_Get_PS_Font_Info function which is defined in the "type1" extension, which is not available in Android.

+senorblanco

[Tony Chang]

(In reply to comment #5)
> It's an Android-specific function which solely retrieves the em size from the font header, whereas getAdvancedTypefaceMetrics retrieves other information (such as the name, font type and italic angle) as well. The latter invokes Freetype's FT_Get_PS_Font_Info function which is defined in the "type1" extension, which is not available in Android.

Should we implement GetUnitsPerEm on all platforms so we don't have to have an #if here?

[Peter Beverloo]

Stephen, could you comment on that option? Removing the #if-guards in Skia code and getting that rolled into WebKit would be an option as well.

[Stephen White]

+reed, who knows Skia text code far better than me.

Mike, any thoughts on this?  Should we fold this into Skia, or leave it here in WebKit?

[Mike Reed]

UnitsPerEm should be an API on SkTypeface (we shouldn't be making calls on SkFontHost). For now do whatever is easier I guess, until the new API arrives.

http://code.google.com/p/skia/issues/detail?id=441

[Stephen White]

Comment on attachment 121431 [details]
Patch

In that case, I'm fine with going ahead with this.  You might want to add a comment referencing the skia bug Mike just created.  r=me

[Peter Beverloo]

I prefer to keep references local to WebKit, so I filed bug 75961. Since it involves the current Android-hack, I'll take care of that cleanup.

[Peter Beverloo]

Created attachment 121845 [details]
Patch for landing

Patch for landing

[WebKit Review Bot]

Comment on attachment 121845 [details]
Patch for landing

Clearing flags on attachment: 121845

Committed r104578: <http://trac.webkit.org/changeset/104578>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.