add readPixels() to SkBitmap

include/core/SkBitmap.h

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkBitmap_DEFINED
 #define SkBitmap_DEFINED
 
@@ skipping 573 matching lines @@
      *                   will be used.
      *  @return true if the copy was made.
      */
     bool copyTo(SkBitmap* dst, SkColorType ct, Allocator* = NULL) const;
 
     bool copyTo(SkBitmap* dst, Allocator* allocator = NULL) const {
         return this->copyTo(dst, this->colorType(), allocator);
     }
 
     /**
+     *  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.
      */
     bool canCopyTo(SkColorType colorType) const;
 
     /** Makes a deep copy of this bitmap, keeping the copied pixels
      *  in the same domain as the source: If the src pixels are allocated for
      *  the cpu, then so will the dst. If the src pixels are allocated on the
      *  gpu (typically as a texture), the it will do the same for the dst.
      *  If the request cannot be fulfilled, returns false and dst is unmodified.
@@ skipping 244 matching lines @@
 #ifdef SK_SUPPORT_LEGACY_BITMAP_CONFIG
 ///////////////////////////////////////////////////////////////////////////////
 //
 // Helpers until we can fully deprecate SkBitmap::Config
 //
 SK_API SkBitmap::Config SkColorTypeToBitmapConfig(SkColorType);
 SK_API SkColorType SkBitmapConfigToColorType(SkBitmap::Config);
 #endif
 
 #endif