replace setConfig+allocPixels with alloc-or-install-pixels

replace setConfig+allocPixels with alloc-or-install-pixels

BUG=skia:

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

[scroggo, 2014-02-13 20:26:03]

lgtm with comments

https://codereview.chromium.org/164203003/diff/40017/tests/BlitRowTest.cpp
File tests/BlitRowTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/BlitRowTest.cpp#ne...
tests/BlitRowTest.cpp:15: // these are in the same order as the SkBitmap::Config
enum
These are now in the same order as SkColorType

https://codereview.chromium.org/164203003/diff/40017/tests/DrawPathTest.cpp
File tests/DrawPathTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/DrawPathTest.cpp#n...
tests/DrawPathTest.cpp:14: static SkCanvas* create(int w, int h, int rb, void*
addr = NULL) {
It looks like create can be merged into new_canvas (no one else calls it) and rb
and addr are never anything but 0 and NULL.

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cpp
File tests/WritePixelsTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cp...
tests/WritePixelsTest.cpp:316: SkPixelRef* pr =
SkMallocPixelRef::NewAllocate(info, rowBytes, NULL);
Won't allocPixels do this for you?

[reed1, 2014-02-13 20:48:36]

https://codereview.chromium.org/164203003/diff/40017/tests/BlitRowTest.cpp
File tests/BlitRowTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/BlitRowTest.cpp#ne...
tests/BlitRowTest.cpp:15: // these are in the same order as the SkBitmap::Config
enum
On 2014/02/13 20:26:03, scroggo wrote:
> These are now in the same order as SkColorType

Done.

https://codereview.chromium.org/164203003/diff/40017/tests/DrawPathTest.cpp
File tests/DrawPathTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/DrawPathTest.cpp#n...
tests/DrawPathTest.cpp:14: static SkCanvas* create(int w, int h, int rb, void*
addr = NULL) {
On 2014/02/13 20:26:03, scroggo wrote:
> It looks like create can be merged into new_canvas (no one else calls it) and
rb
> and addr are never anything but 0 and NULL.

Done.

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cpp
File tests/WritePixelsTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cp...
tests/WritePixelsTest.cpp:316: SkPixelRef* pr =
SkMallocPixelRef::NewAllocate(info, rowBytes, NULL);
On 2014/02/13 20:26:03, scroggo wrote:
> Won't allocPixels do this for you?

allocPixels doesn't let me pass in a rowBytes, which is deliberate, as we're
trying to convince our clients to *not* specify the rowBytes unless they are
explicitly specifying the pixelref at the same time.

I have/had a CL for installPixelRef() that encapsulates this, but have had other
problems landing it. It may be landable now/soon.

[reed1, 2014-02-13 20:50:20]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-02-13 20:50:26]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/reed@chromium.org/164203003/140001

[scroggo, 2014-02-13 21:01:14]

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cpp
File tests/WritePixelsTest.cpp (right):

https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cp...
tests/WritePixelsTest.cpp:316: SkPixelRef* pr =
SkMallocPixelRef::NewAllocate(info, rowBytes, NULL);
On 2014/02/13 20:48:37, reed1 wrote:
> On 2014/02/13 20:26:03, scroggo wrote:
> > Won't allocPixels do this for you?
> 
> allocPixels doesn't let me pass in a rowBytes, which is deliberate, as we're
> trying to convince our clients to *not* specify the rowBytes unless they are
> explicitly specifying the pixelref at the same time.
> 
> I have/had a CL for installPixelRef() that encapsulates this, but have had
other
> problems landing it. It may be landable now/soon.

The version of allocPixels that calls the standard allocator respects the
rowbytes on the bitmap (which you set above). Maybe in some future world you
won't be able to modify the rowbytes in setConfig (since it won't be known
unless locked), but for now allocPixels should work fine here.

[reed1, 2014-02-13 21:05:22]

On 2014/02/13 21:01:14, scroggo wrote:
> https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cpp
> File tests/WritePixelsTest.cpp (right):
> 
>
https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cp...
> tests/WritePixelsTest.cpp:316: SkPixelRef* pr =
> SkMallocPixelRef::NewAllocate(info, rowBytes, NULL);
> On 2014/02/13 20:48:37, reed1 wrote:
> > On 2014/02/13 20:26:03, scroggo wrote:
> > > Won't allocPixels do this for you?
> > 
> > allocPixels doesn't let me pass in a rowBytes, which is deliberate, as we're
> > trying to convince our clients to *not* specify the rowBytes unless they are
> > explicitly specifying the pixelref at the same time.
> > 
> > I have/had a CL for installPixelRef() that encapsulates this, but have had
> other
> > problems landing it. It may be landable now/soon.
> 
> The version of allocPixels that calls the standard allocator respects the
> rowbytes on the bitmap (which you set above). Maybe in some future world you
> won't be able to modify the rowbytes in setConfig (since it won't be known
> unless locked), but for now allocPixels should work fine here.

Ah, you are referring to allocPixels() with no (visible) arguments?

Yes, tho I think we should stop using that too, since it relys on the bitmap
knowing a rowBytes up front, without any pixelRef installed.

[commit-bot: I haz the power, 2014-02-13 22:00:11]

Change committed as 13442

[hal.canary, 2014-02-14 14:44:31]

On 2014/02/13 21:05:22, reed1 wrote:
> On 2014/02/13 21:01:14, scroggo wrote:
> >
https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cpp
> > File tests/WritePixelsTest.cpp (right):
> > 
> >
>
https://codereview.chromium.org/164203003/diff/40017/tests/WritePixelsTest.cp...
> > tests/WritePixelsTest.cpp:316: SkPixelRef* pr =
> > SkMallocPixelRef::NewAllocate(info, rowBytes, NULL);
> > On 2014/02/13 20:48:37, reed1 wrote:
> > > On 2014/02/13 20:26:03, scroggo wrote:
> > > > Won't allocPixels do this for you?
> > > 
> > > allocPixels doesn't let me pass in a rowBytes, which is deliberate, as
we're
> > > trying to convince our clients to *not* specify the rowBytes unless they
are
> > > explicitly specifying the pixelref at the same time.
> > > 
> > > I have/had a CL for installPixelRef() that encapsulates this, but have had
> > other
> > > problems landing it. It may be landable now/soon.
> > 
> > The version of allocPixels that calls the standard allocator respects the
> > rowbytes on the bitmap (which you set above). Maybe in some future world you
> > won't be able to modify the rowbytes in setConfig (since it won't be known
> > unless locked), but for now allocPixels should work fine here.
> 
> Ah, you are referring to allocPixels() with no (visible) arguments?
> 
> Yes, tho I think we should stop using that too, since it relys on the bitmap
> knowing a rowBytes up front, without any pixelRef installed.

We should be clear in SkBitmap::Allocator's documentation whether it is allowed
to modify rowBytes, required to respect it, or just required to validate it.  Or
we can mark SkBitmap::Allocator as deprecated and replace it with the version of
allocPixels that uses a SkPixelRefFactory.