Move instanced index buffer creation to flush time

Move instanced index buffer creation to flush time

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

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

[bsalomon, 2015-05-02 02:48:38]

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

[commit-bot: I haz the power, 2015-05-02 02:48:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1116943004/20001

[bsalomon, 2015-05-02 02:49:20]

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

[commit-bot: I haz the power, 2015-05-02 03:01:17]

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

[commit-bot: I haz the power, 2015-05-02 03:01:18]

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

[bsalomon, 2015-05-02 13:05:36]

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

[commit-bot: I haz the power, 2015-05-02 13:05:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1116943004/40001

[commit-bot: I haz the power, 2015-05-02 13:09:12]

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

[commit-bot: I haz the power, 2015-05-02 13:09:13]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-05-02 13:26:06]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-05-02 13:26:07]

The CQ bit was unchecked by bsalomon@google.com

[bsalomon, 2015-05-02 13:26:14]

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

[commit-bot: I haz the power, 2015-05-02 13:26:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1116943004/60001

[commit-bot: I haz the power, 2015-05-02 13:30:33]

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

[commit-bot: I haz the power, 2015-05-02 13:30:34]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-05-04 13:16:00]

lgtm + nits & questions

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp
File gm/beziereffects.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp#ne...
gm/beziereffects.cpp:72: 
abort early here?

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp#ne...
gm/beziereffects.cpp:480: 
here?

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrResourceK...
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrResourceK...
include/gpu/GrResourceKey.h:276: /** Place outside of function/class
definitions. */
single line - maybe ?

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrTexturePr...
File include/gpu/GrTextureProvider.h (right):

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrTexturePr...
include/gpu/GrTextureProvider.h:159: 
Hmm - do we need this? It could perpetuate the everyone-points-to-everyone-else
problem.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAADistanceFie...
File src/gpu/GrAADistanceFieldPathRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAADistanceFie...
src/gpu/GrAADistanceFieldPathRenderer.cpp:207: const GrVertexBuffer*
vertexBuffer;
kVertsPerQuad ?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAAHairLinePat...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAAHairLinePat...
src/gpu/GrAAHairLinePathRenderer.cpp:856:
ref_lines_index_buffer(batchTarget->resourceProvider()));
Check if this fails ? We seem to do it every where else.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
File src/gpu/GrAARectRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.cpp:519: 
check indexBuffer here?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRenderer.h
File src/gpu/GrAARectRenderer.h (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.h:11: #include "GrColor.h"
Do we need this here ?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.h:21: class GrPipelineBuilder;
This too?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
File src/gpu/GrResourceProvider.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.cpp:61: 
So we can't just use gQuadIndexBufferKey in here ?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
File src/gpu/GrResourceProvider.h (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.h:30: /**
instance -> instanced ?

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.h:61: const GrIndexBuffer* refQuadIndexBuffer() {
Why not create gQuadIndexBufferKey in here ?

[bsalomon, 2015-05-04 15:01:10]

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp
File gm/beziereffects.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp#ne...
gm/beziereffects.cpp:72: 
On 2015/05/04 13:15:59, robertphillips wrote:
> abort early here?

It's not really supposed to ever happen, I care more about fewer LOC, simplicity
than exiting early.

https://codereview.chromium.org/1116943004/diff/60001/gm/beziereffects.cpp#ne...
gm/beziereffects.cpp:480: 
On 2015/05/04 13:15:59, robertphillips wrote:
> here?

Ditto

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrResourceK...
File include/gpu/GrResourceKey.h (right):

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrResourceK...
include/gpu/GrResourceKey.h:276: /** Place outside of function/class
definitions. */
On 2015/05/04 13:15:59, robertphillips wrote:
> single line - maybe ?

Done.

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrTexturePr...
File include/gpu/GrTextureProvider.h (right):

https://codereview.chromium.org/1116943004/diff/60001/include/gpu/GrTexturePr...
include/gpu/GrTextureProvider.h:159: 
On 2015/05/04 13:15:59, robertphillips wrote:
> Hmm - do we need this? It could perpetuate the
everyone-points-to-everyone-else
> problem.

This is protected. It's so the subclass, ResourceProvider, can create resources
(index buffers in this case). At some future point I'd like to split up GrGpu
into an API-specific execution engine for GrDrawTargets and a API-specific
resource creator, but that's a ways off.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAADistanceFie...
File src/gpu/GrAADistanceFieldPathRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAADistanceFie...
src/gpu/GrAADistanceFieldPathRenderer.cpp:207: const GrVertexBuffer*
vertexBuffer;
On 2015/05/04 13:15:59, robertphillips wrote:
> kVertsPerQuad ?

Done, nice catch!

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAAHairLinePat...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAAHairLinePat...
src/gpu/GrAAHairLinePathRenderer.cpp:856:
ref_lines_index_buffer(batchTarget->resourceProvider()));
On 2015/05/04 13:16:00, robertphillips wrote:
> Check if this fails ? We seem to do it every where else.

Done.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
File src/gpu/GrAARectRenderer.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.cpp:519: 
On 2015/05/04 13:16:00, robertphillips wrote:
> check indexBuffer here?

Done.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRenderer.h
File src/gpu/GrAARectRenderer.h (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.h:11: #include "GrColor.h"
On 2015/05/04 13:16:00, robertphillips wrote:
> Do we need this here ?

Done.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrAARectRendere...
src/gpu/GrAARectRenderer.h:21: class GrPipelineBuilder;
On 2015/05/04 13:16:00, robertphillips wrote:
> This too?

Done.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
File src/gpu/GrResourceProvider.cpp (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.cpp:61: 
On 2015/05/04 13:16:00, robertphillips wrote:
> So we can't just use gQuadIndexBufferKey in here ?

Right, it is not globally visible, just static in one function (currently the
class cons). Also, we use the quad ib so often that I thought it was safer to
create it in the cons and copy it to a member so that we never have to branch to
see if it had been created yet or not.

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
File src/gpu/GrResourceProvider.h (right):

https://codereview.chromium.org/1116943004/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.h:30: /**
On 2015/05/04 13:16:00, robertphillips wrote:
> instance -> instanced ?

Done.

[bsalomon, 2015-05-04 15:03:00]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-05-04 15:03:01]

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/1116943004/#ps100001
(title: "save all files")

[commit-bot: I haz the power, 2015-05-04 15:03:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1116943004/100001

[commit-bot: I haz the power, 2015-05-04 15:09:37]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/ab622c7b8cc8c39f0a594e4392b9e31b7e1ddb26

[bsalomon, 2015-05-04 17:08:52]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1126613003/ by bsalomon@google.com.

The reason for reverting is: messed up caching, recreating index buffers all the
time..

[bsalomon, 2015-05-04 17:27:01]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-05-04 17:27:01]

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/1116943004/#ps120001
(title: "fix missing assignment of keys to index buffers")

[commit-bot: I haz the power, 2015-05-04 17:27:19]

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

[commit-bot: I haz the power, 2015-05-04 17:36:51]

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