Commit f1d3c20c by Luc Ferron Committed by Commit Bot

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: 's avatarCorentin Wallez <cwallez@chromium.org>
parent ef2fda72
...@@ -214,7 +214,16 @@ void TOutputGLSLBase::writeLayoutQualifier(TIntermTyped *variable) ...@@ -214,7 +214,16 @@ void TOutputGLSLBase::writeLayoutQualifier(TIntermTyped *variable)
out << ") "; 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 && if (sh::IsGLSL410OrOlder(mOutput) && mShaderVersion >= 300 &&
(mCompileOptions & SH_REMOVE_INVARIANT_AND_CENTROID_FOR_ESSL3) != 0) (mCompileOptions & SH_REMOVE_INVARIANT_AND_CENTROID_FOR_ESSL3) != 0)
...@@ -265,11 +274,7 @@ void TOutputGLSLBase::writeVariableType(const TType &type, const TSymbol *symbol ...@@ -265,11 +274,7 @@ void TOutputGLSLBase::writeVariableType(const TType &type, const TSymbol *symbol
} }
if (qualifier != EvqTemporary && qualifier != EvqGlobal) if (qualifier != EvqTemporary && qualifier != EvqGlobal)
{ {
const char *qualifierString = mapQualifierToString(qualifier, symbol); writeQualifier(qualifier, symbol);
if (qualifierString && qualifierString[0] != '\0')
{
out << qualifierString << " ";
}
} }
const TMemoryQualifier &memoryQualifier = type.getMemoryQualifier(); const TMemoryQualifier &memoryQualifier = type.getMemoryQualifier();
......
...@@ -77,6 +77,7 @@ class TOutputGLSLBase : public TIntermTraverser ...@@ -77,6 +77,7 @@ class TOutputGLSLBase : public TIntermTraverser
virtual ImmutableString translateTextureFunction(const ImmutableString &name) { return name; } virtual ImmutableString translateTextureFunction(const ImmutableString &name) { return name; }
void declareStruct(const TStructure *structure); void declareStruct(const TStructure *structure);
virtual void writeQualifier(TQualifier qualifier, const TSymbol *symbol);
private: private:
bool structDeclared(const TStructure *structure) const; bool structDeclared(const TStructure *structure) const;
...@@ -86,7 +87,7 @@ class TOutputGLSLBase : public TIntermTraverser ...@@ -86,7 +87,7 @@ class TOutputGLSLBase : public TIntermTraverser
void writeBuiltInFunctionTriplet(Visit visit, TOperator op, bool useEmulatedFunction); void writeBuiltInFunctionTriplet(Visit visit, TOperator op, bool useEmulatedFunction);
const char *mapQualifierToString(TQualifier qualifier, const TSymbol *symbol); const char *mapQualifierToString(TQualifier qualifier);
TInfoSinkBase &mObjSink; TInfoSinkBase &mObjSink;
bool mDeclaringVariable; bool mDeclaringVariable;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "compiler/translator/OutputVulkanGLSL.h" #include "compiler/translator/OutputVulkanGLSL.h"
#include "compiler/translator/Symbol.h"
#include "compiler/translator/util.h" #include "compiler/translator/util.h"
namespace sh namespace sh
...@@ -54,7 +55,6 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -54,7 +55,6 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
TInfoSinkBase &out = objSink(); TInfoSinkBase &out = objSink();
const TLayoutQualifier &layoutQualifier = type.getLayoutQualifier(); const TLayoutQualifier &layoutQualifier = type.getLayoutQualifier();
out << "layout(";
// This isn't super clean, but it gets the job done. // This isn't super clean, but it gets the job done.
// See corresponding code in GlslangWrapper.cpp. // See corresponding code in GlslangWrapper.cpp.
...@@ -67,6 +67,10 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -67,6 +67,10 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
{ {
out << "@@ LAYOUT-" << symbol->getName() << " @@"; out << "@@ LAYOUT-" << symbol->getName() << " @@";
} }
else
{
out << "layout(";
}
if (IsImage(type.getBasicType()) && layoutQualifier.imageInternalFormat != EiifUnspecified) if (IsImage(type.getBasicType()) && layoutQualifier.imageInternalFormat != EiifUnspecified)
{ {
...@@ -74,7 +78,30 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -74,7 +78,30 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
out << getImageInternalFormatString(layoutQualifier.imageInternalFormat); 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) void TOutputVulkanGLSL::writeStructType(const TStructure *structure)
......
...@@ -31,6 +31,7 @@ class TOutputVulkanGLSL : public TOutputGLSL ...@@ -31,6 +31,7 @@ class TOutputVulkanGLSL : public TOutputGLSL
protected: protected:
void writeLayoutQualifier(TIntermTyped *variable) override; void writeLayoutQualifier(TIntermTyped *variable) override;
void writeQualifier(TQualifier qualifier, const TSymbol *symbol) override;
}; };
} // namespace sh } // namespace sh
...@@ -28,15 +28,37 @@ namespace rx ...@@ -28,15 +28,37 @@ namespace rx
namespace namespace
{ {
constexpr char kQualifierMarkerBegin[] = "@@ QUALIFIER-";
constexpr char kLayoutMarkerBegin[] = "@@ LAYOUT-";
constexpr char kMarkerEnd[] = " @@";
constexpr char kUniformQualifier[] = "uniform";
void InsertLayoutSpecifierString(std::string *shaderString, void InsertLayoutSpecifierString(std::string *shaderString,
const std::string &variableName, const std::string &variableName,
const std::string &layoutString) const std::string &layoutString)
{ {
std::stringstream searchStringBuilder; std::stringstream searchStringBuilder;
searchStringBuilder << "@@ LAYOUT-" << variableName << " @@"; searchStringBuilder << kLayoutMarkerBegin << variableName << kMarkerEnd;
std::string searchString = searchStringBuilder.str(); 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 } // anonymous namespace
...@@ -100,15 +122,19 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext, ...@@ -100,15 +122,19 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
// TODO(jmadill): Also do the same for ESSL 3 fragment outputs. // TODO(jmadill): Also do the same for ESSL 3 fragment outputs.
for (const auto &attribute : programState.getAttributes()) for (const auto &attribute : programState.getAttributes())
{ {
if (!attribute.staticUse) if (!attribute.active)
{
InsertQualifierSpecifierString(&vertexSource, attribute.name, "");
continue; continue;
}
std::string locationString = "location = " + Str(attribute.location); std::string locationString = "location = " + Str(attribute.location);
InsertLayoutSpecifierString(&vertexSource, attribute.name, locationString); InsertLayoutSpecifierString(&vertexSource, attribute.name, locationString);
InsertQualifierSpecifierString(&vertexSource, attribute.name, "in");
} }
// Assign varying locations. // Assign varying locations.
for (const auto &varyingReg : resources.varyingPacking.getRegisterList()) for (const gl::PackedVaryingRegister &varyingReg : resources.varyingPacking.getRegisterList())
{ {
const auto &varying = *varyingReg.packedVarying; const auto &varying = *varyingReg.packedVarying;
...@@ -116,6 +142,19 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext, ...@@ -116,6 +142,19 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
", component = " + Str(varyingReg.registerColumn); ", component = " + Str(varyingReg.registerColumn);
InsertLayoutSpecifierString(&vertexSource, varying.varying->name, locationString); InsertLayoutSpecifierString(&vertexSource, varying.varying->name, locationString);
InsertLayoutSpecifierString(&fragmentSource, 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. // Bind the default uniforms for vertex and fragment shaders.
...@@ -144,18 +183,27 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext, ...@@ -144,18 +183,27 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
if (samplerUniform.isActive(gl::ShaderType::Vertex)) if (samplerUniform.isActive(gl::ShaderType::Vertex))
{ {
InsertLayoutSpecifierString(&vertexSource, samplerUniform.name, setBindingString); InsertLayoutSpecifierString(&vertexSource, samplerUniform.name, setBindingString);
InsertQualifierSpecifierString(&vertexSource, samplerUniform.name, kUniformQualifier);
}
else
{
InsertQualifierSpecifierString(&vertexSource, samplerUniform.name, "");
} }
if (samplerUniform.isActive(gl::ShaderType::Fragment)) if (samplerUniform.isActive(gl::ShaderType::Fragment))
{ {
InsertLayoutSpecifierString(&fragmentSource, samplerUniform.name, setBindingString); InsertLayoutSpecifierString(&fragmentSource, samplerUniform.name, setBindingString);
InsertQualifierSpecifierString(&fragmentSource, samplerUniform.name, kUniformQualifier);
}
else
{
InsertQualifierSpecifierString(&fragmentSource, samplerUniform.name, "");
} }
textureCount += samplerUniform.getBasicTypeElementCount(); textureCount += samplerUniform.getBasicTypeElementCount();
} }
std::array<const char *, 2> strings = {{vertexSource.c_str(), fragmentSource.c_str()}}; std::array<const char *, 2> strings = {{vertexSource.c_str(), fragmentSource.c_str()}};
std::array<int, 2> lengths = { std::array<int, 2> lengths = {
{static_cast<int>(vertexSource.length()), static_cast<int>(fragmentSource.length())}}; {static_cast<int>(vertexSource.length()), static_cast<int>(fragmentSource.length())}};
......
...@@ -928,10 +928,6 @@ TEST_P(GLSLTest, InvariantAllBoth) ...@@ -928,10 +928,6 @@ TEST_P(GLSLTest, InvariantAllBoth)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnFloat) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"float f() { if (v_varying > 0.0) return 1.0; }\n" "float f() { if (v_varying > 0.0) return 1.0; }\n"
...@@ -944,10 +940,6 @@ TEST_P(GLSLTest, MissingReturnFloat) ...@@ -944,10 +940,6 @@ TEST_P(GLSLTest, MissingReturnFloat)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec2) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"vec2 f() { if (v_varying > 0.0) return vec2(1.0, 1.0); }\n" "vec2 f() { if (v_varying > 0.0) return vec2(1.0, 1.0); }\n"
...@@ -960,10 +952,6 @@ TEST_P(GLSLTest, MissingReturnVec2) ...@@ -960,10 +952,6 @@ TEST_P(GLSLTest, MissingReturnVec2)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec3) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"vec3 f() { if (v_varying > 0.0) return vec3(1.0, 1.0, 1.0); }\n" "vec3 f() { if (v_varying > 0.0) return vec3(1.0, 1.0, 1.0); }\n"
...@@ -976,10 +964,6 @@ TEST_P(GLSLTest, MissingReturnVec3) ...@@ -976,10 +964,6 @@ TEST_P(GLSLTest, MissingReturnVec3)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnVec4) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"vec4 f() { if (v_varying > 0.0) return vec4(1.0, 1.0, 1.0, 1.0); }\n" "vec4 f() { if (v_varying > 0.0) return vec4(1.0, 1.0, 1.0, 1.0); }\n"
...@@ -992,10 +976,6 @@ TEST_P(GLSLTest, MissingReturnVec4) ...@@ -992,10 +976,6 @@ TEST_P(GLSLTest, MissingReturnVec4)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnIVec4) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"ivec4 f() { if (v_varying > 0.0) return ivec4(1, 1, 1, 1); }\n" "ivec4 f() { if (v_varying > 0.0) return ivec4(1, 1, 1, 1); }\n"
...@@ -1008,10 +988,6 @@ TEST_P(GLSLTest, MissingReturnIVec4) ...@@ -1008,10 +988,6 @@ TEST_P(GLSLTest, MissingReturnIVec4)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnMat4) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"mat4 f() { if (v_varying > 0.0) return mat4(1.0); }\n" "mat4 f() { if (v_varying > 0.0) return mat4(1.0); }\n"
...@@ -1024,10 +1000,6 @@ TEST_P(GLSLTest, MissingReturnMat4) ...@@ -1024,10 +1000,6 @@ TEST_P(GLSLTest, MissingReturnMat4)
// Verify that functions without return statements still compile // Verify that functions without return statements still compile
TEST_P(GLSLTest, MissingReturnStruct) 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 = const std::string vertexShaderSource =
"varying float v_varying;\n" "varying float v_varying;\n"
"struct s { float a; int b; vec2 c; };\n" "struct s { float a; int b; vec2 c; };\n"
......
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