Incrementally flush GrDrawTarget

Incrementally flush GrDrawTarget

BUG=skia:

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

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

[joshualitt, 2015-10-01 20:09:49]

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

[joshualitt, 2015-10-01 20:09:50]

I will land after the branch, ptal

[robertphillips, 2015-10-02 13:02:57]

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

https://codereview.chromium.org/1386463004/diff/1/src/gpu/GrDrawTarget.cpp#ne...
src/gpu/GrDrawTarget.cpp:114: 
Maybe flushNextN(int N) and just assume 'index' is 'fLastFlushedBatch' ?

https://codereview.chromium.org/1386463004/diff/1/src/gpu/GrDrawTarget.cpp#ne...
src/gpu/GrDrawTarget.cpp:115: void GrDrawTarget::flushN(int index, int N) {
SkASSERT(index == fLastFlushedBatch); ?

[joshualitt, 2015-10-02 13:54:59]

feedback inc

[bsalomon, 2015-10-02 13:56:37]

lgtm

https://codereview.chromium.org/1386463004/diff/60001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/1386463004/diff/60001/src/gpu/GrDrawTarget.cp...
src/gpu/GrDrawTarget.cpp:447: this->flushN(1);
Maybe assert that we're only 1 ahead? Otherwise, lgtm.

[joshualitt, 2015-10-02 14:33:52]

On 2015/10/02 13:56:37, bsalomon wrote:
> lgtm
> 
> https://codereview.chromium.org/1386463004/diff/60001/src/gpu/GrDrawTarget.cpp
> File src/gpu/GrDrawTarget.cpp (right):
> 
>
https://codereview.chromium.org/1386463004/diff/60001/src/gpu/GrDrawTarget.cp...
> src/gpu/GrDrawTarget.cpp:447: this->flushN(1);
> Maybe assert that we're only 1 ahead? Otherwise, lgtm.

okay, ptal.  This breaks up the prepares and flushes.  Will this help Robert? 
If not, I can add a flush threshold in a follow on(i'd like to do it in a
separate patch for measurement reasons)

[bsalomon, 2015-10-02 14:40:17]

https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.cpp
File src/gpu/GrDrawTarget.cpp (right):

https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.c...
src/gpu/GrDrawTarget.cpp:115: void GrDrawTarget::prepareN(int N) {
I'm wondering how helpful this helper function is, rather than just inlining and
reducing into the two callers. One is just

for (int i = fLastPreparedBatch; i < fBatches.count(); ++i) {
    fBatches[i]->prepare(&fFlushState);
}

and the other is just:

batch->prepare(&fFlushState);
++fLastPreparedBatch;

Also, isn't fLastPreparedBatch really fFirstUnpreparedBatch?

And why is the code that is preparing not also updating fLastPreparedBatch?

[joshualitt, 2015-10-02 15:53:08]

On 2015/10/02 14:40:17, bsalomon wrote:
>
https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.cpp
> File src/gpu/GrDrawTarget.cpp (right):
> 
>
https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.c...
> src/gpu/GrDrawTarget.cpp:115: void GrDrawTarget::prepareN(int N) {
> I'm wondering how helpful this helper function is, rather than just inlining
and
> reducing into the two callers. One is just
> 
> for (int i = fLastPreparedBatch; i < fBatches.count(); ++i) {
>     fBatches[i]->prepare(&fFlushState);
> }
> 
> and the other is just:
> 
> batch->prepare(&fFlushState);
> ++fLastPreparedBatch;
> 
> Also, isn't fLastPreparedBatch really fFirstUnpreparedBatch?
> 
> And why is the code that is preparing not also updating fLastPreparedBatch?

feedback inc

[bsalomon, 2015-10-02 15:53:55]

On 2015/10/02 15:53:08, joshualitt wrote:
> On 2015/10/02 14:40:17, bsalomon wrote:
> >
>
https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.cpp
> > File src/gpu/GrDrawTarget.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1386463004/diff/100001/src/gpu/GrDrawTarget.c...
> > src/gpu/GrDrawTarget.cpp:115: void GrDrawTarget::prepareN(int N) {
> > I'm wondering how helpful this helper function is, rather than just inlining
> and
> > reducing into the two callers. One is just
> > 
> > for (int i = fLastPreparedBatch; i < fBatches.count(); ++i) {
> >     fBatches[i]->prepare(&fFlushState);
> > }
> > 
> > and the other is just:
> > 
> > batch->prepare(&fFlushState);
> > ++fLastPreparedBatch;
> > 
> > Also, isn't fLastPreparedBatch really fFirstUnpreparedBatch?
> > 
> > And why is the code that is preparing not also updating fLastPreparedBatch?
> 
> feedback inc

lgtm

[joshualitt, 2015-10-02 15:59:48]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-10-02 15:59:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1386463004/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1386463004/130001

[commit-bot: I haz the power, 2015-10-02 16:06:42]

Committed patchset #8 (id:130001) as
https://skia.googlesource.com/skia/+/a7e878064509ef96b54d5507dab0b50def66dc13

[joshualitt, 2015-10-02 17:15:41]

A revert of this CL (patchset #8 id:130001) has been created in
https://codereview.chromium.org/1386683002/ by joshualitt@google.com.

The reason for reverting is: breaking bots.

[robertphillips, 2015-10-02 17:16:09]

A revert of this CL (patchset #8 id:130001) has been created in
https://codereview.chromium.org/1374313004/ by robertphillips@google.com.

The reason for reverting is: naughty.

[joshualitt, 2015-10-02 18:20:18]

On 2015/10/02 17:16:09, robertphillips wrote:
> A revert of this CL (patchset #8 id:130001) has been created in
> https://codereview.chromium.org/1374313004/ by
mailto:robertphillips@google.com.
> 
> The reason for reverting is: naughty.

Okay, everything should be fixed now, relanding

[joshualitt, 2015-10-02 18:20:22]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-10-02 18:20:23]

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/1386463004/#ps150001
(title: "fixed")

[commit-bot: I haz the power, 2015-10-02 18:20:36]

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

[commit-bot: I haz the power, 2015-10-02 18:27:19]

Committed patchset #9 (id:150001) as
https://skia.googlesource.com/skia/+/f6d259bd96b315dd344f129e0d1c0157f67ca288