Commit 905fbdea by Olli Etuaho Committed by Jamie Madill

Fix splitting nested sequence operators

Make sure that only one sequence operator is split on one iteration of SplitSequenceOperator. This prevents multiple successive PostVisit calls to nested sequence operator nodes from adding duplicate nodes to the AST. The sequence operators are split starting from the outermost one to preserve execution order. Note that the shader translator somewhat unexpectedly generates nested sequence operators in the AST when there is a sequence operator with more than two operands, so this bug ended up affecting shaders in the wild. The code around parsing sequence operators could be clarified separately. BUG=638313 TEST=angle_end2end_tests Change-Id: Ic6400a484ceff0c790c2290f7b4b80980f87cd88 Reviewed-on: https://chromium-review.googlesource.com/376678Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com> Reviewed-on: https://chromium-review.googlesource.com/387173
parent ca05a081
...@@ -82,7 +82,9 @@ bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregat ...@@ -82,7 +82,9 @@ bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregat
} }
else if (visit == PostVisit) else if (visit == PostVisit)
{ {
if (mFoundExpressionToSplit) // Split sequence operators starting from the outermost one to preserve correct
// execution order.
if (mFoundExpressionToSplit && mInsideSequenceOperator == 1)
{ {
// Move all operands of the sequence operation except the last one into separate // Move all operands of the sequence operation except the last one into separate
// statements in the parent block. // statements in the parent block.
......
...@@ -2181,6 +2181,35 @@ TEST_P(GLSLTest, NestedPowStatements) ...@@ -2181,6 +2181,35 @@ TEST_P(GLSLTest, NestedPowStatements)
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green); EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
} }
// Test a nested sequence operator with a ternary operator inside. The ternary operator is
// intended to be such that it gets converted to an if statement on the HLSL backend.
TEST_P(GLSLTest, NestedSequenceOperatorWithTernaryInside)
{
const std::string &vert =
"attribute vec2 position;\n"
"void main()\n"
"{\n"
" gl_Position = vec4(position, 0, 1);\n"
"}";
// Note that the uniform keep_flop_positive doesn't need to be set - the test expects it to have
// its default value false.
const std::string &frag =
"precision mediump float;\n"
"uniform bool keep_flop_positive;\n"
"float flop;\n"
"void main() {\n"
" flop = -1.0,\n"
" (flop *= -1.0,\n"
" keep_flop_positive ? 0.0 : flop *= -1.0),\n"
" gl_FragColor = vec4(0, -flop, 0, 1);\n"
"}";
ANGLE_GL_PROGRAM(prog, vert, frag);
drawQuad(prog.get(), "position", 0.5f);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
}
} // anonymous namespace } // anonymous namespace
// 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.
......
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