Move distance field generation to the glyph cache

Move distance field generation to the glyph cache.

Distance fields are currently created in GrTextStrike, which is the wrong place. This moves that to the glyph cache where it belongs. 

As part of my testing, I found that when we fall back to paths in the GrDistanceFieldTextContext it was not scaling them properly, so that's fixed in here too.

Committed: http://code.google.com/p/skia/source/detail?r=14193

[jvanverth1, 2014-04-11 13:36:51]

Two notes:

This will create the mask and cache it as well. I'm not sure if that's the right
approach.

I didn't see any performance loss or gain from this.

[robertphillips, 2014-04-11 14:13:00]

lgtm + nits & questions

https://codereview.chromium.org/227593010/diff/140001/include/gpu/GrFontScaler.h
File include/gpu/GrFontScaler.h (right):

https://codereview.chromium.org/227593010/diff/140001/include/gpu/GrFontScale...
include/gpu/GrFontScaler.h:34: virtual bool
getPackedGlyphDFImage(GrGlyph::PackedID, int width, int height,
dfImage

https://codereview.chromium.org/227593010/diff/140001/include/gpu/SkGr.h
File include/gpu/SkGr.h (right):

https://codereview.chromium.org/227593010/diff/140001/include/gpu/SkGr.h#newc...
include/gpu/SkGr.h:88: int rowBytes, void* image);
override? up above too?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
File src/core/SkDistanceFieldGen.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.cpp:473: // around the outside
curSrcScanLine and curDstScanLine might be clearer

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
File src/core/SkDistanceFieldGen.h (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:11: 
Is this comment right? Seems like this would be max-sample-distance (or
something). Also isn't this half of something?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:26: *  @param distanceField     The distance field
to be generated. Should already be allocated
padding below?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:30: *  @param h                 Height of the
image.
rowBytes?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:42: *  @param h                 Height of the
image.
rowBytes?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.cpp
File src/core/SkGlyphCache.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:368: }
this->?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:370: // now generate the distance field
NULL != ?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:373: SkChunkAlloc::kReturnNil_AllocFailType);
NULL != ?

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:379: (unsigned char*)glyph.fImage,
overlength

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:385: (unsigned char*)glyph.fImage,
overlength

https://codereview.chromium.org/227593010/diff/140001/src/gpu/GrDistanceField...
File src/gpu/GrDistanceFieldTextContext.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/gpu/GrDistanceField...
src/gpu/GrDistanceFieldTextContext.cpp:243: ctm.setScale(fTextRatio,
fTextRatio);
seems like these should be tx & ty rather then sx & sy?

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler.cpp
File src/gpu/SkGrFontScaler.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:116: bool
SkGrFontScaler::getPackedGlyphDFBounds(GrGlyph::PackedID packed,
Align or fit on one line?

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:118: const SkGlyph& glyph =
fStrike->getGlyphIDMetrics(GrGlyph::UnpackID(packed),
align?

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:208: const SkGlyph& glyph =
fStrike->getGlyphIDMetrics(GrGlyph::UnpackID(packed),
align?

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:217: 
At a higher-level why are we copying out here?

[jvanverth1, 2014-04-11 17:54:14]

Nits addressed. I think I still need an OK from Mike before I can commit.

https://codereview.chromium.org/227593010/diff/140001/include/gpu/GrFontScaler.h
File include/gpu/GrFontScaler.h (right):

https://codereview.chromium.org/227593010/diff/140001/include/gpu/GrFontScale...
include/gpu/GrFontScaler.h:34: virtual bool
getPackedGlyphDFImage(GrGlyph::PackedID, int width, int height,
On 2014/04/11 14:13:00, robertphillips wrote:
> dfImage

Done.

https://codereview.chromium.org/227593010/diff/140001/include/gpu/SkGr.h
File include/gpu/SkGr.h (right):

https://codereview.chromium.org/227593010/diff/140001/include/gpu/SkGr.h#newc...
include/gpu/SkGr.h:88: int rowBytes, void* image);
On 2014/04/11 14:13:00, robertphillips wrote:
> override? up above too?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
File src/core/SkDistanceFieldGen.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.cpp:473: // around the outside
On 2014/04/11 14:13:00, robertphillips wrote:
> curSrcScanLine and curDstScanLine might be clearer

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
File src/core/SkDistanceFieldGen.h (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:11: 
On 2014/04/11 14:13:00, robertphillips wrote:
> Is this comment right? Seems like this would be max-sample-distance (or
> something). Also isn't this half of something?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:26: *  @param distanceField     The distance field
to be generated. Should already be allocated
On 2014/04/11 14:13:00, robertphillips wrote:
> padding below?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:30: *  @param h                 Height of the
image.
On 2014/04/11 14:13:00, robertphillips wrote:
> rowBytes?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkDistanceFiel...
src/core/SkDistanceFieldGen.h:42: *  @param h                 Height of the
image.
On 2014/04/11 14:13:00, robertphillips wrote:
> rowBytes?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.cpp
File src/core/SkGlyphCache.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:368: }
On 2014/04/11 14:13:00, robertphillips wrote:
> this->?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:370: // now generate the distance field
On 2014/04/11 14:13:00, robertphillips wrote:
> NULL != ?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:373: SkChunkAlloc::kReturnNil_AllocFailType);
On 2014/04/11 14:13:00, robertphillips wrote:
> NULL != ?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:379: (unsigned char*)glyph.fImage,
On 2014/04/11 14:13:00, robertphillips wrote:
> overlength

Done.

https://codereview.chromium.org/227593010/diff/140001/src/core/SkGlyphCache.c...
src/core/SkGlyphCache.cpp:385: (unsigned char*)glyph.fImage,
On 2014/04/11 14:13:00, robertphillips wrote:
> overlength

Done.

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler.cpp
File src/gpu/SkGrFontScaler.cpp (right):

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:116: bool
SkGrFontScaler::getPackedGlyphDFBounds(GrGlyph::PackedID packed,
On 2014/04/11 14:13:00, robertphillips wrote:
> Align or fit on one line?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:118: const SkGlyph& glyph =
fStrike->getGlyphIDMetrics(GrGlyph::UnpackID(packed),
On 2014/04/11 14:13:00, robertphillips wrote:
> align?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:208: const SkGlyph& glyph =
fStrike->getGlyphIDMetrics(GrGlyph::UnpackID(packed),
On 2014/04/11 14:13:00, robertphillips wrote:
> align?

Done.

https://codereview.chromium.org/227593010/diff/140001/src/gpu/SkGrFontScaler....
src/gpu/SkGrFontScaler.cpp:217: 
On 2014/04/11 14:13:00, robertphillips wrote:
> At a higher-level why are we copying out here?

The short answer is: to match getPackedGlyphImage. The long is that I noticed
this as well and the src pointer could be returned directly. The only issue is
that might make the cache less thread-friendly (i.e. what happens if you request
a distance field and another thread deletes it before you copy it to the
driver).

[jvanverth1, 2014-04-14 14:12:26]

Mike, can you take a look at this please?

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

Does GrFontScaler.h need to be public?

https://codereview.chromium.org/227593010/diff/170001/include/gpu/GrFontScaler.h
File include/gpu/GrFontScaler.h (right):

https://codereview.chromium.org/227593010/diff/170001/include/gpu/GrFontScale...
include/gpu/GrFontScaler.h:34: virtual bool
getPackedGlyphDFImage(GrGlyph::PackedID, int width, int height,
What does this guy return? Does it set dfImage to point to the pixels? Does it
copy the pixels?

[jvanverth1, 2014-04-14 17:58:42]

GrFontScaler is now a private interface. Also removed an unused parameter from
the getPackedGlyphDFImage() call.

https://codereview.chromium.org/227593010/diff/170001/include/gpu/GrFontScaler.h
File include/gpu/GrFontScaler.h (right):

https://codereview.chromium.org/227593010/diff/170001/include/gpu/GrFontScale...
include/gpu/GrFontScaler.h:34: virtual bool
getPackedGlyphDFImage(GrGlyph::PackedID, int width, int height,
On 2014/04/14 14:18:05, reed1 wrote:
> What does this guy return? Does it set dfImage to point to the pixels? Does it
> copy the pixels?

It generates distance field pixels from the image pixels, stores it in the
cache, and returns a pointer to the distance field pixels.

[jvanverth1, 2014-04-14 17:59:26]

On 2014/04/14 17:58:42, JimVV wrote:
> It generates distance field pixels from the image pixels, stores it in the
> cache, and returns a pointer to the distance field pixels.

More accurately: it only generates distance field pixels if they're not already
in the cache.

[reed1, 2014-04-14 19:43:02]

lgtm

https://codereview.chromium.org/227593010/diff/190001/src/gpu/GrFontScaler.h
File src/gpu/GrFontScaler.h (right):

https://codereview.chromium.org/227593010/diff/190001/src/gpu/GrFontScaler.h#...
src/gpu/GrFontScaler.h:36: virtual bool getPackedGlyphDFImage(GrGlyph::PackedID,
int width, int height,
ah, much clearer

[jvanverth1, 2014-04-14 20:33:51]

I had to roll back the change which puts GrFontScaler.h in the private
interface, since SkGr.h depends on it. It still might be possible to make it
private, but I suspect there will be more dependencies, so I think it should be
part of a separate change.

[reed1, 2014-04-14 20:38:30]

lgtm

[jvanverth1, 2014-04-14 21:35:04]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-04-14 21:35:24]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/227593010/210001

[commit-bot: I haz the power, 2014-04-14 22:06:15]

Change committed as 14193