Validate PPO sampler uniforms

src/libANGLE/Observer.h

@@ -57,6 +57,8 @@ enum class SubjectMessage
 
     // Indicates a separable program was successfully re-linked.
     ProgramRelinked,
+    // Indicates a separable program's sampler uniforms were updated.
+    SamplerUniformsUpdated,
 };
 
 // The observing class inherits from this interface class.

src/libANGLE/Program.cpp

@@ -3033,34 +3033,6 @@ void Program::validate(const Caps &caps)
     }
 }
 
-bool Program::validateSamplersImpl(InfoLog *infoLog, const Caps &caps)
-{
-    const ProgramExecutable *executable = mState.mExecutable.get();
-    ASSERT(!mLinkingState);
-
-    // if any two active samplers in a program are of different types, but refer to the same
-    // texture image unit, and this is the current program, then ValidateProgram will fail, and
-    // DrawArrays and DrawElements will issue the INVALID_OPERATION error.
-    for (size_t textureUnit : executable->mActiveSamplersMask)
-    {
-        if (executable->mActiveSamplerTypes[textureUnit] == TextureType::InvalidEnum)
-        {
-            if (infoLog)
-            {
-                (*infoLog) << "Samplers of conflicting types refer to the same texture "
-                              "image unit ("
-                           << textureUnit << ").";
-            }
-
-            mCachedValidateSamplersResult = false;
-            return false;
-        }
-    }
-
-    mCachedValidateSamplersResult = true;
-    return true;
-}
-
 bool Program::isValidated() const
 {
     ASSERT(!mLinkingState);
@@ -4482,7 +4454,9 @@ void Program::updateSamplerUniform(Context *context,
     }
 
     // Invalidate the validation cache.
-    mCachedValidateSamplersResult.reset();
+    getExecutable().resetCachedValidateSamplersResult();
+    // Inform any PPOs this Program may be bound to.
+    onStateChange(angle::SubjectMessage::SamplerUniformsUpdated);
 }
 
 void ProgramState::setSamplerUniformTextureTypeAndFormat(size_t textureUnitIndex)

src/libANGLE/Program.h

@@ -714,23 +714,8 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt
     bool isFlaggedForDeletion() const;
 
     void validate(const Caps &caps);
-    bool validateSamplers(InfoLog *infoLog, const Caps &caps)
-    {
-        // Skip cache if we're using an infolog, so we get the full error.
-        // Also skip the cache if the sample mapping has changed, or if we haven't ever validated.
-        if (infoLog == nullptr && mCachedValidateSamplersResult.valid())
-        {
-            return mCachedValidateSamplersResult.value();
-        }
-
-        return validateSamplersImpl(infoLog, caps);
-    }
-
     bool isValidated() const;
 
-    Optional<bool> getCachedValidateSamplersResult() { return mCachedValidateSamplersResult; }
-    void setCachedValidateSamplersResult(bool result) { mCachedValidateSamplersResult = result; }
-
     const std::vector<ImageBinding> &getImageBindings() const
     {
         ASSERT(!mLinkingState);
@@ -913,8 +898,6 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt
     GLuint getSamplerUniformBinding(const VariableLocation &uniformLocation) const;
     GLuint getImageUniformBinding(const VariableLocation &uniformLocation) const;
 
-    bool validateSamplersImpl(InfoLog *infoLog, const Caps &caps);
-
     // Block until linking is finished and resolve it.
     void resolveLinkImpl(const gl::Context *context);
 
@@ -941,9 +924,6 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt
     ShaderProgramManager *mResourceManager;
     const ShaderProgramID mHandle;
 
-    // Cache for sampler validation
-    Optional<bool> mCachedValidateSamplersResult;
-
     DirtyBits mDirtyBits;
 };
 }  // namespace gl

src/libANGLE/ProgramExecutable.cpp

@@ -751,6 +751,7 @@ void ProgramExecutable::updateActiveSamplers(const ProgramState &programState)
             {
                 if (mActiveSamplerTypes[textureUnit] != samplerBinding.textureType)
                 {
+                    // Conflicts are marked with InvalidEnum
                     mActiveSamplerTypes[textureUnit] = TextureType::InvalidEnum;
                 }
                 if (mActiveSamplerYUV.test(textureUnit) !=
@@ -1112,4 +1113,29 @@ void ProgramExecutable::updateTransformFeedbackStrides()
     }
 }
 
+bool ProgramExecutable::validateSamplersImpl(InfoLog *infoLog, const Caps &caps) const
+{
+    // if any two active samplers in a program are of different types, but refer to the same
+    // texture image unit, and this is the current program, then ValidateProgram will fail, and
+    // DrawArrays and DrawElements will issue the INVALID_OPERATION error.
+    for (size_t textureUnit : mActiveSamplersMask)
+    {
+        if (mActiveSamplerTypes[textureUnit] == TextureType::InvalidEnum)
+        {
+            if (infoLog)
+            {
+                (*infoLog) << "Samplers of conflicting types refer to the same texture "
+                              "image unit ("
+                           << textureUnit << ").";
+            }
+
+            mCachedValidateSamplersResult = false;
+            return false;
+        }
+    }
+
+    mCachedValidateSamplersResult = true;
+    return true;
+}
+
 }  // namespace gl

src/libANGLE/ProgramExecutable.h

@@ -344,6 +344,20 @@ class ProgramExecutable final : public angle::Subject
 
     GLenum getTessGenMode() const { return mTessGenMode; }
 
+    void resetCachedValidateSamplersResult() { mCachedValidateSamplersResult.reset(); }
+    bool validateSamplers(InfoLog *infoLog, const Caps &caps) const
+    {
+        // Use the cache if:
+        // - we aren't using an infolog (which gives the full error).
+        // - The sample mapping hasn't changed and we've already validated.
+        if (infoLog == nullptr && mCachedValidateSamplersResult.valid())
+        {
+            return mCachedValidateSamplersResult.value();
+        }
+
+        return validateSamplersImpl(infoLog, caps);
+    }
+
   private:
     // TODO(timvp): http://anglebug.com/3570: Investigate removing these friend
     // class declarations and accessing the necessary members with getters/setters.
@@ -377,6 +391,8 @@ class ProgramExecutable final : public angle::Subject
 
     void updateTransformFeedbackStrides();
 
+    bool validateSamplersImpl(InfoLog *infoLog, const Caps &caps) const;
+
     InfoLog mInfoLog;
 
     ShaderBitSet mLinkedGraphicsShaderStages;
@@ -481,6 +497,9 @@ class ProgramExecutable final : public angle::Subject
     GLenum mTessGenSpacing;
     GLenum mTessGenVertexOrder;
     GLenum mTessGenPointMode;
+
+    // Cache for sampler validation
+    mutable Optional<bool> mCachedValidateSamplersResult;
 };
 }  // namespace gl
 

src/libANGLE/ProgramPipeline.cpp

@@ -656,20 +656,6 @@ void ProgramPipeline::validate(const gl::Context *context)
     }
 }
 
-bool ProgramPipeline::validateSamplers(InfoLog *infoLog, const Caps &caps)
-{
-    for (const ShaderType shaderType : gl::AllShaderTypes())
-    {
-        Program *shaderProgram = mState.mPrograms[shaderType];
-        if (shaderProgram && !shaderProgram->validateSamplers(infoLog, caps))
-        {
-            return false;
-        }
-    }
-
-    return true;
-}
-
 void ProgramPipeline::onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message)
 {
     switch (message)
@@ -682,6 +668,9 @@ void ProgramPipeline::onSubjectStateChange(angle::SubjectIndex index, angle::Sub
             mState.mIsLinked = false;
             updateExecutable();
             break;
+        case angle::SubjectMessage::SamplerUniformsUpdated:
+            getExecutable().resetCachedValidateSamplersResult();
+            break;
         default:
             UNREACHABLE();
             break;

src/libANGLE/ProgramPipeline.h

@@ -131,13 +131,6 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>,
     angle::Result link(const gl::Context *context);
     bool linkVaryings(InfoLog &infoLog) const;
     void validate(const gl::Context *context);
-    bool validateSamplers(InfoLog *infoLog, const Caps &caps);
-
-    bool usesShaderProgram(ShaderProgramID program) const
-    {
-        return mState.usesShaderProgram(program);
-    }
-
     GLboolean isValid() const { return mState.isValid(); }
 
     // ObserverInterface implementation.

src/libANGLE/validationES.cpp

@@ -3808,11 +3808,6 @@ const char *ValidateDrawStates(const Context *context)
 
         if (program)
         {
-            if (!program->validateSamplers(nullptr, context->getCaps()))
-            {
-                return kTextureTypeConflict;
-            }
-
             const char *errorMsg = ValidateProgramDrawStates(state, extensions, program);
             if (errorMsg)
             {
@@ -3829,11 +3824,6 @@ const char *ValidateDrawStates(const Context *context)
                 return errorMsg;
             }
 
-            if (!programPipeline->validateSamplers(nullptr, context->getCaps()))
-            {
-                return kTextureTypeConflict;
-            }
-
             errorMsg = ValidateProgramPipelineDrawStates(state, extensions, programPipeline);
             if (errorMsg)
             {
@@ -3855,17 +3845,25 @@ const char *ValidateDrawStates(const Context *context)
             programIsYUVOutput = executable->isYUVOutput();
         }
 
-        if (executable && executable->hasLinkedTessellationShader())
+        if (executable)
         {
-            if (!executable->hasLinkedShaderStage(ShaderType::Vertex))
+            if (!executable->validateSamplers(nullptr, context->getCaps()))
             {
-                return kTessellationShaderRequiresVertexShader;
+                return kTextureTypeConflict;
             }
 
-            if (!executable->hasLinkedShaderStage(ShaderType::TessControl) ||
-                !executable->hasLinkedShaderStage(ShaderType::TessEvaluation))
+            if (executable->hasLinkedTessellationShader())
             {
-                return kTessellationShaderRequiresBothControlAndEvaluation;
+                if (!executable->hasLinkedShaderStage(ShaderType::Vertex))
+                {
+                    return kTessellationShaderRequiresVertexShader;
+                }
+
+                if (!executable->hasLinkedShaderStage(ShaderType::TessControl) ||
+                    !executable->hasLinkedShaderStage(ShaderType::TessEvaluation))
+                {
+                    return kTessellationShaderRequiresBothControlAndEvaluation;
+                }
             }
         }
 

src/tests/gl_tests/ProgramPipelineTest.cpp

@@ -459,10 +459,6 @@ TEST_P(ProgramPipelineTest31, DifferentTextureTypes)
     // Only the Vulkan backend supports PPO
     ANGLE_SKIP_TEST_IF(!IsVulkan());
 
-    // TODO (timvp): Fix this test for Vulkan with PPO
-    // http://anglebug.com/3570
-    ANGLE_SKIP_TEST_IF(IsVulkan());
-
     // Per the OpenGL ES 3.1 spec:
     //
     // It is not allowed to have variables of different sampler types pointing to the same texture