Reland "Vulkan: Generate gl_Position.z correction in SPIR-V"

src/compiler/translator/TranslatorVulkan.cpp

@@ -266,46 +266,6 @@ ANGLE_NO_DISCARD bool ReplaceGLDepthRangeWithDriverUniform(TCompiler *compiler,
     return ReplaceVariableWithTyped(compiler, root, depthRangeVar, angleEmulatedDepthRangeRef);
 }
 
-// This operation performs the viewport depth translation needed by Vulkan. In GL the viewport
-// transformation is slightly different - see the GL 2.0 spec section "2.12.1 Controlling the
-// Viewport". In Vulkan the corresponding spec section is currently "23.4. Coordinate
-// Transformations".
-// The equations reduce to an expression:
-//
-//     z_vk = 0.5 * (w_gl + z_gl)
-//
-// where z_vk is the depth output of a Vulkan vertex shader and z_gl is the same for GL.
-ANGLE_NO_DISCARD bool AppendVertexShaderDepthCorrectionToMain(TCompiler *compiler,
-                                                              TIntermBlock *root,
-                                                              TSymbolTable *symbolTable)
-{
-    // Create a symbol reference to "gl_Position"
-    const TVariable *position  = BuiltInVariable::gl_Position();
-    TIntermSymbol *positionRef = new TIntermSymbol(position);
-
-    // Create a swizzle to "gl_Position.z"
-    TVector<int> swizzleOffsetZ = {2};
-    TIntermSwizzle *positionZ   = new TIntermSwizzle(positionRef, swizzleOffsetZ);
-
-    // Create a constant "0.5"
-    TIntermConstantUnion *oneHalf = CreateFloatNode(0.5f);
-
-    // Create a swizzle to "gl_Position.w"
-    TVector<int> swizzleOffsetW = {3};
-    TIntermSwizzle *positionW   = new TIntermSwizzle(positionRef->deepCopy(), swizzleOffsetW);
-
-    // Create the expression "(gl_Position.z + gl_Position.w) * 0.5".
-    TIntermBinary *zPlusW     = new TIntermBinary(EOpAdd, positionZ, positionW);
-    TIntermBinary *halfZPlusW = new TIntermBinary(EOpMul, zPlusW, oneHalf);
-
-    // Create the assignment "gl_Position.z = (gl_Position.z + gl_Position.w) * 0.5"
-    TIntermTyped *positionZLHS = positionZ->deepCopy();
-    TIntermBinary *assignment  = new TIntermBinary(TOperator::EOpAssign, positionZLHS, halfZPlusW);
-
-    // Append the assignment as a statement at the end of the shader.
-    return RunAtTheEndOfShader(compiler, root, assignment, symbolTable);
-}
-
 ANGLE_NO_DISCARD bool AppendTransformFeedbackOutputToMain(TCompiler *compiler,
                                                           TIntermBlock *root,
                                                           TSymbolTable *symbolTable)
@@ -1117,10 +1077,18 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
             {
                 return false;
             }
-            if (!AppendVertexShaderDepthCorrectionToMain(this, root, &getSymbolTable()))
+
+            // Work around a bug in SwiftShader where missing gl_Position causes an assertion error.
+            // This code appends gl_Position = gl_Position; to the end of the shader.
+            // b/176161380
+            TIntermSymbol *position = new TIntermSymbol(BuiltInVariable::gl_Position());
+            TIntermBinary *assignment =
+                new TIntermBinary(TOperator::EOpAssign, position, position->deepCopy());
+            if (!RunAtTheEndOfShader(this, root, assignment, &getSymbolTable()))
             {
                 return false;
             }
+
             break;
         }
 

src/libANGLE/renderer/glslang_wrapper_utils.cpp

@@ -1174,6 +1174,8 @@ class SpirvTransformerBase : angle::NonCopyable
                                  const angle::FixedVector<uint32_t, 4> &constituents);
     void writeCompositeExtract(uint32_t id, uint32_t typeId, uint32_t compositeId, uint32_t field);
     void writeConstant(uint32_t id, uint32_t typeId, uint32_t value);
+    void writeFAdd(uint32_t id, uint32_t typeId, uint32_t operand1, uint32_t operand2);
+    void writeFMul(uint32_t id, uint32_t typeId, uint32_t operand1, uint32_t operand2);
     void writeFNegate(uint32_t id, uint32_t typeId, uint32_t operand);
     void writeLoad(uint32_t id, uint32_t typeId, uint32_t pointerId);
     void writeMemberDecorate(uint32_t typeId, uint32_t member, uint32_t decoration, uint32_t value);
@@ -1381,6 +1383,56 @@ void SpirvTransformerBase::writeConstant(uint32_t id, uint32_t typeId, uint32_t
     copyInstruction(constant.data(), kConstantInstructionLength);
 }
 
+void SpirvTransformerBase::writeFAdd(uint32_t id,
+                                     uint32_t typeId,
+                                     uint32_t operand1,
+                                     uint32_t operand2)
+{
+    // SPIR-V 1.0 Section 3.32 Instructions, OpFAdd
+    constexpr size_t kTypeIdIndex           = 1;
+    constexpr size_t kIdIndex               = 2;
+    constexpr size_t kOperand1Index         = 3;
+    constexpr size_t kOperand2Index         = 4;
+    constexpr size_t kFAddInstructionLength = 5;
+
+    std::array<uint32_t, kFAddInstructionLength> fAdd = {};
+
+    // Fill the fields.
+    SetSpirvInstructionOp(fAdd.data(), spv::OpFAdd);
+    SetSpirvInstructionLength(fAdd.data(), kFAddInstructionLength);
+    fAdd[kTypeIdIndex]   = typeId;
+    fAdd[kIdIndex]       = id;
+    fAdd[kOperand1Index] = operand1;
+    fAdd[kOperand2Index] = operand2;
+
+    copyInstruction(fAdd.data(), kFAddInstructionLength);
+}
+
+void SpirvTransformerBase::writeFMul(uint32_t id,
+                                     uint32_t typeId,
+                                     uint32_t operand1,
+                                     uint32_t operand2)
+{
+    // SPIR-V 1.0 Section 3.32 Instructions, OpFMul
+    constexpr size_t kTypeIdIndex           = 1;
+    constexpr size_t kIdIndex               = 2;
+    constexpr size_t kOperand1Index         = 3;
+    constexpr size_t kOperand2Index         = 4;
+    constexpr size_t kFMulInstructionLength = 5;
+
+    std::array<uint32_t, kFMulInstructionLength> fMul = {};
+
+    // Fill the fields.
+    SetSpirvInstructionOp(fMul.data(), spv::OpFMul);
+    SetSpirvInstructionLength(fMul.data(), kFMulInstructionLength);
+    fMul[kTypeIdIndex]   = typeId;
+    fMul[kIdIndex]       = id;
+    fMul[kOperand1Index] = operand1;
+    fMul[kOperand2Index] = operand2;
+
+    copyInstruction(fMul.data(), kFMulInstructionLength);
+}
+
 void SpirvTransformerBase::writeFNegate(uint32_t id, uint32_t typeId, uint32_t operand)
 {
     // SPIR-V 1.0 Section 3.32 Instructions, OpFNegate
@@ -1652,6 +1704,7 @@ class SpirvTransformer final : public SpirvTransformerBase
     void writeInputPreamble();
     void writeOutputPrologue();
     void preRotateXY(uint32_t xId, uint32_t yId, uint32_t *rotatedXIdOut, uint32_t *rotatedYIdOut);
+    void transformZToVulkanClipSpace(uint32_t zId, uint32_t wId, uint32_t *correctedZIdOut);
 
     // Special flags:
     GlslangSpirvOptions mOptions;
@@ -1710,6 +1763,7 @@ class SpirvTransformer final : public SpirvTransformerBase
     // - mVec4OutTypePointerId: id of OpTypePointer Output %mVec4ID
     // - mIntId: id of OpTypeInt 32 1
     // - mInt0Id: id of OpConstant %mIntID 0
+    // - mFloatHalfId: id of OpConstant %mFloatId 0.5f
     // - mOutputPerVertexTypePointerId: id of OpTypePointer Output %mOutputPerVertex.typeId
     // - mOutputPerVertexId: id of OpVariable %mOutputPerVertexTypePointerId Output
     //
@@ -1718,6 +1772,7 @@ class SpirvTransformer final : public SpirvTransformerBase
     uint32_t mVec4OutTypePointerId         = 0;
     uint32_t mIntId                        = 0;
     uint32_t mInt0Id                       = 0;
+    uint32_t mFloatHalfId                  = 0;
     uint32_t mOutputPerVertexTypePointerId = 0;
     uint32_t mOutputPerVertexId            = 0;
 };
@@ -1941,8 +1996,9 @@ void SpirvTransformer::transformInstruction()
 // present in the original shader need to be done here.
 void SpirvTransformer::writePendingDeclarations()
 {
-    // Currently, only pre-rotation requires declarations that may not necessarily be in the shader.
-    if (IsRotationIdentity(mOptions.preRotation))
+    // Pre-rotation and transformation of depth to Vulkan clip space require declarations that may
+    // not necessarily be in the shader.
+    if (IsRotationIdentity(mOptions.preRotation) && !mOptions.transformPositionToVulkanClipSpace)
     {
         return;
     }
@@ -1974,6 +2030,12 @@ void SpirvTransformer::writePendingDeclarations()
     ASSERT(mInt0Id == 0);
     mInt0Id = getNewId();
     writeConstant(mInt0Id, mIntId, 0);
+
+    constexpr uint32_t kFloatHalfAsUint = 0x3F00'0000;
+
+    ASSERT(mFloatHalfId == 0);
+    mFloatHalfId = getNewId();
+    writeConstant(mFloatHalfId, mFloatId, kFloatHalfAsUint);
 }
 
 // Called by transformInstruction to insert necessary instructions for casting varyings.
@@ -2041,13 +2103,22 @@ void SpirvTransformer::writeOutputPrologue()
         }
     }
 
-    // Transform gl_Position to account for prerotation if necessary.
-    if (mOutputPerVertexId == 0 || IsRotationIdentity(mOptions.preRotation))
+    // Transform gl_Position to account for prerotation and Vulkan clip space if necessary.
+    if (mOutputPerVertexId == 0 ||
+        (IsRotationIdentity(mOptions.preRotation) && !mOptions.transformPositionToVulkanClipSpace))
     {
         return;
     }
 
-    // Generate the following SPIR-V for prerotation:
+    // In GL the viewport transformation is slightly different - see the GL 2.0 spec section "2.12.1
+    // Controlling the Viewport".  In Vulkan the corresponding spec section is currently "23.4.
+    // Coordinate Transformations".  The following transformation needs to be done:
+    //
+    //     z_vk = 0.5 * (w_gl + z_gl)
+    //
+    // where z_vk is the depth output of a Vulkan geometry-stage shader and z_gl is the same for GL.
+
+    // Generate the following SPIR-V for prerotation and depth transformation:
     //
     //     // Create an access chain to output gl_PerVertex.gl_Position, which is always at index 0.
     //     %PositionPointer = OpAccessChain %mVec4OutTypePointerId %mOutputPerVertexId %mInt0Id
@@ -2059,12 +2130,18 @@ void SpirvTransformer::writeOutputPrologue()
     //     %y = OpCompositeExtract %mFloatId %Position 1
     //     %z = OpCompositeExtract %mFloatId %Position 2
     //     %w = OpCompositeExtract %mFloatId %Position 3
+    //
     //     // Transform %x and %y based on pre-rotation.  This could include swapping the two ids
     //     // (in the transformer, no need to generate SPIR-V instructions for that), and/or
     //     // negating either component.  To negate a component, the following instruction is used:
     //     (optional:) %negated = OpFNegate %mFloatId %component
-    //     // Create the rotated gl_Position from the rotated x and y components.
-    //     %RotatedPosition = OpCompositeConstruct %mVec4Id %rotatedX %rotatedY %z %w
+    //
+    //     // Transform %z if necessary, based on the above formula.
+    //     %zPlusW = OpFAdd %mFloatId %z %w
+    //     %correctedZ = OpFMul %mFloatId %zPlusW %mFloatHalfId
+    //
+    //     // Create the rotated gl_Position from the rotated x and y and corrected z components.
+    //     %RotatedPosition = OpCompositeConstruct %mVec4Id %rotatedX %rotatedY %correctedZ %w
     //     // Store the results back in gl_Position
     //     OpStore %PositionPointer %RotatedPosition
     //
@@ -2087,7 +2164,11 @@ void SpirvTransformer::writeOutputPrologue()
     uint32_t rotatedYId = 0;
     preRotateXY(xId, yId, &rotatedXId, &rotatedYId);
 
-    writeCompositeConstruct(rotatedPositionId, mVec4Id, {rotatedXId, rotatedYId, zId, wId});
+    uint32_t correctedZId = 0;
+    transformZToVulkanClipSpace(zId, wId, &correctedZId);
+
+    writeCompositeConstruct(rotatedPositionId, mVec4Id,
+                            {rotatedXId, rotatedYId, correctedZId, wId});
     writeStore(positionPointerId, rotatedPositionId);
 }
 
@@ -2135,6 +2216,26 @@ void SpirvTransformer::preRotateXY(uint32_t xId,
     }
 }
 
+void SpirvTransformer::transformZToVulkanClipSpace(uint32_t zId,
+                                                   uint32_t wId,
+                                                   uint32_t *correctedZIdOut)
+{
+    if (!mOptions.transformPositionToVulkanClipSpace)
+    {
+        *correctedZIdOut = zId;
+        return;
+    }
+
+    const uint32_t zPlusWId = getNewId();
+    *correctedZIdOut        = getNewId();
+
+    // %zPlusW = OpFAdd %mFloatId %z %w
+    writeFAdd(zPlusWId, mFloatId, zId, wId);
+
+    // %correctedZ = OpFMul %mFloatId %zPlusW %mFloatHalfId
+    writeFMul(*correctedZIdOut, mFloatId, zPlusWId, mFloatHalfId);
+}
+
 void SpirvTransformer::visitDecorate(const uint32_t *instruction)
 {
     // SPIR-V 1.0 Section 3.32 Instructions, OpDecorate

src/libANGLE/renderer/glslang_wrapper_utils.h

@@ -57,6 +57,7 @@ struct GlslangSpirvOptions
 {
     gl::ShaderType shaderType                 = gl::ShaderType::InvalidEnum;
     SurfaceRotation preRotation               = SurfaceRotation::Identity;
+    bool transformPositionToVulkanClipSpace   = false;
     bool removeEarlyFragmentTestsOptimization = false;
     bool removeDebugInfo                      = false;
     bool isTransformFeedbackStage             = false;

src/libANGLE/renderer/metal/mtl_glslang_utils.mm

@@ -458,8 +458,9 @@ angle::Result GlslangGetShaderSpirvCode(ErrorHandler *context,
     for (const gl::ShaderType shaderType : linkedShaderStages)
     {
         GlslangSpirvOptions options;
-        options.shaderType               = shaderType;
-        options.isTransformFeedbackStage = shaderType == gl::ShaderType::Vertex;
+        options.shaderType                         = shaderType;
+        options.transformPositionToVulkanClipSpace = true;
+        options.isTransformFeedbackStage           = shaderType == gl::ShaderType::Vertex;
 
         angle::Result status = GlslangTransformSpirvCode(
             [context](GlslangError error) { return HandleError(context, error); }, options,

src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp

@@ -33,10 +33,11 @@ bool ValidateTransformedSpirV(ContextVk *contextVk,
     for (gl::ShaderType shaderType : linkedShaderStages)
     {
         GlslangSpirvOptions options;
-        options.shaderType               = shaderType;
-        options.preRotation              = SurfaceRotation::FlippedRotated90Degrees;
-        options.removeDebugInfo          = true;
-        options.isTransformFeedbackStage = shaderType == lastPreFragmentStage;
+        options.shaderType                         = shaderType;
+        options.preRotation                        = SurfaceRotation::FlippedRotated90Degrees;
+        options.transformPositionToVulkanClipSpace = true;
+        options.removeDebugInfo                    = true;
+        options.isTransformFeedbackStage           = shaderType == lastPreFragmentStage;
 
         SpirvBlob transformed;
         if (GlslangWrapperVk::TransformSpirV(contextVk, options, variableInfoMap,
@@ -140,6 +141,7 @@ angle::Result ProgramInfo::initProgram(ContextVk *contextVk,
     if (isLastPreFragmentStage)
     {
         options.preRotation = static_cast<SurfaceRotation>(optionBits.surfaceRotation);
+        options.transformPositionToVulkanClipSpace = true;
     }
 
     ANGLE_TRY(GlslangWrapperVk::TransformSpirV(contextVk, options, variableInfoMap,

src/tests/gl_tests/GeometryShaderTest.cpp

@@ -1412,9 +1412,6 @@ TEST_P(GeometryShaderTest, DepthViewportTransform)
 {
     ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_geometry_shader"));
 
-    // http://anglebug.com/5479
-    ANGLE_SKIP_TEST_IF(IsVulkan());
-
     constexpr char kVS[] = R"(#version 310 es
 void main()
 {