add installPixels

add installPixels

BUG=skia:
R=halcanary@google.com

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

[reed1, 2014-01-23 21:30:49]

just previewing API before making impl

[hal.canary, 2014-01-23 21:39:10]

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

https://codereview.chromium.org/143073008/diff/1/include/core/SkBitmap.h#newc...
include/core/SkBitmap.h:268: void* context);
bool installPixels(const SkImageInfo&, void* pixels, size_t rowBytes,
                   void (*ReleaseProc)(void* pixels, void* context),
                   void* context);

For consistency wither use `void* addr` or `void* pixels`.

[reed1, 2014-01-23 21:39:13]

ptal

[scroggo, 2014-01-23 21:53:01]

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

https://codereview.chromium.org/143073008/diff/40001/include/core/SkBitmap.h#...
include/core/SkBitmap.h:257: bool allocPixels(const SkImageInfo&, SkColorTable*
ctable = NULL);
I assume these are to replace the existing setConfig + allocPixels. Does it make
sense to continue to allow the caller to specify how the pixels are allocated?
Or will they just then be forced to call installPixels?

Currently, Android passes an SkBitmap::Allocator to the image decoders. Their
special Allocator provides a Java allocation of memory that they can reuse for
future bitmaps. How can we continue to support this pattern? Perhaps their
Allocator can call installPixels?

[hal.canary, 2014-01-23 22:05:24]

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

https://codereview.chromium.org/143073008/diff/40001/include/core/SkBitmap.h#...
include/core/SkBitmap.h:257: bool allocPixels(const SkImageInfo&, SkColorTable*
ctable = NULL);
On 2014/01/23 21:53:02, scroggo wrote:
> Allocator can call installPixels?

Why not:

    bool allocPixels(const SkImageInfo&, 
                     Allocator*,
                     SkColorTable*);

[hal.canary, 2014-01-23 22:27:50]

We should modify the tests in tests/MallocPixelRefTest.cpp to call these
functions.

[reed1, 2014-01-24 14:54:48]

ptal

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

https://codereview.chromium.org/143073008/diff/40001/include/core/SkBitmap.h#...
include/core/SkBitmap.h:257: bool allocPixels(const SkImageInfo&, SkColorTable*
ctable = NULL);
On 2014/01/23 21:53:02, scroggo wrote:
> I assume these are to replace the existing setConfig + allocPixels. Does it
make
> sense to continue to allow the caller to specify how the pixels are allocated?
> Or will they just then be forced to call installPixels?
> 
> Currently, Android passes an SkBitmap::Allocator to the image decoders. Their
> special Allocator provides a Java allocation of memory that they can reuse for
> future bitmaps. How can we continue to support this pattern? Perhaps their
> Allocator can call installPixels?

Done.

[scroggo, 2014-01-24 16:18:58]

1gtm, besides the pr, which needs to be unref'ed.

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

https://codereview.chromium.org/143073008/diff/140001/src/core/SkBitmap.cpp#n...
src/core/SkBitmap.cpp:523: return this->setPixelRef(pr);
pr needs to be unref'ed.

[hal.canary, 2014-01-24 16:46:50]

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

https://codereview.chromium.org/143073008/diff/140001/src/core/SkBitmap.cpp#n...
src/core/SkBitmap.cpp:523: return this->setPixelRef(pr);
On 2014/01/24 16:18:59, scroggo wrote:
> pr needs to be unref'ed.

Use a SkAutoTUnref<SkPixelRef>!

[commit-bot: I haz the power, 2014-01-24 17:18:05]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[hal.canary, 2014-01-24 18:51:45]

lgtm

[reed1, 2014-01-24 18:53:56]

Committed patchset #6 manually as r13174 (presubmit successful).