Detach separable shaders

src/libANGLE/Context.cpp

@@ -859,7 +859,7 @@ GLuint Context::createShaderProgramv(ShaderType type, GLsizei count, const GLcha
                 // We must wait to mark the program separable until it's successfully compiled.
                 programObject->setSeparable(true);
 
-                programObject->attachShader(this, shaderObject);
+                programObject->attachShader(shaderObject);
 
                 if (programObject->link(this) != angle::Result::Continue)
                 {
@@ -5915,7 +5915,7 @@ void Context::attachShader(ShaderProgramID program, ShaderProgramID shader)
     Program *programObject = mState.mShaderProgramManager->getProgram(program);
     Shader *shaderObject   = mState.mShaderProgramManager->getShader(shader);
     ASSERT(programObject && shaderObject);
-    programObject->attachShader(this, shaderObject);
+    programObject->attachShader(shaderObject);
 }
 
 void Context::copyBufferSubData(BufferBinding readTarget,

src/libANGLE/GLES1Renderer.cpp

@@ -559,8 +559,8 @@ angle::Result GLES1Renderer::linkProgram(Context *context,
 
     *programOut = program;
 
-    programObject->attachShader(context, getShader(vertexShader));
-    programObject->attachShader(context, getShader(fragmentShader));
+    programObject->attachShader(getShader(vertexShader));
+    programObject->attachShader(getShader(fragmentShader));
 
     for (auto it : attribLocs)
     {

src/libANGLE/Program.cpp

@@ -1166,7 +1166,6 @@ ImageBinding::~ImageBinding() = default;
 ProgramState::ProgramState()
     : mLabel(),
       mAttachedShaders{},
-      mAttachedShadersMarkedForDetach{},
       mLocationsUsedForXfbExtension(0),
       mAtomicCounterUniformRange(0, 0),
       mYUVOutput(false),
@@ -1342,8 +1341,7 @@ void Program::onDestroy(const Context *context)
         if (mState.mAttachedShaders[shaderType])
         {
             mState.mAttachedShaders[shaderType]->release(context);
-            mState.mAttachedShaders[shaderType]                = nullptr;
-            mState.mAttachedShadersMarkedForDetach[shaderType] = false;
+            mState.mAttachedShaders[shaderType] = nullptr;
         }
     }
 
@@ -1372,21 +1370,11 @@ const std::string &Program::getLabel() const
     return mState.mLabel;
 }
 
-void Program::attachShader(const Context *context, Shader *shader)
+void Program::attachShader(Shader *shader)
 {
-    resolveLink(context);
     ShaderType shaderType = shader->getType();
     ASSERT(shaderType != ShaderType::InvalidEnum);
 
-    // Since detachShader doesn't actually detach anymore, we need to do that work when attaching a
-    // new shader to make sure we don't lose track of it and free the resources.
-    if (mState.mAttachedShaders[shaderType])
-    {
-        mState.mAttachedShaders[shaderType]->release(context);
-        mState.mAttachedShaders[shaderType]                = nullptr;
-        mState.mAttachedShadersMarkedForDetach[shaderType] = false;
-    }
-
     mState.mAttachedShaders[shaderType] = shader;
     mState.mAttachedShaders[shaderType]->addRef();
 }
@@ -1398,19 +1386,8 @@ void Program::detachShader(const Context *context, Shader *shader)
     ASSERT(shaderType != ShaderType::InvalidEnum);
 
     ASSERT(mState.mAttachedShaders[shaderType] == shader);
-
-    if (isSeparable())
-    {
-        // Don't actually detach the shader since we still need it in case this
-        // Program is part of a Program Pipeline Object. Instead, leave a mark
-        // that indicates we intended to.
-        mState.mAttachedShadersMarkedForDetach[shaderType] = true;
-        return;
-    }
-
     shader->release(context);
-    mState.mAttachedShaders[shaderType]                = nullptr;
-    mState.mAttachedShadersMarkedForDetach[shaderType] = false;
+    mState.mAttachedShaders[shaderType] = nullptr;
 }
 
 int Program::getAttachedShadersCount() const

src/libANGLE/Program.h

@@ -337,20 +337,11 @@ class ProgramState final : angle::NonCopyable
     bool hasEarlyFragmentTestsOptimization() const { return mEarlyFramentTestsOptimization; }
     rx::SpecConstUsageBits getSpecConstUsageBits() const { return mSpecConstUsageBits; }
 
-    bool isShaderMarkedForDetach(gl::ShaderType shaderType) const
-    {
-        return mAttachedShadersMarkedForDetach[shaderType];
-    }
-
     // A Program can only either be graphics or compute, but never both, so it
     // can answer isCompute() based on which shaders it has.
     bool isCompute() const { return mExecutable->hasLinkedShaderStage(ShaderType::Compute); }
 
     const std::string &getLabel() const { return mLabel; }
-    const ShaderMap<bool> &getAttachedShadersMarkedForDetach() const
-    {
-        return mAttachedShadersMarkedForDetach;
-    }
 
     uint32_t getLocationsUsedForXfbExtension() const { return mLocationsUsedForXfbExtension; }
 
@@ -388,7 +379,6 @@ class ProgramState final : angle::NonCopyable
     sh::WorkGroupSize mComputeShaderLocalSize;
 
     ShaderMap<Shader *> mAttachedShaders;
-    ShaderMap<bool> mAttachedShadersMarkedForDetach;
 
     uint32_t mLocationsUsedForXfbExtension;
     std::vector<std::string> mTransformFeedbackVaryingNames;
@@ -480,7 +470,7 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt
         return mProgram;
     }
 
-    void attachShader(const Context *context, Shader *shader);
+    void attachShader(Shader *shader);
     void detachShader(const Context *context, Shader *shader);
     int getAttachedShadersCount() const;
 

src/libANGLE/capture/frame_capture_utils.cpp

@@ -1035,14 +1035,6 @@ void SerializeProgramState(JsonSerializer *json, const gl::ProgramState &program
             json->addScalar("Handle", 0);
         }
     }
-    const gl::ShaderMap<bool> &sm = programState.getAttachedShadersMarkedForDetach();
-    for (gl::ShaderType shaderType : gl::AllShaderTypes())
-    {
-        if (sm[shaderType])
-        {
-            json->addCString(gl::ShaderTypeToString(shaderType), "Attached");
-        }
-    }
     json->addScalar("LocationsUsedForXfbExtension", programState.getLocationsUsedForXfbExtension());
     for (const std::string &transformFeedbackVaryingName :
          programState.getTransformFeedbackVaryingNames())

src/libANGLE/validationES2.cpp

@@ -3709,8 +3709,7 @@ bool ValidateAttachShader(const Context *context, ShaderProgramID program, Shade
         return false;
     }
 
-    if (programObject->getAttachedShader(shaderObject->getType()) &&
-        !programObject->getState().isShaderMarkedForDetach(shaderObject->getType()))
+    if (programObject->getAttachedShader(shaderObject->getType()))
     {
         context->validationError(GL_INVALID_OPERATION, kShaderAttachmentHasShader);
         return false;

src/tests/gl_tests/ProgramPipelineTest.cpp

@@ -395,10 +395,6 @@ TEST_P(ProgramPipelineTest31, DetachAndModifyShader)
 {
     ANGLE_SKIP_TEST_IF(!IsVulkan());
 
-    // TODO (timvp): Fix this test for Vulkan with PPO
-    // http://anglebug.com/3570
-    ANGLE_SKIP_TEST_IF(IsVulkan());
-
     const GLchar *vertString = essl31_shaders::vs::Simple();
     const GLchar *fragString = essl31_shaders::fs::Green();