WIP -- SkFont

WIP -- SkFont

BUG=skia:
R=bungeman@google.com, fmalita@chromium.org

Committed: https://code.google.com/p/skia/source/detail?r=14090

[bungeman-skia, 2014-03-25 19:51:44]

https://codereview.chromium.org/185293018/diff/70001/include/core/SkFont.h
File include/core/SkFont.h (right):

https://codereview.chromium.org/185293018/diff/70001/include/core/SkFont.h#ne...
include/core/SkFont.h:22: enum Flags {
Should requests for synthetics go here (fake bold)? This one in particular is
interesting as FreeType has it's own fake bolder (which both Chromium and
Android currently use).

https://codereview.chromium.org/185293018/diff/70001/include/core/SkFont.h#ne...
include/core/SkFont.h:25: kGridAlignMetrics_Flag  = 1 << 2,
These three bits are awesome, but I almost want a list of values these three can
take in a table with a description of the expected outcome for each combination.
It should also be made obvious that these three bits are rather tied to each
other (not that the other bits are completely independent either).

Maybe these are easier to understand with names like

kEnableAutoHint_Flag
kEnableByteCodeHint_Flag
kApplyHintToMetrics_Flag

where the explanation is that "Only enabled hinting methods will be applied.
Bytecode will take priority over auto if both are enabled. Enabled hinting
methods always apply to shapes and will also be applied to metrics if requested.
If it is requested that hinting be applied to metrics but no hinting is enabled,
metrics will be rounded to integers."

[bungeman-skia, 2014-03-26 16:23:39]

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h
File include/core/SkFont.h (right):

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:24: *  Us the system's automatic hinting mechanism to hint
the typeface.
nit: s/Us/Use

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:32: *  If both bytecode and auto hints are specified,
attempt to use the bytecodes first.
nit: semicolon at the end of this to link to the next sentence

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:51: kLCD_Mask_Flag              = 1 << 6,
Since we're changing up all of this anyway, I'd like to avoid using 'LCD' here.
I'd prefer to refer to it as 'SurfaceDependentRendering' or something like that.

kAllAntialiasedLikeSurfaceDependentRendering
kEnableSurfaceDependentRendering

The idea is to make it obvious that these are just flags to enable the feature,
but that it's up to the SkDeviceProperties (or whatever we end up calling this
sort of thing in the Surface world) to specify the geometry. I'd like to avoid
the temptation to try to add a 'rgb vs bgr' flag here, or for anyone to confuse
kVertical with vertical subpixels. Also, this is an enabler, but only if
available. For example, on Mac it doesn't matter how much one asks for 'lcd' if
the user has it turned off in the system settings, there's no documented way to
get it.

This is also an area where the confusion about exploiting the geometry vs
specifying the geometry has bitten FontConfig. FontConfig allows one to specify
the actual geometry (rgb, bgr, vrgb, vbgr) on a per font basis, which is simply
silly. They should have simply had a per font setting for 'exploit or not,
possibly on a per-geometry basis' and allowed the system settings (for the
display) to indicate what the geometry actually is.

[reed1, 2014-03-26 18:05:00]

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h
File include/core/SkFont.h (right):

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:24: *  Us the system's automatic hinting mechanism to hint
the typeface.
On 2014/03/26 16:23:40, bungeman1 wrote:
> nit: s/Us/Use

Done.

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:32: *  If both bytecode and auto hints are specified,
attempt to use the bytecodes first.
On 2014/03/26 16:23:40, bungeman1 wrote:
> nit: semicolon at the end of this to link to the next sentence

Done.

https://codereview.chromium.org/185293018/diff/150001/include/core/SkFont.h#n...
include/core/SkFont.h:51: kLCD_Mask_Flag              = 1 << 6,
On 2014/03/26 16:23:40, bungeman1 wrote:
> Since we're changing up all of this anyway, I'd like to avoid using '<span
id="abbrex-spnAbbr_69873" class="abbrex-Abbr abbrex-Abbr_LCD abbrex-AbbrUnknown
abbrex-AbbrUnknownVisible">LCD</span>' here.
> I'd prefer to refer to it as 'SurfaceDependentRendering' or something like
that.
> 
> kAllAntialiasedLikeSurfaceDependentRendering
> kEnableSurfaceDependentRendering
> 
> The idea is to make it obvious that these are just flags to enable the
feature,
> but that it's up to the SkDeviceProperties (or whatever we end up calling this
> sort of thing in the Surface world) to specify the geometry. I'd like to avoid
> the temptation to try to add a 'rgb vs bgr' flag here, or for anyone to
confuse
> kVertical with vertical subpixels. Also, this is an enabler, but only if
> available. For example, on Mac it doesn't matter how much one asks for 'lcd'
if
> the user has it turned off in the system settings, there's no documented way
to
> get it.
> 
> This is also an area where the confusion about exploiting the geometry vs
> specifying the geometry has bitten FontConfig. FontConfig allows one to
specify
> the actual geometry (rgb, bgr, vrgb, vbgr) on a per font basis, which is
simply
> silly. They should have simply had a per font setting for 'exploit or not,
> possibly on a per-geometry basis' and allowed the system settings (for the
> display) to indicate what the geometry actually is.

Trying yet another way to organize mask types...

[bungeman-skia, 2014-03-27 19:13:15]

https://codereview.chromium.org/185293018/diff/210001/src/core/SkFont.cpp
File src/core/SkFont.cpp (right):

https://codereview.chromium.org/185293018/diff/210001/src/core/SkFont.cpp#new...
src/core/SkFont.cpp:134: if (SkPaint::kFull_Hinting == paint.getHinting()) {
I believe that on all back-ends we currently consider both normal and full to
enable bytecode (then autohint), with 'slight' meaning autohint (no bytecode),
and 'none' meaning none. The only distinction we currently have between normal
and full is on FreeType, where it enables subpixel (cleartype) style hints. I'm
not sure that's a distinction we're really interest in, and I'm not sure who
uses it or notices the difference (on most platforms there is none).

https://codereview.chromium.org/185293018/diff/210001/src/core/SkFont.cpp#new...
src/core/SkFont.cpp:137: if (paint.isAutohinted()) {
If set, the 'autohinted' flag means (on FreeType only, since it's the only one
which cares about this flag) that if paint hinting is set to normal or full that
autohinting should be enabled and bytecode hinting disabled.

https://codereview.chromium.org/185293018/diff/210001/src/core/SkFont.cpp#new...
src/core/SkFont.cpp:140: if (paint.isSubpixelText() || paint.isLinearText()) {
According to the android docs, setting linear should probably override any
hinting and disable the hinting flags? I can honestly say that there is no
general consensus amongst the back-ends about what the 'subpixel' flag actually
does, but this looks like what it might actually be doing.

[reed1, 2014-04-08 14:36:18]

ptal -- I propose to land this, and then continue iterating on it (but several
of us could then contribute). OK?

[bungeman-skia, 2014-04-08 14:57:39]

sure, why not? lgtm

[f(malita), 2014-04-08 14:59:11]

lgtm

[reed1, 2014-04-08 15:04:37]

Committed patchset #11 manually as r14090 (presubmit successful).

[bungeman-skia, 2014-05-02 19:29:43]

This is a rather late comment, but seemed the best place to put it.

https://codereview.chromium.org/185293018/diff/250001/include/core/SkFont.h
File include/core/SkFont.h (right):

https://codereview.chromium.org/185293018/diff/250001/include/core/SkFont.h#n...
include/core/SkFont.h:99: kEmbeddedBitmaps_Flag       = 1 << 4,
I was just working on this with relation to DirectWrite, and wanted to point out
that the kEmbeddedBitmaps_Flag is actually a hinting flag. In a certain sense,
it's 'hinting' taken to an extreme. As a result, I propose that this flag be
renamed kEnableBitmapHints_Flag (If the typeface contains embedded bitmaps, use
them. If bitmap, bytecode, and auto hints are specified, attempt to use the
bitmaps first.) Part of the reason for this is to allow the use of embedded
bitmaps, but allow the user to specify whether they want the linear metrics or
to use the bitmap metrics.

Unfortunately, I can also see a user wanting to use linear metrics for bytecode
and auto hinting, but use the bitmap advances when using bitmaps. This is, for
example, what FireFox does. However, we were trying to avoid having
kEnableXXXHints_Flag/kApplyXXXHintsToMetrics_Flag combinations.