Commit e3b9dbef by Tim Van Patten Committed by Commit Bot

Update PPO's executable when attached program is re-linked

When a program that's being used by a PPO is successfully re-linked, the PPO's ProgramExecutable needs to be updated. This takes advantage of the subject/observer pattern that's already established between programs and PPOs to ensure only the PPOs that the program is a part of are updated. Bug: b/182409935 Test: ProgramPipelineTest31.ModifyAndRelinkShader Change-Id: Idcc11e7e819b4a9bef02bdd71afc8ea58111acc6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2770684 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarCody Northrop <cnorthrop@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org>
parent 00d872d0
...@@ -8815,6 +8815,7 @@ angle::Result Context::onProgramLink(Program *programObject) ...@@ -8815,6 +8815,7 @@ angle::Result Context::onProgramLink(Program *programObject)
if (programObject->isLinked()) if (programObject->isLinked())
{ {
ANGLE_TRY(mState.onProgramExecutableChange(this, programObject)); ANGLE_TRY(mState.onProgramExecutableChange(this, programObject));
programObject->onStateChange(angle::SubjectMessage::ProgramRelinked);
} }
mStateCache.onProgramExecutableChange(this); mStateCache.onProgramExecutableChange(this);
} }
......
...@@ -54,6 +54,9 @@ enum class SubjectMessage ...@@ -54,6 +54,9 @@ enum class SubjectMessage
// Indicates an external change to the default framebuffer. // Indicates an external change to the default framebuffer.
SurfaceChanged, SurfaceChanged,
// Indicates a separable program was successfully re-linked.
ProgramRelinked,
}; };
// The observing class inherits from this interface class. // The observing class inherits from this interface class.
......
...@@ -484,6 +484,7 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt ...@@ -484,6 +484,7 @@ class Program final : public LabeledObject, public angle::Subject, public HasAtt
void detachShader(const Context *context, Shader *shader); void detachShader(const Context *context, Shader *shader);
int getAttachedShadersCount() const; int getAttachedShadersCount() const;
// HasAttachedShaders implementation
Shader *getAttachedShader(ShaderType shaderType) const override; Shader *getAttachedShader(ShaderType shaderType) const override;
void bindAttributeLocation(GLuint index, const char *name); void bindAttributeLocation(GLuint index, const char *name);
......
...@@ -312,15 +312,19 @@ class ProgramExecutable final : public angle::Subject ...@@ -312,15 +312,19 @@ class ProgramExecutable final : public angle::Subject
GLuint getUniformIndexFromImageIndex(GLuint imageIndex) const; GLuint getUniformIndexFromImageIndex(GLuint imageIndex) const;
void saveLinkedStateInfo(const ProgramState &state); void saveLinkedStateInfo(const ProgramState &state);
std::vector<sh::ShaderVariable> getLinkedOutputVaryings(ShaderType shaderType) const std::vector<sh::ShaderVariable> &getLinkedOutputVaryings(ShaderType shaderType) const
{ {
return mLinkedOutputVaryings[shaderType]; return mLinkedOutputVaryings[shaderType];
} }
std::vector<sh::ShaderVariable> getLinkedInputVaryings(ShaderType shaderType) const std::vector<sh::ShaderVariable> &getLinkedInputVaryings(ShaderType shaderType) const
{ {
return mLinkedInputVaryings[shaderType]; return mLinkedInputVaryings[shaderType];
} }
int getLinkedShaderVersion(ShaderType shaderType) { return mLinkedShaderVersions[shaderType]; }
int getLinkedShaderVersion(ShaderType shaderType) const
{
return mLinkedShaderVersions[shaderType];
}
bool isYUVOutput() const; bool isYUVOutput() const;
......
...@@ -383,6 +383,33 @@ void ProgramPipeline::updateFragmentInoutRange() ...@@ -383,6 +383,33 @@ void ProgramPipeline::updateFragmentInoutRange()
mState.mExecutable->mFragmentInoutRange = fragmentExecutable.mFragmentInoutRange; mState.mExecutable->mFragmentInoutRange = fragmentExecutable.mFragmentInoutRange;
} }
void ProgramPipeline::updateLinkedVaryings()
{
// Need to check all of the shader stages, not just linked, so we handle Compute correctly.
for (const gl::ShaderType shaderType : kAllGraphicsShaderTypes)
{
const Program *shaderProgram = getShaderProgram(shaderType);
if (shaderProgram && shaderProgram->isLinked())
{
const ProgramExecutable &executable = shaderProgram->getExecutable();
mState.mExecutable->mLinkedOutputVaryings[shaderType] =
executable.getLinkedOutputVaryings(shaderType);
mState.mExecutable->mLinkedInputVaryings[shaderType] =
executable.getLinkedInputVaryings(shaderType);
}
}
const Program *computeProgram = getShaderProgram(ShaderType::Compute);
if (computeProgram && computeProgram->isLinked())
{
const ProgramExecutable &executable = computeProgram->getExecutable();
mState.mExecutable->mLinkedOutputVaryings[ShaderType::Compute] =
executable.getLinkedOutputVaryings(ShaderType::Compute);
mState.mExecutable->mLinkedInputVaryings[ShaderType::Compute] =
executable.getLinkedInputVaryings(ShaderType::Compute);
}
}
void ProgramPipeline::updateHasBooleans() void ProgramPipeline::updateHasBooleans()
{ {
// Need to check all of the shader stages, not just linked, so we handle Compute correctly. // Need to check all of the shader stages, not just linked, so we handle Compute correctly.
...@@ -473,6 +500,7 @@ void ProgramPipeline::updateExecutable() ...@@ -473,6 +500,7 @@ void ProgramPipeline::updateExecutable()
// All Shader ProgramExecutable properties // All Shader ProgramExecutable properties
mState.updateExecutableTextures(); mState.updateExecutableTextures();
updateLinkedVaryings();
// Must be last, since it queries things updated by earlier functions // Must be last, since it queries things updated by earlier functions
updateHasBooleans(); updateHasBooleans();
...@@ -546,7 +574,7 @@ bool ProgramPipeline::linkVaryings(InfoLog &infoLog) const ...@@ -546,7 +574,7 @@ bool ProgramPipeline::linkVaryings(InfoLog &infoLog) const
{ {
Program *previousProgram = getShaderProgram(previousShaderType); Program *previousProgram = getShaderProgram(previousShaderType);
ASSERT(previousProgram); ASSERT(previousProgram);
ProgramExecutable &previousExecutable = previousProgram->getExecutable(); const ProgramExecutable &previousExecutable = previousProgram->getExecutable();
if (!LinkValidateShaderInterfaceMatching( if (!LinkValidateShaderInterfaceMatching(
previousExecutable.getLinkedOutputVaryings(previousShaderType), previousExecutable.getLinkedOutputVaryings(previousShaderType),
...@@ -650,6 +678,10 @@ void ProgramPipeline::onSubjectStateChange(angle::SubjectIndex index, angle::Sub ...@@ -650,6 +678,10 @@ void ProgramPipeline::onSubjectStateChange(angle::SubjectIndex index, angle::Sub
mState.mIsLinked = false; mState.mIsLinked = false;
mState.updateExecutableTextures(); mState.updateExecutableTextures();
break; break;
case angle::SubjectMessage::ProgramRelinked:
mState.mIsLinked = false;
updateExecutable();
break;
default: default:
UNREACHABLE(); UNREACHABLE();
break; break;
......
...@@ -143,6 +143,7 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, ...@@ -143,6 +143,7 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>,
// ObserverInterface implementation. // ObserverInterface implementation.
void onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message) override; void onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message) override;
// HasAttachedShaders implementation
Shader *getAttachedShader(ShaderType shaderType) const override; Shader *getAttachedShader(ShaderType shaderType) const override;
private: private:
...@@ -154,6 +155,7 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, ...@@ -154,6 +155,7 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>,
void updateExecutableGeometryProperties(); void updateExecutableGeometryProperties();
void updateExecutableTessellationProperties(); void updateExecutableTessellationProperties();
void updateFragmentInoutRange(); void updateFragmentInoutRange();
void updateLinkedVaryings();
void updateHasBooleans(); void updateHasBooleans();
void updateExecutable(); void updateExecutable();
......
...@@ -692,6 +692,65 @@ TEST_P(ProgramPipelineTest31, VaryingIOBlockSeparableProgram) ...@@ -692,6 +692,65 @@ TEST_P(ProgramPipelineTest31, VaryingIOBlockSeparableProgram)
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red); EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
} }
// Test modifying a shader and re-linking it updates the PPO too
TEST_P(ProgramPipelineTest31, ModifyAndRelinkShader)
{
ANGLE_SKIP_TEST_IF(!IsVulkan());
const GLchar *vertString = essl31_shaders::vs::Simple();
const GLchar *fragStringGreen = essl31_shaders::fs::Green();
const GLchar *fragStringRed = essl31_shaders::fs::Red();
GLShader vertShader(GL_VERTEX_SHADER);
GLShader fragShader(GL_FRAGMENT_SHADER);
mVertProg = glCreateProgram();
mFragProg = glCreateProgram();
// Compile and link a separable vertex shader
glShaderSource(vertShader, 1, &vertString, nullptr);
glCompileShader(vertShader);
glProgramParameteri(mVertProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
glAttachShader(mVertProg, vertShader);
glLinkProgram(mVertProg);
EXPECT_GL_NO_ERROR();
// Compile and link a separable fragment shader
glShaderSource(fragShader, 1, &fragStringGreen, nullptr);
glCompileShader(fragShader);
glProgramParameteri(mFragProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
glAttachShader(mFragProg, fragShader);
glLinkProgram(mFragProg);
EXPECT_GL_NO_ERROR();
// Generate a program pipeline and attach the programs
glGenProgramPipelines(1, &mPipeline);
glUseProgramStages(mPipeline, GL_VERTEX_SHADER_BIT, mVertProg);
glUseProgramStages(mPipeline, GL_FRAGMENT_SHADER_BIT, mFragProg);
glBindProgramPipeline(mPipeline);
EXPECT_GL_NO_ERROR();
// Draw once to ensure this worked fine
drawQuadWithPPO("a_position", 0.5f, 1.0f);
ASSERT_GL_NO_ERROR();
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
// Detach the fragment shader and modify it such that it no longer fits with this pipeline
glDetachShader(mFragProg, fragShader);
// Modify the FS and re-link it
glShaderSource(fragShader, 1, &fragStringRed, nullptr);
glCompileShader(fragShader);
glProgramParameteri(mFragProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
glAttachShader(mFragProg, fragShader);
glLinkProgram(mFragProg);
EXPECT_GL_NO_ERROR();
// Draw with the PPO again and verify it's now red
drawQuadWithPPO("a_position", 0.5f, 1.0f);
ASSERT_GL_NO_ERROR();
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
}
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ProgramPipelineTest); GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ProgramPipelineTest);
ANGLE_INSTANTIATE_TEST_ES3_AND_ES31(ProgramPipelineTest); ANGLE_INSTANTIATE_TEST_ES3_AND_ES31(ProgramPipelineTest);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment