Revert of Caching the result of readPixelsSupported (https://codereview.chromium.org/364193004/)

Reason for revert:
appears to crash GM on Ubuntu and Win8

http://108.170.220.120:10117/builders/Test-Ubuntu12-ShuttleA-GTX660-x86-Release/builds/1237/steps/GenerateGMs/logs/stdio

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

R=junov@chromium.org, piotaixr@chromium.org
TBR=junov@chromium.org, piotaixr@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:2681

Author: reed@chromium.org

Review URL: https://codereview.chromium.org/367323003
This commit is contained in:
reed 2014-07-03 07:54:08 -07:00 committed by Commit bot
parent e1085e0381
commit 2fe7b466c1
3 changed files with 9 additions and 82 deletions

View File

@ -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::doReadPixelsSupported(const GrGLInterface* intf,
bool GrGLCaps::readPixelsSupported(const GrGLInterface* intf,
GrGLenum format,
GrGLenum type) const {
if (GR_GL_RGBA == format && GR_GL_UNSIGNED_BYTE == type) {
@ -589,25 +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 {
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;
@ -836,17 +817,3 @@ 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;
}

View File

@ -13,7 +13,6 @@
#include "GrGLStencilBuffer.h"
#include "SkTArray.h"
#include "SkTDArray.h"
#include "SkTDynamicHash.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,38 +364,6 @@ 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;
};

View File

@ -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,
fHWBoundRenderTarget->config()
)) {
} 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.