Fix ASSERT when rewriting else-if blocks with no else.
The code assumed an else-if would end in an else. We can also save a
few allocations in the cases where there is no else.
BUG=angle:699
Change-Id: I550857366775b4a34aea97e117ef732297d3f448
Reviewed-on: https://chromium-review.googlesource.com/208681
Tested-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Nicolas Capens <capn@chromium.org>
diff --git a/src/compiler/translator/RewriteElseBlocks.cpp b/src/compiler/translator/RewriteElseBlocks.cpp
index 3a32aa1..b03beb5 100644
--- a/src/compiler/translator/RewriteElseBlocks.cpp
+++ b/src/compiler/translator/RewriteElseBlocks.cpp
@@ -14,6 +14,24 @@
namespace sh
{
+namespace
+{
+
+class ElseBlockRewriter : public TIntermTraverser
+{
+ public:
+ ElseBlockRewriter();
+
+ protected:
+ bool visitAggregate(Visit visit, TIntermAggregate *aggregate);
+
+ private:
+ int mTemporaryIndex;
+ const TType *mFunctionType;
+
+ TIntermNode *rewriteSelection(TIntermSelection *selection);
+};
+
TIntermSymbol *MakeNewTemporary(const TString &name, TBasicType type)
{
TType variableType(type, EbpHigh, EvqInternal);
@@ -83,46 +101,54 @@
TIntermNode *ElseBlockRewriter::rewriteSelection(TIntermSelection *selection)
{
- ASSERT(selection->getFalseBlock() != NULL);
+ ASSERT(selection != NULL);
TString temporaryName = "cond_" + str(mTemporaryIndex++);
TIntermTyped *typedCondition = selection->getCondition()->getAsTyped();
TType resultType(EbtBool, EbpUndefined);
- TIntermSymbol *conditionSymbolA = MakeNewTemporary(temporaryName, EbtBool);
- TIntermSymbol *conditionSymbolB = MakeNewTemporary(temporaryName, EbtBool);
- TIntermSymbol *conditionSymbolC = MakeNewTemporary(temporaryName, EbtBool);
- TIntermBinary *storeCondition = MakeNewBinary(EOpInitialize, conditionSymbolA,
+ TIntermSymbol *conditionSymbolInit = MakeNewTemporary(temporaryName, EbtBool);
+ TIntermBinary *storeCondition = MakeNewBinary(EOpInitialize, conditionSymbolInit,
typedCondition, resultType);
- TIntermUnary *negatedCondition = MakeNewUnary(EOpLogicalNot, conditionSymbolB);
TIntermNode *negatedElse = NULL;
- // crbug.com/346463
- // D3D generates error messages claiming a function has no return value, when rewriting
- // an if-else clause that returns something non-void in a function. By appending dummy
- // returns (that are unreachable) we can silence this compile error.
- if (mFunctionType && mFunctionType->getBasicType() != EbtVoid)
+ TIntermSelection *falseBlock = NULL;
+
+ if (selection->getFalseBlock())
{
- TString typeString = mFunctionType->getStruct() ? mFunctionType->getStruct()->name() :
- mFunctionType->getBasicString();
- TString rawText = "return (" + typeString + ")0";
- negatedElse = new TIntermRaw(*mFunctionType, rawText);
+ // crbug.com/346463
+ // D3D generates error messages claiming a function has no return value, when rewriting
+ // an if-else clause that returns something non-void in a function. By appending dummy
+ // returns (that are unreachable) we can silence this compile error.
+ if (mFunctionType && mFunctionType->getBasicType() != EbtVoid)
+ {
+ TString typeString = mFunctionType->getStruct() ? mFunctionType->getStruct()->name() :
+ mFunctionType->getBasicString();
+ TString rawText = "return (" + typeString + ")0";
+ negatedElse = new TIntermRaw(*mFunctionType, rawText);
+ }
+
+ TIntermSymbol *conditionSymbolElse = MakeNewTemporary(temporaryName, EbtBool);
+ TIntermUnary *negatedCondition = MakeNewUnary(EOpLogicalNot, conditionSymbolElse);
+ falseBlock = new TIntermSelection(negatedCondition,
+ selection->getFalseBlock(), negatedElse);
}
- TIntermSelection *falseBlock = new TIntermSelection(negatedCondition,
- selection->getFalseBlock(), negatedElse);
- TIntermSelection *newIfElse = new TIntermSelection(conditionSymbolC,
- selection->getTrueBlock(), falseBlock);
+ TIntermSymbol *conditionSymbolSel = MakeNewTemporary(temporaryName, EbtBool);
+ TIntermSelection *newSelection = new TIntermSelection(conditionSymbolSel,
+ selection->getTrueBlock(), falseBlock);
TIntermAggregate *declaration = new TIntermAggregate(EOpDeclaration);
declaration->getSequence()->push_back(storeCondition);
TIntermAggregate *block = new TIntermAggregate(EOpSequence);
block->getSequence()->push_back(declaration);
- block->getSequence()->push_back(newIfElse);
+ block->getSequence()->push_back(newSelection);
return block;
}
+}
+
void RewriteElseBlocks(TIntermNode *node)
{
ElseBlockRewriter rewriter;
diff --git a/src/compiler/translator/RewriteElseBlocks.h b/src/compiler/translator/RewriteElseBlocks.h
index 172928f..39963d6 100644
--- a/src/compiler/translator/RewriteElseBlocks.h
+++ b/src/compiler/translator/RewriteElseBlocks.h
@@ -15,21 +15,6 @@
namespace sh
{
-class ElseBlockRewriter : public TIntermTraverser
-{
- public:
- ElseBlockRewriter();
-
- protected:
- bool visitAggregate(Visit visit, TIntermAggregate *aggregate);
-
- private:
- int mTemporaryIndex;
- const TType *mFunctionType;
-
- TIntermNode *rewriteSelection(TIntermSelection *selection);
-};
-
void RewriteElseBlocks(TIntermNode *node);
}
diff --git a/tests/angle_tests/GLSLTest.cpp b/tests/angle_tests/GLSLTest.cpp
index ea04478..ee6735f 100644
--- a/tests/angle_tests/GLSLTest.cpp
+++ b/tests/angle_tests/GLSLTest.cpp
@@ -143,3 +143,65 @@
GLuint program = compileProgram(vertexShaderSource, fragmentShaderSource);
EXPECT_NE(0u, program);
}
+
+TEST_F(GLSLTest, ElseIfRewriting)
+{
+ const std::string &vertexShaderSource =
+ "attribute vec4 a_position;\n"
+ "varying float v;\n"
+ "void main() {\n"
+ " gl_Position = a_position;\n"
+ " v = 1.0;\n"
+ " if (a_position.x <= 0.5) {\n"
+ " v = 0.0;\n"
+ " } else if (a_position.x >= 0.5) {\n"
+ " v = 2.0;\n"
+ " }\n"
+ "}\n";
+
+ const std::string &fragmentShaderSource =
+ "precision highp float;\n"
+ "varying float v;\n"
+ "void main() {\n"
+ " vec4 color = vec4(1.0, 0.0, 0.0, 1.0);\n"
+ " if (v >= 1.0) color = vec4(0.0, 1.0, 0.0, 1.0);\n"
+ " if (v >= 2.0) color = vec4(0.0, 0.0, 1.0, 1.0);\n"
+ " gl_FragColor = color;\n"
+ "}\n";
+
+ GLuint program = compileProgram(vertexShaderSource, fragmentShaderSource);
+ ASSERT_NE(0u, program);
+
+ drawQuad(program, "a_position", 0.5f);
+ swapBuffers();
+
+ EXPECT_PIXEL_EQ(0, 0, 255, 0, 0, 255);
+ EXPECT_PIXEL_EQ(getWindowWidth()-1, 0, 0, 255, 0, 255);
+}
+
+TEST_F(GLSLTest, TwoElseIfRewriting)
+{
+ const std::string &vertexShaderSource =
+ "attribute vec4 a_position;\n"
+ "varying float v;\n"
+ "void main() {\n"
+ " gl_Position = a_position;\n"
+ " if (a_position.x == 0.0`) {\n"
+ " v = 1.0;\n"
+ " } else if (a_position.x > 0.5) {\n"
+ " v = 0.0;\n"
+ " } else if (a_position.x > 0.75) {\n"
+ " v = 0.5;\n"
+ " }\n"
+ "}\n";
+
+ const std::string &fragmentShaderSource =
+ "precision highp float;\n"
+ "varying float v;\n"
+ "void main() {\n"
+ " gl_FragColor = vec4(v, 0.0, 0.0, 1.0);\n"
+ "}\n";
+
+ GLuint program = compileProgram(vertexShaderSource, fragmentShaderSource);
+ EXPECT_NE(0u, program);
+}
