Split drawing functionality out of GrContext and into new GrDrawContext

Split drawing functionality out of GrContext and into new GrDrawContext

This is mainly a mechanical CL. There were some fiddly bits in GrContext.cpp where it no longer had access to the GrDrawTarget (and had to use the new GrDrawContext).

I've converted GrAARectRenderer & GrOvalRenderer into static classes so I could stop allocating them.

Committed: https://skia.googlesource.com/skia/+/ea4615034498aca2f9ca1753fb9a1ef10508d8cc

[robertphillips, 2015-05-22 19:48:56]

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

[bsalomon, 2015-05-22 20:33:58]

Seems like a nice step forward. Can see how this will evolve to multiple
drawcontexts.

Q: longer term does DrawTarget go away? Seems like it is being eroded on both
sides and losing its importance. It's API is narrowing and it's implementation
is getting thin. Don't need an answer now, but we should think about what the
layering should look like (currently something like drawcontext, drawtarget,
commandbuilder, targetcommands, batchtarget, gpu)

Don't love the Mgr abbrevation.

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkBlurMaskFilte...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkBlurMaskFilte...
src/effects/SkBlurMaskFilter.cpp:1167:
drawContext->drawNonAARectWithLocalMatrix(rt, clip, *grp, SkMatrix::I(),
proxy_rect, inverse);
wrap?

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkGpuBlurUtils.cpp
File src/effects/SkGpuBlurUtils.cpp (right):

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkGpuBlurUtils....
src/effects/SkGpuBlurUtils.cpp:66: GrRenderTarget* rt,
could a next evolution GrDrawContext() include the render target in the drawing
context?

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:62: fContext = context;
Do we need to remember this context? I'm working hard to get rid of as many back
ptrs to context as possible. Soon GrDrawTarget won't know it's owning GrContext.

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:629: if (NULL == dst->asRenderTarget()) {
!dst->asRenderTarget()

?

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:25: ~AutoCheckFlush() { fContext->flushIfNecessary();
}
Is it possible for the flushing to be moved to the DrawContext or Manager?
Trying to categorize why we need a GrContext in GrDC:

1) autocheckflush

2) debug markers (die!)

3) Getting path renderers (can the chain be passed down here?)

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:135:
GR_CREATE_TRACE_MARKER_CONTEXT("GrDrawContext::drawPaint", fContext);
Let's delete these

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:1: /*
does this file need to be in include/gpu for implementing third party skia
effect classes?

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:15: class GrAARectRenderer;
need all this?

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:20: class GrOvalRenderer;
or this?

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:49: void drawText(GrPipelineBuilder* pipelineBuilder,
GrBatch* batch);
e-gads! pipelinebuilders have escaped the zoo!

[bsalomon, 2015-05-22 20:52:38]

Oops.. I launched a trybot on the wrong CL. I guess we'll find out how the Link
likes this CL.

[robertphillips, 2015-05-26 16:12:59]

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkBlurMaskFilte...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkBlurMaskFilte...
src/effects/SkBlurMaskFilter.cpp:1167:
drawContext->drawNonAARectWithLocalMatrix(rt, clip, *grp, SkMatrix::I(),
proxy_rect, inverse);
On 2015/05/22 20:33:57, bsalomon wrote:
> wrap?

Done.

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkGpuBlurUtils.cpp
File src/effects/SkGpuBlurUtils.cpp (right):

https://codereview.chromium.org/1151283004/diff/1/src/effects/SkGpuBlurUtils....
src/effects/SkGpuBlurUtils.cpp:66: GrRenderTarget* rt,
On 2015/05/22 20:33:57, bsalomon wrote:
> could a next evolution GrDrawContext() include the render target in the
drawing
> context?

Right - in the future each GrDrawContext will hold the GrRenderTarget it wraps
so only the GrDrawContext will be handed in here (and GrContext::drawContext
will take a GrRenderTarget* parameter).

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:62: fContext = context;
On 2015/05/22 20:33:57, bsalomon wrote:
> Do we need to remember this context? I'm working hard to get rid of as many
back
> ptrs to context as possible. Soon GrDrawTarget won't know it's owning
GrContext.

Done.

Note that the GrDrawContext has its GrContext pointer to:
1) Allow use of AutoCheckFlush
2) Support trace markers
3) Query the path renderers in internalDrawPath

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:629: if (NULL == dst->asRenderTarget()) {
On 2015/05/22 20:33:58, bsalomon wrote:
> !dst->asRenderTarget()
> 
> ?

Done.

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:25: ~AutoCheckFlush() { fContext->flushIfNecessary();
}
> 1) autocheckflush

I think this check could go away when we're in a world where no resources are
actually being allocated during the assembly of the drawing commands.
 
> 2) debug markers (die!)

I'm slowly removing the debug marker stuff as I go.
 
> 3) Getting path renderers (can the chain be passed down here?)

This one we should probably discuss. Its two user are the GrClipMaskManager &
the GrDrawContext. It could be we could funnel everything through the
GrDrawContext and then move the path chain into it.

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:135:
GR_CREATE_TRACE_MARKER_CONTEXT("GrDrawContext::drawPaint", fContext);
On 2015/05/22 20:33:58, bsalomon wrote:
> Let's delete these

Done.

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

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:1: /*
On 2015/05/22 20:33:58, bsalomon wrote:
> does this file need to be in include/gpu for implementing third party skia
> effect classes?

Done. :(

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:15: class GrAARectRenderer;
On 2015/05/22 20:33:58, bsalomon wrote:
> need all this?

Done.

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:20: class GrOvalRenderer;
On 2015/05/22 20:33:58, bsalomon wrote:
> or this?

Done.

https://codereview.chromium.org/1151283004/diff/1/src/gpu/GrDrawContext.h#new...
src/gpu/GrDrawContext.h:49: void drawText(GrPipelineBuilder* pipelineBuilder,
GrBatch* batch);
On 2015/05/22 20:33:58, bsalomon wrote:
> e-gads! pipelinebuilders have escaped the zoo!

Short term only.

[joshualitt, 2015-05-26 16:25:13]

nit, lgtm

https://codereview.chromium.org/1151283004/diff/20001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1151283004/diff/20001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:430: GrDrawContext* drawContext =
fContext->drawContext();
Might want to do this higher up because there is no sense in generating the
cache textblobs if we can't flush them.

[robertphillips, 2015-05-26 16:46:22]

https://codereview.chromium.org/1151283004/diff/20001/src/gpu/GrAtlasTextCont...
File src/gpu/GrAtlasTextContext.cpp (right):

https://codereview.chromium.org/1151283004/diff/20001/src/gpu/GrAtlasTextCont...
src/gpu/GrAtlasTextContext.cpp:430: GrDrawContext* drawContext =
fContext->drawContext();
On 2015/05/26 16:25:13, joshualitt wrote:
> Might want to do this higher up because there is no sense in generating the
> cache textblobs if we can't flush them.

Done.

[robertphillips, 2015-05-26 16:46:37]

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

[robertphillips, 2015-05-26 16:46:38]

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/1151283004/#ps40001
(title: "Address code review comments")

[commit-bot: I haz the power, 2015-05-26 16:46:45]

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

[commit-bot: I haz the power, 2015-05-26 16:47:49]

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

[commit-bot: I haz the power, 2015-05-26 16:47:50]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[robertphillips, 2015-05-26 17:17:17]

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

[robertphillips, 2015-05-26 17:17:18]

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/1151283004/#ps60001
(title: "update to ToT")

[commit-bot: I haz the power, 2015-05-26 17:17:35]

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

[commit-bot: I haz the power, 2015-05-26 17:18:42]

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

[commit-bot: I haz the power, 2015-05-26 17:18:43]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[robertphillips, 2015-05-26 17:40:54]

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

[robertphillips, 2015-05-26 17:40:55]

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/1151283004/#ps80001
(title: "Patch up GrDrawContext.h for its new home in include\gpu")

[commit-bot: I haz the power, 2015-05-26 17:41:17]

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

[commit-bot: I haz the power, 2015-05-26 17:42:51]

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

[commit-bot: I haz the power, 2015-05-26 17:42:52]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[robertphillips, 2015-05-26 17:56:51]

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

[robertphillips, 2015-05-26 17:56:52]

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/1151283004/#ps100001
(title: "Fix build on high-maintenance machines")

[commit-bot: I haz the power, 2015-05-26 17:57:04]

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

[commit-bot: I haz the power, 2015-05-26 17:58:31]

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

[commit-bot: I haz the power, 2015-05-26 17:58:32]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[robertphillips, 2015-05-26 18:06:27]

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

[robertphillips, 2015-05-26 18:06:28]

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/1151283004/#ps120001
(title: "Fix no-GPU builds")

[commit-bot: I haz the power, 2015-05-26 18:06:41]

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

[robertphillips, 2015-05-26 18:10:18]

Brian?

[bsalomon, 2015-05-26 18:12:48]

On 2015/05/26 18:10:18, robertphillips wrote:
> Brian?

If you think we can put GrPipelineBuilder back in its cage real fast, then lgtm.

[bsalomon, 2015-05-26 18:12:52]

On 2015/05/26 18:10:18, robertphillips wrote:
> Brian?

If you think we can put GrPipelineBuilder back in its cage real fast, then lgtm.

[commit-bot: I haz the power, 2015-05-26 18:13:14]

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

[commit-bot: I haz the power, 2015-05-26 18:13:16]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-05-26 18:36:46]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-05-26 18:37:01]

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

[commit-bot: I haz the power, 2015-05-26 18:38:09]

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