Revert of Reopened: Caching the result of readPixelsSupported (https://codereview.chromium.org/364193004/)
Reason for revert: This appears to be causing failures on Android when running tests. Original issue's description: > 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 > > Committed: https://skia.googlesource.com/skia/+/8339371f1ec3c57a0741932fd96bff32c53d4e54 R=junov@chromium.org, reed@chromium.org, bsalomon@chromium.org, mtklein@google.com, bsalomon@google.com, piotaixr@chromium.org TBR=bsalomon@chromium.org, bsalomon@google.com, junov@chromium.org, mtklein@google.com, piotaixr@chromium.org, reed@chromium.org NOTREECHECKS=true NOTRY=true BUG=skia:2681 Author: bungeman@google.com Review URL: https://codereview.chromium.org/395203002
This commit is contained in:
parent
33ac9506fb
commit
c7af812b42
@ -199,7 +199,6 @@
|
||||
'<(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',
|
||||
|
@ -174,7 +174,6 @@
|
||||
'../tests/StrokeTest.cpp',
|
||||
'../tests/SurfaceTest.cpp',
|
||||
'../tests/TArrayTest.cpp',
|
||||
'../tests/THashCache.cpp',
|
||||
'../tests/TLSTest.cpp',
|
||||
'../tests/TSetTest.cpp',
|
||||
'../tests/TextureCompressionTest.cpp',
|
||||
|
@ -1,77 +0,0 @@
|
||||
/*
|
||||
* 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 */
|
||||
|
@ -48,8 +48,6 @@ void GrGLCaps::reset() {
|
||||
fIsCoreProfile = false;
|
||||
fFullClearIsFree = false;
|
||||
fDropsTileOnZeroDivide = false;
|
||||
|
||||
fReadPixelsSupportedCache.reset();
|
||||
}
|
||||
|
||||
GrGLCaps::GrGLCaps(const GrGLCaps& caps) : GrDrawTargetCaps() {
|
||||
@ -506,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 &&
|
||||
@ -562,9 +560,9 @@ void GrGLCaps::initConfigTexturableTable(const GrGLContextInfo& ctxInfo, const G
|
||||
}
|
||||
}
|
||||
|
||||
bool GrGLCaps::doReadPixelsSupported(const GrGLInterface* intf,
|
||||
GrGLenum format,
|
||||
GrGLenum type) const {
|
||||
bool GrGLCaps::readPixelsSupported(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;
|
||||
@ -591,26 +589,6 @@ bool GrGLCaps::doReadPixelsSupported(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;
|
||||
|
@ -11,9 +11,8 @@
|
||||
|
||||
#include "GrDrawTargetCaps.h"
|
||||
#include "GrGLStencilBuffer.h"
|
||||
#include "SkChecksum.h"
|
||||
#include "SkTHashCache.h"
|
||||
#include "SkTArray.h"
|
||||
#include "SkTDArray.h"
|
||||
|
||||
class GrGLContextInfo;
|
||||
|
||||
@ -254,8 +253,7 @@ public:
|
||||
/// Does ReadPixels support the provided format/type combo?
|
||||
bool readPixelsSupported(const GrGLInterface* intf,
|
||||
GrGLenum format,
|
||||
GrGLenum type,
|
||||
GrGLenum currFboFormat) const;
|
||||
GrGLenum type) const;
|
||||
|
||||
bool isCoreProfile() const { return fIsCoreProfile; }
|
||||
|
||||
@ -326,10 +324,6 @@ 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;
|
||||
@ -370,46 +364,6 @@ 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;
|
||||
};
|
||||
|
||||
|
@ -170,6 +170,8 @@ GrGpuGL::~GrGpuGL() {
|
||||
}
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
|
||||
GrPixelConfig GrGpuGL::preferredReadPixelsConfig(GrPixelConfig readConfig,
|
||||
GrPixelConfig surfaceConfig) const {
|
||||
if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && kRGBA_8888_GrPixelConfig == readConfig) {
|
||||
@ -180,13 +182,9 @@ 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,
|
||||
surfaceConfig
|
||||
)) {
|
||||
} else if (readConfig == kBGRA_8888_GrPixelConfig &&
|
||||
!this->glCaps().readPixelsSupported(this->glInterface(),
|
||||
GR_GL_BGRA, GR_GL_UNSIGNED_BYTE)) {
|
||||
return kRGBA_8888_GrPixelConfig;
|
||||
} else {
|
||||
return readConfig;
|
||||
@ -715,7 +713,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.
|
||||
|
@ -1,91 +0,0 @@
|
||||
/*
|
||||
* 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));
|
||||
}
|
Loading…
Reference in New Issue
Block a user