Make peekPixels() usable with raster surface snapshots

Make peekPixels() usable with raster surface snapshots

SkSurface_Raster snapshots do not lock their backing bitmaps when the
pixel ref is shared - they only lock on deep-copy.

But since for raster surfaces the pixels are always in memory, I think
it would be OK to also lock in the former case.

This allows for optimized (zero-copy) reads of raster surface snapshot
data.

R=reed@google.com

Committed: https://skia.googlesource.com/skia/+/9a5d1ab54d52a912bb3ac9f74ee01bba079639e5

[f(malita), 2015-07-26 04:23:47]

WDYT about something like this?

It solves a webgl regression I've been chasing for the last few days
(snapshotting introduced and extra copy), but I can't quite convince myself it's
100% safe.

[reed1, 2015-07-27 16:07:56]

lgtm

- perhaps we fix up the other callers so we don't have to have a default arg.
- perhaps we take an enum instead of a bool (easy since this is a private
header).

https://codereview.chromium.org/1256993002/diff/1/src/image/SkImagePriv.h
File src/image/SkImagePriv.h (right):

https://codereview.chromium.org/1256993002/diff/1/src/image/SkImagePriv.h#new...
src/image/SkImagePriv.h:27: *
Perhaps a note about why one might pass true or false for the last parameter:

e.g. passing true for "lock" allows the image's peekPixels() method to succeed,
but it will force any lazy decodes/generators to execute if they exist on the
pixelref.

[f(malita), 2015-07-27 16:28:38]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-27 16:28:51]

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

[commit-bot: I haz the power, 2015-07-27 16:37:22]

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

[commit-bot: I haz the power, 2015-07-27 16:37:23]

Dry run: This issue passed the CQ dry run.

[f(malita), 2015-07-27 16:53:39]

> 
> - perhaps we fix up the other callers so we don't have to have a default arg.

done

> - perhaps we take an enum instead of a bool (easy since this is a private
> header).

done:

enum SharedPixelRefMode {
    kLocked_SharedPixelRefMode,
    kUnlocked_SharedPixelRefMode
};

https://codereview.chromium.org/1256993002/diff/1/src/image/SkImagePriv.h
File src/image/SkImagePriv.h (right):

https://codereview.chromium.org/1256993002/diff/1/src/image/SkImagePriv.h#new...
src/image/SkImagePriv.h:27: *
On 2015/07/27 16:07:56, reed1 wrote:
> Perhaps a note about why one might pass true or false for the last parameter:
> 
> e.g. passing true for "lock" allows the image's peekPixels() method to
succeed,
> but it will force any lazy decodes/generators to execute if they exist on the
> pixelref.

Done.

[f(malita), 2015-07-27 17:26:39]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2015-07-27 17:26:40]

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

[commit-bot: I haz the power, 2015-07-27 17:26:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1256993002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1256993002/20001

[commit-bot: I haz the power, 2015-07-27 17:27:32]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/9a5d1ab54d52a912bb3ac9f74ee01bba079639e5