Move GrDrawTarget::clear logic into GrDrawContext

Move GrDrawTarget::clear logic into GrDrawContext

I found it a bit worrisome that GrDrawTarget was calling back into GrDrawContext. This also moves GrDrawTarget closer to being a simple-ish container of batches.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003

Committed: https://skia.googlesource.com/skia/+/9199a9fef9896636f673372d2ac1c00af036bf85

[robertphillips, 2016-07-12 18:32:12]

Description was changed from

==========
Move GrDrawTarget::clear logic into GrDrawContext
==========

to

==========
Move GrDrawTarget::clear logic into GrDrawContext
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003
==========

[robertphillips, 2016-07-12 18:34:24]

Description was changed from

==========
Move GrDrawTarget::clear logic into GrDrawContext
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003
==========

to

==========
Move GrDrawTarget::clear logic into GrDrawContext

I found it a bit worrisome that GrDrawTarget was calling back into
GrDrawContext. This also moves GrDrawTarget closer to being a simple-ish
container of batches.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003
==========

[robertphillips, 2016-07-12 18:34:25]

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

[bsalomon, 2016-07-12 18:54:26]

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

https://codereview.chromium.org/2145643003/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:228: sk_sp<GrBatch>
batch(sk_make_sp<GrClearBatch>(*rect, color, this->accessRenderTarget()));
It seems to me like this use of sk_sp is hurting readability. There are two
moves to follow and there is a bunch of stuff to parse to find GrClearBatch.
Maybe a sk_sp<> GrClearBatch::Make(..) would help? Also this is an instance
where I find the ban on auto for sk_sp to be unfortunate. I think this reads a
whole lot better as:

auto batch = GrClearBatch::Make(*rect, color, this->accessRenderTarget());

I find that much easier since as a reader what I probably want to get out of
this is "A clear batch is being made and called 'batch'" Make (by our current
conventions) tells me this is a sk_sp<> thing.

https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
File src/gpu/batches/GrClearStencilClipBatch.h (right):

https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
src/gpu/batches/GrClearStencilClipBatch.h:17: class GrClearStencilClipBatch
final : public GrBatch {
Is this file move related?

[robertphillips, 2016-07-12 19:22:47]

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

https://codereview.chromium.org/2145643003/diff/1/src/gpu/GrDrawContext.cpp#n...
src/gpu/GrDrawContext.cpp:228: sk_sp<GrBatch>
batch(sk_make_sp<GrClearBatch>(*rect, color, this->accessRenderTarget()));
On 2016/07/12 18:54:26, bsalomon wrote:
> It seems to me like this use of sk_sp is hurting readability. There are two
> moves to follow and there is a bunch of stuff to parse to find GrClearBatch.
> Maybe a sk_sp<> GrClearBatch::Make(..) would help? Also this is an instance
> where I find the ban on auto for sk_sp to be unfortunate. I think this reads a
> whole lot better as:
> 
> auto batch = GrClearBatch::Make(*rect, color, this->accessRenderTarget());
> 
> I find that much easier since as a reader what I probably want to get out of
> this is "A clear batch is being made and called 'batch'" Make (by our current
> conventions) tells me this is a sk_sp<> thing.

Let's try this - hopefully improved.

https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
File src/gpu/batches/GrClearStencilClipBatch.h (right):

https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
src/gpu/batches/GrClearStencilClipBatch.h:17: class GrClearStencilClipBatch
final : public GrBatch {
On 2016/07/12 18:54:26, bsalomon wrote:
> Is this file move related?

Not totally necessary. I wanted to deprive GrDrawTarget of access to the
GrClearBatch but this one was jammed into the same header.

[bsalomon, 2016-07-12 19:24:50]

On 2016/07/12 19:22:47, robertphillips wrote:
> https://codereview.chromium.org/2145643003/diff/1/src/gpu/GrDrawContext.cpp
> File src/gpu/GrDrawContext.cpp (right):
> 
>
https://codereview.chromium.org/2145643003/diff/1/src/gpu/GrDrawContext.cpp#n...
> src/gpu/GrDrawContext.cpp:228: sk_sp<GrBatch>
> batch(sk_make_sp<GrClearBatch>(*rect, color, this->accessRenderTarget()));
> On 2016/07/12 18:54:26, bsalomon wrote:
> > It seems to me like this use of sk_sp is hurting readability. There are two
> > moves to follow and there is a bunch of stuff to parse to find GrClearBatch.
> > Maybe a sk_sp<> GrClearBatch::Make(..) would help? Also this is an instance
> > where I find the ban on auto for sk_sp to be unfortunate. I think this reads
a
> > whole lot better as:
> > 
> > auto batch = GrClearBatch::Make(*rect, color, this->accessRenderTarget());
> > 
> > I find that much easier since as a reader what I probably want to get out of
> > this is "A clear batch is being made and called 'batch'" Make (by our
current
> > conventions) tells me this is a sk_sp<> thing.
> 
> Let's try this - hopefully improved.
> 
>
https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
> File src/gpu/batches/GrClearStencilClipBatch.h (right):
> 
>
https://codereview.chromium.org/2145643003/diff/1/src/gpu/batches/GrClearSten...
> src/gpu/batches/GrClearStencilClipBatch.h:17: class GrClearStencilClipBatch
> final : public GrBatch {
> On 2016/07/12 18:54:26, bsalomon wrote:
> > Is this file move related?
> 
> Not totally necessary. I wanted to deprive GrDrawTarget of access to the
> GrClearBatch but this one was jammed into the same header.

Oh I see. I didn't catch that.

lgtm

[robertphillips, 2016-07-12 19:32:05]

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

[commit-bot: I haz the power, 2016-07-12 19:32:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 19:35:14]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 19:35:14]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)

[robertphillips, 2016-07-12 19:52:45]

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

[commit-bot: I haz the power, 2016-07-12 19:52:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 20:04:59]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 20:05:01]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-07-13 14:47:40]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2016-07-13 14:47:40]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/2145643003/#ps40001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-07-13 14:47:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 14:48:46]

Description was changed from

==========
Move GrDrawTarget::clear logic into GrDrawContext

I found it a bit worrisome that GrDrawTarget was calling back into
GrDrawContext. This also moves GrDrawTarget closer to being a simple-ish
container of batches.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003
==========

to

==========
Move GrDrawTarget::clear logic into GrDrawContext

I found it a bit worrisome that GrDrawTarget was calling back into
GrDrawContext. This also moves GrDrawTarget closer to being a simple-ish
container of batches.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2145643003

Committed:
https://skia.googlesource.com/skia/+/9199a9fef9896636f673372d2ac1c00af036bf85
==========

[commit-bot: I haz the power, 2016-07-13 14:48:47]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/9199a9fef9896636f673372d2ac1c00af036bf85