From 1001f843a45e95f6df1d44242b6b06c77898e870 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Fri, 12 Jan 2018 10:16:41 -0500 Subject: [PATCH] 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 Commit-Queue: Brian Osman --- include/gpu/GrShaderCaps.h | 4 +++ src/gpu/GrShaderCaps.cpp | 2 ++ src/gpu/gl/GrGLCaps.cpp | 6 +++++ src/sksl/SkSLGLSLCodeGenerator.cpp | 43 ++++++++++++++++++++++++++++-- src/sksl/SkSLUtil.h | 11 ++++++++ tests/SkSLGLSLTest.cpp | 36 +++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 2 deletions(-) diff --git a/include/gpu/GrShaderCaps.h b/include/gpu/GrShaderCaps.h index b56694c7dc..66e7d42306 100644 --- a/include/gpu/GrShaderCaps.h +++ b/include/gpu/GrShaderCaps.h @@ -121,6 +121,9 @@ 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; } @@ -275,6 +278,7 @@ 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 2041ada635..2ab19478af 100644 --- a/src/gpu/GrShaderCaps.cpp +++ b/src/gpu/GrShaderCaps.cpp @@ -38,6 +38,7 @@ GrShaderCaps::GrShaderCaps(const GrContextOptions& options) { fRequiresLocalOutputColorForFBFetch = false; fMustObfuscateUniformColor = false; fMustGuardDivisionEvenAfterExplicitZeroCheck = false; + fCanUseFragCoord = true; fFlatInterpolationSupport = false; fPreferFlatInterpolation = false; fNoPerspectiveInterpolationSupport = false; @@ -113,6 +114,7 @@ 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 4996859c51..d310cd219b 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -1048,6 +1048,12 @@ 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 9e2336e22e..f3400c7f5a 100644 --- a/src/sksl/SkSLGLSLCodeGenerator.cpp +++ b/src/sksl/SkSLGLSLCodeGenerator.cpp @@ -597,6 +597,12 @@ 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. @@ -619,7 +625,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 float2beforehand + // do the same thing. Copying gl_FragCoord.xy into a temp float2 beforehand // (and only accessing .xy) seems to "fix" things. const char* precision = usesPrecisionModifiers() ? "highp " : ""; fHeader.writeText("uniform "); @@ -640,7 +646,6 @@ void GLSLCodeGenerator::writeFragCoord() { } } - void GLSLCodeGenerator::writeVariableReference(const VariableReference& ref) { switch (ref.fVariable.fModifiers.fLayout.fBuiltin) { case SK_FRAGCOLOR_BUILTIN: @@ -680,6 +685,10 @@ 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); @@ -755,11 +764,22 @@ 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(")"); } @@ -1188,6 +1208,25 @@ 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 7a39386269..787698f2c6 100644 --- a/src/sksl/SkSLUtil.h +++ b/src/sksl/SkSLUtil.h @@ -174,6 +174,10 @@ public: bool canUseFractForNegativeValues() const { return true; } + + bool canUseFragCoord() const { + return true; + } }; extern StandaloneShaderCaps standaloneCaps; @@ -300,6 +304,13 @@ public: result->fCanUseAnyFunctionInShader = false; return result; } + + static sk_sp CannotUseFragCoord() { + sk_sp result = sk_make_sp(GrContextOptions()); + result->fVersionDeclString = "#version 400"; + result->fCanUseFragCoord = false; + return result; + } }; #endif diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp index 1ae421f8af..9b0571bcdc 100644 --- a/tests/SkSLGLSLTest.cpp +++ b/tests/SkSLGLSLTest.cpp @@ -1059,6 +1059,42 @@ 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) {