add readPixels() to SkBitmap

include/core/SkBitmap.h

Index: include/core/SkBitmap.h
diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 51e9a9243a13b156ab8591d888b79bbbd11588e6..e97548c2cf6b36252c15a6f3da49f1ea5d879157 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -591,6 +591,27 @@ public:
     }
 
     /**
+     *  Copy the bitmap's pixels into the specified buffer (pixels + rowBytes),
+     *  converting them into the requested format (SkImageInfo). The src pixels are read
+     *  starting at the specified (x,y) offset, relative to the top-left corner.
+     *
+     *  The specified ImageInfo and (x,y) offset specifies a source rectangle
+     *
+     *      srcR.setXYWH(x, y, info.width(), info.height());
+     *
+     *  SrcR is intersected with the bounds of the bitmap. If this intersection is not empty,
+     *  then we have two sets of pixels (of equal size), the "src" beginning at (x,y)
+     *  and the "dst" by info+pixels+rowBytes. Replace the dst pixels with the corresponding src
+     *  pixels, performing any colortype/alphatype transformations needed (in the case where the
+     *  src and dst have different colortypes or alphatypes).
+     *
+     *  This call can fail, returning false, for several reasons:
+     *  - If the requested colortype/alphatype cannot be converted from the src's types.
+     *  - If the src pixels are not available.
+     */
+    bool readPixels(const SkImageInfo&, void* pixels, size_t rowBytes, int x, int y) const;

[scroggo, 2014/07/09 15:22:02]

I take it the intent to perform the work done in copyTo here? Might there be a
better name that makes it more clear that it might end up doing conversions
(e.g. SkDitherARGB32To4444)? copyTo also does more than a simple "copy", but I
think it makes more sense because it means copying a bitmap (which in turn does
more work on the pixels), but this function does more work on the pixels
themselves.

It looks like SkCanvas::readPixels behaves similarly, so it's at least
consistent.

Should we make the names of the parameters explicit about what they refer to?
Maybe it's obvious enough that x and y refer to the source, and the others refer
to the destination.

Otherwise this seems like a good API to me.

Will it work for texture-backed bitmaps? I'm guessing not, since those won't
always exist and it allows us to keep this simpler. Maybe we should document
that?

[reed1, 2014/07/10 14:10:40]

This function is intended to be a mirror of canvas::readPixels, which already
does pixel transformations, so I think that's consistent.

It will be the underside impl for copyTo, but will not perform any memory
allocations.
Good idea, will try renaming them.
It will, as long as bitmaps can be texture backed. This is consistent with
canvas::readPixels, which works if the canvas is texture/gpu backed.

+
+    /**
      *  Returns true if this bitmap's pixels can be converted into the requested
      *  colorType, such that copyTo() could succeed.
      */