add bitmap::eraseArea

add bitmap::eraseArea

BUG=
R=scroggo@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9815

[scroggo, 2013-06-28 19:02:35]

lgtm

https://codereview.chromium.org/18029021/diff/2001/include/core/SkBitmap.h
File include/core/SkBitmap.h (right):

https://codereview.chromium.org/18029021/diff/2001/include/core/SkBitmap.h#ne...
include/core/SkBitmap.h:411: void eraseArea(const SkIRect& area, SkColor c)
const;
Documentation?

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
File src/core/SkBitmap.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
src/core/SkBitmap.cpp:820: if (kARGB_4444_Config == fConfig) {
Should we remove 4444?

https://codereview.chromium.org/18029021/diff/2001/tests/BitmapGetColorTest.cpp
File tests/BitmapGetColorTest.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/tests/BitmapGetColorTest.c...
tests/BitmapGetColorTest.cpp:28: static void test_equal(skiatest::Reporter*
reporter,
Maybe add a comment explaining that this function assumes specific configs for
each bitmap?

[reed1, 2013-06-28 19:31:54]

https://codereview.chromium.org/18029021/diff/2001/include/core/SkBitmap.h
File include/core/SkBitmap.h (right):

https://codereview.chromium.org/18029021/diff/2001/include/core/SkBitmap.h#ne...
include/core/SkBitmap.h:411: void eraseArea(const SkIRect& area, SkColor c)
const;
On 2013/06/28 19:02:35, scroggo wrote:
> Documentation?

Done.

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
File src/core/SkBitmap.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
src/core/SkBitmap.cpp:820: if (kARGB_4444_Config == fConfig) {
On 2013/06/28 19:02:35, scroggo wrote:
> Should we remove 4444?

This is left in until I see if/how people are creating 4444 bitmaps as
src-targets. The previous CL was intended to remove drawing/blitting support for
4444. This erase helper is sort of on the fence :)

https://codereview.chromium.org/18029021/diff/2001/tests/BitmapGetColorTest.cpp
File tests/BitmapGetColorTest.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/tests/BitmapGetColorTest.c...
tests/BitmapGetColorTest.cpp:28: static void test_equal(skiatest::Reporter*
reporter,
On 2013/06/28 19:02:35, scroggo wrote:
> Maybe add a comment explaining that this function assumes specific configs for
> each bitmap?

Done.

[urvang (Google), 2013-06-28 19:33:07]

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
File src/core/SkBitmap.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
src/core/SkBitmap.cpp:857: if (area.intersect(rect)) {
Shouldn't this be 'contains' instead? (Looking at the ASSERT inside
InternalErase()).

[reed1, 2013-06-28 19:38:38]

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
File src/core/SkBitmap.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
src/core/SkBitmap.cpp:857: if (area.intersect(rect)) {
On 2013/06/28 19:33:08, urvang wrote:
> Shouldn't this be 'contains' instead? (Looking at the ASSERT inside
> InternalErase()).

Intersect is actually computing the intersection of the bitmap (0...
width/height) and the specified rect. It returns true if the intersection is
non-empty. Thus is it ilegal for the caller to pass a rect larger than the
bitmap... I just will ignore the parts that are outside.

[reed1, 2013-06-28 19:39:14]

On 2013/06/28 19:38:38, reed1 wrote:
> https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
> File src/core/SkBitmap.cpp (right):
> 
>
https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
> src/core/SkBitmap.cpp:857: if (area.intersect(rect)) {
> On 2013/06/28 19:33:08, urvang wrote:
> > Shouldn't this be 'contains' instead? (Looking at the ASSERT inside
> > InternalErase()).
> 
> Intersect is actually computing the intersection of the bitmap (0...
> width/height) and the specified rect. It returns true if the intersection is
> non-empty. Thus is it ilegal for the caller to pass a rect larger than the
> bitmap... I just will ignore the parts that are outside.

"thus it is LEGAL for the caller ..." -- unfortunate typo above.

[urvang (Google), 2013-06-28 19:40:26]

Thanks for doing this!

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp
File src/core/SkBitmap.cpp (right):

https://codereview.chromium.org/18029021/diff/2001/src/core/SkBitmap.cpp#newc...
src/core/SkBitmap.cpp:857: if (area.intersect(rect)) {
On 2013/06/28 19:38:38, reed1 wrote:
> On 2013/06/28 19:33:08, urvang wrote:
> > Shouldn't this be 'contains' instead? (Looking at the ASSERT inside
> > InternalErase()).
> 
> Intersect is actually computing the intersection of the bitmap (0...
> width/height) and the specified rect. It returns true if the intersection is
> non-empty. Thus is it ilegal for the caller to pass a rect larger than the
> bitmap... I just will ignore the parts that are outside.

Ahh, sgtm

[reed1, 2013-06-28 19:40:53]

Committed patchset #3 manually as r9815 (presubmit successful).