Use Subject/Observer pattern for Programs/Executables in PPOs

src/libANGLE/Context.cpp

@@ -256,7 +256,8 @@ enum SubjectIndexes : angle::SubjectIndex
     kSamplerMaxSubjectIndex  = kSampler0SubjectIndex + IMPLEMENTATION_MAX_ACTIVE_TEXTURES,
     kVertexArraySubjectIndex = kSamplerMaxSubjectIndex,
     kReadFramebufferSubjectIndex,
-    kDrawFramebufferSubjectIndex
+    kDrawFramebufferSubjectIndex,
+    kProgramPipelineSubjectIndex
 };
 }  // anonymous namespace
 
@@ -308,6 +309,7 @@ Context::Context(egl::Display *display,
       mVertexArrayObserverBinding(this, kVertexArraySubjectIndex),
       mDrawFramebufferObserverBinding(this, kDrawFramebufferSubjectIndex),
       mReadFramebufferObserverBinding(this, kReadFramebufferSubjectIndex),
+      mProgramPipelineObserverBinding(this, kProgramPipelineSubjectIndex),
       mThreadPool(nullptr),
       mFrameCapture(new angle::FrameCapture),
       mOverlay(mImplementation.get())
@@ -1153,6 +1155,7 @@ void Context::bindProgramPipeline(ProgramPipelineID pipelineHandle)
         mImplementation.get(), pipelineHandle);
     ANGLE_CONTEXT_TRY(mState.setProgramPipelineBinding(this, pipeline));
     mStateCache.onProgramExecutableChange(this);
+    mProgramPipelineObserverBinding.bind(pipeline);
 }
 
 void Context::beginQuery(QueryType target, QueryID query)
@@ -2716,6 +2719,7 @@ void Context::detachSampler(SamplerID sampler)
 void Context::detachProgramPipeline(ProgramPipelineID pipeline)
 {
     mState.detachProgramPipeline(this, pipeline);
+    mProgramPipelineObserverBinding.bind(nullptr);
 }
 
 void Context::vertexAttribDivisor(GLuint index, GLuint divisor)
@@ -8005,6 +8009,11 @@ void Context::onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMess
             mStateCache.onDrawFramebufferChange(this);
             break;
 
+        case kProgramPipelineSubjectIndex:
+            ASSERT(message == angle::SubjectMessage::DirtyBitsFlagged);
+            mState.setProgramPipelineDirty();
+            break;
+
         default:
             if (index < kTextureMaxSubjectIndex)
             {
@@ -8057,25 +8066,6 @@ angle::Result Context::onProgramLink(Program *programObject)
         mStateCache.onProgramExecutableChange(this);
     }
 
-    // TODO(http://anglebug.com/4559): Use the Subject/Observer pattern for
-    // Programs in PPOs so we can remove this.
-    // Need to mark any PPOs that this Program is bound to as dirty
-    bool foundPipeline = false;
-    for (ResourceMap<ProgramPipeline, ProgramPipelineID>::Iterator ppoIterator =
-             mState.mProgramPipelineManager->begin();
-         ppoIterator != mState.mProgramPipelineManager->end(); ++ppoIterator)
-    {
-        ProgramPipeline *pipeline = ppoIterator->second;
-        pipeline->setDirtyBit(ProgramPipeline::DirtyBitType::DIRTY_BIT_PROGRAM_STAGE);
-        foundPipeline = true;
-    }
-    // Also need to make sure the PPO dirty bits get handled by marking the PPO
-    // objects dirty.
-    if (foundPipeline)
-    {
-        mState.mDirtyObjects.set(State::DIRTY_OBJECT_PROGRAM_PIPELINE);
-    }
-
     return angle::Result::Continue;
 }
 

src/libANGLE/Context.h

@@ -754,6 +754,7 @@ class Context final : public egl::LabeledObject, angle::NonCopyable, public angl
     angle::ObserverBinding mVertexArrayObserverBinding;
     angle::ObserverBinding mDrawFramebufferObserverBinding;
     angle::ObserverBinding mReadFramebufferObserverBinding;
+    angle::ObserverBinding mProgramPipelineObserverBinding;
     std::vector<angle::ObserverBinding> mUniformBufferObserverBindings;
     std::vector<angle::ObserverBinding> mSamplerObserverBindings;
     std::vector<angle::ObserverBinding> mImageObserverBindings;

src/libANGLE/Program.h

@@ -420,7 +420,7 @@ struct ProgramVaryingRef
 
 using ProgramMergedVaryings = std::vector<ProgramVaryingRef>;
 
-class Program final : angle::NonCopyable, public LabeledObject
+class Program final : public LabeledObject, public angle::Subject
 {
   public:
     Program(rx::GLImplFactory *factory, ShaderProgramManager *manager, ShaderProgramID handle);

src/libANGLE/ProgramExecutable.h

@@ -98,12 +98,12 @@ struct TransformFeedbackVarying : public sh::ShaderVariable
 class ProgramState;
 class ProgramPipelineState;
 
-class ProgramExecutable
+class ProgramExecutable final : public angle::Subject
 {
   public:
     ProgramExecutable();
     ProgramExecutable(const ProgramExecutable &other);
-    virtual ~ProgramExecutable();
+    ~ProgramExecutable() override;
 
     void reset();
 
@@ -167,6 +167,7 @@ class ProgramExecutable
     AttributesMask getAttributesMask() const;
 
     const ActiveTextureMask &getActiveSamplersMask() const { return mActiveSamplersMask; }
+    void setActiveTextureMask(ActiveTextureMask mask) { mActiveSamplersMask = mask; }
     SamplerFormat getSamplerFormatForTextureUnitIndex(size_t textureUnitIndex) const
     {
         return mActiveSamplerFormats[textureUnitIndex];
@@ -176,6 +177,7 @@ class ProgramExecutable
         return mActiveSamplerShaderBits[textureUnitIndex];
     }
     const ActiveTextureMask &getActiveImagesMask() const { return mActiveImagesMask; }
+    void setActiveImagesMask(ActiveTextureMask mask) { mActiveImagesMask = mask; }
     const ActiveTextureArray<ShaderBitSet> &getActiveImageShaderBits() const
     {
         return mActiveImageShaderBits;
@@ -186,6 +188,8 @@ class ProgramExecutable
         return mActiveSamplerTypes;
     }
 
+    void updateActiveSamplers(const ProgramState &programState);
+
     bool hasDefaultUniforms() const;
     bool hasTextures() const;
     bool hasUniformBuffers() const;
@@ -290,7 +294,6 @@ class ProgramExecutable
     friend class ProgramPipeline;
     friend class ProgramState;
 
-    void updateActiveSamplers(const ProgramState &programState);
     void updateActiveImages(const ProgramExecutable &executable);
 
     // Scans the sampler bindings for type conflicts with sampler 'textureUnitIndex'.

src/libANGLE/ProgramPipeline.cpp

@@ -13,7 +13,6 @@
 #include <algorithm>
 
 #include "libANGLE/Context.h"
-#include "libANGLE/ErrorStrings.h"
 #include "libANGLE/Program.h"
 #include "libANGLE/angletypes.h"
 #include "libANGLE/renderer/GLImplFactory.h"
@@ -22,6 +21,11 @@
 namespace gl
 {
 
+enum SubjectIndexes : angle::SubjectIndex
+{
+    kExecutableSubjectIndex = 0
+};
+
 ProgramPipelineState::ProgramPipelineState()
     : mLabel(), mActiveShaderProgram(nullptr), mValid(false), mExecutable(new ProgramExecutable())
 {
@@ -48,7 +52,8 @@ void ProgramPipelineState::activeShaderProgram(Program *shaderProgram)
 
 void ProgramPipelineState::useProgramStage(const Context *context,
                                            const ShaderType shaderType,
-                                           Program *shaderProgram)
+                                           Program *shaderProgram,
+                                           angle::ObserverBinding *programObserverBindings)
 {
     Program *oldProgram = mPrograms[shaderType];
     if (oldProgram)
@@ -72,34 +77,51 @@ void ProgramPipelineState::useProgramStage(const Context *context,
         // indicated shader stage.
         mPrograms[shaderType] = nullptr;
     }
+
+    Program *program = mPrograms[shaderType];
+    programObserverBindings->bind(program);
 }
 
-void ProgramPipelineState::useProgramStages(const Context *context,
-                                            GLbitfield stages,
-                                            Program *shaderProgram)
+void ProgramPipelineState::useProgramStages(
+    const Context *context,
+    GLbitfield stages,
+    Program *shaderProgram,
+    std::vector<angle::ObserverBinding> *programObserverBindings)
 {
     if (stages == GL_ALL_SHADER_BITS)
     {
         for (const ShaderType shaderType : gl::AllShaderTypes())
         {
-            useProgramStage(context, shaderType, shaderProgram);
+            size_t index = static_cast<size_t>(shaderType);
+            ASSERT(index < programObserverBindings->size());
+            useProgramStage(context, shaderType, shaderProgram,
+                            &programObserverBindings->at(index));
         }
     }
     else
     {
         if (stages & GL_VERTEX_SHADER_BIT)
         {
-            useProgramStage(context, ShaderType::Vertex, shaderProgram);
+            size_t index = static_cast<size_t>(ShaderType::Vertex);
+            ASSERT(index < programObserverBindings->size());
+            useProgramStage(context, ShaderType::Vertex, shaderProgram,
+                            &programObserverBindings->at(index));
         }
 
         if (stages & GL_FRAGMENT_SHADER_BIT)
         {
-            useProgramStage(context, ShaderType::Fragment, shaderProgram);
+            size_t index = static_cast<size_t>(ShaderType::Fragment);
+            ASSERT(index < programObserverBindings->size());
+            useProgramStage(context, ShaderType::Fragment, shaderProgram,
+                            &programObserverBindings->at(index));
         }
 
         if (stages & GL_COMPUTE_SHADER_BIT)
         {
-            useProgramStage(context, ShaderType::Compute, shaderProgram);
+            size_t index = static_cast<size_t>(ShaderType::Compute);
+            ASSERT(index < programObserverBindings->size());
+            useProgramStage(context, ShaderType::Compute, shaderProgram,
+                            &programObserverBindings->at(index));
         }
     }
 }
@@ -117,11 +139,31 @@ bool ProgramPipelineState::usesShaderProgram(ShaderProgramID programId) const
     return false;
 }
 
+void ProgramPipelineState::updateExecutableTextures()
+{
+    for (const ShaderType shaderType : mExecutable->getLinkedShaderStages())
+    {
+        const Program *program = getShaderProgram(shaderType);
+        ASSERT(program);
+        mExecutable->setActiveTextureMask(program->getExecutable().getActiveSamplersMask());
+        mExecutable->setActiveImagesMask(program->getExecutable().getActiveImagesMask());
+        // Updates mActiveSamplerRefCounts, mActiveSamplerTypes, and mActiveSamplerFormats
+        mExecutable->updateActiveSamplers(program->getState());
+    }
+}
+
 ProgramPipeline::ProgramPipeline(rx::GLImplFactory *factory, ProgramPipelineID handle)
     : RefCountObject(factory->generateSerial(), handle),
-      mProgramPipelineImpl(factory->createProgramPipeline(mState))
+      mProgramPipelineImpl(factory->createProgramPipeline(mState)),
+      mExecutableObserverBinding(this, kExecutableSubjectIndex)
 {
     ASSERT(mProgramPipelineImpl);
+
+    for (const ShaderType shaderType : gl::AllShaderTypes())
+    {
+        mProgramObserverBindings.emplace_back(this, static_cast<angle::SubjectIndex>(shaderType));
+    }
+    mExecutableObserverBinding.bind(mState.mExecutable);
 }
 
 ProgramPipeline::~ProgramPipeline()
@@ -167,7 +209,7 @@ void ProgramPipeline::useProgramStages(const Context *context,
                                        GLbitfield stages,
                                        Program *shaderProgram)
 {
-    mState.useProgramStages(context, stages, shaderProgram);
+    mState.useProgramStages(context, stages, shaderProgram, &mProgramObserverBindings);
     updateLinkedShaderStages();
     updateExecutable();
 
@@ -221,22 +263,6 @@ void ProgramPipeline::updateTransformFeedbackMembers()
         vertexExecutable.mLinkedTransformFeedbackVaryings;
 }
 
-void ProgramPipeline::updateExecutableTextures()
-{
-    for (const ShaderType shaderType : mState.mExecutable->getLinkedShaderStages())
-    {
-        const Program *program = getShaderProgram(shaderType);
-        if (program)
-        {
-            mState.mExecutable->mActiveSamplersMask |=
-                program->getExecutable().getActiveSamplersMask();
-            mState.mExecutable->mActiveImagesMask |= program->getExecutable().getActiveImagesMask();
-            // Updates mActiveSamplerRefCounts, mActiveSamplerTypes, and mActiveSamplerFormats
-            mState.mExecutable->updateActiveSamplers(program->getState());
-        }
-    }
-}
-
 void ProgramPipeline::updateHasBooleans()
 {
     // Need to check all of the shader stages, not just linked, so we handle Compute correctly.
@@ -315,7 +341,7 @@ void ProgramPipeline::updateExecutable()
     updateTransformFeedbackMembers();
 
     // All Shader ProgramExecutable properties
-    updateExecutableTextures();
+    mState.updateExecutableTextures();
     updateHasBooleans();
 }
 
@@ -616,6 +642,21 @@ angle::Result ProgramPipeline::syncState(const Context *context)
     return angle::Result::Continue;
 }
 
+void ProgramPipeline::onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message)
+{
+    switch (message)
+    {
+        case angle::SubjectMessage::SubjectChanged:
+            setDirtyBit(ProgramPipeline::DirtyBitType::DIRTY_BIT_PROGRAM_STAGE);
+            mState.updateExecutableTextures();
+            onStateChange(angle::SubjectMessage::DirtyBitsFlagged);
+            break;
+        default:
+            UNREACHABLE();
+            break;
+    }
+}
+
 void ProgramPipeline::fillProgramStateMap(ShaderMap<const ProgramState *> *programStatesOut)
 {
     for (ShaderType shaderType : AllShaderTypes())

src/libANGLE/ProgramPipeline.h

@@ -50,7 +50,10 @@ class ProgramPipelineState final : angle::NonCopyable
     }
 
     void activeShaderProgram(Program *shaderProgram);
-    void useProgramStages(const Context *context, GLbitfield stages, Program *shaderProgram);
+    void useProgramStages(const Context *context,
+                          GLbitfield stages,
+                          Program *shaderProgram,
+                          std::vector<angle::ObserverBinding> *programObserverBindings);
 
     Program *getActiveShaderProgram() { return mActiveShaderProgram; }
 
@@ -60,8 +63,13 @@ class ProgramPipelineState final : angle::NonCopyable
 
     bool usesShaderProgram(ShaderProgramID program) const;
 
+    void updateExecutableTextures();
+
   private:
-    void useProgramStage(const Context *context, ShaderType shaderType, Program *shaderProgram);
+    void useProgramStage(const Context *context,
+                         ShaderType shaderType,
+                         Program *shaderProgram,
+                         angle::ObserverBinding *programObserverBindings);
 
     friend class ProgramPipeline;
 
@@ -77,7 +85,10 @@ class ProgramPipelineState final : angle::NonCopyable
     ProgramExecutable *mExecutable;
 };
 
-class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, public LabeledObject
+class ProgramPipeline final : public RefCountObject<ProgramPipelineID>,
+                              public LabeledObject,
+                              public angle::ObserverInterface,
+                              public angle::Subject
 {
   public:
     ProgramPipeline(rx::GLImplFactory *factory, ProgramPipelineID handle);
@@ -141,7 +152,8 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, public L
     angle::Result syncState(const Context *context);
     void setDirtyBit(DirtyBitType dirtyBitType) { mDirtyBits.set(dirtyBitType); }
 
-    void updateExecutableTextures();
+    // ObserverInterface implementation.
+    void onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message) override;
 
     void fillProgramStateMap(gl::ShaderMap<const gl::ProgramState *> *programStatesOut);
 
@@ -157,6 +169,9 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, public L
     ProgramPipelineState mState;
 
     DirtyBits mDirtyBits;
+
+    std::vector<angle::ObserverBinding> mProgramObserverBindings;
+    angle::ObserverBinding mExecutableObserverBinding;
 };
 }  // namespace gl
 

src/libANGLE/State.cpp

@@ -3409,25 +3409,6 @@ void State::setImageUnit(const Context *context,
     onImageStateChange(context, unit);
 }
 
-void State::updatePPOActiveTextures()
-{
-    // TODO(http://anglebug.com/4559): Use the Subject/Observer pattern for
-    // Programs in PPOs so we can remove this.
-    if (!mProgram)
-    {
-        // There is no Program bound, so we are updating the textures for a separable Program.
-        // Only that Program's Executable has been updated so far, so we need to update the
-        // Executable for each of the PPO's in case the Program is in there as well.
-        for (ResourceMap<ProgramPipeline, ProgramPipelineID>::Iterator ppoIterator =
-                 mProgramPipelineManager->begin();
-             ppoIterator != mProgramPipelineManager->end(); ++ppoIterator)
-        {
-            ProgramPipeline *pipeline = ppoIterator->second;
-            pipeline->updateExecutableTextures();
-        }
-    }
-}
-
 // Handle a dirty texture event.
 void State::onActiveTextureChange(const Context *context, size_t textureUnit)
 {
@@ -3439,7 +3420,7 @@ void State::onActiveTextureChange(const Context *context, size_t textureUnit)
                                      : nullptr;
         updateActiveTexture(context, textureUnit, activeTexture);
 
-        updatePPOActiveTextures();
+        mExecutable->onStateChange(angle::SubjectMessage::SubjectChanged);
     }
 }
 
@@ -3477,7 +3458,7 @@ void State::onImageStateChange(const Context *context, size_t unit)
             mDirtyObjects.set(DIRTY_OBJECT_IMAGES_INIT);
         }
 
-        updatePPOActiveTextures();
+        mExecutable->onStateChange(angle::SubjectMessage::SubjectChanged);
     }
 }
 

src/libANGLE/State.h

@@ -685,6 +685,11 @@ class State : angle::NonCopyable
         mDirtyObjects.set(DIRTY_OBJECT_DRAW_ATTACHMENTS);
     }
 
+    ANGLE_INLINE void setProgramPipelineDirty()
+    {
+        mDirtyObjects.set(DIRTY_OBJECT_PROGRAM_PIPELINE);
+    }
+
     // This actually clears the current value dirty bits.
     // TODO(jmadill): Pass mutable dirty bits into Impl.
     AttributesMask getAndResetDirtyCurrentValues() const;
@@ -821,8 +826,6 @@ class State : angle::NonCopyable
     angle::Result syncProgram(const Context *context);
     angle::Result syncProgramPipeline(const Context *context);
 
-    void updatePPOActiveTextures();
-
     using DirtyObjectHandler = angle::Result (State::*)(const Context *context);
     static constexpr DirtyObjectHandler kDirtyObjectHandlers[DIRTY_OBJECT_MAX] = {
         &State::syncTexturesInit,    &State::syncImagesInit,      &State::syncReadAttachments,