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 75ae776..3385dbc 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -792,6 +792,13 @@
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 1f4b855..b33e308 100755
--- a/src/gpu/glsl/GrGLSLCaps.cpp
+++ b/src/gpu/glsl/GrGLSLCaps.cpp
@@ -23,6 +23,7 @@
fCanUseAnyFunctionInShader = true;
fCanUseMinAndAbsTogether = true;
fMustForceNegatedAtanParamToFloat = false;
+ fRequiresLocalOutputColorForFBFetch = false;
fFlatInterpolationSupport = false;
fNoPerspectiveInterpolationSupport = false;
fMultisampleInterpolationSupport = false;
@@ -73,6 +74,8 @@
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 19a3462..d8145e7 100755
--- a/src/gpu/glsl/GrGLSLCaps.h
+++ b/src/gpu/glsl/GrGLSLCaps.h
@@ -91,6 +91,8 @@
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 @@
// 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 e6ecf8c..4513a69 100644
--- 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 @@
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 6845376..55ef9db 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
@@ -212,7 +212,7 @@
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 f0f5efd..0f7a3db 100644
--- a/src/gpu/glsl/GrGLSLXferProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLXferProcessor.cpp
@@ -22,6 +22,8 @@
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 @@
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 @@
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) {