Make readpixels work on GrTextures
authorbsalomon <bsalomon@google.com>
Thu, 16 Jul 2015 15:23:13 +0000 (08:23 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 16 Jul 2015 15:23:13 +0000 (08:23 -0700)
Review URL: https://codereview.chromium.org/1234313002

include/gpu/GrContext.h
src/gpu/GrContext.cpp
src/gpu/GrSurface.cpp
src/gpu/GrSurfacePriv.h
src/gpu/gl/GrGLGpu.cpp
src/image/SkImage_Gpu.cpp
tests/ReadPixelsTest.cpp

index 370830e..65fac23 100644 (file)
@@ -228,8 +228,8 @@ public:
     };
 
     /**
-     * Reads a rectangle of pixels from a render target.
-     * @param target        the render target to read from.
+     * Reads a rectangle of pixels from a surface.
+     * @param surface       the surface to read from.
      * @param left          left edge of the rectangle to read (inclusive)
      * @param top           top edge of the rectangle to read (inclusive)
      * @param width         width of rectangle to read in pixels.
@@ -241,14 +241,13 @@ public:
      * @param pixelOpsFlags see PixelOpsFlags enum above.
      *
      * @return true if the read succeeded, false if not. The read can fail because of an unsupported
-     *         pixel config or because no render target is currently set and NULL was passed for
-     *         target.
+     *         pixel configs
      */
-    bool readRenderTargetPixels(GrRenderTarget* target,
-                                int left, int top, int width, int height,
-                                GrPixelConfig config, void* buffer,
-                                size_t rowBytes = 0,
-                                uint32_t pixelOpsFlags = 0);
+    bool readSurfacePixels(GrSurface* surface,
+                           int left, int top, int width, int height,
+                           GrPixelConfig config, void* buffer,
+                           size_t rowBytes = 0,
+                           uint32_t pixelOpsFlags = 0);
 
     /**
      * Writes a rectangle of pixels to a surface.
index 1c60074..17b4b74 100755 (executable)
@@ -347,6 +347,14 @@ bool GrContext::writeSurfacePixels(GrSurface* surface,
         }
     }
 
+    // Trim the params here so that if we wind up making a temporary surface it can be as small as
+    // necessary.
+    if (!GrSurfacePriv::AdjustWritePixelParams(surface->width(), surface->height(),
+                                               GrBytesPerPixel(srcConfig), &left, &top, &width,
+                                               &height, &buffer, &rowBytes)) {
+        return false;
+    }
+
     // If we didn't do a direct texture write then we upload the pixels to a texture and draw.
     GrRenderTarget* renderTarget = surface->asRenderTarget();
     if (!renderTarget) {
@@ -452,25 +460,28 @@ static SkColorType toggle_colortype32(SkColorType ct) {
     }
 }
 
-bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
-                                       int left, int top, int width, int height,
-                                       GrPixelConfig dstConfig, void* buffer, size_t rowBytes,
-                                       uint32_t flags) {
+bool GrContext::readSurfacePixels(GrSurface* src,
+                                  int left, int top, int width, int height,
+                                  GrPixelConfig dstConfig, void* buffer, size_t rowBytes,
+                                  uint32_t flags) {
     RETURN_FALSE_IF_ABANDONED
-    ASSERT_OWNED_RESOURCE(target);
-    SkASSERT(target);
+    ASSERT_OWNED_RESOURCE(src);
+    SkASSERT(src);
+
+    // Adjust the params so that if we wind up using an intermediate surface we've already done
+    // all the trimming and the temporary can be the min size required.
+    if (!GrSurfacePriv::AdjustReadPixelParams(src->width(), src->height(),
+                                              GrBytesPerPixel(dstConfig), &left,
+                                              &top, &width, &height, &buffer, &rowBytes)) {
+        return false;
+    }
 
-    if (!(kDontFlush_PixelOpsFlag & flags) && target->surfacePriv().hasPendingWrite()) {
+    if (!(kDontFlush_PixelOpsFlag & flags) && src->surfacePriv().hasPendingWrite()) {
         this->flush();
     }
 
     // Determine which conversions have to be applied: flipY, swapRAnd, and/or unpremul.
 
-    // If fGpu->readPixels would incur a y-flip cost then we will read the pixels upside down. We'll
-    // either do the flipY by drawing into a scratch with a matrix or on the cpu after the read.
-    bool flipY = fGpu->readPixelsWillPayForYFlip(target, left, top,
-                                                 width, height, dstConfig,
-                                                 rowBytes);
     // We ignore the preferred config if it is different than our config unless it is an R/B swap.
     // In that case we'll perform an R and B swap while drawing to a scratch texture of the swapped
     // config. Then we will call readPixels on the scratch with the swapped config. The swaps during
@@ -479,13 +490,23 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
     GrPixelConfig readConfig = dstConfig;
     bool swapRAndB = false;
     if (GrPixelConfigSwapRAndB(dstConfig) ==
-        fGpu->preferredReadPixelsConfig(dstConfig, target->config())) {
+        fGpu->preferredReadPixelsConfig(dstConfig, src->config())) {
         readConfig = GrPixelConfigSwapRAndB(readConfig);
         swapRAndB = true;
     }
 
-    bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags);
+    bool flipY = false;
+    GrRenderTarget* srcAsRT = src->asRenderTarget();
+    if (srcAsRT) {
+        // If fGpu->readPixels would incur a y-flip cost then we will read the pixels upside down.
+        // We'll either do the flipY by drawing into a scratch with a matrix or on the cpu after the
+        // read.
+        flipY = fGpu->readPixelsWillPayForYFlip(srcAsRT, left, top,
+                                                width, height, dstConfig,
+                                                rowBytes);
+    }
 
+    bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags);
     if (unpremul && !GrPixelConfigIs8888(dstConfig)) {
         // The unpremul flag is only allowed for these two configs.
         return false;
@@ -496,9 +517,11 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
     // If the src is a texture and we would have to do conversions after read pixels, we instead
     // do the conversions by drawing the src to a scratch texture. If we handle any of the
     // conversions in the draw we set the corresponding bool to false so that we don't reapply it
-    // on the read back pixels.
-    GrTexture* src = target->asTexture();
-    if (src && (swapRAndB || unpremul || flipY)) {
+    // on the read back pixels. We also do an intermediate draw if the src is not a render target as
+    // GrGpu currently supports reading from render targets but not textures.
+    GrTexture* srcAsTex = src->asTexture();
+    GrRenderTarget* rtToRead = srcAsRT;
+    if (srcAsTex && (swapRAndB || unpremul || flipY || !srcAsRT)) {
         // Make the scratch a render so we can read its pixels.
         GrSurfaceDesc desc;
         desc.fFlags = kRenderTarget_GrSurfaceFlag;
@@ -514,8 +537,8 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
         GrTextureProvider::ScratchTexMatch match = GrTextureProvider::kApprox_ScratchTexMatch;
         if (0 == left &&
             0 == top &&
-            target->width() == width &&
-            target->height() == height &&
+            src->width() == width &&
+            src->height() == height &&
             fGpu->fullReadPixelsIsFasterThanPartial()) {
             match = GrTextureProvider::kExact_ScratchTexMatch;
         }
@@ -529,18 +552,18 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
             GrPaint paint;
             SkAutoTUnref<const GrFragmentProcessor> fp;
             if (unpremul) {
-                fp.reset(this->createPMToUPMEffect(paint.getProcessorDataManager(), src, swapRAndB,
-                                                   textureMatrix));
+                fp.reset(this->createPMToUPMEffect(paint.getProcessorDataManager(), srcAsTex,
+                                                   swapRAndB, textureMatrix));
                 if (fp) {
                     unpremul = false; // we no longer need to do this on CPU after the read back.
                 }
             }
             // If we failed to create a PM->UPM effect and have no other conversions to perform then
             // there is no longer any point to using the scratch.
-            if (fp || flipY || swapRAndB) {
+            if (fp || flipY || swapRAndB || !srcAsRT) {
                 if (!fp) {
                     fp.reset(GrConfigConversionEffect::Create(paint.getProcessorDataManager(),
-                            src, swapRAndB, GrConfigConversionEffect::kNone_PMConversion,
+                            srcAsTex, swapRAndB, GrConfigConversionEffect::kNone_PMConversion,
                             textureMatrix));
                 }
                 swapRAndB = false; // we will handle the swap in the draw.
@@ -564,16 +587,15 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
                     // we want to read back from the scratch's origin
                     left = 0;
                     top = 0;
-                    target = tempTexture->asRenderTarget();
+                    rtToRead = tempTexture->asRenderTarget();
                 }
-                this->flushSurfaceWrites(target);
+                this->flushSurfaceWrites(tempTexture);
             }
         }
     }
 
-    if (!fGpu->readPixels(target,
-                          left, top, width, height,
-                          readConfig, buffer, rowBytes)) {
+    if (!rtToRead ||
+        !fGpu->readPixels(rtToRead, left, top, width, height, readConfig, buffer, rowBytes)) {
         return false;
     }
     // Perform any conversions we weren't able to perform using a scratch texture.
index 9685c56..9bd9e8b 100644 (file)
 #include "SkImageEncoder.h"
 #include <stdio.h>
 
+template<typename T> static bool adjust_params(int surfaceWidth,
+                                               int surfaceHeight,
+                                               size_t bpp,
+                                               int* left, int* top, int* width, int* height,
+                                               T** data,
+                                               size_t* rowBytes) {
+    if (!*rowBytes) {
+        *rowBytes = *width * bpp;
+    }
+
+    SkIRect subRect = SkIRect::MakeXYWH(*left, *top, *width, *height);
+    SkIRect bounds = SkIRect::MakeWH(surfaceWidth, surfaceHeight);
+
+    if (!subRect.intersect(bounds)) {
+        return false;
+    }
+    *data = reinterpret_cast<void*>(reinterpret_cast<intptr_t>(*data) +
+            (subRect.fTop - *top) * *rowBytes + (subRect.fLeft - *left) * bpp);
+
+    *left = subRect.fLeft;
+    *top = subRect.fTop;
+    *width = subRect.width();
+    *height = subRect.height();
+    return true;
+}
+
+bool GrSurfacePriv::AdjustReadPixelParams(int surfaceWidth,
+                                          int surfaceHeight,
+                                          size_t bpp,
+                                          int* left, int* top, int* width, int* height,
+                                          void** data,
+                                          size_t* rowBytes) {
+    return adjust_params<void>(surfaceWidth, surfaceHeight, bpp, left, top, width, height, data,
+                               rowBytes);
+}
+
+bool GrSurfacePriv::AdjustWritePixelParams(int surfaceWidth,
+                                           int surfaceHeight,
+                                           size_t bpp,
+                                           int* left, int* top, int* width, int* height,
+                                           const void** data,
+                                           size_t* rowBytes) {
+    return adjust_params<const void>(surfaceWidth, surfaceHeight, bpp, left, top, width, height,
+                                     data, rowBytes);
+}
+
+
+//////////////////////////////////////////////////////////////////////////////
+
 bool GrSurface::writePixels(int left, int top, int width, int height,
                             GrPixelConfig config, const void* buffer, size_t rowBytes,
                             uint32_t pixelOpsFlags) {
@@ -33,12 +82,8 @@ bool GrSurface::readPixels(int left, int top, int width, int height,
     if (NULL == context) {
         return false;
     }
-    GrRenderTarget* target = this->asRenderTarget();
-    if (target) {
-        return context->readRenderTargetPixels(target, left, top, width, height, config, buffer,
-                                               rowBytes, pixelOpsFlags);
-    }
-    return false;
+    return context->readSurfacePixels(this, left, top, width, height, config, buffer,
+                                      rowBytes, pixelOpsFlags);
 }
 
 SkImageInfo GrSurface::info(SkAlphaType alphaType) const {
index 47bd44f..bff411e 100644 (file)
     implemented privately in GrSurface with a inline public method here). */
 class GrSurfacePriv {
 public:
+    /** Helpers used in read/write pixels implementations. The paramters are adjusted so that the
+        read/write respects the bounds of a surface. If the input *rowBytes is 0 it will be
+        the tight row bytes (based on width and bpp) on output. */
+    static bool AdjustReadPixelParams(int surfaceWidth,
+                                      int surfaceHeight,
+                                      size_t bpp,
+                                      int* left, int* top, int* width, int* height,
+                                      void** data,
+                                      size_t* rowBytes);
+    static bool AdjustWritePixelParams(int surfaceWidth,
+                                      int surfaceHeight,
+                                      size_t bpp,
+                                      int* left, int* top, int* width, int* height,
+                                      const void** data,
+                                      size_t* rowBytes);
     /**
      * Derive a SkImageInfo from the surface's descriptor. The caller must provide the alpha type as
      * GrSurface has no equivalent.
index 5c03ab5..1a9566a 100644 (file)
@@ -596,32 +596,6 @@ bool GrGLGpu::onWriteTexturePixels(GrTexture* texture,
     return false;
 }
 
-static bool adjust_pixel_ops_params(int surfaceWidth,
-                                    int surfaceHeight,
-                                    size_t bpp,
-                                    int* left, int* top, int* width, int* height,
-                                    const void** data,
-                                    size_t* rowBytes) {
-    if (!*rowBytes) {
-        *rowBytes = *width * bpp;
-    }
-
-    SkIRect subRect = SkIRect::MakeXYWH(*left, *top, *width, *height);
-    SkIRect bounds = SkIRect::MakeWH(surfaceWidth, surfaceHeight);
-
-    if (!subRect.intersect(bounds)) {
-        return false;
-    }
-    *data = reinterpret_cast<const void*>(reinterpret_cast<intptr_t>(*data) +
-          (subRect.fTop - *top) * *rowBytes + (subRect.fLeft - *left) * bpp);
-
-    *left = subRect.fLeft;
-    *top = subRect.fTop;
-    *width = subRect.width();
-    *height = subRect.height();
-    return true;
-}
-
 static inline GrGLenum check_alloc_error(const GrSurfaceDesc& desc,
                                          const GrGLInterface* interface) {
     if (SkToBool(desc.fFlags & kCheckAllocation_GrSurfaceFlag)) {
@@ -643,8 +617,8 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
     SkASSERT(!GrPixelConfigIsCompressed(dataConfig));
 
     size_t bpp = GrBytesPerPixel(dataConfig);
-    if (!adjust_pixel_ops_params(desc.fWidth, desc.fHeight, bpp, &left, &top,
-                                 &width, &height, &data, &rowBytes)) {
+    if (!GrSurfacePriv::AdjustWritePixelParams(desc.fWidth, desc.fHeight, bpp, &left, &top,
+                                               &width, &height, &data, &rowBytes)) {
         return false;
     }
     size_t trimRowBytes = width * bpp;
@@ -1781,10 +1755,10 @@ bool GrGLGpu::onReadPixels(GrRenderTarget* target,
         return false;
     }
     size_t bpp = GrBytesPerPixel(config);
-    if (!adjust_pixel_ops_params(target->width(), target->height(), bpp,
-                                 &left, &top, &width, &height,
-                                 const_cast<const void**>(&buffer),
-                                 &rowBytes)) {
+    if (!GrSurfacePriv::AdjustReadPixelParams(target->width(), target->height(), bpp,
+                                              &left, &top, &width, &height,
+                                              &buffer,
+                                              &rowBytes)) {
         return false;
     }
 
index a3fe022..31ae5a9 100644 (file)
@@ -248,8 +248,6 @@ GrTexture* GrDeepCopyTexture(GrTexture* src, bool budgeted) {
     GrContext* ctx = src->getContext();
 
     GrSurfaceDesc desc = src->desc();
-    // need to be a rendertarget for readpixels to work, instead of kNone_GrSurfaceFlags
-    desc.fFlags = kRenderTarget_GrSurfaceFlag;
     GrTexture* dst = ctx->textureProvider()->createTexture(desc, budgeted, NULL, 0);
     if (!dst) {
         return NULL;
index 59c826f..9e9f53c 100644 (file)
@@ -15,6 +15,7 @@
 #if SK_SUPPORT_GPU
 #include "GrContextFactory.h"
 #include "SkGpuDevice.h"
+#include "SkGr.h"
 #endif
 
 static const int DEV_W = 100, DEV_H = 100;
@@ -22,7 +23,7 @@ static const SkIRect DEV_RECT = SkIRect::MakeWH(DEV_W, DEV_H);
 static const SkRect DEV_RECT_S = SkRect::MakeWH(DEV_W * SK_Scalar1,
                                                 DEV_H * SK_Scalar1);
 
-static SkPMColor getCanvasColor(int x, int y) {
+static SkPMColor get_src_color(int x, int y) {
     SkASSERT(x >= 0 && x < DEV_W);
     SkASSERT(y >= 0 && y < DEV_H);
 
@@ -50,8 +51,8 @@ static SkPMColor getCanvasColor(int x, int y) {
     }
     return SkPremultiplyARGBInline(a, r, g, b);
 }
-
-static SkPMColor getBitmapColor(int x, int y, int w) {
+    
+static SkPMColor get_dst_bmp_init_color(int x, int y, int w) {
     int n = y * w + x;
 
     U8CPU b = n & 0xff;
@@ -60,8 +61,8 @@ static SkPMColor getBitmapColor(int x, int y, int w) {
     return SkPackARGB32(0xff, r, g , b);
 }
 
-static SkPMColor convertToPMColor(SkColorType ct, SkAlphaType at, const uint32_t* addr,
-                                  bool* doUnpremul) {
+static SkPMColor convert_to_pmcolor(SkColorType ct, SkAlphaType at, const uint32_t* addr,
+                                    bool* doUnpremul) {
     *doUnpremul = (kUnpremul_SkAlphaType == at);
 
     const uint8_t* c = reinterpret_cast<const uint8_t*>(addr);
@@ -92,7 +93,7 @@ static SkPMColor convertToPMColor(SkColorType ct, SkAlphaType at, const uint32_t
     return SkPackARGB32(a, r, g, b);
 }
 
-static void fillCanvas(SkCanvas* canvas) {
+static SkBitmap make_src_bitmap() {
     static SkBitmap bmp;
     if (bmp.isNull()) {
         bmp.allocN32Pixels(DEV_W, DEV_H);
@@ -100,20 +101,34 @@ static void fillCanvas(SkCanvas* canvas) {
         for (int y = 0; y < DEV_H; ++y) {
             for (int x = 0; x < DEV_W; ++x) {
                 SkPMColor* pixel = reinterpret_cast<SkPMColor*>(pixels + y * bmp.rowBytes() + x * bmp.bytesPerPixel());
-                *pixel = getCanvasColor(x, y);
+                *pixel = get_src_color(x, y);
             }
         }
     }
+    return bmp;
+}
+
+static void fill_src_canvas(SkCanvas* canvas) {
     canvas->save();
     canvas->setMatrix(SkMatrix::I());
     canvas->clipRect(DEV_RECT_S, SkRegion::kReplace_Op);
     SkPaint paint;
     paint.setXfermodeMode(SkXfermode::kSrc_Mode);
-    canvas->drawBitmap(bmp, 0, 0, &paint);
+    canvas->drawBitmap(make_src_bitmap(), 0, 0, &paint);
     canvas->restore();
 }
 
-static void fillBitmap(SkBitmap* bitmap) {
+#if SK_SUPPORT_GPU
+static void fill_src_texture(GrTexture* texture) {
+    SkBitmap bmp = make_src_bitmap();
+    bmp.lockPixels();
+    texture->writePixels(0, 0, DEV_W, DEV_H, kSkia8888_GrPixelConfig, bmp.getPixels(),
+                         bmp.rowBytes());
+    bmp.unlockPixels();
+}
+#endif
+
+static void fill_dst_bmp_with_init_data(SkBitmap* bitmap) {
     SkASSERT(bitmap->lockPixelsAreWritable());
     SkAutoLockPixels alp(*bitmap);
     int w = bitmap->width();
@@ -122,12 +137,12 @@ static void fillBitmap(SkBitmap* bitmap) {
     for (int y = 0; y < h; ++y) {
         for (int x = 0; x < w; ++x) {
             SkPMColor* pixel = reinterpret_cast<SkPMColor*>(pixels + y * bitmap->rowBytes() + x * bitmap->bytesPerPixel());
-            *pixel = getBitmapColor(x, y, w);
+            *pixel = get_dst_bmp_init_color(x, y, w);
         }
     }
 }
 
-static bool checkPixel(SkPMColor a, SkPMColor b, bool didPremulConversion) {
+static bool check_read_pixel(SkPMColor a, SkPMColor b, bool didPremulConversion) {
     if (!didPremulConversion) {
         return a == b;
     }
@@ -150,11 +165,11 @@ static bool checkPixel(SkPMColor a, SkPMColor b, bool didPremulConversion) {
 // checks the bitmap contains correct pixels after the readPixels
 // if the bitmap was prefilled with pixels it checks that these weren't
 // overwritten in the area outside the readPixels.
-static bool checkRead(skiatest::Reporter* reporter,
-                      const SkBitmap& bitmap,
-                      int x, int y,
-                      bool checkCanvasPixels,
-                      bool checkBitmapPixels) {
+static bool check_read(skiatest::Reporter* reporter,
+                       const SkBitmap& bitmap,
+                       int x, int y,
+                       bool checkCanvasPixels,
+                       bool checkBitmapPixels) {
     SkASSERT(4 == bitmap.bytesPerPixel());
     SkASSERT(!bitmap.isNull());
     SkASSERT(checkCanvasPixels || checkBitmapPixels);
@@ -180,18 +195,18 @@ static bool checkRead(skiatest::Reporter* reporter,
 
             if (clippedSrcRect.contains(devx, devy)) {
                 if (checkCanvasPixels) {
-                    SkPMColor canvasPixel = getCanvasColor(devx, devy);
+                    SkPMColor canvasPixel = get_src_color(devx, devy);
                     bool didPremul;
-                    SkPMColor pmPixel = convertToPMColor(ct, at, pixel, &didPremul);
-                    bool check;
-                    REPORTER_ASSERT(reporter, check = checkPixel(pmPixel, canvasPixel, didPremul));
+                    SkPMColor pmPixel = convert_to_pmcolor(ct, at, pixel, &didPremul);
+                    bool check = check_read_pixel(pmPixel, canvasPixel, didPremul);
+                    REPORTER_ASSERT(reporter, check);
                     if (!check) {
                         return false;
                     }
                 }
             } else if (checkBitmapPixels) {
-                REPORTER_ASSERT(reporter, getBitmapColor(bx, by, bw) == *pixel);
-                if (getBitmapColor(bx, by, bw) != *pixel) {
+                REPORTER_ASSERT(reporter, get_dst_bmp_init_color(bx, by, bw) == *pixel);
+                if (get_dst_bmp_init_color(bx, by, bw) != *pixel) {
                     return false;
                 }
             }
@@ -291,6 +306,9 @@ DEF_GPUTEST(ReadPixels, reporter, factory) {
     for (int dtype = 0; dtype < 3; ++dtype) {
         int glCtxTypeCnt = 1;
 #if SK_SUPPORT_GPU
+        // On the GPU we will also try reading back from a non-renderable texture.
+        SkAutoTUnref<GrTexture> texture;
+
         if (0 != dtype)  {
             glCtxTypeCnt = GrContextFactory::kGLContextTypeCnt;
         }
@@ -317,15 +335,24 @@ DEF_GPUTEST(ReadPixels, reporter, factory) {
                 desc.fHeight = DEV_H;
                 desc.fConfig = kSkia8888_GrPixelConfig;
                 desc.fOrigin = 1 == dtype ? kBottomLeft_GrSurfaceOrigin : kTopLeft_GrSurfaceOrigin;
-                SkAutoTUnref<GrTexture> texture(
+                SkAutoTUnref<GrTexture> surfaceTexture(
                     context->textureProvider()->createTexture(desc, false));
-                surface.reset(SkSurface::NewRenderTargetDirect(texture->asRenderTarget()));
+                surface.reset(SkSurface::NewRenderTargetDirect(surfaceTexture->asRenderTarget()));
+                desc.fFlags = kNone_GrSurfaceFlags;
+
+                texture.reset(context->textureProvider()->createTexture(desc, false));
 #else
                 continue;
 #endif
             }
             SkCanvas& canvas = *surface->getCanvas();
-            fillCanvas(&canvas);
+            fill_src_canvas(&canvas);
+
+#if SK_SUPPORT_GPU
+            if (texture) {
+                fill_src_texture(texture);
+            }
+#endif
 
             static const struct {
                 SkColorType fColorType;
@@ -348,7 +375,7 @@ DEF_GPUTEST(ReadPixels, reporter, factory) {
                         // note that and fill them with pattern
                         bool startsWithPixels = !bmp.isNull();
                         if (startsWithPixels) {
-                            fillBitmap(&bmp);
+                            fill_dst_bmp_with_init_data(&bmp);
                         }
                         uint32_t idBefore = surface->generationID();
                         bool success = canvas.readPixels(&bmp, srcRect.fLeft, srcRect.fTop);
@@ -363,13 +390,34 @@ DEF_GPUTEST(ReadPixels, reporter, factory) {
                         REPORTER_ASSERT(reporter, idBefore == idAfter);
 
                         if (success || startsWithPixels) {
-                            checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop,
-                                      success, startsWithPixels);
+                            check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop,
+                                       success, startsWithPixels);
                         } else {
                             // if we had no pixels beforehand and the readPixels
                             // failed then our bitmap should still not have pixels
                             REPORTER_ASSERT(reporter, bmp.isNull());
                         }
+#if SK_SUPPORT_GPU
+                        // Try doing the read directly from a non-renderable texture
+                        if (texture && startsWithPixels) {
+                            fill_dst_bmp_with_init_data(&bmp);
+                            GrPixelConfig dstConfig =
+                                SkImageInfo2GrPixelConfig(gReadConfigs[c].fColorType,
+                                                          gReadConfigs[c].fAlphaType,
+                                                          kLinear_SkColorProfileType);
+                            uint32_t flags = 0;
+                            if (gReadConfigs[c].fAlphaType == kUnpremul_SkAlphaType) {
+                                flags = GrContext::kUnpremul_PixelOpsFlag;
+                            }
+                            bmp.lockPixels();
+                            success = texture->readPixels(srcRect.fLeft, srcRect.fTop, bmp.width(),
+                                                bmp.height(), dstConfig, bmp.getPixels(),
+                                                bmp.rowBytes(), flags);
+                            bmp.unlockPixels();
+                            check_read(reporter, bmp, srcRect.fLeft, srcRect.fTop,
+                                       success, true);
+                        }
+#endif
                     }
                     // check the old webkit version of readPixels that clips the
                     // bitmap size
@@ -380,8 +428,8 @@ DEF_GPUTEST(ReadPixels, reporter, factory) {
                         REPORTER_ASSERT(reporter, success);
                         REPORTER_ASSERT(reporter, kN32_SkColorType == wkbmp.colorType());
                         REPORTER_ASSERT(reporter, kPremul_SkAlphaType == wkbmp.alphaType());
-                        checkRead(reporter, wkbmp, clippedRect.fLeft,
-                                  clippedRect.fTop, true, false);
+                        check_read(reporter, wkbmp, clippedRect.fLeft,
+                                   clippedRect.fTop, true, false);
                     } else {
                         REPORTER_ASSERT(reporter, !success);
                     }