Commit c39d25ae by Tim Van Patten Committed by Commit Bot

Update State to check mExecutable

A user may be using Program Pipelines, rather than monolithic Programs, so State should check if mExecutable is valid, rather than mProgram, since that indicates the presence of either a PPO or a Program. Exercising these paths requires additional tests: SimpleStateChangeTestComputeES31PPO::DeleteImageTextureInUse() Texture2DTestES31PPO::TexStorage() Texture2DTestES31PPO::SingleTextureMultipleSamplers() These new tests exposed bugs in the PPO implementation where updates to the active Program's ProgramExecutable were not being propagated to the Executables of the PPO's containing that Program. In these particular cases, updates to the active samplers/images/textures were not being copied to the PPO's Executable. Bug: angleproject:3570 Test: end2end tests listed above Change-Id: I297cac2d0367f180dd7fa01a1ee7ba53996867c4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2225417 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org>
parent 707868ae
......@@ -157,11 +157,12 @@ class ProgramPipeline final : public RefCountObject<ProgramPipelineID>, public L
angle::Result syncState(const Context *context);
void setDirtyBit(DirtyBitType dirtyBitType) { mDirtyBits.set(dirtyBitType); }
void updateExecutableTextures();
private:
void updateLinkedShaderStages();
void updateExecutableAttributes();
void updateTransformFeedbackMembers();
void updateExecutableTextures();
void updateHasBooleans();
void updateExecutable();
......
......@@ -567,7 +567,9 @@ void State::reset(const Context *context)
mExecutable = nullptr;
if (mTransformFeedback.get())
{
mTransformFeedback->onBindingChanged(context, false);
}
mTransformFeedback.set(context, nullptr);
for (QueryType type : angle::AllEnums<QueryType>())
......@@ -629,7 +631,7 @@ ANGLE_INLINE void State::updateActiveTextureState(const Context *context,
}
}
if (texture && mProgram)
if (texture && mExecutable)
{
const SamplerState &samplerState =
sampler ? sampler->getSamplerState() : texture->getSamplerState();
......@@ -1539,7 +1541,9 @@ void State::invalidateTexture(TextureType type)
void State::setSamplerBinding(const Context *context, GLuint textureUnit, Sampler *sampler)
{
if (mSamplers[textureUnit].get() == sampler)
{
return;
}
mSamplers[textureUnit].set(context, sampler);
mDirtyBits.set(DIRTY_BIT_SAMPLER_BINDINGS);
......@@ -3368,22 +3372,43 @@ 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)
{
if (mProgram)
if (mExecutable)
{
TextureType type = mExecutable->getActiveSamplerTypes()[textureUnit];
Texture *activeTexture = (type != TextureType::InvalidEnum)
? getTextureForActiveSampler(type, textureUnit)
: nullptr;
updateActiveTexture(context, textureUnit, activeTexture);
updatePPOActiveTextures();
}
}
void State::onActiveTextureStateChange(const Context *context, size_t textureUnit)
{
if (mProgram)
if (mExecutable)
{
TextureType type = mExecutable->getActiveSamplerTypes()[textureUnit];
Texture *activeTexture = (type != TextureType::InvalidEnum)
......@@ -3396,7 +3421,7 @@ void State::onActiveTextureStateChange(const Context *context, size_t textureUni
void State::onImageStateChange(const Context *context, size_t unit)
{
if (mProgram)
if (mExecutable)
{
const ImageUnit &image = mImageUnits[unit];
......@@ -3414,6 +3439,8 @@ void State::onImageStateChange(const Context *context, size_t unit)
{
mDirtyObjects.set(DIRTY_OBJECT_IMAGES_INIT);
}
updatePPOActiveTextures();
}
}
......
......@@ -818,6 +818,8 @@ 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,
......
......@@ -3592,6 +3592,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context)
gl::Texture *texture = textures[textureUnit];
gl::Sampler *sampler = mState.getSampler(static_cast<uint32_t>(textureUnit));
gl::TextureType textureType = textureTypes[textureUnit];
ASSERT(textureType != gl::TextureType::InvalidEnum);
// Null textures represent incomplete textures.
if (texture == nullptr)
......
......@@ -1449,6 +1449,56 @@ void main()
GLuint mTexture = 0;
};
class ImageES31PPO
{
protected:
ImageES31PPO() : mComputeProg(0), mPipeline(0) {}
void bindProgramPipeline(const GLchar *computeString)
{
mComputeProg = glCreateShaderProgramv(GL_COMPUTE_SHADER, 1, &computeString);
ASSERT_NE(mComputeProg, 0u);
// Generate a program pipeline and attach the programs to their respective stages
glGenProgramPipelines(1, &mPipeline);
EXPECT_GL_NO_ERROR();
glUseProgramStages(mPipeline, GL_COMPUTE_SHADER_BIT, mComputeProg);
EXPECT_GL_NO_ERROR();
glBindProgramPipeline(mPipeline);
EXPECT_GL_NO_ERROR();
glActiveShaderProgram(mPipeline, mComputeProg);
EXPECT_GL_NO_ERROR();
}
GLuint mComputeProg;
GLuint mPipeline;
};
class SimpleStateChangeTestComputeES31PPO : public ImageES31PPO, public SimpleStateChangeTest
{
protected:
SimpleStateChangeTestComputeES31PPO() : ImageES31PPO(), SimpleStateChangeTest() {}
void testTearDown() override
{
if (mFramebuffer != 0)
{
glDeleteFramebuffers(1, &mFramebuffer);
mFramebuffer = 0;
}
if (mTexture != 0)
{
glDeleteTextures(1, &mTexture);
mTexture = 0;
}
glDeleteProgramPipelines(1, &mPipeline);
}
GLuint mFramebuffer = 0;
GLuint mTexture = 0;
};
constexpr char kSimpleVertexShader[] = R"(attribute vec2 position;
attribute vec4 color;
varying vec4 vColor;
......@@ -3425,6 +3475,60 @@ TEST_P(SimpleStateChangeTestComputeES31, DispatchImageTextureAThenTextureBThenTe
ASSERT_GL_NO_ERROR();
}
// Copied from SimpleStateChangeTestComputeES31::DeleteImageTextureInUse
// Tests that deleting an in-flight image texture does not immediately delete the resource.
TEST_P(SimpleStateChangeTestComputeES31PPO, DeleteImageTextureInUse)
{
ANGLE_SKIP_TEST_IF(!IsVulkan());
glGenFramebuffers(1, &mFramebuffer);
glGenTextures(1, &mTexture);
glBindTexture(GL_TEXTURE_2D, mTexture);
glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 2, 2);
EXPECT_GL_NO_ERROR();
constexpr char kCS[] = R"(#version 310 es
layout(local_size_x=2, local_size_y=2) in;
layout (rgba8, binding = 0) readonly uniform highp image2D srcImage;
layout (rgba8, binding = 1) writeonly uniform highp image2D dstImage;
void main()
{
imageStore(dstImage, ivec2(gl_LocalInvocationID.xy),
imageLoad(srcImage, ivec2(gl_LocalInvocationID.xy)));
})";
bindProgramPipeline(kCS);
glBindImageTexture(1, mTexture, 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_RGBA8);
glBindFramebuffer(GL_READ_FRAMEBUFFER, mFramebuffer);
glFramebufferTexture2D(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTexture, 0);
ASSERT_GL_NO_ERROR();
std::array<GLColor, 4> colors = {
{GLColor::red, GLColor::green, GLColor::blue, GLColor::yellow}};
GLTexture texRead;
glBindTexture(GL_TEXTURE_2D, texRead);
glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 2, 2);
glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 2, 2, GL_RGBA, GL_UNSIGNED_BYTE, colors.data());
EXPECT_GL_NO_ERROR();
glBindImageTexture(0, texRead, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8);
glDispatchCompute(1, 1, 1);
texRead.reset();
std::array<GLColor, 4> results;
glReadPixels(0, 0, 2, 2, GL_RGBA, GL_UNSIGNED_BYTE, results.data());
EXPECT_GL_NO_ERROR();
for (int i = 0; i < 4; i++)
{
EXPECT_EQ(colors[i], results[i]);
}
}
static constexpr char kColorVS[] = R"(attribute vec2 position;
attribute vec4 color;
varying vec4 vColor;
......@@ -4731,6 +4835,7 @@ ANGLE_INSTANTIATE_TEST_ES2(SimpleStateChangeTest);
ANGLE_INSTANTIATE_TEST_ES3(SimpleStateChangeTestES3);
ANGLE_INSTANTIATE_TEST_ES31(SimpleStateChangeTestES31);
ANGLE_INSTANTIATE_TEST_ES31(SimpleStateChangeTestComputeES31);
ANGLE_INSTANTIATE_TEST_ES31(SimpleStateChangeTestComputeES31PPO);
ANGLE_INSTANTIATE_TEST_ES3(ValidationStateChangeTest);
ANGLE_INSTANTIATE_TEST_ES3(WebGL2ValidationStateChangeTest);
ANGLE_INSTANTIATE_TEST_ES31(ValidationStateChangeTestES31);
......
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