Use only one CGFont/CTFont per scaler context on Mac.

Use only one CGFont/CTFont per scaler context on Mac.

The unrotated CTFont was introduced to fix color emoji scaling.
Use only the unrotated CTFont and apply the rotations manually.
This allows removal of one CTFont and allows us to apply (correctly)
the transformation CoreText would otherwise need to apply
(which it appears to do inconsistently).

Committed: https://skia.googlesource.com/skia/+/ef27ce3730d4032515e27e5aa7b31b2088969237

[bungeman-skia, 2015-09-22 16:07:52]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-22 16:07:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360663002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360663002/20001

[bungeman-skia, 2015-09-22 16:11:16]

bungeman@google.com changed reviewers:
+ erikchen@chromium.org

[bungeman-skia, 2015-09-22 16:11:16]

erikchen: could you take a look and make sure that this fixes the issue for you
also? If so, this change may be slightly less disruptive and somewhat more
correct.

[commit-bot: I haz the power, 2015-09-22 16:13:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-22 16:13:02]

Dry run: This issue passed the CQ dry run.

[erikchen, 2015-09-22 16:45:02]

On 2015/09/22 16:13:02, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

This CL does not fix the problem on 10.11.

[bungeman-skia, 2015-09-22 16:49:45]

On 2015/09/22 16:45:02, erikchen wrote:
> On 2015/09/22 16:13:02, commit-bot: I haz the power wrote:
> > Dry run: This issue passed the CQ dry run.
> 
> This CL does not fix the problem on 10.11.

Thanks, this at least confirms that the issue is directly with
CTFontCreateCopyWithAttributes, and not a result of calling it twice or using
two CTFonts.

[erikchen, 2015-09-22 16:54:06]

On 2015/09/22 16:49:45, bungeman1 wrote:
> On 2015/09/22 16:45:02, erikchen wrote:
> > On 2015/09/22 16:13:02, commit-bot: I haz the power wrote:
> > > Dry run: This issue passed the CQ dry run.
> > 
> > This CL does not fix the problem on 10.11.
> 
> Thanks, this at least confirms that the issue is directly with
> CTFontCreateCopyWithAttributes, and not a result of calling it twice or using
> two CTFonts.

(This may be what you're saying as well, I can't tell). By my reading of your
code, and my previous debugging of this problem, the problem with this CL is
that we're passing CTFontCreateCopyWithAttributes a system font (for size=13),
with size=26, and that causes CTFontCreateCopyWithAttributes to return a
different system font with a different glyph table).

[bungeman-skia, 2015-09-22 17:03:39]

On 2015/09/22 16:54:06, erikchen wrote:
> On 2015/09/22 16:49:45, bungeman1 wrote:
> > On 2015/09/22 16:45:02, erikchen wrote:
> > > On 2015/09/22 16:13:02, commit-bot: I haz the power wrote:
> > > > Dry run: This issue passed the CQ dry run.
> > > 
> > > This CL does not fix the problem on 10.11.
> > 
> > Thanks, this at least confirms that the issue is directly with
> > CTFontCreateCopyWithAttributes, and not a result of calling it twice or
using
> > two CTFonts.
> 
> (This may be what you're saying as well, I can't tell). By my reading of your
> code, and my previous debugging of this problem, the problem with this CL is
> that we're passing CTFontCreateCopyWithAttributes a system font (for size=13),
> with size=26, and that causes CTFontCreateCopyWithAttributes to return a
> different system font with a different glyph table).

This change makes it so that the SkScalarContext only ever has one CTFont, and
uses that for everything. However, it is the case that
WebKit/Source/platform/fonts/SimpleFontData.cpp is calling
SkTypeface::charsToGlyphs instead of going through the paint (and the
SkScalarContext), and so effectively using a different backing font. I was
against adding charsToGlyphs on SkTypeface for exactly this potential reason in
the past, it's nice to be vindicated, but now a pain to work around. Thanks for
testing this out.

[bungeman-skia, 2015-10-05 21:31:05]

bungeman@google.com changed reviewers:
+ mtklein@google.com, reed@google.com
- erikchen@chromium.org

[bungeman-skia, 2015-10-05 21:31:05]

Since the CTFont now being created has nullptr for the transform, CoreText /
CoreGraphics won't by applying it (as it does so inconsistently). As a result,
Skia applying the transform itself should be the same amount of work, but be
consistent.

[mtklein, 2015-10-05 21:38:43]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-05 21:39:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360663002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360663002/60001

[commit-bot: I haz the power, 2015-10-05 21:45:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-05 21:45:45]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-10-05 21:58:12]

mtklein@google.com changed required reviewers:
+ reed@google.com

[mtklein, 2015-10-05 21:58:12]

grain-of-salt lgtm

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
src/ports/SkFontHost_mac.cpp:1379: CGAffineTransform
scale(CGAffineTransformMakeScale(ScalarToCG(scaleX), ScalarToCG(scaleY)));
I feel like this diff is confusing, but I'm not sure the new code is confusing. 
I think the old code is confusing.  I maybe be generally confused.  Might help
to diagram what matrices are concatted here.

[bungeman-skia, 2015-10-29 15:36:35]

I've run this change through the Skia trybots here and the Chromium trybots at
https://codereview.chromium.org/1426923003/ . This changed no images in any GMs
or LayoutTests.

[reed1, 2015-10-29 15:44:23]

lgtm w/ duplicate line nit

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
src/ports/SkFontHost_mac.cpp:937: skVertOffset = {
CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
are these two lines the same?

[bungeman-skia, 2015-10-29 15:52:13]

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/1360663002/diff/60001/src/ports/SkFontHost_ma...
src/ports/SkFontHost_mac.cpp:937: skVertOffset = {
CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
On 2015/10/29 15:44:23, reed1 wrote:
> are these two lines the same?

Done.

[bungeman-skia, 2015-10-29 15:52:19]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 15:52:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360663002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360663002/80001

[commit-bot: I haz the power, 2015-10-29 16:03:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 16:03:03]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-10-29 16:29:54]

The CQ bit was checked by bungeman@google.com

[bungeman-skia, 2015-10-29 16:29:54]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com,
reed@google.com
Link to the patchset: https://codereview.chromium.org/1360663002/#ps80001
(title: "Remove duplicate code.")

[commit-bot: I haz the power, 2015-10-29 16:30:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360663002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360663002/80001

[commit-bot: I haz the power, 2015-10-29 16:30:34]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/ef27ce3730d4032515e27e5aa7b31b2088969237