Fix various issues with framebuffer fetch

This CL does two things. First it fixes a bug of ours where when using
a custom fbFetch variable (es 3.0 and higher), we sometimes overwrite
the out color and then attempt to use it as the dst color later. This is
fixed here by using an intermediate variable.

Secondly I've added a workaround to an andreno fbFetch where reading from
the outColor always returns the original dstColor even if we've overwritten
the value.

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

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

diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 75ae77687975c595f879d01446b6bce8b8e7ac60..3385dbce31e3efb64a5058603f13f69082fc52ae 100644 (file)
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -792,6 +792,13 @@ void GrGLCaps::initGLSL(const GrGLContextInfo& ctxInfo) {
     if (kIntel_GrGLVendor == ctxInfo.vendor()) {
         glslCaps->fMustForceNegatedAtanParamToFloat = true;
     }
+
+    // On Adreno devices with framebuffer fetch support, there is a bug where they always return
+    // the original dst color when reading the outColor even after being written to. By using a
+    // local outColor we can work around this bug.
+    if (glslCaps->fFBFetchSupport && kQualcomm_GrGLVendor == ctxInfo.vendor()) {
+        glslCaps->fRequiresLocalOutputColorForFBFetch = true;
+    }
 }
 
 bool GrGLCaps::hasPathRenderingSupport(const GrGLContextInfo& ctxInfo, const GrGLInterface* gli) {

diff --git a/src/gpu/glsl/GrGLSLCaps.cpp b/src/gpu/glsl/GrGLSLCaps.cpp
index 1f4b855f5dacea30baea57094dc5176ddf7e170d..b33e3082ecdc4f49ccfd4d2154c7a510d7ccd516 100755 (executable)
--- a/src/gpu/glsl/GrGLSLCaps.cpp
+++ b/src/gpu/glsl/GrGLSLCaps.cpp
@@ -23,6 +23,7 @@ GrGLSLCaps::GrGLSLCaps(const GrContextOptions& options) {
     fCanUseAnyFunctionInShader = true;
     fCanUseMinAndAbsTogether = true;
     fMustForceNegatedAtanParamToFloat = false;
+    fRequiresLocalOutputColorForFBFetch = false;
     fFlatInterpolationSupport = false;
     fNoPerspectiveInterpolationSupport = false;
     fMultisampleInterpolationSupport = false;
@@ -73,6 +74,8 @@ SkString GrGLSLCaps::dump() const {
     r.appendf("Can use min() and abs() together: %s\n", (fCanUseMinAndAbsTogether ? "YES" : "NO"));
     r.appendf("Must force negated atan param to float: %s\n", (fMustForceNegatedAtanParamToFloat ?
                                                                "YES" : "NO"));
+    r.appendf("Must use local out color for FBFetch: %s\n", (fRequiresLocalOutputColorForFBFetch ?
+                                                             "YES" : "NO"));
     r.appendf("Flat interpolation support: %s\n", (fFlatInterpolationSupport ?  "YES" : "NO"));
     r.appendf("No perspective interpolation support: %s\n", (fNoPerspectiveInterpolationSupport ?
                                                              "YES" : "NO"));

diff --git a/src/gpu/glsl/GrGLSLCaps.h b/src/gpu/glsl/GrGLSLCaps.h
index 19a34620dc0f41c2a84e641f76f12d24b279a53f..d8145e729d69bbf46f014ba1f30f4596d8acbc01 100755 (executable)
--- a/src/gpu/glsl/GrGLSLCaps.h
+++ b/src/gpu/glsl/GrGLSLCaps.h
@@ -91,6 +91,8 @@ public:
 
     bool mustForceNegatedAtanParamToFloat() const { return fMustForceNegatedAtanParamToFloat; }
 
+    bool requiresLocalOutputColorForFBFetch() const { return fRequiresLocalOutputColorForFBFetch; }
+
     // Returns the string of an extension that must be enabled in the shader to support
     // derivatives. If nullptr is returned then no extension needs to be enabled. Before calling
     // this function, the caller should check that shaderDerivativeSupport exists.
@@ -199,6 +201,7 @@ private:
     // Used for specific driver bug work arounds
     bool fCanUseMinAndAbsTogether : 1;
     bool fMustForceNegatedAtanParamToFloat : 1;
+    bool fRequiresLocalOutputColorForFBFetch : 1;
 
     const char* fVersionDeclString;
 

diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index e6ecf8c1719ece297c40923846be36c49c198ac9..4513a69fc85bbab46ac7c306ca7beef50c1c8050 100644 (file)
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -15,7 +15,7 @@
 #include "glsl/GrGLSLUniformHandler.h"
 #include "glsl/GrGLSLVarying.h"
 
-const char* GrGLSLFragmentShaderBuilder::kDstTextureColorName = "_dstColor";
+const char* GrGLSLFragmentShaderBuilder::kDstColorName = "_dstColor";
 
 static const char* sample_offset_array_name(GrGLSLFPFragmentBuilder::Coordinates coords) {
     static const char* kArrayNames[] = {
@@ -264,11 +264,11 @@ const char* GrGLSLFragmentShaderBuilder::dstColor() {
             this->enableCustomOutput();
             fOutputs[fCustomColorOutputIndex].setTypeModifier(GrShaderVar::kInOut_TypeModifier);
             fbFetchColorName = DeclaredColorOutputName();
+            // Set the dstColor to an intermediate variable so we don't override it with the output
+            this->codeAppendf("vec4 %s = %s;", kDstColorName, fbFetchColorName);
         }
-        return fbFetchColorName;
-    } else {
-        return kDstTextureColorName;
     }
+    return kDstColorName;
 }
 
 void GrGLSLFragmentShaderBuilder::enableAdvancedBlendEquationIfNeeded(GrBlendEquation equation) {

diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
index 6845376261aee8e22e16fdf565ce003e04bc4d21..55ef9db207ff4af643ba8a7414d12d5acf72ba95 100644 (file)
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
@@ -212,7 +212,7 @@ private:
     void onFinalize() override;
     void defineSampleOffsetArray(const char* name, const SkMatrix&);
 
-    static const char* kDstTextureColorName;
+    static const char* kDstColorName;
 
     /*
      * State that tracks which child proc in the proc tree is currently emitting code.  This is

diff --git a/src/gpu/glsl/GrGLSLXferProcessor.cpp b/src/gpu/glsl/GrGLSLXferProcessor.cpp
index f0f5efd37ba2f174318a81cd7d0130328ccfd488..0f7a3db718fa44e3c2b0c1d52b49a92e49e01bc2 100644 (file)
--- a/src/gpu/glsl/GrGLSLXferProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLXferProcessor.cpp
@@ -22,6 +22,8 @@ void GrGLSLXferProcessor::emitCode(const EmitArgs& args) {
     GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
     const char* dstColor = fragBuilder->dstColor();
 
+    bool needsLocalOutColor = false;
+
     if (args.fXP.getDstTexture()) {
         bool topDown = kTopLeft_GrSurfaceOrigin == args.fXP.getDstTexture()->origin();
 
@@ -59,6 +61,15 @@ void GrGLSLXferProcessor::emitCode(const EmitArgs& args) {
         fragBuilder->codeAppendf("vec4 %s = ", dstColor);
         fragBuilder->appendTextureLookup(args.fTexSamplers[0], "_dstTexCoord", kVec2f_GrSLType);
         fragBuilder->codeAppend(";");
+    } else {
+        needsLocalOutColor = args.fGLSLCaps->requiresLocalOutputColorForFBFetch();
+    }
+
+    const char* outColor = "_localColorOut";
+    if (!needsLocalOutColor) {
+        outColor = args.fOutputPrimary;
+    } else {
+        fragBuilder->codeAppendf("vec4 %s;", outColor);
     }
 
     this->emitBlendCodeForDstRead(fragBuilder,
@@ -66,9 +77,12 @@ void GrGLSLXferProcessor::emitCode(const EmitArgs& args) {
                                   args.fInputColor,
                                   args.fInputCoverage,
                                   dstColor,
-                                  args.fOutputPrimary,
+                                  outColor,
                                   args.fOutputSecondary,
                                   args.fXP);
+    if (needsLocalOutColor) {
+        fragBuilder->codeAppendf("%s = %s;", args.fOutputPrimary, outColor);
+    }
 }
 
 void GrGLSLXferProcessor::setData(const GrGLSLProgramDataManager& pdm, const GrXferProcessor& xp) {