Refactor GrBufferAllocPools to use resource cache

Refactor GrBufferAllocPools to use resource cache

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

Committed: https://skia.googlesource.com/skia/+/1b8e1b5c499e31a671232c8ccb10e778e0d8b154

[robertphillips, 2015-05-13 14:15:24]

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

[bsalomon, 2015-05-13 14:41:36]

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

https://codereview.chromium.org/1139753002/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.h:74: GrIndexBuffer* refScratchIndexBuffer(size_t
size, bool dynamic,
Do we need a scratch match param at all? When would we use an exact match for
buffers? For textures I was hoping to delete the enum entirely as well as the
notion of scratch.

createTexture() {
    // might be a scratch might be new, who can say!
}

refApproxTexture() {
}

[robertphillips, 2015-05-13 14:44:55]

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

https://codereview.chromium.org/1139753002/diff/60001/src/gpu/GrResourceProvi...
src/gpu/GrResourceProvider.h:74: GrIndexBuffer* refScratchIndexBuffer(size_t
size, bool dynamic,
Well - the static, instanced geometry index buffers might as well be exactly
sized.

[bsalomon, 2015-05-13 15:00:08]

On 2015/05/13 14:44:55, robertphillips wrote:
>
https://codereview.chromium.org/1139753002/diff/60001/src/gpu/GrResourceProvi...
> File src/gpu/GrResourceProvider.h (right):
> 
>
https://codereview.chromium.org/1139753002/diff/60001/src/gpu/GrResourceProvi...
> src/gpu/GrResourceProvider.h:74: GrIndexBuffer* refScratchIndexBuffer(size_t
> size, bool dynamic,
> Well - the static, instanced geometry index buffers might as well be exactly
> sized.

Those aren't scratch, right? They have a content key. Could we just have
something like createIndexBuffer()/refApproxScratchBuffer()?

[robertphillips, 2015-05-13 16:06:05]

PTAL

[bsalomon, 2015-05-13 16:24:07]

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
File include/gpu/GrTextureProvider.h (right):

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
include/gpu/GrTextureProvider.h:163: bool isAbandoned() const {
protected?

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

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBatchTarget....
src/gpu/GrBatchTarget.cpp:22:
fVertexPool.reset(SkNEW_ARGS(GrVertexBufferAllocPool, (fGpu)));
need to be dynamic?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.cpp:300: GrResourceProvider* rp =
fGpu->getContext()->resourceProvider();
Can we get GrGpu off this guy?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.h (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.h:70: size_t   bufferSize = 0);
should we remove bufferSize?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
File src/gpu/GrResourceProvider.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
src/gpu/GrResourceProvider.cpp:94: GrVertexBuffer*
GrResourceProvider::getVertBuffer(size_t size, bool dynamic,
Vertex, pretty please?

[bsalomon, 2015-05-13 16:24:07]

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
File include/gpu/GrTextureProvider.h (right):

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
include/gpu/GrTextureProvider.h:163: bool isAbandoned() const {
protected?

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

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBatchTarget....
src/gpu/GrBatchTarget.cpp:22:
fVertexPool.reset(SkNEW_ARGS(GrVertexBufferAllocPool, (fGpu)));
need to be dynamic?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.cpp:300: GrResourceProvider* rp =
fGpu->getContext()->resourceProvider();
Can we get GrGpu off this guy?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.h (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.h:70: size_t   bufferSize = 0);
should we remove bufferSize?

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
File src/gpu/GrResourceProvider.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
src/gpu/GrResourceProvider.cpp:94: GrVertexBuffer*
GrResourceProvider::getVertBuffer(size_t size, bool dynamic,
Vertex, pretty please?

[robertphillips, 2015-05-13 16:56:15]

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
File include/gpu/GrTextureProvider.h (right):

https://codereview.chromium.org/1139753002/diff/100001/include/gpu/GrTextureP...
include/gpu/GrTextureProvider.h:163: bool isAbandoned() const {
On 2015/05/13 16:24:07, bsalomon wrote:
> protected?

All these guys already are.

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

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBatchTarget....
src/gpu/GrBatchTarget.cpp:22:
fVertexPool.reset(SkNEW_ARGS(GrVertexBufferAllocPool, (fGpu)));
On 2015/05/13 16:24:07, bsalomon wrote:
> need to be dynamic?

Done.

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.cpp:300: GrResourceProvider* rp =
fGpu->getContext()->resourceProvider();
On 2015/05/13 16:24:07, bsalomon wrote:
> Can we get GrGpu off this guy?

Unfortunately, GrBufferAllocPool needs access to the caps.

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.h (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.h:70: size_t   bufferSize = 0);
On 2015/05/13 16:24:07, bsalomon wrote:
> should we remove bufferSize?

I still use it in the GrVertexBufferAllocPool ctor to give vertex buffers a
larger minimum size than index buffers.

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
File src/gpu/GrResourceProvider.cpp (right):

https://codereview.chromium.org/1139753002/diff/100001/src/gpu/GrResourceProv...
src/gpu/GrResourceProvider.cpp:94: GrVertexBuffer*
GrResourceProvider::getVertBuffer(size_t size, bool dynamic,
On 2015/05/13 16:24:07, bsalomon wrote:
> Vertex, pretty please?

Done.

[bsalomon, 2015-05-13 17:32:50]

lgtm

https://codereview.chromium.org/1139753002/diff/120001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.h (right):

https://codereview.chromium.org/1139753002/diff/120001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.h:72: virtual ~GrBufferAllocPool();
override?

[robertphillips, 2015-05-13 17:39:18]

https://codereview.chromium.org/1139753002/diff/120001/src/gpu/GrBufferAllocP...
File src/gpu/GrBufferAllocPool.h (right):

https://codereview.chromium.org/1139753002/diff/120001/src/gpu/GrBufferAllocP...
src/gpu/GrBufferAllocPool.h:72: virtual ~GrBufferAllocPool();
On 2015/05/13 17:32:49, bsalomon wrote:
> override?

SkNoncopyable doesn't define one.

[robertphillips, 2015-05-13 17:39:29]

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

[commit-bot: I haz the power, 2015-05-13 17:39:44]

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

[commit-bot: I haz the power, 2015-05-13 17:41:11]

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

[commit-bot: I haz the power, 2015-05-13 17:41:13]

Dry run: Try jobs failed on following builders:
  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...)

[robertphillips, 2015-05-13 17:44:33]

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

[robertphillips, 2015-05-13 17:44:34]

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/1139753002/#ps140001
(title: "Patch up GrNextPow2")

[commit-bot: I haz the power, 2015-05-13 17:44:50]

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

[commit-bot: I haz the power, 2015-05-13 17:46:01]

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

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

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

[robertphillips, 2015-05-13 17:47:50]

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

[robertphillips, 2015-05-13 17:47:51]

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/1139753002/#ps160001
(title: "Patch up SkTMax call")

[commit-bot: I haz the power, 2015-05-13 17:48:07]

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

[commit-bot: I haz the power, 2015-05-13 17:54:15]

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

[commit-bot: I haz the power, 2015-05-13 17:54:16]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-05-13 17:54:50]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-05-13 17:55:11]

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

[commit-bot: I haz the power, 2015-05-13 17:55:40]

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

[robertphillips, 2015-05-14 12:24:34]

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

The reason for reverting is: Perf Regression for some bot/skp combinations.

[robertphillips, 2015-06-23 14:41:33]

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

[robertphillips, 2015-06-23 14:50:56]

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

[robertphillips, 2015-06-23 14:50:57]

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/1139753002/#ps260001
(title: "update to ToT")

[commit-bot: I haz the power, 2015-06-23 14:51:14]

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

[commit-bot: I haz the power, 2015-06-23 14:51:59]

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

[commit-bot: I haz the power, 2015-06-23 14:52:01]

Dry run: Exceeded global retry quota

[robertphillips, 2015-06-23 14:55:01]

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

[robertphillips, 2015-06-23 14:55:02]

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/1139753002/#ps280001
(title: "fix merge issue")

[commit-bot: I haz the power, 2015-06-23 14:55:11]

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

[commit-bot: I haz the power, 2015-06-23 15:00:49]

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

[commit-bot: I haz the power, 2015-06-23 15:00:50]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-06-23 15:22:04]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-06-23 15:22:20]

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

[commit-bot: I haz the power, 2015-06-23 15:23:12]

Committed patchset #15 (id:280001) as
https://skia.googlesource.com/skia/+/e935f1a0e2351373c33600b8388492ce1218014a

[robertphillips, 2015-06-23 19:35:44]

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

The reason for reverting is: Will reland after Chromium branch.

[robertphillips, 2015-06-24 13:53:11]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-06-24 13:53:19]

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

[commit-bot: I haz the power, 2015-06-24 13:54:16]

Committed patchset #15 (id:280001) as
https://skia.googlesource.com/skia/+/1b8e1b5c499e31a671232c8ccb10e778e0d8b154