Commit d7518622 by Jamie Madill Committed by Commit Bot

Buffer11: Refactor Subject/Observer pattern.

Instead of having a direct/static observer distinction, add two messages for 'Contents Changed' and 'Storage Changed'. This makes Buffer11 itself the subject with two different message handling cases in the onSubjectStateChange methods. Bug: angleproject:2389 Change-Id: I645cd4b7cc7ce51cb7f48a01c7fc72939cbe89fe Reviewed-on: https://chromium-review.googlesource.com/957940 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarYuly Novikov <ynovikov@chromium.org>
parent e403eef9
......@@ -353,9 +353,9 @@ Error FramebufferAttachmentObject::getAttachmentRenderTarget(
return getAttachmentImpl()->getAttachmentRenderTarget(context, binding, imageIndex, rtOut);
}
void FramebufferAttachmentObject::onStateChange(const gl::Context *context) const
void FramebufferAttachmentObject::onStorageChange(const gl::Context *context) const
{
return getAttachmentImpl()->onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
return getAttachmentImpl()->onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
}
angle::Subject *FramebufferAttachmentObject::getSubject() const
......
......@@ -210,7 +210,7 @@ class FramebufferAttachmentObject
Error initializeContents(const Context *context, const ImageIndex &imageIndex);
void onStateChange(const gl::Context *context) const;
void onStorageChange(const gl::Context *context) const;
angle::Subject *getSubject() const;
protected:
......
......@@ -27,7 +27,8 @@ using SubjectIndex = size_t;
enum class SubjectMessage
{
STATE_CHANGE,
CONTENTS_CHANGED,
STORAGE_CHANGED,
DEPENDENT_DIRTY_BITS,
};
......
......@@ -36,7 +36,7 @@ TEST(ObserverTest, BasicUsage)
binding.bind(&subject);
ASSERT_FALSE(observer.wasNotified);
subject.onStateChange(nullptr, SubjectMessage::STATE_CHANGE);
subject.onStateChange(nullptr, SubjectMessage::STORAGE_CHANGED);
ASSERT_TRUE(observer.wasNotified);
}
......
......@@ -109,7 +109,7 @@ Error Renderbuffer::setStorage(const Context *context,
mState.update(static_cast<GLsizei>(width), static_cast<GLsizei>(height), Format(internalformat),
0, InitState::MayNeedInit);
onStateChange(context);
onStorageChange(context);
return NoError();
}
......@@ -126,7 +126,7 @@ Error Renderbuffer::setStorageMultisample(const Context *context,
mState.update(static_cast<GLsizei>(width), static_cast<GLsizei>(height), Format(internalformat),
static_cast<GLsizei>(samples), InitState::MayNeedInit);
onStateChange(context);
onStorageChange(context);
return NoError();
}
......@@ -140,7 +140,7 @@ Error Renderbuffer::setStorageEGLImageTarget(const Context *context, egl::Image
mState.update(static_cast<GLsizei>(image->getWidth()), static_cast<GLsizei>(image->getHeight()),
Format(image->getFormat()), 0, image->sourceInitState());
onStateChange(context);
onStorageChange(context);
return NoError();
}
......
......@@ -156,7 +156,7 @@ void Surface::postSwap(const gl::Context *context)
if (mRobustResourceInitialization && mSwapBehavior != EGL_BUFFER_PRESERVED)
{
mInitState = gl::InitState::MayNeedInit;
onStateChange(context);
onStorageChange(context);
}
}
......
......@@ -922,7 +922,7 @@ egl::Stream *Texture::getBoundStream() const
void Texture::signalDirty(const Context *context, InitState initState)
{
mState.mInitState = initState;
onStateChange(context);
onStorageChange(context);
invalidateCompletenessCache();
}
......
......@@ -399,9 +399,8 @@ gl::Error Buffer11::setSubData(const gl::Context *context,
ANGLE_TRY(writeBuffer->setData(static_cast<const uint8_t *>(data), offset, size));
onStorageUpdate(writeBuffer);
// Notify any vertex arrays that we have dirty data.
// TODO(jmadill): Use a more fine grained notification for data updates.
mDirectSubject.onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
// Notify when data changes.
onStateChange(context, angle::SubjectMessage::CONTENTS_CHANGED);
}
mSize = std::max(mSize, requiredSize);
......@@ -471,8 +470,8 @@ gl::Error Buffer11::copySubData(const gl::Context *context,
mSize = std::max<size_t>(mSize, destOffset + size);
invalidateStaticData(context);
// Also notify that direct buffers are dirty.
mDirectSubject.onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
// Also notify that the contents are dirty.
onStateChange(context, angle::SubjectMessage::CONTENTS_CHANGED);
return gl::NoError();
}
......@@ -758,7 +757,7 @@ Buffer11::BufferStorage *Buffer11::allocateStorage(BufferUsage usage)
return new EmulatedIndexedStorage(mRenderer);
case BUFFER_USAGE_INDEX:
case BUFFER_USAGE_VERTEX_OR_TRANSFORM_FEEDBACK:
return new NativeStorage(mRenderer, usage, &mDirectSubject);
return new NativeStorage(mRenderer, usage, this);
default:
return new NativeStorage(mRenderer, usage, nullptr);
}
......@@ -919,27 +918,13 @@ bool Buffer11::supportsDirectBinding() const
void Buffer11::initializeStaticData(const gl::Context *context)
{
BufferD3D::initializeStaticData(context);
// Notify when static data changes.
mStaticSubject.onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
}
void Buffer11::invalidateStaticData(const gl::Context *context)
{
BufferD3D::invalidateStaticData(context);
// Notify when static data changes.
mStaticSubject.onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
}
angle::Subject *Buffer11::getStaticSubject()
{
return &mStaticSubject;
}
angle::Subject *Buffer11::getDirectSubject()
{
return &mDirectSubject;
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
}
void Buffer11::onCopyStorage(BufferStorage *dest, BufferStorage *source)
......@@ -1109,7 +1094,7 @@ gl::Error Buffer11::NativeStorage::resize(const gl::Context *context,
// Notify that the storage has changed.
if (mOnStorageChanged)
{
mOnStorageChanged->onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
mOnStorageChanged->onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
}
return gl::NoError();
......
......@@ -47,7 +47,10 @@ enum BufferUsage
typedef size_t DataRevision;
class Buffer11 : public BufferD3D
// We use two set of Subject messages. The CONTENTS_CHANGED message is signaled whenever data
// changes, to trigger re-translation or other events. Some buffers only need to be updated when the
// underlying ID3D11Buffer object changes - this is notified via the STORAGE_CHANGED message.
class Buffer11 : public BufferD3D, public angle::Subject
{
public:
Buffer11(const gl::BufferState &state, Renderer11 *renderer);
......@@ -104,12 +107,6 @@ class Buffer11 : public BufferD3D
gl::Error unmap(const gl::Context *context, GLboolean *result) override;
gl::Error markTransformFeedbackUsage(const gl::Context *context) override;
// We use two set of dirty events. Static buffers are marked dirty whenever
// data changes, because they must be re-translated. Direct buffers only need to be
// updated when the underlying ID3D11Buffer pointer changes - hopefully far less often.
angle::Subject *getStaticSubject();
angle::Subject *getDirectSubject();
private:
class BufferStorage;
class EmulatedIndexedStorage;
......@@ -179,9 +176,6 @@ class Buffer11 : public BufferD3D
ConstantBufferCache mConstantBufferRangeStoragesCache;
size_t mConstantBufferStorageAdditionalSize;
unsigned int mMaxConstantBufferLruCount;
angle::Subject mStaticSubject;
angle::Subject mDirectSubject;
};
} // namespace rx
......
......@@ -3262,7 +3262,7 @@ gl::Error StateManager11::syncUniformBuffers(const gl::Context *context, Program
ID3D11DeviceContext *deviceContext = mRenderer->getDeviceContext();
ID3D11DeviceContext1 *deviceContext1 = mRenderer->getDeviceContext1IfSupported();
mOnConstantBufferDirtyReceiver.reset();
mConstantBufferObserver.reset();
for (size_t bufferIndex = 0; bufferIndex < vertexUniformBuffers.size(); bufferIndex++)
{
......@@ -3317,7 +3317,7 @@ gl::Error StateManager11::syncUniformBuffers(const gl::Context *context, Program
mCurrentConstantBufferVSOffset[appliedIndex] = uniformBufferOffset;
mCurrentConstantBufferVSSize[appliedIndex] = uniformBufferSize;
mOnConstantBufferDirtyReceiver.bindVS(bufferIndex, bufferStorage);
mConstantBufferObserver.bindVS(bufferIndex, bufferStorage);
}
for (size_t bufferIndex = 0; bufferIndex < fragmentUniformBuffers.size(); bufferIndex++)
......@@ -3372,7 +3372,7 @@ gl::Error StateManager11::syncUniformBuffers(const gl::Context *context, Program
mCurrentConstantBufferPSOffset[appliedIndex] = uniformBufferOffset;
mCurrentConstantBufferPSSize[appliedIndex] = uniformBufferSize;
mOnConstantBufferDirtyReceiver.bindPS(bufferIndex, bufferStorage);
mConstantBufferObserver.bindPS(bufferIndex, bufferStorage);
}
return gl::NoError();
......@@ -3414,8 +3414,8 @@ gl::Error StateManager11::syncTransformFeedbackBuffers(const gl::Context *contex
return gl::NoError();
}
// OnConstantBufferDirtyReceiver implementation.
StateManager11::OnConstantBufferDirtyReceiver::OnConstantBufferDirtyReceiver()
// ConstantBufferObserver implementation.
StateManager11::ConstantBufferObserver::ConstantBufferObserver()
{
for (size_t vsIndex = 0; vsIndex < gl::IMPLEMENTATION_MAX_VERTEX_SHADER_UNIFORM_BUFFERS;
++vsIndex)
......@@ -3430,34 +3430,37 @@ StateManager11::OnConstantBufferDirtyReceiver::OnConstantBufferDirtyReceiver()
}
}
StateManager11::OnConstantBufferDirtyReceiver::~OnConstantBufferDirtyReceiver()
StateManager11::ConstantBufferObserver::~ConstantBufferObserver()
{
}
void StateManager11::OnConstantBufferDirtyReceiver::onSubjectStateChange(
const gl::Context *context,
angle::SubjectIndex index,
angle::SubjectMessage message)
void StateManager11::ConstantBufferObserver::onSubjectStateChange(const gl::Context *context,
angle::SubjectIndex index,
angle::SubjectMessage message)
{
StateManager11 *stateManager = GetImplAs<Context11>(context)->getRenderer()->getStateManager();
stateManager->invalidateProgramUniformBuffers();
if (message == angle::SubjectMessage::STORAGE_CHANGED)
{
StateManager11 *stateManager =
GetImplAs<Context11>(context)->getRenderer()->getStateManager();
stateManager->invalidateProgramUniformBuffers();
}
}
void StateManager11::OnConstantBufferDirtyReceiver::bindVS(size_t index, Buffer11 *buffer)
void StateManager11::ConstantBufferObserver::bindVS(size_t index, Buffer11 *buffer)
{
ASSERT(buffer);
ASSERT(index < mBindingsVS.size());
mBindingsVS[index].bind(buffer->getDirectSubject());
mBindingsVS[index].bind(buffer);
}
void StateManager11::OnConstantBufferDirtyReceiver::bindPS(size_t index, Buffer11 *buffer)
void StateManager11::ConstantBufferObserver::bindPS(size_t index, Buffer11 *buffer)
{
ASSERT(buffer);
ASSERT(index < mBindingsPS.size());
mBindingsPS[index].bind(buffer->getDirectSubject());
mBindingsPS[index].bind(buffer);
}
void StateManager11::OnConstantBufferDirtyReceiver::reset()
void StateManager11::ConstantBufferObserver::reset()
{
for (angle::ObserverBinding &vsBinding : mBindingsVS)
{
......
......@@ -552,11 +552,11 @@ class StateManager11 final : angle::NonCopyable
FragmentConstantBufferArray<GLintptr> mCurrentConstantBufferPSOffset;
FragmentConstantBufferArray<GLsizeiptr> mCurrentConstantBufferPSSize;
class OnConstantBufferDirtyReceiver : public angle::ObserverInterface
class ConstantBufferObserver : public angle::ObserverInterface
{
public:
OnConstantBufferDirtyReceiver();
~OnConstantBufferDirtyReceiver() override;
ConstantBufferObserver();
~ConstantBufferObserver() override;
void onSubjectStateChange(const gl::Context *context,
angle::SubjectIndex index,
......@@ -570,7 +570,7 @@ class StateManager11 final : angle::NonCopyable
std::vector<angle::ObserverBinding> mBindingsVS;
std::vector<angle::ObserverBinding> mBindingsPS;
};
OnConstantBufferDirtyReceiver mOnConstantBufferDirtyReceiver;
ConstantBufferObserver mConstantBufferObserver;
// Currently applied transform feedback buffers
Serial mAppliedTFSerial;
......
......@@ -18,26 +18,6 @@ using namespace angle;
namespace rx
{
namespace
{
angle::Subject *GetBufferSubject(Buffer11 *buffer11, IndexStorageType storageType)
{
switch (storageType)
{
case IndexStorageType::Direct:
return buffer11->getDirectSubject();
case IndexStorageType::Static:
return buffer11->getStaticSubject();
case IndexStorageType::Dynamic:
return buffer11 ? buffer11->getStaticSubject() : nullptr;
default:
UNREACHABLE();
return nullptr;
}
}
} // anonymous namespace
VertexArray11::VertexArray11(const gl::VertexArrayState &data)
: VertexArrayImpl(data),
mAttributeStorageTypes(data.getMaxAttribs(), VertexStorageType::CURRENT_VALUE),
......@@ -157,10 +137,8 @@ bool VertexArray11::updateElementArrayStorage(const gl::Context *context,
{
Buffer11 *newBuffer11 = SafeGetImplAs<Buffer11>(newBuffer);
angle::Subject *subject = GetBufferSubject(newBuffer11, newStorageType);
mCurrentElementArrayStorage = newStorageType;
mOnElementArrayBufferDataDirty.bind(subject);
mOnElementArrayBufferDataDirty.bind(newBuffer11);
needsTranslation = true;
}
......@@ -216,38 +194,18 @@ void VertexArray11::updateVertexAttribStorage(const gl::Context *context, size_t
stateManager->invalidateVertexAttributeTranslation();
gl::Buffer *oldBufferGL = mCurrentArrayBuffers[attribIndex].get();
gl::Buffer *newBufferGL = binding.getBuffer().get();
gl::Buffer *newBufferGL = attrib.enabled ? binding.getBuffer().get() : nullptr;
Buffer11 *oldBuffer11 = oldBufferGL ? GetImplAs<Buffer11>(oldBufferGL) : nullptr;
Buffer11 *newBuffer11 = newBufferGL ? GetImplAs<Buffer11>(newBufferGL) : nullptr;
if (oldBuffer11 != newBuffer11 || oldStorageType != newStorageType)
{
angle::Subject *subject = nullptr;
if (newStorageType == VertexStorageType::CURRENT_VALUE)
{
stateManager->invalidateCurrentValueAttrib(attribIndex);
}
else if (newBuffer11 != nullptr)
{
// Note that for static callbacks, promotion to a static buffer from a dynamic buffer
// means we need to tag dynamic buffers with static callbacks.
switch (newStorageType)
{
case VertexStorageType::DIRECT:
subject = newBuffer11->getDirectSubject();
break;
case VertexStorageType::STATIC:
case VertexStorageType::DYNAMIC:
subject = newBuffer11->getStaticSubject();
break;
default:
UNREACHABLE();
break;
}
}
mOnArrayBufferDataDirty[attribIndex].bind(subject);
mOnArrayBufferDataDirty[attribIndex].bind(newBuffer11);
mCurrentArrayBuffers[attribIndex].set(context, binding.getBuffer().get());
}
}
......@@ -365,11 +323,15 @@ void VertexArray11::onSubjectStateChange(const gl::Context *context,
ASSERT(mAttributeStorageTypes[index] != VertexStorageType::CURRENT_VALUE);
// This can change a buffer's storage, we'll need to re-check.
mAttribsToUpdate.set(index);
if (mAttributeStorageTypes[index] != VertexStorageType::DIRECT ||
message != angle::SubjectMessage::CONTENTS_CHANGED)
{
mAttribsToUpdate.set(index);
// Changing the vertex attribute state can affect the vertex shader.
Renderer11 *renderer = GetImplAs<Context11>(context)->getRenderer();
renderer->getStateManager()->invalidateShaders();
// Changing the vertex attribute state can affect the vertex shader.
Renderer11 *renderer = GetImplAs<Context11>(context)->getRenderer();
renderer->getStateManager()->invalidateShaders();
}
}
}
......
......@@ -84,7 +84,7 @@ gl::Error BufferVk::setData(const gl::Context *context,
ANGLE_TRY(setDataImpl(contextVk, static_cast<const uint8_t *>(data), size, 0));
}
onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
return gl::NoError();
}
......@@ -100,7 +100,7 @@ gl::Error BufferVk::setSubData(const gl::Context *context,
ContextVk *contextVk = vk::GetImpl(context);
ANGLE_TRY(setDataImpl(contextVk, static_cast<const uint8_t *>(data), size, offset));
onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
return gl::NoError();
}
......@@ -124,7 +124,7 @@ gl::Error BufferVk::map(const gl::Context *context, GLenum access, void **mapPtr
ANGLE_TRY(
mBufferMemory.map(device, 0, mState.getSize(), 0, reinterpret_cast<uint8_t **>(mapPtr)));
onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
return gl::NoError();
}
......@@ -141,7 +141,7 @@ gl::Error BufferVk::mapRange(const gl::Context *context,
ANGLE_TRY(mBufferMemory.map(device, offset, length, 0, reinterpret_cast<uint8_t **>(mapPtr)));
onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
return gl::NoError();
}
......@@ -154,7 +154,7 @@ gl::Error BufferVk::unmap(const gl::Context *context, GLboolean *result)
mBufferMemory.unmap(device);
onStateChange(context, angle::SubjectMessage::STATE_CHANGE);
onStateChange(context, angle::SubjectMessage::STORAGE_CHANGED);
return gl::NoError();
}
......
......@@ -1482,7 +1482,7 @@ void LineLoopHandler::onSubjectStateChange(const gl::Context *context,
angle::SubjectMessage message)
{
// Indicate we want to recopy on next draw since something changed in the buffer.
if (message == angle::SubjectMessage::STATE_CHANGE)
if (message == angle::SubjectMessage::STORAGE_CHANGED)
{
mLineLoopIndexBuffer = VK_NULL_HANDLE;
}
......
......@@ -277,6 +277,65 @@ TEST_P(StateChangeTest, FramebufferIncompleteDepthStencilAttachment)
ASSERT_GL_NO_ERROR();
}
const char kSimpleAttributeVS[] = R"(attribute vec2 position;
attribute vec4 testAttrib;
varying vec4 testVarying;
void main()
{
gl_Position = vec4(position, 0, 1);
testVarying = testAttrib;
})";
const char kSimpleAttributeFS[] = R"(precision mediump float;
varying vec4 testVarying;
void main()
{
gl_FragColor = testVarying;
})";
// Tests that using a buffered attribute, then disabling it and using current value, works.
TEST_P(StateChangeTest, DisablingBufferedVertexAttribute)
{
ANGLE_GL_PROGRAM(program, kSimpleAttributeVS, kSimpleAttributeFS);
glUseProgram(program);
GLint attribLoc = glGetAttribLocation(program, "testAttrib");
GLint positionLoc = glGetAttribLocation(program, "position");
ASSERT_NE(-1, attribLoc);
ASSERT_NE(-1, positionLoc);
// Set up the buffered attribute.
std::vector<GLColor> red(6, GLColor::red);
GLBuffer attribBuffer;
glBindBuffer(GL_ARRAY_BUFFER, attribBuffer);
glBufferData(GL_ARRAY_BUFFER, red.size() * sizeof(GLColor), red.data(), GL_STATIC_DRAW);
glEnableVertexAttribArray(attribLoc);
glVertexAttribPointer(attribLoc, 4, GL_UNSIGNED_BYTE, GL_TRUE, 0, nullptr);
// Also set the current value to green now.
glVertexAttrib4f(attribLoc, 0.0f, 1.0f, 0.0f, 1.0f);
// Set up the position attribute as well.
setupQuadVertexBuffer(0.5f, 1.0f);
glEnableVertexAttribArray(positionLoc);
glVertexAttribPointer(positionLoc, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
// Draw with the buffered attribute. Verify red.
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
// Draw with the disabled "current value attribute". Verify green.
glDisableVertexAttribArray(attribLoc);
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
// Verify setting buffer data on the disabled buffer doesn't change anything.
std::vector<GLColor> blue(128, GLColor::blue);
glBindBuffer(GL_ARRAY_BUFFER, attribBuffer);
glBufferData(GL_ARRAY_BUFFER, blue.size() * sizeof(GLColor), blue.data(), GL_STATIC_DRAW);
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
}
// Ensure that CopyTexSubImage3D syncs framebuffer changes.
TEST_P(StateChangeTestES3, CopyTexSubImage3DSync)
{
......
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