Vulkan: Fix a validation bug in glBeginTransformFeedback

src/libANGLE/ProgramExecutable.cpp

@@ -173,6 +173,7 @@ void ProgramExecutable::reset()
 
     mProgramInputs.clear();
     mLinkedTransformFeedbackVaryings.clear();
+    mTransformFeedbackStrides.clear();
     mUniforms.clear();
     mUniformBlocks.clear();
     mActiveUniformBlockBindings.reset();

src/libANGLE/ProgramPipeline.cpp

@@ -241,17 +241,21 @@ void ProgramPipeline::updateExecutableAttributes()
 
 void ProgramPipeline::updateTransformFeedbackMembers()
 {
-    Program *vertexProgram = getShaderProgram(gl::ShaderType::Vertex);
-
-    if (!vertexProgram)
+    ShaderType lastVertexProcessingStage =
+        gl::GetLastPreFragmentStage(getExecutable().getLinkedShaderStages());
+    if (lastVertexProcessingStage == ShaderType::InvalidEnum)
     {
         return;
     }
 
-    const ProgramExecutable &vertexExecutable     = vertexProgram->getExecutable();
-    mState.mExecutable->mTransformFeedbackStrides = vertexExecutable.mTransformFeedbackStrides;
+    Program *shaderProgram = getShaderProgram(lastVertexProcessingStage);
+    ASSERT(shaderProgram);
+
+    const ProgramExecutable &lastPreFragmentExecutable = shaderProgram->getExecutable();
+    mState.mExecutable->mTransformFeedbackStrides =
+        lastPreFragmentExecutable.mTransformFeedbackStrides;
     mState.mExecutable->mLinkedTransformFeedbackVaryings =
-        vertexExecutable.mLinkedTransformFeedbackVaryings;
+        lastPreFragmentExecutable.mLinkedTransformFeedbackVaryings;
 }
 
 void ProgramPipeline::updateShaderStorageBlocks()
@@ -537,7 +541,16 @@ angle::Result ProgramPipeline::link(const Context *context)
         mergedVaryings = GetMergedVaryingsFromShaders(*this, getExecutable());
         // If separable program objects are in use, the set of attributes captured is taken
         // from the program object active on the last vertex processing stage.
-        Program *tfProgram = mState.mPrograms[ShaderType::Geometry];
+        ShaderType lastVertexProcessingStage =
+            gl::GetLastPreFragmentStage(getExecutable().getLinkedShaderStages());
+        if (lastVertexProcessingStage == ShaderType::InvalidEnum)
+        {
+            return angle::Result::Stop;
+        }
+
+        Program *tfProgram = getShaderProgram(lastVertexProcessingStage);
+        ASSERT(tfProgram);
+
         if (!tfProgram)
         {
             tfProgram = mState.mPrograms[ShaderType::Vertex];

src/libANGLE/TransformFeedback.cpp

@@ -134,6 +134,9 @@ angle::Result TransformFeedback::begin(const Context *context,
                                        PrimitiveMode primitiveMode,
                                        Program *program)
 {
+    // TODO: http://anglebug.com/5486: This method should take in as parameter a
+    // ProgramExecutable instead of a Program.
+
     ANGLE_TRY(mImplementation->begin(context, primitiveMode));
     mState.mActive        = true;
     mState.mPrimitiveMode = primitiveMode;
@@ -141,10 +144,14 @@ angle::Result TransformFeedback::begin(const Context *context,
     mState.mVerticesDrawn = 0;
     bindProgram(context, program);
 
-    if (program)
+    // In one of the angle_unittests - "TransformFeedbackTest.SideEffectsOfStartAndStop"
+    // there is a code path where <context> is a nullptr, account for that possiblity.
+    const ProgramExecutable *programExecutable =
+        context ? context->getState().getProgramExecutable() : nullptr;
+    if (programExecutable)
     {
         // Compute the number of vertices we can draw before overflowing the bound buffers.
-        auto strides = program->getTransformFeedbackStrides();
+        auto strides = programExecutable->getTransformFeedbackStrides();
         ASSERT(strides.size() <= mState.mIndexedBuffers.size() && !strides.empty());
         GLsizeiptr minCapacity = std::numeric_limits<GLsizeiptr>::max();
         for (size_t index = 0; index < strides.size(); index++)

src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp

@@ -307,10 +307,9 @@ void TransformFeedbackVk::getBufferOffsets(ContextVk *contextVk,
     }
 
     GLsizeiptr verticesDrawn                = mState.getVerticesDrawn();
-    const std::vector<GLsizei> &bufferStrides =
-        mState.getBoundProgram()->getTransformFeedbackStrides();
     const gl::ProgramExecutable *executable = contextVk->getState().getProgramExecutable();
     ASSERT(executable);
+    const std::vector<GLsizei> &bufferStrides = executable->getTransformFeedbackStrides();
     size_t xfbBufferCount                     = executable->getTransformFeedbackBufferCount();
 
     ASSERT(xfbBufferCount > 0);

src/libANGLE/validationES3.cpp

@@ -2703,17 +2703,18 @@ bool ValidateBeginTransformFeedback(const Context *context, PrimitiveMode primit
         }
     }
 
-    Program *program = context->getState().getLinkedProgram(context);
-
-    if (!program)
+    const ProgramExecutable *programExecutable = context->getState().getProgramExecutable();
+    if (programExecutable)
     {
-        context->validationError(GL_INVALID_OPERATION, kProgramNotBound);
+        if (programExecutable->getLinkedTransformFeedbackVaryings().size() == 0)
+        {
+            context->validationError(GL_INVALID_OPERATION, kNoTransformFeedbackOutputVariables);
             return false;
         }
-
-    if (program->getTransformFeedbackVaryingCount() == 0)
+    }
+    else
     {
-        context->validationError(GL_INVALID_OPERATION, kNoTransformFeedbackOutputVariables);
+        context->validationError(GL_INVALID_OPERATION, kProgramNotBound);
         return false;
     }
 

src/tests/deqp_support/deqp_khr_gles32_test_expectations.txt

@@ -71,8 +71,6 @@
 5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.tessellation_invariance.invariance_rule* = SKIP
 // Crash on assert in libcpp. CTS passes to a function '&vector[0], size', where vector is empty.
 5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.tessellation_shader_point_mode.points_verification = SKIP
-// PPO Linking failure
-5557 VULKAN WIN : KHR-GLES32.core.separable_programs_tf.tessellation_active = FAIL
 
 // Geometry and tessellation failures on the Android and Linux bots, potentially due to missing
 // features from old drivers.  On up-to-date Linux, the expectations should be as above for windows.

src/tests/gl_tests/ProgramPipelineTest.cpp

@@ -87,6 +87,45 @@ class ProgramPipelineTest31 : public ProgramPipelineTest
     GLuint mPipeline;
 };
 
+class ProgramPipelineXFBTest31 : public ProgramPipelineTest31
+{
+  protected:
+    void testSetUp() override
+    {
+        glGenBuffers(1, &mTransformFeedbackBuffer);
+        glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, mTransformFeedbackBuffer);
+        glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, mTransformFeedbackBufferSize, nullptr,
+                     GL_STATIC_DRAW);
+
+        glGenTransformFeedbacks(1, &mTransformFeedback);
+
+        ASSERT_GL_NO_ERROR();
+    }
+    void testTearDown() override
+    {
+        if (mTransformFeedbackBuffer != 0)
+        {
+            glDeleteBuffers(1, &mTransformFeedbackBuffer);
+            mTransformFeedbackBuffer = 0;
+        }
+
+        if (mTransformFeedback != 0)
+        {
+            glDeleteTransformFeedbacks(1, &mTransformFeedback);
+            mTransformFeedback = 0;
+        }
+    }
+
+    void bindProgramPipelineWithXFBVaryings(const GLchar *vertString,
+                                            const GLchar *fragStringconst,
+                                            const std::vector<std::string> &tfVaryings,
+                                            GLenum bufferMode);
+
+    static const size_t mTransformFeedbackBufferSize = 1 << 24;
+    GLuint mTransformFeedbackBuffer;
+    GLuint mTransformFeedback;
+};
+
 void ProgramPipelineTest31::bindProgramPipeline(const GLchar *vertString, const GLchar *fragString)
 {
     mVertProg = glCreateShaderProgramv(GL_VERTEX_SHADER, 1, &vertString);
@@ -105,6 +144,41 @@ void ProgramPipelineTest31::bindProgramPipeline(const GLchar *vertString, const
     EXPECT_GL_NO_ERROR();
 }
 
+void ProgramPipelineXFBTest31::bindProgramPipelineWithXFBVaryings(
+    const GLchar *vertString,
+    const GLchar *fragString,
+    const std::vector<std::string> &tfVaryings,
+    GLenum bufferMode)
+{
+    mVertProg = glCreateShaderProgramv(GL_VERTEX_SHADER, 1, &vertString);
+    ASSERT_NE(mVertProg, 0u);
+    mFragProg = glCreateShaderProgramv(GL_FRAGMENT_SHADER, 1, &fragString);
+    ASSERT_NE(mFragProg, 0u);
+
+    if (tfVaryings.size() > 0)
+    {
+        std::vector<const char *> constCharTFVaryings;
+
+        for (const std::string &transformFeedbackVarying : tfVaryings)
+        {
+            constCharTFVaryings.push_back(transformFeedbackVarying.c_str());
+        }
+
+        glTransformFeedbackVaryings(mVertProg, static_cast<GLsizei>(tfVaryings.size()),
+                                    &constCharTFVaryings[0], bufferMode);
+        glLinkProgram(mVertProg);
+    }
+    // Generate a program pipeline and attach the programs to their respective stages
+    glGenProgramPipelines(1, &mPipeline);
+    EXPECT_GL_NO_ERROR();
+    glUseProgramStages(mPipeline, GL_VERTEX_SHADER_BIT, mVertProg);
+    EXPECT_GL_NO_ERROR();
+    glUseProgramStages(mPipeline, GL_FRAGMENT_SHADER_BIT, mFragProg);
+    EXPECT_GL_NO_ERROR();
+    glBindProgramPipeline(mPipeline);
+    EXPECT_GL_NO_ERROR();
+}
+
 // Test generate or delete program pipeline.
 TEST_P(ProgramPipelineTest31, GenOrDeleteProgramPipelineTest)
 {
@@ -678,10 +752,71 @@ TEST_P(ProgramPipelineTest31, VaryingIOBlockSeparableProgram)
         })";
 
     bindProgramPipeline(kVS, kFS);
+    drawQuadWithPPO("inputAttribute", 0.5f, 1.0f);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+}
+
+// Test that a shader IO block varying with separable program links
+// successfully.
+TEST_P(ProgramPipelineXFBTest31, VaryingIOBlockSeparableProgramWithXFB)
+{
+    // Only the Vulkan backend supports PPOs
+    ANGLE_SKIP_TEST_IF(!IsVulkan());
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_shader_io_blocks"));
+    // http://anglebug.com/5486
+    ANGLE_SKIP_TEST_IF(IsVulkan());
+
+    constexpr char kVS[] =
+        R"(#version 310 es
+        #extension GL_EXT_shader_io_blocks : require
+
+        precision highp float;
+        in vec4 inputAttribute;
+        out Block_inout { vec4 value; } user_out;
+
+        void main()
+        {
+            gl_Position    = inputAttribute;
+            user_out.value = vec4(4.0, 5.0, 6.0, 7.0);
+        })";
+
+    constexpr char kFS[] =
+        R"(#version 310 es
+        #extension GL_EXT_shader_io_blocks : require
+
+        precision highp float;
+        layout(location = 0) out mediump vec4 color;
+        in Block_inout { vec4 value; } user_in;
+
+        void main()
+        {
+            color = vec4(1, 0, 0, 1);
+        })";
+    std::vector<std::string> tfVaryings;
+    tfVaryings.push_back("Block_inout.value");
+    bindProgramPipelineWithXFBVaryings(kVS, kFS, tfVaryings, GL_INTERLEAVED_ATTRIBS);
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
 
+    glBeginTransformFeedback(GL_TRIANGLES);
     drawQuadWithPPO("inputAttribute", 0.5f, 1.0f);
+    glEndTransformFeedback();
+
     ASSERT_GL_NO_ERROR();
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+
+    void *mappedBuffer =
+        glMapBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, sizeof(float) * 4, GL_MAP_READ_BIT);
+    ASSERT_NE(nullptr, mappedBuffer);
+
+    float *mappedFloats = static_cast<float *>(mappedBuffer);
+    for (unsigned int cnt = 0; cnt < 4; ++cnt)
+    {
+        EXPECT_EQ(4 + cnt, mappedFloats[cnt]);
+    }
+    glUnmapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER);
+
+    EXPECT_GL_NO_ERROR();
 }
 
 // Test modifying a shader and re-linking it updates the PPO too
@@ -920,4 +1055,7 @@ ANGLE_INSTANTIATE_TEST_ES3_AND_ES31(ProgramPipelineTest);
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ProgramPipelineTest31);
 ANGLE_INSTANTIATE_TEST_ES31(ProgramPipelineTest31);
 
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ProgramPipelineXFBTest31);
+ANGLE_INSTANTIATE_TEST_ES31(ProgramPipelineXFBTest31);
+
 }  // namespace

src/tests/test_utils/ANGLETest.cpp

@@ -873,7 +873,12 @@ void ANGLETestBase::drawQuadPPO(GLuint vertProgram,
                                 const GLfloat positionAttribZ,
                                 const GLfloat positionAttribXYScale)
 {
+    GLint activeProgram = 0;
+    glGetIntegerv(GL_CURRENT_PROGRAM, &activeProgram);
+    if (activeProgram)
+    {
         glUseProgram(0);
+    }
 
     std::array<Vector3, 6> quadVertices = GetQuadVertices();
 
@@ -894,6 +899,11 @@ void ANGLETestBase::drawQuadPPO(GLuint vertProgram,
 
     glDisableVertexAttribArray(positionLocation);
     glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, nullptr);
+
+    if (activeProgram)
+    {
+        glUseProgram(static_cast<GLuint>(activeProgram));
+    }
 }
 
 void ANGLETestBase::drawIndexedQuad(GLuint program,