Use per-typeface sets of glyphs for nvpr text

Uses a single pre-baked set of paths for drawing nvpr text of a given
typeface. Loads the paths using the driver's glyph loading routines.

Refactors GrPathRange to accept a PathGenerator class that it uses to
lazily initialize its paths. The client code is no longer expected to
initialize the paths in a GrPathRange; instead it must provide a
PathGenerator* instance to createPathRange().

Adds a new createGlyphs() method to GrPathRendering that creates a
range of glyph paths, indexed by glyph id. GrPathRendering implements
createGlyphs() with a PathGenerator that loads glyph paths using the
skia frameworks. GrGLPathRendering uses glMemoryGlyphIndexArrayNV()
instead, when possible, to load the glyph paths.

Removes all GlyphPathRange logic from GrStencilAndCoverTextContext.
It instead uses createGlyphs().

BUG=skia:2939
Committed: https://skia.googlesource.com/skia/+/855d83ff79c6c822b2ad653f2f890178ad0f637b

[Chris Dalton, 2014-09-12 22:18:41]

cdalton@nvidia.com changed reviewers:
+ bsalomon@google.com

[bsalomon, 2014-09-15 14:16:20]

bsalomon@google.com changed reviewers:
+ jvanverth@google.com

[bsalomon, 2014-09-15 14:16:21]

Some questions, adding Jim who knows the text code better than I do.

https://codereview.chromium.org/563283004/diff/1/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/563283004/diff/1/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:1071: * Creates a range of glyph paths, indexed by glyph
id. The glyphs will have an
Does this have to go here or can it go in the text context?

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrGpu.h
File src/gpu/GrGpu.h (right):

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrGpu.h#newcode154
src/gpu/GrGpu.h:154: * Creates a range of glyph paths, indexed by glyph id. The
glyphs will have an
I'm wondering if the PR class should be accessible to the text context to avoid
having a bunch of shims on GrGpu.

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h
File src/gpu/GrPathRange.h (right):

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h#newcode42
src/gpu/GrPathRange.h:42: class PathGenerator {
Since this object has a semi-interesting lifetime (ownership gets transferred
from the code that allocated it to the path range class), I think it makes sense
for it to be ref counted.

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h#newcode67
src/gpu/GrPathRange.h:67: uint32_t getNumPaths() const { return fNumPaths; }
tiny nit: we usually use (signed) int for counts.

[Chris Dalton, 2014-09-15 21:05:25]

https://codereview.chromium.org/563283004/diff/1/include/gpu/GrContext.h
File include/gpu/GrContext.h (right):

https://codereview.chromium.org/563283004/diff/1/include/gpu/GrContext.h#newc...
include/gpu/GrContext.h:1071: * Creates a range of glyph paths, indexed by glyph
id. The glyphs will have an
On 2014/09/15 14:16:21, bsalomon wrote:
> Does this have to go here or can it go in the text context?

I put it here because I thought it ought to follow the same pattern as
createPath(), but it could also go in the text context if you think that makes
more sense.

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrGpu.h
File src/gpu/GrGpu.h (right):

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrGpu.h#newcode154
src/gpu/GrGpu.h:154: * Creates a range of glyph paths, indexed by glyph id. The
glyphs will have an
On 2014/09/15 14:16:21, bsalomon wrote:
> I'm wondering if the PR class should be accessible to the text context to
avoid
> having a bunch of shims on GrGpu.

That could work, but I think createPath() ought to follow the same pattern.
Should we change the behavior for that one too? What about having the
createPath/createGlyphs methods on the GrContext? Should we take both out of
there as well?

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h
File src/gpu/GrPathRange.h (right):

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h#newcode42
src/gpu/GrPathRange.h:42: class PathGenerator {
On 2014/09/15 14:16:21, bsalomon wrote:
> Since this object has a semi-interesting lifetime (ownership gets transferred
> from the code that allocated it to the path range class), I think it makes
sense
> for it to be ref counted.

Agreed. I was actually wondering myself what the standard procedure was for this
type of object.

https://codereview.chromium.org/563283004/diff/1/src/gpu/GrPathRange.h#newcode67
src/gpu/GrPathRange.h:67: uint32_t getNumPaths() const { return fNumPaths; }
On 2014/09/15 14:16:21, bsalomon wrote:
> tiny nit: we usually use (signed) int for counts.

Acknowledged.

[bsalomon, 2014-09-17 20:57:29]

lgtm but, Jim, can you review the new code in
src/gpu/GrStencilAndCoverTextContext.cpp?

https://codereview.chromium.org/563283004/diff/40001/src/gpu/GrStencilAndCove...
File src/gpu/GrStencilAndCoverTextContext.h (right):

https://codereview.chromium.org/563283004/diff/40001/src/gpu/GrStencilAndCove...
src/gpu/GrStencilAndCoverTextContext.h:43: enum RenderMode {
Maybe some comment here on the nature of the shortcuts taken in kMaxPerf?

[Chris Dalton, 2014-09-17 22:22:57]

https://codereview.chromium.org/563283004/diff/40001/src/gpu/GrStencilAndCove...
File src/gpu/GrStencilAndCoverTextContext.h (right):

https://codereview.chromium.org/563283004/diff/40001/src/gpu/GrStencilAndCove...
src/gpu/GrStencilAndCoverTextContext.h:43: enum RenderMode {
On 2014/09/17 20:57:29, bsalomon wrote:
> Maybe some comment here on the nature of the shortcuts taken in kMaxPerf?

Done.

[jvanverth1, 2014-09-18 19:21:17]

lgtm

[Chris Dalton, 2014-09-18 19:28:36]

The CQ bit was checked by cdalton@nvidia.com

[commit-bot: I haz the power, 2014-09-18 19:29:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/563283004/60001

[commit-bot: I haz the power, 2014-09-18 19:36:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-18 19:36:01]

Try jobs failed on following builders:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Mac10.8-Clang-x86_64-Release-Trybo...)

[Chris Dalton, 2014-09-18 19:48:32]

The CQ bit was checked by cdalton@nvidia.com

[commit-bot: I haz the power, 2014-09-18 19:49:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/563283004/80001

[commit-bot: I haz the power, 2014-09-18 20:51:57]

Committed patchset #5 (id:80001) as 855d83ff79c6c822b2ad653f2f890178ad0f637b

[reed1, 2014-09-19 18:06:32]

reed@google.com changed reviewers:
+ bungeman@google.com, reed@google.com

[reed1, 2014-09-19 18:06:33]

Nervous about loading entire fonts into ram (and then driver), esp. when CJK
fonts rarely use more than 10-20% of their available glyphs...