Vulkan: Fix the issue with unused attributes / varyings
When an attribute, a uniform or a varying isn't used, we now remove
their layout and in/out qualifiers so that the shader can still refer
to these var names.
Bug: angleproject:2456
Change-Id: I5f1241d91bd46f663750adfab2562ef87ce69ae5
Reviewed-on: https://chromium-review.googlesource.com/1014009
Commit-Queue: Luc Ferron <lucferron@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index a7534b0..fd1958c 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -214,7 +214,16 @@
out << ") ";
}
-const char *TOutputGLSLBase::mapQualifierToString(TQualifier qualifier, const TSymbol *symbol)
+void TOutputGLSLBase::writeQualifier(TQualifier qualifier, const TSymbol *symbol)
+{
+ const char *result = mapQualifierToString(qualifier);
+ if (result && result[0] != '\0')
+ {
+ objSink() << result << " ";
+ }
+}
+
+const char *TOutputGLSLBase::mapQualifierToString(TQualifier qualifier)
{
if (sh::IsGLSL410OrOlder(mOutput) && mShaderVersion >= 300 &&
(mCompileOptions & SH_REMOVE_INVARIANT_AND_CENTROID_FOR_ESSL3) != 0)
@@ -265,11 +274,7 @@
}
if (qualifier != EvqTemporary && qualifier != EvqGlobal)
{
- const char *qualifierString = mapQualifierToString(qualifier, symbol);
- if (qualifierString && qualifierString[0] != '\0')
- {
- out << qualifierString << " ";
- }
+ writeQualifier(qualifier, symbol);
}
const TMemoryQualifier &memoryQualifier = type.getMemoryQualifier();
diff --git a/src/compiler/translator/OutputGLSLBase.h b/src/compiler/translator/OutputGLSLBase.h
index c000b95..272d276 100644
--- a/src/compiler/translator/OutputGLSLBase.h
+++ b/src/compiler/translator/OutputGLSLBase.h
@@ -77,6 +77,7 @@
virtual ImmutableString translateTextureFunction(const ImmutableString &name) { return name; }
void declareStruct(const TStructure *structure);
+ virtual void writeQualifier(TQualifier qualifier, const TSymbol *symbol);
private:
bool structDeclared(const TStructure *structure) const;
@@ -86,7 +87,7 @@
void writeBuiltInFunctionTriplet(Visit visit, TOperator op, bool useEmulatedFunction);
- const char *mapQualifierToString(TQualifier qualifier, const TSymbol *symbol);
+ const char *mapQualifierToString(TQualifier qualifier);
TInfoSinkBase &mObjSink;
bool mDeclaringVariable;
diff --git a/src/compiler/translator/OutputVulkanGLSL.cpp b/src/compiler/translator/OutputVulkanGLSL.cpp
index bd7c4e3..c4b44b1 100644
--- a/src/compiler/translator/OutputVulkanGLSL.cpp
+++ b/src/compiler/translator/OutputVulkanGLSL.cpp
@@ -11,6 +11,7 @@
#include "compiler/translator/OutputVulkanGLSL.h"
+#include "compiler/translator/Symbol.h"
#include "compiler/translator/util.h"
namespace sh
@@ -54,7 +55,6 @@
TInfoSinkBase &out = objSink();
const TLayoutQualifier &layoutQualifier = type.getLayoutQualifier();
- out << "layout(";
// This isn't super clean, but it gets the job done.
// See corresponding code in GlslangWrapper.cpp.
@@ -67,6 +67,10 @@
{
out << "@@ LAYOUT-" << symbol->getName() << " @@";
}
+ else
+ {
+ out << "layout(";
+ }
if (IsImage(type.getBasicType()) && layoutQualifier.imageInternalFormat != EiifUnspecified)
{
@@ -74,7 +78,30 @@
out << getImageInternalFormatString(layoutQualifier.imageInternalFormat);
}
- out << ") ";
+ if (!needsCustomLayout)
+ {
+ out << ") ";
+ }
+}
+
+void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier, const TSymbol *symbol)
+{
+ if (qualifier != EvqUniform && qualifier != EvqVaryingIn && qualifier != EvqVaryingOut &&
+ qualifier != EvqAttribute)
+ {
+ TOutputGLSLBase::writeQualifier(qualifier, symbol);
+ return;
+ }
+
+ if (symbol == nullptr)
+ {
+ return;
+ }
+
+ TInfoSinkBase &out = objSink();
+ out << "@@ QUALIFIER-";
+ out << symbol->name().data();
+ out << " @@ ";
}
void TOutputVulkanGLSL::writeStructType(const TStructure *structure)
diff --git a/src/compiler/translator/OutputVulkanGLSL.h b/src/compiler/translator/OutputVulkanGLSL.h
index 80561e5..d98517b 100644
--- a/src/compiler/translator/OutputVulkanGLSL.h
+++ b/src/compiler/translator/OutputVulkanGLSL.h
@@ -31,6 +31,7 @@
protected:
void writeLayoutQualifier(TIntermTyped *variable) override;
+ void writeQualifier(TQualifier qualifier, const TSymbol *symbol) override;
};
} // namespace sh
diff --git a/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp b/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp
index 9f05255..ffb9662 100644
--- a/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp
+++ b/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp
@@ -28,15 +28,37 @@
namespace
{
+constexpr char kQualifierMarkerBegin[] = "@@ QUALIFIER-";
+constexpr char kLayoutMarkerBegin[] = "@@ LAYOUT-";
+constexpr char kMarkerEnd[] = " @@";
+constexpr char kUniformQualifier[] = "uniform";
+
void InsertLayoutSpecifierString(std::string *shaderString,
const std::string &variableName,
const std::string &layoutString)
{
std::stringstream searchStringBuilder;
- searchStringBuilder << "@@ LAYOUT-" << variableName << " @@";
+ searchStringBuilder << kLayoutMarkerBegin << variableName << kMarkerEnd;
std::string searchString = searchStringBuilder.str();
- angle::ReplaceSubstring(shaderString, searchString, layoutString);
+ if (layoutString != "")
+ {
+ angle::ReplaceSubstring(shaderString, searchString, "layout(" + layoutString + ")");
+ }
+ else
+ {
+ angle::ReplaceSubstring(shaderString, searchString, layoutString);
+ }
+}
+
+void InsertQualifierSpecifierString(std::string *shaderString,
+ const std::string &variableName,
+ const std::string &replacementString)
+{
+ std::stringstream searchStringBuilder;
+ searchStringBuilder << kQualifierMarkerBegin << variableName << kMarkerEnd;
+ std::string searchString = searchStringBuilder.str();
+ angle::ReplaceSubstring(shaderString, searchString, replacementString);
}
} // anonymous namespace
@@ -100,15 +122,19 @@
// TODO(jmadill): Also do the same for ESSL 3 fragment outputs.
for (const auto &attribute : programState.getAttributes())
{
- if (!attribute.staticUse)
+ if (!attribute.active)
+ {
+ InsertQualifierSpecifierString(&vertexSource, attribute.name, "");
continue;
+ }
std::string locationString = "location = " + Str(attribute.location);
InsertLayoutSpecifierString(&vertexSource, attribute.name, locationString);
+ InsertQualifierSpecifierString(&vertexSource, attribute.name, "in");
}
// Assign varying locations.
- for (const auto &varyingReg : resources.varyingPacking.getRegisterList())
+ for (const gl::PackedVaryingRegister &varyingReg : resources.varyingPacking.getRegisterList())
{
const auto &varying = *varyingReg.packedVarying;
@@ -116,6 +142,19 @@
", component = " + Str(varyingReg.registerColumn);
InsertLayoutSpecifierString(&vertexSource, varying.varying->name, locationString);
InsertLayoutSpecifierString(&fragmentSource, varying.varying->name, locationString);
+
+ ASSERT(varying.interpolation == sh::INTERPOLATION_SMOOTH);
+ InsertQualifierSpecifierString(&vertexSource, varying.varying->name, "out");
+ InsertQualifierSpecifierString(&fragmentSource, varying.varying->name, "in");
+ }
+
+ // Remove all the markers for unused varyings.
+ for (const std::string &varyingName : resources.varyingPacking.getInactiveVaryingNames())
+ {
+ InsertLayoutSpecifierString(&vertexSource, varyingName, "");
+ InsertLayoutSpecifierString(&fragmentSource, varyingName, "");
+ InsertQualifierSpecifierString(&vertexSource, varyingName, "");
+ InsertQualifierSpecifierString(&fragmentSource, varyingName, "");
}
// Bind the default uniforms for vertex and fragment shaders.
@@ -144,18 +183,27 @@
if (samplerUniform.isActive(gl::ShaderType::Vertex))
{
InsertLayoutSpecifierString(&vertexSource, samplerUniform.name, setBindingString);
+ InsertQualifierSpecifierString(&vertexSource, samplerUniform.name, kUniformQualifier);
+ }
+ else
+ {
+ InsertQualifierSpecifierString(&vertexSource, samplerUniform.name, "");
}
if (samplerUniform.isActive(gl::ShaderType::Fragment))
{
InsertLayoutSpecifierString(&fragmentSource, samplerUniform.name, setBindingString);
+ InsertQualifierSpecifierString(&fragmentSource, samplerUniform.name, kUniformQualifier);
+ }
+ else
+ {
+ InsertQualifierSpecifierString(&fragmentSource, samplerUniform.name, "");
}
textureCount += samplerUniform.getBasicTypeElementCount();
}
std::array<const char *, 2> strings = {{vertexSource.c_str(), fragmentSource.c_str()}};
-
std::array<int, 2> lengths = {
{static_cast<int>(vertexSource.length()), static_cast<int>(fragmentSource.length())}};
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 792e252..68ede33 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -928,10 +928,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnFloat)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"float f() { if (v_varying > 0.0) return 1.0; }\n"
@@ -944,10 +940,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec2)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"vec2 f() { if (v_varying > 0.0) return vec2(1.0, 1.0); }\n"
@@ -960,10 +952,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec3)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"vec3 f() { if (v_varying > 0.0) return vec3(1.0, 1.0, 1.0); }\n"
@@ -976,10 +964,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec4)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"vec4 f() { if (v_varying > 0.0) return vec4(1.0, 1.0, 1.0, 1.0); }\n"
@@ -992,10 +976,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnIVec4)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"ivec4 f() { if (v_varying > 0.0) return ivec4(1, 1, 1, 1); }\n"
@@ -1008,10 +988,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnMat4)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"mat4 f() { if (v_varying > 0.0) return mat4(1.0); }\n"
@@ -1024,10 +1000,6 @@
// Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnStruct)
{
- // TODO(lucferron): Varyings that aren't used as an output in the FS are not yet supported.
- // http://anglebug.com/2460
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string vertexShaderSource =
"varying float v_varying;\n"
"struct s { float a; int b; vec2 c; };\n"