impl charsToGlyphs for freetype

impl charsToGlyphs for freetype

BUG=
R=bungeman@google.com

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

[reed1, 2013-07-02 15:27:58]

will disable calling the cmap test before checkin (since win not impl yet)

[bungeman-skia, 2013-07-02 16:04:20]

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType.cpp:1396: int SkTypeface_FreeType::onCountGlyphs()
const {
Is this intended to replace SkScalerContext::generateGlyphCount? If so, it seems
unfortunate that all of these accessors have so much overhead. Should the
SkTypeface_FreeType keep a canonical sized SkFaceRec to avoid this on each call
as the scaler context does?

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType_common.h (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType_common.h:63: virtual int onCharsToGlyphs(const
void* chars, Encoding, uint16_t glyphs[],
Adding declarations to common, but putting implementation only in the
non-common. I don't think SkFontHost_FreeType_common.cpp will compile with this
change. Should the impl go in common.cpp?

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_fontconf...
File src/ports/SkFontHost_fontconfig.cpp (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_fontconf...
src/ports/SkFontHost_fontconfig.cpp:200: 
?? just two extra lines?

[reed1, 2013-07-02 16:14:37]

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType.cpp:1396: int SkTypeface_FreeType::onCountGlyphs()
const {
On 2013/07/02 16:04:20, bungeman1 wrote:
> Is this intended to replace SkScalerContext::generateGlyphCount? If so, it
seems
> unfortunate that all of these accessors have so much overhead. Should the
> SkTypeface_FreeType keep a canonical sized SkFaceRec to avoid this on each
call
> as the scaler context does?

Not sure I follow : I'm extracting a cached FT_Face from a global cache, which
is size-independent. The only "key" is the typeface object.

I definitely think we can rethink older scalercontext apis if these new typeface
ones prove to be as fast or faster (e.g. glyph-count, char2glyph)

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType_common.h (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType_common.h:63: virtual int onCharsToGlyphs(const
void* chars, Encoding, uint16_t glyphs[],
On 2013/07/02 16:04:20, bungeman1 wrote:
> Adding declarations to common, but putting implementation only in the
> non-common. I don't think SkFontHost_FreeType_common.cpp will compile with
this
> change. Should the impl go in common.cpp?

Doh! I didn't notice that disconnect. I will fix it one way or the other.

[reed1, 2013-07-02 16:19:18]

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType_common.h (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType_common.h:63: virtual int onCharsToGlyphs(const
void* chars, Encoding, uint16_t glyphs[],
On 2013/07/02 16:14:37, reed1 wrote:
> On 2013/07/02 16:04:20, bungeman1 wrote:
> > Adding declarations to common, but putting implementation only in the
> > non-common. I don't think SkFontHost_FreeType_common.cpp will compile with
> this
> > change. Should the impl go in common.cpp?
> 
> Doh! I didn't notice that disconnect. I will fix it one way or the other.

Actually, this is already happening (e.g. onGetUPEM), so I'm not sure what is
right or wrong. I may leave it as is for now, since it does not seem to be worse
than before. Separately I will explore this separation with whomever wanted it
(mozilla?)

[bungeman-skia, 2013-07-02 16:25:31]

> Actually, this is already happening (e.g. onGetUPEM), so I'm not sure what is
> right or wrong. I may leave it as is for now, since it does not seem to be
worse
> than before. Separately I will explore this separation with whomever wanted it
> (mozilla?)

Heh, yes, I just noticed this. The split here is just for mozilla and frankly,
there isn't much in common except the generateImage (effectively doing the same
as Blink for Windows, just using Skia to stamp the glyph masks out).

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
File src/ports/SkFontHost_FreeType.cpp (right):

https://codereview.chromium.org/18040004/diff/1/src/ports/SkFontHost_FreeType...
src/ports/SkFontHost_FreeType.cpp:1396: int SkTypeface_FreeType::onCountGlyphs()
const {
On 2013/07/02 16:14:37, reed1 wrote:
> On 2013/07/02 16:04:20, bungeman1 wrote:
> > Is this intended to replace SkScalerContext::generateGlyphCount? If so, it
> seems
> > unfortunate that all of these accessors have so much overhead. Should the
> > SkTypeface_FreeType keep a canonical sized SkFaceRec to avoid this on each
> call
> > as the scaler context does?
> 
> Not sure I follow : I'm extracting a cached FT_Face from a global cache, which
> is size-independent. The only "key" is the typeface object.
> 
> I definitely think we can rethink older scalercontext apis if these new
typeface
> ones prove to be as fast or faster (e.g. glyph-count, char2glyph)

I suppose I'm just pointing out that the entirety of generateGlyphCount is just
'return fFace->num_glyphs' since all the lookup has been done.

All of these typeface level methods make me uneasy since I like the idea that
different strikes may have different glyphs. I suppose if that were the case we
could just call them separate typefaces (since they would be, effectively).

[bungeman-skia, 2013-07-02 16:26:36]

lgtm

[reed1, 2013-07-02 16:32:32]

Committed patchset #4 manually as r9866 (presubmit successful).