Commit 14e95b38 by Jamie Madill

translator: Reject shaders that use both FragColor+FragData.

*re-land with fix for unused var in release* We checked this at link time in the D3D back-end, but this restriction applies to all shaders. TEST=dEQP-GLES2.functional.shaders.fragdata.* BUG=angleproject:995 BUG=478572 Change-Id: I63258f4de47e658812822f31601cc235f48c0826 Reviewed-on: https://chromium-review.googlesource.com/271470Tested-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarOlli Etuaho <oetuaho@nvidia.com> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 0ea959f4
...@@ -1049,6 +1049,27 @@ const TVariable *TParseContext::getNamedVariable(const TSourceLoc &location, ...@@ -1049,6 +1049,27 @@ const TVariable *TParseContext::getNamedVariable(const TSourceLoc &location,
{ {
recover(); recover();
} }
// Reject shaders using both gl_FragData and gl_FragColor
TQualifier qualifier = variable->getType().getQualifier();
if (qualifier == EvqFragData)
{
mUsesFragData = true;
}
else if (qualifier == EvqFragColor)
{
mUsesFragColor = true;
}
// This validation is not quite correct - it's only an error to write to
// both FragData and FragColor. For simplicity, and because users shouldn't
// be rewarded for reading from undefined varaibles, return an error
// if they are both referenced, rather than assigned.
if (mUsesFragData && mUsesFragColor)
{
error(location, "cannot use both gl_FragData and gl_FragColor", name->c_str());
recover();
}
} }
if (!variable) if (!variable)
......
...@@ -56,7 +56,9 @@ class TParseContext : angle::NonCopyable ...@@ -56,7 +56,9 @@ class TParseContext : angle::NonCopyable
mDirectiveHandler(ext, mDiagnostics, mShaderVersion, debugShaderPrecisionSupported), mDirectiveHandler(ext, mDiagnostics, mShaderVersion, debugShaderPrecisionSupported),
mPreprocessor(&mDiagnostics, &mDirectiveHandler), mPreprocessor(&mDiagnostics, &mDirectiveHandler),
mScanner(nullptr), mScanner(nullptr),
mDeferredSingleDeclarationErrorCheck(false) mDeferredSingleDeclarationErrorCheck(false),
mUsesFragData(false),
mUsesFragColor(false)
{ {
} }
...@@ -73,6 +75,7 @@ class TParseContext : angle::NonCopyable ...@@ -73,6 +75,7 @@ class TParseContext : angle::NonCopyable
const char *extraInfo=""); const char *extraInfo="");
void warning(const TSourceLoc &loc, const char *reason, const char *token, void warning(const TSourceLoc &loc, const char *reason, const char *token,
const char *extraInfo=""); const char *extraInfo="");
void recover(); void recover();
TIntermNode *getTreeRoot() const { return mTreeRoot; } TIntermNode *getTreeRoot() const { return mTreeRoot; }
void setTreeRoot(TIntermNode *treeRoot) { mTreeRoot = treeRoot; } void setTreeRoot(TIntermNode *treeRoot) { mTreeRoot = treeRoot; }
...@@ -339,6 +342,8 @@ class TParseContext : angle::NonCopyable ...@@ -339,6 +342,8 @@ class TParseContext : angle::NonCopyable
TDirectiveHandler mDirectiveHandler; TDirectiveHandler mDirectiveHandler;
pp::Preprocessor mPreprocessor; pp::Preprocessor mPreprocessor;
void *mScanner; void *mScanner;
bool mUsesFragData; // track if we are using both gl_FragData and gl_FragColor
bool mUsesFragColor;
}; };
int PaParseStrings( int PaParseStrings(
......
...@@ -750,18 +750,13 @@ bool DynamicHLSL::generateShaderLinkHLSL(const gl::Data &data, InfoLog &infoLog, ...@@ -750,18 +750,13 @@ bool DynamicHLSL::generateShaderLinkHLSL(const gl::Data &data, InfoLog &infoLog,
} }
bool usesMRT = fragmentShader->mUsesMultipleRenderTargets; bool usesMRT = fragmentShader->mUsesMultipleRenderTargets;
bool usesFragColor = fragmentShader->mUsesFragColor;
bool usesFragData = fragmentShader->mUsesFragData;
bool usesFragCoord = fragmentShader->mUsesFragCoord; bool usesFragCoord = fragmentShader->mUsesFragCoord;
bool usesPointCoord = fragmentShader->mUsesPointCoord; bool usesPointCoord = fragmentShader->mUsesPointCoord;
bool usesPointSize = vertexShader->mUsesPointSize; bool usesPointSize = vertexShader->mUsesPointSize;
bool useInstancedPointSpriteEmulation = usesPointSize && mRenderer->getWorkarounds().useInstancedPointSpriteEmulation; bool useInstancedPointSpriteEmulation = usesPointSize && mRenderer->getWorkarounds().useInstancedPointSpriteEmulation;
if (usesFragColor && usesFragData) // Validation done in the compiler
{ ASSERT(!fragmentShader->mUsesFragColor || !fragmentShader->mUsesFragData);
infoLog << "Cannot use both gl_FragColor and gl_FragData in the same fragment shader.";
return false;
}
// Write the HLSL input/output declarations // Write the HLSL input/output declarations
const int shaderModel = mRenderer->getMajorShaderModel(); const int shaderModel = mRenderer->getMajorShaderModel();
......
...@@ -545,3 +545,20 @@ TEST_F(MalformedShaderTest, AssignConstGlobalToGlobal) ...@@ -545,3 +545,20 @@ TEST_F(MalformedShaderTest, AssignConstGlobalToGlobal)
FAIL() << "Shader compilation failed, expecting success " << mInfoLog; FAIL() << "Shader compilation failed, expecting success " << mInfoLog;
} }
} }
// Statically assigning to both gl_FragData and gl_FragColor is forbidden (ESSL 1.00 section 7.2)
TEST_F(MalformedShaderTest, WriteBothFragDataAndFragColor)
{
const std::string &shaderString =
"precision mediump float;\n"
"void foo() {\n"
" gl_FragData[0].a++;\n"
"}\n"
"void main() {\n"
" gl_FragColor.x += 0.0;\n"
"}\n";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment