StateManagerGL: Remove setGenericShaderState.

We can mutate the BitSetIterator as it clears dirty bits. This removes
the risk of doing a double state update. Improves the proformance of
the GL back-end state update.

Also do an early-out before calling syncDrawArraysState.

Bug: angleproject:2763
Change-Id: Idd25bdd67a6aceff05529a533260b661b07c2928
Reviewed-on: https://chromium-review.googlesource.com/c/1262740
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/common/bitset_utils.h b/src/common/bitset_utils.h
index 437343a..e171ef7 100644
--- a/src/common/bitset_utils.h
+++ b/src/common/bitset_utils.h
@@ -56,6 +56,20 @@
         bool operator!=(const Iterator &other) const;
         ParamT operator*() const;
 
+        // These helper functions allow mutating an iterator in-flight.
+        // They only operate on later bits to ensure we don't iterate the same bit twice.
+        void resetLaterBit(std::size_t index)
+        {
+            ASSERT(index > mCurrentBit);
+            mBitsCopy.reset(index);
+        }
+
+        void setLaterBit(std::size_t index)
+        {
+            ASSERT(index > mCurrentBit);
+            mBitsCopy.set(index);
+        }
+
       private:
         std::size_t getNextBit();
 
@@ -144,6 +158,20 @@
         bool operator!=(const Iterator &other) const;
         unsigned long operator*() const { return mCurrentBit; }
 
+        // These helper functions allow mutating an iterator in-flight.
+        // They only operate on later bits to ensure we don't iterate the same bit twice.
+        void resetLaterBit(std::size_t index)
+        {
+            ASSERT(index > mCurrentBit);
+            mBits.reset(index - mOffset);
+        }
+
+        void setLaterBit(std::size_t index)
+        {
+            ASSERT(index > mCurrentBit);
+            mBits.set(index - mOffset);
+        }
+
       private:
         unsigned long getNextBit();
 
diff --git a/src/common/bitset_utils_unittest.cpp b/src/common/bitset_utils_unittest.cpp
index fc2c752..fea451f 100644
--- a/src/common/bitset_utils_unittest.cpp
+++ b/src/common/bitset_utils_unittest.cpp
@@ -109,4 +109,53 @@
     }
 }
 
+// Tests adding bits to the iterator during iteration.
+TEST_F(BitSetIteratorTest, SetLaterBit)
+{
+    std::set<size_t> expectedValues = {1, 3, 5, 7, 9};
+    mStateBits.set(1);
+
+    std::set<size_t> actualValues;
+
+    for (auto iter = mStateBits.begin(), end = mStateBits.end(); iter != end; ++iter)
+    {
+        if (*iter == 1)
+        {
+            iter.setLaterBit(3);
+            iter.setLaterBit(5);
+            iter.setLaterBit(7);
+            iter.setLaterBit(9);
+        }
+
+        actualValues.insert(*iter);
+    }
+
+    EXPECT_EQ(expectedValues, actualValues);
+}
+
+// Tests removing bits from the iterator during iteration.
+TEST_F(BitSetIteratorTest, ResetLaterBit)
+{
+    std::set<size_t> expectedValues = {1, 3, 5, 7, 9};
+
+    for (size_t index = 1; index <= 9; ++index)
+        mStateBits.set(index);
+
+    std::set<size_t> actualValues;
+
+    for (auto iter = mStateBits.begin(), end = mStateBits.end(); iter != end; ++iter)
+    {
+        if (*iter == 1)
+        {
+            iter.resetLaterBit(2);
+            iter.resetLaterBit(4);
+            iter.resetLaterBit(6);
+            iter.resetLaterBit(8);
+        }
+
+        actualValues.insert(*iter);
+    }
+
+    EXPECT_EQ(expectedValues, actualValues);
+}
 }  // anonymous namespace
diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp
index c5b180b..902d903 100644
--- a/src/libANGLE/Context.cpp
+++ b/src/libANGLE/Context.cpp
@@ -5296,14 +5296,14 @@
 
 Error Context::prepareForDispatch()
 {
-    ANGLE_TRY(syncState(mComputeDirtyBits, mComputeDirtyObjects));
+    ANGLE_TRY(syncDirtyObjects(mComputeDirtyObjects));
 
     if (isRobustResourceInitEnabled())
     {
         ANGLE_TRY(mGLState.clearUnclearedActiveTextures(this));
     }
 
-    return NoError();
+    return syncDirtyBits(mComputeDirtyBits);
 }
 
 void Context::dispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ)
diff --git a/src/libANGLE/State.h b/src/libANGLE/State.h
index ae38ab9..d997a37 100644
--- a/src/libANGLE/State.h
+++ b/src/libANGLE/State.h
@@ -439,10 +439,7 @@
         DIRTY_BIT_VERTEX_ARRAY_BINDING,
         DIRTY_BIT_DRAW_INDIRECT_BUFFER_BINDING,
         DIRTY_BIT_DISPATCH_INDIRECT_BUFFER_BINDING,
-        DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING,
-        DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING,
         // TODO(jmadill): Fine-grained dirty bits for each index.
-        DIRTY_BIT_UNIFORM_BUFFER_BINDINGS,
         DIRTY_BIT_PROGRAM_BINDING,
         DIRTY_BIT_PROGRAM_EXECUTABLE,
         // TODO(jmadill): Fine-grained dirty bits for each texture/sampler.
@@ -450,6 +447,9 @@
         DIRTY_BIT_SAMPLER_BINDINGS,
         DIRTY_BIT_IMAGE_BINDINGS,
         DIRTY_BIT_TRANSFORM_FEEDBACK_BINDING,
+        DIRTY_BIT_UNIFORM_BUFFER_BINDINGS,
+        DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING,
+        DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING,
         DIRTY_BIT_MULTISAMPLING,
         DIRTY_BIT_SAMPLE_ALPHA_TO_ONE,
         DIRTY_BIT_COVERAGE_MODULATION,  // CHROMIUM_framebuffer_mixed_samples
diff --git a/src/libANGLE/renderer/gl/RendererGL.cpp b/src/libANGLE/renderer/gl/RendererGL.cpp
index 212f982..a7e5807 100644
--- a/src/libANGLE/renderer/gl/RendererGL.cpp
+++ b/src/libANGLE/renderer/gl/RendererGL.cpp
@@ -683,14 +683,12 @@
                                           GLuint numGroupsY,
                                           GLuint numGroupsZ)
 {
-    ANGLE_TRY(mStateManager->setDispatchComputeState(context));
     mFunctions->dispatchCompute(numGroupsX, numGroupsY, numGroupsZ);
     return angle::Result::Continue();
 }
 
 angle::Result RendererGL::dispatchComputeIndirect(const gl::Context *context, GLintptr indirect)
 {
-    ANGLE_TRY(mStateManager->setDispatchComputeState(context));
     mFunctions->dispatchComputeIndirect(indirect);
     return angle::Result::Continue();
 }
diff --git a/src/libANGLE/renderer/gl/StateManagerGL.cpp b/src/libANGLE/renderer/gl/StateManagerGL.cpp
index 32e4d6c..f442a1d 100644
--- a/src/libANGLE/renderer/gl/StateManagerGL.cpp
+++ b/src/libANGLE/renderer/gl/StateManagerGL.cpp
@@ -162,12 +162,7 @@
       mIsSideBySideDrawFramebuffer(false),
       mIsMultiviewEnabled(extensions.multiview),
       mLocalDirtyBits(),
-      mMultiviewDirtyBits(),
-      mProgramTexturesDirty(true),
-      mProgramStorageBuffersDirty(true),
-      mProgramUniformBuffersDirty(true),
-      mProgramAtomicCounterBuffersDirty(true),
-      mProgramImagesDirty(true)
+      mMultiviewDirtyBits()
 {
     ASSERT(mFunctions);
     ASSERT(extensions.maxViews >= 1u);
@@ -684,15 +679,16 @@
                                                  GLsizei count,
                                                  GLsizei instanceCount)
 {
-    const gl::State &glState = context->getGLState();
+    if (context->getStateCache().hasAnyActiveClientAttrib())
+    {
+        const gl::State &glState   = context->getGLState();
+        const gl::Program *program = glState.getProgram();
+        const gl::VertexArray *vao = glState.getVertexArray();
+        const VertexArrayGL *vaoGL = GetImplAs<VertexArrayGL>(vao);
 
-    const gl::Program *program = glState.getProgram();
-
-    const gl::VertexArray *vao = glState.getVertexArray();
-    const VertexArrayGL *vaoGL = GetImplAs<VertexArrayGL>(vao);
-
-    ANGLE_TRY(vaoGL->syncDrawArraysState(context, program->getActiveAttribLocationsMask(), first,
-                                         count, instanceCount));
+        ANGLE_TRY(vaoGL->syncClientSideData(context, program->getActiveAttribLocationsMask(), first,
+                                            count, instanceCount));
+    }
 
     return setGenericDrawState(context);
 }
@@ -745,12 +741,6 @@
     }
 }
 
-angle::Result StateManagerGL::setDispatchComputeState(const gl::Context *context)
-{
-    setGenericShaderState(context);
-    return angle::Result::Continue();
-}
-
 void StateManagerGL::pauseTransformFeedback()
 {
     if (mCurrentTransformFeedback != nullptr)
@@ -866,44 +856,15 @@
     return angle::Result::Continue();
 }
 
-void StateManagerGL::setGenericShaderState(const gl::Context *context)
-{
-    if (mProgramUniformBuffersDirty)
-    {
-        updateProgramUniformBufferBindings(context);
-        mProgramUniformBuffersDirty = false;
-    }
-
-    if (mProgramTexturesDirty)
-    {
-        updateProgramTextureBindings(context);
-        mProgramTexturesDirty = false;
-    }
-
-    if (mProgramStorageBuffersDirty)
-    {
-        updateProgramStorageBufferBindings(context);
-        mProgramStorageBuffersDirty = false;
-    }
-
-    if (mProgramImagesDirty)
-    {
-        updateProgramImageBindings(context);
-        mProgramImagesDirty = false;
-    }
-
-    if (mProgramAtomicCounterBuffersDirty)
-    {
-        updateProgramAtomicCounterBufferBindings(context);
-        mProgramAtomicCounterBuffersDirty = false;
-    }
-}
-
 void StateManagerGL::updateProgramTextureBindings(const gl::Context *context)
 {
     const gl::State &glState   = context->getGLState();
     const gl::Program *program = glState.getProgram();
 
+    // It is possible there is no active program during a path operation.
+    if (!program)
+        return;
+
     const gl::ActiveTexturePointerArray &textures  = glState.getActiveTexturesCache();
     const gl::ActiveTextureMask &activeTextures    = program->getActiveSamplersMask();
     const gl::ActiveTextureTypeArray &textureTypes = program->getActiveSamplerTypes();
@@ -936,6 +897,10 @@
     const gl::State &glState   = context->getGLState();
     const gl::Program *program = glState.getProgram();
 
+    // It is possible there is no active program during a path operation.
+    if (!program)
+        return;
+
     for (size_t blockIndex = 0; blockIndex < program->getActiveShaderStorageBlockCount();
          blockIndex++)
     {
@@ -964,6 +929,11 @@
     // Sync the current program state
     const gl::State &glState   = context->getGLState();
     const gl::Program *program = glState.getProgram();
+
+    // It is possible there is no active program during a path operation.
+    if (!program)
+        return;
+
     for (size_t uniformBlockIndex = 0; uniformBlockIndex < program->getActiveUniformBlockCount();
          uniformBlockIndex++)
     {
@@ -992,6 +962,10 @@
     const gl::State &glState   = context->getGLState();
     const gl::Program *program = glState.getProgram();
 
+    // It is possible there is no active program during a path operation.
+    if (!program)
+        return;
+
     for (const auto &atomicCounterBuffer : program->getState().getAtomicCounterBuffers())
     {
         GLuint binding     = atomicCounterBuffer.binding;
@@ -1019,6 +993,10 @@
     const gl::State &glState   = context->getGLState();
     const gl::Program *program = glState.getProgram();
 
+    // It is possible there is no active program during a path operation.
+    if (!program)
+        return;
+
     ASSERT(context->getClientVersion() >= gl::ES_3_1 || program->getImageBindings().size() == 0);
     for (size_t imageUnitIndex : program->getActiveImagesMask())
     {
@@ -1040,8 +1018,6 @@
 
 angle::Result StateManagerGL::setGenericDrawState(const gl::Context *context)
 {
-    setGenericShaderState(context);
-
     if (context->getExtensions().webglCompatibility)
     {
         const gl::State &glState     = context->getGLState();
@@ -1755,9 +1731,10 @@
     }
 
     // TODO(jmadill): Investigate only syncing vertex state for active attributes
-    for (auto dirtyBit : glAndLocalDirtyBits)
+    for (auto iter = glAndLocalDirtyBits.begin(), endIter = glAndLocalDirtyBits.end();
+         iter != endIter; ++iter)
     {
-        switch (dirtyBit)
+        switch (*iter)
         {
             case gl::State::DIRTY_BIT_SCISSOR_TEST_ENABLED:
                 setScissorTestEnabled(state.isScissorTestEnabled());
@@ -1993,44 +1970,67 @@
                 }
                 break;
             }
+            case gl::State::DIRTY_BIT_PROGRAM_EXECUTABLE:
+            {
+                const gl::Program *program = state.getProgram();
+                if (program)
+                {
+                    iter.setLaterBit(gl::State::DIRTY_BIT_TEXTURE_BINDINGS);
+
+                    if (program->getActiveImagesMask().any())
+                    {
+                        iter.setLaterBit(gl::State::DIRTY_BIT_IMAGE_BINDINGS);
+                    }
+
+                    if (program->getActiveShaderStorageBlockCount() > 0)
+                    {
+                        iter.setLaterBit(gl::State::DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING);
+                    }
+
+                    if (program->getActiveUniformBlockCount() > 0)
+                    {
+                        iter.setLaterBit(gl::State::DIRTY_BIT_UNIFORM_BUFFER_BINDINGS);
+                    }
+
+                    if (program->getActiveAtomicCounterBufferCount() > 0)
+                    {
+                        iter.setLaterBit(gl::State::DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING);
+                    }
+
+                    if (mIsMultiviewEnabled && program->usesMultiview())
+                    {
+                        updateMultiviewBaseViewLayerIndexUniform(
+                            program, state.getDrawFramebuffer()->getImplementation()->getState());
+                    }
+                }
+
+                if (!program || !program->hasLinkedShaderStage(gl::ShaderType::Compute))
+                {
+                    propagateProgramToVAO(program,
+                                          GetImplAs<VertexArrayGL>(state.getVertexArray()));
+                }
+                break;
+            }
             case gl::State::DIRTY_BIT_TEXTURE_BINDINGS:
-                mProgramTexturesDirty = true;
+                updateProgramTextureBindings(context);
                 break;
             case gl::State::DIRTY_BIT_SAMPLER_BINDINGS:
                 syncSamplersState(context);
                 break;
             case gl::State::DIRTY_BIT_IMAGE_BINDINGS:
-                mProgramImagesDirty = true;
+                updateProgramImageBindings(context);
                 break;
             case gl::State::DIRTY_BIT_TRANSFORM_FEEDBACK_BINDING:
                 syncTransformFeedbackState(context);
                 break;
-            case gl::State::DIRTY_BIT_PROGRAM_EXECUTABLE:
-            {
-                mProgramTexturesDirty             = true;
-                mProgramImagesDirty               = true;
-                mProgramStorageBuffersDirty      = true;
-                mProgramUniformBuffersDirty       = true;
-                mProgramAtomicCounterBuffersDirty = true;
-                const gl::Program *program        = state.getProgram();
-                if (!(program && program->hasLinkedShaderStage(gl::ShaderType::Compute)))
-                {
-                    propagateProgramToVAO(program,
-                                          GetImplAs<VertexArrayGL>(state.getVertexArray()));
-                    updateMultiviewBaseViewLayerIndexUniform(
-                        state.getProgram(),
-                        state.getDrawFramebuffer()->getImplementation()->getState());
-                }
-                break;
-            }
             case gl::State::DIRTY_BIT_SHADER_STORAGE_BUFFER_BINDING:
-                mProgramStorageBuffersDirty = true;
+                updateProgramStorageBufferBindings(context);
                 break;
             case gl::State::DIRTY_BIT_UNIFORM_BUFFER_BINDINGS:
-                mProgramUniformBuffersDirty = true;
+                updateProgramUniformBufferBindings(context);
                 break;
             case gl::State::DIRTY_BIT_ATOMIC_COUNTER_BUFFER_BINDING:
-                mProgramAtomicCounterBuffersDirty = true;
+                updateProgramAtomicCounterBufferBindings(context);
                 break;
             case gl::State::DIRTY_BIT_MULTISAMPLING:
                 setMultisamplingStateEnabled(state.isMultisamplingEnabled());
@@ -2301,24 +2301,22 @@
     }
 }
 
-void StateManagerGL::updateMultiviewBaseViewLayerIndexUniform(
+void StateManagerGL::updateMultiviewBaseViewLayerIndexUniformImpl(
     const gl::Program *program,
     const gl::FramebufferState &drawFramebufferState) const
 {
-    if (mIsMultiviewEnabled && program != nullptr && program->usesMultiview())
+    ASSERT(mIsMultiviewEnabled && program && program->usesMultiview());
+    const ProgramGL *programGL = GetImplAs<ProgramGL>(program);
+    switch (drawFramebufferState.getMultiviewLayout())
     {
-        const ProgramGL *programGL = GetImplAs<ProgramGL>(program);
-        switch (drawFramebufferState.getMultiviewLayout())
-        {
-            case GL_FRAMEBUFFER_MULTIVIEW_SIDE_BY_SIDE_ANGLE:
-                programGL->enableSideBySideRenderingPath();
-                break;
-            case GL_FRAMEBUFFER_MULTIVIEW_LAYERED_ANGLE:
-                programGL->enableLayeredRenderingPath(drawFramebufferState.getBaseViewIndex());
-                break;
-            default:
-                break;
-        }
+        case GL_FRAMEBUFFER_MULTIVIEW_SIDE_BY_SIDE_ANGLE:
+            programGL->enableSideBySideRenderingPath();
+            break;
+        case GL_FRAMEBUFFER_MULTIVIEW_LAYERED_ANGLE:
+            programGL->enableLayeredRenderingPath(drawFramebufferState.getBaseViewIndex());
+            break;
+        default:
+            break;
     }
 }
 
diff --git a/src/libANGLE/renderer/gl/StateManagerGL.h b/src/libANGLE/renderer/gl/StateManagerGL.h
index f83d109..47b8b9d 100644
--- a/src/libANGLE/renderer/gl/StateManagerGL.h
+++ b/src/libANGLE/renderer/gl/StateManagerGL.h
@@ -23,7 +23,7 @@
 class ContextState;
 class State;
 class FramebufferState;
-}
+}  // namespace gl
 
 namespace rx
 {
@@ -165,8 +165,6 @@
                                        const void **outIndices);
     angle::Result setDrawIndirectState(const gl::Context *context);
 
-    angle::Result setDispatchComputeState(const gl::Context *context);
-
     void pauseTransformFeedback();
     angle::Result pauseAllQueries(const gl::Context *context);
     angle::Result pauseQuery(const gl::Context *context, gl::QueryType type);
@@ -178,16 +176,19 @@
                    const gl::State::DirtyBits &glDirtyBits,
                    const gl::State::DirtyBits &bitMask);
 
-    void updateMultiviewBaseViewLayerIndexUniform(
+    ANGLE_INLINE void updateMultiviewBaseViewLayerIndexUniform(
         const gl::Program *program,
-        const gl::FramebufferState &drawFramebufferState) const;
+        const gl::FramebufferState &drawFramebufferState) const
+    {
+        if (mIsMultiviewEnabled && program && program->usesMultiview())
+        {
+            updateMultiviewBaseViewLayerIndexUniformImpl(program, drawFramebufferState);
+        }
+    }
 
     GLuint getVertexArrayID() const { return mVAO; }
 
   private:
-    // Set state that's common among draw commands and compute invocations.
-    void setGenericShaderState(const gl::Context *context);
-
     // Set state that's common among draw commands.
     angle::Result setGenericDrawState(const gl::Context *context);
 
@@ -211,6 +212,10 @@
     void syncSamplersState(const gl::Context *context);
     void syncTransformFeedbackState(const gl::Context *context);
 
+    void updateMultiviewBaseViewLayerIndexUniformImpl(
+        const gl::Program *program,
+        const gl::FramebufferState &drawFramebufferState) const;
+
     enum MultiviewDirtyBitType
     {
         MULTIVIEW_DIRTY_BIT_SIDE_BY_SIDE_LAYOUT,
@@ -368,13 +373,7 @@
 
     // ANGLE_multiview dirty bits.
     angle::BitSet<MULTIVIEW_DIRTY_BIT_MAX> mMultiviewDirtyBits;
-
-    bool mProgramTexturesDirty;
-    bool mProgramStorageBuffersDirty;
-    bool mProgramUniformBuffersDirty;
-    bool mProgramAtomicCounterBuffersDirty;
-    bool mProgramImagesDirty;
 };
-}
+}  // namespace rx
 
 #endif  // LIBANGLE_RENDERER_GL_STATEMANAGERGL_H_
diff --git a/src/libANGLE/renderer/gl/VertexArrayGL.cpp b/src/libANGLE/renderer/gl/VertexArrayGL.cpp
index 138303f..0390782 100644
--- a/src/libANGLE/renderer/gl/VertexArrayGL.cpp
+++ b/src/libANGLE/renderer/gl/VertexArrayGL.cpp
@@ -104,11 +104,11 @@
     }
 }
 
-angle::Result VertexArrayGL::syncDrawArraysState(const gl::Context *context,
-                                                 const gl::AttributesMask &activeAttributesMask,
-                                                 GLint first,
-                                                 GLsizei count,
-                                                 GLsizei instanceCount) const
+angle::Result VertexArrayGL::syncClientSideData(const gl::Context *context,
+                                                const gl::AttributesMask &activeAttributesMask,
+                                                GLint first,
+                                                GLsizei count,
+                                                GLsizei instanceCount) const
 {
     return syncDrawState(context, activeAttributesMask, first, count, GL_NONE, nullptr,
                          instanceCount, false, nullptr);
diff --git a/src/libANGLE/renderer/gl/VertexArrayGL.h b/src/libANGLE/renderer/gl/VertexArrayGL.h
index 3828e0d..c6062fb 100644
--- a/src/libANGLE/renderer/gl/VertexArrayGL.h
+++ b/src/libANGLE/renderer/gl/VertexArrayGL.h
@@ -27,11 +27,11 @@
 
     void destroy(const gl::Context *context) override;
 
-    angle::Result syncDrawArraysState(const gl::Context *context,
-                                      const gl::AttributesMask &activeAttributesMask,
-                                      GLint first,
-                                      GLsizei count,
-                                      GLsizei instanceCount) const;
+    angle::Result syncClientSideData(const gl::Context *context,
+                                     const gl::AttributesMask &activeAttributesMask,
+                                     GLint first,
+                                     GLsizei count,
+                                     GLsizei instanceCount) const;
     angle::Result syncDrawElementsState(const gl::Context *context,
                                         const gl::AttributesMask &activeAttributesMask,
                                         GLsizei count,