Revert "New read pixels implementation that is simpler but does all conversions on CPU."

This reverts commit c9a642edf2d1c7f5380fe829adbb1a692f9969a6.

Reason for revert: 1010102 gms broke

Original change's description:
> New read pixels implementation that is simpler but does all conversions on CPU.
> 
> Change-Id: Ia548cd24a8544b35a233311706faf48de353b7cf
> Reviewed-on: https://skia-review.googlesource.com/109902
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>

TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com

Change-Id: I7724a6eef79885ba2a32c1ac871e5b2a9a3c0c12
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/115140
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

8 files changed, 18 insertions, 215 deletions

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 626b3f6ecd..f9c941565c 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -26,7 +26,6 @@
 #include "GrTextureContext.h"
 #include "GrTextureStripAtlas.h"
 #include "GrTracing.h"
-#include "SkAutoPixmapStorage.h"
 #include "SkConvertPixels.h"
 #include "SkDeferredDisplayList.h"
 #include "SkGr.h"
@@ -612,10 +611,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top,
                                       int height, GrColorType dstColorType,
                                       SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
                                       uint32_t flags) {
-#ifndef SK_LEGACY_GPU_PIXEL_OPS
-    return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace,
-                                    buffer, rowBytes, flags);
-#endif
     // TODO: Color space conversion
 
     ASSERT_SINGLE_OWNER_PRIV
@@ -623,10 +618,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top,
     SkASSERT(src);
     ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy());
     GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels", fContext);
-    SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
-    if (flags & kDontFlush_PixelOpsFlag) {
-        return false;
-    }
 
     // MDB TODO: delay this instantiation until later in the method
     if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) {
@@ -676,7 +667,7 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top,
         return false;
     }
 
-    if (srcSurface->surfacePriv().hasPendingWrite()) {
+    if (!(kDontFlush_PixelOpsFlag & flags) && srcSurface->surfacePriv().hasPendingWrite()) {
         this->flush(nullptr); // MDB TODO: tighten this
     }
 
@@ -808,12 +799,6 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top
     }
 
     if (!fContext->caps()->surfaceSupportsWritePixels(dstSurface)) {
-        // We don't expect callers that are skipping flushes to require an intermediate draw.
-        SkASSERT(!(pixelOpsFlags & kDontFlush_PixelOpsFlag));
-        if (pixelOpsFlags & kDontFlush_PixelOpsFlag) {
-            return false;
-        }
-
         GrSurfaceDesc desc;
         desc.fConfig = dstProxy->config();
         desc.fWidth = width;
@@ -886,7 +871,7 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top
                 memcpy(tempSrc.writable_addr(0, y), tempSrc.addr(0, height - 1 - y), rowBytes);
                 memcpy(tempSrc.writable_addr(0, height - 1 - y), row.get(), rowBytes);
             }
-            top = dstSurface->height() - top - height;
+            top = dstProxy->height() - top - height;
         }
     } else if (dstProxy->origin() == kBottomLeft_GrSurfaceOrigin) {
         size_t trimRowBytes = GrColorTypeBytesPerPixel(srcColorType) * width;
@@ -898,7 +883,7 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top
         }
         buffer = tempBuffer.get();
         rowBytes = trimRowBytes;
-        top = dstSurface->height() - top - height;
+        top = dstProxy->height() - top - height;
     }
 
     if (!(kDontFlush_PixelOpsFlag & pixelOpsFlags) && dstSurface->surfacePriv().hasPendingIO()) {
@@ -909,136 +894,6 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top
                                        rowBytes);
 }
 
-bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top, int width,
-                                       int height, GrColorType dstColorType,
-                                       SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
-                                       uint32_t flags) {
-    SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
-    if (flags & kDontFlush_PixelOpsFlag){
-        return false;
-    }
-    ASSERT_SINGLE_OWNER_PRIV
-    RETURN_FALSE_IF_ABANDONED_PRIV
-    SkASSERT(src);
-    SkASSERT(buffer);
-    ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy());
-    GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels2", fContext);
-
-    // MDB TODO: delay this instantiation until later in the method
-    if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) {
-        return false;
-    }
-
-    GrSurfaceProxy* srcProxy = src->asSurfaceProxy();
-    GrSurface* srcSurface = srcProxy->priv().peekSurface();
-
-    if (!GrSurfacePriv::AdjustReadPixelParams(srcSurface->width(), srcSurface->height(),
-                                              GrColorTypeBytesPerPixel(dstColorType), &left, &top,
-                                              &width, &height, &buffer, &rowBytes)) {
-        return false;
-    }
-
-    // TODO: Make GrSurfaceContext know its alpha type and pass dst buffer's alpha type.
-    bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags);
-    bool convert = unpremul;
-
-    if (!valid_pixel_conversion(dstColorType, srcProxy->config(), unpremul)) {
-        return false;
-    }
-
-    if (!fContext->caps()->surfaceSupportsReadPixels(srcSurface)) {
-        GrSurfaceDesc desc;
-        desc.fConfig = srcProxy->config();
-        desc.fWidth = width;
-        desc.fHeight = height;
-        desc.fSampleCnt = 1;
-        auto tempProxy = this->proxyProvider()->createProxy(
-                desc, kTopLeft_GrSurfaceOrigin, SkBackingFit::kApprox, SkBudgeted::kYes);
-        if (!tempProxy) {
-            return false;
-        }
-        auto tempCtx = this->drawingManager()->makeTextureContext(
-                tempProxy, src->colorSpaceInfo().refColorSpace());
-        if (!tempCtx) {
-            return false;
-        }
-        if (!tempCtx->copy(srcProxy, {left, top, width, height}, {0, 0})) {
-            return false;
-        }
-        return this->readSurfacePixels2(tempCtx.get(), 0, 0, width, height, dstColorType,
-                                        dstColorSpace, buffer, rowBytes, flags);
-    }
-
-    bool flip = srcProxy->origin() == kBottomLeft_GrSurfaceOrigin;
-    if (flip) {
-        top = srcSurface->height() - top - height;
-    }
-
-    GrColorType allowedColorType =
-            fContext->caps()->supportedReadPixelsColorType(srcProxy->config(), dstColorType);
-    convert = convert || (dstColorType != allowedColorType);
-
-    if (!src->colorSpaceInfo().colorSpace()) {
-        // "Legacy" mode - no color space conversions.
-        dstColorSpace = nullptr;
-    }
-    convert = convert || !SkColorSpace::Equals(dstColorSpace, src->colorSpaceInfo().colorSpace());
-
-    SkAutoPixmapStorage tempPixmap;
-    SkPixmap finalPixmap;
-    if (convert) {
-        SkColorType srcSkColorType = GrColorTypeToSkColorType(allowedColorType);
-        SkColorType dstSkColorType = GrColorTypeToSkColorType(dstColorType);
-        if (kUnknown_SkColorType == srcSkColorType || kUnknown_SkColorType == dstSkColorType) {
-            return false;
-        }
-        auto tempAT = SkColorTypeIsAlwaysOpaque(srcSkColorType) ? kOpaque_SkAlphaType
-                                                                : kPremul_SkAlphaType;
-        auto tempII = SkImageInfo::Make(width, height, srcSkColorType, tempAT,
-                                        src->colorSpaceInfo().refColorSpace());
-        SkASSERT(!unpremul || !SkColorTypeIsAlwaysOpaque(dstSkColorType));
-        auto finalAT = SkColorTypeIsAlwaysOpaque(srcSkColorType)
-                               ? kOpaque_SkAlphaType
-                               : unpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType;
-        auto finalII =
-                SkImageInfo::Make(width, height, dstSkColorType, finalAT, sk_ref_sp(dstColorSpace));
-        if (!SkImageInfoValidConversion(finalII, tempII)) {
-            return false;
-        }
-        tempPixmap.alloc(tempII);
-        finalPixmap.reset(finalII, buffer, rowBytes);
-        buffer = tempPixmap.writable_addr();
-        rowBytes = tempPixmap.rowBytes();
-    }
-
-    if (srcSurface->surfacePriv().hasPendingWrite()) {
-        this->flush(nullptr);  // MDB TODO: tighten this
-    }
-
-    if (!fContext->fGpu->readPixels(srcSurface, left, top, width, height, allowedColorType, buffer,
-                                    rowBytes)) {
-        return false;
-    }
-
-    if (flip) {
-        size_t trimRowBytes = GrColorTypeBytesPerPixel(allowedColorType) * width;
-        std::unique_ptr<char[]> row(new char[trimRowBytes]);
-        char* upper = reinterpret_cast<char*>(buffer);
-        char* lower = reinterpret_cast<char*>(buffer) + (height - 1) * rowBytes;
-        for (int y = 0; y < height / 2; ++y, upper += rowBytes, lower -= rowBytes) {
-            memcpy(row.get(), upper, trimRowBytes);
-            memcpy(upper, lower, trimRowBytes);
-            memcpy(lower, row.get(), trimRowBytes);
-        }
-    }
-    if (convert) {
-        if (!tempPixmap.readPixels(finalPixmap)) {
-            return false;
-        }
-    }
-    return true;
-}
-
 void GrContextPriv::prepareSurfaceForExternalIO(GrSurfaceProxy* proxy) {
     ASSERT_SINGLE_OWNER_PRIV
     RETURN_IF_ABANDONED_PRIV
diff --git a/src/gpu/GrContextPriv.h b/src/gpu/GrContextPriv.h
index 61ff3c1e34..f7bd911b1e 100644
--- a/src/gpu/GrContextPriv.h
+++ b/src/gpu/GrContextPriv.h
@@ -128,16 +128,13 @@ public:
     };
 
     /**
-     * Reads a rectangle of pixels from a surface. There are currently two versions of this.
-     * readSurfacePixels() is the older version which will be replaced by the more robust and
-     * maintainable (but perhaps slower) readSurfacePixels2().
-     *
+     * Reads a rectangle of pixels from a surface.
      * @param src           the surface context 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.
      * @param height        height of rectangle to read in pixels.
-     * @param dstColorType  the color type of the destination buffer
+     * @param dstConfig     the pixel config of the destination buffer
      * @param dstColorSpace color space of the destination buffer
      * @param buffer        memory to read the rectangle into.
      * @param rowBytes      number of bytes bewtween consecutive rows. Zero means rows are tightly
@@ -150,9 +147,6 @@ public:
     bool readSurfacePixels(GrSurfaceContext* src, int left, int top, int width, int height,
                            GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer,
                            size_t rowBytes = 0, uint32_t pixelOpsFlags = 0);
-    bool readSurfacePixels2(GrSurfaceContext* src, int left, int top, int width, int height,
-                            GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer,
-                            size_t rowBytes = 0, uint32_t pixelOpsFlags = 0);
 
     /**
      * Writes a rectangle of pixels to a surface. There are currently two versions of this.
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 95c5117799..16561418aa 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -272,15 +272,6 @@ public:
      */
     bool readPixels(GrSurface* surface, GrSurfaceOrigin, int left, int top, int width, int height,
                     GrColorType dstColorType, void* buffer, size_t rowBytes);
-    /**
-     * This version of readPixels doesn't take an origin. TODO: Remove origin handling from
-     * GrGpu::readPixels entirely.
-     */
-    bool readPixels(GrSurface* surface, int left, int top, int width, int height,
-                    GrColorType dstColorType, void* buffer, size_t rowBytes) {
-        return this->readPixels(surface, kTopLeft_GrSurfaceOrigin, left, top, width, height,
-                                dstColorType, buffer, rowBytes);
-    }
 
     /**
      * Updates the pixels in a rectangle of a surface.  No sRGB/linear conversions are performed.
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index e05a4bb970..0a5c2fe7b7 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -890,6 +890,11 @@ bool GrGLCaps::readPixelsSupported(GrPixelConfig surfaceConfig,
                 return true;
             }
             break;
+        case kInteger_FormatType:
+            if (GR_GL_RGBA_INTEGER == readFormat && GR_GL_INT == readType) {
+                return true;
+            }
+            break;
         case kFloat_FormatType:
             if (GR_GL_RGBA == readFormat && GR_GL_FLOAT == readType) {
                 return true;
@@ -1221,8 +1226,7 @@ bool GrGLCaps::getExternalFormat(GrPixelConfig surfaceConfig, GrPixelConfig memo
     bool memoryIsAlphaOnly = GrPixelConfigIsAlphaOnly(memoryConfig);
 
     // We don't currently support moving RGBA data into and out of ALPHA surfaces. It could be
-    // made to work. However, this is complicated by the use of GL_RED for alpha-only textures but
-    // is not needed currently.
+    // made to work in many cases using glPixelStore and what not but is not needed currently.
     if (surfaceIsAlphaOnly && !memoryIsAlphaOnly) {
         return false;
     }
@@ -2445,7 +2449,8 @@ bool GrGLCaps::surfaceSupportsWritePixels(const GrSurface* surface) const {
                 return false;
             }
         }
-    }    if (auto rt = surface->asRenderTarget()) {
+    }
+    if (auto rt = surface->asRenderTarget()) {
         if (fUseDrawInsteadOfAllRenderTargetWrites) {
             return false;
         }
@@ -2457,44 +2462,6 @@ bool GrGLCaps::surfaceSupportsWritePixels(const GrSurface* surface) const {
     return true;
 }
 
-bool GrGLCaps::surfaceSupportsReadPixels(const GrSurface* surface) const {
-    if (auto tex = static_cast<const GrGLTexture*>(surface->asTexture())) {
-        // We don't support reading pixels directly from EXTERNAL textures as it would require
-        // binding the texture to a FBO.
-        if (tex->target() == GR_GL_TEXTURE_EXTERNAL) {
-            return false;
-        }
-    }
-    return true;
-}
-
-GrColorType GrGLCaps::supportedReadPixelsColorType(GrPixelConfig config,
-                                                   GrColorType dstColorType) const {
-    // For now, we mostly report the read back format that is required by the ES spec without
-    // checking for implementation allowed formats or consider laxer rules in non-ES GL. TODO: Relax
-    // this as makes sense to increase performance and correctness.
-    switch (fConfigTable[config].fFormatType) {
-        case kNormalizedFixedPoint_FormatType:
-            return GrColorType::kRGBA_8888;
-        case kFloat_FormatType:
-            // We cheat a little here and allow F16 read back if the src and dst match.
-            if (kRGBA_half_GrPixelConfig == config && GrColorType::kRGBA_F16 == dstColorType) {
-                return GrColorType::kRGBA_F16;
-            }
-            if ((kAlpha_half_GrPixelConfig == config ||
-                 kAlpha_half_as_Red_GrPixelConfig == config) &&
-                GrColorType::kAlpha_F16 == dstColorType) {
-                return GrColorType::kAlpha_F16;
-            }
-            // And similar for full float RG.
-            if (kRG_float_GrPixelConfig == config && GrColorType::kRG_F32 == dstColorType) {
-                return GrColorType::kRG_F32;
-            }
-            return GrColorType::kRGBA_F32;
-    }
-    return GrColorType::kUnknown;
-}
-
 bool GrGLCaps::onIsWindowRectanglesSupportedForRT(const GrBackendRenderTarget& backendRT) const {
     const GrGLFramebufferInfo* fbInfo = backendRT.getGLFramebufferInfo();
     SkASSERT(fbInfo);
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index eed3ab3a17..5d3fa6a6f6 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -316,9 +316,7 @@ public:
     /// Use indices or vertices in CPU arrays rather than VBOs for dynamic content.
     bool useNonVBOVertexAndIndexDynamicData() const { return fUseNonVBOVertexAndIndexDynamicData; }
 
-    bool surfaceSupportsWritePixels(const GrSurface*) const override;
-    bool surfaceSupportsReadPixels(const GrSurface*) const override;
-    GrColorType supportedReadPixelsColorType(GrPixelConfig, GrColorType) const override;
+    bool surfaceSupportsWritePixels(const GrSurface* surface) const override;
 
     /// Does ReadPixels support reading readConfig pixels from a FBO that is surfaceConfig?
     bool readPixelsSupported(GrPixelConfig surfaceConfig,
@@ -505,6 +503,7 @@ private:
     enum FormatType {
         kNormalizedFixedPoint_FormatType,
         kFloat_FormatType,
+        kInteger_FormatType,
     };
 
     struct ReadPixelsFormat {
diff --git a/src/gpu/mock/GrMockCaps.h b/src/gpu/mock/GrMockCaps.h
index 27d295ebd5..4bcd4599c5 100644
--- a/src/gpu/mock/GrMockCaps.h
+++ b/src/gpu/mock/GrMockCaps.h
@@ -67,8 +67,7 @@ public:
         return 0;
     }
 
-    bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; }
-    bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
+    bool surfaceSupportsWritePixels(const GrSurface* surface) const override { return true; }
 
     bool initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc* desc, GrSurfaceOrigin*,
                             bool* rectsMustMatch, bool* disallowSubrect) const override {
diff --git a/src/gpu/mtl/GrMtlCaps.h b/src/gpu/mtl/GrMtlCaps.h
index c43d596e7f..130af67c70 100644
--- a/src/gpu/mtl/GrMtlCaps.h
+++ b/src/gpu/mtl/GrMtlCaps.h
@@ -31,8 +31,7 @@ public:
     int getRenderTargetSampleCount(int requestedCount, GrPixelConfig) const override;
     int maxRenderTargetSampleCount(GrPixelConfig) const override;
 
-    bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; }
-    bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
+    bool surfaceSupportsWritePixels(const GrSurface* surface) const override { return true; }
 
     bool isConfigCopyable(GrPixelConfig config) const override {
         return true;
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index 881aacc9dc..63bd4e90d4 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -40,8 +40,7 @@ public:
     int getRenderTargetSampleCount(int requestedCount, GrPixelConfig config) const override;
     int maxRenderTargetSampleCount(GrPixelConfig config) const override;
 
-    bool surfaceSupportsWritePixels(const GrSurface*) const override;
-    bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
+    bool surfaceSupportsWritePixels(const GrSurface* surface) const override;
 
     bool isConfigTexturableLinearly(GrPixelConfig config) const {
         return SkToBool(ConfigInfo::kTextureable_Flag & fConfigTable[config].fLinearFlags);