Use an image generator to back SkPictureShader tiles.

Use an image generator to back SkPictureShader tiles.

To avoid lifetime issues for tiles backed by discardable memory, use an
image generator to re-generate them on the fly.

With this CL, we are now caching bitmap shaders wrapping discardable
pixel ref bitmaps backed by picture image generators.

(the CL also includes some minor/unrelated SkPictureShader cleanup)

BUG=skia:3220
R=reed@google.com,halcanary@google.com

Committed: https://skia.googlesource.com/skia/+/4739955e98ba86900a5bbac0e3661601427a54d4

[f(malita), 2015-01-22 15:23:02]

fmalita@chromium.org changed reviewers:
+ halcanary@google.com
- robertphillips@google.com

[f(malita), 2015-01-22 15:46:00]

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

https://codereview.chromium.org/866773002/diff/1/src/core/SkPictureShader.cpp...
src/core/SkPictureShader.cpp:121: return sizeof(fKey) + sizeof(SkShader) +
fBitmapBytes;
Is this a lie now? Since we use discardable pixel refs, accounting for that
memory here doesn't feel quite right.

[hal.canary, 2015-01-22 16:30:29]

ImageGenerator seems like the right thing to use.  That part looks good to me.

I don't understand enough about the ResourceCache to say if you are using it
correctly.  My guess is that you double-count with bytesUsed().

[f(malita), 2015-01-22 16:31:58]

fmalita@chromium.org changed reviewers:
+ mtklein@google.com

[f(malita), 2015-01-22 16:33:15]

Whoops, is it me or is SkPixelRef::isLocked() not thread-safe?

http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...

WARNING: ThreadSanitizer: data race (pid=19278)
  Read of size 4 at 0x7d2800015768 by main thread:
    #0 SkPixelRef::isLocked() const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../include/core/SkPixelRef.h:92:29
(dm+0x000000395a07)
    #1 SkBitmapProcState::lockBaseBitmap()
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkBitmapProcState.cpp:265
(dm+0x000000395a07)
    #2 SkBitmapProcState::chooseProcs(SkMatrix const&, SkPaint const&)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkBitmapProcState.cpp:338:14
(dm+0x000000395c97)
    #3 SkBitmapProcShader::onCreateContext(SkShader::ContextRec const&, void*)
const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkBitmapProcShader.cpp:101:10
(dm+0x00000038b97b)
    #4 SkShader::createContext(SkShader::ContextRec const&, void*) const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkShader.cpp:92:12
(dm+0x000000469642)
    #5
SkPictureShader::PictureShaderContext::PictureShaderContext(SkPictureShader
const&, SkShader::ContextRec const&, SkShader*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkPictureShader.cpp:267:28
(dm+0x0000004e420f)
    #6 SkPictureShader::PictureShaderContext::Create(void*, SkPictureShader
const&, SkShader::ContextRec const&, SkShader*)
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkPictureShader.cpp:252
(dm+0x0000004e420f)
    #7 SkPictureShader::onCreateContext(SkShader::ContextRec const&, void*)
const
/home/default/storage/skia-repo/buildbot/skiabot-linux-xsan-001/build/slave/Test-Ubuntu13_10-GCE-NoGPU-x86_64-Release-TSAN-Trybot/build/skia/out/Release/../../src/core/SkPictureShader.cpp:245
(dm+0x0000004e420f)

[hal.canary, 2015-01-22 17:10:32]

On 2015/01/22 16:33:15, f(malita) wrote:
> Whoops, is it me or is SkPixelRef::isLocked() not thread-safe?


It looks like the questionable call is in SkBitmapProcState::lockBaseBitmap().

[f(malita), 2015-02-24 20:02:00]

I believe SkPixelRef::isLocked() has been taken care of
(https://codereview.chromium.org/940083002).

PTAL.

[reed2, 2015-02-24 20:18:20]

reed@chromium.org changed reviewers:
+ reed@chromium.org

[reed2, 2015-02-24 20:18:20]

lgtm

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

https://codereview.chromium.org/866773002/diff/40001/src/core/SkPictureShader...
src/core/SkPictureShader.h:26: virtual ~SkPictureShader() {}
can probably just remove this guy entirely.

[f(malita), 2015-02-24 20:39:30]

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

https://codereview.chromium.org/866773002/diff/40001/src/core/SkPictureShader...
src/core/SkPictureShader.h:26: virtual ~SkPictureShader() {}
On 2015/02/24 20:18:20, reed2 wrote:
> can probably just remove this guy entirely.

Done.

[f(malita), 2015-02-24 20:39:34]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2015-02-24 20:39:34]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@chromium.org
Link to the patchset: https://codereview.chromium.org/866773002/#ps60001 (title:
"review comments")

[commit-bot: I haz the power, 2015-02-24 20:39:55]

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

[commit-bot: I haz the power, 2015-02-24 21:03:01]

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

[f(malita), 2015-02-26 01:45:51]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/956143002/ by fmalita@chromium.org.

The reason for reverting is: TSAN troubles..