Revert "Revert "Added SkSL workaround for devices which cannot safely access gl_FragCoord""

This reverts commit 9d6929cccf.

Reason for revert: Re-landing, backfill reveals none of the red was related to this CL.

Original change's description:
> Revert "Added SkSL workaround for devices which cannot safely access gl_FragCoord"
> 
> This reverts commit 1001f843a4.
> 
> 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>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: skia:7410
Change-Id: Ib22bda7ff25bb7c8630cc6fa6dc809bf628ea853
Reviewed-on: https://skia-review.googlesource.com/94800
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2018-01-16 13:52:29 +00:00 committed by Skia Commit-Bot
parent 485b8c639c
commit cd3261ac65
6 changed files with 100 additions and 2 deletions

View File

@ -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;

View File

@ -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);

View File

@ -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) {

View File

@ -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) {

View File

@ -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<GrShaderCaps> CannotUseFragCoord() {
sk_sp<GrShaderCaps> result = sk_make_sp<GrShaderCaps>(GrContextOptions());
result->fVersionDeclString = "#version 400";
result->fCanUseFragCoord = false;
return result;
}
};
#endif

View File

@ -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) {