diff options
author | Brian Osman <brianosman@google.com> | 2018-01-12 20:21:08 +0000 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-01-12 20:21:13 +0000 |
commit | 9d6929cccfc3e64e75e6ef5b37f284b01b68fb28 (patch) | |
tree | 57ce1a556ed186ef251af56e299e8438aefc2541 | |
parent | 4f0f933579dd9be6e99942175ecfe2b16ab98f7f (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.h | 4 | ||||
-rw-r--r-- | src/gpu/GrShaderCaps.cpp | 2 | ||||
-rw-r--r-- | src/gpu/gl/GrGLCaps.cpp | 6 | ||||
-rw-r--r-- | src/sksl/SkSLGLSLCodeGenerator.cpp | 43 | ||||
-rw-r--r-- | src/sksl/SkSLUtil.h | 11 | ||||
-rw-r--r-- | tests/SkSLGLSLTest.cpp | 36 |
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) { |