Cache valid draw modes with transform feedback.

src/libANGLE/Context.cpp

@@ -8355,6 +8355,7 @@ void StateCache::onQueryChange(Context *context)
 void StateCache::onActiveTransformFeedbackChange(Context *context)
 {
     updateBasicDrawStatesError();
+    updateValidDrawModes(context);
 }
 
 void StateCache::onUniformBufferStateChange(Context *context)
@@ -8367,33 +8368,79 @@ void StateCache::onBufferBindingChange(Context *context)
     updateBasicDrawStatesError();
 }
 
+void StateCache::setValidDrawModes(bool pointsOK,
+                                   bool linesOK,
+                                   bool trisOK,
+                                   bool lineAdjOK,
+                                   bool triAdjOK)
+{
+    mCachedValidDrawModes[PrimitiveMode::Points]                 = pointsOK;
+    mCachedValidDrawModes[PrimitiveMode::Lines]                  = linesOK;
+    mCachedValidDrawModes[PrimitiveMode::LineStrip]              = linesOK;
+    mCachedValidDrawModes[PrimitiveMode::LineLoop]               = linesOK;
+    mCachedValidDrawModes[PrimitiveMode::Triangles]              = trisOK;
+    mCachedValidDrawModes[PrimitiveMode::TriangleFan]            = trisOK;
+    mCachedValidDrawModes[PrimitiveMode::TriangleStrip]          = trisOK;
+    mCachedValidDrawModes[PrimitiveMode::LinesAdjacency]         = lineAdjOK;
+    mCachedValidDrawModes[PrimitiveMode::LineStripAdjacency]     = lineAdjOK;
+    mCachedValidDrawModes[PrimitiveMode::TrianglesAdjacency]     = triAdjOK;
+    mCachedValidDrawModes[PrimitiveMode::TriangleStripAdjacency] = triAdjOK;
+}
+
 void StateCache::updateValidDrawModes(Context *context)
 {
-    Program *program = context->getGLState().getProgram();
+    const State &state = context->getGLState();
+    Program *program   = state.getProgram();
+
+    TransformFeedback *curTransformFeedback = state.getCurrentTransformFeedback();
+    if (curTransformFeedback && curTransformFeedback->isActive() &&
+        !curTransformFeedback->isPaused())
+    {
+        // ES Spec 3.0 validation text:
+        // When transform feedback is active and not paused, all geometric primitives generated must
+        // match the value of primitiveMode passed to BeginTransformFeedback. The error
+        // INVALID_OPERATION is generated by DrawArrays and DrawArraysInstanced if mode is not
+        // identical to primitiveMode. The error INVALID_OPERATION is also generated by
+        // DrawElements, DrawElementsInstanced, and DrawRangeElements while transform feedback is
+        // active and not paused, regardless of mode. Any primitive type may be used while transform
+        // feedback is paused.
+        if (!context->getExtensions().geometryShader)
+        {
+            mCachedValidDrawModes.fill(false);
+            mCachedValidDrawModes[curTransformFeedback->getPrimitiveMode()] = true;
+            return;
+        }
+
+        // EXT_geometry_shader validation text:
+        // When transform feedback is active and not paused, all geometric primitives generated must
+        // be compatible with the value of <primitiveMode> passed to BeginTransformFeedback. If a
+        // geometry shader is active, the type of primitive emitted by that shader is used instead
+        // of the <mode> parameter passed to drawing commands for the purposes of this error check.
+        // Any primitive type may be used while transform feedback is paused.
+        bool pointsOK = curTransformFeedback->getPrimitiveMode() == PrimitiveMode::Points;
+        bool linesOK  = curTransformFeedback->getPrimitiveMode() == PrimitiveMode::Lines;
+        bool trisOK   = curTransformFeedback->getPrimitiveMode() == PrimitiveMode::Triangles;
+
+        setValidDrawModes(pointsOK, linesOK, trisOK, false, false);
+        return;
+    }
+
     if (!program || !program->hasLinkedShaderStage(ShaderType::Geometry))
     {
         mCachedValidDrawModes = kValidBasicDrawModes;
+        return;
     }
-    else
-    {
-        ASSERT(program && program->hasLinkedShaderStage(ShaderType::Geometry));
 
-        PrimitiveMode gsMode = program->getGeometryShaderInputPrimitiveType();
+    ASSERT(program && program->hasLinkedShaderStage(ShaderType::Geometry));
+    PrimitiveMode gsMode = program->getGeometryShaderInputPrimitiveType();
 
-        mCachedValidDrawModes = {{
-            {PrimitiveMode::Points, gsMode == PrimitiveMode::Points},
-            {PrimitiveMode::Lines, gsMode == PrimitiveMode::Lines},
-            {PrimitiveMode::LineLoop, gsMode == PrimitiveMode::Lines},
-            {PrimitiveMode::LineStrip, gsMode == PrimitiveMode::Lines},
-            {PrimitiveMode::Triangles, gsMode == PrimitiveMode::Triangles},
-            {PrimitiveMode::TriangleStrip, gsMode == PrimitiveMode::Triangles},
-            {PrimitiveMode::TriangleFan, gsMode == PrimitiveMode::Triangles},
-            {PrimitiveMode::LinesAdjacency, gsMode == PrimitiveMode::LinesAdjacency},
-            {PrimitiveMode::LineStripAdjacency, gsMode == PrimitiveMode::LinesAdjacency},
-            {PrimitiveMode::TrianglesAdjacency, gsMode == PrimitiveMode::TrianglesAdjacency},
-            {PrimitiveMode::TriangleStripAdjacency, gsMode == PrimitiveMode::TrianglesAdjacency},
-        }};
-    }
+    bool pointsOK  = gsMode == PrimitiveMode::Points;
+    bool linesOK   = gsMode == PrimitiveMode::Lines;
+    bool trisOK    = gsMode == PrimitiveMode::Triangles;
+    bool lineAdjOK = gsMode == PrimitiveMode::LinesAdjacency;
+    bool triAdjOK  = gsMode == PrimitiveMode::TrianglesAdjacency;
+
+    setValidDrawModes(pointsOK, linesOK, trisOK, lineAdjOK, triAdjOK);
 }
 
 void StateCache::updateValidBindTextureTypes(Context *context)

src/libANGLE/Context.h

@@ -130,7 +130,7 @@ class StateCache final : angle::NonCopyable
     // 9. onDefaultVertexAttributeChange.
     // 10. onActiveTextureChange.
     // 11. onQueryChange.
-    // 12. onTransformFeedbackChange.
+    // 12. onActiveTransformFeedbackChange.
     // 13. onUniformBufferStateChange.
     // 14. onBufferBindingChange.
     intptr_t getBasicDrawStatesError(Context *context) const
@@ -145,6 +145,7 @@ class StateCache final : angle::NonCopyable
 
     // Places that can trigger updateValidDrawModes:
     // 1. onProgramExecutableChange.
+    // 2. onActiveTransformFeedbackChange.
     bool isValidDrawMode(PrimitiveMode primitiveMode) const
     {
         return mCachedValidDrawModes[primitiveMode];
@@ -189,6 +190,8 @@ class StateCache final : angle::NonCopyable
     void updateValidBindTextureTypes(Context *context);
     void updateValidDrawElementsTypes(Context *context);
 
+    void setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, bool lineAdjOK, bool triAdjOK);
+
     intptr_t getBasicDrawStatesErrorImpl(Context *context) const;
 
     static constexpr intptr_t kInvalidPointer = 1;

src/libANGLE/validationES.cpp

@@ -2776,6 +2776,19 @@ const char *ValidateDrawStates(Context *context)
 
 bool ValidateDrawMode(Context *context, PrimitiveMode mode)
 {
+    const State &state                      = context->getGLState();
+    TransformFeedback *curTransformFeedback = state.getCurrentTransformFeedback();
+    if (curTransformFeedback && curTransformFeedback->isActive() &&
+        !curTransformFeedback->isPaused())
+    {
+        if (!ValidateTransformFeedbackPrimitiveMode(context,
+                                                    curTransformFeedback->getPrimitiveMode(), mode))
+        {
+            context->validationError(GL_INVALID_OPERATION, kInvalidDrawModeTransformFeedback);
+            return false;
+        }
+    }
+
     const Extensions &extensions = context->getExtensions();
 
     switch (mode)
@@ -2807,8 +2820,6 @@ bool ValidateDrawMode(Context *context, PrimitiveMode mode)
     // If we are running GLES1, there is no current program.
     if (context->getClientVersion() >= Version(2, 0))
     {
-        const State &state = context->getGLState();
-
         Program *program = state.getLinkedProgram(context);
         ASSERT(program);
 
@@ -2850,13 +2861,6 @@ bool ValidateDrawArraysCommon(Context *context,
     if (curTransformFeedback && curTransformFeedback->isActive() &&
         !curTransformFeedback->isPaused())
     {
-        if (!ValidateTransformFeedbackPrimitiveMode(context,
-                                                    curTransformFeedback->getPrimitiveMode(), mode))
-        {
-            context->validationError(GL_INVALID_OPERATION, kInvalidDrawModeTransformFeedback);
-            return false;
-        }
-
         if (!curTransformFeedback->checkBufferSpaceForDraw(count, primcount))
         {
             context->validationError(GL_INVALID_OPERATION, kTransformFeedbackBufferTooSmall);
@@ -2931,16 +2935,7 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
     {
         // EXT_geometry_shader allows transform feedback to work with all draw commands.
         // [EXT_geometry_shader] Section 12.1, "Transform Feedback"
-        if (context->getExtensions().geometryShader)
-        {
-            if (!ValidateTransformFeedbackPrimitiveMode(
-                    context, curTransformFeedback->getPrimitiveMode(), mode))
-            {
-                context->validationError(GL_INVALID_OPERATION, kInvalidDrawModeTransformFeedback);
-                return false;
-            }
-        }
-        else
+        if (!context->getExtensions().geometryShader)
         {
             // It is an invalid operation to call DrawElements, DrawRangeElements or
             // DrawElementsInstanced while transform feedback is active, (3.0.2, section 2.14, pg
@@ -2949,6 +2944,8 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
                                      kUnsupportedDrawModeForTransformFeedback);
             return false;
         }
+
+        // Note that we are missing overflow checks for the transform feedback buffers.
     }
 
     const VertexArray *vao     = state.getVertexArray();

src/tests/gl_tests/StateChangeTest.cpp

@@ -2857,6 +2857,90 @@ void main()
     EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "Simultaneous pack buffer binding should fail";
 }
 
+// Tests that we retain the correct draw mode settings with transform feedback changes.
+TEST_P(ValidationStateChangeTest, TransformFeedbackDrawModes)
+{
+    ANGLE_SKIP_TEST_IF(IsAMD() && IsOSX());
+
+    std::vector<std::string> tfVaryings = {"gl_Position"};
+    ANGLE_GL_PROGRAM_TRANSFORM_FEEDBACK(program, essl3_shaders::vs::Simple(),
+                                        essl3_shaders::fs::Red(), tfVaryings,
+                                        GL_INTERLEAVED_ATTRIBS);
+    glUseProgram(program);
+
+    std::vector<Vector4> positionData;
+    for (const Vector3 &quadVertex : GetQuadVertices())
+    {
+        positionData.emplace_back(quadVertex.x(), quadVertex.y(), quadVertex.z(), 1.0f);
+    }
+
+    GLBuffer arrayBuffer;
+    glBindBuffer(GL_ARRAY_BUFFER, arrayBuffer);
+    glBufferData(GL_ARRAY_BUFFER, positionData.size() * sizeof(Vector4), positionData.data(),
+                 GL_STATIC_DRAW);
+
+    GLint positionLoc = glGetAttribLocation(program, essl3_shaders::PositionAttrib());
+    ASSERT_NE(-1, positionLoc);
+
+    glVertexAttribPointer(positionLoc, 4, GL_FLOAT, GL_FALSE, 0, nullptr);
+    glEnableVertexAttribArray(positionLoc);
+
+    // Set up transform feedback.
+    GLTransformFeedback transformFeedback;
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, transformFeedback);
+
+    constexpr size_t kTransformFeedbackSize = 6 * sizeof(Vector4);
+
+    GLBuffer transformFeedbackBuffer;
+    glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, transformFeedbackBuffer);
+    glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kTransformFeedbackSize * 2, nullptr, GL_STATIC_DRAW);
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, transformFeedbackBuffer);
+
+    GLTransformFeedback pointsXFB;
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, pointsXFB);
+    GLBuffer pointsXFBBuffer;
+    glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, pointsXFBBuffer);
+    glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, 1024, nullptr, GL_STREAM_DRAW);
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, pointsXFBBuffer);
+
+    // Begin TRIANGLES, switch to paused POINTS, should be valid.
+    glBeginTransformFeedback(GL_POINTS);
+    glPauseTransformFeedback();
+    ASSERT_GL_NO_ERROR() << "Starting point transform feedback should succeed";
+
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, transformFeedback);
+    glBeginTransformFeedback(GL_TRIANGLES);
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    EXPECT_GL_NO_ERROR() << "Triangle rendering should succeed";
+    glDrawArrays(GL_POINTS, 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "Point rendering should fail";
+    glDrawArrays(GL_LINES, 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "Lines rendering should fail";
+    glPauseTransformFeedback();
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, pointsXFB);
+    glResumeTransformFeedback();
+    glDrawArrays(GL_POINTS, 0, 6);
+    EXPECT_GL_NO_ERROR() << "Point rendering should succeed";
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "Triangle rendering should fail";
+    glDrawArrays(GL_LINES, 0, 6);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION) << "Lines rendering should fail";
+
+    glEndTransformFeedback();
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, transformFeedback);
+    glEndTransformFeedback();
+    ASSERT_GL_NO_ERROR() << "Ending transform feeback should pass";
+
+    glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0);
+
+    glDrawArrays(GL_POINTS, 0, 6);
+    EXPECT_GL_NO_ERROR() << "Point rendering should succeed";
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    EXPECT_GL_NO_ERROR() << "Triangle rendering should succeed";
+    glDrawArrays(GL_LINES, 0, 6);
+    EXPECT_GL_NO_ERROR() << "Line rendering should succeed";
+}
+
 // Tests a valid rendering setup with two textures. Followed by a draw with conflicting samplers.
 TEST_P(ValidationStateChangeTest, TextureConflict)
 {