Add Windows-compatible font metrics to SkPaint::FontMetrics. This is needed to

Add Windows-compatible font metrics to SkPaint::FontMetrics. This is needed to
get Windows-compatible line spacing for fonts using VDMX table metrics in
Chromium Linux and BlackBerry WebKit.

BUG=webkit:83279

[epoger, 2012-04-05 19:48:01]

Adding bungeman, who has familiarity with Windows font issues...

https://codereview.appspot.com/5985052/diff/1/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.appspot.com/5985052/diff/1/include/core/SkPaint.h#newcode4
include/core/SkPaint.h:4: * Copyright (C) 2012 Research In Motion Limited. All
rights reserved.
I don't know what the implication of adding this copyright would be.  Anyone
know a good person to ask about this?

https://codereview.appspot.com/5985052/diff/1/src/core/SkPaint.cpp
File src/core/SkPaint.cpp (right):

https://codereview.appspot.com/5985052/diff/1/src/core/SkPaint.cpp#newcode4
src/core/SkPaint.cpp:4: * Copyright (C) 2012 Research In Motion Limited. All
rights reserved.
another copyright change

https://codereview.appspot.com/5985052/diff/1/src/ports/SkFontHost_FreeType.cpp
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.appspot.com/5985052/diff/1/src/ports/SkFontHost_FreeType.c...
src/ports/SkFontHost_FreeType.cpp:4: * Copyright (C) 2012 Research In Motion
Limited. All rights reserved.
...and another

[Stephen White, 2012-04-05 19:52:40]

https://codereview.appspot.com/5985052/diff/1/include/core/SkPaint.h
File include/core/SkPaint.h (right):

https://codereview.appspot.com/5985052/diff/1/include/core/SkPaint.h#newcode4
include/core/SkPaint.h:4: * Copyright (C) 2012 Research In Motion Limited. All
rights reserved.
On 2012/04/05 19:48:01, epoger wrote:
> I don't know what the implication of adding this copyright would be.  Anyone
> know a good person to ask about this?

I think the first thing to do is make sure that someone at RIM has filled out
the Corporate Contributor License Agreement: 
http://source.android.com/source/cla-corporate.html to make them part of the
Android open source project.

Once that is done, the single Android copyright should suffice, since RIM will
be included as a contributor.

[efidler, 2012-04-05 20:26:18]

> On 2012/04/05 19:48:01, epoger wrote:
> > I don't know what the implication of adding this copyright would be.  Anyone
> > know a good person to ask about this?
> 
> I think the first thing to do is make sure that someone at RIM has filled out
> the Corporate Contributor License Agreement: 
> http://source.android.com/source/cla-corporate.html to make them part of the
> Android open source project.

I don't think there's any way RIM will sign that. This change is so minimal that
it should be fine to land it with no copyright change.

[epoger, 2012-04-05 20:29:08]

Please re-upload without the copyright changes then, and let's get bungeman or
reed to take a look...

[bungeman, 2012-04-05 21:15:21]

On 2012/04/05 19:40:04, tony wrote:

I'm not sure if the reasoning on the WebKit bug is even correct. Have you tried
with FreeType 2.4.6 or later (after b0962ac34e66052ccfee7996e5468f30d4bd5a72)?
This change made the metrics line up with the glyphs which FreeType is actually
producing. It also causes several other headaches which Skia will need to clean
up.

Even if we do want exactly this, all of the other FontHosts will need to be
updated as well.

I don't think this change is what we want though, it would be more reasonable to
have the caller be able to request which kind of metrics they desire, Mac, Win,
Typo, WinButTypoIfFlaged, etc and return the desired values in the existing
fAscent and fDescent fields. If the user needs more than that, they probably
need to just look in the tables themselves.

This is actually quite an invasive change either way and I would prefer to make
sure Reed gets a good look first.

[efidler, 2012-04-05 22:07:43]

Since we're doing the VDMX handling in WebKit, we explicitly need the
Ascent/Descent/LineGap from the HHEA table and the WinAscent/WinDescent from the
OS/2 table. I suppose we could do OS/2 parsing in WebKit, but that's a lot of
duplicate work, both in code and at runtime.

If Skia wants to hide all Windows metrics handling including VDMX, that's fine
with me too, but I think that's potentially more invasive to Skia.

[reed1, 2012-04-09 16:22:48]

(sorry, late to the party)

There are at least 3 sets of these values (unfortunately), since there are typo
variants in the OS/2 table. Given this, and other misc. uses, I'd like to
formally add getTable() functions to SkTypeface. Once those land, would we still
need to expand SkPaint::FontMetrics?

[efidler, 2012-04-09 17:14:30]

For this use case, I need:

HEAD unitsPerEm, yMin, yMax
HHEA yAscender, yDescender, yLineGap
OS/2 usWinAscent, usWinDescent
VDMX yMin and yMax (if they exist) at the current pel size

I don't need (for now, but probably soon will):
OS/2 sTypoAscender, sTypoDescender, sTypoLineGap

They don't necessarily need to be in the SkPaint::FontMetrics object, as long as
I can get them.

[efidler, 2012-04-17 16:08:43]

Further thoughts? We're going to be forked for both Skia and WebKit until we can
get this data. I'm happy to adapt our code to whatever Skia API you prefer, but
I don't think I can write the whole patch myself.

[reed1, 2012-04-17 16:35:03]

https://codereview.appspot.com/6050046

My suggestion is to use these APIs directly, once that CL lands. From those you
can get to whatever specific metrics you want, and tailor those querys
per-platform if you wish.

Do you think that will meet your needs?

[efidler, 2012-04-17 17:17:49]

I don't currently have code in WebKit to parse HEAD, HHEA and OS/2 table data.
Wouldn't it make sense for Skia to have that code?

[bungeman, 2012-04-17 17:51:15]

On 2012/04/17 17:17:49, efidler wrote:
> I don't currently have code in WebKit to parse HEAD, HHEA and OS/2 table data.
> Wouldn't it make sense for Skia to have that code?

Feel free to copy (with all appropriate attributions, of course) from Skia's
src/sfnt directory. This directory contains some header files with rather
detailed definitions of the tables in question. Skia currently is not exposing
these publicly since we don't want to have to support them if we don't have to,
but they're rather well researched, at least. Since we will be exposing the
table data getters, it may be possible to make the argument that these headers
should be exposed.