Hide SkMallocPixelRef class befind SkPixelRef factory functions.

Hide SkMallocPixelRef class befind SkPixelRef factory functions.

Motivation:  After this lands, we can move Chrome to calling these new
functions, then move SkMallocPixelref.h from include/ to src/.

TODO:  Transition Android as well

BUG=skia:2391

[scroggo, 2014-04-04 21:21:06]

I like this direction.

Comments in line.

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
File include/core/SkPixelRef.h (right):

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
include/core/SkPixelRef.h:64: static SkPixelRef* NewPrelockedAllocate(
I know the goal is to hide SkMallocPixelRef, but I have to say I think
SkPrelockedPixelRef would be much better!

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
include/core/SkPixelRef.h:82: static SkPixelRef* NewPrelockedWithData(
I thought Mike wanted to move people away from dealing with SkPixelRef directly.
Am I misremembering that?

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
include/core/SkPixelRef.h:87: size_t offset = 0);
This seems like an odd choice, given that we switched SkBitmap to take an x,y.
That said, perhaps a byte offset is more logical here because we're talking
about an offset into an SkData?

[reed1, 2014-04-07 12:44:59]

I'm nervous about making "prelock" part of our public contract. Today I don't
think it is, rather it is just an internal optimization.

I think we should stick with x,y for local regions of a larger bitmap.

[hal.canary, 2014-04-07 14:43:02]

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
File include/core/SkPixelRef.h (right):

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
include/core/SkPixelRef.h:87: size_t offset = 0);
On 2014/04/04 21:21:06, scroggo wrote:
> This seems like an odd choice, given that we switched SkBitmap to take an x,y.
> That said, perhaps a byte offset is more logical here because we're talking
> about an offset into an SkData?

Look at our existing use of SkMallocPixelRef::NewWithData in dm/DMWriteTask.cpp.
 a size_t offset is exactly what we want there.

[mtklein, 2014-04-07 14:51:01]

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
File include/core/SkPixelRef.h (right):

https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
include/core/SkPixelRef.h:87: size_t offset = 0);
> That said, perhaps a byte offset is more logical here because we're talking
> about an offset into an SkData?

Yep, exactly.  x and y don't make sense until you're at an offset where there
are pixels.  E.g. an 8888 image after a 6 byte header.

[reed1, 2014-04-07 14:54:38]

On 2014/04/07 14:51:01, mtklein wrote:
> https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
> File include/core/SkPixelRef.h (right):
> 
>
https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
> include/core/SkPixelRef.h:87: size_t offset = 0);
> > That said, perhaps a byte offset is more logical here because we're talking
> > about an offset into an SkData?
> 
> Yep, exactly.  x and y don't make sense until you're at an offset where there
> are pixels.  E.g. an 8888 image after a 6 byte header.

Hmmm, maybe we need both then, x,y and offset. Knowing the logical rectangle is
useful for re-subsetting later it seems.

[hal.canary, 2014-04-07 14:56:29]

On 2014/04/07 14:54:38, reed1 wrote:
> On 2014/04/07 14:51:01, mtklein wrote:
> >
https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
> > File include/core/SkPixelRef.h (right):
> > 
> >
>
https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
> > include/core/SkPixelRef.h:87: size_t offset = 0);
> > > That said, perhaps a byte offset is more logical here because we're
talking
> > > about an offset into an SkData?
> > 
> > Yep, exactly.  x and y don't make sense until you're at an offset where
there
> > are pixels.  E.g. an 8888 image after a 6 byte header.
> 
> Hmmm, maybe we need both then, x,y and offset. Knowing the logical rectangle
is
> useful for re-subsetting later it seems.

rectangle subsetting is a bitmap thing.  raw byte pointers and offsets are
pixelref things.

[reed1, 2014-04-07 14:56:29]

On 2014/04/07 14:54:38, reed1 wrote:
> On 2014/04/07 14:51:01, mtklein wrote:
> >
https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef.h
> > File include/core/SkPixelRef.h (right):
> > 
> >
>
https://codereview.chromium.org/197873039/diff/10001/include/core/SkPixelRef....
> > include/core/SkPixelRef.h:87: size_t offset = 0);
> > > That said, perhaps a byte offset is more logical here because we're
talking
> > > about an offset into an SkData?
> > 
> > Yep, exactly.  x and y don't make sense until you're at an offset where
there
> > are pixels.  E.g. an 8888 image after a 6 byte header.
> 
> Hmmm, maybe we need both then, x,y and offset. Knowing the logical rectangle
is
> useful for re-subsetting later it seems.

double-hmmm. why do we need to accept SkData that doesn't point to the start of
the pixels?

[mtklein, 2014-04-07 15:05:50]

> double-hmmm. why do we need to accept SkData that doesn't point to the start
of
> the pixels?

I think this would work fine too.  SkData can do its own offsetting.

[reed1, 2014-04-07 15:06:37]

On 2014/04/07 15:05:50, mtklein wrote:
> > double-hmmm. why do we need to accept SkData that doesn't point to the start
> of
> > the pixels?
> 
> I think this would work fine too.  SkData can do its own offsetting.

zackly -- at least lets try that and see if it hurts.

[hal.canary, 2014-04-07 15:24:22]

On 2014/04/07 15:05:50, mtklein wrote:
SkData can do its own offsetting.

Duh.  Of course.  Why didn't I see that before?