prototype for kerning api

prototype for kerning api

BUG=skia:1710
R=bungeman@google.com

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

[reed1, 2013-10-23 16:07:43]

Skia has 4 core font backends :  GDI, DirectWrite, CoreText, FreeType

For kerning...
GDI - returns all pairs of characters contained in the font
DirectWrite - returns design units for a specified run of glyphs
CoreText - no api that I can find
FreeType - 1 pair at a time

GDI is rather awkward, since it doesn't take glyphIDs, and it returns *all* the
data for the entire font.

DirectWrite and GDI explicitly return 1D data. FreeType claims to return 2D, but
all of its internal drivers (tt, cff, pfr) actually only return 1D data.

FreeType alone claims to perform "fitting" optionally, but that appears to be
two things:
1. ppem values below 25 are scaled down by the factor ppem/25
2. fractional values are rounded to ints (pixels)

Skia does not perform any shaping or complex layout, including kerning today. If
we were to expose kerning, it would be a convenience API only, since there is
not plan nor request that Skia *apply* kerning automatically to drawText.

Given the above, I can think of two ways to assist clients of Skia that want to
do kerning themselves:

1. Expose the low-level API in this CL, which would front for DirectWrite and
FreeType, and just return false for GDI and CoreText. If the clients wants to
perform ppem-specific rounding, that is trivial for them to do outside of Skia.

2. Make it trivial for the client to create their own FreeType FT_Face object.
With this the client can access any/all of the extended APIs in freetype,
including kerning.

[bungeman-skia, 2013-10-23 16:19:31]

https://codereview.chromium.org/29363009/diff/1/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

https://codereview.chromium.org/29363009/diff/1/include/core/SkTypeface.h#new...
include/core/SkTypeface.h:239: *  These values are in design units, which are
relative to the typeface's
Along the horizontal baseline? We don't have a 'vertical' bit on typeface, so
we'd have to assume something here. Of course, I'm actually for 'vertical' being
on the typeface (if metrics are part of a typeface).

[efidler1, 2013-10-25 12:24:33]

> 
> 1. Expose the low-level API in this CL, which would front for DirectWrite and
> FreeType, and just return false for GDI and CoreText. If the clients wants to
> perform ppem-specific rounding, that is trivial for them to do outside of
Skia.
> 
> 2. Make it trivial for the client to create their own FreeType FT_Face object.
> With this the client can access any/all of the extended APIs in freetype,
> including kerning.

Either of those are fine with me, although this API can't handle cross-stream
kerning.

[reed1, 2013-10-25 14:28:06]

On 2013/10/25 12:24:33, efidler1 wrote:
> > 
> > 1. Expose the low-level API in this CL, which would front for DirectWrite
and
> > FreeType, and just return false for GDI and CoreText. If the clients wants
to
> > perform ppem-specific rounding, that is trivial for them to do outside of
> Skia.
> > 
> > 2. Make it trivial for the client to create their own FreeType FT_Face
object.
> > With this the client can access any/all of the extended APIs in freetype,
> > including kerning.
> 
> Either of those are fine with me, although this API can't handle cross-stream
> kerning.

True. None of our font backends do today [freetype doesn't, even though they
have a 2D looking API]. If/when that becomes needed/supportable, we can relook
at the API.

[efidler1, 2013-10-25 14:34:21]

Ok. Do you want me to implement the FreeType backend for this interface, or do
you have the non-API parts of the code ready?

[reed1, 2013-10-25 14:44:15]

On 2013/10/25 14:34:21, efidler1 wrote:
> Ok. Do you want me to implement the FreeType backend for this interface, or do
> you have the non-API parts of the code ready?

Do you prefer this, or do you want direct access to the FT_Face object?

[efidler1, 2013-10-25 14:50:49]

This is probably better, since the Blink text code uses Skia interfaces. I'd
have to either only support FreeType or do multiple implementations upstream,
and then we'd be talking about exposing handles to other font engines as well.

[reed1, 2013-10-25 14:56:14]

On 2013/10/25 14:50:49, efidler1 wrote:
> This is probably better, since the Blink text code uses Skia interfaces. I'd
> have to either only support FreeType or do multiple implementations upstream,
> and then we'd be talking about exposing handles to other font engines as well.

OK, I'll proceed and add some backend code for this.

[efidler1, 2013-10-29 13:12:33]

any progress?

[reed1, 2013-10-29 19:29:59]

ptal

[bungeman-skia, 2013-10-29 21:38:41]

https://codereview.chromium.org/29363009/diff/160001/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

https://codereview.chromium.org/29363009/diff/160001/include/core/SkTypeface....
include/core/SkTypeface.h:233: *  parameter is not NULL, then it must have room
for at least count-1
This seems a bit odd. I would thing that if we wanted a magic set of values to
pass in to determine if we support kerning, it would be to pass NULL for glyphs,
(and imply that count should be 0 and adjustments NULL as well). This seems to
more ask the question "If I tried to get kerning information for no glyphs,
would that work?"

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
src/ports/SkFontHost_FreeType.cpp:735: FT_Face face = fta.face();
I think you want a test around FT_HAS_KERNING(face) here and return false if if
it evaluates to false.

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
src/ports/SkFontHost_FreeType.cpp:742: return false;
Hey, the documentation implies that adjustments won't be touched if this returns
false. The documentation should probably be tweaked to say that this will return
false also on failure.

[reed1, 2013-10-30 13:59:37]

ptal

https://codereview.chromium.org/29363009/diff/160001/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

https://codereview.chromium.org/29363009/diff/160001/include/core/SkTypeface....
include/core/SkTypeface.h:233: *  parameter is not NULL, then it must have room
for at least count-1
On 2013/10/29 21:38:41, bungeman1 wrote:
> This seems a bit odd. I would thing that if we wanted a magic set of values to
> pass in to determine if we support kerning, it would be to pass NULL for
glyphs,
> (and imply that count should be 0 and adjustments NULL as well). This seems to
> more ask the question "If I tried to get kerning information for no glyphs,
> would that work?"

Done.

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
src/ports/SkFontHost_FreeType.cpp:735: FT_Face face = fta.face();
On 2013/10/29 21:38:41, bungeman1 wrote:
> I think you want a test around FT_HAS_KERNING(face) here and return false if
if
> it evaluates to false.

Done.

https://codereview.chromium.org/29363009/diff/160001/src/ports/SkFontHost_Fre...
src/ports/SkFontHost_FreeType.cpp:742: return false;
On 2013/10/29 21:38:41, bungeman1 wrote:
> Hey, the documentation implies that adjustments won't be touched if this
returns
> false. The documentation should probably be tweaked to say that this will
return
> false also on failure.

Done.

[bungeman-skia, 2013-10-30 14:52:12]

https://codereview.chromium.org/29363009/diff/230001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/29363009/diff/230001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:238: if (count <= 1) {
I think this conditional should go away now that the doc has been updated.

[reed1, 2013-10-30 14:57:41]

ptal

https://codereview.chromium.org/29363009/diff/230001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/29363009/diff/230001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:238: if (count <= 1) {
On 2013/10/30 14:52:13, bungeman1 wrote:
> I think this conditional should go away now that the doc has been updated.

Done.

[bungeman-skia, 2013-10-30 15:05:56]

lgtm with spelling fixes

https://codereview.chromium.org/29363009/diff/290001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/29363009/diff/290001/src/core/SkTypeface.cpp#...
src/core/SkTypeface.cpp:239: // chedk for the only legal way to pass a NULL..
everthing is 0
s/chedk/check
s/everthing/everything

[reed1, 2013-10-30 15:07:08]

Committed patchset #5 manually as r12018 (presubmit successful).