Take all attributes into account when checking for aliasing
This makes ANGLE to follow GLSL ES 3.00.6 spec section 12.46. The spec
requires that all attributes are taken into account when checking for
aliasing, regardless of if they are active or not. WebGL 2.0 spec was
also recently changed to reflect GLSL ES 3.00.6 correctly. Aliasing
checks for GLSL ES 1.00 shaders are left as-is.
BUG=chromium:829541
TEST=angle_end2end_tests, WebGL conformance tests
Change-Id: I71c36ac123f18dadf075e81f93af29321c15136b
Reviewed-on: https://chromium-review.googlesource.com/1005077
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index b1080d8..df8fa6d 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -2662,8 +2662,20 @@
const ContextState &data = context->getContextState();
Shader *vertexShader = mState.getAttachedShader(ShaderType::Vertex);
+ int shaderVersion = vertexShader->getShaderVersion(context);
+
unsigned int usedLocations = 0;
- mState.mAttributes = vertexShader->getActiveAttributes(context);
+ if (shaderVersion >= 300)
+ {
+ // In GLSL ES 3.00.6, aliasing checks should be done with all declared attributes - see GLSL
+ // ES 3.00.6 section 12.46. Inactive attributes will be pruned after aliasing checks.
+ mState.mAttributes = vertexShader->getAllAttributes(context);
+ }
+ else
+ {
+ // In GLSL ES 1.00.17 we only do aliasing checks for active attributes.
+ mState.mAttributes = vertexShader->getActiveAttributes(context);
+ }
GLuint maxAttribs = data.getCaps().maxVertexAttributes;
// TODO(jmadill): handle aliasing robustly
@@ -2675,7 +2687,7 @@
std::vector<sh::Attribute *> usedAttribMap(maxAttribs, nullptr);
- // Link attributes that have a binding location
+ // Assign locations to attributes that have a binding location and check for attribute aliasing.
for (sh::Attribute &attribute : mState.mAttributes)
{
// GLSL ES 3.10 January 2016 section 4.3.4: Vertex shader inputs can't be arrays or
@@ -2696,8 +2708,8 @@
if (static_cast<GLuint>(regs + attribute.location) > maxAttribs)
{
- infoLog << "Active attribute (" << attribute.name << ") at location "
- << attribute.location << " is too big to fit";
+ infoLog << "Attribute (" << attribute.name << ") at location " << attribute.location
+ << " is too big to fit";
return false;
}
@@ -2707,12 +2719,13 @@
const int regLocation = attribute.location + reg;
sh::ShaderVariable *linkedAttribute = usedAttribMap[regLocation];
- // In GLSL 3.00, attribute aliasing produces a link error
- // In GLSL 1.00, attribute aliasing is allowed, but ANGLE currently has a bug
+ // In GLSL ES 3.00.6 and in WebGL, attribute aliasing produces a link error.
+ // In non-WebGL GLSL ES 1.00.17, attribute aliasing is allowed with some
+ // restrictions - see GLSL ES 1.00.17 section 2.10.4, but ANGLE currently has a bug.
if (linkedAttribute)
{
// TODO(jmadill): fix aliasing on ES2
- // if (mProgram->getShaderVersion() >= 300)
+ // if (shaderVersion >= 300 && !webgl)
{
infoLog << "Attribute '" << attribute.name << "' aliases attribute '"
<< linkedAttribute->name << "' at location " << regLocation;
@@ -2729,7 +2742,7 @@
}
}
- // Link attributes that don't have a binding location
+ // Assign locations to attributes that don't have a binding location.
for (sh::Attribute &attribute : mState.mAttributes)
{
// Not set by glBindAttribLocation or by location layout qualifier
@@ -2740,7 +2753,7 @@
if (availableIndex == -1 || static_cast<GLuint>(availableIndex + regs) > maxAttribs)
{
- infoLog << "Too many active attributes (" << attribute.name << ")";
+ infoLog << "Too many attributes (" << attribute.name << ")";
return false;
}
@@ -2751,8 +2764,27 @@
ASSERT(mState.mAttributesTypeMask.none());
ASSERT(mState.mAttributesMask.none());
+ // Prune inactive attributes. This step is only needed on shaderVersion >= 300 since on earlier
+ // shader versions we're only processing active attributes to begin with.
+ if (shaderVersion >= 300)
+ {
+ for (auto attributeIter = mState.mAttributes.begin();
+ attributeIter != mState.mAttributes.end();)
+ {
+ if (attributeIter->active)
+ {
+ ++attributeIter;
+ }
+ else
+ {
+ attributeIter = mState.mAttributes.erase(attributeIter);
+ }
+ }
+ }
+
for (const sh::Attribute &attribute : mState.mAttributes)
{
+ ASSERT(attribute.active);
ASSERT(attribute.location != -1);
unsigned int regs = static_cast<unsigned int>(VariableRegisterCount(attribute.type));
diff --git a/src/libANGLE/Shader.cpp b/src/libANGLE/Shader.cpp
index 3fc9ff8..a43b147 100644
--- a/src/libANGLE/Shader.cpp
+++ b/src/libANGLE/Shader.cpp
@@ -403,8 +403,8 @@
{
{
mState.mOutputVaryings = GetShaderVariables(sh::GetOutputVaryings(compilerHandle));
- mState.mActiveAttributes =
- GetActiveShaderVariables(sh::GetAttributes(compilerHandle));
+ mState.mAllAttributes = GetShaderVariables(sh::GetAttributes(compilerHandle));
+ mState.mActiveAttributes = GetActiveShaderVariables(&mState.mAllAttributes);
mState.mNumViews = sh::GetVertexShaderNumViews(compilerHandle);
}
break;
@@ -529,6 +529,12 @@
return mState.getActiveAttributes();
}
+const std::vector<sh::Attribute> &Shader::getAllAttributes(const Context *context)
+{
+ resolveCompile(context);
+ return mState.getAllAttributes();
+}
+
const std::vector<sh::OutputVariable> &Shader::getActiveOutputVariables(const Context *context)
{
resolveCompile(context);
diff --git a/src/libANGLE/Shader.h b/src/libANGLE/Shader.h
index 1d0b021..a3b5f24 100644
--- a/src/libANGLE/Shader.h
+++ b/src/libANGLE/Shader.h
@@ -71,6 +71,7 @@
return mShaderStorageBlocks;
}
const std::vector<sh::Attribute> &getActiveAttributes() const { return mActiveAttributes; }
+ const std::vector<sh::Attribute> &getAllAttributes() const { return mAllAttributes; }
const std::vector<sh::OutputVariable> &getActiveOutputVariables() const
{
return mActiveOutputVariables;
@@ -95,6 +96,7 @@
std::vector<sh::Uniform> mUniforms;
std::vector<sh::InterfaceBlock> mUniformBlocks;
std::vector<sh::InterfaceBlock> mShaderStorageBlocks;
+ std::vector<sh::Attribute> mAllAttributes;
std::vector<sh::Attribute> mActiveAttributes;
std::vector<sh::OutputVariable> mActiveOutputVariables;
@@ -165,6 +167,7 @@
const std::vector<sh::InterfaceBlock> &getUniformBlocks(const Context *context);
const std::vector<sh::InterfaceBlock> &getShaderStorageBlocks(const Context *context);
const std::vector<sh::Attribute> &getActiveAttributes(const Context *context);
+ const std::vector<sh::Attribute> &getAllAttributes(const Context *context);
const std::vector<sh::OutputVariable> &getActiveOutputVariables(const Context *context);
// Returns mapped name of a transform feedback varying. The original name may contain array
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 49e803c..7fce607 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp
@@ -1471,6 +1471,39 @@
}
}
+// Test that even inactive attributes are taken into account when checking for aliasing in case the
+// shader version is >= 3.00. GLSL ES 3.00.6 section 12.46.
+TEST_P(VertexAttributeTestES3, InactiveAttributeAliasing)
+{
+ const std::string &vertexShader =
+ R"(#version 300 es
+ precision mediump float;
+ in vec4 input_active;
+ in vec4 input_unused;
+ void main()
+ {
+ gl_Position = input_active;
+ })";
+
+ const std::string &fragmentShader =
+ R"(#version 300 es
+ precision mediump float;
+ out vec4 color;
+ void main()
+ {
+ color = vec4(0.0);
+ })";
+
+ ANGLE_GL_PROGRAM(program, vertexShader, fragmentShader);
+ glBindAttribLocation(program, 0, "input_active");
+ glBindAttribLocation(program, 0, "input_unused");
+ glLinkProgram(program);
+ EXPECT_GL_NO_ERROR();
+ GLint linkStatus = 0;
+ glGetProgramiv(program, GL_LINK_STATUS, &linkStatus);
+ EXPECT_GL_FALSE(linkStatus);
+}
+
// Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against.
// D3D11 Feature Level 9_3 uses different D3D formats for vertex attribs compared to Feature Levels