Make GrTextContext be owned by the GrDrawContext

Make GrTextContext be owned by the GrDrawContext

This CL makes the GrTextContext be owned (and hidden) by the GrDrawContext. This funnels all the drawText* calls through the GrDrawContext and hides the (dispreferred) GrPipelineBuilder drawText variant. 

Some consequences of this are:

GrDrawContext now has to get the text drawing settings (i.e., SkDeviceProperties & useDFT). This means that we need a separate GrDrawContext for each combination of pixel geometry and DFT-use.

All the GrTextContext-derived classes now get a back pointer to the originating GrDrawContext so their method calls no longer take one.

Committed: https://skia.googlesource.com/skia/+/5b16e740fe6ab6d679083d06f07651602265081b

Committed: https://skia.googlesource.com/skia/+/2334fb655f8d4ef5915770d32bf845c88d3627f4

[robertphillips, 2015-06-09 13:46:11]

robertphillips@google.com changed reviewers:
+ joshualitt@chromium.org, joshualitt@google.com, jvanverth@google.com,
reed@google.com

[joshualitt, 2015-06-09 17:10:02]

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

https://codereview.chromium.org/1175553002/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:83: for (int j = 0; j < 2; ++j) {
magic numbers :(

[robertphillips, 2015-06-09 17:26:40]

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

https://codereview.chromium.org/1175553002/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:83: for (int j = 0; j < 2; ++j) {
On 2015/06/09 17:10:02, joshualitt wrote:
> magic numbers :(

Done.

[joshualitt, 2015-06-09 18:05:32]

On 2015/06/09 17:26:40, robertphillips wrote:
> https://codereview.chromium.org/1175553002/diff/1/src/gpu/GrContext.cpp
> File src/gpu/GrContext.cpp (right):
> 
>
https://codereview.chromium.org/1175553002/diff/1/src/gpu/GrContext.cpp#newco...
> src/gpu/GrContext.cpp:83: for (int j = 0; j < 2; ++j) {
> On 2015/06/09 17:10:02, joshualitt wrote:
> > magic numbers :(
> 
> Done.

lgtm

[jvanverth1, 2015-06-09 19:22:38]

Just one comment from me.

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

https://codereview.chromium.org/1175553002/diff/20001/include/gpu/GrContext.h...
include/gpu/GrContext.h:420: static const int kNumContextFlavors = 5;
It's not clear that these "flavors" are pixel geometry.

[robertphillips, 2015-06-09 19:26:20]

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

https://codereview.chromium.org/1175553002/diff/20001/include/gpu/GrContext.h...
include/gpu/GrContext.h:420: static const int kNumContextFlavors = 5;
On 2015/06/09 19:22:38, jvanverth1 wrote:
> It's not clear that these "flavors" are pixel geometry.

I agree - should be fixed in PS #3.

[robertphillips, 2015-06-10 14:19:42]

Still need API review (Brian, Mike)

[robertphillips, 2015-06-10 14:29:18]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[robertphillips, 2015-06-10 14:29:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1175553002/#ps40001
(title: "Add some dox")

[commit-bot: I haz the power, 2015-06-10 14:29:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175553002/40001

[commit-bot: I haz the power, 2015-06-10 14:30:22]

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

[commit-bot: I haz the power, 2015-06-10 14:30:23]

Dry run: Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[robertphillips, 2015-06-10 15:06:08]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[robertphillips, 2015-06-10 15:06:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1175553002/#ps60001
(title: "update to ToT")

[commit-bot: I haz the power, 2015-06-10 15:06:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175553002/60001

[reed1, 2015-06-10 15:10:44]

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

[reed1, 2015-06-10 15:10:45]

defers to brian

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

https://codereview.chromium.org/1175553002/diff/60001/include/gpu/GrContext.h...
include/gpu/GrContext.h:175: * Returns a helper object to orchestrate draws.
I presume there is no ownership change w/ this? i.e. the caller doesn't need to
delete or unref anything?

[commit-bot: I haz the power, 2015-06-10 15:11:23]

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

[commit-bot: I haz the power, 2015-06-10 15:11:24]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-06-10 15:17:44]

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

https://codereview.chromium.org/1175553002/diff/60001/include/gpu/GrContext.h...
include/gpu/GrContext.h:175: * Returns a helper object to orchestrate draws.
On 2015/06/10 15:10:45, reed1 wrote:
> I presume there is no ownership change w/ this? i.e. the caller doesn't need
to
> delete or unref anything?

Done - added some comments.

[bsalomon, 2015-06-11 01:47:57]

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

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrContext.h...
include/gpu/GrContext.h:179: * @param  devProps the device properties (mainly
defines text drawing)
devprops doesn't already have a DF flag? I guess I'm thinking of surfaceprops.
Should it?

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
File include/gpu/GrDrawContext.h (right):

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:50: void drawText(GrRenderTarget*, const GrClip&, 
const GrPaint&, const SkPaint&,
Can we have a TODO (here or a bug) to remove the need for SkPaint? It seems
weird that we pass both paints. It'd be cleaner to extract the text params from
SkPaint and not pass the whole thing (equivalent to GrStrokeInfo).

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:271: GrTextContext*      fTextContext; // lazily
created
would we share these in the future when there is a DC per RT? Curious cuz the DC
params from TC draw methods are removed.

https://codereview.chromium.org/1175553002/diff/80001/src/gpu/GrTextContext.h
File src/gpu/GrTextContext.h (right):

https://codereview.chromium.org/1175553002/diff/80001/src/gpu/GrTextContext.h...
src/gpu/GrTextContext.h:60: GrTextContext(GrContext*, GrDrawContext*, const
SkDeviceProperties&, bool useDFT);
Seems odd for the base class to know about DF. Is it really needed?

[robertphillips, 2015-06-16 16:52:37]

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

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrContext.h...
include/gpu/GrContext.h:179: * @param  devProps the device properties (mainly
defines text drawing)
I think that would be reasonable.

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
File include/gpu/GrDrawContext.h (right):

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:50: void drawText(GrRenderTarget*, const GrClip&, 
const GrPaint&, const SkPaint&,
On 2015/06/11 01:47:57, bsalomon wrote:
> Can we have a TODO (here or a bug) to remove the need for SkPaint? It seems
> weird that we pass both paints. It'd be cleaner to extract the text params
from
> SkPaint and not pass the whole thing (equivalent to GrStrokeInfo).

Done.

https://codereview.chromium.org/1175553002/diff/80001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:271: GrTextContext*      fTextContext; // lazily
created
I think the TextContexts will have to morph a bit when we have a DC per RT. It
seems like we would want a portion that is specific to each DC (that, say, hold
the glyph bitmaps that need to be uploaded to flush the DC) and a portion that
is shared (which, say, manage the caches).

[bsalomon, 2015-06-16 19:04:57]

lgtm

[robertphillips, 2015-06-16 19:15:58]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[robertphillips, 2015-06-16 19:15:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1175553002/#ps120001
(title: "Address code review comments")

[commit-bot: I haz the power, 2015-06-16 19:16:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175553002/120001

[commit-bot: I haz the power, 2015-06-16 19:21:54]

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

[commit-bot: I haz the power, 2015-06-16 19:21:55]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-06-16 19:22:27]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-06-16 19:22:58]

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

[commit-bot: I haz the power, 2015-06-16 19:23:54]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/5b16e740fe6ab6d679083d06f07651602265081b

[bsalomon, 2015-06-16 22:02:45]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1178383003/ by bsalomon@google.com.

The reason for reverting is: Breaking
Test-Win8-MSVC-ShuttleA-GPU-GTX660-x86_64-Debug ?

https://build.chromium.org/p/client.skia/builders/Test-Win8-MSVC-ShuttleA-GPU...
.

[robertphillips, 2015-06-17 12:31:14]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[robertphillips, 2015-06-17 12:31:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1175553002/#ps140001
(title: "Fix nvpr config")

[commit-bot: I haz the power, 2015-06-17 12:31:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175553002/140001

[commit-bot: I haz the power, 2015-06-17 12:36:49]

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

[commit-bot: I haz the power, 2015-06-17 12:36:51]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-06-17 12:42:44]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-06-17 12:42:55]

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

[commit-bot: I haz the power, 2015-06-17 12:43:38]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/2334fb655f8d4ef5915770d32bf845c88d3627f4