Reland "Fix genID cloning bugs."
authorscroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 15 Jan 2014 16:56:52 +0000 (16:56 +0000)
committerscroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 15 Jan 2014 16:56:52 +0000 (16:56 +0000)
commit5ccae2c5b134d1b971c188215433305e88491aec
treedce979407d9f01c1e5777bac01d870855ec2fd67
parent9fcd012f8c985830e75b2295fd2c9aa068dd1a80
Reland "Fix genID cloning bugs."

SkBitmap.cpp:
When copyTo calls readPixels, only clone the genID if the resulting
SkPixelRef has the same dimensions as the original. This catches a
bug where copying an SkBitmap representing the subset of an SkPixelRef
(which implements onReadPixels) would result in the copy sharing the
genID. (Thanks to r6710, this case can only happen using setPixelRef,
so the updated GpuBitmapCopyTest checks for that.)

Move some unnecessary NULL checks to asserts.

When copyTo performs a memcpy, only clone the genID if the resulting
SkPixelRef has the same dimensions as the original. This catches a bug
where copying an extracted SkBitmap with the same width as its original
SkPixelRef would incorrectly have the same genID.

Add a comment and assert in deepCopyTo, when cloning the genID, since
that case correctly clones it.

BitmapCopyTest.cpp:
Pull redundant work out of the inner loop (setting up the source bitmaps
and testing extractSubset). Create a new inner loop for extractSubset, to
test copying the result to each different config.

Extract a subset that has the same width as the original, to catch the
bug mentioned above.

Remove the reporter assert which checks for the resulting rowbytes.

Add checks to ensure that copying the extracted subset changes the genID.

GpuBitmapCopyTest:
Create an SkBitmap that shares an existing SkPixelRef, but only represents
a subset. This is to test the first call to cloneGenID in SkBitmap::copyTo.
In this case, the genID should NOT be copied, since only a portion of the
SkPixelRef was copied.

Also test deepCopy on this subset.

TestIndividualCopy now takes a parameter stating whether the genID should
change in the copy. It also does a read back using the appropriate subset.
It no longer differentiates between copyTo and deepCopyTo, since that
distinction was only necessary for copying from/to configs other than 8888
(which are no longer being tested), where copyTo did a read back in 8888 and
then drew the result to the desired config (resulting in an imperfect copy).

BUG=skia:1742

Committed: http://code.google.com/p/skia/source/detail?r=13021

R=mtklein@google.com

Review URL: https://codereview.chromium.org/112113005

git-svn-id: http://skia.googlecode.com/svn/trunk@13090 2bbb7eff-a529-9590-31e7-b0007b416f81
src/core/SkBitmap.cpp
tests/BitmapCopyTest.cpp
tests/GpuBitmapCopyTest.cpp