From 753a2964afe5661ce9b2a8ca77ca9d0aabd3173c Mon Sep 17 00:00:00 2001 From: piotaixr Date: Thu, 3 Jul 2014 06:50:50 -0700 Subject: [PATCH] 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 | 37 +++++++++++++++++++++++++++++++++++-- src/gpu/gl/GrGLCaps.h | 40 +++++++++++++++++++++++++++++++++++++++- src/gpu/gl/GrGpuGL.cpp | 14 ++++++++------ 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index 8f2d4c1297..2e85eb5adc 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/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; +} diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h index ccf04fd7ba..24bf83afbb 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/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 fReadPixelsSupportedCache; + typedef GrDrawTargetCaps INHERITED; }; diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp index 2eb76a5aff..73938b8374 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, + 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.