Vulkan: Set varying location & xfb decorations in SPIR-V

The shader translator outputs arbitrary location indices.  Once compiled
by glslang, the SPIR-V transformer modifies these decorations.  If the
transform feedback extension is used, it will also add the relevant
decorations to the varyings that are captured.

Bug: angleproject:3394
Change-Id: I5ecafd0536408612a5d4b920dbabbfabe650657c
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2008468
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

include/GLSLANG/ShaderLang.h

@@ -765,6 +765,9 @@ extern const char kDriverUniformsVarName[];
 // Interface block array variable name used for atomic counter emulation
 extern const char kAtomicCountersVarName[];
 
+// Line raster emulation varying
+extern const char kLineRasterEmulationPosition[];
+
 }  // namespace vk
 }  // namespace sh
 

src/compiler/translator/OutputGLSLBase.h

@@ -88,7 +88,7 @@ class TOutputGLSLBase : public TIntermTraverser
     }
 
     void declareStruct(const TStructure *structure);
-    virtual void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol);
+    void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol);
     bool structDeclared(const TStructure *structure) const;
 
     const char *mapQualifierToString(TQualifier qualifier);

src/compiler/translator/OutputVulkanGLSL.cpp

@@ -46,15 +46,13 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
 {
     const TType &type = variable->getType();
 
-    bool needsCustomLayout = IsVarying(type.getQualifier());
     bool needsSetBinding =
         IsSampler(type.getBasicType()) || type.isInterfaceBlock() || IsImage(type.getBasicType());
     bool needsLocation = type.getQualifier() == EvqAttribute ||
                          type.getQualifier() == EvqVertexIn ||
-                         type.getQualifier() == EvqFragmentOut;
+                         type.getQualifier() == EvqFragmentOut || IsVarying(type.getQualifier());
 
-    if (!NeedsToWriteLayoutQualifier(type) && !needsCustomLayout && !needsSetBinding &&
+    if (!NeedsToWriteLayoutQualifier(type) && !needsSetBinding && !needsLocation)
-        !needsLocation)
     {
         return;
     }
@@ -103,33 +101,25 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
     }
 
     const char *separator = "";
-    if (needsCustomLayout)
+    out << "layout(";
+
+    // If the resource declaration requires set & binding layout qualifiers, specify arbitrary
+    // ones.
+    if (needsSetBinding)
     {
-        out << "@@ LAYOUT-" << name << "(";
+        out << "set=0, binding=" << nextUnusedBinding();
+        separator = ", ";
     }
-    else
+
+    if (needsLocation)
     {
-        out << "layout(";
+        const unsigned int locationCount = CalculateVaryingLocationCount(symbol, getShaderType());
+        uint32_t location                = IsShaderIn(type.getQualifier())
+                                ? nextUnusedInputLocation(locationCount)
+                                : nextUnusedOutputLocation(locationCount);
 
-        // If the resource declaration requires set & binding layout qualifiers, specify arbitrary
+        out << "location=" << location;
-        // ones.
+        separator = ", ";
-        if (needsSetBinding)
-        {
-            out << "set=0, binding=" << nextUnusedBinding();
-            separator = ", ";
-        }
-
-        if (needsLocation)
-        {
-            const unsigned int locationCount =
-                CalculateVaryingLocationCount(symbol, getShaderType());
-            uint32_t location = IsShaderIn(type.getQualifier())
-                                    ? nextUnusedInputLocation(locationCount)
-                                    : nextUnusedOutputLocation(locationCount);
-
-            out << "location=" << location;
-            separator = ", ";
-        }
     }
 
     // Output the list of qualifiers already known at this stage, i.e. everything other than
@@ -152,39 +142,6 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
     }
 
     out << ") ";
-    if (needsCustomLayout)
-    {
-        out << "@@";
-    }
-}
-
-void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier,
-                                       const TType &type,
-                                       const TSymbol *symbol)
-{
-    // Only varyings need to output qualifiers through a @@ QUALIFIER macro.  Glslang wrapper may
-    // decide to remove them if they are inactive and turn them into global variables.  This is only
-    // necessary for varyings because they are the only shader interface variables that could be
-    // referenced in the shader source and still be inactive.
-    if (!sh::IsVarying(qualifier))
-    {
-        TOutputGLSLBase::writeQualifier(qualifier, type, symbol);
-        return;
-    }
-
-    if (symbol == nullptr)
-    {
-        return;
-    }
-
-    ImmutableString name = symbol->name();
-
-    // The in/out qualifiers are calculated here so glslang wrapper doesn't need to guess them.
-    ASSERT(IsShaderIn(qualifier) || IsShaderOut(qualifier));
-    const char *inOutQualifier = mapQualifierToString(qualifier);
-
-    TInfoSinkBase &out = objSink();
-    out << "@@ QUALIFIER-" << name.data() << "(" << inOutQualifier << ") @@ ";
 }
 
 void TOutputVulkanGLSL::writeVariableType(const TType &type,

src/compiler/translator/OutputVulkanGLSL.h

@@ -45,7 +45,6 @@ class TOutputVulkanGLSL : public TOutputGLSL
 
   protected:
     void writeLayoutQualifier(TIntermTyped *variable) override;
-    void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol) override;
     void writeVariableType(const TType &type,
                            const TSymbol *symbol,
                            bool isFunctionArgument) override;

src/compiler/translator/ShaderLang.cpp

@@ -720,6 +720,8 @@ const char kDriverUniformsVarName[]   = "ANGLEUniforms";
 // Interface block array variable name used for atomic counter emulation
 const char kAtomicCountersVarName[] = "atomicCounters";
 
+const char kLineRasterEmulationPosition[] = "ANGLEPosition";
+
 }  // namespace vk
 
 }  // namespace sh

src/compiler/translator/TranslatorVulkan.cpp

@@ -443,10 +443,11 @@ TVariable *AddANGLEPositionVaryingDeclaration(TIntermBlock *root,
                                               TSymbolTable *symbolTable,
                                               TQualifier qualifier)
 {
-    // Define a driver varying vec2 "ANGLEPosition".
+    // Define a vec2 driver varying to hold the line rasterization emulation position.
-    TType *varyingType               = new TType(EbtFloat, EbpMedium, qualifier, 2);
+    TType *varyingType = new TType(EbtFloat, EbpMedium, qualifier, 2);
-    TVariable *varyingVar            = new TVariable(symbolTable, ImmutableString("ANGLEPosition"),
+    TVariable *varyingVar =
-                                          varyingType, SymbolType::AngleInternal);
+        new TVariable(symbolTable, ImmutableString(vk::kLineRasterEmulationPosition), varyingType,
+                      SymbolType::AngleInternal);
     TIntermSymbol *varyingDeclarator = new TIntermSymbol(varyingVar);
     TIntermDeclaration *varyingDecl  = new TIntermDeclaration;
     varyingDecl->appendDeclarator(varyingDeclarator);

src/libANGLE/VaryingPacking.cpp

@@ -58,15 +58,17 @@ bool ComparePackedVarying(const PackedVarying &x, const PackedVarying &y)
 // Implementation of PackedVarying
 PackedVarying::PackedVarying(const sh::ShaderVariable &varyingIn,
                              sh::InterpolationType interpolationIn)
-    : PackedVarying(varyingIn, interpolationIn, "", false)
+    : PackedVarying(varyingIn, interpolationIn, "", "", false)
 {}
 PackedVarying::PackedVarying(const sh::ShaderVariable &varyingIn,
                              sh::InterpolationType interpolationIn,
                              const std::string &parentStructNameIn,
+                             const std::string &parentStructMappedNameIn,
                              GLuint fieldIndexIn)
     : varying(&varyingIn),
       interpolation(interpolationIn),
       parentStructName(parentStructNameIn),
+      parentStructMappedName(parentStructMappedNameIn),
       arrayIndex(GL_INVALID_INDEX),
       fieldIndex(fieldIndexIn)
 {}
@@ -84,6 +86,7 @@ PackedVarying &PackedVarying::operator=(PackedVarying &&other)
     std::swap(shaderStages, other.shaderStages);
     std::swap(interpolation, other.interpolation);
     std::swap(parentStructName, other.parentStructName);
+    std::swap(parentStructMappedName, other.parentStructMappedName);
     std::swap(arrayIndex, other.arrayIndex);
     std::swap(fieldIndex, other.fieldIndex);
 
@@ -364,10 +367,11 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
 
                         ASSERT(!field.isStruct() && !field.isArray());
                         mPackedVaryings.emplace_back(field, interpolation, varying->name,
-                                                     fieldIndex);
+                                                     varying->mappedName, fieldIndex);
                         mPackedVaryings.back().shaderStages = shaderStages;
                         uniqueFullNames.insert(mPackedVaryings.back().fullName());
                     }
+                    uniqueFullNames.insert(varying->name);
                 }
                 else
                 {
@@ -382,7 +386,7 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
         // If the varying is not used in the input, we know it is inactive.
         if (!input)
         {
-            mInactiveVaryingNames.push_back(ref.first);
+            mInactiveVaryingMappedNames.push_back(output->mappedName);
             continue;
         }
 
@@ -410,11 +414,12 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
                 {
                     ASSERT(!field->isStruct() && !field->isArray());
                     mPackedVaryings.emplace_back(*field, input->interpolation, input->name,
-                                                 fieldIndex);
+                                                 input->mappedName, fieldIndex);
                     mPackedVaryings.back().shaderStages.set(ShaderType::Vertex);
                     mPackedVaryings.back().arrayIndex = GL_INVALID_INDEX;
                     uniqueFullNames.insert(tfVarying);
                 }
+                uniqueFullNames.insert(input->name);
             }
             // Array as a whole and array element conflict has already been checked in
             // linkValidateTransformFeedback.
@@ -439,7 +444,7 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
 
         if (uniqueFullNames.count(ref.first) == 0)
         {
-            mInactiveVaryingNames.push_back(ref.first);
+            mInactiveVaryingMappedNames.push_back(input->mappedName);
         }
     }
 

src/libANGLE/VaryingPacking.h

@@ -33,6 +33,7 @@ struct PackedVarying : angle::NonCopyable
     PackedVarying(const sh::ShaderVariable &varyingIn,
                   sh::InterpolationType interpolationIn,
                   const std::string &parentStructNameIn,
+                  const std::string &parentStructMappedNameIn,
                   GLuint fieldIndexIn);
     PackedVarying(PackedVarying &&other);
     ~PackedVarying();
@@ -76,6 +77,7 @@ struct PackedVarying : angle::NonCopyable
 
     // Struct name
     std::string parentStructName;
+    std::string parentStructMappedName;
 
     GLuint arrayIndex;
 
@@ -180,9 +182,9 @@ class VaryingPacking final : angle::NonCopyable
         return static_cast<unsigned int>(mRegisterList.size());
     }
 
-    const std::vector<std::string> &getInactiveVaryingNames() const
+    const std::vector<std::string> &getInactiveVaryingMappedNames() const
     {
-        return mInactiveVaryingNames;
+        return mInactiveVaryingMappedNames;
     }
 
     const std::vector<sh::ShaderVariable> &getInputVaryings() const { return mInputVaryings; }
@@ -201,7 +203,7 @@ class VaryingPacking final : angle::NonCopyable
     std::vector<PackedVaryingRegister> mRegisterList;
     std::vector<sh::ShaderVariable> mInputVaryings;
     std::vector<PackedVarying> mPackedVaryings;
-    std::vector<std::string> mInactiveVaryingNames;
+    std::vector<std::string> mInactiveVaryingMappedNames;
 
     PackMode mPackMode;
 };

src/libANGLE/renderer/glslang_wrapper_utils.h

@@ -38,6 +38,7 @@ struct GlslangSourceOptions
     bool useOldRewriteStructSamplers        = false;
     bool supportsTransformFeedbackExtension = false;
     bool emulateTransformFeedback           = false;
+    bool emulateBresenhamLines              = false;
 };
 
 using SpirvBlob = std::vector<uint32_t>;
@@ -59,10 +60,11 @@ struct ShaderInterfaceVariableInfo
     // Used for vertex attributes, fragment shader outputs and varyings.  There could be different
     // variables that share the same name, such as a vertex attribute and a fragment output.  They
     // will share this object since they have the same name, but will find possibly different
-    // locations in their respective slots.  This is also used to indicate in which stages a varying
+    // locations in their respective slots.
-    // is active, as the rest would contain kInvalid.
     gl::ShaderMap<uint32_t> location;
     gl::ShaderMap<uint32_t> component;
+    // The stages this shader interface variable is active.
+    gl::ShaderBitSet activeStages;
     // Used for transform feedback extension to decorate vertex shader output.
     uint32_t xfbBuffer = kInvalid;
     uint32_t xfbOffset = kInvalid;

src/libANGLE/renderer/vulkan/GlslangWrapperVk.cpp

@@ -33,6 +33,7 @@ GlslangSourceOptions CreateSourceOptions(const angle::FeaturesVk &features)
     options.supportsTransformFeedbackExtension =
         features.supportsTransformFeedbackExtension.enabled;
     options.emulateTransformFeedback = features.emulateTransformFeedback.enabled;
+    options.emulateBresenhamLines    = features.basicGLLineRasterization.enabled;
     return options;
 }
 }  // namespace

src/libANGLE/renderer/vulkan/ProgramVk.cpp

@@ -455,6 +455,7 @@ angle::Result ProgramVk::loadSpirvBlob(ContextVk *contextVk, gl::BinaryInputStre
 
         info.descriptorSet = stream->readInt<uint32_t>();
         info.binding       = stream->readInt<uint32_t>();
+        info.activeStages  = gl::ShaderBitSet(static_cast<uint8_t>(stream->readInt<uint32_t>()));
         info.xfbBuffer     = stream->readInt<uint32_t>();
         info.xfbOffset     = stream->readInt<uint32_t>();
         info.xfbStride     = stream->readInt<uint32_t>();
@@ -491,6 +492,7 @@ void ProgramVk::saveSpirvBlob(gl::BinaryOutputStream *stream)
         stream->writeString(nameInfo.first);
         stream->writeIntOrNegOne(nameInfo.second.descriptorSet);
         stream->writeIntOrNegOne(nameInfo.second.binding);
+        stream->writeIntOrNegOne(nameInfo.second.activeStages.bits());
         stream->writeIntOrNegOne(nameInfo.second.xfbBuffer);
         stream->writeIntOrNegOne(nameInfo.second.xfbOffset);
         stream->writeIntOrNegOne(nameInfo.second.xfbStride);

src/tests/gl_tests/GLSLTest.cpp

@@ -4712,6 +4712,36 @@ TEST_P(GLSLTest_ES3, ComplexVaryingStructsUsedInFragmentShader)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that an inactive varying array that doesn't get used in the fragment shader works.
+TEST_P(GLSLTest_ES3, InactiveVaryingArrayUnusedInFragmentShader)
+{
+    constexpr char kVS[] =
+        "#version 300 es\n"
+        "in vec4 inputAttribute;\n"
+        "out vec4 varArray[4];\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position = inputAttribute;\n"
+        "    varArray[0] = vec4(1.0, 0.0, 0.0, 1.0);\n"
+        "    varArray[1] = vec4(0.0, 1.0, 0.0, 1.0);\n"
+        "    varArray[2] = vec4(0.0, 0.0, 1.0, 1.0);\n"
+        "    varArray[3] = vec4(1.0, 1.0, 0.0, 1.0);\n"
+        "}\n";
+
+    constexpr char kFS[] =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out vec4 col;\n"
+        "void main()\n"
+        "{\n"
+        "    col = vec4(0.0, 0.0, 0.0, 1.0);\n"
+        "}\n";
+
+    ANGLE_GL_PROGRAM(program, kVS, kFS);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black);
+}
+
 // Test that an inactive varying struct that doesn't get used in the fragment shader works.
 TEST_P(GLSLTest_ES3, InactiveVaryingStructUnusedInFragmentShader)
 {