Reopened: Caching the result of readPixelsSupported

src/gpu/gl/GrGLCaps.h

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #ifndef GrGLCaps_DEFINED
 #define GrGLCaps_DEFINED
 
 #include "GrDrawTargetCaps.h"
 #include "GrGLStencilBuffer.h"
 #include "SkTArray.h"
 #include "SkTDArray.h"
+#include "SkTDynamicHash.h"
 
 class GrGLContextInfo;
 
 /**
  * Stores some capabilities of a GL context. Most are determined by the GL
  * version and the extensions string. It also tracks formats that have passed
  * the FBO completeness test.
  */
 class GrGLCaps : public GrDrawTargetCaps {
 public:
@@ skipping 220 matching lines @@
     bool vertexArrayObjectSupport() const { return fVertexArrayObjectSupport; }
 
     /// Use indices or vertices in CPU arrays rather than VBOs for dynamic content.
     bool useNonVBOVertexAndIndexDynamicData() const {
         return fUseNonVBOVertexAndIndexDynamicData;
     }
 
     /// 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; }
 
 
     bool fullClearIsFree() const { return fFullClearIsFree; }
 
     bool dropsTileOnZeroDivide() const { return fDropsTileOnZeroDivide; }
 
     /**
      * Returns a string containing the caps info.
@@ skipping 50 matching lines @@
             return SkToBool(fVerifiedColorConfigs[u32Idx] & (1 << bitIdx));
         }
     };
 
     void initFSAASupport(const GrGLContextInfo&, const GrGLInterface*);
     void initStencilFormats(const GrGLContextInfo&);
     // This must be called after initFSAASupport().
     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;
 
     SkTArray<StencilFormat, true> fStencilFormats;
     // tracks configs that have been verified to pass the FBO completeness when
     // used as a color attachment when a particular stencil format is used
     // as a stencil attachment.
     SkTArray<VerifiedColorConfigs, true> fStencilVerifiedColorConfigs;
 
@@ skipping 20 matching lines @@
     bool fTextureRedSupport : 1;
     bool fImagingSupport  : 1;
     bool fTwoFormatLimit : 1;
     bool fFragCoordsConventionSupport : 1;
     bool fVertexArrayObjectSupport : 1;
     bool fUseNonVBOVertexAndIndexDynamicData : 1;
     bool fIsCoreProfile : 1;
     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;
+    };
+
+    template <typename T,
+            typename Key,
+            typename Traits = T,
+            int kGrowPercent = 75>
+    class DynamicHashCache {

[Justin Novosad, 2014/07/04 14:42:27]

This seems like a generally useful wrapper for SkTDynamicHash, not to mention
that it is generic. I think it would make sense to move it to its own header
file. Not sure where it should go though. Seems like a good fit for src/core,
but nothing in core depends on it so it could reside in src/gpu.  Mike?

[bsalomon, 2014/07/07 13:31:10]

Adding the other Mike WRT this comment.

[mtklein, 2014/07/07 13:44:30]

Yup, does sound useful to me too.  We seem to have managed to keep
SkTDynamicHash itself in src/core so far, so maybe just put it right in
SkTDynamicHash.h?

[mtklein, 2014/07/07 13:44:30]

If possible, class DynamicHashCache : SkNoncopyable { ?

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+    public:
+        DynamicHashCache() {
+            fDict = new  SkTDynamicHash<T, Key, Traits, kGrowPercent>();

[mtklein, 2014/07/07 13:44:30]

fDict = SkNEW(SkTDynamicHash<T, Key, Traits, kGrowPercent>);

or, even better, use SkAutoTDelete<SkTDynamicHash<...> > and call
fDict.reset(...) here?

[Rémi Piotaix, 2014/07/07 18:10:16]

Done.

+        }
+
+        virtual ~DynamicHashCache() {

[mtklein, 2014/07/07 13:44:30]

Don't make this virtual?  Nothing else is virtual.

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+            delete fDict;
+        }
+
+        bool has(Key& key) const {

[mtklein, 2014/07/07 13:44:30]

Can any of these parameters be passed as const Key&  / const T& ?

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+            return NULL != find(key);

[mtklein, 2014/07/07 13:44:30]

return NULL != this->find(key);

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+        }
+
+        T* find(Key& key) const {
+            return fDict->find(key);
+        }
+
+        // if element already in cache, return immediately the cached value
+        T& put(T& element) {
+            Key key = Traits::GetKey(element);
+            if(has(key))

[mtklein, 2014/07/07 13:44:30]

if (this->has(key)) {
  return *this->find(key);
}

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+                return *find(key);

[mtklein, 2014/07/07 13:44:30]

Does this all optimize away into one call to find()?  I'd have a hard time
seeing that.

Maybe

if (T* val = this->find(key)) {
  return *val;
}

?

[Rémi Piotaix, 2014/07/07 18:10:17]

Done.

+
+            T& e = fValues.push_back(element);
+
+            fDict->add(&e);

[mtklein, 2014/07/07 13:44:30]

Aren't these pointers invalidated when fValues resizes?

[Rémi Piotaix, 2014/07/07 18:10:17]

Yes, you're right. I've corrected this in the new patchset. fValues now holds
pointers so the objects addresses don't change when the array is resized.

+
+            return e;
+        }
+
+        int size() {
+            return fValues.count();
+        }
+
+        void clear() {
+            fValues.reset();
+            delete fDict;
+            fDict = new  SkTDynamicHash<T, Key, Traits, kGrowPercent>();

[mtklein, 2014/07/07 13:44:30]

Seems like we could make fDict a non-pointer member if SkTDynamicHash gained a
clear() method?  I'm up for that.

+        }
+    private:
+        SkTDynamicHash<T, Key, Traits, kGrowPercent>* fDict;
+        SkTArray<T, true> fValues;
+    };
+
+    mutable DynamicHashCache<ReadPixelsSupportedFormats, ReadPixelsSupportedFormatsKey> fReadPixelsSupportedCache;
+
     typedef GrDrawTargetCaps INHERITED;
 };
 
 #endif