Scratch textures are no longer removed from the cache in Debug

This CL also stops the freeing & re-allocation of the GrResourceEntry for textures that are detached from an AutoScratchTexture.

[bsalomon, 2012-08-21 15:40:57]

You can ignore my dllist comments since we talked about it already more broadly.

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp#newcode472
src/gpu/GrContext.cpp:472: // to return the same texture the second time (unless
it was previously
Does this comment need an update?

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp#newcode487
src/gpu/GrContext.cpp:487: // This texture should already have a cache entry
since it was once
Just for my own understanding, am I correct that this gets called when a scratch
texture has been detached and then hits internal_dispose?

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.cpp
File src/gpu/GrResourceCache.cpp (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.cpp#newc...
src/gpu/GrResourceCache.cpp:328: GrAssert(fExclusiveList.countEntries() ==
fClientDetachedCount);
Should these asserts be outside the above if? It seems like they should always
hold.

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.cpp#newc...
src/gpu/GrResourceCache.cpp:403: void GrDLinkedList::remove(GrResourceEntry*
entry) {
Nice capturing this in a separate class.

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.h
File src/gpu/GrResourceCache.h (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.h#newcod...
src/gpu/GrResourceCache.h:167: class GrDLinkedList {
Should this be a nested class in GrResourceCache? Having it in the global
namespace with the name GrDLinkedList makes it seem more generic than it is
(it's a list of a specific type).

[robertphillips, 2012-08-21 18:03:55]

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp#newcode472
src/gpu/GrContext.cpp:472: // to return the same texture the second time (unless
it was previously
On 2012/08/21 15:40:57, bsalomon wrote:
> Does this comment need an update?

Done.

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp#newcode487
src/gpu/GrContext.cpp:487: // This texture should already have a cache entry
since it was once
Yep - although as we expand the reffing/unreffing system other use cases will
come through here.

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.cpp
File src/gpu/GrResourceCache.cpp (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrResourceCache.cpp#newc...
src/gpu/GrResourceCache.cpp:328: GrAssert(fExclusiveList.countEntries() ==
fClientDetachedCount);
On 2012/08/21 15:40:57, bsalomon wrote:
> Should these asserts be outside the above if? It seems like they should always
> hold.

Done.

[bsalomon, 2012-08-21 20:00:16]

LGTM

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/6465079/diff/1/src/gpu/GrContext.cpp#newcode487
src/gpu/GrContext.cpp:487: // This texture should already have a cache entry
since it was once
On 2012/08/21 18:03:55, robertphillips wrote:
> Yep - although as we expand the reffing/unreffing system other use cases will
> come through here.

Maybe we should (in future CLs) allow the GrResources to talk directly to the
cache rather than always having the GrContext as the intermediary. We could have
a getResourceCache() method on GrContext and only code that peers into src/gpu
would know about the returned type (GrResourceCache*).

[robertphillips, 2012-08-22 11:03:40]

committed as r5221