unify pixelref and image ID space, so we can share IDs when we share pixels

unify pixelref and image ID space, so we can share IDs when we share pixels

I view this as a performance opportunity, not a feature or bug fix per-se.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/80c772b2a46aea7efe0632e580fbc8233ff2a190

[reed1, 2015-07-30 19:56:02]

reed@google.com changed reviewers:
+ bsalomon@google.com, fmalita@chromium.org

[reed1, 2015-07-30 19:56:02]

will add tests to assert that we we create an image from an immutable bitmap, we
end up w/ the same ID, and we we don't, we don't.

[f(malita), 2015-07-30 20:24:26]

lgtm

https://codereview.chromium.org/1266883002/diff/1/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.chromium.org/1266883002/diff/1/include/core/SkImage.h#newc...
include/core/SkImage.h:289: fUniqueID(uniqueID ? uniqueID : NextUniqueID()) {
Nit: since we're using a constant in subclases (kNeedNewImageUniqueID), it seems
a bit odd to compare directly with 0 here.  Can you push the test down into
SkImage_Base() where the constant is visible?

https://codereview.chromium.org/1266883002/diff/1/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1266883002/diff/1/src/image/SkImage.cpp#newco...
src/image/SkImage.cpp:26: uint32_t SkImage::NextUniqueID() {
Nit: any reason to keep this wrapper around?

[reed1, 2015-07-30 21:17:08]

https://codereview.chromium.org/1266883002/diff/1/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.chromium.org/1266883002/diff/1/include/core/SkImage.h#newc...
include/core/SkImage.h:289: fUniqueID(uniqueID ? uniqueID : NextUniqueID()) {
On 2015/07/30 20:24:26, f(malita) wrote:
> Nit: since we're using a constant in subclases (kNeedNewImageUniqueID), it
seems
> a bit odd to compare directly with 0 here.  Can you push the test down into
> SkImage_Base() where the constant is visible?

Problem: that symbol is in a private header
Solution: move the constructor into .cpp

https://codereview.chromium.org/1266883002/diff/1/src/image/SkImage.cpp
File src/image/SkImage.cpp (right):

https://codereview.chromium.org/1266883002/diff/1/src/image/SkImage.cpp#newco...
src/image/SkImage.cpp:26: uint32_t SkImage::NextUniqueID() {
On 2015/07/30 20:24:26, f(malita) wrote:
> Nit: any reason to keep this wrapper around?

Done.

[reed1, 2015-07-30 21:17:40]

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

[commit-bot: I haz the power, 2015-07-30 21:17:50]

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

[commit-bot: I haz the power, 2015-07-30 21:25:14]

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

[commit-bot: I haz the power, 2015-07-30 21:25:15]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2015-07-30 21:54:37]

lgtm

[reed2, 2015-07-31 01:57:36]

The CQ bit was checked by reed@chromium.org

[reed2, 2015-07-31 01:57:36]

The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1266883002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-07-31 01:57:50]

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

[commit-bot: I haz the power, 2015-07-31 01:58:27]

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