Skia: merge font metrics work from Chromium

In order to match Windows font metrics on Linux with Chromium, several additional font metrics needed to be extracted from the TrueType files.

This patch is upstreaming those changes.

[reed, 2009-04-07 16:40:21]

In general I think the additions are fine, but its hard to know for sure w/o
seeing how the new fields are used. Since this represents both an addition to
the public API and an addition to the porting layer requirements, its important
that the additions be necessary, minimal, and clear (to future backend
implementors).

http://codereview.appspot.com/28145/diff/7/1006
File include/core/SkPaint.h (right):

http://codereview.appspot.com/28145/diff/7/1006#newcode616
Line 616: SkScalar    fLeading;   //!< The recommended distance to add between
lines of text (will be >= 0)
Does fHeight == fDescent - fAscent + fLeading? If so, do we need it? If not, how
do we explain it?

http://codereview.appspot.com/28145/diff/7/1006#newcode618
Line 618: SkScalar    fAvgCharWidth;  //!< the average charactor width (>= 0)
Not sure how this is useful if we don't know the xmin and/or xmax. If we are
going to add this data, I think I'd prefer fXMin and fXMax (or better names
since we have H and V versions of this struct.

http://codereview.appspot.com/28145/diff/7/1006#newcode624
Line 624: 
These last three are a little wacky, since they may not reflect what our font
scaler actually computes (since they were built off of the windows rasterizer).
Are they really necessary to add?

http://codereview.appspot.com/28145/diff/7/1007
File src/ports/SkFontHost_FreeType.cpp (right):

http://codereview.appspot.com/28145/diff/7/1007#newcode319
Line 319: case kNormal_Hints:
Good addition. This should have been here from the beginning.

http://codereview.appspot.com/28145/diff/7/1007#newcode721
Line 721: //
-----------------------------------------------------------------------------
Has this file name changed?

http://codereview.appspot.com/28145/diff/7/1007#newcode724
Line 724: //
-----------------------------------------------------------------------------
Should we create a formal header file for this function?

http://codereview.appspot.com/28145/diff/7/1007#newcode806
Line 806: ys[4] = leading;
How is this height different from ascent+descent+leading? If it is different,
that would be weird.

http://codereview.appspot.com/28145/diff/7/1007#newcode810
Line 810: SkScalar x_height;
why aren't we using another ys[] slot for xheight?

http://codereview.appspot.com/28145/diff/7/1010
File src/ports/SkFontHost_TrueType_Tables.cpp (right):

http://codereview.appspot.com/28145/diff/7/1010#newcode44
Line 44: //
-----------------------------------------------------------------------------
Are we in danger of collisions with another class by this name? perhaps we
should uglify the name (e.g. TrueTypeTableBufferUtil) or use some local
namespace to reduce that chance.

[agl, 2009-04-07 22:03:26]

http://codereview.appspot.com/28145/diff/7/1006
File include/core/SkPaint.h (right):

http://codereview.appspot.com/28145/diff/7/1006#newcode616
Line 616: SkScalar    fLeading;   //!< The recommended distance to add between
lines of text (will be >= 0)
On 2009/04/07 16:40:21, reed wrote:
> Does fHeight == fDescent - fAscent + fLeading? If so, do we need it? If not,
how
> do we explain it?

I've removed it. Leading was previously wrong as explained below.

http://codereview.appspot.com/28145/diff/7/1006#newcode618
Line 618: SkScalar    fAvgCharWidth;  //!< the average charactor width (>= 0)
On 2009/04/07 16:40:21, reed wrote:
> Not sure how this is useful if we don't know the xmin and/or xmax. If we are
> going to add this data, I think I'd prefer fXMin and fXMax (or better names
> since we have H and V versions of this struct.

I've switched to XMin and XMax (we already have fTop and fBottom). The average
width is needed to calculate the width of text boxes in WebKit.

http://codereview.appspot.com/28145/diff/7/1007
File src/ports/SkFontHost_FreeType.cpp (right):

http://codereview.appspot.com/28145/diff/7/1007#newcode721
Line 721: //
-----------------------------------------------------------------------------
On 2009/04/07 16:40:21, reed wrote:
> Has this file name changed?

Yes, fixed. thanks.

http://codereview.appspot.com/28145/diff/7/1007#newcode724
Line 724: //
-----------------------------------------------------------------------------
On 2009/04/07 16:40:21, reed wrote:
> Should we create a formal header file for this function?

It seems like overkill for a single function. Esp if we're going to be moving
this into our WebKit port anyway.

http://codereview.appspot.com/28145/diff/7/1007#newcode793
Line 793: int leading = face->height - face->ascender + face->descender;
The reason that I added fHeight was because I couldn't get it to work otherwise.
I suspect the problem is missing brackets here. This line is doing leading =
(height - ascent) + descent. Fixed and removed fHeight.

http://codereview.appspot.com/28145/diff/7/1007#newcode806
Line 806: ys[4] = leading;
On 2009/04/07 16:40:21, reed wrote:
> How is this height different from ascent+descent+leading? If it is different,
> that would be weird.

Removed. See comment on line 793.

http://codereview.appspot.com/28145/diff/7/1007#newcode810
Line 810: SkScalar x_height;
On 2009/04/07 16:40:21, reed wrote:
> why aren't we using another ys[] slot for xheight?

It doesn't work when the font doesn't have xHeight in the OS/2 table, right? We
don't want to double translate the result from Get_CBox I believe.

http://codereview.appspot.com/28145/diff/7/1010
File src/ports/SkFontHost_TrueType_Tables.cpp (right):

http://codereview.appspot.com/28145/diff/7/1010#newcode44
Line 44: //
-----------------------------------------------------------------------------
On 2009/04/07 16:40:21, reed wrote:
> Are we in danger of collisions with another class by this name? perhaps we
> should uglify the name (e.g. TrueTypeTableBufferUtil) or use some local
> namespace to reduce that chance.

It's local to the file and we don't have any C++ includes, so I think we're
safe.