Caching the result of readPixelsSupported

The call was calling GR_GL_GetIntegerv 2 times for each readPixels
and thus was causing a loss of performance

(resubmit of issue 344793008)

Benchmark url: http://packages.gkny.fr/tst/index.html

BUG=skia:2681
R=junov@chromium.org

Author: piotaixr@chromium.org

Review URL: https://codereview.chromium.org/364193004

src/gpu/gl/GrGLCaps.cpp

@@ -504,7 +504,7 @@ void GrGLCaps::initConfigTexturableTable(const GrGLContextInfo& ctxInfo, const G
     // First check version for support
     if (kGL_GrGLStandard == standard) {
         hasETC1 = hasCompressTex2D &&
-            (version >= GR_GL_VER(4, 3) || 
+            (version >= GR_GL_VER(4, 3) ||
              ctxInfo.hasExtension("GL_ARB_ES3_compatibility"));
     } else {
         hasETC1 = hasCompressTex2D &&
@@ -560,7 +560,7 @@ void GrGLCaps::initConfigTexturableTable(const GrGLContextInfo& ctxInfo, const G
     }
 }
 
-bool GrGLCaps::readPixelsSupported(const GrGLInterface* intf,
+bool GrGLCaps::doReadPixelsSupported(const GrGLInterface* intf,
                                    GrGLenum format,
                                    GrGLenum type) const {
     if (GR_GL_RGBA == format && GR_GL_UNSIGNED_BYTE == type) {
@@ -589,6 +589,25 @@ bool GrGLCaps::readPixelsSupported(const GrGLInterface* intf,
     return (GrGLenum)otherFormat == format && (GrGLenum)otherType == type;
 }
 
+bool GrGLCaps::readPixelsSupported(const GrGLInterface* intf,
+                                   GrGLenum format,
+                                   GrGLenum type,
+                                   GrGLenum currFboFormat) const {
+
+    ReadPixelsSupportedFormatsKey key = {format, type, currFboFormat};
+
+    ReadPixelsSupportedFormats* cachedValue = fReadPixelsSupportedCache.find(key);
+
+    if (cachedValue == NULL) {
+        bool value = doReadPixelsSupported(intf, format, type);
+        cachedValue = new ReadPixelsSupportedFormats(key, value);
+
+        fReadPixelsSupportedCache.add(cachedValue);
+    }
+
+    return cachedValue->value();
+}
+
 void GrGLCaps::initFSAASupport(const GrGLContextInfo& ctxInfo, const GrGLInterface* gli) {
 
     fMSFBOType = kNone_MSFBOType;
@@ -817,3 +836,17 @@ SkString GrGLCaps::dump() const {
     r.appendf("Drops tile on zero divide: %s\n", (fDropsTileOnZeroDivide ? "YES" : "NO"));
     return r;
 }
+
+//Computes a hash based on the three values in the key struct
+// bits 31------------15---------7---------------0
+//      fFormat(15:0) fType(7:0) fFboFormat(7:0)
+uint32_t GrGLCaps::ReadPixelsSupportedFormats::Hash(const ReadPixelsSupportedFormatsKey& key) {
+    // fFormat has different values like 0x190X or 0x8XXX: 16 bits are required
+    uint32_t hash = ((key.fFormat & 0xFFFF) << 16);
+    // fType is 0x14XX: 8 lower bits are enough
+    hash |= ((key.fType & 0xFF) << 8);
+    // fFboFormat is enum GrPixelConfig which has less than 15 values: 8 bits OK
+    hash |= (key.fFboFormat & 0xFF);
+
+    return hash;
+}

src/gpu/gl/GrGLCaps.h

@@ -13,6 +13,7 @@
 #include "GrGLStencilBuffer.h"
 #include "SkTArray.h"
 #include "SkTDArray.h"
+#include "SkTDynamicHash.h"
 
 class GrGLContextInfo;
 
@@ -253,7 +254,8 @@ public:
     /// Does ReadPixels support the provided format/type combo?
     bool readPixelsSupported(const GrGLInterface* intf,
                              GrGLenum format,
-                             GrGLenum type) const;
+                             GrGLenum type,
+                             GrGLenum currFboFormat) const;
 
     bool isCoreProfile() const { return fIsCoreProfile; }
 
@@ -324,6 +326,10 @@ private:
     void initConfigRenderableTable(const GrGLContextInfo&);
     void initConfigTexturableTable(const GrGLContextInfo&, const GrGLInterface*);
 
+    bool doReadPixelsSupported(const GrGLInterface* intf,
+                                   GrGLenum format,
+                                   GrGLenum type) const;
+
     // tracks configs that have been verified to pass the FBO completeness when
     // used as a color attachment
     VerifiedColorConfigs fVerifiedColorConfigs;
@@ -364,6 +370,38 @@ private:
     bool fFullClearIsFree : 1;
     bool fDropsTileOnZeroDivide : 1;
 
+    struct ReadPixelsSupportedFormatsKey {
+        GrGLenum fFormat;
+        GrGLenum fType;
+        GrGLenum fFboFormat;
+
+        bool operator==(const ReadPixelsSupportedFormatsKey& rhs) const {
+            return fFormat == rhs.fFormat
+                    && fType == rhs.fType
+                    && fFboFormat == rhs.fFboFormat;
+        }
+    };
+
+    class ReadPixelsSupportedFormats {
+    public:
+        ReadPixelsSupportedFormats(ReadPixelsSupportedFormatsKey key,
+                                   bool value)
+            :fKey(key), fValue(value) {
+        }
+
+        static const ReadPixelsSupportedFormatsKey& GetKey(const ReadPixelsSupportedFormats& element) {
+            return element.fKey;
+        }
+        static uint32_t Hash(const ReadPixelsSupportedFormatsKey&);
+
+        bool value() const { return fValue; }
+    private:
+        ReadPixelsSupportedFormatsKey fKey;
+        bool fValue;
+    };
+
+    mutable SkTDynamicHash<ReadPixelsSupportedFormats, ReadPixelsSupportedFormatsKey> fReadPixelsSupportedCache;
+
     typedef GrDrawTargetCaps INHERITED;
 };
 

src/gpu/gl/GrGpuGL.cpp

@@ -170,8 +170,6 @@ GrGpuGL::~GrGpuGL() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-
-
 GrPixelConfig GrGpuGL::preferredReadPixelsConfig(GrPixelConfig readConfig,
                                                  GrPixelConfig surfaceConfig) const {
     if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && kRGBA_8888_GrPixelConfig == readConfig) {
@@ -182,9 +180,13 @@ GrPixelConfig GrGpuGL::preferredReadPixelsConfig(GrPixelConfig readConfig,
         // Mesa 3D takes a slow path on when reading back  BGRA from an RGBA surface and vice-versa.
         // Perhaps this should be guarded by some compiletime or runtime check.
         return surfaceConfig;
-    } else if (readConfig == kBGRA_8888_GrPixelConfig &&
+    } else if (readConfig == kBGRA_8888_GrPixelConfig
-               !this->glCaps().readPixelsSupported(this->glInterface(),
+            && this->glCaps().readPixelsSupported(
-                                                   GR_GL_BGRA, GR_GL_UNSIGNED_BYTE)) {
+                this->glInterface(),
+                GR_GL_BGRA,
+                GR_GL_UNSIGNED_BYTE,
+                fHWBoundRenderTarget->config()
+            )) {
         return kRGBA_8888_GrPixelConfig;
     } else {
         return readConfig;
@@ -713,7 +715,7 @@ bool GrGpuGL::uploadTexData(const GrGLTexture::Desc& desc,
 }
 
 // TODO: This function is using a lot of wonky semantics like, if width == -1
-// then set width = desc.fWdith ... blah. A better way to do it might be to 
+// then set width = desc.fWdith ... blah. A better way to do it might be to
 // create a CompressedTexData struct that takes a desc/ptr and figures out
 // the proper upload semantics. Then users can construct this function how they
 // see fit if they want to go against the "standard" way to do it.