Cache common DrawElements states.

src/libANGLE/Context.cpp

@@ -8191,7 +8191,8 @@ StateCache::StateCache()
     : mCachedHasAnyEnabledClientAttrib(false),
       mCachedNonInstancedVertexElementLimit(0),
       mCachedInstancedVertexElementLimit(0),
-      mCachedBasicDrawStatesError(kInvalidPointer)
+      mCachedBasicDrawStatesError(kInvalidPointer),
+      mCachedBasicDrawElementsError(kInvalidPointer)
 {}
 
 StateCache::~StateCache() = default;
@@ -8201,6 +8202,8 @@ void StateCache::initialize(Context *context)
     updateValidDrawModes(context);
     updateValidBindTextureTypes(context);
     updateValidDrawElementsTypes(context);
+    updateBasicDrawStatesError();
+    updateBasicDrawElementsError();
 }
 
 void StateCache::updateActiveAttribsMask(Context *context)
@@ -8277,6 +8280,11 @@ void StateCache::updateBasicDrawStatesError()
     mCachedBasicDrawStatesError = kInvalidPointer;
 }
 
+void StateCache::updateBasicDrawElementsError()
+{
+    mCachedBasicDrawElementsError = kInvalidPointer;
+}
+
 intptr_t StateCache::getBasicDrawStatesErrorImpl(Context *context) const
 {
     ASSERT(mCachedBasicDrawStatesError == kInvalidPointer);
@@ -8284,6 +8292,13 @@ intptr_t StateCache::getBasicDrawStatesErrorImpl(Context *context) const
     return mCachedBasicDrawStatesError;
 }
 
+intptr_t StateCache::getBasicDrawElementsErrorImpl(Context *context) const
+{
+    ASSERT(mCachedBasicDrawElementsError == kInvalidPointer);
+    mCachedBasicDrawElementsError = reinterpret_cast<intptr_t>(ValidateDrawElementsStates(context));
+    return mCachedBasicDrawElementsError;
+}
+
 void StateCache::onVertexArrayBindingChange(Context *context)
 {
     updateActiveAttribsMask(context);
@@ -8320,6 +8335,7 @@ void StateCache::onVertexArrayStateChange(Context *context)
 void StateCache::onVertexArrayBufferStateChange(Context *context)
 {
     updateBasicDrawStatesError();
+    updateBasicDrawElementsError();
 }
 
 void StateCache::onGLES1ClientStateChange(Context *context)
@@ -8360,6 +8376,7 @@ void StateCache::onQueryChange(Context *context)
 void StateCache::onActiveTransformFeedbackChange(Context *context)
 {
     updateBasicDrawStatesError();
+    updateBasicDrawElementsError();
     updateValidDrawModes(context);
 }
 
@@ -8371,6 +8388,7 @@ void StateCache::onUniformBufferStateChange(Context *context)
 void StateCache::onBufferBindingChange(Context *context)
 {
     updateBasicDrawStatesError();
+    updateBasicDrawElementsError();
 }
 
 void StateCache::setValidDrawModes(bool pointsOK,

src/libANGLE/Context.h

@@ -143,6 +143,20 @@ class StateCache final : angle::NonCopyable
         return getBasicDrawStatesErrorImpl(context);
     }
 
+    // Places that can trigger updateBasicDrawElementsError:
+    // 1. onActiveTransformFeedbackChange.
+    // 2. onVertexArrayBufferStateChange.
+    // 3. onBufferBindingChange.
+    intptr_t getBasicDrawElementsError(Context *context) const
+    {
+        if (mCachedBasicDrawElementsError != kInvalidPointer)
+        {
+            return mCachedBasicDrawElementsError;
+        }
+
+        return getBasicDrawElementsErrorImpl(context);
+    }
+
     // Places that can trigger updateValidDrawModes:
     // 1. onProgramExecutableChange.
     // 2. onActiveTransformFeedbackChange.
@@ -185,14 +199,16 @@ class StateCache final : angle::NonCopyable
     // Cache update functions.
     void updateActiveAttribsMask(Context *context);
     void updateVertexElementLimits(Context *context);
-    void updateBasicDrawStatesError();
     void updateValidDrawModes(Context *context);
     void updateValidBindTextureTypes(Context *context);
     void updateValidDrawElementsTypes(Context *context);
+    void updateBasicDrawStatesError();
+    void updateBasicDrawElementsError();
 
     void setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, bool lineAdjOK, bool triAdjOK);
 
     intptr_t getBasicDrawStatesErrorImpl(Context *context) const;
+    intptr_t getBasicDrawElementsErrorImpl(Context *context) const;
 
     static constexpr intptr_t kInvalidPointer = 1;
 
@@ -203,6 +219,7 @@ class StateCache final : angle::NonCopyable
     GLint64 mCachedNonInstancedVertexElementLimit;
     GLint64 mCachedInstancedVertexElementLimit;
     mutable intptr_t mCachedBasicDrawStatesError;
+    mutable intptr_t mCachedBasicDrawElementsError;
 
     // Reserve an extra slot at the end of these maps for invalid enum.
     angle::PackedEnumMap<PrimitiveMode, bool, angle::EnumSize<PrimitiveMode>() + 1>

src/libANGLE/VertexArray.cpp

@@ -438,8 +438,8 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
             if (!IsElementArrayBufferSubjectIndex(index))
             {
                 updateCachedMappedArrayBuffers(&mState.mVertexBindings[index]);
-                onStateChange(context, angle::SubjectMessage::RESOURCE_MAPPED);
             }
+            onStateChange(context, angle::SubjectMessage::RESOURCE_MAPPED);
             break;
 
         case angle::SubjectMessage::RESOURCE_UNMAPPED:
@@ -448,8 +448,8 @@ void VertexArray::onSubjectStateChange(const gl::Context *context,
             if (!IsElementArrayBufferSubjectIndex(index))
             {
                 updateCachedMappedArrayBuffers(&mState.mVertexBindings[index]);
-                onStateChange(context, angle::SubjectMessage::RESOURCE_UNMAPPED);
             }
+            onStateChange(context, angle::SubjectMessage::RESOURCE_UNMAPPED);
             break;
 
         default:

src/libANGLE/validationES.cpp

@@ -2926,7 +2926,21 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
         return false;
     }
 
-    // TODO(jmadill): Cache all of these into fast checks. http://anglebug.com/2966
+    intptr_t drawElementsError = context->getStateCache().getBasicDrawElementsError(context);
+    if (drawElementsError)
+    {
+        // All errors from ValidateDrawElementsStates return INVALID_OPERATION.
+        const char *errorMessage = reinterpret_cast<const char *>(drawElementsError);
+        context->validationError(GL_INVALID_OPERATION, errorMessage);
+        return false;
+    }
+
+    // Note that we are missing overflow checks for active transform feedback buffers.
+    return true;
+}
+
+const char *ValidateDrawElementsStates(Context *context)
+{
     const State &state = context->getGLState();
 
     TransformFeedback *curTransformFeedback = state.getCurrentTransformFeedback();
@@ -2940,12 +2954,8 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
             // It is an invalid operation to call DrawElements, DrawRangeElements or
             // DrawElementsInstanced while transform feedback is active, (3.0.2, section 2.14, pg
             // 86)
-            context->validationError(GL_INVALID_OPERATION,
-                                     kUnsupportedDrawModeForTransformFeedback);
-            return false;
+            return kUnsupportedDrawModeForTransformFeedback;
         }
-
-        // Note that we are missing overflow checks for the transform feedback buffers.
     }
 
     const VertexArray *vao     = state.getVertexArray();
@@ -2957,9 +2967,7 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
         {
             if (elementArrayBuffer->isBoundForTransformFeedbackAndOtherUse())
             {
-                context->validationError(GL_INVALID_OPERATION,
-                                         kElementArrayBufferBoundForTransformFeedback);
-                return false;
+                return kElementArrayBufferBoundForTransformFeedback;
             }
         }
         else if (elementArrayBuffer->isMapped())
@@ -2967,8 +2975,7 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
             // WebGL buffers cannot be mapped/unmapped because the MapBufferRange,
             // FlushMappedBufferRange, and UnmapBuffer entry points are removed from the
             // WebGL 2.0 API. https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.14
-            context->validationError(GL_INVALID_OPERATION, kBufferMapped);
-            return false;
+            return kBufferMapped;
         }
     }
     else
@@ -2979,12 +2986,11 @@ bool ValidateDrawElementsBase(Context *context, PrimitiveMode mode, DrawElements
         if (!context->getGLState().areClientArraysEnabled() ||
             context->getExtensions().webglCompatibility)
         {
-            context->validationError(GL_INVALID_OPERATION, kMustHaveElementArrayBinding);
-            return false;
+            return kMustHaveElementArrayBinding;
         }
     }
 
-    return true;
+    return nullptr;
 }
 
 bool ValidateDrawElementsCommon(Context *context,

src/libANGLE/validationES.h

@@ -287,6 +287,7 @@ bool ValidateCopyTexImageParametersBase(Context *context,
                                         Format *textureFormatOut);
 
 bool ValidateDrawMode(Context *context, PrimitiveMode mode);
+const char *ValidateDrawElementsStates(Context *context);
 
 ANGLE_INLINE bool ValidateDrawBase(Context *context, PrimitiveMode mode, GLsizei count)
 {

src/tests/gl_tests/StateChangeTest.cpp

@@ -3038,6 +3038,47 @@ void main()
     glDrawArrays(GL_TRIANGLES, 0, 6);
     EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }
+
+// Tests that mapping the element array buffer triggers errors.
+TEST_P(ValidationStateChangeTest, MapElementArrayBuffer)
+{
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), essl3_shaders::fs::Red());
+    glUseProgram(program);
+
+    std::array<GLushort, 6> quadIndices = GetQuadIndices();
+    std::array<Vector3, 4> quadVertices = GetIndexedQuadVertices();
+
+    GLsizei elementBufferSize = sizeof(quadIndices[0]) * quadIndices.size();
+
+    GLBuffer elementArrayBuffer;
+    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, elementArrayBuffer);
+    glBufferData(GL_ELEMENT_ARRAY_BUFFER, elementBufferSize, quadIndices.data(), GL_STATIC_DRAW);
+
+    GLBuffer arrayBuffer;
+    glBindBuffer(GL_ARRAY_BUFFER, arrayBuffer);
+    glBufferData(GL_ARRAY_BUFFER, sizeof(quadVertices[0]) * quadVertices.size(),
+                 quadVertices.data(), GL_STATIC_DRAW);
+
+    GLint positionLoc = glGetAttribLocation(program, essl3_shaders::PositionAttrib());
+    ASSERT_NE(-1, positionLoc);
+    glVertexAttribPointer(positionLoc, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
+    glEnableVertexAttribArray(positionLoc);
+
+    glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, nullptr);
+    ASSERT_GL_NO_ERROR();
+
+    void *ptr = glMapBufferRange(GL_ELEMENT_ARRAY_BUFFER, 0, elementBufferSize, GL_MAP_READ_BIT);
+    ASSERT_NE(nullptr, ptr);
+
+    glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, nullptr);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+    glUnmapBuffer(GL_ELEMENT_ARRAY_BUFFER);
+
+    glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, nullptr);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+}
 }  // anonymous namespace
 
 ANGLE_INSTANTIATE_TEST(StateChangeTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL(), ES2_VULKAN());

src/tests/test_utils/ANGLETest.cpp

@@ -42,7 +42,7 @@ GLubyte ColorDenorm(float colorValue)
     return static_cast<GLubyte>(colorValue * 255.0f);
 }
 
-void TestPlatform_logError(angle::PlatformMethods *platform, const char *errorMessage)
+void TestPlatform_logError(PlatformMethods *platform, const char *errorMessage)
 {
     auto *testPlatformContext = static_cast<TestPlatformContext *>(platform->context);
     if (testPlatformContext->ignoreMessages)
@@ -51,7 +51,7 @@ void TestPlatform_logError(angle::PlatformMethods *platform, const char *errorMe
     FAIL() << errorMessage;
 }
 
-void TestPlatform_logWarning(angle::PlatformMethods *platform, const char *warningMessage)
+void TestPlatform_logWarning(PlatformMethods *platform, const char *warningMessage)
 {
     auto *testPlatformContext = static_cast<TestPlatformContext *>(platform->context);
     if (testPlatformContext->ignoreMessages)
@@ -60,17 +60,16 @@ void TestPlatform_logWarning(angle::PlatformMethods *platform, const char *warni
     std::cerr << "Warning: " << warningMessage << std::endl;
 }
 
-void TestPlatform_logInfo(angle::PlatformMethods *platform, const char *infoMessage)
+void TestPlatform_logInfo(PlatformMethods *platform, const char *infoMessage)
 {
     auto *testPlatformContext = static_cast<TestPlatformContext *>(platform->context);
     if (testPlatformContext->ignoreMessages)
         return;
 
-    angle::WriteDebugMessage("%s\n", infoMessage);
+    WriteDebugMessage("%s\n", infoMessage);
 }
 
-void TestPlatform_overrideWorkaroundsD3D(angle::PlatformMethods *platform,
-                                         WorkaroundsD3D *workaroundsD3D)
+void TestPlatform_overrideWorkaroundsD3D(PlatformMethods *platform, WorkaroundsD3D *workaroundsD3D)
 {
     auto *testPlatformContext = static_cast<TestPlatformContext *>(platform->context);
     if (testPlatformContext->currentTest)
@@ -79,8 +78,7 @@ void TestPlatform_overrideWorkaroundsD3D(angle::PlatformMethods *platform,
     }
 }
 
-void TestPlatform_overrideFeaturesVk(angle::PlatformMethods *platform,
-                                     FeaturesVk *workaroundsVulkan)
+void TestPlatform_overrideFeaturesVk(PlatformMethods *platform, FeaturesVk *workaroundsVulkan)
 {
     auto *testPlatformContext = static_cast<TestPlatformContext *>(platform->context);
     if (testPlatformContext->currentTest)
@@ -89,17 +87,23 @@ void TestPlatform_overrideFeaturesVk(angle::PlatformMethods *platform,
     }
 }
 
-std::array<angle::Vector3, 4> GetIndexedQuadVertices()
-{
-    std::array<angle::Vector3, 4> vertices;
-    vertices[0] = angle::Vector3(-1.0f, 1.0f, 0.5f);
-    vertices[1] = angle::Vector3(-1.0f, -1.0f, 0.5f);
-    vertices[2] = angle::Vector3(1.0f, -1.0f, 0.5f);
-    vertices[3] = angle::Vector3(1.0f, 1.0f, 0.5f);
-    return vertices;
-}
+const std::array<Vector3, 6> kQuadVertices = {{
+    Vector3(-1.0f, 1.0f, 0.5f),
+    Vector3(-1.0f, -1.0f, 0.5f),
+    Vector3(1.0f, -1.0f, 0.5f),
+    Vector3(-1.0f, 1.0f, 0.5f),
+    Vector3(1.0f, -1.0f, 0.5f),
+    Vector3(1.0f, 1.0f, 0.5f),
+}};
 
-static constexpr std::array<GLushort, 6> IndexedQuadIndices = {{0, 1, 2, 0, 2, 3}};
+const std::array<Vector3, 4> kIndexedQuadVertices = {{
+    Vector3(-1.0f, 1.0f, 0.5f),
+    Vector3(-1.0f, -1.0f, 0.5f),
+    Vector3(1.0f, -1.0f, 0.5f),
+    Vector3(1.0f, 1.0f, 0.5f),
+}};
+
+constexpr std::array<GLushort, 6> kIndexedQuadIndices = {{0, 1, 2, 0, 2, 3}};
 
 const char *GetColorName(GLColor color)
 {
@@ -156,7 +160,7 @@ GLColorRGB::GLColorRGB() : R(0), G(0), B(0) {}
 
 GLColorRGB::GLColorRGB(GLubyte r, GLubyte g, GLubyte b) : R(r), G(g), B(b) {}
 
-GLColorRGB::GLColorRGB(const angle::Vector3 &floatColor)
+GLColorRGB::GLColorRGB(const Vector3 &floatColor)
     : R(ColorDenorm(floatColor.x())), G(ColorDenorm(floatColor.y())), B(ColorDenorm(floatColor.z()))
 {}
 
@@ -164,7 +168,7 @@ GLColor::GLColor() : R(0), G(0), B(0), A(0) {}
 
 GLColor::GLColor(GLubyte r, GLubyte g, GLubyte b, GLubyte a) : R(r), G(g), B(b), A(a) {}
 
-GLColor::GLColor(const angle::Vector4 &floatColor)
+GLColor::GLColor(const Vector4 &floatColor)
     : R(ColorDenorm(floatColor.x())),
       G(ColorDenorm(floatColor.y())),
       B(ColorDenorm(floatColor.z())),
@@ -201,9 +205,9 @@ void CreatePixelCenterWindowCoords(const std::vector<Vector2> &pixelPoints,
     }
 }
 
-angle::Vector4 GLColor::toNormalizedVector() const
+Vector4 GLColor::toNormalizedVector() const
 {
-    return angle::Vector4(ColorNorm(R), ColorNorm(G), ColorNorm(B), ColorNorm(A));
+    return Vector4(ColorNorm(R), ColorNorm(G), ColorNorm(B), ColorNorm(A));
 }
 
 GLColor ReadColor(GLint x, GLint y)
@@ -262,20 +266,19 @@ GLColor32F ReadColor32F(GLint x, GLint y)
 // static
 std::array<angle::Vector3, 6> ANGLETestBase::GetQuadVertices()
 {
-    std::array<angle::Vector3, 6> vertices;
-    vertices[0] = angle::Vector3(-1.0f, 1.0f, 0.5f);
-    vertices[1] = angle::Vector3(-1.0f, -1.0f, 0.5f);
-    vertices[2] = angle::Vector3(1.0f, -1.0f, 0.5f);
-    vertices[3] = angle::Vector3(-1.0f, 1.0f, 0.5f);
-    vertices[4] = angle::Vector3(1.0f, -1.0f, 0.5f);
-    vertices[5] = angle::Vector3(1.0f, 1.0f, 0.5f);
-    return vertices;
+    return angle::kQuadVertices;
 }
 
 // static
 std::array<GLushort, 6> ANGLETestBase::GetQuadIndices()
 {
-    return angle::IndexedQuadIndices;
+    return angle::kIndexedQuadIndices;
+}
+
+// static
+std::array<angle::Vector3, 4> ANGLETestBase::GetIndexedQuadVertices()
+{
+    return angle::kIndexedQuadVertices;
 }
 
 ANGLETestBase::ANGLETestBase(const angle::PlatformParameters &params)
@@ -454,7 +457,7 @@ void ANGLETestBase::setupIndexedQuadVertexBuffer(GLfloat positionAttribZ,
         glGenBuffers(1, &mQuadVertexBuffer);
     }
 
-    auto quadVertices = angle::GetIndexedQuadVertices();
+    auto quadVertices = angle::kIndexedQuadVertices;
     for (angle::Vector3 &vertex : quadVertices)
     {
         vertex.x() *= positionAttribXYScale;
@@ -474,8 +477,8 @@ void ANGLETestBase::setupIndexedQuadIndexBuffer()
     }
 
     glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, mQuadIndexBuffer);
-    glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(angle::IndexedQuadIndices),
-                 angle::IndexedQuadIndices.data(), GL_STATIC_DRAW);
+    glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(angle::kIndexedQuadIndices),
+                 angle::kIndexedQuadIndices.data(), GL_STATIC_DRAW);
 }
 
 // static
@@ -646,7 +649,7 @@ void ANGLETestBase::drawIndexedQuad(GLuint program,
     }
     else
     {
-        indices = angle::IndexedQuadIndices.data();
+        indices = angle::kIndexedQuadIndices.data();
     }
 
     if (!restrictedRange)

src/tests/test_utils/ANGLETest.h

@@ -289,6 +289,8 @@ class ANGLETestBase
 
     static std::array<angle::Vector3, 6> GetQuadVertices();
     static std::array<GLushort, 6> GetQuadIndices();
+    static std::array<angle::Vector3, 4> GetIndexedQuadVertices();
+
     void drawIndexedQuad(GLuint program,
                          const std::string &positionAttribName,
                          GLfloat positionAttribZ);