aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Brian Osman <brianosman@google.com>2018-01-12 20:21:08 +0000
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2018-01-12 20:21:13 +0000
commit9d6929cccfc3e64e75e6ef5b37f284b01b68fb28 (patch)
tree57ce1a556ed186ef251af56e299e8438aefc2541
parent4f0f933579dd9be6e99942175ecfe2b16ab98f7f (diff)
Revert "Added SkSL workaround for devices which cannot safely access gl_FragCoord"
This reverts commit 1001f843a45e95f6df1d44242b6b06c77898e870. Reason for revert: Many failures. Original change's description: > Added SkSL workaround for devices which cannot safely access gl_FragCoord > > This is the root cause of https://github.com/flutter/flutter/issues/13216 > I've got a GM that demonstrates the bug, but only in Viewer. > > Bug: skia:7410 > Change-Id: Iaa1f27b10166aa09e4dc5949e5a6ca1bd14c99ac > Reviewed-on: https://skia-review.googlesource.com/93920 > Reviewed-by: Brian Salomon <bsalomon@google.com> > Commit-Queue: Brian Osman <brianosman@google.com> TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com Change-Id: I2a2edc0a8fa11fe9dac1045dc79ae91106518b02 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:7410 Reviewed-on: https://skia-review.googlesource.com/94281 Reviewed-by: Brian Osman <brianosman@google.com> Commit-Queue: Brian Osman <brianosman@google.com>
-rw-r--r--include/gpu/GrShaderCaps.h4
-rw-r--r--src/gpu/GrShaderCaps.cpp2
-rw-r--r--src/gpu/gl/GrGLCaps.cpp6
-rw-r--r--src/sksl/SkSLGLSLCodeGenerator.cpp43
-rw-r--r--src/sksl/SkSLUtil.h11
-rw-r--r--tests/SkSLGLSLTest.cpp36
6 files changed, 2 insertions, 100 deletions
diff --git a/include/gpu/GrShaderCaps.h b/include/gpu/GrShaderCaps.h
index 66e7d42306..b56694c7dc 100644
--- a/include/gpu/GrShaderCaps.h
+++ b/include/gpu/GrShaderCaps.h
@@ -121,9 +121,6 @@ public:
// the shader.
bool mustDoOpBetweenFloorAndAbs() const { return fMustDoOpBetweenFloorAndAbs; }
- // If false, SkSL uses a workaround so that sk_FragCoord doesn't actually query gl_FragCoord
- bool canUseFragCoord() const { return fCanUseFragCoord; }
-
bool requiresLocalOutputColorForFBFetch() const { return fRequiresLocalOutputColorForFBFetch; }
bool mustObfuscateUniformColor() const { return fMustObfuscateUniformColor; }
@@ -278,7 +275,6 @@ private:
bool fRequiresLocalOutputColorForFBFetch : 1;
bool fMustObfuscateUniformColor : 1;
bool fMustGuardDivisionEvenAfterExplicitZeroCheck : 1;
- bool fCanUseFragCoord : 1;
const char* fVersionDeclString;
diff --git a/src/gpu/GrShaderCaps.cpp b/src/gpu/GrShaderCaps.cpp
index 2ab19478af..2041ada635 100644
--- a/src/gpu/GrShaderCaps.cpp
+++ b/src/gpu/GrShaderCaps.cpp
@@ -38,7 +38,6 @@ GrShaderCaps::GrShaderCaps(const GrContextOptions& options) {
fRequiresLocalOutputColorForFBFetch = false;
fMustObfuscateUniformColor = false;
fMustGuardDivisionEvenAfterExplicitZeroCheck = false;
- fCanUseFragCoord = true;
fFlatInterpolationSupport = false;
fPreferFlatInterpolation = false;
fNoPerspectiveInterpolationSupport = false;
@@ -114,7 +113,6 @@ void GrShaderCaps::dumpJSON(SkJSONWriter* writer) const {
writer->appendBool("Must obfuscate uniform color", fMustObfuscateUniformColor);
writer->appendBool("Must guard division even after explicit zero check",
fMustGuardDivisionEvenAfterExplicitZeroCheck);
- writer->appendBool("Can use gl_FragCoord", fCanUseFragCoord);
writer->appendBool("Flat interpolation support", fFlatInterpolationSupport);
writer->appendBool("Prefer flat interpolation", fPreferFlatInterpolation);
writer->appendBool("No perspective interpolation support", fNoPerspectiveInterpolationSupport);
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index d310cd219b..4996859c51 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -1048,12 +1048,6 @@ void GrGLCaps::initGLSL(const GrGLContextInfo& ctxInfo, const GrGLInterface* gli
shaderCaps->fMustGuardDivisionEvenAfterExplicitZeroCheck = true;
}
#endif
-
- // We've seen Adreno 3xx devices produce incorrect (flipped) values for gl_FragCoord, in some
- // (rare) situations. It's sporadic, and mostly on older drviers.
- if (kAdreno3xx_GrGLRenderer == ctxInfo.renderer()) {
- shaderCaps->fCanUseFragCoord = false;
- }
}
bool GrGLCaps::hasPathRenderingSupport(const GrGLContextInfo& ctxInfo, const GrGLInterface* gli) {
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index f3400c7f5a..9e2336e22e 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -597,12 +597,6 @@ void GLSLCodeGenerator::writeConstructor(const Constructor& c, Precedence parent
}
void GLSLCodeGenerator::writeFragCoord() {
- if (!fProgram.fSettings.fCaps->canUseFragCoord()) {
- this->write("vec4(sk_FragCoord_Workaround.xyz / sk_FragCoord_Workaround.w, "
- "1.0 / sk_FragCoord_Workaround.w)");
- return;
- }
-
// We only declare "gl_FragCoord" when we're in the case where we want to use layout qualifiers
// to reverse y. Otherwise it isn't necessary and whether the "in" qualifier appears in the
// declaration varies in earlier GLSL specs. So it is simpler to omit it.
@@ -625,7 +619,7 @@ void GLSLCodeGenerator::writeFragCoord() {
// The Adreno compiler seems to be very touchy about access to "gl_FragCoord".
// Accessing glFragCoord.zw can cause a program to fail to link. Additionally,
// depending on the surrounding code, accessing .xy with a uniform involved can
- // do the same thing. Copying gl_FragCoord.xy into a temp float2 beforehand
+ // do the same thing. Copying gl_FragCoord.xy into a temp float2beforehand
// (and only accessing .xy) seems to "fix" things.
const char* precision = usesPrecisionModifiers() ? "highp " : "";
fHeader.writeText("uniform ");
@@ -646,6 +640,7 @@ void GLSLCodeGenerator::writeFragCoord() {
}
}
+
void GLSLCodeGenerator::writeVariableReference(const VariableReference& ref) {
switch (ref.fVariable.fModifiers.fLayout.fBuiltin) {
case SK_FRAGCOLOR_BUILTIN:
@@ -685,10 +680,6 @@ void GLSLCodeGenerator::writeIndexExpression(const IndexExpression& expr) {
this->write("]");
}
-bool is_sk_position(const FieldAccess& f) {
- return "sk_Position" == f.fBase->fType.fields()[f.fFieldIndex].fName;
-}
-
void GLSLCodeGenerator::writeFieldAccess(const FieldAccess& f) {
if (f.fOwnerKind == FieldAccess::kDefault_OwnerKind) {
this->writeExpression(*f.fBase, kPostfix_Precedence);
@@ -764,22 +755,11 @@ void GLSLCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
if (precedence >= parentPrecedence) {
this->write("(");
}
- bool positionWorkaround = Compiler::IsAssignment(b.fOperator) &&
- Expression::kFieldAccess_Kind == b.fLeft->fKind &&
- is_sk_position((FieldAccess&) *b.fLeft) &&
- !strstr(b.fRight->description().c_str(), "sk_RTAdjust") &&
- !fProgram.fSettings.fCaps->canUseFragCoord();
- if (positionWorkaround) {
- this->write("sk_FragCoord_Workaround = (");
- }
this->writeExpression(*b.fLeft, precedence);
this->write(" ");
this->write(Compiler::OperatorName(b.fOperator));
this->write(" ");
this->writeExpression(*b.fRight, precedence);
- if (positionWorkaround) {
- this->write(")");
- }
if (precedence >= parentPrecedence) {
this->write(")");
}
@@ -1208,25 +1188,6 @@ void GLSLCodeGenerator::writeHeader() {
this->writeExtension((Extension&) *e);
}
}
- if (!fProgram.fSettings.fCaps->canUseFragCoord()) {
- Layout layout;
- switch (fProgram.fKind) {
- case Program::kVertex_Kind: {
- Modifiers modifiers(layout, Modifiers::kOut_Flag | Modifiers::kHighp_Flag);
- this->writeModifiers(modifiers, true);
- this->write("vec4 sk_FragCoord_Workaround;\n");
- break;
- }
- case Program::kFragment_Kind: {
- Modifiers modifiers(layout, Modifiers::kIn_Flag | Modifiers::kHighp_Flag);
- this->writeModifiers(modifiers, true);
- this->write("vec4 sk_FragCoord_Workaround;\n");
- break;
- }
- default:
- break;
- }
- }
}
void GLSLCodeGenerator::writeProgramElement(const ProgramElement& e) {
diff --git a/src/sksl/SkSLUtil.h b/src/sksl/SkSLUtil.h
index 787698f2c6..7a39386269 100644
--- a/src/sksl/SkSLUtil.h
+++ b/src/sksl/SkSLUtil.h
@@ -174,10 +174,6 @@ public:
bool canUseFractForNegativeValues() const {
return true;
}
-
- bool canUseFragCoord() const {
- return true;
- }
};
extern StandaloneShaderCaps standaloneCaps;
@@ -304,13 +300,6 @@ public:
result->fCanUseAnyFunctionInShader = false;
return result;
}
-
- static sk_sp<GrShaderCaps> CannotUseFragCoord() {
- sk_sp<GrShaderCaps> result = sk_make_sp<GrShaderCaps>(GrContextOptions());
- result->fVersionDeclString = "#version 400";
- result->fCanUseFragCoord = false;
- return result;
- }
};
#endif
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index 9b0571bcdc..1ae421f8af 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -1059,42 +1059,6 @@ DEF_TEST(SkSLFragCoord, r) {
"}\n",
&inputs);
REPORTER_ASSERT(r, !inputs.fRTHeight);
-
- test(r,
- "in float4 pos; void main() { sk_Position = pos; }",
- *SkSL::ShaderCapsFactory::CannotUseFragCoord(),
- "#version 400\n"
- "out vec4 sk_FragCoord_Workaround;\n"
- "in vec4 pos;\n"
- "void main() {\n"
- " sk_FragCoord_Workaround = (gl_Position = pos);\n"
- "}\n",
- SkSL::Program::kVertex_Kind);
-
- test(r,
- "in uniform float4 sk_RTAdjust; in float4 pos; void main() { sk_Position = pos; }",
- *SkSL::ShaderCapsFactory::CannotUseFragCoord(),
- "#version 400\n"
- "out vec4 sk_FragCoord_Workaround;\n"
- "in uniform vec4 sk_RTAdjust;\n"
- "in vec4 pos;\n"
- "void main() {\n"
- " sk_FragCoord_Workaround = (gl_Position = pos);\n"
- " gl_Position = vec4(gl_Position.x * sk_RTAdjust.x + gl_Position.w * sk_RTAdjust.y, "
- "gl_Position.y * sk_RTAdjust.z + gl_Position.w * sk_RTAdjust.w, 0, gl_Position.w);\n"
- "}\n",
- SkSL::Program::kVertex_Kind);
-
- test(r,
- "void main() { sk_FragColor.xy = sk_FragCoord.xy; }",
- *SkSL::ShaderCapsFactory::CannotUseFragCoord(),
- "#version 400\n"
- "in vec4 sk_FragCoord_Workaround;\n"
- "out vec4 sk_FragColor;\n"
- "void main() {\n"
- " sk_FragColor.xy = vec4(sk_FragCoord_Workaround.xyz / sk_FragCoord_Workaround.w, "
- "1.0 / sk_FragCoord_Workaround.w).xy;\n"
- "}\n");
}
DEF_TEST(SkSLVertexID, r) {