diff options
author | 2014-07-15 19:41:08 -0700 | |
---|---|---|
committer | 2014-07-15 19:41:08 -0700 | |
commit | 8339371f1ec3c57a0741932fd96bff32c53d4e54 (patch) | |
tree | 0b55a2a35986e75941aa517b62892c40c6ac553b | |
parent | e51b6bd1f9910b676e26c9db930d25c651c96c71 (diff) |
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
Committed: https://skia.googlesource.com/skia/+/753a2964afe5661ce9b2a8ca77ca9d0aabd3173c
R=junov@chromium.org, reed@chromium.org, bsalomon@chromium.org, mtklein@google.com, bsalomon@google.com
Author: piotaixr@chromium.org
Review URL: https://codereview.chromium.org/364193004
-rw-r--r-- | gyp/core.gypi | 1 | ||||
-rw-r--r-- | gyp/tests.gypi | 1 | ||||
-rw-r--r-- | src/core/SkTHashCache.h | 77 | ||||
-rw-r--r-- | src/gpu/gl/GrGLCaps.cpp | 30 | ||||
-rw-r--r-- | src/gpu/gl/GrGLCaps.h | 50 | ||||
-rw-r--r-- | src/gpu/gl/GrGpuGL.cpp | 14 | ||||
-rw-r--r-- | tests/THashCache.cpp | 91 |
7 files changed, 252 insertions, 12 deletions
diff --git a/gyp/core.gypi b/gyp/core.gypi index f7853f91d5..f321f017b9 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -199,6 +199,7 @@ '<(skia_src_path)/core/SkStrokerPriv.h', '<(skia_src_path)/core/SkTextFormatParams.h', '<(skia_src_path)/core/SkTextMapStateProc.h', + '<(skia_src_path)/core/SkTHashCache.h', '<(skia_src_path)/core/SkTileGrid.cpp', '<(skia_src_path)/core/SkTileGrid.h', '<(skia_src_path)/core/SkTLList.h', diff --git a/gyp/tests.gypi b/gyp/tests.gypi index ec12461198..8542868dda 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -174,6 +174,7 @@ '../tests/StrokeTest.cpp', '../tests/SurfaceTest.cpp', '../tests/TArrayTest.cpp', + '../tests/THashCache.cpp', '../tests/TLSTest.cpp', '../tests/TSetTest.cpp', '../tests/TextureCompressionTest.cpp', diff --git a/src/core/SkTHashCache.h b/src/core/SkTHashCache.h new file mode 100644 index 0000000000..cfee9722f2 --- /dev/null +++ b/src/core/SkTHashCache.h @@ -0,0 +1,77 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkTHASHCACHE_DEFINED +#define SkTHASHCACHE_DEFINED + +#include "SkTypes.h" +#include "SkTDynamicHash.h" + +template <typename T, +typename Key, +typename Traits = T, +int kGrowPercent = 75 > +class SkTHashCache : public SkNoncopyable { +public: + + SkTHashCache() { + this->reset(); + } + + ~SkTHashCache() { + this->clear(); + } + + T* find(const Key& key) const { + return fDict->find(key); + } + + /** + * If element already in cache, return immediately the cached value + */ + T& add(const T& add) { + Key key = Traits::GetKey(add); + if (T* val = this->find(key)) { + return *val; + } + + T* element = SkNEW_ARGS(T, (add)); + + fDict->add(element); + + return *element; + } + + int size() const { + return fDict->count(); + } + + void reset() { + this->clear(); + + fDict.reset(SkNEW(DictType)); + } + +private: + typedef SkTDynamicHash<T, Key, Traits, kGrowPercent> DictType; + + void clear() { + if (fDict.get()) { + typename DictType::Iter it(fDict.get()); + + while (!it.done()) { + SkDELETE(&(*it)); + ++it; + } + } + } + + SkAutoTDelete<DictType> fDict; +}; + +#endif /* SkHASHCACHE_DEFINED */ + diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index 8f2d4c1297..89d873bdd8 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -48,6 +48,8 @@ void GrGLCaps::reset() { fIsCoreProfile = false; fFullClearIsFree = false; fDropsTileOnZeroDivide = false; + + fReadPixelsSupportedCache.reset(); } GrGLCaps::GrGLCaps(const GrGLCaps& caps) : GrDrawTargetCaps() { @@ -504,7 +506,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,9 +562,9 @@ void GrGLCaps::initConfigTexturableTable(const GrGLContextInfo& ctxInfo, const G } } -bool GrGLCaps::readPixelsSupported(const GrGLInterface* intf, - GrGLenum format, - GrGLenum type) const { +bool GrGLCaps::doReadPixelsSupported(const GrGLInterface* intf, + GrGLenum format, + GrGLenum type) const { if (GR_GL_RGBA == format && GR_GL_UNSIGNED_BYTE == type) { // ES 2 guarantees this format is supported return true; @@ -589,6 +591,26 @@ 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 { + + ReadPixelsSupportedFormats::Key key = {format, type, currFboFormat}; + + ReadPixelsSupportedFormats* cachedValue = fReadPixelsSupportedCache.find(key); + + if (NULL == cachedValue) { + bool value = doReadPixelsSupported(intf, format, type); + ReadPixelsSupportedFormats newValue(key, value); + fReadPixelsSupportedCache.add(newValue); + + return newValue.value(); + } + + return cachedValue->value(); +} + void GrGLCaps::initFSAASupport(const GrGLContextInfo& ctxInfo, const GrGLInterface* gli) { fMSFBOType = kNone_MSFBOType; diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h index ccf04fd7ba..2d391b122c 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/src/gpu/gl/GrGLCaps.h @@ -11,8 +11,9 @@ #include "GrDrawTargetCaps.h" #include "GrGLStencilBuffer.h" +#include "SkChecksum.h" +#include "SkTHashCache.h" #include "SkTArray.h" -#include "SkTDArray.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,46 @@ private: bool fFullClearIsFree : 1; bool fDropsTileOnZeroDivide : 1; + class ReadPixelsSupportedFormats { + public: + struct Key { + GrGLenum fFormat; + GrGLenum fType; + GrGLenum fFboFormat; + + bool operator==(const Key& rhs) const { + return fFormat == rhs.fFormat + && fType == rhs.fType + && fFboFormat == rhs.fFboFormat; + } + + uint32_t getHash() const { + return SkChecksum::Murmur3(reinterpret_cast<const uint32_t*>(this), sizeof(*this)); + } + }; + + ReadPixelsSupportedFormats(Key key, bool value) : fKey(key), fValue(value) { + } + + static const Key& GetKey(const ReadPixelsSupportedFormats& element) { + return element.fKey; + } + + static uint32_t Hash(const Key& key) { + return key.getHash(); + } + + bool value() const { + return fValue; + } + private: + Key fKey; + bool fValue; + }; + + mutable SkTHashCache<ReadPixelsSupportedFormats, + ReadPixelsSupportedFormats::Key> fReadPixelsSupportedCache; + typedef GrDrawTargetCaps INHERITED; }; diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp index eddccc39f6..3eca3d6247 100644 --- a/src/gpu/gl/GrGpuGL.cpp +++ b/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 && - !this->glCaps().readPixelsSupported(this->glInterface(), - GR_GL_BGRA, GR_GL_UNSIGNED_BYTE)) { + } else if (readConfig == kBGRA_8888_GrPixelConfig + && this->glCaps().readPixelsSupported( + this->glInterface(), + GR_GL_BGRA, + GR_GL_UNSIGNED_BYTE, + surfaceConfig + )) { 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. diff --git a/tests/THashCache.cpp b/tests/THashCache.cpp new file mode 100644 index 0000000000..7797431553 --- /dev/null +++ b/tests/THashCache.cpp @@ -0,0 +1,91 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "Test.h" +#include "SkTHashCache.h" + + +// Tests the SkTHashCache<T> class template. + +struct Tint { + uint32_t value; + + bool operator==(const Tint& rhs) const { + return value == rhs.value; + } +}; + +class Element { +public: + + bool operator==(const Element& rhs) const { + return value == rhs.value && key == rhs.key; + } + + static const Tint& GetKey(const Element& element) { + return element.key; + } + + static uint32_t Hash(const Tint& key) { + return key.value; + } + + Element(Tint key, int value) : key(key), value(value) { + } + + Tint key; + int value; +}; + +typedef SkTHashCache<Element, Tint> CacheType; + +DEF_TEST(THashCache, reporter) { + Tint k11 = {11}; + Element e11(k11, 22); + + e11.value = e11.value; + Element e11Collision(k11, 0); + // Element e42(4, 2); + + //Some tests for the class Element + REPORTER_ASSERT(reporter, Element::GetKey(e11) == k11); + REPORTER_ASSERT(reporter, Element::Hash(k11) == 11); + + CacheType cache; + + // Is the cahce correctly initialized ? + REPORTER_ASSERT(reporter, 0 == cache.size()); + REPORTER_ASSERT(reporter, NULL == cache.find(k11)); + + Element& e11_c = cache.add(e11); + e11_c.value = e11_c.value; + + // Tests for simple insertion, verifying that the returned element + // has the same values as the original one + REPORTER_ASSERT(reporter, 1 == cache.size()); + REPORTER_ASSERT(reporter, NULL != cache.find(k11)); + REPORTER_ASSERT(reporter, e11_c == e11); + + Element& e11Collision_c = cache.add(e11Collision); + // Verifying that, in case of collision, the element alerady in the cache is not removed + REPORTER_ASSERT(reporter, e11Collision_c == e11); + REPORTER_ASSERT(reporter, 1 == cache.size()); + + Tint k42 = {42}; + Element e42(k42, 2); + cache.add(e42); + // Can we add more than one element? + REPORTER_ASSERT(reporter, NULL != cache.find(k11)); + REPORTER_ASSERT(reporter, NULL != cache.find(k42)); + REPORTER_ASSERT(reporter, 2 == cache.size()); + + cache.reset(); + // Does clear do its job? + REPORTER_ASSERT(reporter, 0 == cache.size()); + REPORTER_ASSERT(reporter, NULL == cache.find(k11)); + REPORTER_ASSERT(reporter, NULL == cache.find(k42)); +} |