Skip to content

Commit edb0562

Browse files
committed
Fix sort uniform bug
Current sort doesn't take care of the case when both uniforms are struct specfiers. Given that struct types are easily broken by sort, make the sort not reordering uniforms if both are structs. Bug: b/437825940 Change-Id: Idda1810ac4234f7e1547735e4e09658ab0a57eed Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6842936 Reviewed-by: Charlie Lao <[email protected]> Reviewed-by: Amirali Abdolrashidi <[email protected]> Reviewed-by: Shahbaz Youssefi <[email protected]>
1 parent 60e3f72 commit edb0562

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

src/compiler/translator/Compiler.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,12 +236,16 @@ struct UniformSortComparator
236236
->getAsSymbolNode()
237237
->variable()
238238
.getType();
239+
// If both uniforms are structs, do not reorder them
240+
if (firstType.getStruct() != nullptr && secondType.getStruct() != nullptr)
241+
{
242+
return false;
243+
}
239244
// First, sort by precision: lowp and mediump are smaller than highp
240245
if (firstType.getPrecision() != secondType.getPrecision())
241246
{
242247
return firstType.getPrecision() != TPrecision::EbpHigh;
243248
}
244-
245249
// We don't sort highp uniforms. If both uniforms are highp, consider them as equivalent
246250
if (firstType.getPrecision() == TPrecision::EbpHigh &&
247251
secondType.getPrecision() == TPrecision::EbpHigh)
@@ -256,11 +260,6 @@ struct UniformSortComparator
256260
{
257261
return firstType.getStruct() == nullptr;
258262
}
259-
// If both are struct, place the one that has specifier in the front
260-
if (firstType.getStruct() != nullptr && secondType.getStruct() != nullptr)
261-
{
262-
return firstType.isStructSpecifier();
263-
}
264263
// criteria 2: sort by arrayness. Non-array element is smaller.
265264
if (firstType.isArray() != secondType.isArray())
266265
{

src/tests/gl_tests/UniformTest.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,6 +2485,22 @@ TEST_P(UniformTestES31, UniformReorderDoesNotBreakStructUniforms)
24852485
ASSERT_NE(program, 0u);
24862486
}
24872487

2488+
// That that TCompiler::sortUniforms() does not break the shader code when there are multiple
2489+
// uniforms of the struct data type, and both of them are struct specifiers, and one struct
2490+
// references the other struct.
2491+
TEST_P(UniformTestES31, UniformReorderDoesNotBreakStructUniformsV2)
2492+
{
2493+
constexpr char kFS[] =
2494+
"#version 310 es\n"
2495+
"precision mediump float;\n"
2496+
"uniform struct S1 { samplerCube ar; } a1;\n"
2497+
"uniform struct S2 { S1 s; } a2;\n"
2498+
"void main (void)\n"
2499+
"{}";
2500+
GLuint program = CompileProgram(essl31_shaders::vs::Simple(), kFS);
2501+
ASSERT_NE(program, 0u);
2502+
}
2503+
24882504
// Test a uniform struct containing a non-square matrix and a boolean.
24892505
// Minimal test case for a bug revealed by dEQP tests.
24902506
TEST_P(UniformTestES3, StructWithNonSquareMatrixAndBool)

0 commit comments

Comments
 (0)