new tinybitmap gm

[Steve VanDeBogart, 2011-07-18 15:13:06]

What's the expected result? 0xFF0000 or 0x800000?  I would guess the later (what
PDF does), but the raster engine seems to produce the former.

http://codereview.appspot.com/4674048/diff/1/tinybitmap.cpp
File tinybitmap.cpp (right):

http://codereview.appspot.com/4674048/diff/1/tinybitmap.cpp#newcode48
tinybitmap.cpp:48: void drawBG(SkCanvas* canvas) {
drawBG isn't used in this test.  I'm guessing you want to call it from onDraw.

[bsalomon, 2011-07-18 17:21:24]

Sorry I just slapped the test up here once I realized it crashed in the pdf
code. The previous version didn't generate correct gpu or raster images because
the device BG color wasn't set as you noticed.

In the new version I set the paint alpha to 50%. So the final color should be (1
- .5 * .5) * (DDDDDD) + .5 * (800000) = E5A5A5A5. This actually exercises the
code path where the gpu code previously had a bug.

http://codereview.appspot.com/4674048/diff/1/tinybitmap.cpp
File tinybitmap.cpp (right):

http://codereview.appspot.com/4674048/diff/1/tinybitmap.cpp#newcode48
tinybitmap.cpp:48: void drawBG(SkCanvas* canvas) {
Oh yes that's true, it was a vestige of copying from SampleApp.

On 2011/07/18 15:13:06, Steve VanDeBogart wrote:
> drawBG isn't used in this test.  I'm guessing you want to call it from onDraw.

[bsalomon, 2011-07-18 19:54:51]

On 2011/07/18 17:21:24, bsalomon wrote:
> - .5 * .5) * (DDDDDD) + .5 * (800000) = E5A5A5A5.

One too many A5's in there, should be E5A5A5 :)

[Steve VanDeBogart, 2011-07-18 20:06:08]

Looks like you list gm/tinybitmap.cpp

[bsalomon, 2011-07-18 20:19:35]

On 2011/07/18 20:06:08, Steve VanDeBogart wrote:
> Looks like you list gm/tinybitmap.cpp

I forgot to re-add tinybitmap.cpp to svn. It's up now.

[Steve VanDeBogart, 2011-07-18 20:26:46]

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp
File gm/tinybitmap.cpp (right):

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode11
gm/tinybitmap.cpp:11: const int N = 1;
This seems a bit overkill.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode24
gm/tinybitmap.cpp:24: for (int y = 0; y < bm.height(); y++) {
The for loops seem unnecessary as well, with the size being statically set to
1x1

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode37
gm/tinybitmap.cpp:37: TinyBitmapGM() {
nit: indent

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode46
gm/tinybitmap.cpp:46: virtual SkISize onISize() { return make_isize(400, 300); }
Would we save anything appreciable on baseline image sizes by making this
smaller?

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode50
gm/tinybitmap.cpp:50: SkShader* s = SkShader::CreateBitmapShader(fBM,
SkShader::kRepeat_TileMode,
nit: line length.

[bsalomon, 2011-07-18 20:44:45]

I'm not sure why the bitmap size could be changed at compile time. Maybe for
debugging? I lifted this code from SampleTinyBitmap.cpp.

Anyway, new upload addresses the comments.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp
File gm/tinybitmap.cpp (right):

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode11
gm/tinybitmap.cpp:11: const int N = 1;
On 2011/07/18 20:26:46, Steve VanDeBogart wrote:
> This seems a bit overkill.

Done.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode24
gm/tinybitmap.cpp:24: for (int y = 0; y < bm.height(); y++) {
On 2011/07/18 20:26:46, Steve VanDeBogart wrote:
> The for loops seem unnecessary as well, with the size being statically set to
> 1x1

Done.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode37
gm/tinybitmap.cpp:37: TinyBitmapGM() {
On 2011/07/18 20:26:46, Steve VanDeBogart wrote:
> nit: indent

Done.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode46
gm/tinybitmap.cpp:46: virtual SkISize onISize() { return make_isize(400, 300); }
On 2011/07/18 20:26:46, Steve VanDeBogart wrote:
> Would we save anything appreciable on baseline image sizes by making this
> smaller?

Done.

http://codereview.appspot.com/4674048/diff/14001/gm/tinybitmap.cpp#newcode50
gm/tinybitmap.cpp:50: SkShader* s = SkShader::CreateBitmapShader(fBM,
SkShader::kRepeat_TileMode,
On 2011/07/18 20:26:46, Steve VanDeBogart wrote:
> nit: line length.

Done.

[Steve VanDeBogart, 2011-07-18 21:16:19]

LGTM

[bsalomon, 2011-07-18 21:34:11]

Closed with r1888 thru r1890 (multiple checkins to get baselines on mac, and
linux)