Vulkan: Correct gl_FragCoord with default FBO.

src/compiler/translator/TranslatorVulkan.cpp

@@ -150,81 +150,84 @@ class DeclareDefaultUniformsTraverser : public TIntermTraverser
 };
 
 constexpr ImmutableString kFlippedPointCoordName = ImmutableString("flippedPointCoord");
+constexpr ImmutableString kFlippedFragCoordName     = ImmutableString("flippedFragCoord");
 constexpr ImmutableString kEmulatedDepthRangeParams = ImmutableString("ANGLEDepthRangeParams");
 
-// Declares a new variable to replace gl_PointCoord with a version that is flipping the Y
-// coordinate.
-void FlipGLPointCoord(TIntermBlock *root,
-                      const TVariable *driverUniforms,
-                      TSymbolTable *symbolTable)
+TIntermConstantUnion *CreateConstantFloat(float value)
 {
-    // Create a symbol reference to "gl_PointCoord"
-    const TVariable *pointCoord  = BuiltInVariable::gl_PointCoord();
-    TIntermSymbol *pointCoordRef = new TIntermSymbol(pointCoord);
+    const TType *constantType     = StaticType::GetBasic<TBasicType::EbtFloat>();
+    TConstantUnion *constantValue = new TConstantUnion();
+    constantValue->setFConst(value);
+    return new TIntermConstantUnion(constantValue, *constantType);
+}
 
-    // Create a swizzle to "gl_PointCoord.x"
-    TVector<int> swizzleOffsetX;
-    swizzleOffsetX.push_back(0);
-    TIntermSwizzle *pointCoordX = new TIntermSwizzle(pointCoordRef, swizzleOffsetX);
+TIntermBinary *CreateDriverUniformRef(const TVariable *driverUniforms, int fieldIndex)
+{
+    TIntermSymbol *angleUniformsRef = new TIntermSymbol(driverUniforms);
+    TConstantUnion *uniformIndex    = new TConstantUnion;
+    uniformIndex->setIConst(fieldIndex);
+    TIntermConstantUnion *indexRef =
+        new TIntermConstantUnion(uniformIndex, *StaticType::GetBasic<EbtInt>());
+    return new TIntermBinary(EOpIndexDirectInterfaceBlock, angleUniformsRef, indexRef);
+}
 
-    // Create a swizzle to "gl_PointCoord.y"
+// Replaces a builtin variable with a version that corrects the Y coordinate.
+void FlipBuiltinVariable(TIntermBlock *root,
+                         const TVariable *driverUniforms,
+                         int driverUniformSwizzleIndex,
+                         TSymbolTable *symbolTable,
+                         const TVariable *builtin,
+                         const ImmutableString &flippedVariableName,
+                         TIntermTyped *pivot)
+{
+    // Create a symbol reference to 'builtin'.
+    TIntermSymbol *builtinRef = new TIntermSymbol(builtin);
+
+    // Create a swizzle to "builtin.y"
     TVector<int> swizzleOffsetY;
     swizzleOffsetY.push_back(1);
-    TIntermSwizzle *pointCoordY = new TIntermSwizzle(pointCoordRef, swizzleOffsetY);
+    TIntermSwizzle *builtinY = new TIntermSwizzle(builtinRef, swizzleOffsetY);
 
-    // Create a symbol reference to our new variable that will hold the modified gl_PointCoord.
+    // Create a symbol reference to our new variable that will hold the modified builtin.
+    const TType *type = StaticType::GetForVec<EbtFloat>(
+        EvqGlobal, static_cast<unsigned char>(builtin->getType().getNominalSize()));
     TVariable *replacementVar =
-        new TVariable(symbolTable, kFlippedPointCoordName,
-                      StaticType::Helpers::GetForVecMatHelper<EbtFloat, EbpMedium, EvqGlobal, 1>(2),
-                      SymbolType::UserDefined);
+        new TVariable(symbolTable, flippedVariableName, type, SymbolType::AngleInternal);
     DeclareGlobalVariable(root, replacementVar);
-    TIntermSymbol *flippedPointCoordsRef = new TIntermSymbol(replacementVar);
+    TIntermSymbol *flippedBuiltinRef = new TIntermSymbol(replacementVar);
 
-    const TType *constantType             = StaticType::GetBasic<EbtFloat>();
-
-    // Create a constant "0.5"
-    TConstantUnion *constantValuePointFive = new TConstantUnion();
-    constantValuePointFive->setFConst(0.5f);
-    TIntermConstantUnion *pointFive =
-        new TIntermConstantUnion(constantValuePointFive, *constantType);
+    // Use this new variable instead of 'builtin' everywhere.
+    ReplaceVariable(root, builtin, replacementVar);
 
     // ANGLEUniforms.viewportScaleFactor
-    TIntermSymbol *angleUniformsRef             = new TIntermSymbol(driverUniforms);
-    TConstantUnion *viewportScaleFactorConstant = new TConstantUnion;
-    viewportScaleFactorConstant->setIConst(1);
-    TIntermConstantUnion *viewportScaleFactorIndex =
-        new TIntermConstantUnion(viewportScaleFactorConstant, *StaticType::GetBasic<EbtInt>());
-    TIntermBinary *viewportScaleFactorRef =
-        new TIntermBinary(EOpIndexDirectInterfaceBlock, angleUniformsRef, viewportScaleFactorIndex);
-
-    // Creates a swizzle to ANGLEUniforms.viewportScaleFactor.y
+    TIntermBinary *viewportScaleFactorRef = CreateDriverUniformRef(driverUniforms, 1);
+
+    // Creates a swizzle to ANGLEUniforms.viewportScaleFactor[index]
     TVector<int> viewportScaleSwizzleOffsetY;
-    viewportScaleSwizzleOffsetY.push_back(1);
+    viewportScaleSwizzleOffsetY.push_back(driverUniformSwizzleIndex);
     TIntermSwizzle *viewportScaleY =
         new TIntermSwizzle(viewportScaleFactorRef->deepCopy(), viewportScaleSwizzleOffsetY);
 
-    // Create the expression "(gl_PointCoord.y - 0.5) * ANGLEUniforms.viewportScaleFactor.y +
-    // 0.5
-    TIntermBinary *removePointFive = new TIntermBinary(EOpSub, pointCoordY, pointFive);
-    TIntermBinary *inverseY        = new TIntermBinary(EOpMul, removePointFive, viewportScaleY);
-    TIntermBinary *plusPointFive   = new TIntermBinary(EOpAdd, inverseY, pointFive->deepCopy());
+    // Create the expression "(builtin.y - pivot) * ANGLEUniforms.viewportScaleFactor[index] +
+    // pivot
+    TIntermBinary *removePivot = new TIntermBinary(EOpSub, builtinY, pivot);
+    TIntermBinary *inverseY    = new TIntermBinary(EOpMul, removePivot, viewportScaleY);
+    TIntermBinary *plusPivot   = new TIntermBinary(EOpAdd, inverseY, pivot->deepCopy());
 
-    // Create the new vec2 using the modified Y
+    // Create the corrected variable and copy the value of the original builtin.
     TIntermSequence *sequence = new TIntermSequence();
-    sequence->push_back(pointCoordX);
-    sequence->push_back(plusPointFive);
-    TIntermAggregate *aggregate =
-        TIntermAggregate::CreateConstructor(BuiltInVariable::gl_PointCoord()->getType(), sequence);
-
-    // Use this new variable instead of gl_PointCoord everywhere.
-    ReplaceVariable(root, pointCoord, replacementVar);
+    sequence->push_back(builtinRef);
+    TIntermAggregate *aggregate = TIntermAggregate::CreateConstructor(builtin->getType(), sequence);
+    TIntermBinary *assignment   = new TIntermBinary(EOpInitialize, flippedBuiltinRef, aggregate);
 
-    // Assign this new value to flippedPointCoord
-    TIntermBinary *assignment = new TIntermBinary(EOpInitialize, flippedPointCoordsRef, aggregate);
+    // Create an assignment to the replaced variable's y.
+    TIntermSwizzle *correctedY = new TIntermSwizzle(flippedBuiltinRef, swizzleOffsetY);
+    TIntermBinary *assignToY   = new TIntermBinary(EOpAssign, correctedY, plusPivot);
 
     // Add this assigment at the beginning of the main function
     TIntermFunctionDefinition *main = FindMain(root);
     TIntermSequence *mainSequence   = main->getBody()->getSequence();
+    mainSequence->insert(mainSequence->begin(), assignToY);
     mainSequence->insert(mainSequence->begin(), assignment);
 }
 
@@ -238,13 +241,7 @@ void ReplaceGLDepthRangeWithDriverUniform(TIntermBlock *root,
         symbolTable->findBuiltIn(ImmutableString("gl_DepthRange"), 0));
 
     // ANGLEUniforms.depthRange
-    TIntermSymbol *angleUniformsRef    = new TIntermSymbol(driverUniforms);
-    TConstantUnion *depthRangeConstant = new TConstantUnion;
-    depthRangeConstant->setIConst(2);
-    TIntermConstantUnion *depthRangeIndex =
-        new TIntermConstantUnion(depthRangeConstant, *StaticType::GetBasic<EbtInt>());
-    TIntermBinary *angleEmulatedDepthRangeRef =
-        new TIntermBinary(EOpIndexDirectInterfaceBlock, angleUniformsRef, depthRangeIndex);
+    TIntermBinary *angleEmulatedDepthRangeRef = CreateDriverUniformRef(driverUniforms, 2);
 
     // Use this variable instead of gl_DepthRange everywhere.
     ReplaceVariableWithTyped(root, depthRangeVar, angleEmulatedDepthRangeRef);
@@ -271,10 +268,7 @@ void AppendVertexShaderDepthCorrectionToMain(TIntermBlock *root, TSymbolTable *s
     TIntermSwizzle *positionZ = new TIntermSwizzle(positionRef, swizzleOffsetZ);
 
     // Create a constant "0.5"
-    const TType *constantType     = StaticType::GetBasic<TBasicType::EbtFloat>();
-    TConstantUnion *constantValue = new TConstantUnion();
-    constantValue->setFConst(0.5f);
-    TIntermConstantUnion *oneHalf = new TIntermConstantUnion(constantValue, *constantType);
+    TIntermConstantUnion *oneHalf = CreateConstantFloat(0.5f);
 
     // Create a swizzle to "gl_Position.w"
     TVector<int> swizzleOffsetW;
@@ -424,9 +418,9 @@ void TranslatorVulkan::translate(TIntermBlock *root,
         sink << "};\n";
     }
 
-    const TVariable *driverUniformsVariable = AddDriverUniformsToShader(root, &getSymbolTable());
+    const TVariable *driverUniforms = AddDriverUniformsToShader(root, &getSymbolTable());
 
-    ReplaceGLDepthRangeWithDriverUniform(root, driverUniformsVariable, &getSymbolTable());
+    ReplaceGLDepthRangeWithDriverUniform(root, driverUniforms, &getSymbolTable());
 
     // Declare gl_FragColor and glFragData as webgl_FragColor and webgl_FragData
     // if it's core profile shaders and they are used.
@@ -470,7 +464,25 @@ void TranslatorVulkan::translate(TIntermBlock *root,
 
             if (inputVarying.name == "gl_PointCoord")
             {
-                FlipGLPointCoord(root, driverUniformsVariable, &getSymbolTable());
+                TIntermConstantUnion *pivot = CreateConstantFloat(0.5f);
+                FlipBuiltinVariable(root, driverUniforms, 1, &getSymbolTable(),
+                                    BuiltInVariable::gl_PointCoord(), kFlippedPointCoordName,
+                                    pivot);
+                break;
+            }
+
+            if (inputVarying.name == "gl_FragCoord")
+            {
+                // Create a reference to ANGLEDriverUniforms.viewport.w * 0.5
+                TIntermBinary *viewportRef = CreateDriverUniformRef(driverUniforms, 0);
+                TVector<int> swizzleOffset;
+                swizzleOffset.push_back(3);
+                TIntermSwizzle *viewportSwizzle = new TIntermSwizzle(viewportRef, swizzleOffset);
+                TIntermTyped *oneHalf           = CreateConstantFloat(0.5f);
+                TIntermBinary *pivot = new TIntermBinary(EOpMul, viewportSwizzle, oneHalf);
+
+                FlipBuiltinVariable(root, driverUniforms, 3, &getSymbolTable(),
+                                    BuiltInVariable::gl_FragCoord(), kFlippedFragCoordName, pivot);
                 break;
             }
         }

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -789,7 +789,6 @@ gl::Error ContextVk::syncState(const gl::Context *context, const gl::State::Dirt
             case gl::State::DIRTY_BIT_DRAW_FRAMEBUFFER_BINDING:
             {
                 mDrawFramebuffer = vk::GetImpl(glState.getDrawFramebuffer());
-                ANGLE_TRY(updateDriverUniforms(glState));
                 updateFlipViewportDrawFramebuffer(glState);
                 mPipelineDesc->updateViewport(mDrawFramebuffer, glState.getViewport(),
                                               glState.getNearPlane(), glState.getFarPlane(),
@@ -807,6 +806,7 @@ gl::Error ContextVk::syncState(const gl::Context *context, const gl::State::Dirt
                                                            glState.getDrawFramebuffer());
                 mPipelineDesc->updateStencilBackWriteMask(glState.getDepthStencilState(),
                                                           glState.getDrawFramebuffer());
+                ANGLE_TRY(updateDriverUniforms(glState));
                 break;
             }
             case gl::State::DIRTY_BIT_RENDERBUFFER_BINDING:
@@ -1096,7 +1096,7 @@ angle::Result ContextVk::updateDriverUniforms(const gl::State &glState)
     bool newBufferAllocated = false;
     ANGLE_TRY(mDriverUniformsBuffer.allocate(this, sizeof(DriverUniforms), &ptr, &buffer, &offset,
                                              &newBufferAllocated));
-    float scaleY = isViewportFlipEnabledForDrawFBO() ? 1.0f : -1.0f;
+    float scaleY = isViewportFlipEnabledForDrawFBO() ? -1.0f : 1.0f;
 
     float depthRangeNear = glState.getNearPlane();
     float depthRangeFar  = glState.getFarPlane();
@@ -1107,7 +1107,7 @@ angle::Result ContextVk::updateDriverUniforms(const gl::State &glState)
     *driverUniforms                = {
         {static_cast<float>(glViewport.x), static_cast<float>(glViewport.y),
          static_cast<float>(glViewport.width), static_cast<float>(glViewport.height)},
-        {1.0f, scaleY, 1.0f, 1.0f},
+        {1.0f, -scaleY, 1.0f, scaleY},
         {depthRangeNear, depthRangeFar, depthRangeDiff, 0.0f}};
 
     ANGLE_TRY(mDriverUniformsBuffer.flush(this));

src/tests/gl_tests/GLSLTest.cpp

@@ -4774,6 +4774,116 @@ TEST_P(GLSLTest, IfElseIfAndReturn)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Tests that PointCoord behaves the same betweeen a user FBO and the back buffer.
+TEST_P(GLSLTest, PointCoordConsistency)
+{
+    // On Intel Windows OpenGL drivers PointCoord appears to be flipped when drawing to the
+    // default framebuffer. http://anglebug.com/2805
+    ANGLE_SKIP_TEST_IF(IsIntel() && IsWindows() && IsOpenGL());
+
+    // AMD's OpenGL drivers may have the same issue. http://anglebug.com/1643
+    ANGLE_SKIP_TEST_IF(IsAMD() && IsWindows() && IsOpenGL());
+
+    constexpr char kPointCoordVS[] = R"(attribute vec2 position;
+uniform vec2 viewportSize;
+void main()
+{
+   gl_Position = vec4(position, 0, 1);
+   gl_PointSize = viewportSize.x;
+})";
+
+    constexpr char kPointCoordFS[] = R"(void main()
+{
+    gl_FragColor = vec4(gl_PointCoord.xy, 0, 1);
+})";
+
+    ANGLE_GL_PROGRAM(program, kPointCoordVS, kPointCoordFS);
+    glUseProgram(program);
+
+    GLint uniLoc = glGetUniformLocation(program, "viewportSize");
+    ASSERT_NE(-1, uniLoc);
+    glUniform2f(uniLoc, getWindowWidth(), getWindowHeight());
+
+    // Draw to backbuffer.
+    glDrawArrays(GL_POINTS, 0, 1);
+    ASSERT_GL_NO_ERROR();
+
+    std::vector<GLColor> backbufferData(getWindowWidth() * getWindowHeight());
+    glReadPixels(0, 0, getWindowWidth(), getWindowHeight(), GL_RGBA, GL_UNSIGNED_BYTE,
+                 backbufferData.data());
+
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, getWindowWidth(), getWindowHeight(), 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0);
+    ASSERT_GL_NO_ERROR();
+    ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+    // Draw to user FBO.
+    glDrawArrays(GL_POINTS, 0, 1);
+    ASSERT_GL_NO_ERROR();
+
+    std::vector<GLColor> userFBOData(getWindowWidth() * getWindowHeight());
+    glReadPixels(0, 0, getWindowWidth(), getWindowHeight(), GL_RGBA, GL_UNSIGNED_BYTE,
+                 userFBOData.data());
+
+    ASSERT_GL_NO_ERROR();
+    ASSERT_EQ(userFBOData.size(), backbufferData.size());
+    EXPECT_EQ(userFBOData, backbufferData);
+}
+
+// Tests that FragCoord behaves the same betweeen a user FBO and the back buffer.
+TEST_P(GLSLTest, FragCoordConsistency)
+{
+    constexpr char kFragCoordShader[] = R"(uniform mediump vec2 viewportSize;
+void main()
+{
+    gl_FragColor = vec4(gl_FragCoord.xy / viewportSize, 0, 1);
+})";
+
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), kFragCoordShader);
+    glUseProgram(program);
+
+    GLint uniLoc = glGetUniformLocation(program, "viewportSize");
+    ASSERT_NE(-1, uniLoc);
+    glUniform2f(uniLoc, getWindowWidth(), getWindowHeight());
+
+    // Draw to backbuffer.
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5);
+    ASSERT_GL_NO_ERROR();
+
+    std::vector<GLColor> backbufferData(getWindowWidth() * getWindowHeight());
+    glReadPixels(0, 0, getWindowWidth(), getWindowHeight(), GL_RGBA, GL_UNSIGNED_BYTE,
+                 backbufferData.data());
+
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, getWindowWidth(), getWindowHeight(), 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0);
+    ASSERT_GL_NO_ERROR();
+    ASSERT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+    // Draw to user FBO.
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5);
+    ASSERT_GL_NO_ERROR();
+
+    std::vector<GLColor> userFBOData(getWindowWidth() * getWindowHeight());
+    glReadPixels(0, 0, getWindowWidth(), getWindowHeight(), GL_RGBA, GL_UNSIGNED_BYTE,
+                 userFBOData.data());
+
+    ASSERT_GL_NO_ERROR();
+    ASSERT_EQ(userFBOData.size(), backbufferData.size());
+    EXPECT_EQ(userFBOData, backbufferData);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these
 // tests should be run against.
 ANGLE_INSTANTIATE_TEST(GLSLTest,