Commit 852fe873 by Olli Etuaho Committed by Commit Bot

Fix HLSL for switch statements that don't end in a branch

In case the last case inside a switch statement is not terminated in a branch statement, RemoveSwitchFallThrough needs to add it before calling handlePreviousCase. This ensures that all preceding fall-through cases will get a copy of the branch statement and so will not fall through. This also fixes running RemoveSwitchFallThrough so that it's only executed once per each switch statement. The error was not caught by the dEQP tests, so a new ANGLE test is added. BUG=angleproject:2178 TEST=angle_end2end_tests Change-Id: I26b6989aa4d32de2d74cde56d72ee24f61195445 Reviewed-on: https://chromium-review.googlesource.com/709196Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 211bff3f
...@@ -2137,9 +2137,12 @@ bool OutputHLSL::visitSwitch(Visit visit, TIntermSwitch *node) ...@@ -2137,9 +2137,12 @@ bool OutputHLSL::visitSwitch(Visit visit, TIntermSwitch *node)
if (node->getStatementList()) if (node->getStatementList())
{ {
node->setStatementList(RemoveSwitchFallThrough(node->getStatementList())); if (visit == PreVisit)
{
node->setStatementList(RemoveSwitchFallThrough(node->getStatementList()));
}
outputTriplet(out, visit, "switch (", ") ", ""); outputTriplet(out, visit, "switch (", ") ", "");
// The curly braces get written when visiting the statementList aggregate // The curly braces get written when visiting the statementList block.
} }
else else
{ {
......
...@@ -56,14 +56,16 @@ TIntermBlock *RemoveSwitchFallThroughTraverser::removeFallThrough(TIntermBlock * ...@@ -56,14 +56,16 @@ TIntermBlock *RemoveSwitchFallThroughTraverser::removeFallThrough(TIntermBlock *
RemoveSwitchFallThroughTraverser rm(statementList); RemoveSwitchFallThroughTraverser rm(statementList);
ASSERT(statementList); ASSERT(statementList);
statementList->traverse(&rm); statementList->traverse(&rm);
bool lastStatementWasBreak = rm.mLastStatementWasBreak; ASSERT(rm.mPreviousCase || statementList->getSequence()->empty());
rm.mLastStatementWasBreak = true; if (!rm.mLastStatementWasBreak && rm.mPreviousCase)
rm.handlePreviousCase();
if (!lastStatementWasBreak)
{ {
// Make sure that there's a branch at the end of the final case inside the switch statement.
// This also ensures that any cases that fall through to the final case will get the break.
TIntermBranch *finalBreak = new TIntermBranch(EOpBreak, nullptr); TIntermBranch *finalBreak = new TIntermBranch(EOpBreak, nullptr);
rm.mStatementListOut->getSequence()->push_back(finalBreak); rm.mPreviousCase->getSequence()->push_back(finalBreak);
rm.mLastStatementWasBreak = true;
} }
rm.handlePreviousCase();
return rm.mStatementListOut; return rm.mStatementListOut;
} }
......
...@@ -3499,6 +3499,45 @@ TEST_P(GLSLTest_ES3, DifferentStatementsInsideSwitch) ...@@ -3499,6 +3499,45 @@ TEST_P(GLSLTest_ES3, DifferentStatementsInsideSwitch)
ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader); ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
} }
// Test that switch fall-through works correctly.
// This is a regression test for http://anglebug.com/2178
TEST_P(GLSLTest_ES3, SwitchFallThroughCodeDuplication)
{
const std::string &fragmentShader =
R"(#version 300 es
precision highp float;
out vec4 my_FragColor;
uniform int u_zero;
void main()
{
int i = 0;
// switch should fall through both cases.
switch(u_zero)
{
case 0:
i += 1;
case 1:
i += 2;
}
if (i == 3)
{
my_FragColor = vec4(0, 1, 0, 1);
}
else
{
my_FragColor = vec4(1, 0, 0, 1);
}
})";
ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
drawQuad(program.get(), "inputAttribute", 0.5f);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against. // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
ANGLE_INSTANTIATE_TEST(GLSLTest, ANGLE_INSTANTIATE_TEST(GLSLTest,
ES2_D3D9(), ES2_D3D9(),
......
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