Fix vertical text scaling on Mac.

This is to fix https://code.google.com/p/chromium/issues/detail?id=238081 . The issue is most easily seen by opening the GM:verttext or GM:verttext2 slide in SampleApp and zooming in or out or flipping on Lion or Mountain Lion. The behavior is actually correct on Snow Leopard.

This change fixes the issues with vertical metrics on Lion and Mountain Lion while also keeping things working on Snow Leopard.  Since Leopard is no longer supported and I have no Leopard machine to test on, this also removes the remaining Leopard code.

[bungeman-skia, 2013-05-10 17:08:03]

Publishing some notes to myself.

https://codereview.chromium.org/15064003/diff/6001/src/ports/SkFontHost_mac.cpp
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/15064003/diff/6001/src/ports/SkFontHost_mac.c...
src/ports/SkFontHost_mac.cpp:303: static SkScalar getFontScale(CGFontRef cgFont)
{
This is an ambiguous name, emPerFUnit would be more accurate.

https://codereview.chromium.org/15064003/diff/6001/src/ports/SkFontHost_mac.c...
src/ports/SkFontHost_mac.cpp:787: CGContextSetTextMatrix(fCG,
context.fTransform);
Can this just be the transform from the CTFont? If so, we can drop fTransform
entirely.

https://codereview.chromium.org/15064003/diff/6001/src/ports/SkFontHost_mac.c...
src/ports/SkFontHost_mac.cpp:918: fFBoundingBoxesMatrix.preScale(fontScale,
fontScale);
The matrix used in getVerticalOffset for SnowLeopard is the same (as it goes
from y-down FUnits to SkGlyph units). Rename fFBoundingBoxesMatrix to
fFUnitTransform (or something to parallel fTransform) and use in both places. In
fact, fTransform is now used outside the constructor only to set up the
offscreen CGContext since it is magically hidden inside the CTFont itself.

https://codereview.chromium.org/15064003/diff/8001/src/ports/SkFontHost_mac.cpp
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/15064003/diff/8001/src/ports/SkFontHost_mac.c...
src/ports/SkFontHost_mac.cpp:811: sk_memset_rect32(image, 0x00333333,
glyph.fWidth, glyph.fHeight, rowBytes);
This is here just to make the bbox visible. Will be removed.

[bungeman-skia, 2013-05-10 17:22:52]

After much hair pulling with regard to CoreText vertical metrics, with this
change the correct vertical metrics are produced on Snow Leopard, Lion, and
Mountain Lion.

[caryclark, 2013-05-10 18:09:30]

lgtm

[bungeman-skia, 2013-05-10 22:30:36]

With this change (at the approved Patch Set 7) the bounds are often slightly
different, causing an equal but different clipping to what was happening prior
to this change. Since the glyphs still looked poor and didn't quite line up, and
because this change is going to change all vertical glyphs anyway, Patch Set 9
changes things up so that the glyph is actually drawn in the correct place. This
simultaneously makes the glyphs line up better and look better (no clipping).

https://codereview.chromium.org/15064003/diff/23001/src/core/SkDraw.cpp
File src/core/SkDraw.cpp (right):

https://codereview.chromium.org/15064003/diff/23001/src/core/SkDraw.cpp#newco...
src/core/SkDraw.cpp:1430: SkScalar delta = xpos - prevXPos;
This is probably a bad solution, it's rather strange that SkTextToPathIter known
about vertical text, but still uses a total scalar advance instead of a total
vector advance. Perhaps SkTextToPathIter should get some attention.

I wanted to make sure that the path stuff was working, and the easiest way to do
that was to hit 'k' in SampleApp. Doing so made all the text lay out
horizontally, but by vertical metrics.

This change need not be part of the final CL, but we should fix this.

[bungeman-skia, 2013-05-14 19:44:37]

The biggest improvement (with Patch Set 10) can be seen in
fast/text/emphasis-vertical.html where we were previously clipping the serifs at
the top of the glyphs (and in one case clipping one glyph enough at the top to
almost split a stroke). A similar large improvement can be seen in
fast/text/international/vertical-text-glyph-test.html where we are currently
clipping the latin glyphs quite severely.

The modification from Patch Set 9 to Patch Set 10 is to deal with the remaining
clipping discovered when running Blink layout tests. CoreGraphics appears to
have issues with OTF CFF fonts and vertical requests. At Patch Set 10 I can no
longer find any examples of glyph clipping in the layout tests.

This results in ~770 rebaselines per Mac platform, as this no longer clips a
number of horizontal 'e', 't', etc, as well as changing most vertical glyphs.

[caryclark, 2013-05-14 20:08:09]

lgtm

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac.cpp
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:642: virtual ~SkScalerContext_Mac() { };
can we remove the destructor altogether at this point?

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:748: fCTFont =
CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, ctFontDesc);
It looks like the rest of Skia mostly uses 1.f instead of 1 only in places where
it is forcing a float result, e.g., 1.f / 255 -- so I think it would be just as
clear and consistent to pass '1' here.

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:753: fCTVerticalFont =
CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, NULL);
and here

[bungeman-skia, 2013-05-14 22:30:20]

Patch Set 11 is the same as Patch Set 10 (modulo small fixes for comments)
except for the re-introduction of original code (slightly modified), which is
guarded by SK_IGNORE_MAC_TEXT_BOUNDS_FIX. I have locally confirmed that with
SK_IGNORE_MAC_TEXT_BOUNDS_FIX defined layout tests behave as before, and without
SK_IGNORE_MAC_TEXT_BOUNDS_FIX defined we get the new behavior.

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac.cpp
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:642: virtual ~SkScalerContext_Mac() { };
On 2013/05/14 20:08:09, caryclark wrote:
> can we remove the destructor altogether at this point?

Done.

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:748: fCTFont =
CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, ctFontDesc);
On 2013/05/14 20:08:09, caryclark wrote:
> It looks like the rest of Skia mostly uses 1.f instead of 1 only in places
where
> it is forcing a float result, e.g., 1.f / 255 -- so I think it would be just
as
> clear and consistent to pass '1' here.

Done.

https://codereview.chromium.org/15064003/diff/32001/src/ports/SkFontHost_mac....
src/ports/SkFontHost_mac.cpp:753: fCTVerticalFont =
CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, NULL);
On 2013/05/14 20:08:09, caryclark wrote:
> and here

Done.

[bungeman-skia, 2013-05-15 15:09:29]

Committed revision 9142.