Change GrTextContext fallbacks to be a linked list chain.

Change GrTextContext fallbacks to be a linked list chain.

Preliminary work for getting color emoji working with
distance field text.

BUG=skia:2887
Committed: https://skia.googlesource.com/skia/+/8c27a188a0b216021d439eb627622d17b9f78343

[jvanverth1, 2014-10-13 18:39:32]

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

[bsalomon, 2014-10-13 18:46:37]

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp#ne...
src/gpu/GrTextContext.cpp:21: SkDELETE(fFallbackTextContext);
Feels like GrTC should be ref counted.

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1697: textContext =
textContext->getFallbackTextContext();
Would it make sense for this to just happen within the text context? Maybe make
drawText() non-virtual with a virtual onDrawText() put the the fallback code on
drawText()?

[jvanverth1, 2014-10-13 18:54:48]

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp#ne...
src/gpu/GrTextContext.cpp:21: SkDELETE(fFallbackTextContext);
On 2014/10/13 18:46:36, bsalomon wrote:
> Feels like GrTC should be ref counted.

I suppose, but while it's created by GrContext, it's only ever used by the
parent. I could create the fallback in the constructors for
GrDistanceFieldTextContext and GrStencilAndCoverTextContext, but I wasn't sure
if that's considered good practice.

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1697: textContext =
textContext->getFallbackTextContext();
On 2014/10/13 18:46:36, bsalomon wrote:
> Would it make sense for this to just happen within the text context? Maybe
make
> drawText() non-virtual with a virtual onDrawText() put the the fallback code
on
> drawText()?

I thought of that, but you'd have to call canDraw() twice -- once to see if you
can draw at all (for the path fallback) and then again to see which your chain
to use. CanDraw() is  currently a little expensive on the DFF side, so I'm
trying to avoid that.

[bsalomon, 2014-10-13 19:01:44]

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp#ne...
src/gpu/GrTextContext.cpp:21: SkDELETE(fFallbackTextContext);
On 2014/10/13 18:54:48, jvanverth1 wrote:
> On 2014/10/13 18:46:36, bsalomon wrote:
> > Feels like GrTC should be ref counted.
> 
> I suppose, but while it's created by GrContext, it's only ever used by the
> parent. I could create the fallback in the constructors for
> GrDistanceFieldTextContext and GrStencilAndCoverTextContext, but I wasn't sure
> if that's considered good practice.

I'd say either ref count or do as you suggested. Having a setter that takes
ownership from the caller feels unnecessarily dangerous unless it is really
evident from the name or very well documented.

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1697: textContext =
textContext->getFallbackTextContext();
On 2014/10/13 18:54:48, jvanverth1 wrote:
> On 2014/10/13 18:46:36, bsalomon wrote:
> > Would it make sense for this to just happen within the text context? Maybe
> make
> > drawText() non-virtual with a virtual onDrawText() put the the fallback code
> on
> > drawText()?
> 
> I thought of that, but you'd have to call canDraw() twice -- once to see if
you
> can draw at all (for the path fallback) and then again to see which your chain
> to use. CanDraw() is  currently a little expensive on the DFF side, so I'm
> trying to avoid that.

I'm not sure I see why... 

GrTextContext::drawText() {
    GrTextContext* context = this;
    do {
        if (context->canDrawText()) {
           context->onDrawText();
           return;
        }
    } while (context = context->getFallback());
}

[jvanverth1, 2014-10-13 19:06:19]

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp
File src/gpu/GrTextContext.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/GrTextContext.cpp#ne...
src/gpu/GrTextContext.cpp:21: SkDELETE(fFallbackTextContext);
On 2014/10/13 19:01:44, bsalomon wrote:
> On 2014/10/13 18:54:48, jvanverth1 wrote:
> > On 2014/10/13 18:46:36, bsalomon wrote:
> > > Feels like GrTC should be ref counted.
> > 
> > I suppose, but while it's created by GrContext, it's only ever used by the
> > parent. I could create the fallback in the constructors for
> > GrDistanceFieldTextContext and GrStencilAndCoverTextContext, but I wasn't
sure
> > if that's considered good practice.
> 
> I'd say either ref count or do as you suggested. Having a setter that takes
> ownership from the caller feels unnecessarily dangerous unless it is really
> evident from the name or very well documented.

I'll add a factory function for each text context -- I think that'll take care
of any concerns.

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/650273003/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1697: textContext =
textContext->getFallbackTextContext();
On 2014/10/13 19:01:44, bsalomon wrote:
> On 2014/10/13 18:54:48, jvanverth1 wrote:
> > On 2014/10/13 18:46:36, bsalomon wrote:
> > > Would it make sense for this to just happen within the text context? Maybe
> > make
> > > drawText() non-virtual with a virtual onDrawText() put the the fallback
code
> > on
> > > drawText()?
> > 
> > I thought of that, but you'd have to call canDraw() twice -- once to see if
> you
> > can draw at all (for the path fallback) and then again to see which your
chain
> > to use. CanDraw() is  currently a little expensive on the DFF side, so I'm
> > trying to avoid that.
> 
> I'm not sure I see why... 
> 
> GrTextContext::drawText() {
>     GrTextContext* context = this;
>     do {
>         if (context->canDrawText()) {
>            context->onDrawText();
>            return;
>         }
>     } while (context = context->getFallback());
> }
> 

Got it. And GrTextContext::drawText can return false if all of them fail, and
SkGpuDevice will draw paths in that case.

[jvanverth1, 2014-10-14 15:10:33]

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

[jvanverth1, 2014-10-14 15:10:34]

Updated to include requested changes. PTAL.

[bsalomon, 2014-10-14 15:13:06]

lgtm

[jvanverth1, 2014-10-14 15:20:23]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-10-14 15:21:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/650273003/150001

[commit-bot: I haz the power, 2014-10-14 15:22:58]

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

[commit-bot: I haz the power, 2014-10-14 15:22:59]

Try jobs failed on following builders:
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-Cl...)

[jvanverth1, 2014-10-14 15:34:05]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-10-14 15:34:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/650273003/170001

[commit-bot: I haz the power, 2014-10-14 15:45:56]

Committed patchset #3 (id:170001) as 8c27a188a0b216021d439eb627622d17b9f78343