Begin switch to SkFontStyle for legacy calls.

Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1873923002

TBR=reed
This doesn't really change API, just modernizes it.

Committed: https://skia.googlesource.com/skia/+/11a77c6e0634e2feb6fe4e74806db2fdd2a799ec

[bungeman-skia, 2016-04-08 19:44:57]

Description was changed from

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.
==========

to

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bungeman-skia, 2016-04-08 19:45:11]

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

[commit-bot: I haz the power, 2016-04-08 19:45:20]

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

[commit-bot: I haz the power, 2016-04-08 19:47:49]

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

[commit-bot: I haz the power, 2016-04-08 19:47:50]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)

[bungeman-skia, 2016-04-08 19:55:55]

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

[commit-bot: I haz the power, 2016-04-08 19:55:58]

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

[commit-bot: I haz the power, 2016-04-08 19:59:08]

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

[commit-bot: I haz the power, 2016-04-08 19:59:09]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[bungeman-skia, 2016-04-08 20:20:32]

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

[commit-bot: I haz the power, 2016-04-08 20:20:37]

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

[commit-bot: I haz the power, 2016-04-08 20:24:07]

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

[commit-bot: I haz the power, 2016-04-08 20:24:08]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[bungeman-skia, 2016-04-08 21:40:05]

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

[commit-bot: I haz the power, 2016-04-08 21:40:09]

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

[commit-bot: I haz the power, 2016-04-08 21:41:39]

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

[commit-bot: I haz the power, 2016-04-08 21:41:40]

Dry run: Try jobs failed on following builders:
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)

[bungeman-skia, 2016-04-11 14:29:37]

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

[commit-bot: I haz the power, 2016-04-11 14:29:38]

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

[commit-bot: I haz the power, 2016-04-11 14:41:58]

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

[commit-bot: I haz the power, 2016-04-11 14:41:59]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2016-04-11 15:09:57]

Description was changed from

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bungeman-skia, 2016-04-11 17:31:34]

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

[commit-bot: I haz the power, 2016-04-11 17:31:46]

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

[bungeman-skia, 2016-04-11 17:36:14]

bungeman@google.com changed reviewers:
+ benjaminwagner@google.com, mtklein@google.com, reed@google.com

[commit-bot: I haz the power, 2016-04-11 17:48:36]

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

[commit-bot: I haz the power, 2016-04-11 17:48:37]

Dry run: This issue passed the CQ dry run.

[dogben, 2016-04-11 17:55:28]

Why not this?
#ifdef SK_VERY_LEGACY_CREATE_TYPEFACE
    SkTypeface* legacyCreateTypeface(const char familyName[], unsigned
typefaceStyleBits) const;
#endif
    SkTypeface* legacyCreateTypeface(const char familyName[], SkFontStyle style)
const;

Then callers can choose whether to use unsigned or SkFontStyle overload. I know
C++ has issues with overloaded overrides, but I think those issues can be
overcome.

This is what reed and fmalita have been doing for sk_sp, and it makes it easier
to update clients incrementally. Also, this way you don't need the #ifdefs in
the calling functions and you can remove one implementation of GetFontStyle in
SkFontConfigInterface_direct.cpp.

Or is this because SkFontStyle needs to remain binary compatible?

[bungeman-skia, 2016-04-11 18:16:13]

On 2016/04/11 17:55:28, Ben Wagner wrote:
> Why not this?
> #ifdef SK_VERY_LEGACY_CREATE_TYPEFACE
>     SkTypeface* legacyCreateTypeface(const char familyName[], unsigned
> typefaceStyleBits) const;
> #endif
>     SkTypeface* legacyCreateTypeface(const char familyName[], SkFontStyle
style)
> const;
> 
> Then callers can choose whether to use unsigned or SkFontStyle overload. I
know
> C++ has issues with overloaded overrides, but I think those issues can be
> overcome.
> 
> This is what reed and fmalita have been doing for sk_sp, and it makes it
easier
> to update clients incrementally. Also, this way you don't need the #ifdefs in
> the calling functions and you can remove one implementation of GetFontStyle in
> SkFontConfigInterface_direct.cpp.
> 
> Or is this because SkFontStyle needs to remain binary compatible?

Because it will cause great headaches to have both at any point. Also, there is
only one target which this should affect, and that will need to be done all at
once anyway.

[dogben, 2016-04-11 20:28:58]

lgtm

A couple comments on things that don't quite seem right, but if I'm mistaken,
LGTM.

https://codereview.chromium.org/1873923002/diff/100001/include/core/SkFontSty...
File include/core/SkFontStyle.h (right):

https://codereview.chromium.org/1873923002/diff/100001/include/core/SkFontSty...
include/core/SkFontStyle.h:49: explicit SkFontStyle(uint32_t d)
I don't see anywhere you use the uint32_t constructor or operator uint32_t().

It seems a little scary to be removing an int constructor and also adding
another int constructor that does something completely different.

https://codereview.chromium.org/1873923002/diff/100001/src/ports/SkFontConfig...
File src/ports/SkFontConfigInterface_direct.cpp (right):

https://codereview.chromium.org/1873923002/diff/100001/src/ports/SkFontConfig...
src/ports/SkFontConfigInterface_direct.cpp:376: if (val < ave(ranges[i].old_val,
ranges[i+1].old_val)) {
I think you would get the same result if you just do:
        if (val < ranges[i+1].old_val) {
            return map_range(val, ranges[i].old_val, ranges[i+1].old_val,
                                  ranges[i].new_val, ranges[i+1].new_val);
        }

It's all linear between the specified points.

Unless you're trying to linearize between the midpoints, which seems wrong.

[bungeman-skia, 2016-04-11 21:55:53]

https://codereview.chromium.org/1873923002/diff/100001/include/core/SkFontSty...
File include/core/SkFontStyle.h (right):

https://codereview.chromium.org/1873923002/diff/100001/include/core/SkFontSty...
include/core/SkFontStyle.h:49: explicit SkFontStyle(uint32_t d)
On 2016/04/11 20:28:58, Ben Wagner wrote:
> I don't see anywhere you use the uint32_t constructor or operator uint32_t().
> 
> It seems a little scary to be removing an int constructor and also adding
> another int constructor that does something completely different.

This was done to make the Chromium side IPC change as small as possible.I may
end up being able to remove it.

Done.

https://codereview.chromium.org/1873923002/diff/100001/src/ports/SkFontConfig...
File src/ports/SkFontConfigInterface_direct.cpp (right):

https://codereview.chromium.org/1873923002/diff/100001/src/ports/SkFontConfig...
src/ports/SkFontConfigInterface_direct.cpp:376: if (val < ave(ranges[i].old_val,
ranges[i+1].old_val)) {
On 2016/04/11 20:28:58, Ben Wagner wrote:
> I think you would get the same result if you just do:
>         if (val < ranges[i+1].old_val) {
>             return map_range(val, ranges[i].old_val, ranges[i+1].old_val,
>                                   ranges[i].new_val, ranges[i+1].new_val);
>         }
> 
> It's all linear between the specified points.
> 
> Unless you're trying to linearize between the midpoints, which seems wrong.

This was all copy-pasted from somewhere else, but I think you're right. Mostly
from the argument that we're linearly interpolating, but we only have two
points, so what else was going to happen?

Done.

[bungeman-skia, 2016-04-11 21:58:29]

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

[commit-bot: I haz the power, 2016-04-11 21:58:35]

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

[commit-bot: I haz the power, 2016-04-11 22:00:05]

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

[commit-bot: I haz the power, 2016-04-11 22:00:06]

Dry run: Try jobs failed on following builders:
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)

[dogben, 2016-04-11 22:23:17]

lgtm

I'm assuming there's a good reason for having two copies of the map_ranges code.

[bungeman-skia, 2016-04-12 15:22:06]

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

[commit-bot: I haz the power, 2016-04-12 15:22:12]

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

[commit-bot: I haz the power, 2016-04-12 15:34:20]

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

[commit-bot: I haz the power, 2016-04-12 15:34:21]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2016-04-12 20:32:48]

Description was changed from

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

TBR=reed
This doesn't really change API, just modernize it.
==========

[bungeman-skia, 2016-04-12 20:33:10]

Description was changed from

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

TBR=reed
This doesn't really change API, just modernize it.
==========

to

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

TBR=reed
This doesn't really change API, just modernizes it.
==========

[bungeman-skia, 2016-04-12 20:33:21]

The CQ bit was checked by bungeman@google.com

[bungeman-skia, 2016-04-12 20:33:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjaminwagner@google.com
Link to the patchset: https://codereview.chromium.org/1873923002/#ps180001
(title: "Dont update bzl file now.")

[commit-bot: I haz the power, 2016-04-12 20:33:30]

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

[commit-bot: I haz the power, 2016-04-12 20:45:10]

Description was changed from

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

TBR=reed
This doesn't really change API, just modernizes it.
==========

to

==========
Begin switch to SkFontStyle for legacy calls.

This adds SK_VERY_LEGACY_CREATE_TYPEFACE which, when defined, provides
only the old interface.

Ideally, everyone would switch directly to SkFontMgr and use one of the
newer calls, but there is currently no path for current users to get
there. This updates all the internals to use SkFontStyle, after
switching these over the higher level APIs can be switched.

The Chromium follow on patch can be seen at https://crrev.com/1877673002

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

TBR=reed
This doesn't really change API, just modernizes it.

Committed:
https://skia.googlesource.com/skia/+/11a77c6e0634e2feb6fe4e74806db2fdd2a799ec
==========

[commit-bot: I haz the power, 2016-04-12 20:45:11]

Committed patchset #10 (id:180001) as
https://skia.googlesource.com/skia/+/11a77c6e0634e2feb6fe4e74806db2fdd2a799ec