Simplify GrDrawBatch uploads and token uage.

Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

Simplifies GrTestBatch

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1835283002

Committed: https://skia.googlesource.com/skia/+/342bfc25de5b0452b1551bf9db4bf45eac7718b2

[bsalomon, 2016-03-28 15:07:09]

Description was changed from

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

BUG=skia:
==========

to

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-03-28 15:07:39]

Description was changed from

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

Simplifies GrTestBatch

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bsalomon, 2016-03-28 15:42:23]

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

[bsalomon, 2016-03-28 15:42:24]

Currently a GrVertexBatch subclass's draws are structured like this:

this->initDraw(gp0);
target->draw(mesh); // uses gp0
..
this->initDraw(gp1);
target->draw(mesh); // uses gp1

which I found confusing as a batch author. I wanted it to look like:

target->draw(gp0, mesh);

This all stems from making that change. I also tried to add more comments and
make them clearer. The asap/inline uploaders seemed simpler as lambdas rather
than functor objects. Also, I found the draw arrays stuff in GrVertexBatch
somewhat confusing and hope the rewrite of how draws are stored is easier for
the next reader.

The way into this is to start with GrBatchFlushState.h, GrDrawBatch.h, and
GrVertexBatch.h. All other changes stem from those.

When a batch issues draws like (gp0, mesh0), (gp0, mesh1), ... (gp0, meshn) I
wind up storing it as (gp0, [mesh0...meshn]) in order to allow the Gpu object to
only process the pipeline/gp0 combo once. I think this added complexity in
GrVertexBatch::draw is justified by making writing a new GrBatch or reading an
existing subclass simpler.

[egdaniel, 2016-03-28 20:05:20]

https://codereview.chromium.org/1835283002/diff/40001/src/gpu/batches/GrDrawB...
File src/gpu/batches/GrDrawBatch.h (right):

https://codereview.chromium.org/1835283002/diff/40001/src/gpu/batches/GrDrawB...
src/gpu/batches/GrDrawBatch.h:56: * Batches can insert uploads when preparing
their draws. The can eithe be inserted before
The->They, eithe->either

https://codereview.chromium.org/1835283002/diff/40001/src/gpu/batches/GrDrawB...
src/gpu/batches/GrDrawBatch.h:58: * them between draws, which is called an
"inline upload". ASAP uploads are typically used
Wouldn't you want inline uploads in the case where two draws want different
content in the same GrSurface?

[bsalomon, 2016-03-28 20:11:03]

https://codereview.chromium.org/1835283002/diff/40001/src/gpu/batches/GrDrawB...
File src/gpu/batches/GrDrawBatch.h (right):

https://codereview.chromium.org/1835283002/diff/40001/src/gpu/batches/GrDrawB...
src/gpu/batches/GrDrawBatch.h:58: * them between draws, which is called an
"inline upload". ASAP uploads are typically used
On 2016/03/28 20:05:20, egdaniel wrote:
> Wouldn't you want inline uploads in the case where two draws want different
> content in the same GrSurface?

Yes, mental slip

[bsalomon, 2016-03-30 02:25:23]

comments updated

[robertphillips, 2016-03-30 14:29:47]

lgtm + nits

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/GrBatchFlushSta...
File src/gpu/GrBatchFlushState.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/GrBatchFlushSta...
src/gpu/GrBatchFlushState.h:36: 
a an -> an ?

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
File src/gpu/batches/GrVertexBatch.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:71: // the batch's pipeline. The meshes for the
draw are stored in the fMeshes array and each
overlength

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:72: // Queued draw uses fMeshCnt meshes from the
fMeshes array. The reason for coallescing meshes that
extra '.' ?

[egdaniel, 2016-03-30 15:52:33]

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrDrawB...
File src/gpu/batches/GrDrawBatch.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrDrawB...
src/gpu/batches/GrDrawBatch.h:54: /** See comments before GrDrawBatch::Target
definition on how deferred uploaders work. */
Can you comment on where GrDrawBatch::Target definition actually is? It does not
seem obvious that it is actually declared in another class's header file

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
File src/gpu/batches/GrVertexBatch.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:36: vertices before calling issueDraws(). */
What is issueDraws here?

Also I think there is an extra "before"

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:72: // Queued draw uses fMeshCnt meshes from the
fMeshes array. The reason for coallescing meshes that
if you're capitalizing Queued make it QueuedDraw

[bsalomon, 2016-03-30 16:59:11]

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/GrBatchFlushSta...
File src/gpu/GrBatchFlushState.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/GrBatchFlushSta...
src/gpu/GrBatchFlushState.h:36: 
On 2016/03/30 14:29:47, robertphillips wrote:
> a an -> an ?

Done.

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrDrawB...
File src/gpu/batches/GrDrawBatch.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrDrawB...
src/gpu/batches/GrDrawBatch.h:54: /** See comments before GrDrawBatch::Target
definition on how deferred uploaders work. */
On 2016/03/30 15:52:33, egdaniel wrote:
> Can you comment on where GrDrawBatch::Target definition actually is? It does
not
> seem obvious that it is actually declared in another class's header file

Not crazy about doing that for 3 reasons: 1) Likelihood that comments about what
files things are in will get out of date is pretty high, 2) I'd like to try to
rearrange where things are defined soon after this CL, 3) seems pretty simple to
grep for it.

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
File src/gpu/batches/GrVertexBatch.h (right):

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:36: vertices before calling issueDraws(). */
On 2016/03/30 15:52:33, egdaniel wrote:
> What is issueDraws here?
> 
> Also I think there is an extra "before"

Done.

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:71: // the batch's pipeline. The meshes for the
draw are stored in the fMeshes array and each
On 2016/03/30 14:29:47, robertphillips wrote:
> overlength 

Done.

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:72: // Queued draw uses fMeshCnt meshes from the
fMeshes array. The reason for coallescing meshes that
On 2016/03/30 14:29:47, robertphillips wrote:
> extra '.' ?

Done.

https://codereview.chromium.org/1835283002/diff/60001/src/gpu/batches/GrVerte...
src/gpu/batches/GrVertexBatch.h:72: // Queued draw uses fMeshCnt meshes from the
fMeshes array. The reason for coallescing meshes that
On 2016/03/30 15:52:33, egdaniel wrote:
> if you're capitalizing Queued make it QueuedDraw

Done.

[bsalomon, 2016-04-01 12:53:15]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-04-01 12:53:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1835283002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-01 12:53:20]

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

[commit-bot: I haz the power, 2016-04-01 13:06:23]

Description was changed from

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

Simplifies GrTestBatch

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Simplify GrDrawBatch uploads and token uage.

GrVertexBatch subclasses no longer need "initDraw".

Simplifies GrTestBatch

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/342bfc25de5b0452b1551bf9db4bf45eac7718b2
==========

[commit-bot: I haz the power, 2016-04-01 13:06:24]

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