Picture shader resource caching.

Picture shader resource caching.

Replace the current/naive shader caching mechanism with a more general
implementation based on SkResourceCache.

Caching the bitmap shader itself (as opposed to just the tile bitmap)
makes for a chunkier key, but OTOH avoids allocating new shaders on
cache hit.

R=reed@google.com,mtklein@google.com

Committed: https://skia.googlesource.com/skia/+/23df2d693304824a2ce7ac3d988b0e48fec1c49d

[mtklein, 2014-10-21 17:27:45]

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
File src/core/SkPictureShader.cpp (right):

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
src/core/SkPictureShader.cpp:83: // FIXME: there's considerable
boilerplate/duplication here vs. the global resource cache.
Why aren't we using the global cache?

[f(malita), 2014-10-21 17:29:42]

On 2014/10/21 17:27:45, mtklein wrote:
>
https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
> File src/core/SkPictureShader.cpp (right):
> 
>
https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
> src/core/SkPictureShader.cpp:83: // FIXME: there's considerable
> boilerplate/duplication here vs. the global resource cache.
> Why aren't we using the global cache?

Different key domain.

[f(malita), 2014-10-21 17:32:30]

On 2014/10/21 17:29:42, Florin Malita wrote:
> On 2014/10/21 17:27:45, mtklein wrote:
> >
>
https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
> > File src/core/SkPictureShader.cpp (right):
> > 
> >
>
https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
> > src/core/SkPictureShader.cpp:83: // FIXME: there's considerable
> > boilerplate/duplication here vs. the global resource cache.
> > Why aren't we using the global cache?
> 
> Different key domain.

(would have been nice to have a domain field in the base SkResourceCache::Key,
but the way things are we can't compare hashes for keys in different domains)

[reed1, 2014-10-21 17:34:01]

lets see if we can easily add some namespacing/domaing keys to R.C.

[mtklein, 2014-10-21 17:38:11]

On 2014/10/21 17:34:01, reed1 wrote:
> lets see if we can easily add some namespacing/domaing keys to R.C.

+1

[reed1, 2014-10-21 20:18:15]

lgtm if you want to land this now, and address the domain issue later.

[reed1, 2014-10-21 20:28:00]

not lgtm until we at least address the allocator question.

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
File src/core/SkPictureShader.cpp (right):

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
src/core/SkPictureShader.cpp:221: if (!bm.tryAllocN32Pixels(tileSize.width(),
tileSize.height())) {
need to use the cache's GetAllocator to ensure we're using discardable memory
when that is the cache's preference.

[f(malita), 2014-10-21 23:49:24]

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
File src/core/SkPictureShader.cpp (right):

https://codereview.chromium.org/671683004/diff/40001/src/core/SkPictureShader...
src/core/SkPictureShader.cpp:221: if (!bm.tryAllocN32Pixels(tileSize.width(),
tileSize.height())) {
On 2014/10/21 20:28:00, reed1 wrote:
> need to use the cache's GetAllocator to ensure we're using discardable memory
> when that is the cache's preference.

Done.

[reed1, 2014-10-22 14:04:02]

lgtm

[f(malita), 2014-10-22 14:32:42]

On 2014/10/22 14:04:02, reed1 wrote:
> lgtm

Thanks. Will land as-is and then take a look at key namespacing to consolidate
the caches.

[f(malita), 2014-10-22 14:36:24]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2014-10-22 14:36:47]

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

[commit-bot: I haz the power, 2014-10-22 14:39:12]

Committed patchset #4 (id:60001) as 23df2d693304824a2ce7ac3d988b0e48fec1c49d