Add GrAARectBatch which can handle a local matrix

Add GrAARectBatch which can handle a local matrix

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/147dc06f3bda762f948cfdcc3fc43cecbf32e826

[joshualitt, 2015-08-12 16:12:08]

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

[joshualitt, 2015-08-12 16:12:09]

ptal.

[robertphillips, 2015-08-12 16:31:20]

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
File src/gpu/batches/GrAAFillRectBatch.cpp (right):

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:27: template <typename Base>
// This compiles b.c. ...
?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:67: this->usesLocalCoords(),
rm the 'this->' on the static method ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:77: size_t vertexStride =
gp->getVertexStride();
same here ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:83: batchTarget->resourceProvider()));
Hmm .. a bit surprised we need the "typename Base::" here.

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:108: // init() so the Batch can initialize
itself
"const Geometry& geometry() const" ?
If not should we return a "Geometry*" ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:230: 
rm "this->" ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:309: 
 Can we document somewhere what the two base classes need to define for the
derived class?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:319: protected:
Is this supposed to be static ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:333: bool usesLocalCoords) {
SkASSERT(!usesLocalCoords); ?

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:427: 
Seems like we need to test both here.

[joshualitt, 2015-08-12 17:07:41]

Feedback incorporated.  I'm gonna miss this guy.  Maybe we will find a use for
that pattern later.

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
File src/gpu/batches/GrAAFillRectBatch.cpp (right):

https://codereview.chromium.org/1290763002/diff/1/src/gpu/batches/GrAAFillRec...
src/gpu/batches/GrAAFillRectBatch.cpp:333: bool usesLocalCoords) {
On 2015/08/12 16:31:19, robertphillips wrote:
> SkASSERT(!usesLocalCoords); ?

this guy can use local coords but he does so through a uniform

[bsalomon, 2015-08-12 17:25:48]

https://codereview.chromium.org/1290763002/diff/20001/src/gpu/batches/GrAAFil...
File src/gpu/batches/GrAAFillRectBatch.cpp (right):

https://codereview.chromium.org/1290763002/diff/20001/src/gpu/batches/GrAAFil...
src/gpu/batches/GrAAFillRectBatch.cpp:28: * To use this template, an
implementation must define the following static functions:
AAFillRectBatch is templated to optionally allow the insertion of an additional
attribute for explicit local coordinates?

https://codereview.chromium.org/1290763002/diff/20001/src/gpu/batches/GrAAFil...
src/gpu/batches/GrAAFillRectBatch.cpp:31: *     bool
OnCanCombineLocalCoords(const SkMatrix& mine, const SkMatrix& theirs,
Why "On"?

https://codereview.chromium.org/1290763002/diff/20001/src/gpu/batches/GrAAFil...
src/gpu/batches/GrAAFillRectBatch.cpp:39: *     void PostGenerateHook(intptr_t,
size_t, SkPoint*, const Geometry&) {}
FillInAttributes?
VerticesWerePopulated?

Doc on what the params are? Not obvious why intptr_t and SkPoint*

https://codereview.chromium.org/1290763002/diff/20001/src/gpu/batches/GrAAFil...
src/gpu/batches/GrAAFillRectBatch.cpp:41: template <typename Base>
Base doesn't seem like the right name anymore

[joshualitt, 2015-08-12 17:33:54]

feedback incorporated

[bsalomon, 2015-08-12 18:05:08]

lgtm

[joshualitt, 2015-08-12 18:28:22]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-08-12 18:28:38]

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

[commit-bot: I haz the power, 2015-08-12 18:29:32]

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

[commit-bot: I haz the power, 2015-08-12 18:29:33]

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

[joshualitt, 2015-08-12 18:44:32]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2015-08-12 18:44:32]

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/1290763002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-08-12 18:44:39]

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

[commit-bot: I haz the power, 2015-08-12 18:51:49]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/147dc06f3bda762f948cfdcc3fc43cecbf32e826