Vulkan: Fix point coord management

src/compiler/translator/TranslatorVulkan.cpp

@@ -19,6 +19,8 @@
 #include "compiler/translator/tree_util/IntermNode_util.h"
 #include "compiler/translator/tree_util/RunAtTheEndOfShader.h"
 #include "compiler/translator/util.h"
+#include "tree_util/FindMain.h"
+#include "tree_util/ReplaceVariable.h"
 
 namespace sh
 {
@@ -140,6 +142,68 @@ class DeclareDefaultUniformsTraverser : public TIntermTraverser
     bool mInDefaultUniform;
 };
 
+constexpr ImmutableString kFlippedPointCoordName = ImmutableString("flippedPointCoord");
+
+// Declares a new variable to replace gl_PointCoord with a version that is flipping the Y
+// coordinate.
+void FlipGLPointCoordinates(TIntermBlock *root, TSymbolTable *symbolTable)
+{
+    // Create a symbol reference to "gl_PointCoord"
+    const TVariable *pointCoord  = BuiltInVariable::gl_PointCoord();
+    TIntermSymbol *pointCoordRef = new TIntermSymbol(pointCoord);
+
+    // Create a swizzle to "gl_PointCoord.x"
+    TVector<int> swizzleOffsetX;
+    swizzleOffsetX.push_back(0);
+    TIntermSwizzle *pointCoordX = new TIntermSwizzle(pointCoordRef, swizzleOffsetX);
+
+    // Create a swizzle to "gl_PointCoord.y"
+    TVector<int> swizzleOffsetY;
+    swizzleOffsetY.push_back(1);
+    TIntermSwizzle *pointCoordY = new TIntermSwizzle(pointCoordRef, swizzleOffsetY);
+
+    // Create a symbol reference to our new variable that will hold the modified gl_PointCoord.
+    TVariable *replacementVar =
+        new TVariable(symbolTable, kFlippedPointCoordName,
+                      StaticType::Helpers::GetForVecMatHelper<EbtFloat, EbpMedium, EvqGlobal, 1>(2),
+                      SymbolType::UserDefined);
+    DeclareGlobalVariable(root, replacementVar);
+    TIntermSymbol *flippedPointCoordsRef = new TIntermSymbol(replacementVar);
+
+    // Create a constant "-1.0"
+    const TType *constantType             = StaticType::GetBasic<EbtFloat>();
+    TConstantUnion *constantValueMinusOne = new TConstantUnion();
+    constantValueMinusOne->setFConst(-1.0f);
+    TIntermConstantUnion *minusOne = new TIntermConstantUnion(constantValueMinusOne, *constantType);
+
+    // Create a constant "1.0"
+    TConstantUnion *constantValueOne = new TConstantUnion();
+    constantValueOne->setFConst(1.0f);
+    TIntermConstantUnion *one = new TIntermConstantUnion(constantValueOne, *constantType);
+
+    // Create the expression "gl_PointCoord.y * -1.0 + 1.0"
+    TIntermBinary *inverseY = new TIntermBinary(EOpMul, pointCoordY, minusOne);
+    TIntermBinary *plusOne  = new TIntermBinary(EOpAdd, inverseY, one);
+
+    // Create the new vec2 using the modified Y
+    TIntermSequence *sequence = new TIntermSequence();
+    sequence->push_back(pointCoordX);
+    sequence->push_back(plusOne);
+    TIntermAggregate *aggregate =
+        TIntermAggregate::CreateConstructor(BuiltInVariable::gl_PointCoord()->getType(), sequence);
+
+    // Use this new variable instead of gl_PointCoord everywhere.
+    ReplaceVariable(root, pointCoord, replacementVar);
+
+    // Assign this new value to flippedPointCoord
+    TIntermBinary *assignment = new TIntermBinary(EOpInitialize, flippedPointCoordsRef, aggregate);
+
+    // Add this assigment at the beginning of the main function
+    TIntermFunctionDefinition *main = FindMain(root);
+    TIntermSequence *mainSequence   = main->getBody()->getSequence();
+    mainSequence->insert(mainSequence->begin(), assignment);
+}
+
 // 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
@@ -271,6 +335,21 @@ void TranslatorVulkan::translate(TIntermBlock *root,
         {
             sink << "layout(location = 0) out vec4 webgl_FragData[gl_MaxDrawBuffers];\n";
         }
+
+        // Search for the gl_PointCoord usage, if its used, we need to flip the y coordinate.
+        for (const Varying &inputVarying : inputVaryings)
+        {
+            if (!inputVarying.isBuiltIn())
+            {
+                continue;
+            }
+
+            if (inputVarying.name == "gl_PointCoord")
+            {
+                FlipGLPointCoordinates(root, &getSymbolTable());
+                break;
+            }
+        }
     }
     else
     {

src/compiler/translator/tree_ops/DeclareAndInitBuiltinsForInstancedMultiview.cpp

@@ -74,15 +74,6 @@ void InitializeViewIDAndInstanceID(const TVariable *viewID,
     initializers->push_back(viewIDInitializer);
 }
 
-void DeclareGlobalVariable(TIntermBlock *root, const TVariable *variable)
-{
-    TIntermDeclaration *declaration = new TIntermDeclaration();
-    declaration->appendDeclarator(new TIntermSymbol(variable));
-
-    TIntermSequence *globalSequence = root->getSequence();
-    globalSequence->insert(globalSequence->begin(), declaration);
-}
-
 // Adds a branch to write int(ViewID_OVR) to either gl_ViewportIndex or gl_Layer. The branch is
 // added to the end of the initializers' sequence.
 void SelectViewIndexInVertexShader(const TVariable *viewID,

src/compiler/translator/util.cpp

@@ -641,6 +641,15 @@ TType GetShaderVariableBasicType(const sh::ShaderVariable &var)
     }
 }
 
+void DeclareGlobalVariable(TIntermBlock *root, const TVariable *variable)
+{
+    TIntermDeclaration *declaration = new TIntermDeclaration();
+    declaration->appendDeclarator(new TIntermSymbol(variable));
+
+    TIntermSequence *globalSequence = root->getSequence();
+    globalSequence->insert(globalSequence->begin(), declaration);
+}
+
 // GLSL ES 1.0.17 4.6.1 The Invariant Qualifier
 bool CanBeInvariantESSL1(TQualifier qualifier)
 {

src/compiler/translator/util.h

@@ -23,6 +23,7 @@ bool atoi_clamp(const char *str, unsigned int *value);
 
 namespace sh
 {
+class TIntermBlock;
 class TSymbolTable;
 
 float NumericLexFloat32OutOfRangeToInfinity(const std::string &str);
@@ -51,6 +52,8 @@ ImmutableString GetTypeName(const TType &type, ShHashFunction64 hashFunction, Na
 
 TType GetShaderVariableBasicType(const sh::ShaderVariable &var);
 
+void DeclareGlobalVariable(TIntermBlock *root, const TVariable *variable);
+
 bool IsBuiltinOutputVariable(TQualifier qualifier);
 bool IsBuiltinFragmentInputVariable(TQualifier qualifier);
 bool CanBeInvariantESSL1(TQualifier qualifier);

src/tests/deqp_support/deqp_gles2_test_expectations.txt

@@ -218,7 +218,6 @@
 2580 VULKAN : dEQP-GLES2.functional.buffer.write.random.* = SKIP
 2494 VULKAN : dEQP-GLES2.functional.shaders.struct.uniform.sampler_* = SKIP
 2592 VULKAN : dEQP-GLES2.functional.shaders.builtin_variable.depth_range* = SKIP
-2592 VULKAN : dEQP-GLES2.functional.shaders.builtin_variable.pointcoord = SKIP
 2594 VULKAN : dEQP-GLES2.functional.shaders.fragdata.valid_static_index = SKIP
 2595 VULKAN : dEQP-GLES2.functional.shaders.random.all_features.fragment* = SKIP
 2597 VULKAN : dEQP-GLES2.functional.fbo.render.color_clear.* = SKIP
@@ -323,4 +322,5 @@
 2444 VULKAN : dEQP-GLES2.functional.default_vertex_attrib.* = SKIP
 
 // Vulkan AMD Windows specific failures
-2602 VULKAN WIN AMD : dEQP-GLES2.functional.buffer.write.* = SKIP
+2602 VULKAN WIN AMD : dEQP-GLES2.functional.buffer.write.* = SKIP
\ No newline at end of file
+2658 VULKAN WIN AMD : dEQP-GLES2.functional.shaders.builtin_variable.pointcoord = SKIP
\ No newline at end of file

src/tests/gl_tests/PointSpritesTest.cpp

@@ -13,6 +13,15 @@
 
 using namespace angle;
 
+constexpr char kVertexShaderSource[] =
+    R"(attribute vec4 vPosition;
+        uniform float uPointSize;
+        void main()
+        {
+            gl_PointSize = uPointSize;
+            gl_Position  = vPosition;
+        })";
+
 class PointSpritesTest : public ANGLETest
 {
   protected:
@@ -31,108 +40,126 @@ class PointSpritesTest : public ANGLETest
     virtual void SetUp() { ANGLETest::SetUp(); }
 
     float s2p(float s) { return (s + 1.0f) * 0.5f * (GLfloat)windowWidth; }
-};
-
-// Checks gl_PointCoord and gl_PointSize
-// https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/variables/gl-pointcoord.html
-TEST_P(PointSpritesTest, PointCoordAndPointSizeCompliance)
-{
-    // TODO(jmadill): Investigate potential AMD driver bug.
-    // http://anglebug.com/1643
-    ANGLE_SKIP_TEST_IF(IsAMD() && IsDesktopOpenGL() && IsWindows());
-
-    // TODO: Point coord not yet implemented in Vulkan.
-    // http://anglebug.com/2457
-    ANGLE_SKIP_TEST_IF(IsVulkan());
-
-    const std::string fs =
-        R"(precision mediump float;
-        void main()
-        {
-            gl_FragColor = vec4(gl_PointCoord.x, gl_PointCoord.y, 0, 1);
-        })";
-
-    const std::string vs =
-        R"(attribute vec4 vPosition;
-        uniform float uPointSize;
-        void main()
-        {
-            gl_PointSize = uPointSize;
-            gl_Position  = vPosition;
-        })";
 
-    ANGLE_GL_PROGRAM(program, vs, fs);
-
-    glUseProgram(program);
+    void testPointCoordAndPointSizeCompliance(priv::GLProgram program)
+    {
+        glUseProgram(program);
 
-    GLfloat pointSizeRange[2] = {};
-    glGetFloatv(GL_ALIASED_POINT_SIZE_RANGE, pointSizeRange);
+        GLfloat pointSizeRange[2] = {};
+        glGetFloatv(GL_ALIASED_POINT_SIZE_RANGE, pointSizeRange);
 
-    GLfloat maxPointSize = pointSizeRange[1];
+        GLfloat maxPointSize = pointSizeRange[1];
 
-    ASSERT_TRUE(maxPointSize >= 1);
-    maxPointSize = floorf(maxPointSize);
-    ASSERT_TRUE((int)maxPointSize % 1 == 0);
+        ASSERT_TRUE(maxPointSize >= 1);
+        maxPointSize = floorf(maxPointSize);
+        ASSERT_TRUE((int)maxPointSize % 1 == 0);
 
-    maxPointSize       = std::min(maxPointSize, 64.0f);
-    GLfloat pointWidth = maxPointSize / windowWidth;
-    GLint step         = static_cast<GLint>(floorf(maxPointSize / 4));
-    GLint pointStep    = std::max<GLint>(1, step);
+        maxPointSize       = std::min(maxPointSize, 64.0f);
+        GLfloat pointWidth = maxPointSize / windowWidth;
+        GLint step         = static_cast<GLint>(floorf(maxPointSize / 4));
+        GLint pointStep    = std::max<GLint>(1, step);
 
-    GLint pointSizeLoc = glGetUniformLocation(program, "uPointSize");
-    ASSERT_GL_NO_ERROR();
+        GLint pointSizeLoc = glGetUniformLocation(program, "uPointSize");
+        ASSERT_GL_NO_ERROR();
 
-    glUniform1f(pointSizeLoc, maxPointSize);
-    ASSERT_GL_NO_ERROR();
+        glUniform1f(pointSizeLoc, maxPointSize);
+        ASSERT_GL_NO_ERROR();
 
-    GLfloat pixelOffset = ((int)maxPointSize % 2) ? (1.0f / (GLfloat)windowWidth) : 0;
-    GLBuffer vertexObject;
+        GLfloat pixelOffset = ((int)maxPointSize % 2) ? (1.0f / (GLfloat)windowWidth) : 0;
+        GLBuffer vertexObject;
 
-    glBindBuffer(GL_ARRAY_BUFFER, vertexObject.get());
-    ASSERT_GL_NO_ERROR();
+        glBindBuffer(GL_ARRAY_BUFFER, vertexObject.get());
+        ASSERT_GL_NO_ERROR();
 
-    GLfloat thePoints[] = {-0.5f + pixelOffset, -0.5f + pixelOffset, 0.5f + pixelOffset,
-                           -0.5f + pixelOffset, -0.5f + pixelOffset, 0.5f + pixelOffset,
-                           0.5f + pixelOffset,  0.5f + pixelOffset};
+        GLfloat thePoints[] = {-0.5f + pixelOffset, -0.5f + pixelOffset, 0.5f + pixelOffset,
+                               -0.5f + pixelOffset, -0.5f + pixelOffset, 0.5f + pixelOffset,
+                               0.5f + pixelOffset,  0.5f + pixelOffset};
 
-    glBufferData(GL_ARRAY_BUFFER, sizeof(thePoints), thePoints, GL_STATIC_DRAW);
-    ASSERT_GL_NO_ERROR();
+        glBufferData(GL_ARRAY_BUFFER, sizeof(thePoints), thePoints, GL_STATIC_DRAW);
+        ASSERT_GL_NO_ERROR();
 
-    glEnableVertexAttribArray(0);
-    glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 0, 0);
+        glEnableVertexAttribArray(0);
+        glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 0, 0);
 
-    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+        glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
 
-    glDrawArrays(GL_POINTS, 0, 4);
-    ASSERT_GL_NO_ERROR();
+        glDrawArrays(GL_POINTS, 0, 4);
+        ASSERT_GL_NO_ERROR();
 
-    std::string debugText;
-    for (float py = 0; py < 2; ++py)
-    {
-        for (float px = 0; px < 2; ++px)
+        for (float py = 0; py < 2; ++py)
         {
-            float pointX = -0.5f + px + pixelOffset;
-            float pointY = -0.5f + py + pixelOffset;
-            for (int yy = 0; yy < maxPointSize; yy += pointStep)
+            for (float px = 0; px < 2; ++px)
             {
-                for (int xx = 0; xx < maxPointSize; xx += pointStep)
+                float pointX = -0.5f + px + pixelOffset;
+                float pointY = -0.5f + py + pixelOffset;
+                for (int yy = 0; yy < maxPointSize; yy += pointStep)
                 {
-                    // formula for s and t from OpenGL ES 2.0 spec section 3.3
-                    float xw         = s2p(pointX);
-                    float yw         = s2p(pointY);
-                    float u          = xx / maxPointSize * 2 - 1;
-                    float v          = yy / maxPointSize * 2 - 1;
-                    int xf           = static_cast<int>(floorf(s2p(pointX + u * pointWidth)));
-                    int yf           = static_cast<int>(floorf(s2p(pointY + v * pointWidth)));
-                    float s          = 0.5f + (xf + 0.5f - xw) / maxPointSize;
-                    float t          = 0.5f + (yf + 0.5f - yw) / maxPointSize;
-                    GLubyte color[4] = {static_cast<GLubyte>(floorf(s * 255)),
-                                        static_cast<GLubyte>(floorf((1 - t) * 255)), 0, 255};
-                    EXPECT_PIXEL_NEAR(xf, yf, color[0], color[1], color[2], color[3], 4);
+                    for (int xx = 0; xx < maxPointSize; xx += pointStep)
+                    {
+                        // formula for s and t from OpenGL ES 2.0 spec section 3.3
+                        float xw         = s2p(pointX);
+                        float yw         = s2p(pointY);
+                        float u          = xx / maxPointSize * 2 - 1;
+                        float v          = yy / maxPointSize * 2 - 1;
+                        int xf           = static_cast<int>(floorf(s2p(pointX + u * pointWidth)));
+                        int yf           = static_cast<int>(floorf(s2p(pointY + v * pointWidth)));
+                        float s          = 0.5f + (xf + 0.5f - xw) / maxPointSize;
+                        float t          = 0.5f + (yf + 0.5f - yw) / maxPointSize;
+                        GLubyte color[4] = {static_cast<GLubyte>(floorf(s * 255)),
+                                            static_cast<GLubyte>(floorf((1 - t) * 255)), 0, 255};
+                        EXPECT_PIXEL_NEAR(xf, yf, color[0], color[1], color[2], color[3], 4);
+                    }
                 }
             }
         }
     }
+};
+
+// Checks gl_PointCoord and gl_PointSize
+// https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/variables/gl-pointcoord.html
+TEST_P(PointSpritesTest, PointCoordAndPointSizeCompliance)
+{
+    // TODO(jmadill): Investigate potential AMD driver bug.
+    // http://anglebug.com/1643
+    ANGLE_SKIP_TEST_IF(IsAMD() && IsDesktopOpenGL() && IsWindows());
+
+    constexpr char fs[] =
+        R"(precision mediump float;
+        void main()
+        {
+            gl_FragColor = vec4(gl_PointCoord.x, gl_PointCoord.y, 0, 1);
+        })";
+
+    ANGLE_GL_PROGRAM(program, kVertexShaderSource, fs);
+
+    testPointCoordAndPointSizeCompliance(program);
+}
+
+// Checks gl_PointCoord and gl_PointSize, but use the gl_PointCoord inside a function.
+// In Vulkan, we need to inject some code into the shader to flip the Y coordinate, and we
+// need to make sure this code injection works even if someone uses gl_PointCoord outside the
+// main function.
+TEST_P(PointSpritesTest, UsingPointCoordInsideFunction)
+{
+    // TODO(jmadill): Investigate potential AMD driver bug.
+    // http://anglebug.com/1643
+    ANGLE_SKIP_TEST_IF(IsAMD() && IsDesktopOpenGL() && IsWindows());
+
+    constexpr char fs[] =
+        R"(precision mediump float;
+        void foo() 
+        {
+            gl_FragColor = vec4(gl_PointCoord.x, gl_PointCoord.y, 0, 1);
+        }
+
+        void main()
+        {
+            foo();
+        })";
+
+    ANGLE_GL_PROGRAM(program, kVertexShaderSource, fs);
+
+    testPointCoordAndPointSizeCompliance(program);
 }
 
 // Verify that drawing a point without enabling any attributes succeeds