Move some parts of onReadPixels up to GrGpu readPixels.

src/gpu/gl/GrGLGpu.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "GrGLGpu.h"
 #include "GrGLGLSL.h"
@@ skipping 1775 matching lines @@
     }
 
     return true;
 }
 
 bool GrGLGpu::onReadPixels(GrSurface* surface,
                            int left, int top,
                            int width, int height,
                            GrPixelConfig config,
                            void* buffer,
-                           size_t rowBytes) {
+                           size_t rowBytes,
+                           size_t tightRowBytes) {

[bsalomon, 2015/07/30 18:27:22]

Personally I find the tightRowBytes param is more clutter than is justified by
the cost of recomputing in the subclass (and I think the computation in
GrGpu::readPixels isn't really necessary in that function).

[egdaniel, 2015/07/30 18:47:34]

Done.

     SkASSERT(surface);
 
-    // We cannot read pixels into a compressed buffer
-    if (GrPixelConfigIsCompressed(config)) {
-        return false;
-    }
-
     GrGLRenderTarget* tgt = static_cast<GrGLRenderTarget*>(surface->asRenderTarget());
     if (!tgt) {
         return false;
     }
 
     GrGLenum format = 0;
     GrGLenum type = 0;
     bool flipY = kBottomLeft_GrSurfaceOrigin == surface->origin();
     if (!this->configToGLFormats(config, false, NULL, &format, &type)) {
         return false;
     }
-    size_t bpp = GrBytesPerPixel(config);
-    if (!GrSurfacePriv::AdjustReadPixelParams(surface->width(), surface->height(), bpp,
-                                              &left, &top, &width, &height,
-                                              &buffer,
-                                              &rowBytes)) {
-        return false;
-    }
 
     // resolve the render target if necessary

[egdaniel, 2015/07/30 18:00:49]

I also wouldn't be against a virtual on Gpu that is like
resolveRT(GrRenderTarget). Then I would just move this up as well (obviously
only resolving if the surface is a RT)

[bsalomon, 2015/07/30 18:27:22]

This is doing more than just resolving (that is handled by the call to
onResolveRenderTarget below). It is ensuring that the correct FBO is bound for
reading. If we want to resolve in the base class we can just call
GrGpu::resolveRenderTarget(). It'd preclude any implementation that could
downsample and blit to CPU accessible memory in one step (though I know of no
such thing). However, we will wind up revisiting this in any event when we
reorganize the GrSurface family of objects.

[egdaniel, 2015/07/30 18:47:34]

Sounds good

     switch (tgt->getResolveType()) {
         case GrGLRenderTarget::kCantResolve_ResolveType:
             return false;
         case GrGLRenderTarget::kAutoResolves_ResolveType:
             this->flushRenderTarget(tgt, &SkIRect::EmptyIRect());
             break;
         case GrGLRenderTarget::kCanResolve_ResolveType:
             this->onResolveRenderTarget(tgt);
             // we don't track the state of the READ FBO ID.
             fStats.incRenderTargetBinds();
             GL_CALL(BindFramebuffer(GR_GL_READ_FRAMEBUFFER,
                                     tgt->textureFBOID()));
             break;
         default:
             SkFAIL("Unknown resolve type");
     }
 
     const GrGLIRect& glvp = tgt->getViewport();
 
     // the read rect is viewport-relative
     GrGLIRect readRect;
     readRect.setRelativeTo(glvp, left, top, width, height, tgt->origin());
 
-    size_t tightRowBytes = bpp * width;
-    if (0 == rowBytes) {
-        rowBytes = tightRowBytes;
-    }
     size_t readDstRowBytes = tightRowBytes;
     void* readDst = buffer;
 
     // determine if GL can read using the passed rowBytes or if we need
     // a scratch buffer.
     SkAutoSMalloc<32 * sizeof(GrColor)> scratch;
     if (rowBytes != tightRowBytes) {
         if (this->glCaps().packRowLengthSupport()) {
             SkASSERT(!(rowBytes % sizeof(GrColor)));
             GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH,
@@ skipping 1326 matching lines @@
             this->setVertexArrayID(gpu, 0);
         }
         int attrCount = gpu->glCaps().maxVertexAttributes();
         if (fDefaultVertexArrayAttribState.count() != attrCount) {
             fDefaultVertexArrayAttribState.resize(attrCount);
         }
         attribState = &fDefaultVertexArrayAttribState;
     }
     return attribState;
 }