Restore bitmap dedup in SkPictureRecord. Cuts RAM usage of DM by half.

Restore bitmap dedup in SkPictureRecord.  Cuts RAM usage of DM by half.

This should fix our failing 32-bit test bots.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/e069400cabd0edd3db52bbf1958063d3ed12ef28

[reed1, 2014-11-12 20:00:55]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2014-11-12 20:00:56]

https://codereview.chromium.org/715423003/diff/20001/src/core/SkPictureRecord...
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/715423003/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:905: return false;
wonder if we should just compare width/height instead of full info...

https://codereview.chromium.org/715423003/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:910: SkAutoLockPixels al(a), bl(b);
1. what if a or b is backed by a texture?
2. what if a or b fails to lock?
3. this will force any lazy images to be decoded :(

https://codereview.chromium.org/715423003/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:911: SkASSERT(a.getSize() == b.getSize());
a and b can have diff rowbytes, but be "logically" the same surface-area, hence
diff getSize() values (which incorporates rowbytes).

https://codereview.chromium.org/715423003/diff/20001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:912: return 0 == memcmp(a.getPixels(),
b.getPixels(), a.getSize());
per-scanline is the safe way, given a.rowbytes may != b.rowbytes

[mtklein, 2014-11-12 20:31:56]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-11-12 20:32:49]

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

[commit-bot: I haz the power, 2014-11-12 20:32:50]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-11-13 02:32 UTC

[reed1, 2014-11-12 20:38:43]

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

https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:915: if (!a.pixelRef() && !b.pixelRef()) {
s/&&/||

https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:918: 
at this point, can we assert that neither is kUnknown_SkColorType? I think so

https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:936: const int bytesToCompare = info.width() *
info.bytesPerPixel();
size_t?

[mtklein, 2014-11-12 20:40:23]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2014-11-12 20:41:37]

The CQ bit was checked by mtklein@google.com

[mtklein, 2014-11-12 20:41:44]

On 2014/11/12 20:38:43, reed1 wrote:
>
https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
> File src/core/SkPictureRecord.cpp (right):
> 
>
https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
> src/core/SkPictureRecord.cpp:915: if (!a.pixelRef() && !b.pixelRef()) {
> s/&&/||
> 
>
https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
> src/core/SkPictureRecord.cpp:918: 
> at this point, can we assert that neither is kUnknown_SkColorType? I think so
> 
>
https://codereview.chromium.org/715423003/diff/40001/src/core/SkPictureRecord...
> src/core/SkPictureRecord.cpp:936: const int bytesToCompare = info.width() *
> info.bytesPerPixel();
> size_t?

All done.

[commit-bot: I haz the power, 2014-11-12 20:41:49]

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

[commit-bot: I haz the power, 2014-11-12 20:41:50]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-11-13 02:41 UTC

[reed1, 2014-11-12 20:47:31]

lgtm

[commit-bot: I haz the power, 2014-11-12 20:49:53]

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

[scroggo, 2015-08-06 18:54:39]

scroggo@google.com changed reviewers:
+ scroggo@google.com

[scroggo, 2015-08-06 18:54:40]

https://codereview.chromium.org/715423003/diff/60001/src/core/SkPictureRecord...
File src/core/SkPictureRecord.cpp (right):

https://codereview.chromium.org/715423003/diff/60001/src/core/SkPictureRecord...
src/core/SkPictureRecord.cpp:953: // In Chrome (and hopefully Android?) they're
typically immutable.
Alas, no, on Android, there is no Java API to make a Bitmap immutable. (At
decode time, you can decode an immutable Bitmap, but supposedly developers want
to decode and then make changes. If you need to make modifications, you cannot
then make it immutable.)