Pass alpha type to GrSurfaceContext::read/writePixels and remove flags.

We deduce whether to premul or unpremul based on the the input/output
alpha types. This means we also now support unpremuling on write and
premuling on read.

Class-ify former struct GrPixelInfo. Remove origin and instead pass a
flip bool to GrConvertPixels.

Unifies read/write methods on GrSurfaceContext via automatic conversion
of SkImageInfo to GrPixelInfo and making GrDirectContext an optional
parameter.

Bug: skia:7580

Change-Id: I42f6997852b4b902fb81264c6de68ca9537606aa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/224281
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrDataUtils.cpp b/src/gpu/GrDataUtils.cpp
index 9dcc25b..bbd6f3d 100644
--- a/src/gpu/GrDataUtils.cpp
+++ b/src/gpu/GrDataUtils.cpp
@@ -469,72 +469,76 @@
     pipeline->append_gamut_clamp_if_normalized(fakeII);
 }
 
-bool GrConvertPixels(const GrPixelInfo& dstInfo, void* dst, const GrPixelInfo& srcInfo,
-                     const void* src, GrSwizzle swizzle) {
+bool GrConvertPixels(const GrPixelInfo& dstInfo,       void* dst, size_t dstRB,
+                     const GrPixelInfo& srcInfo, const void* src, size_t srcRB,
+                     bool flipY, GrSwizzle swizzle) {
     TRACE_EVENT0("skia.gpu", TRACE_FUNC);
-
-    if (dstInfo.fWidth != srcInfo.fWidth || srcInfo.fHeight != dstInfo.fHeight) {
+    if (!srcInfo.isValid() || !dstInfo.isValid()) {
         return false;
     }
-    if (dstInfo.fWidth <= 0 || dstInfo.fHeight <= 0) {
+    if (!src || !dst) {
         return false;
     }
-    if (GrColorTypeComponentFlags(dstInfo.fColorInfo.fColorType) & kGray_SkColorTypeComponentFlag) {
+    if (dstInfo.width() != srcInfo.width() || srcInfo.height() != dstInfo.height()) {
+        return false;
+    }
+    if (GrColorTypeComponentFlags(dstInfo.colorType()) & kGray_SkColorTypeComponentFlag) {
         // We don't currently support conversion to Gray.
         return false;
     }
-    size_t srcBpp = GrColorTypeBytesPerPixel(srcInfo.fColorInfo.fColorType);
-    size_t dstBpp = GrColorTypeBytesPerPixel(dstInfo.fColorInfo.fColorType);
-    if (!srcBpp || !dstBpp) {
-        // Either src or dst is compressed or kUnknown.
+    if (dstRB < dstInfo.minRowBytes() || srcRB < srcInfo.minRowBytes()) {
         return false;
     }
+
+    size_t srcBpp = srcInfo.bpp();
+    size_t dstBpp = dstInfo.bpp();
+
     // SkRasterPipeline operates on row-pixels not row-bytes.
-    SkASSERT(dstInfo.fRowBytes % dstBpp == 0);
-    SkASSERT(srcInfo.fRowBytes % srcBpp == 0);
+    SkASSERT(dstRB % dstBpp == 0);
+    SkASSERT(srcRB % srcBpp == 0);
 
     SkRasterPipeline::StockStage load;
     bool srcIsNormalized;
-    auto loadSwizzle =
-            get_load_and_get_swizzle(srcInfo.fColorInfo.fColorType, &load, &srcIsNormalized);
+    auto loadSwizzle = get_load_and_get_swizzle(srcInfo.colorType(), &load, &srcIsNormalized);
     loadSwizzle = GrSwizzle::Concat(loadSwizzle, swizzle);
 
     SkRasterPipeline::StockStage store;
     bool dstIsNormalized;
-    auto storeSwizzle =
-            get_dst_swizzle_and_store(dstInfo.fColorInfo.fColorType, &store, &dstIsNormalized);
+    auto storeSwizzle = get_dst_swizzle_and_store(dstInfo.colorType(), &store, &dstIsNormalized);
 
+    bool premul   = srcInfo.alphaType() == kUnpremul_SkAlphaType &&
+                    dstInfo.alphaType() == kPremul_SkAlphaType;
+    bool unpremul = srcInfo.alphaType() == kPremul_SkAlphaType &&
+                    dstInfo.alphaType() == kUnpremul_SkAlphaType;
     bool alphaOrCSConversion =
-            (srcInfo.fColorInfo.fAlphaType != dstInfo.fColorInfo.fAlphaType &&
-             srcInfo.fColorInfo.fAlphaType != kOpaque_SkAlphaType) ||
-            !SkColorSpace::Equals(srcInfo.fColorInfo.fColorSpace, dstInfo.fColorInfo.fColorSpace);
+            premul || unpremul || !SkColorSpace::Equals(srcInfo.colorSpace(), dstInfo.colorSpace());
 
     bool clampGamut;
     SkTLazy<SkColorSpaceXformSteps> steps;
     GrSwizzle loadStoreSwizzle;
     if (alphaOrCSConversion) {
-        steps.init(srcInfo.fColorInfo.fColorSpace, srcInfo.fColorInfo.fAlphaType,
-                   dstInfo.fColorInfo.fColorSpace, dstInfo.fColorInfo.fAlphaType);
-        clampGamut = dstIsNormalized && dstInfo.fColorInfo.fAlphaType == kPremul_SkAlphaType;
+        steps.init(srcInfo.colorSpace(), srcInfo.alphaType(),
+                   dstInfo.colorSpace(), dstInfo.alphaType());
+        clampGamut = dstIsNormalized && dstInfo.alphaType() == kPremul_SkAlphaType;
     } else {
-        clampGamut = dstIsNormalized && !srcIsNormalized &&
-                     dstInfo.fColorInfo.fAlphaType == kPremul_SkAlphaType;
+        clampGamut =
+                dstIsNormalized && !srcIsNormalized && dstInfo.alphaType() == kPremul_SkAlphaType;
         if (!clampGamut) {
             loadStoreSwizzle = GrSwizzle::Concat(loadSwizzle, storeSwizzle);
         }
     }
     int cnt = 1;
-    int height = srcInfo.fHeight;
-    SkRasterPipeline_MemoryCtx srcCtx{const_cast<void*>(src), SkToInt(srcInfo.fRowBytes / srcBpp)},
-                               dstCtx{                  dst , SkToInt(dstInfo.fRowBytes / dstBpp)};
+    int height = srcInfo.height();
+    SkRasterPipeline_MemoryCtx srcCtx{const_cast<void*>(src), SkToInt(srcRB / srcBpp)},
+                               dstCtx{                  dst , SkToInt(dstRB / dstBpp)};
 
-    if (srcInfo.fOrigin != dstInfo.fOrigin) {
+    if (flipY) {
         // It *almost* works to point the src at the last row and negate the stride and run the
         // whole rectangle. However, SkRasterPipeline::run()'s control loop uses size_t loop
         // variables so it winds up relying on unsigned overflow math. It works out in practice
         // but UBSAN says "no!" as it's technically undefined and in theory a compiler could emit
         // code that didn't do what is intended. So we go one row at a time. :(
-        srcCtx.pixels = static_cast<char*>(srcCtx.pixels) + srcInfo.fRowBytes * (height - 1);
+        srcCtx.pixels = static_cast<char*>(srcCtx.pixels) + srcRB * (height - 1);
         std::swap(cnt, height);
     }
     for (int i = 0; i < cnt; ++i) {
@@ -558,9 +562,9 @@
             }
         }
         pipeline.append(store, &dstCtx);
-        pipeline.run(0, 0, srcInfo.fWidth, height);
-        srcCtx.pixels = static_cast<char*>(srcCtx.pixels) - srcInfo.fRowBytes;
-        dstCtx.pixels = static_cast<char*>(dstCtx.pixels) + dstInfo.fRowBytes;
+        pipeline.run(0, 0, srcInfo.width(), height);
+        srcCtx.pixels = static_cast<char*>(srcCtx.pixels) - srcRB;
+        dstCtx.pixels = static_cast<char*>(dstCtx.pixels) + dstRB;
     }
     return true;
 }
diff --git a/src/gpu/GrDataUtils.h b/src/gpu/GrDataUtils.h
index a55342f..2b8b691 100644
--- a/src/gpu/GrDataUtils.h
+++ b/src/gpu/GrDataUtils.h
@@ -26,22 +26,114 @@
 void GrFillInCompressedData(SkImage::CompressionType, int width, int height, char* dest,
                             const SkColor4f& color);
 
-struct GrColorInfo {
+// TODO: Replace with GrColorSpaceInfo once GrPixelConfig is excised from that type.
+class GrColorInfo {
+public:
+    GrColorInfo() = default;
+
+    GrColorInfo(GrColorType ct, SkAlphaType at, sk_sp<SkColorSpace> cs)
+            : fColorSpace(std::move(cs)), fColorType(ct), fAlphaType(at) {}
+
+    GrColorInfo(const GrColorInfo&) = default;
+    GrColorInfo(GrColorInfo&&) = default;
+    GrColorInfo& operator=(const GrColorInfo&) = default;
+    GrColorInfo& operator=(GrColorInfo&&) = default;
+
+    GrColorType colorType() const { return fColorType; }
+
+    SkAlphaType alphaType() const { return fAlphaType; }
+
+    SkColorSpace* colorSpace() const { return fColorSpace.get(); }
+
+    sk_sp<SkColorSpace> refColorSpace() const { return fColorSpace; }
+
+    bool isValid() const {
+        return fColorType != GrColorType::kUnknown && fAlphaType != kUnknown_SkAlphaType;
+    }
+
+private:
+    sk_sp<SkColorSpace> fColorSpace;
     GrColorType fColorType = GrColorType::kUnknown;
-    SkColorSpace* fColorSpace = nullptr;
-    SkAlphaType fAlphaType = kPremul_SkAlphaType;
+    SkAlphaType fAlphaType = kUnknown_SkAlphaType;
 };
 
-struct GrPixelInfo {
+class GrPixelInfo {
+public:
+    GrPixelInfo() = default;
+
+    // not explicit
+    GrPixelInfo(const SkImageInfo& info)
+            : fColorInfo(SkColorTypeToGrColorType(info.colorType()), info.alphaType(),
+                         info.refColorSpace())
+            , fWidth(info.width())
+            , fHeight(info.height()) {}
+
+    GrPixelInfo(GrColorType ct, SkAlphaType at, sk_sp<SkColorSpace> cs, int w, int h)
+            : fColorInfo(ct, at, std::move(cs)), fWidth(w), fHeight(h) {}
+
+    GrPixelInfo(const GrPixelInfo&) = default;
+    GrPixelInfo(GrPixelInfo&&) = default;
+    GrPixelInfo& operator=(const GrPixelInfo&) = default;
+    GrPixelInfo& operator=(GrPixelInfo&&) = default;
+
+    GrPixelInfo makeColorType(GrColorType ct) {
+        return {ct, this->alphaType(), this->refColorSpace(), this->width(), this->height()};
+    }
+
+    GrPixelInfo makeAlphaType(SkAlphaType at) {
+        return {this->colorType(), at, this->refColorSpace(), this->width(), this->height()};
+    }
+
+    GrColorType colorType() const { return fColorInfo.colorType(); }
+
+    SkAlphaType alphaType() const { return fColorInfo.alphaType(); }
+
+    SkColorSpace* colorSpace() const { return fColorInfo.colorSpace(); }
+
+    sk_sp<SkColorSpace> refColorSpace() const { return fColorInfo.refColorSpace(); }
+
+    int width() const { return fWidth; }
+
+    int height() const { return fHeight; }
+
+    size_t bpp() const { return GrColorTypeBytesPerPixel(this->colorType()); }
+
+    size_t minRowBytes() const { return this->bpp() * this->width(); }
+
+    /**
+     * Place this pixel rect in a surface of dimensions surfaceWidth x surfaceHeight size offset at
+     * surfacePt and then clip the pixel rectangle to the bounds of the surface. If the pixel rect
+     * does not intersect the rectangle or is empty then return false. If clipped, the input
+     * surfacePt, the width/height of this GrPixelInfo, and the data pointer will be modified to
+     * reflect the clipped rectangle.
+     */
+    template <typename T>
+    bool clip(int surfaceWidth, int surfaceHeight, SkIPoint* surfacePt, T** data, size_t rowBytes) {
+        auto bounds = SkIRect::MakeWH(surfaceWidth, surfaceHeight);
+        auto rect = SkIRect::MakeXYWH(surfacePt->fX, surfacePt->fY, fWidth, fHeight);
+        if (!rect.intersect(bounds)) {
+            return false;
+        }
+        *data = SkTAddOffset<T>(*data, (rect.fTop  - surfacePt->fY) * rowBytes +
+                                       (rect.fLeft - surfacePt->fX) * this->bpp());
+        surfacePt->fX = rect.fLeft;
+        surfacePt->fY = rect.fTop;
+        fWidth = rect.width();
+        fHeight = rect.height();
+        return true;
+    }
+
+    bool isValid() const { return fColorInfo.isValid() && fWidth > 0 && fHeight > 0; }
+
+private:
     GrColorInfo fColorInfo = {};
-    GrSurfaceOrigin fOrigin = kTopLeft_GrSurfaceOrigin;
     int fWidth = 0;
     int fHeight = 0;
-    size_t fRowBytes = 0;
 };
 
 // Swizzle param is applied after loading and before converting from srcInfo to dstInfo.
-bool GrConvertPixels(const GrPixelInfo& dstInfo, void* dst, const GrPixelInfo& srcInfo,
-                     const void* src, GrSwizzle swizzle = GrSwizzle{});
+bool GrConvertPixels(const GrPixelInfo& dstInfo,       void* dst, size_t dstRB,
+                     const GrPixelInfo& srcInfo, const void* src, size_t srcRB,
+                     bool flipY = false, GrSwizzle swizzle = GrSwizzle{});
 
 #endif
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index 6c22e5c..e4a0fdf 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -265,11 +265,9 @@
     TRACE_EVENT0("skia.gpu", TRACE_FUNC);
     SkASSERT(surface);
 
-    int bpp = GrColorTypeBytesPerPixel(dstColorType);
-    if (!GrSurfacePriv::AdjustReadPixelParams(surface->width(), surface->height(), bpp,
-                                              &left, &top, &width, &height,
-                                              &buffer,
-                                              &rowBytes)) {
+    auto subRect = SkIRect::MakeXYWH(left, top, width, height);
+    auto bounds  = SkIRect::MakeWH(surface->width(), surface->height());
+    if (!bounds.contains(subRect)) {
         return false;
     }
 
@@ -293,8 +291,8 @@
 
     if (1 == mipLevelCount) {
         // We require that if we are not mipped, then the write region is contained in the surface
-        SkIRect subRect = SkIRect::MakeXYWH(left, top, width, height);
-        SkIRect bounds = SkIRect::MakeWH(surface->width(), surface->height());
+        auto subRect = SkIRect::MakeXYWH(left, top, width, height);
+        auto bounds  = SkIRect::MakeWH(surface->width(), surface->height());
         if (!bounds.contains(subRect)) {
             return false;
         }
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index e52c476..10fa09e 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -179,8 +179,9 @@
      *                      to top-to-bottom (skia's usual order)
      *
      * @return true if the read succeeded, false if not. The read can fail
-     *              because of a unsupported pixel config or because no render
-     *              target is currently set.
+     *              because of the surface doesn't support reading, the color type
+     *              is not allowed for the format of the surface or if the rectangle
+     *              read is not contained in the surface.
      */
     bool readPixels(GrSurface* surface, int left, int top, int width, int height,
                     GrColorType dstColorType, void* buffer, size_t rowBytes);
@@ -196,6 +197,11 @@
      * @param srcColorType  the color type of the source buffer.
      * @param texels        array of mipmap levels containing texture data
      * @param mipLevelCount number of levels in 'texels'
+     *
+     * @return true if the write succeeded, false if not. The read can fail
+     *              because of the surface doesn't support writing (e.g. read only),
+     *              the color type is not allowed for the format of the surface or
+     *              if the rectangle written is not contained in the surface.
      */
     bool writePixels(GrSurface* surface, int left, int top, int width, int height,
                      GrColorType srcColorType, const GrMipLevel texels[], int mipLevelCount);
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 354aacf..f8254b2 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -1936,22 +1936,11 @@
     if (supportedRead.fColorType != dstCT || supportedRead.fSwizzle != GrSwizzle("rgba") || flip) {
         result.fPixelConverter = [w = rect.width(), h = rect.height(), dstCT, supportedRead](
                                          void* dst, const void* src) {
-            GrPixelInfo srcInfo;
-            srcInfo.fColorInfo.fAlphaType = kPremul_SkAlphaType;
-            srcInfo.fColorInfo.fColorType = supportedRead.fColorType;
-            srcInfo.fColorInfo.fColorSpace = nullptr;
-            srcInfo.fRowBytes = GrColorTypeBytesPerPixel(supportedRead.fColorType) * w;
-
-            GrPixelInfo dstInfo;
-            dstInfo.fColorInfo.fAlphaType = kPremul_SkAlphaType;
-            dstInfo.fColorInfo.fColorType = dstCT;
-            dstInfo.fColorInfo.fColorSpace = nullptr;
-            dstInfo.fRowBytes = GrColorTypeBytesPerPixel(dstCT) * w;
-
-            srcInfo.fWidth  = dstInfo.fWidth  = w;
-            srcInfo.fHeight = dstInfo.fHeight = h;
-
-            GrConvertPixels(dstInfo, dst, srcInfo, src, supportedRead.fSwizzle);
+            GrPixelInfo srcInfo(supportedRead.fColorType, kPremul_SkAlphaType, nullptr, w, h);
+            GrPixelInfo dstInfo(dstCT,                    kPremul_SkAlphaType, nullptr, w, h);
+            GrConvertPixels(dstInfo, dst, dstInfo.minRowBytes(),
+                            srcInfo, src, srcInfo.minRowBytes(),
+                            /* flipY = */ false, supportedRead.fSwizzle);
         };
     }
     return result;
@@ -1970,7 +1959,7 @@
         auto ii = SkImageInfo::Make(rect.width(), rect.height(), colorType, kPremul_SkAlphaType,
                                     this->colorSpaceInfo().refColorSpace());
         pm.alloc(ii);
-        if (!this->readPixels(ii, pm.writable_addr(), pm.rowBytes(), rect.fLeft, rect.fTop)) {
+        if (!this->readPixels(ii, pm.writable_addr(), pm.rowBytes(), {rect.fLeft, rect.fTop})) {
             callback(context, nullptr, 0);
         }
         callback(context, pm.addr(), pm.rowBytes());
diff --git a/src/gpu/GrResourceProvider.cpp b/src/gpu/GrResourceProvider.cpp
index 023f86d..332dd25 100644
--- a/src/gpu/GrResourceProvider.cpp
+++ b/src/gpu/GrResourceProvider.cpp
@@ -104,14 +104,18 @@
         if (!proxy) {
             return nullptr;
         }
-        auto srcInfo = SkImageInfo::Make(desc.fWidth, desc.fHeight, colorType,
-                                         kUnknown_SkAlphaType);
+        // Here we don't really know the alpha type of the data we want to upload. All we really
+        // care about is that it is not converted. So we use the same alpha type of the data
+        // and the surface context.
+        static constexpr auto kAlphaType = kPremul_SkAlphaType;
+        auto srcInfo = SkImageInfo::Make(desc.fWidth, desc.fHeight, colorType, kAlphaType);
         sk_sp<GrSurfaceContext> sContext = context->priv().makeWrappedSurfaceContext(
-                std::move(proxy), SkColorTypeToGrColorType(colorType), kUnknown_SkAlphaType);
+                std::move(proxy), SkColorTypeToGrColorType(colorType), kAlphaType);
         if (!sContext) {
             return nullptr;
         }
-        SkAssertResult(sContext->writePixels(srcInfo, mipLevel.fPixels, mipLevel.fRowBytes, 0, 0));
+        SkAssertResult(
+                sContext->writePixels(srcInfo, mipLevel.fPixels, mipLevel.fRowBytes, {0, 0}));
         return sk_ref_sp(sContext->asTextureProxy()->peekTexture());
     } else {
         return fGpu->createTexture(desc, budgeted, &mipLevel, 1);
diff --git a/src/gpu/GrSurface.cpp b/src/gpu/GrSurface.cpp
index 52ba3eb..d29d50d 100644
--- a/src/gpu/GrSurface.cpp
+++ b/src/gpu/GrSurface.cpp
@@ -91,53 +91,6 @@
     return finalSize;
 }
 
-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::hasPendingRead() const {
diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp
index 1a62316..b549caa 100644
--- a/src/gpu/GrSurfaceContext.cpp
+++ b/src/gpu/GrSurfaceContext.cpp
@@ -55,85 +55,31 @@
     return fContext->priv().singleOwner();
 }
 #endif
-static bool valid_premul_color_type(GrColorType ct) {
-    switch (ct) {
-        case GrColorType::kUnknown:          return false;
-        case GrColorType::kAlpha_8:          return false;
-        case GrColorType::kBGR_565:          return false;
-        case GrColorType::kABGR_4444:        return true;
-        case GrColorType::kRGBA_8888:        return true;
-        case GrColorType::kRGB_888x:         return false;
-        case GrColorType::kRG_88:            return false;
-        case GrColorType::kBGRA_8888:        return true;
-        case GrColorType::kRGBA_1010102:     return true;
-        case GrColorType::kGray_8:           return false;
-        case GrColorType::kAlpha_F16:        return false;
-        case GrColorType::kRGBA_F16:         return true;
-        case GrColorType::kRGBA_F16_Clamped: return true;
-        case GrColorType::kRG_F32:           return false;
-        case GrColorType::kRGBA_F32:         return true;
-        case GrColorType::kR_16:             return false;
-        case GrColorType::kRG_1616:          return false;
-        // Experimental (for Y416 and mutant P016/P010)
-        case GrColorType::kRGBA_16161616:    return false;
-        case GrColorType::kRG_F16:           return false;
-    }
-    SK_ABORT("Invalid GrColorType");
-    return false;
-}
 
-// TODO: This will be removed when GrSurfaceContexts are aware of their color types.
-// (skbug.com/6718)
-static bool valid_premul_config(GrPixelConfig config) {
-    switch (config) {
-        case kUnknown_GrPixelConfig:            return false;
-        case kAlpha_8_GrPixelConfig:            return false;
-        case kGray_8_GrPixelConfig:             return false;
-        case kRGB_565_GrPixelConfig:            return false;
-        case kRGBA_4444_GrPixelConfig:          return true;
-        case kRGBA_8888_GrPixelConfig:          return true;
-        case kRGB_888_GrPixelConfig:            return false;
-        case kRGB_888X_GrPixelConfig:           return false;
-        case kRG_88_GrPixelConfig:              return false;
-        case kBGRA_8888_GrPixelConfig:          return true;
-        case kSRGBA_8888_GrPixelConfig:         return true;
-        case kRGBA_1010102_GrPixelConfig:       return true;
-        case kRGBA_float_GrPixelConfig:         return true;
-        case kRG_float_GrPixelConfig:           return false;
-        case kAlpha_half_GrPixelConfig:         return false;
-        case kRGBA_half_GrPixelConfig:          return true;
-        case kRGBA_half_Clamped_GrPixelConfig:  return true;
-        case kRGB_ETC1_GrPixelConfig:           return false;
-        case kAlpha_8_as_Alpha_GrPixelConfig:   return false;
-        case kAlpha_8_as_Red_GrPixelConfig:     return false;
-        case kAlpha_half_as_Red_GrPixelConfig:  return false;
-        case kGray_8_as_Lum_GrPixelConfig:      return false;
-        case kGray_8_as_Red_GrPixelConfig:      return false;
-        case kR_16_GrPixelConfig:               return false;
-        case kRG_1616_GrPixelConfig:            return false;
-        // Experimental (for Y416 and mutant P016/P010)
-        case kRGBA_16161616_GrPixelConfig:      return false;
-        case kRG_half_GrPixelConfig:            return false;
-    }
-    SK_ABORT("Invalid GrPixelConfig");
-    return false;
-}
+bool GrSurfaceContext::readPixels(const GrPixelInfo& origDstInfo, void* dst, size_t rowBytes,
+                                  SkIPoint pt, GrContext* direct) {
+    ASSERT_SINGLE_OWNER
+    RETURN_FALSE_IF_ABANDONED
+    SkDEBUGCODE(this->validate();)
+    GR_AUDIT_TRAIL_AUTO_FRAME(this->auditTrail(), "GrSurfaceContext::readPixels");
 
-static bool valid_pixel_conversion(GrColorType cpuColorType, GrPixelConfig gpuConfig,
-                                   bool premulConversion) {
-    // We only allow premul <-> unpremul conversions for some formats
-    if (premulConversion &&
-        (!valid_premul_color_type(cpuColorType) || !valid_premul_config(gpuConfig))) {
+    if (!direct && !(direct = fContext->priv().asDirectContext())) {
         return false;
     }
-    return true;
-}
 
-bool GrSurfaceContext::readPixelsImpl(GrContext* direct, int left, int top, int width,
-                                      int height, GrColorType dstColorType,
-                                      SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
-                                      uint32_t pixelOpsFlags) {
-    SkASSERT(buffer);
+    if (!dst) {
+        return false;
+    }
+
+    if (!rowBytes) {
+        rowBytes = origDstInfo.minRowBytes();
+    } else if (rowBytes < origDstInfo.minRowBytes()) {
+        return false;
+    }
+
+    if (!origDstInfo.isValid()) {
+        return false;
+    }
 
     GrSurfaceProxy* srcProxy = this->asSurfaceProxy();
 
@@ -144,22 +90,18 @@
 
     GrSurface* srcSurface = srcProxy->peekSurface();
 
-    if (!GrSurfacePriv::AdjustReadPixelParams(srcSurface->width(), srcSurface->height(),
-                                              GrColorTypeBytesPerPixel(dstColorType), &left, &top,
-                                              &width, &height, &buffer, &rowBytes)) {
+    auto dstInfo = origDstInfo;
+    if (!dstInfo.clip(this->width(), this->height(), &pt, &dst, rowBytes)) {
         return false;
     }
 
-    // TODO: Pass dst buffer's alpha type.
-    bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & pixelOpsFlags);
-    SkASSERT(!unpremul || this->colorSpaceInfo().alphaType() != kUnpremul_SkAlphaType);
+    bool premul   = this->colorSpaceInfo().alphaType() == kUnpremul_SkAlphaType &&
+                    dstInfo.alphaType() == kPremul_SkAlphaType;
+    bool unpremul = this->colorSpaceInfo().alphaType() == kPremul_SkAlphaType &&
+                    dstInfo.alphaType() == kUnpremul_SkAlphaType;
 
-    if (!valid_pixel_conversion(dstColorType, srcProxy->config(), unpremul)) {
-        return false;
-    }
-
-    bool needColorConversion =
-            SkColorSpaceXformSteps::Required(this->colorSpaceInfo().colorSpace(), dstColorSpace);
+    bool needColorConversion = SkColorSpaceXformSteps::Required(this->colorSpaceInfo().colorSpace(),
+                                                                dstInfo.colorSpace());
 
     const GrCaps* caps = direct->priv().caps();
     // This is the getImageData equivalent to the canvas2D putImageData fast path. We probably don't
@@ -167,15 +109,14 @@
     // getImageData in "legacy" mode are round-trippable we use the GPU to do the complementary
     // unpremul step to writeSurfacePixels's premul step (which is determined empirically in
     // fContext->vaildaPMUPMConversionExists()).
-    bool canvas2DFastPath =
-            unpremul &&
-            !needColorConversion &&
-            (GrColorType::kRGBA_8888 == dstColorType || GrColorType::kBGRA_8888 == dstColorType) &&
-            SkToBool(srcProxy->asTextureProxy()) &&
-            (srcProxy->config() == kRGBA_8888_GrPixelConfig ||
-             srcProxy->config() == kBGRA_8888_GrPixelConfig) &&
-            caps->isConfigRenderable(kRGBA_8888_GrPixelConfig) &&
-            direct->priv().validPMUPMConversionExists();
+    bool canvas2DFastPath = unpremul && !needColorConversion &&
+                            (GrColorType::kRGBA_8888 == dstInfo.colorType() ||
+                             GrColorType::kBGRA_8888 == dstInfo.colorType()) &&
+                            SkToBool(srcProxy->asTextureProxy()) &&
+                            (srcProxy->config() == kRGBA_8888_GrPixelConfig ||
+                             srcProxy->config() == kBGRA_8888_GrPixelConfig) &&
+                            caps->isConfigRenderable(kRGBA_8888_GrPixelConfig) &&
+                            direct->priv().validPMUPMConversionExists();
 
     auto readFlag = caps->surfaceSupportsReadPixels(srcSurface);
     if (readFlag == GrCaps::kProtected_ReadFlag) {
@@ -201,8 +142,9 @@
         sk_sp<SkColorSpace> cs = canvas2DFastPath ? nullptr : this->colorSpaceInfo().refColorSpace();
 
         sk_sp<GrRenderTargetContext> tempCtx = direct->priv().makeDeferredRenderTargetContext(
-                format, SkBackingFit::kApprox, width, height, config, colorType, std::move(cs), 1,
-                GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin, nullptr, SkBudgeted::kYes);
+                format, SkBackingFit::kApprox, dstInfo.width(), dstInfo.height(), config, colorType,
+                std::move(cs), 1, GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin, nullptr,
+                SkBudgeted::kYes);
         if (!tempCtx) {
             return false;
         }
@@ -212,10 +154,14 @@
             fp = direct->priv().createPMToUPMEffect(
                     GrSimpleTextureEffect::Make(sk_ref_sp(srcProxy->asTextureProxy()),
                                                 SkMatrix::I()));
-            if (dstColorType == GrColorType::kBGRA_8888) {
+            if (dstInfo.colorType() == GrColorType::kBGRA_8888) {
                 fp = GrFragmentProcessor::SwizzleOutput(std::move(fp), GrSwizzle::BGRA());
-                dstColorType = GrColorType::kRGBA_8888;
+                dstInfo = dstInfo.makeColorType(GrColorType::kRGBA_8888);
             }
+            // The render target context is incorrectly tagged as kPremul even though we're writing
+            // unpremul data thanks to the PMToUPM effect. Fake out the dst alpha type so we don't
+            // double unpremul.
+            dstInfo = dstInfo.makeAlphaType(kPremul_SkAlphaType);
         } else {
             fp = GrSimpleTextureEffect::Make(sk_ref_sp(srcProxy->asTextureProxy()), SkMatrix::I());
         }
@@ -228,94 +174,79 @@
 
         tempCtx->asRenderTargetContext()->fillRectToRect(
                 GrNoClip(), std::move(paint), GrAA::kNo, SkMatrix::I(),
-                SkRect::MakeWH(width, height), SkRect::MakeXYWH(left, top, width, height));
+                SkRect::MakeWH(dstInfo.width(), dstInfo.height()),
+                SkRect::MakeXYWH(pt.fX, pt.fY, dstInfo.width(), dstInfo.height()));
 
-        uint32_t flags = canvas2DFastPath ? 0 : pixelOpsFlags;
-        return tempCtx->readPixelsImpl(direct, 0, 0, width, height, dstColorType, dstColorSpace,
-                                       buffer, rowBytes, flags);
+        return tempCtx->readPixels(dstInfo, dst, rowBytes, {0, 0}, direct);
     }
 
     bool flip = srcProxy->origin() == kBottomLeft_GrSurfaceOrigin;
-    auto supportedRead = caps->supportedReadPixelsColorType(
-            srcProxy->config(), srcProxy->backendFormat(), dstColorType);
 
-    bool convert = unpremul || needColorConversion || flip ||
-                   (dstColorType != supportedRead.fColorType) ||
+    auto supportedRead = caps->supportedReadPixelsColorType(
+            srcProxy->config(), srcProxy->backendFormat(), dstInfo.colorType());
+
+    bool convert = unpremul || premul || needColorConversion || flip ||
+                   (dstInfo.colorType() != supportedRead.fColorType) ||
                    supportedRead.fSwizzle != GrSwizzle::RGBA();
 
     std::unique_ptr<char[]> tmpPixels;
     GrPixelInfo tmpInfo;
-    GrPixelInfo dstInfo;
-    void* readDst = buffer;
+    void* readDst = dst;
+    size_t readRB = rowBytes;
     if (convert) {
-        tmpInfo.fColorInfo.fColorType = supportedRead.fColorType;
-        tmpInfo.fColorInfo.fAlphaType = kPremul_SkAlphaType;
-        tmpInfo.fColorInfo.fColorSpace = this->colorSpaceInfo().colorSpace();
-        tmpInfo.fOrigin = srcProxy->origin();
-        tmpInfo.fRowBytes = GrColorTypeBytesPerPixel(supportedRead.fColorType) * width;
+        tmpInfo = {supportedRead.fColorType, this->colorSpaceInfo().alphaType(),
+                   this->colorSpaceInfo().refColorSpace(), dstInfo.width(), dstInfo.height()};
+        size_t tmpRB = tmpInfo.minRowBytes();
+        size_t size = tmpRB * tmpInfo.height();
+        // Chrome MSAN bots require the data to be initialized (hence the ()).
+        tmpPixels.reset(new char[size]());
 
-        dstInfo.fColorInfo.fColorType = dstColorType;
-        dstInfo.fColorInfo.fAlphaType = unpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType;
-        dstInfo.fColorInfo.fColorSpace = dstColorSpace;
-        dstInfo.fOrigin = kTopLeft_GrSurfaceOrigin;
-        dstInfo.fRowBytes = rowBytes;
-
-        dstInfo.fWidth = tmpInfo.fWidth = width;
-        dstInfo.fHeight = tmpInfo.fHeight = height;
-
-        size_t size = tmpInfo.fRowBytes * height;
-        tmpPixels.reset(new char[size]);
-        // Chrome MSAN bots require this.
-        sk_bzero(tmpPixels.get(), size);
         readDst = tmpPixels.get();
-        rowBytes = tmpInfo.fRowBytes;
-        top = flip ? srcSurface->height() - top - height : top;
+        readRB = tmpRB;
+        pt.fY = flip ? srcSurface->height() - pt.fY - dstInfo.height() : pt.fY;
     }
 
     direct->priv().flushSurface(srcProxy);
 
-    if (!direct->priv().getGpu()->readPixels(srcSurface, left, top, width, height,
-                                             supportedRead.fColorType, readDst, rowBytes)) {
+    if (!direct->priv().getGpu()->readPixels(srcSurface, pt.fX, pt.fY, dstInfo.width(),
+                                             dstInfo.height(), supportedRead.fColorType, readDst,
+                                             readRB)) {
         return false;
     }
 
     if (convert) {
-        return GrConvertPixels(dstInfo, buffer, tmpInfo, tmpPixels.get(), supportedRead.fSwizzle);
+        return GrConvertPixels(dstInfo, dst, rowBytes, tmpInfo, readDst, readRB, flip,
+                               supportedRead.fSwizzle);
     }
     return true;
 }
 
-bool GrSurfaceContext::readPixels(const SkImageInfo& dstInfo, void* dstBuffer,
-                                  size_t dstRowBytes, int x, int y, uint32_t flags) {
+bool GrSurfaceContext::writePixels(const GrPixelInfo& origSrcInfo, const void* src, size_t rowBytes,
+                                   SkIPoint pt, GrContext* direct) {
     ASSERT_SINGLE_OWNER
     RETURN_FALSE_IF_ABANDONED
     SkDEBUGCODE(this->validate();)
-    GR_AUDIT_TRAIL_AUTO_FRAME(this->auditTrail(), "GrSurfaceContext::readPixels");
+    GR_AUDIT_TRAIL_AUTO_FRAME(this->auditTrail(), "GrSurfaceContext::writePixels");
 
-    // TODO: this seems to duplicate code in SkImage_Gpu::onReadPixels
-    if (kUnpremul_SkAlphaType == dstInfo.alphaType() &&
-        !GrPixelConfigIsOpaque(this->asSurfaceProxy()->config())) {
-        flags |= kUnpremul_PixelOpsFlag;
-    }
-    auto colorType = SkColorTypeToGrColorType(dstInfo.colorType());
-    if (GrColorType::kUnknown == colorType) {
+    if (!direct && !(direct = fContext->priv().asDirectContext())) {
         return false;
     }
 
-    auto direct = fContext->priv().asDirectContext();
-    if (!direct) {
+    if (this->asSurfaceProxy()->readOnly()) {
         return false;
     }
 
-    return this->readPixelsImpl(direct, x, y, dstInfo.width(), dstInfo.height(), colorType,
-                                dstInfo.colorSpace(), dstBuffer, dstRowBytes, flags);
-}
+    if (!src) {
+        return false;
+    }
 
-bool GrSurfaceContext::writePixelsImpl(GrContext* direct, int left, int top, int width, int height,
-                                       GrColorType srcColorType, SkColorSpace* srcColorSpace,
-                                       const void* srcBuffer, size_t srcRowBytes,
-                                       uint32_t pixelOpsFlags) {
-    if (GrColorType::kUnknown == srcColorType) {
+    if (!rowBytes) {
+        rowBytes = origSrcInfo.minRowBytes();
+    } else if (rowBytes < origSrcInfo.minRowBytes()) {
+        return false;
+    }
+
+    if (!origSrcInfo.isValid()) {
         return false;
     }
 
@@ -326,37 +257,35 @@
 
     GrSurface* dstSurface = dstProxy->peekSurface();
 
-    if (!GrSurfacePriv::AdjustWritePixelParams(dstSurface->width(), dstSurface->height(),
-                                               GrColorTypeBytesPerPixel(srcColorType), &left, &top,
-                                               &width, &height, &srcBuffer, &srcRowBytes)) {
+    auto srcInfo = origSrcInfo;
+    if (!srcInfo.clip(this->width(), this->height(), &pt, &src, rowBytes)) {
         return false;
     }
 
-    // TODO: Pass src buffer's alpha type.
-    bool premul = SkToBool(kUnpremul_PixelOpsFlag & pixelOpsFlags);
-    SkASSERT(!premul || this->colorSpaceInfo().alphaType() != kUnpremul_SkAlphaType);
+    bool premul = this->colorSpaceInfo().alphaType() == kPremul_SkAlphaType &&
+            srcInfo.alphaType() == kUnpremul_SkAlphaType;
+    bool unpremul = this->colorSpaceInfo().alphaType() == kUnpremul_SkAlphaType &&
+            srcInfo.alphaType() == kPremul_SkAlphaType;
 
-    bool needColorConversion =
-            SkColorSpaceXformSteps::Required(srcColorSpace, this->colorSpaceInfo().colorSpace());
+    bool needColorConversion = SkColorSpaceXformSteps::Required(
+            srcInfo.colorSpace(), this->colorSpaceInfo().colorSpace());
 
     const GrCaps* caps = direct->priv().caps();
     // For canvas2D putImageData performance we have a special code path for unpremul RGBA_8888 srcs
     // that are premultiplied on the GPU. This is kept as narrow as possible for now.
-    bool canvas2DFastPath =
-            !caps->avoidWritePixelsFastPath() &&
-            premul &&
-            !needColorConversion &&
-            (srcColorType == GrColorType::kRGBA_8888 || srcColorType == GrColorType::kBGRA_8888) &&
-            SkToBool(this->asRenderTargetContext()) &&
-            (dstProxy->config() == kRGBA_8888_GrPixelConfig ||
-             dstProxy->config() == kBGRA_8888_GrPixelConfig) &&
-            direct->priv().caps()->isConfigTexturable(kRGBA_8888_GrPixelConfig) &&
-            direct->priv().validPMUPMConversionExists();
+    bool canvas2DFastPath = !caps->avoidWritePixelsFastPath() && premul && !needColorConversion &&
+                            (srcInfo.colorType() == GrColorType::kRGBA_8888 ||
+                             srcInfo.colorType() == GrColorType::kBGRA_8888) &&
+                            SkToBool(this->asRenderTargetContext()) &&
+                            (dstProxy->config() == kRGBA_8888_GrPixelConfig ||
+                             dstProxy->config() == kBGRA_8888_GrPixelConfig) &&
+                            direct->priv().caps()->isConfigTexturable(kRGBA_8888_GrPixelConfig) &&
+                            direct->priv().validPMUPMConversionExists();
 
     if (!caps->surfaceSupportsWritePixels(dstSurface) || canvas2DFastPath) {
         GrSurfaceDesc desc;
-        desc.fWidth = width;
-        desc.fHeight = height;
+        desc.fWidth = srcInfo.width();
+        desc.fHeight = srcInfo.height();
         desc.fSampleCnt = 1;
         GrColorType colorType;
 
@@ -395,15 +324,15 @@
         if (!tempCtx) {
             return false;
         }
-        uint32_t flags = canvas2DFastPath ? 0 : pixelOpsFlags;
 
         // In the fast path we always write the srcData to the temp context as though it were RGBA.
         // When the data is really BGRA the write will cause the R and B channels to be swapped in
         // the intermediate surface which gets corrected by a swizzle effect when drawing to the
         // dst.
-        auto tmpColorType = canvas2DFastPath ? GrColorType::kRGBA_8888 : srcColorType;
-        if (!tempCtx->writePixelsImpl(direct, 0, 0, width, height, tmpColorType, srcColorSpace,
-                                      srcBuffer, srcRowBytes, flags)) {
+        if (canvas2DFastPath) {
+            srcInfo = srcInfo.makeColorType(GrColorType::kRGBA_8888);
+        }
+        if (!tempCtx->writePixels(srcInfo, src, rowBytes, {0, 0}, direct)) {
             return false;
         }
 
@@ -412,7 +341,8 @@
             if (canvas2DFastPath) {
                 fp = direct->priv().createUPMToPMEffect(
                         GrSimpleTextureEffect::Make(std::move(tempProxy), SkMatrix::I()));
-                if (srcColorType == GrColorType::kBGRA_8888) {
+                // Important: check the original src color type here!
+                if (origSrcInfo.colorType() == GrColorType::kBGRA_8888) {
                     fp = GrFragmentProcessor::SwizzleOutput(std::move(fp), GrSwizzle::BGRA());
                 }
             } else {
@@ -426,10 +356,11 @@
             paint.addColorFragmentProcessor(std::move(fp));
             this->asRenderTargetContext()->fillRectToRect(
                     GrNoClip(), std::move(paint), GrAA::kNo, SkMatrix::I(),
-                    SkRect::MakeXYWH(left, top, width, height), SkRect::MakeWH(width, height));
+                    SkRect::MakeXYWH(pt.fX, pt.fY, srcInfo.width(), srcInfo.height()),
+                    SkRect::MakeWH(srcInfo.width(), srcInfo.height()));
         } else {
-            SkIRect srcRect = SkIRect::MakeWH(width, height);
-            SkIPoint dstPoint = SkIPoint::Make(left, top);
+            SkIRect srcRect = SkIRect::MakeWH(srcInfo.width(), srcInfo.height());
+            SkIPoint dstPoint = SkIPoint::Make(pt.fX, pt.fY);
             if (!this->copy(tempProxy.get(), srcRect, dstPoint)) {
                 return false;
             }
@@ -437,44 +368,27 @@
         return true;
     }
 
-    if (!valid_pixel_conversion(srcColorType, dstProxy->config(), premul)) {
-        return false;
-    }
-
-    GrColorType allowedColorType = caps->supportedWritePixelsColorType(dstProxy->config(),
-                                                                       srcColorType);
-    bool convert = premul || needColorConversion || (srcColorType != allowedColorType) ||
-                   dstProxy->origin() == kBottomLeft_GrSurfaceOrigin;
+    GrColorType allowedColorType =
+            caps->supportedWritePixelsColorType(dstProxy->config(), srcInfo.colorType());
+    bool flip = dstProxy->origin() == kBottomLeft_GrSurfaceOrigin;
+    bool convert = premul || unpremul || needColorConversion ||
+                   (srcInfo.colorType() != allowedColorType) || flip;
 
     std::unique_ptr<char[]> tmpPixels;
+    GrColorType srcColorType = srcInfo.colorType();
     if (convert) {
-        GrPixelInfo srcInfo;
-        srcInfo.fColorInfo.fAlphaType = (premul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType);
-        srcInfo.fColorInfo.fColorType = srcColorType;
-        srcInfo.fColorInfo.fColorSpace = srcColorSpace;
-        srcInfo.fRowBytes = srcRowBytes;
-        srcInfo.fOrigin = kTopLeft_GrSurfaceOrigin;
+        GrPixelInfo tmpInfo(allowedColorType, this->colorSpaceInfo().alphaType(),
+                            this->colorSpaceInfo().refColorSpace(), srcInfo.width(),
+                            srcInfo.height());
+        auto tmpRB = tmpInfo.minRowBytes();
+        tmpPixels.reset(new char[tmpRB * tmpInfo.height()]);
 
-        GrPixelInfo tmpInfo;
-        tmpInfo.fColorInfo.fAlphaType = kPremul_SkAlphaType;
-        tmpInfo.fColorInfo.fColorType = allowedColorType;
-        tmpInfo.fColorInfo.fColorSpace = this->colorSpaceInfo().colorSpace();
-        tmpInfo.fRowBytes = GrColorTypeBytesPerPixel(allowedColorType) * width;
-        tmpInfo.fOrigin = dstProxy->origin();
+        GrConvertPixels(tmpInfo, tmpPixels.get(), tmpRB, srcInfo, src, rowBytes, flip);
 
-        srcInfo.fWidth  = tmpInfo.fWidth  = width;
-        srcInfo.fHeight = tmpInfo.fHeight = height;
-
-        tmpPixels.reset(new char[tmpInfo.fRowBytes * height]);
-
-        GrConvertPixels(tmpInfo, tmpPixels.get(), srcInfo, srcBuffer);
-
-        srcColorType = tmpInfo.fColorInfo.fColorType;
-        srcBuffer = tmpPixels.get();
-        srcRowBytes = tmpInfo.fRowBytes;
-        if (dstProxy->origin() == kBottomLeft_GrSurfaceOrigin) {
-            top = dstSurface->height() - top - height;
-        }
+        srcColorType = tmpInfo.colorType();
+        rowBytes = tmpRB;
+        src = tmpPixels.get();
+        pt.fY = flip ? dstSurface->height() - pt.fY - tmpInfo.height() : pt.fY;
     }
 
     // On platforms that prefer flushes over VRAM use (i.e., ANGLE) we're better off forcing a
@@ -484,36 +398,8 @@
     // TODO: should this policy decision just be moved into the drawing manager?
     direct->priv().flushSurface(caps->preferVRAMUseOverFlushes() ? dstProxy : nullptr);
 
-    return direct->priv().getGpu()->writePixels(dstSurface, left, top, width, height, srcColorType,
-                                                srcBuffer, srcRowBytes);
-}
-
-bool GrSurfaceContext::writePixels(const SkImageInfo& srcInfo, const void* srcBuffer,
-                                   size_t srcRowBytes, int x, int y, uint32_t flags) {
-    ASSERT_SINGLE_OWNER
-    RETURN_FALSE_IF_ABANDONED
-    SkDEBUGCODE(this->validate();)
-    GR_AUDIT_TRAIL_AUTO_FRAME(this->auditTrail(), "GrSurfaceContext::writePixels");
-
-    if (kUnpremul_SkAlphaType == srcInfo.alphaType()) {
-        flags |= kUnpremul_PixelOpsFlag;
-    }
-    auto colorType = SkColorTypeToGrColorType(srcInfo.colorType());
-    if (GrColorType::kUnknown == colorType) {
-        return false;
-    }
-
-    auto direct = fContext->priv().asDirectContext();
-    if (!direct) {
-        return false;
-    }
-
-    if (this->asSurfaceProxy()->readOnly()) {
-        return false;
-    }
-
-    return this->writePixelsImpl(direct, x, y, srcInfo.width(), srcInfo.height(), colorType,
-                                 srcInfo.colorSpace(), srcBuffer, srcRowBytes, flags);
+    return direct->priv().getGpu()->writePixels(dstSurface, pt.fX, pt.fY, srcInfo.width(),
+                                                srcInfo.height(), srcColorType, src, rowBytes);
 }
 
 bool GrSurfaceContext::copy(GrSurfaceProxy* src, const SkIRect& srcRect, const SkIPoint& dstPoint) {
diff --git a/src/gpu/GrSurfaceContext.h b/src/gpu/GrSurfaceContext.h
index 1b61e5a..5bac870 100644
--- a/src/gpu/GrSurfaceContext.h
+++ b/src/gpu/GrSurfaceContext.h
@@ -10,6 +10,7 @@
 
 #include "include/core/SkRefCnt.h"
 #include "src/gpu/GrColorSpaceInfo.h"
+#include "src/gpu/GrDataUtils.h"
 #include "src/gpu/GrSurfaceProxy.h"
 
 class GrAuditTrail;
@@ -39,60 +40,30 @@
     int width() const { return this->asSurfaceProxy()->width(); }
     int height() const { return this->asSurfaceProxy()->height(); }
 
-   /**
-    * These flags can be used with the read/write pixels functions below.
-    */
-    enum PixelOpsFlags {
-        /** The src for write or dst read is unpremultiplied. This is only respected if both the
-            config src and dst configs are an RGBA/BGRA 8888 format. */
-        kUnpremul_PixelOpsFlag  = 0x4,
-    };
-
     /**
      * Reads a rectangle of pixels from the render target context.
      * @param dstInfo       image info for the destination
-     * @param dstBuffer     destination pixels for the read
-     * @param dstRowBytes   bytes in a row of 'dstBuffer'
-     * @param x             x offset w/in the render target context from which to read
-     * @param y             y offset w/in the render target context from which to read
-     *
-     * @return true if the read succeeded, false if not. The read can fail because of an
-     *              unsupported pixel config.
+     * @param dst           destination pixels for the read
+     * @param rowBytes      bytes in a row of 'dst'
+     * @param srcPt         offset w/in the surface context from which to read
+     * @param direct        The direct context to use. If null will use our GrRecordingContext if it
+     *                      is a GrDirectContext and fail otherwise.
      */
-    bool readPixels(const SkImageInfo& dstInfo, void* dstBuffer, size_t dstRowBytes,
-                    int x, int y, uint32_t flags = 0);
+    bool readPixels(const GrPixelInfo& dstInfo, void* dst, size_t rowBytes, SkIPoint srcPt,
+                    GrContext* direct = nullptr);
 
     /**
      * Writes a rectangle of pixels [srcInfo, srcBuffer, srcRowbytes] into the
      * renderTargetContext at the specified position.
      * @param srcInfo       image info for the source pixels
-     * @param srcBuffer     source for the write
-     * @param srcRowBytes   bytes in a row of 'srcBuffer'
-     * @param x             x offset w/in the render target context at which to write
-     * @param y             y offset w/in the render target context at which to write
-     *
-     * @return true if the write succeeded, false if not. The write can fail because of an
-     *              unsupported pixel config.
+     * @param src           source for the write
+     * @param rowBytes      bytes in a row of 'src'
+     * @param dstPt         offset w/in the surface context at which to write
+     * @param direct        The direct context to use. If null will use our GrRecordingContext if it
+     *                      is a GrDirectContext and fail otherwise.
      */
-    bool writePixels(const SkImageInfo& srcInfo, const void* srcBuffer, size_t srcRowBytes,
-                     int x, int y, uint32_t flags = 0);
-
-#if GR_TEST_UTILS
-    // Accessors for tests to directly call read/writePixelsImpl
-    bool writePixels(GrContext* direct, int left, int top, int width, int height,
-                     GrColorType srcColorType, SkColorSpace* srcColorSpace,
-                     const void* srcBuffer, size_t srcRowBytes = 0, uint32_t pixelOpsFlags = 0) {
-        return writePixelsImpl(direct, left, top, width, height, srcColorType, srcColorSpace,
-                               srcBuffer, srcRowBytes, pixelOpsFlags);
-    }
-
-    bool readPixels(GrContext* direct, int left, int top, int width, int height,
-                    GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer,
-                    size_t rowBytes = 0, uint32_t pixelOpsFlags = 0) {
-        return readPixelsImpl(direct, left, top, width, height, dstColorType, dstColorSpace,
-                              buffer, rowBytes, pixelOpsFlags);
-    }
-#endif
+    bool writePixels(const GrPixelInfo& srcInfo, const void* src, size_t rowBytes, SkIPoint dstPt,
+                     GrContext* direct = nullptr);
 
     // TODO: this is virtual b.c. this object doesn't have a pointer to the wrapped GrSurfaceProxy?
     virtual GrSurfaceProxy* asSurfaceProxy() = 0;
@@ -168,15 +139,6 @@
         return this->copy(src, SkIRect::MakeWH(src->width(), src->height()), SkIPoint::Make(0, 0));
     }
 
-    bool writePixelsImpl(GrContext* direct, int left, int top, int width, int height,
-                         GrColorType srcColorType, SkColorSpace* srcColorSpace,
-                         const void* srcBuffer, size_t srcRowBytes, uint32_t pixelOpsFlags);
-
-    bool readPixelsImpl(GrContext* direct, int left, int top, int width,
-                        int height, GrColorType dstColorType,
-                        SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
-                        uint32_t pixelOpsFlags);
-
     GrColorSpaceInfo    fColorSpaceInfo;
 
     typedef SkRefCnt INHERITED;
diff --git a/src/gpu/GrSurfacePriv.h b/src/gpu/GrSurfacePriv.h
index 1cb3b06..e3c5eea 100644
--- a/src/gpu/GrSurfacePriv.h
+++ b/src/gpu/GrSurfacePriv.h
@@ -17,22 +17,6 @@
     implemented privately in GrSurface with a inline public method here). */
 class GrSurfacePriv {
 public:
-    /** Helpers used in read/write pixels implementations. The parameters 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);
-
     bool hasPendingRead() const { return fSurface->hasPendingRead(); }
     bool hasPendingWrite() const { return fSurface->hasPendingWrite(); }
     bool hasPendingIO() const { return fSurface->hasPendingIO(); }
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index c03526a..e9e3d7a 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -203,12 +203,7 @@
         return false;
     }
 
-    SkReadPixelsRec rec(pm, x, y);
-    if (!rec.trim(this->width(), this->height())) {
-        return false;
-    }
-
-    return fRenderTargetContext->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
+    return fRenderTargetContext->readPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), {x, y});
 }
 
 bool SkGpuDevice::onWritePixels(const SkPixmap& pm, int x, int y) {
@@ -218,12 +213,7 @@
         return false;
     }
 
-    SkWritePixelsRec rec(pm, x, y);
-    if (!rec.trim(this->width(), this->height())) {
-        return false;
-    }
-
-    return fRenderTargetContext->writePixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
+    return fRenderTargetContext->writePixels(pm.info(), pm.addr(), pm.rowBytes(), {x, y});
 }
 
 bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
diff --git a/src/gpu/effects/GrConfigConversionEffect.fp b/src/gpu/effects/GrConfigConversionEffect.fp
index d60c680..bc1e3a0 100644
--- a/src/gpu/effects/GrConfigConversionEffect.fp
+++ b/src/gpu/effects/GrConfigConversionEffect.fp
@@ -96,7 +96,7 @@
 
         readRTC->fillRectToRect(GrNoClip(), std::move(paint1), GrAA::kNo, SkMatrix::I(), kRect,
                                 kRect);
-        if (!readRTC->readPixels(ii, firstRead, 0, 0, 0)) {
+        if (!readRTC->readPixels(ii, firstRead, 0, {0, 0})) {
             return false;
         }
 
@@ -118,7 +118,7 @@
         readRTC->fillRectToRect(GrNoClip(), std::move(paint3), GrAA::kNo, SkMatrix::I(), kRect,
                                 kRect);
 
-        if (!readRTC->readPixels(ii, secondRead, 0, 0, 0)) {
+        if (!readRTC->readPixels(ii, secondRead, 0, {0, 0})) {
             return false;
         }
 
diff --git a/src/gpu/effects/generated/GrConfigConversionEffect.h b/src/gpu/effects/generated/GrConfigConversionEffect.h
index f7d898d..f855454 100644
--- a/src/gpu/effects/generated/GrConfigConversionEffect.h
+++ b/src/gpu/effects/generated/GrConfigConversionEffect.h
@@ -97,7 +97,7 @@
 
         readRTC->fillRectToRect(GrNoClip(), std::move(paint1), GrAA::kNo, SkMatrix::I(), kRect,
                                 kRect);
-        if (!readRTC->readPixels(ii, firstRead, 0, 0, 0)) {
+        if (!readRTC->readPixels(ii, firstRead, 0, {0, 0})) {
             return false;
         }
 
@@ -119,7 +119,7 @@
         readRTC->fillRectToRect(GrNoClip(), std::move(paint3), GrAA::kNo, SkMatrix::I(), kRect,
                                 kRect);
 
-        if (!readRTC->readPixels(ii, secondRead, 0, 0, 0)) {
+        if (!readRTC->readPixels(ii, secondRead, 0, {0, 0})) {
             return false;
         }
 
diff --git a/src/gpu/text/GrAtlasManager.cpp b/src/gpu/text/GrAtlasManager.cpp
index 952c58b..5ca8495 100644
--- a/src/gpu/text/GrAtlasManager.cpp
+++ b/src/gpu/text/GrAtlasManager.cpp
@@ -101,7 +101,7 @@
         return false;
     }
 
-    bool result = sContext->readPixels(ii, bm.getPixels(), bm.rowBytes(), 0, 0);
+    bool result = sContext->readPixels(ii, bm.getPixels(), bm.rowBytes(), {0, 0});
     if (!result) {
         SkDebugf("------ failed to read pixels for %s\n", filename);
         return false;