start a wrapper for color fonts

start a wrapper for color fonts

BUG=
R=bungeman@google.com

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

[bungeman-skia, 2013-05-29 21:21:14]

I took a look at it. It is impressive that this is now possible and also so
little code.

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp
File gm/colortype.cpp (right):

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp#newcode14
gm/colortype.cpp:14: SkTypeface* fColorType;
Any reason this isn't in the end private section?

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp#newcode17
gm/colortype.cpp:17: SkColor colors[] = { SK_ColorRED, SK_ColorGREEN,
SK_ColorBLUE, SK_ColorMAGENTA, SK_ColorCYAN, SK_ColorYELLOW };
My, what a long line you have.

https://codereview.chromium.org/14890016/diff/7001/gyp/utils.gyp
File gyp/utils.gyp (right):

https://codereview.chromium.org/14890016/diff/7001/gyp/utils.gyp#newcode116
gyp/utils.gyp:116: '../src/fonts/SkGScalerContext.cpp',
A new fonts directory? It is interesting that this isn't a port (it just relies
on the ports) like all the other scaler contexts.

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

https://codereview.chromium.org/14890016/diff/7001/include/core/SkTypeface.h#...
include/core/SkTypeface.h:207: void filterRec(SkScalerContextRec* rec) const {
Do these two want to be public? I shudder at exposing even the name
ScalerContext, let alone SkScalerContextRec.

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
File src/fonts/SkGScalerContext.cpp (right):

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:1: 
Copyright?

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:19: virtual void
generateFontMetrics(SkPaint::FontMetrics* mX,
Fix it with fire!!! (Ok, so that's a different change.)

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:206: class SkGFont : public SkRefCnt {
I suppose this is still under construction, but what is the intended difference
between this and SkTypeface?

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext.h
File src/fonts/SkGScalerContext.h (right):

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.h:1: 
Copyright?

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.h:22: SkAdvancedTypefaceMetrics::PerGlyphInfo,
These lines are also rather long.

[reed1, 2013-05-30 16:04:29]

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp
File gm/colortype.cpp (right):

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp#newcode14
gm/colortype.cpp:14: SkTypeface* fColorType;
On 2013/05/29 21:21:14, bungeman1 wrote:
> Any reason this isn't in the end private section?

Done.

https://codereview.chromium.org/14890016/diff/7001/gm/colortype.cpp#newcode17
gm/colortype.cpp:17: SkColor colors[] = { SK_ColorRED, SK_ColorGREEN,
SK_ColorBLUE, SK_ColorMAGENTA, SK_ColorCYAN, SK_ColorYELLOW };
On 2013/05/29 21:21:14, bungeman1 wrote:
> My, what a long line you have.

Done.

https://codereview.chromium.org/14890016/diff/7001/gyp/utils.gyp
File gyp/utils.gyp (right):

https://codereview.chromium.org/14890016/diff/7001/gyp/utils.gyp#newcode116
gyp/utils.gyp:116: '../src/fonts/SkGScalerContext.cpp',
On 2013/05/29 21:21:14, bungeman1 wrote:
> A new fonts directory? It is interesting that this isn't a port (it just
relies
> on the ports) like all the other scaler contexts.

Yea, I can move it, though I think 'ports' has gotten awfully big lately. Also,
we have some 'ports' stuff in utils/... as well, so I'm no longer sure I know
exactly where what goes. I thought fonts/ might could expand to hold lots of our
font files, just to segregate them for clarity.

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

https://codereview.chromium.org/14890016/diff/7001/include/core/SkTypeface.h#...
include/core/SkTypeface.h:207: void filterRec(SkScalerContextRec* rec) const {
On 2013/05/29 21:21:14, bungeman1 wrote:
> Do these two want to be public? I shudder at exposing even the name
> ScalerContext, let alone SkScalerContextRec.

I don't know how else to allow one typeface to call another w/o expose these. I
am very open to suggestions for an alternate mechanism.

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
File src/fonts/SkGScalerContext.cpp (right):

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:1: 
On 2013/05/29 21:21:14, bungeman1 wrote:
> Copyright?

Done.

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:19: virtual void
generateFontMetrics(SkPaint::FontMetrics* mX,
On 2013/05/29 21:21:14, bungeman1 wrote:
> Fix it with fire!!! (Ok, so that's a different change.)

Agreed.

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.cpp:206: class SkGFont : public SkRefCnt {
On 2013/05/29 21:21:14, bungeman1 wrote:
> I suppose this is still under construction, but what is the intended
difference
> between this and SkTypeface?

Added comment.

I *think* one would construct a SkGFont (or some other name) to hold the
glyphs/etc., and then instantiate a typeface (which is immutable) from a
snapshot of the GFont (which is mutable).

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext.h
File src/fonts/SkGScalerContext.h (right):

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.h:1: 
On 2013/05/29 21:21:14, bungeman1 wrote:
> Copyright?

Done.

https://codereview.chromium.org/14890016/diff/7001/src/fonts/SkGScalerContext...
src/fonts/SkGScalerContext.h:22: SkAdvancedTypefaceMetrics::PerGlyphInfo,
On 2013/05/29 21:21:14, bungeman1 wrote:
> These lines are also rather long.

Done.

[bungeman-skia, 2013-05-31 14:24:18]

lgtm (looks ok to me?) with comments on public, but not really public, methods.

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

https://codereview.chromium.org/14890016/diff/14001/include/core/SkTypeface.h...
include/core/SkTypeface.h:204: SkScalerContext* createScalerContext(const
SkDescriptor*) const;
For the moment I don't have an issue with these being here, and I suppose that
no one can actually use them since they take types which no one outside of Skia
should be able to create. However, there should be a comment on
createScalerContext, filterRec, and getFontDescriptor that these are internal
and that they may go away (if we can figure out a good way to hide them.

[reed1, 2013-05-31 20:18:05]

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

[robertphillips, 2013-06-02 14:58:06]

This GM appears to be crashing on Linux:

#0  0x00000000005210c4 in SkTypeface::style (this=0x0) at
../include/core/SkTypeface.h:53
#1  0x0000000000520cda in SkGTypeface::SkGTypeface (this=0x6f01780, proxy=0x0,
paint=...) at ../src/fonts/SkGScalerContext.cpp:166
#2  0x000000000042f3db in ColorTypeGM::ColorTypeGM (this=0x4c11a60) at
../gm/colortype.cpp:32
#3  0x000000000042f213 in F_70 () at ../gm/colortype.cpp:70
#4  0x00000000004174d6 in Iter::next (this=0x7fffffffdd60) at
../gm/gmmain.cpp:104
#5  0x0000000000415422 in tool_main (argc=13, argv=0x7fffffffdf48) at
../gm/gmmain.cpp:1992
#6  0x0000000000415b4b in main (argc=13, argv=0x7fffffffdf48) at
../gm/gmmain.cpp:2093

(gdb) p this
$1 = (const SkTypeface * const) 0x0