First pass at drawAtlas batching.

First pass at drawAtlas batching.

Moves drawAtlas setup into its own method in GrDrawContext, and adds
DrawAtlasBatch.
Uses pre-built index buffer for quads.

TBR=bsalomon@google.com

Committed: https://skia.googlesource.com/skia/+/31ff762dc8bfcd86eb4af92b18fdad36913a04e5

[jvanverth1, 2015-08-06 20:58:45]

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

[jvanverth1, 2015-08-06 20:58:46]

Reduces percentage of time in memmove from ~20% to ~17% (it's pretty noisy).
This is mainly due to using the pre-built quad index buffer.

[robertphillips, 2015-08-07 13:36:38]

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

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:701: SkTDArray<SkPoint> fPositions;
Can we remove 'fIndices' ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:816: bool batchablePrimitiveType() const {
Don't we always know fPrimitiveType will be kTriangles_GrPrimitiveType ?
Can we just have an assert and return true ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:817: return kTriangles_GrPrimitiveType ==
fBatch.fPrimitiveType ||
tab these over ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:821: GrColor color() const { return fBatch.fColor; }
Can we just return true from 'usesLocalCoords' ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:835: 
Can we skip this check ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:846: 
We could, in theory, generate white vertex colors for one that doesn't have
them. I don't think that would be a common scenario however (two drawAtlas calls
from the same texture - one with colors - on without).

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:897: SkRect bounds;
Hmmm ... I think you have to transform each texRect[i] by its matching xform[i]
and then compute the bound from those rects.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:910: 
Is the plan to interleave here ?

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:939: 
:(

[jvanverth1, 2015-08-07 15:55:17]

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

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:701: SkTDArray<SkPoint> fPositions;
On 2015/08/07 13:36:37, robertphillips wrote:
> Can we remove 'fIndices' ?

Done.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:816: bool batchablePrimitiveType() const {
On 2015/08/07 13:36:38, robertphillips wrote:
> Don't we always know fPrimitiveType will be kTriangles_GrPrimitiveType ?
> Can we just have an assert and return true ?

Done.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:817: return kTriangles_GrPrimitiveType ==
fBatch.fPrimitiveType ||
On 2015/08/07 13:36:37, robertphillips wrote:
> tab these over ?

Removed.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:821: GrColor color() const { return fBatch.fColor; }
On 2015/08/07 13:36:38, robertphillips wrote:
> Can we just return true from 'usesLocalCoords' ?

Removed.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:835: 
On 2015/08/07 13:36:37, robertphillips wrote:
> Can we skip this check ?

Done.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:846: 
On 2015/08/07 13:36:37, robertphillips wrote:
> We could, in theory, generate white vertex colors for one that doesn't have
> them. I don't think that would be a common scenario however (two drawAtlas
calls
> from the same texture - one with colors - on without).

It's possible, though, so for now I think I'll leave the check.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:897: SkRect bounds;
On 2015/08/07 13:36:38, robertphillips wrote:
> Hmmm ... I think you have to transform each texRect[i] by its matching
xform[i]
> and then compute the bound from those rects.

Done. Moved to after we build the vertex locations. I'll add the client bounds
in a later change.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:910: 
On 2015/08/07 13:36:38, robertphillips wrote:
> Is the plan to interleave here ?

The plan is that the interleaving will happen when the batch generates the
vertex buffer.

https://codereview.chromium.org/1277933003/diff/20001/src/gpu/GrDrawContext.c...
src/gpu/GrDrawContext.cpp:939: 
On 2015/08/07 13:36:37, robertphillips wrote:
> :(

Acknowledged.

[robertphillips, 2015-08-07 16:12:38]

lgtm (with dox questions) although I would like Joshua to weigh in too.

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

https://codereview.chromium.org/1277933003/diff/60001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:231: * @param   xform           array of compressed
transformation data, required.
'access the atlas' ?

https://codereview.chromium.org/1277933003/diff/60001/include/gpu/GrDrawConte...
include/gpu/GrDrawContext.h:232: * @param   texRect         array of texture
rectangles used to access the paint.
'per sprite' ?

[joshualitt, 2015-08-07 16:23:56]

On 2015/08/07 16:12:38, robertphillips wrote:
> lgtm (with dox questions) although I would like Joshua to weigh in too.
> 
>
https://codereview.chromium.org/1277933003/diff/60001/include/gpu/GrDrawConte...
> File include/gpu/GrDrawContext.h (right):
> 
>
https://codereview.chromium.org/1277933003/diff/60001/include/gpu/GrDrawConte...
> include/gpu/GrDrawContext.h:231: * @param   xform           array of
compressed
> transformation data, required.
> 'access the atlas' ?
> 
>
https://codereview.chromium.org/1277933003/diff/60001/include/gpu/GrDrawConte...
> include/gpu/GrDrawContext.h:232: * @param   texRect         array of texture
> rectangles used to access the paint.
> 'per sprite' ?

lgtm too, please move DrawAtlasBatch to its own file in a follow on.

[jvanverth1, 2015-08-07 16:59:26]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-08-07 16:59:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com, robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1277933003/#ps80001
(title: "Fix comment")

[commit-bot: I haz the power, 2015-08-07 16:59:40]

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

[commit-bot: I haz the power, 2015-08-07 17:00:40]

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

[commit-bot: I haz the power, 2015-08-07 17:00:41]

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...)

[jvanverth1, 2015-08-07 17:02:05]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2015-08-07 17:02:13]

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

[commit-bot: I haz the power, 2015-08-07 17:09:31]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/31ff762dc8bfcd86eb4af92b18fdad36913a04e5