commit 2c3db3263bdc4b91cd7d21dbfe0501fe9bd476a5
author bsalomon <bsalomon@google.com> Tue Nov 08 13:26:24 2016 -0800
committer Commit bot <commit-bot@chromium.org> Tue Nov 08 13:26:24 2016 -0800
tree ee8d83a3ba4a771c0f7c48aa0a5b1ce63d1dfd78
parent bf111d7bc9ba3857433e30eae27f0272c34ed0fb

Fix query for GR_GL_IMPLEMENTATION_COLOR_READ_FORMAT for GrPixelConfigs that aren't renderable

We recently separated renderabibilty by Skia from FBO-bindable ( https://skia-review.googlesource.com/4345). Overlooked in that change was the necessity to be able to query GR_GL_IMPLEMENTATION_COLOR_READ_FORMAT and _TYPE for formats that we don't support rendering to Skia but are legal color attachment formats.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2489643002

Review-Url: https://codereview.chromium.org/2489643002

src/gpu/gl/GrGLCaps.cpp

diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 0af5c57..a37d0e0 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -855,7 +855,8 @@
 bool GrGLCaps::readPixelsSupported(GrPixelConfig surfaceConfig,
                                    GrPixelConfig readConfig,
                                    std::function<void (GrGLenum, GrGLint*)> getIntegerv,
-                                   std::function<bool ()> bindRenderTarget) const {
+                                   std::function<bool ()> bindRenderTarget,
+                                   std::function<void ()> unbindRenderTarget) const {
     // If it's not possible to even have a color attachment of surfaceConfig then read pixels is
     // not supported regardless of readConfig.
     if (!this->canConfigBeFBOColorAttachment(surfaceConfig)) {
@@ -912,6 +913,7 @@
         getIntegerv(GR_GL_IMPLEMENTATION_COLOR_READ_TYPE, &type);
         rpFormat->fFormat = format;
         rpFormat->fType = type;
+        unbindRenderTarget();
     }
 
     return fConfigTable[surfaceConfig].fSecondReadPixelsFormat.fFormat == readFormat &&

src/gpu/gl/GrGLCaps.h

diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index 1aed2f6..c277323 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -320,7 +320,8 @@
     bool readPixelsSupported(GrPixelConfig surfaceConfig,
                              GrPixelConfig readConfig,
                              std::function<void (GrGLenum, GrGLint*)> getIntegerv,
-                             std::function<bool ()> bindRenderTarget) const;
+                             std::function<bool ()> bindRenderTarget,
+                             std::function<void ()> unbindRenderTarget) const;
 
     bool isCoreProfile() const { return fIsCoreProfile; }
 

src/gpu/gl/GrGLGpu.cpp

diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 838ad58..266f3a8 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2293,31 +2293,50 @@
         this->flushRenderTarget(static_cast<GrGLRenderTarget*>(target), &SkIRect::EmptyIRect());
         return true;
     };
+    auto unbindRenderTarget = []{};
     auto getIntegerv = [this](GrGLenum query, GrGLint* value) {
         GR_GL_GetIntegerv(this->glInterface(), query, value);
     };
     GrPixelConfig rtConfig = target->config();
-    return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget);
+    return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget,
+                                              unbindRenderTarget);
 }
 
 bool GrGLGpu::readPixelsSupported(GrPixelConfig rtConfig, GrPixelConfig readConfig) {
-    auto bindRenderTarget = [this, rtConfig]() -> bool {
+    sk_sp<GrTexture> temp;
+    auto bindRenderTarget = [this, rtConfig, &temp]() -> bool {
         GrTextureDesc desc;
         desc.fConfig = rtConfig;
         desc.fWidth = desc.fHeight = 16;
-        desc.fFlags = kRenderTarget_GrSurfaceFlag;
-        sk_sp<GrTexture> temp(this->createTexture(desc, SkBudgeted::kNo));
-        if (!temp) {
-            return false;
+        if (this->glCaps().isConfigRenderable(rtConfig, false)) {
+            desc.fFlags = kRenderTarget_GrSurfaceFlag;
+            temp.reset(this->createTexture(desc, SkBudgeted::kNo));
+            if (!temp) {
+                return false;
+            }
+            GrGLRenderTarget* glrt = static_cast<GrGLRenderTarget*>(temp->asRenderTarget());
+            this->flushRenderTarget(glrt, &SkIRect::EmptyIRect());
+            return true;
+        } else if (this->glCaps().canConfigBeFBOColorAttachment(rtConfig)) {
+            temp.reset(this->createTexture(desc, SkBudgeted::kNo));
+            if (!temp) {
+                return false;
+            }
+            GrGLIRect vp;
+            this->bindSurfaceFBOForPixelOps(temp.get(), GR_GL_FRAMEBUFFER, &vp, kDst_TempFBOTarget);
+            fHWBoundRenderTargetUniqueID = 0;
+            return true;
         }
-        GrGLRenderTarget* glrt = static_cast<GrGLRenderTarget*>(temp->asRenderTarget());
-        this->flushRenderTarget(glrt, &SkIRect::EmptyIRect());
-        return true;
+        return false;
+    };
+    auto unbindRenderTarget = [this, &temp]() {
+        this->unbindTextureFBOForPixelOps(GR_GL_FRAMEBUFFER, temp.get());
     };
     auto getIntegerv = [this](GrGLenum query, GrGLint* value) {
         GR_GL_GetIntegerv(this->glInterface(), query, value);
     };
-    return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget);
+    return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget,
+                                              unbindRenderTarget);
 }
 
 bool GrGLGpu::readPixelsSupported(GrSurface* surfaceForConfig, GrPixelConfig readConfig) {