Commit e8a93c6e by James Darpinian Committed by Commit Bot

New transform feedback buffer binding rules

Detects undefined behavior when a buffer is bound to a transform feedback binding point and a non transform feedback binding point at the same time. Also moves the transform feedback buffer generic binding point out of the transform feedback object and into the context's global state, to match driver behavior. This way binding a new transform feedback object does not affect GL_TRANSFORM_FEEDBACK_BUFFER_BINDING which is similar to how VAOs work with GL_ARRAY_BUFFER_BINDING. Bug: 696345 Change-Id: If3b9306cde7cd2197a8ce35e10c3af9ee58da0b8 Reviewed-on: https://chromium-review.googlesource.com/853130 Commit-Queue: James Darpinian <jdarpinian@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 80964f97
...@@ -26,7 +26,9 @@ BufferState::BufferState() ...@@ -26,7 +26,9 @@ BufferState::BufferState()
mMapped(GL_FALSE), mMapped(GL_FALSE),
mMapPointer(nullptr), mMapPointer(nullptr),
mMapOffset(0), mMapOffset(0),
mMapLength(0) mMapLength(0),
mBindingCount(0),
mTransformFeedbackBindingCount(0)
{ {
} }
...@@ -212,4 +214,21 @@ Error Buffer::getIndexRange(const gl::Context *context, ...@@ -212,4 +214,21 @@ Error Buffer::getIndexRange(const gl::Context *context,
return NoError(); return NoError();
} }
bool Buffer::isBoundForTransformFeedbackAndOtherUse() const
{
return mState.mTransformFeedbackBindingCount > 0 &&
mState.mTransformFeedbackBindingCount != mState.mBindingCount;
}
void Buffer::onBindingChanged(bool bound, BufferBinding target)
{
ASSERT(bound || mState.mBindingCount > 0);
mState.mBindingCount += bound ? 1 : -1;
if (target == BufferBinding::TransformFeedback)
{
ASSERT(bound || mState.mTransformFeedbackBindingCount > 0);
mState.mTransformFeedbackBindingCount += bound ? 1 : -1;
}
}
} // namespace gl } // namespace gl
...@@ -59,6 +59,8 @@ class BufferState final : angle::NonCopyable ...@@ -59,6 +59,8 @@ class BufferState final : angle::NonCopyable
void *mMapPointer; void *mMapPointer;
GLint64 mMapOffset; GLint64 mMapOffset;
GLint64 mMapLength; GLint64 mMapLength;
int mBindingCount;
int mTransformFeedbackBindingCount;
}; };
class Buffer final : public RefCountObject, public LabeledObject class Buffer final : public RefCountObject, public LabeledObject
...@@ -111,6 +113,9 @@ class Buffer final : public RefCountObject, public LabeledObject ...@@ -111,6 +113,9 @@ class Buffer final : public RefCountObject, public LabeledObject
rx::BufferImpl *getImplementation() const { return mImpl; } rx::BufferImpl *getImplementation() const { return mImpl; }
bool isBoundForTransformFeedbackAndOtherUse() const;
void onBindingChanged(bool bound, BufferBinding target);
private: private:
BufferState mState; BufferState mState;
rx::BufferImpl *mImpl; rx::BufferImpl *mImpl;
......
...@@ -5700,6 +5700,15 @@ void Context::onTextureChange(const Texture *texture) ...@@ -5700,6 +5700,15 @@ void Context::onTextureChange(const Texture *texture)
mGLState.setObjectDirty(GL_TEXTURE); mGLState.setObjectDirty(GL_TEXTURE);
} }
bool Context::isCurrentTransformFeedback(const TransformFeedback *tf) const
{
return mGLState.isCurrentTransformFeedback(tf);
}
bool Context::isCurrentVertexArray(const VertexArray *va) const
{
return mGLState.isCurrentVertexArray(va);
}
void Context::genProgramPipelines(GLsizei count, GLuint *pipelines) void Context::genProgramPipelines(GLsizei count, GLuint *pipelines)
{ {
for (int i = 0; i < count; i++) for (int i = 0; i < count; i++)
......
...@@ -1164,6 +1164,9 @@ class Context final : public ValidationContext ...@@ -1164,6 +1164,9 @@ class Context final : public ValidationContext
bool isRobustResourceInitEnabled() const { return mGLState.isRobustResourceInitEnabled(); } bool isRobustResourceInitEnabled() const { return mGLState.isRobustResourceInitEnabled(); }
bool isCurrentTransformFeedback(const TransformFeedback *tf) const;
bool isCurrentVertexArray(const VertexArray *va) const;
private: private:
Error prepareForDraw(); Error prepareForDraw();
Error syncRendererState(); Error syncRendererState();
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
namespace gl namespace gl
{ {
ERRMSG(BufferBoundForTransformFeedback, "Buffer is bound for transform feedback.");
ERRMSG(BufferNotBound, "A buffer must be bound."); ERRMSG(BufferNotBound, "A buffer must be bound.");
ERRMSG(CompressedTextureDimensionsMustMatchData, ERRMSG(CompressedTextureDimensionsMustMatchData,
"Compressed texture dimensions must exactly match the dimensions of the data passed in."); "Compressed texture dimensions must exactly match the dimensions of the data passed in.");
...@@ -29,6 +30,9 @@ ERRMSG(DefaultFramebufferTarget, "It is invalid to change default FBO's attachme ...@@ -29,6 +30,9 @@ ERRMSG(DefaultFramebufferTarget, "It is invalid to change default FBO's attachme
ERRMSG(DispatchIndirectBufferNotBound, "Dispatch indirect buffer must be bound."); ERRMSG(DispatchIndirectBufferNotBound, "Dispatch indirect buffer must be bound.");
ERRMSG(DrawBufferTypeMismatch, ERRMSG(DrawBufferTypeMismatch,
"Fragment shader output type does not match the bound framebuffer attachment type."); "Fragment shader output type does not match the bound framebuffer attachment type.");
ERRMSG(ElementArrayBufferBoundForTransformFeedback,
"It is undefined behavior to use an element array buffer that is bound for transform "
"feedback.");
ERRMSG(EnumNotSupported, "Enum is not currently supported."); ERRMSG(EnumNotSupported, "Enum is not currently supported.");
ERRMSG(EnumRequiresGLES31, "Enum requires GLES 3.1"); ERRMSG(EnumRequiresGLES31, "Enum requires GLES 3.1");
ERRMSG(ES31Required, "OpenGL ES 3.1 Required"); ERRMSG(ES31Required, "OpenGL ES 3.1 Required");
...@@ -149,6 +153,11 @@ ERRMSG(OutsideOfBounds, "Parameter outside of bounds."); ...@@ -149,6 +153,11 @@ ERRMSG(OutsideOfBounds, "Parameter outside of bounds.");
ERRMSG(ParamOverflow, "The provided parameters overflow with the provided buffer."); ERRMSG(ParamOverflow, "The provided parameters overflow with the provided buffer.");
ERRMSG(PixelDataNotNull, "Pixel data must be null."); ERRMSG(PixelDataNotNull, "Pixel data must be null.");
ERRMSG(PixelDataNull, "Pixel data cannot be null."); ERRMSG(PixelDataNull, "Pixel data cannot be null.");
ERRMSG(PixelPackBufferBoundForTransformFeedback,
"It is undefined behavior to use a pixel pack buffer that is bound for transform feedback.");
ERRMSG(
PixelUnpackBufferBoundForTransformFeedback,
"It is undefined behavior to use a pixel unpack buffer that is bound for transform feedback.");
ERRMSG(ProgramDoesNotExist, "Program doesn't exist."); ERRMSG(ProgramDoesNotExist, "Program doesn't exist.");
ERRMSG(ProgramNotBound, "A program must be bound."); ERRMSG(ProgramNotBound, "A program must be bound.");
ERRMSG(ProgramNotLinked, "Program not linked."); ERRMSG(ProgramNotLinked, "Program not linked.");
...@@ -168,15 +177,22 @@ ERRMSG(StencilReferenceMaskOrMismatch, ...@@ -168,15 +177,22 @@ ERRMSG(StencilReferenceMaskOrMismatch,
ERRMSG(StrideMustBeMultipleOfType, "Stride must be a multiple of the passed in datatype."); ERRMSG(StrideMustBeMultipleOfType, "Stride must be a multiple of the passed in datatype.");
ERRMSG(TextureNotBound, "A texture must be bound."); ERRMSG(TextureNotBound, "A texture must be bound.");
ERRMSG(TextureNotPow2, "The texture is a non-power-of-two texture."); ERRMSG(TextureNotPow2, "The texture is a non-power-of-two texture.");
ERRMSG(TransformFeedbackBufferDoubleBound,
"A transform feedback buffer that would be written to is also bound to a "
"non-transform-feedback target, which would cause undefined behavior.");
ERRMSG(TransformFeedbackDoesNotExist, "Transform feedback object that does not exist."); ERRMSG(TransformFeedbackDoesNotExist, "Transform feedback object that does not exist.");
ERRMSG(TypeMismatch, ERRMSG(TypeMismatch,
"Passed in texture target and format must match the one originally used to define the " "Passed in texture target and format must match the one originally used to define the "
"texture."); "texture.");
ERRMSG(TypeNotUnsignedShortByte, "Only UNSIGNED_SHORT and UNSIGNED_BYTE types are supported."); ERRMSG(TypeNotUnsignedShortByte, "Only UNSIGNED_SHORT and UNSIGNED_BYTE types are supported.");
ERRMSG(UniformBufferBoundForTransformFeedback,
"It is undefined behavior to use an uniform buffer that is bound for transform feedback.");
ERRMSG(UniformSizeMismatch, "Uniform size does not match uniform method."); ERRMSG(UniformSizeMismatch, "Uniform size does not match uniform method.");
ERRMSG(UnknownParameter, "Unknown parameter value."); ERRMSG(UnknownParameter, "Unknown parameter value.");
ERRMSG(VertexArrayNoBuffer, "An enabled vertex array has no buffer."); ERRMSG(VertexArrayNoBuffer, "An enabled vertex array has no buffer.");
ERRMSG(VertexArrayNoBufferPointer, "An enabled vertex array has no buffer and no pointer."); ERRMSG(VertexArrayNoBufferPointer, "An enabled vertex array has no buffer and no pointer.");
ERRMSG(VertexBufferBoundForTransformFeedback,
"It is undefined behavior to use a vertex buffer that is bound for transform feedback.");
ERRMSG(VertexShaderTypeMismatch, ERRMSG(VertexShaderTypeMismatch,
"Vertex shader input type does not match the type of the bound vertex attribute.") "Vertex shader input type does not match the type of the bound vertex attribute.")
ERRMSG(ViewportNegativeSize, "Viewport size cannot be negative."); ERRMSG(ViewportNegativeSize, "Viewport size cannot be negative.");
......
...@@ -124,9 +124,12 @@ class BindingPointer ...@@ -124,9 +124,12 @@ class BindingPointer
{ {
// addRef first in case newObject == mObject and this is the last reference to it. // addRef first in case newObject == mObject and this is the last reference to it.
if (newObject != nullptr) reinterpret_cast<const RefCountObjectNoID*>(newObject)->addRef(); if (newObject != nullptr) reinterpret_cast<const RefCountObjectNoID*>(newObject)->addRef();
if (mObject != nullptr) // Store the old pointer in a temporary so we can set the pointer before calling release.
reinterpret_cast<RefCountObjectNoID *>(mObject)->release(context); // Otherwise the object could still be referenced when its destructor is called.
ObjectType *oldObject = mObject;
mObject = newObject; mObject = newObject;
if (oldObject != nullptr)
reinterpret_cast<RefCountObjectNoID *>(oldObject)->release(context);
} }
ObjectType *get() const { return mObject; } ObjectType *get() const { return mObject; }
......
...@@ -470,6 +470,9 @@ class State : public angle::ObserverInterface, angle::NonCopyable ...@@ -470,6 +470,9 @@ class State : public angle::ObserverInterface, angle::NonCopyable
Error clearUnclearedActiveTextures(const Context *context); Error clearUnclearedActiveTextures(const Context *context);
bool isCurrentTransformFeedback(const TransformFeedback *tf) const;
bool isCurrentVertexArray(const VertexArray *va) const;
private: private:
void syncProgramTextures(const Context *context); void syncProgramTextures(const Context *context);
...@@ -557,10 +560,9 @@ class State : public angle::ObserverInterface, angle::NonCopyable ...@@ -557,10 +560,9 @@ class State : public angle::ObserverInterface, angle::NonCopyable
typedef std::map<GLenum, BindingPointer<Query>> ActiveQueryMap; typedef std::map<GLenum, BindingPointer<Query>> ActiveQueryMap;
ActiveQueryMap mActiveQueries; ActiveQueryMap mActiveQueries;
// Stores the currently bound buffer for each binding point. It has entries for the element // Stores the currently bound buffer for each binding point. It has an entry for the element
// array buffer and the transform feedback buffer but these should not be used. Instead these // array buffer but it should not be used. Instead this bind point is owned by the current
// bind points are respectively owned by current the vertex array object and the current // vertex array object.
// transform feedback object.
using BoundBufferMap = angle::PackedEnumMap<BufferBinding, BindingPointer<Buffer>>; using BoundBufferMap = angle::PackedEnumMap<BufferBinding, BindingPointer<Buffer>>;
BoundBufferMap mBoundBuffers; BoundBufferMap mBoundBuffers;
...@@ -571,9 +573,7 @@ class State : public angle::ObserverInterface, angle::NonCopyable ...@@ -571,9 +573,7 @@ class State : public angle::ObserverInterface, angle::NonCopyable
BindingPointer<TransformFeedback> mTransformFeedback; BindingPointer<TransformFeedback> mTransformFeedback;
BindingPointer<Buffer> mPixelUnpackBuffer;
PixelUnpackState mUnpack; PixelUnpackState mUnpack;
BindingPointer<Buffer> mPixelPackBuffer;
PixelPackState mPack; PixelPackState mPack;
bool mPrimitiveRestart; bool mPrimitiveRestart;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "libANGLE/Buffer.h" #include "libANGLE/Buffer.h"
#include "libANGLE/Caps.h" #include "libANGLE/Caps.h"
#include "libANGLE/Context.h"
#include "libANGLE/ContextState.h" #include "libANGLE/ContextState.h"
#include "libANGLE/Program.h" #include "libANGLE/Program.h"
#include "libANGLE/renderer/GLImplFactory.h" #include "libANGLE/renderer/GLImplFactory.h"
...@@ -22,7 +23,6 @@ TransformFeedbackState::TransformFeedbackState(size_t maxIndexedBuffers) ...@@ -22,7 +23,6 @@ TransformFeedbackState::TransformFeedbackState(size_t maxIndexedBuffers)
mPrimitiveMode(GL_NONE), mPrimitiveMode(GL_NONE),
mPaused(false), mPaused(false),
mProgram(nullptr), mProgram(nullptr),
mGenericBuffer(),
mIndexedBuffers(maxIndexedBuffers) mIndexedBuffers(maxIndexedBuffers)
{ {
} }
...@@ -31,11 +31,6 @@ TransformFeedbackState::~TransformFeedbackState() ...@@ -31,11 +31,6 @@ TransformFeedbackState::~TransformFeedbackState()
{ {
} }
const BindingPointer<Buffer> &TransformFeedbackState::getGenericBuffer() const
{
return mGenericBuffer;
}
const OffsetBindingPointer<Buffer> &TransformFeedbackState::getIndexedBuffer(size_t idx) const const OffsetBindingPointer<Buffer> &TransformFeedbackState::getIndexedBuffer(size_t idx) const
{ {
return mIndexedBuffers[idx]; return mIndexedBuffers[idx];
...@@ -56,6 +51,7 @@ TransformFeedback::TransformFeedback(rx::GLImplFactory *implFactory, GLuint id, ...@@ -56,6 +51,7 @@ TransformFeedback::TransformFeedback(rx::GLImplFactory *implFactory, GLuint id,
Error TransformFeedback::onDestroy(const Context *context) Error TransformFeedback::onDestroy(const Context *context)
{ {
ASSERT(!context || !context->isCurrentTransformFeedback(this));
if (mState.mProgram) if (mState.mProgram)
{ {
mState.mProgram->release(context); mState.mProgram->release(context);
...@@ -63,7 +59,6 @@ Error TransformFeedback::onDestroy(const Context *context) ...@@ -63,7 +59,6 @@ Error TransformFeedback::onDestroy(const Context *context)
} }
ASSERT(!mState.mProgram); ASSERT(!mState.mProgram);
mState.mGenericBuffer.set(context, nullptr);
for (size_t i = 0; i < mState.mIndexedBuffers.size(); i++) for (size_t i = 0; i < mState.mIndexedBuffers.size(); i++)
{ {
mState.mIndexedBuffers[i].set(context, nullptr); mState.mIndexedBuffers[i].set(context, nullptr);
...@@ -157,33 +152,22 @@ bool TransformFeedback::hasBoundProgram(GLuint program) const ...@@ -157,33 +152,22 @@ bool TransformFeedback::hasBoundProgram(GLuint program) const
return mState.mProgram != nullptr && mState.mProgram->id() == program; return mState.mProgram != nullptr && mState.mProgram->id() == program;
} }
void TransformFeedback::bindGenericBuffer(const Context *context, Buffer *buffer)
{
mState.mGenericBuffer.set(context, buffer);
mImplementation->bindGenericBuffer(mState.mGenericBuffer);
}
void TransformFeedback::detachBuffer(const Context *context, GLuint bufferName) void TransformFeedback::detachBuffer(const Context *context, GLuint bufferName)
{ {
bool isBound = context->isCurrentTransformFeedback(this);
for (size_t index = 0; index < mState.mIndexedBuffers.size(); index++) for (size_t index = 0; index < mState.mIndexedBuffers.size(); index++)
{ {
if (mState.mIndexedBuffers[index].id() == bufferName) if (mState.mIndexedBuffers[index].id() == bufferName)
{ {
if (isBound)
{
mState.mIndexedBuffers[index]->onBindingChanged(false,
BufferBinding::TransformFeedback);
}
mState.mIndexedBuffers[index].set(context, nullptr); mState.mIndexedBuffers[index].set(context, nullptr);
mImplementation->bindIndexedBuffer(index, mState.mIndexedBuffers[index]); mImplementation->bindIndexedBuffer(index, mState.mIndexedBuffers[index]);
} }
} }
if (mState.mGenericBuffer.id() == bufferName)
{
mState.mGenericBuffer.set(context, nullptr);
mImplementation->bindGenericBuffer(mState.mGenericBuffer);
}
}
const BindingPointer<Buffer> &TransformFeedback::getGenericBuffer() const
{
return mState.mGenericBuffer;
} }
void TransformFeedback::bindIndexedBuffer(const Context *context, void TransformFeedback::bindIndexedBuffer(const Context *context,
...@@ -193,7 +177,17 @@ void TransformFeedback::bindIndexedBuffer(const Context *context, ...@@ -193,7 +177,17 @@ void TransformFeedback::bindIndexedBuffer(const Context *context,
size_t size) size_t size)
{ {
ASSERT(index < mState.mIndexedBuffers.size()); ASSERT(index < mState.mIndexedBuffers.size());
bool isBound = context && context->isCurrentTransformFeedback(this);
if (isBound && mState.mIndexedBuffers[index].get())
{
mState.mIndexedBuffers[index]->onBindingChanged(false, BufferBinding::TransformFeedback);
}
mState.mIndexedBuffers[index].set(context, buffer, offset, size); mState.mIndexedBuffers[index].set(context, buffer, offset, size);
if (isBound && buffer)
{
buffer->onBindingChanged(true, BufferBinding::TransformFeedback);
}
mImplementation->bindIndexedBuffer(index, mState.mIndexedBuffers[index]); mImplementation->bindIndexedBuffer(index, mState.mIndexedBuffers[index]);
} }
...@@ -208,6 +202,18 @@ size_t TransformFeedback::getIndexedBufferCount() const ...@@ -208,6 +202,18 @@ size_t TransformFeedback::getIndexedBufferCount() const
return mState.mIndexedBuffers.size(); return mState.mIndexedBuffers.size();
} }
bool TransformFeedback::buffersBoundForOtherUse() const
{
for (auto &buffer : mState.mIndexedBuffers)
{
if (buffer.get() && buffer->isBoundForTransformFeedbackAndOtherUse())
{
return true;
}
}
return false;
}
rx::TransformFeedbackImpl *TransformFeedback::getImplementation() rx::TransformFeedbackImpl *TransformFeedback::getImplementation()
{ {
return mImplementation; return mImplementation;
...@@ -218,4 +224,14 @@ const rx::TransformFeedbackImpl *TransformFeedback::getImplementation() const ...@@ -218,4 +224,14 @@ const rx::TransformFeedbackImpl *TransformFeedback::getImplementation() const
return mImplementation; return mImplementation;
} }
void TransformFeedback::onBindingChanged(bool bound)
{
for (auto &buffer : mState.mIndexedBuffers)
{
if (buffer.get())
{
buffer->onBindingChanged(bound, BufferBinding::TransformFeedback);
}
}
}
} }
...@@ -33,7 +33,6 @@ class TransformFeedbackState final : angle::NonCopyable ...@@ -33,7 +33,6 @@ class TransformFeedbackState final : angle::NonCopyable
TransformFeedbackState(size_t maxIndexedBuffers); TransformFeedbackState(size_t maxIndexedBuffers);
~TransformFeedbackState(); ~TransformFeedbackState();
const BindingPointer<Buffer> &getGenericBuffer() const;
const OffsetBindingPointer<Buffer> &getIndexedBuffer(size_t idx) const; const OffsetBindingPointer<Buffer> &getIndexedBuffer(size_t idx) const;
const std::vector<OffsetBindingPointer<Buffer>> &getIndexedBuffers() const; const std::vector<OffsetBindingPointer<Buffer>> &getIndexedBuffers() const;
...@@ -48,7 +47,6 @@ class TransformFeedbackState final : angle::NonCopyable ...@@ -48,7 +47,6 @@ class TransformFeedbackState final : angle::NonCopyable
Program *mProgram; Program *mProgram;
BindingPointer<Buffer> mGenericBuffer;
std::vector<OffsetBindingPointer<Buffer>> mIndexedBuffers; std::vector<OffsetBindingPointer<Buffer>> mIndexedBuffers;
}; };
...@@ -73,9 +71,6 @@ class TransformFeedback final : public RefCountObject, public LabeledObject ...@@ -73,9 +71,6 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
bool hasBoundProgram(GLuint program) const; bool hasBoundProgram(GLuint program) const;
void bindGenericBuffer(const Context *context, Buffer *buffer);
const BindingPointer<Buffer> &getGenericBuffer() const;
void bindIndexedBuffer(const Context *context, void bindIndexedBuffer(const Context *context,
size_t index, size_t index,
Buffer *buffer, Buffer *buffer,
...@@ -84,11 +79,16 @@ class TransformFeedback final : public RefCountObject, public LabeledObject ...@@ -84,11 +79,16 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
const OffsetBindingPointer<Buffer> &getIndexedBuffer(size_t index) const; const OffsetBindingPointer<Buffer> &getIndexedBuffer(size_t index) const;
size_t getIndexedBufferCount() const; size_t getIndexedBufferCount() const;
// Returns true if any buffer bound to this object is also bound to another target.
bool buffersBoundForOtherUse() const;
void detachBuffer(const Context *context, GLuint bufferName); void detachBuffer(const Context *context, GLuint bufferName);
rx::TransformFeedbackImpl *getImplementation(); rx::TransformFeedbackImpl *getImplementation();
const rx::TransformFeedbackImpl *getImplementation() const; const rx::TransformFeedbackImpl *getImplementation() const;
void onBindingChanged(bool bound);
private: private:
void bindProgram(const Context *context, Program *program); void bindProgram(const Context *context, Program *program);
......
...@@ -115,10 +115,6 @@ TEST_F(TransformFeedbackTest, BufferBinding) ...@@ -115,10 +115,6 @@ TEST_F(TransformFeedbackTest, BufferBinding)
EXPECT_EQ(mFeedback->getIndexedBufferCount(), mCaps.maxTransformFeedbackSeparateAttributes); EXPECT_EQ(mFeedback->getIndexedBufferCount(), mCaps.maxTransformFeedbackSeparateAttributes);
EXPECT_CALL(*mImpl, bindGenericBuffer(_));
mFeedback->bindGenericBuffer(nullptr, buffer);
EXPECT_EQ(mFeedback->getGenericBuffer().get(), buffer);
EXPECT_CALL(*mImpl, bindIndexedBuffer(_, _)); EXPECT_CALL(*mImpl, bindIndexedBuffer(_, _));
mFeedback->bindIndexedBuffer(nullptr, bindIndex, buffer, 0, 1); mFeedback->bindIndexedBuffer(nullptr, bindIndex, buffer, 0, 1);
for (size_t i = 0; i < mFeedback->getIndexedBufferCount(); i++) for (size_t i = 0; i < mFeedback->getIndexedBufferCount(); i++)
......
...@@ -42,10 +42,13 @@ VertexArray::VertexArray(rx::GLImplFactory *factory, ...@@ -42,10 +42,13 @@ VertexArray::VertexArray(rx::GLImplFactory *factory,
void VertexArray::onDestroy(const Context *context) void VertexArray::onDestroy(const Context *context)
{ {
bool isBound = context->isCurrentVertexArray(this);
for (auto &binding : mState.mVertexBindings) for (auto &binding : mState.mVertexBindings)
{ {
binding.setBuffer(context, nullptr); binding.setBuffer(context, nullptr, isBound);
} }
if (isBound && mState.mElementArrayBuffer.get())
mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::ElementArray);
mState.mElementArrayBuffer.set(context, nullptr); mState.mElementArrayBuffer.set(context, nullptr);
mVertexArray->destroy(context); mVertexArray->destroy(context);
SafeDelete(mVertexArray); SafeDelete(mVertexArray);
...@@ -74,16 +77,19 @@ const std::string &VertexArray::getLabel() const ...@@ -74,16 +77,19 @@ const std::string &VertexArray::getLabel() const
void VertexArray::detachBuffer(const Context *context, GLuint bufferName) void VertexArray::detachBuffer(const Context *context, GLuint bufferName)
{ {
bool isBound = context->isCurrentVertexArray(this);
for (auto &binding : mState.mVertexBindings) for (auto &binding : mState.mVertexBindings)
{ {
if (binding.getBuffer().id() == bufferName) if (binding.getBuffer().id() == bufferName)
{ {
binding.setBuffer(context, nullptr); binding.setBuffer(context, nullptr, isBound);
} }
} }
if (mState.mElementArrayBuffer.id() == bufferName) if (mState.mElementArrayBuffer.id() == bufferName)
{ {
if (isBound && mState.mElementArrayBuffer.get())
mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::Array);
mState.mElementArrayBuffer.set(context, nullptr); mState.mElementArrayBuffer.set(context, nullptr);
} }
} }
...@@ -115,10 +121,11 @@ void VertexArray::bindVertexBufferImpl(const Context *context, ...@@ -115,10 +121,11 @@ void VertexArray::bindVertexBufferImpl(const Context *context,
GLsizei stride) GLsizei stride)
{ {
ASSERT(bindingIndex < getMaxBindings()); ASSERT(bindingIndex < getMaxBindings());
bool isBound = context->isCurrentVertexArray(this);
VertexBinding *binding = &mState.mVertexBindings[bindingIndex]; VertexBinding *binding = &mState.mVertexBindings[bindingIndex];
binding->setBuffer(context, boundBuffer); binding->setBuffer(context, boundBuffer, isBound);
binding->setOffset(offset); binding->setOffset(offset);
binding->setStride(stride); binding->setStride(stride);
} }
...@@ -244,7 +251,12 @@ void VertexArray::setVertexAttribPointer(const Context *context, ...@@ -244,7 +251,12 @@ void VertexArray::setVertexAttribPointer(const Context *context,
void VertexArray::setElementArrayBuffer(const Context *context, Buffer *buffer) void VertexArray::setElementArrayBuffer(const Context *context, Buffer *buffer)
{ {
bool isBound = context->isCurrentVertexArray(this);
if (isBound && mState.mElementArrayBuffer.get())
mState.mElementArrayBuffer->onBindingChanged(false, BufferBinding::ElementArray);
mState.mElementArrayBuffer.set(context, buffer); mState.mElementArrayBuffer.set(context, buffer);
if (isBound && mState.mElementArrayBuffer.get())
mState.mElementArrayBuffer->onBindingChanged(true, BufferBinding::ElementArray);
mDirtyBits.set(DIRTY_BIT_ELEMENT_ARRAY_BUFFER); mDirtyBits.set(DIRTY_BIT_ELEMENT_ARRAY_BUFFER);
} }
...@@ -257,4 +269,14 @@ void VertexArray::syncState(const Context *context) ...@@ -257,4 +269,14 @@ void VertexArray::syncState(const Context *context)
} }
} }
void VertexArray::onBindingChanged(bool bound)
{
if (mState.mElementArrayBuffer.get())
mState.mElementArrayBuffer->onBindingChanged(bound, BufferBinding::ElementArray);
for (auto &binding : mState.mVertexBindings)
{
binding.onContainerBindingChanged(bound);
}
}
} // namespace gl } // namespace gl
...@@ -13,9 +13,9 @@ ...@@ -13,9 +13,9 @@
#ifndef LIBANGLE_VERTEXARRAY_H_ #ifndef LIBANGLE_VERTEXARRAY_H_
#define LIBANGLE_VERTEXARRAY_H_ #define LIBANGLE_VERTEXARRAY_H_
#include "libANGLE/RefCountObject.h"
#include "libANGLE/Constants.h" #include "libANGLE/Constants.h"
#include "libANGLE/Debug.h" #include "libANGLE/Debug.h"
#include "libANGLE/RefCountObject.h"
#include "libANGLE/State.h" #include "libANGLE/State.h"
#include "libANGLE/VertexAttribute.h" #include "libANGLE/VertexAttribute.h"
...@@ -196,6 +196,8 @@ class VertexArray final : public LabeledObject ...@@ -196,6 +196,8 @@ class VertexArray final : public LabeledObject
ComponentTypeMask getAttributesTypeMask() const { return mState.mVertexAttributesTypeMask; } ComponentTypeMask getAttributesTypeMask() const { return mState.mVertexAttributesTypeMask; }
AttributesMask getAttributesMask() const { return mState.mEnabledAttributesMask; } AttributesMask getAttributesMask() const { return mState.mEnabledAttributesMask; }
void onBindingChanged(bool bound);
private: private:
~VertexArray() override; ~VertexArray() override;
......
...@@ -38,6 +38,21 @@ VertexBinding &VertexBinding::operator=(VertexBinding &&binding) ...@@ -38,6 +38,21 @@ VertexBinding &VertexBinding::operator=(VertexBinding &&binding)
return *this; return *this;
} }
void VertexBinding::setBuffer(const gl::Context *context, Buffer *bufferIn, bool containerIsBound)
{
if (mBuffer.get() && containerIsBound)
mBuffer->onBindingChanged(false, BufferBinding::Array);
mBuffer.set(context, bufferIn);
if (mBuffer.get() && containerIsBound)
mBuffer->onBindingChanged(true, BufferBinding::Array);
}
void VertexBinding::onContainerBindingChanged(bool bound)
{
if (mBuffer.get())
mBuffer->onBindingChanged(bound, BufferBinding::Array);
}
VertexAttribute::VertexAttribute(GLuint bindingIndex) VertexAttribute::VertexAttribute(GLuint bindingIndex)
: enabled(false), : enabled(false),
type(GL_FLOAT), type(GL_FLOAT),
......
...@@ -37,7 +37,8 @@ class VertexBinding final : angle::NonCopyable ...@@ -37,7 +37,8 @@ class VertexBinding final : angle::NonCopyable
void setOffset(GLintptr offsetIn) { mOffset = offsetIn; } void setOffset(GLintptr offsetIn) { mOffset = offsetIn; }
const BindingPointer<Buffer> &getBuffer() const { return mBuffer; } const BindingPointer<Buffer> &getBuffer() const { return mBuffer; }
void setBuffer(const gl::Context *context, Buffer *bufferIn) { mBuffer.set(context, bufferIn); } void setBuffer(const gl::Context *context, Buffer *bufferIn, bool containerIsBound);
void onContainerBindingChanged(bool bound);
private: private:
GLuint mStride; GLuint mStride;
......
...@@ -393,6 +393,10 @@ void StateManagerGL::bindVertexArray(GLuint vao, GLuint elementArrayBuffer) ...@@ -393,6 +393,10 @@ void StateManagerGL::bindVertexArray(GLuint vao, GLuint elementArrayBuffer)
void StateManagerGL::bindBuffer(gl::BufferBinding target, GLuint buffer) void StateManagerGL::bindBuffer(gl::BufferBinding target, GLuint buffer)
{ {
// GL drivers differ in whether the transform feedback bind point is modified when
// glBindTransformFeedback is called. To avoid these behavior differences we shouldn't try to
// use it.
ASSERT(target != gl::BufferBinding::TransformFeedback);
if (mBuffers[target] != buffer) if (mBuffers[target] != buffer)
{ {
mBuffers[target] = buffer; mBuffers[target] = buffer;
......
...@@ -104,7 +104,7 @@ void VertexArrayGL::destroy(const gl::Context *context) ...@@ -104,7 +104,7 @@ void VertexArrayGL::destroy(const gl::Context *context)
mAppliedElementArrayBuffer.set(context, nullptr); mAppliedElementArrayBuffer.set(context, nullptr);
for (auto &binding : mAppliedBindings) for (auto &binding : mAppliedBindings)
{ {
binding.setBuffer(context, nullptr); binding.setBuffer(context, nullptr, false);
} }
} }
...@@ -482,7 +482,7 @@ void VertexArrayGL::updateAttribPointer(const gl::Context *context, size_t attri ...@@ -482,7 +482,7 @@ void VertexArrayGL::updateAttribPointer(const gl::Context *context, size_t attri
{ {
// Mark the applied binding isn't using a buffer by setting its buffer to nullptr so that if // Mark the applied binding isn't using a buffer by setting its buffer to nullptr so that if
// it starts to use a buffer later, there is no chance that the caching will skip it. // it starts to use a buffer later, there is no chance that the caching will skip it.
mAppliedBindings[attribIndex].setBuffer(context, nullptr); mAppliedBindings[attribIndex].setBuffer(context, nullptr, false);
return; return;
} }
...@@ -521,7 +521,7 @@ void VertexArrayGL::updateAttribPointer(const gl::Context *context, size_t attri ...@@ -521,7 +521,7 @@ void VertexArrayGL::updateAttribPointer(const gl::Context *context, size_t attri
mAppliedBindings[attribIndex].setStride(binding.getStride()); mAppliedBindings[attribIndex].setStride(binding.getStride());
mAppliedBindings[attribIndex].setOffset(binding.getOffset()); mAppliedBindings[attribIndex].setOffset(binding.getOffset());
mAppliedBindings[attribIndex].setBuffer(context, binding.getBuffer().get()); mAppliedBindings[attribIndex].setBuffer(context, binding.getBuffer().get(), false);
} }
void VertexArrayGL::callVertexAttribPointer(GLuint attribIndex, void VertexArrayGL::callVertexAttribPointer(GLuint attribIndex,
...@@ -614,7 +614,7 @@ void VertexArrayGL::updateBindingBuffer(const gl::Context *context, size_t bindi ...@@ -614,7 +614,7 @@ void VertexArrayGL::updateBindingBuffer(const gl::Context *context, size_t bindi
mAppliedBindings[bindingIndex].setStride(binding.getStride()); mAppliedBindings[bindingIndex].setStride(binding.getStride());
mAppliedBindings[bindingIndex].setOffset(binding.getOffset()); mAppliedBindings[bindingIndex].setOffset(binding.getOffset());
mAppliedBindings[bindingIndex].setBuffer(context, binding.getBuffer().get()); mAppliedBindings[bindingIndex].setBuffer(context, binding.getBuffer().get(), false);
} }
void VertexArrayGL::updateBindingDivisor(size_t bindingIndex) void VertexArrayGL::updateBindingDivisor(size_t bindingIndex)
......
...@@ -178,6 +178,13 @@ bool ValidateDrawAttribs(ValidationContext *context, ...@@ -178,6 +178,13 @@ bool ValidateDrawAttribs(ValidationContext *context,
ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientVertexBufferSize); ANGLE_VALIDATION_ERR(context, InvalidOperation(), InsufficientVertexBufferSize);
return false; return false;
} }
if (webglCompatibility && buffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(),
VertexBufferBoundForTransformFeedback);
return false;
}
} }
return true; return true;
...@@ -908,6 +915,13 @@ bool ValidImageDataSize(ValidationContext *context, ...@@ -908,6 +915,13 @@ bool ValidImageDataSize(ValidationContext *context,
context->handleError(InvalidOperation()); context->handleError(InvalidOperation());
return false; return false;
} }
if (context->getExtensions().webglCompatibility &&
pixelUnpackBuffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(),
PixelUnpackBufferBoundForTransformFeedback);
return false;
}
} }
else else
{ {
...@@ -2601,11 +2615,26 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count) ...@@ -2601,11 +2615,26 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count)
<< "It is undefined behaviour to use a uniform buffer that is too small."); << "It is undefined behaviour to use a uniform buffer that is too small.");
return false; return false;
} }
if (extensions.webglCompatibility &&
uniformBuffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(),
UniformBufferBoundForTransformFeedback);
return false;
}
} }
// Do some additonal WebGL-specific validation // Do some additonal WebGL-specific validation
if (extensions.webglCompatibility) if (extensions.webglCompatibility)
{ {
const TransformFeedback *transformFeedbackObject = state.getCurrentTransformFeedback();
if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() &&
transformFeedbackObject->buffersBoundForOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), TransformFeedbackBufferDoubleBound);
return false;
}
// Detect rendering feedback loops for WebGL. // Detect rendering feedback loops for WebGL.
if (framebuffer->formsRenderingFeedbackLoopWith(state)) if (framebuffer->formsRenderingFeedbackLoopWith(state))
{ {
...@@ -2854,6 +2883,14 @@ bool ValidateDrawElementsCommon(ValidationContext *context, ...@@ -2854,6 +2883,14 @@ bool ValidateDrawElementsCommon(ValidationContext *context,
ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType); ANGLE_VALIDATION_ERR(context, InvalidOperation(), MismatchedByteCountType);
return false; return false;
} }
if (context->getExtensions().webglCompatibility &&
elementArrayBuffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(),
ElementArrayBufferBoundForTransformFeedback);
return false;
}
} }
if (context->getExtensions().robustBufferAccessBehavior) if (context->getExtensions().robustBufferAccessBehavior)
...@@ -5231,6 +5268,12 @@ bool ValidateReadPixelsBase(Context *context, ...@@ -5231,6 +5268,12 @@ bool ValidateReadPixelsBase(Context *context,
context->handleError(InvalidOperation() << "Pixel pack buffer is mapped."); context->handleError(InvalidOperation() << "Pixel pack buffer is mapped.");
return false; return false;
} }
if (context->getExtensions().webglCompatibility && pixelPackBuffer != nullptr &&
pixelPackBuffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), PixelPackBufferBoundForTransformFeedback);
return false;
}
// .. the data would be packed to the buffer object such that the memory writes required // .. the data would be packed to the buffer object such that the memory writes required
// would exceed the data store size. // would exceed the data store size.
......
...@@ -2923,6 +2923,13 @@ bool ValidateMapBufferBase(Context *context, BufferBinding target) ...@@ -2923,6 +2923,13 @@ bool ValidateMapBufferBase(Context *context, BufferBinding target)
} }
} }
if (context->getExtensions().webglCompatibility &&
buffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), BufferBoundForTransformFeedback);
return false;
}
return true; return true;
} }
...@@ -4232,6 +4239,13 @@ bool ValidateBufferData(ValidationContext *context, ...@@ -4232,6 +4239,13 @@ bool ValidateBufferData(ValidationContext *context,
return false; return false;
} }
if (context->getExtensions().webglCompatibility &&
buffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), BufferBoundForTransformFeedback);
return false;
}
return true; return true;
} }
...@@ -4273,6 +4287,13 @@ bool ValidateBufferSubData(ValidationContext *context, ...@@ -4273,6 +4287,13 @@ bool ValidateBufferSubData(ValidationContext *context,
return false; return false;
} }
if (context->getExtensions().webglCompatibility &&
buffer->isBoundForTransformFeedbackAndOtherUse())
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), BufferBoundForTransformFeedback);
return false;
}
// Check for possible overflow of size + offset // Check for possible overflow of size + offset
angle::CheckedNumeric<size_t> checkedSize(size); angle::CheckedNumeric<size_t> checkedSize(size);
checkedSize += offset; checkedSize += offset;
......
...@@ -2529,6 +2529,14 @@ bool ValidateCopyBufferSubData(ValidationContext *context, ...@@ -2529,6 +2529,14 @@ bool ValidateCopyBufferSubData(ValidationContext *context,
return false; return false;
} }
if (context->getExtensions().webglCompatibility &&
(readBuffer->isBoundForTransformFeedbackAndOtherUse() ||
writeBuffer->isBoundForTransformFeedbackAndOtherUse()))
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), BufferBoundForTransformFeedback);
return false;
}
CheckedNumeric<GLintptr> checkedReadOffset(readOffset); CheckedNumeric<GLintptr> checkedReadOffset(readOffset);
CheckedNumeric<GLintptr> checkedWriteOffset(writeOffset); CheckedNumeric<GLintptr> checkedWriteOffset(writeOffset);
CheckedNumeric<GLintptr> checkedSize(size); CheckedNumeric<GLintptr> checkedSize(size);
......
...@@ -291,7 +291,7 @@ TEST_P(TransformFeedbackTest, BufferBinding) ...@@ -291,7 +291,7 @@ TEST_P(TransformFeedbackTest, BufferBinding)
{ {
// Reset any state // Reset any state
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0); glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0);
glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, 0); glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0);
// Generate a new buffer // Generate a new buffer
GLuint scratchBuffer = 0; GLuint scratchBuffer = 0;
...@@ -301,7 +301,7 @@ TEST_P(TransformFeedbackTest, BufferBinding) ...@@ -301,7 +301,7 @@ TEST_P(TransformFeedbackTest, BufferBinding)
// Bind TF 0 and a buffer // Bind TF 0 and a buffer
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0); glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0);
glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, mTransformFeedbackBuffer); glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
...@@ -310,14 +310,21 @@ TEST_P(TransformFeedbackTest, BufferBinding) ...@@ -310,14 +310,21 @@ TEST_P(TransformFeedbackTest, BufferBinding)
glGetIntegerv(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, &currentBufferBinding); glGetIntegerv(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, &currentBufferBinding);
EXPECT_EQ(static_cast<GLuint>(currentBufferBinding), mTransformFeedbackBuffer); EXPECT_EQ(static_cast<GLuint>(currentBufferBinding), mTransformFeedbackBuffer);
glGetIntegeri_v(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, 0, &currentBufferBinding);
EXPECT_EQ(static_cast<GLuint>(currentBufferBinding), mTransformFeedbackBuffer);
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
// Check that the buffer ID for the newly bound transform feedback is zero // Check that the buffer ID for the newly bound transform feedback is zero
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, mTransformFeedback); glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, mTransformFeedback);
glGetIntegerv(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, &currentBufferBinding); glGetIntegeri_v(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, 0, &currentBufferBinding);
EXPECT_EQ(0, currentBufferBinding); EXPECT_EQ(0, currentBufferBinding);
// But the generic bind point is unaffected by glBindTransformFeedback.
glGetIntegerv(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, &currentBufferBinding);
EXPECT_EQ(static_cast<GLuint>(currentBufferBinding), mTransformFeedbackBuffer);
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
// Bind a buffer to this TF // Bind a buffer to this TF
...@@ -332,7 +339,7 @@ TEST_P(TransformFeedbackTest, BufferBinding) ...@@ -332,7 +339,7 @@ TEST_P(TransformFeedbackTest, BufferBinding)
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0); glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, 0);
glGetIntegeri_v(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, 0, &currentBufferBinding); glGetIntegeri_v(GL_TRANSFORM_FEEDBACK_BUFFER_BINDING, 0, &currentBufferBinding);
EXPECT_EQ(0, currentBufferBinding); EXPECT_EQ(static_cast<GLuint>(currentBufferBinding), mTransformFeedbackBuffer);
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
......
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