From 020abb8b82e4c02534b384151126e814280535ff Mon Sep 17 00:00:00 2001 From: James Dong Date: Wed, 24 Jul 2019 11:33:49 -0600 Subject: [PATCH] Vulkan: invalidate translation buffers for SSBOs Translation buffers weren't being marked dirty after running a compute shader in which they are bound as SSBOs. This change invalidates all SSBOs after a draw or compute call. Bug: angleproject:3739 Change-Id: I66b56df7e619b55afc7e3da6b5613b6d050e06bb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1717144 Reviewed-by: Shahbaz Youssefi Reviewed-by: Jamie Madill Commit-Queue: James Dong --- src/libANGLE/Buffer.cpp | 10 ++---- src/libANGLE/Buffer.h | 3 +- src/libANGLE/Context.cpp | 31 +++++++++++++++++++ src/libANGLE/Context.h | 9 ++++++ src/libANGLE/Context.inl.h | 12 +++++++ src/libANGLE/Framebuffer.cpp | 2 +- src/libANGLE/TransformFeedback.cpp | 2 +- src/libANGLE/angletypes.h | 3 ++ src/libANGLE/renderer/BufferImpl.h | 2 ++ src/libANGLE/renderer/vulkan/BufferVk.cpp | 5 +++ src/libANGLE/renderer/vulkan/BufferVk.h | 2 ++ src/libANGLE/renderer/vulkan/RendererVk.cpp | 7 +++++ .../renderer/vulkan/vk_caps_utils.cpp | 9 ++++-- src/tests/gl_tests/VertexAttributeTest.cpp | 12 +++++-- 14 files changed, 93 insertions(+), 16 deletions(-) diff --git a/src/libANGLE/Buffer.cpp b/src/libANGLE/Buffer.cpp index 9ffcddba2..7e036b975 100644 --- a/src/libANGLE/Buffer.cpp +++ b/src/libANGLE/Buffer.cpp @@ -210,20 +210,14 @@ angle::Result Buffer::unmap(const Context *context, GLboolean *result) return angle::Result::Continue; } -void Buffer::onTransformFeedback() +void Buffer::onDataChanged() { mIndexRangeCache.clear(); // Notify when data changes. onStateChange(angle::SubjectMessage::ContentsChanged); -} -void Buffer::onPixelPack() -{ - mIndexRangeCache.clear(); - - // Notify when data changes. - onStateChange(angle::SubjectMessage::ContentsChanged); + mImpl->onDataChanged(); } angle::Result Buffer::getIndexRange(const gl::Context *context, diff --git a/src/libANGLE/Buffer.h b/src/libANGLE/Buffer.h index ef3eefe4f..e43ddf691 100644 --- a/src/libANGLE/Buffer.h +++ b/src/libANGLE/Buffer.h @@ -100,8 +100,7 @@ class Buffer final : public RefCountObject, angle::Result unmap(const Context *context, GLboolean *result); // These are called when another operation changes Buffer data. - void onTransformFeedback(); - void onPixelPack(); + void onDataChanged(); angle::Result getIndexRange(const gl::Context *context, DrawElementsType type, diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp index d6edd72c1..d83f863f1 100644 --- a/src/libANGLE/Context.cpp +++ b/src/libANGLE/Context.cpp @@ -2244,6 +2244,7 @@ void Context::drawArraysInstanced(PrimitiveMode mode, ANGLE_CONTEXT_TRY( mImplementation->drawArraysInstanced(this, mode, first, count, instanceCount)); MarkTransformFeedbackBufferUsage(this, count, instanceCount); + MarkShaderStorageBufferUsage(this); } void Context::drawElementsInstanced(PrimitiveMode mode, @@ -2261,6 +2262,7 @@ void Context::drawElementsInstanced(PrimitiveMode mode, ANGLE_CONTEXT_TRY(prepareForDraw(mode)); ANGLE_CONTEXT_TRY( mImplementation->drawElementsInstanced(this, mode, count, type, indices, instances)); + MarkShaderStorageBufferUsage(this); } void Context::drawRangeElements(PrimitiveMode mode, @@ -2279,18 +2281,21 @@ void Context::drawRangeElements(PrimitiveMode mode, ANGLE_CONTEXT_TRY(prepareForDraw(mode)); ANGLE_CONTEXT_TRY( mImplementation->drawRangeElements(this, mode, start, end, count, type, indices)); + MarkShaderStorageBufferUsage(this); } void Context::drawArraysIndirect(PrimitiveMode mode, const void *indirect) { ANGLE_CONTEXT_TRY(prepareForDraw(mode)); ANGLE_CONTEXT_TRY(mImplementation->drawArraysIndirect(this, mode, indirect)); + MarkShaderStorageBufferUsage(this); } void Context::drawElementsIndirect(PrimitiveMode mode, DrawElementsType type, const void *indirect) { ANGLE_CONTEXT_TRY(prepareForDraw(mode)); ANGLE_CONTEXT_TRY(mImplementation->drawElementsIndirect(this, mode, type, indirect)); + MarkShaderStorageBufferUsage(this); } void Context::flush() @@ -5395,12 +5400,16 @@ void Context::dispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGr ANGLE_CONTEXT_TRY(prepareForDispatch()); ANGLE_CONTEXT_TRY(mImplementation->dispatchCompute(this, numGroupsX, numGroupsY, numGroupsZ)); + + MarkShaderStorageBufferUsage(this); } void Context::dispatchComputeIndirect(GLintptr indirect) { ANGLE_CONTEXT_TRY(prepareForDispatch()); ANGLE_CONTEXT_TRY(mImplementation->dispatchComputeIndirect(this, indirect)); + + MarkShaderStorageBufferUsage(this); } void Context::texStorage2D(TextureType target, @@ -5456,6 +5465,7 @@ void Context::multiDrawArrays(PrimitiveMode mode, ANGLE_CONTEXT_TRY( mImplementation->drawArrays(this, mode, firsts[drawID], counts[drawID])); MarkTransformFeedbackBufferUsage(this, counts[drawID], 1); + MarkShaderStorageBufferUsage(this); } } else @@ -5469,6 +5479,7 @@ void Context::multiDrawArrays(PrimitiveMode mode, ANGLE_CONTEXT_TRY( mImplementation->drawArrays(this, mode, firsts[drawID], counts[drawID])); MarkTransformFeedbackBufferUsage(this, counts[drawID], 1); + MarkShaderStorageBufferUsage(this); } } } @@ -5494,6 +5505,7 @@ void Context::multiDrawArraysInstanced(PrimitiveMode mode, ANGLE_CONTEXT_TRY(mImplementation->drawArraysInstanced( this, mode, firsts[drawID], counts[drawID], instanceCounts[drawID])); MarkTransformFeedbackBufferUsage(this, counts[drawID], instanceCounts[drawID]); + MarkShaderStorageBufferUsage(this); } } else @@ -5507,6 +5519,7 @@ void Context::multiDrawArraysInstanced(PrimitiveMode mode, ANGLE_CONTEXT_TRY(mImplementation->drawArraysInstanced( this, mode, firsts[drawID], counts[drawID], instanceCounts[drawID])); MarkTransformFeedbackBufferUsage(this, counts[drawID], instanceCounts[drawID]); + MarkShaderStorageBufferUsage(this); } } } @@ -5531,6 +5544,7 @@ void Context::multiDrawElements(PrimitiveMode mode, programObject->setDrawIDUniform(drawID); ANGLE_CONTEXT_TRY( mImplementation->drawElements(this, mode, counts[drawID], type, indices[drawID])); + MarkShaderStorageBufferUsage(this); } } else @@ -5543,6 +5557,7 @@ void Context::multiDrawElements(PrimitiveMode mode, } ANGLE_CONTEXT_TRY( mImplementation->drawElements(this, mode, counts[drawID], type, indices[drawID])); + MarkShaderStorageBufferUsage(this); } } } @@ -5568,6 +5583,7 @@ void Context::multiDrawElementsInstanced(PrimitiveMode mode, programObject->setDrawIDUniform(drawID); ANGLE_CONTEXT_TRY(mImplementation->drawElementsInstanced( this, mode, counts[drawID], type, indices[drawID], instanceCounts[drawID])); + MarkShaderStorageBufferUsage(this); } } else @@ -5580,6 +5596,7 @@ void Context::multiDrawElementsInstanced(PrimitiveMode mode, } ANGLE_CONTEXT_TRY(mImplementation->drawElementsInstanced( this, mode, counts[drawID], type, indices[drawID], instanceCounts[drawID])); + MarkShaderStorageBufferUsage(this); } } } @@ -8705,6 +8722,7 @@ void StateCache::onProgramExecutableChange(Context *context) updateVertexElementLimits(context); updateBasicDrawStatesError(); updateValidDrawModes(context); + updateActiveShaderStorageBufferIndices(context); } void StateCache::onVertexArrayFormatChange(Context *context) @@ -8934,4 +8952,17 @@ void StateCache::updateVertexAttribTypesValidation(Context *context) }}; } } + +void StateCache::updateActiveShaderStorageBufferIndices(Context *context) +{ + mCachedActiveShaderStorageBufferIndices.reset(); + Program *program = context->getState().getProgram(); + if (program) + { + for (const InterfaceBlock &block : program->getState().getShaderStorageBlocks()) + { + mCachedActiveShaderStorageBufferIndices.set(block.binding); + } + } +} } // namespace gl diff --git a/src/libANGLE/Context.h b/src/libANGLE/Context.h index 4ae7c7e0d..1a972bcb4 100644 --- a/src/libANGLE/Context.h +++ b/src/libANGLE/Context.h @@ -247,6 +247,13 @@ class StateCache final : angle::NonCopyable return mCachedIntegerVertexAttribTypesValidation[type]; } + // Places that can trigger updateActiveShaderStorageBufferIndices: + // 1. onProgramExecutableChange. + StorageBuffersMask getActiveShaderStorageBufferIndices() const + { + return mCachedActiveShaderStorageBufferIndices; + } + // State change notifications. void onVertexArrayBindingChange(Context *context); void onProgramExecutableChange(Context *context); @@ -278,6 +285,7 @@ class StateCache final : angle::NonCopyable void updateBasicDrawElementsError(); void updateTransformFeedbackActiveUnpaused(Context *context); void updateVertexAttribTypesValidation(Context *context); + void updateActiveShaderStorageBufferIndices(Context *context); void setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, bool lineAdjOK, bool triAdjOK); @@ -295,6 +303,7 @@ class StateCache final : angle::NonCopyable mutable intptr_t mCachedBasicDrawStatesError; mutable intptr_t mCachedBasicDrawElementsError; bool mCachedTransformFeedbackActiveUnpaused; + StorageBuffersMask mCachedActiveShaderStorageBufferIndices; // Reserve an extra slot at the end of these maps for invalid enum. angle::PackedEnumMap() + 1> diff --git a/src/libANGLE/Context.inl.h b/src/libANGLE/Context.inl.h index 4738160c3..af72e72f3 100644 --- a/src/libANGLE/Context.inl.h +++ b/src/libANGLE/Context.inl.h @@ -46,6 +46,18 @@ ANGLE_INLINE void MarkTransformFeedbackBufferUsage(const Context *context, } } +ANGLE_INLINE void MarkShaderStorageBufferUsage(const Context *context) +{ + for (size_t index : context->getStateCache().getActiveShaderStorageBufferIndices()) + { + gl::Buffer *buffer = context->getState().getIndexedShaderStorageBuffer(index).get(); + if (buffer) + { + buffer->onDataChanged(); + } + } +} + // Return true if the draw is a no-op, else return false. // A no-op draw occurs if the count of vertices is less than the minimum required to // have a valid primitive for this mode (0 for points, 0-1 for lines, 0-2 for tris). diff --git a/src/libANGLE/Framebuffer.cpp b/src/libANGLE/Framebuffer.cpp index 64acc8a4c..0346894f8 100644 --- a/src/libANGLE/Framebuffer.cpp +++ b/src/libANGLE/Framebuffer.cpp @@ -1512,7 +1512,7 @@ angle::Result Framebuffer::readPixels(const Context *context, Buffer *unpackBuffer = context->getState().getTargetBuffer(BufferBinding::PixelUnpack); if (unpackBuffer) { - unpackBuffer->onPixelPack(); + unpackBuffer->onDataChanged(); } return angle::Result::Continue; diff --git a/src/libANGLE/TransformFeedback.cpp b/src/libANGLE/TransformFeedback.cpp index 3222f14b8..129f9c090 100644 --- a/src/libANGLE/TransformFeedback.cpp +++ b/src/libANGLE/TransformFeedback.cpp @@ -214,7 +214,7 @@ void TransformFeedback::onVerticesDrawn(const Context *context, GLsizei count, G { if (buffer.get() != nullptr) { - buffer->onTransformFeedback(); + buffer->onDataChanged(); } } } diff --git a/src/libANGLE/angletypes.h b/src/libANGLE/angletypes.h index d859073fb..8c36ab145 100644 --- a/src/libANGLE/angletypes.h +++ b/src/libANGLE/angletypes.h @@ -387,6 +387,9 @@ using UniformBlockBindingMask = angle::BitSet; +// Used in StateCache +using StorageBuffersMask = angle::BitSet; + template using TexLevelArray = std::array; diff --git a/src/libANGLE/renderer/BufferImpl.h b/src/libANGLE/renderer/BufferImpl.h index 77b773741..ae52f7fd3 100644 --- a/src/libANGLE/renderer/BufferImpl.h +++ b/src/libANGLE/renderer/BufferImpl.h @@ -68,6 +68,8 @@ class BufferImpl : public angle::Subject // Override if accurate native memory size information is available virtual GLint64 getMemorySize() const; + virtual void onDataChanged() {} + protected: const gl::BufferState &mState; }; diff --git a/src/libANGLE/renderer/vulkan/BufferVk.cpp b/src/libANGLE/renderer/vulkan/BufferVk.cpp index cc555a3c6..6f84f893b 100644 --- a/src/libANGLE/renderer/vulkan/BufferVk.cpp +++ b/src/libANGLE/renderer/vulkan/BufferVk.cpp @@ -377,4 +377,9 @@ void BufferVk::markConversionBuffersDirty() } } +void BufferVk::onDataChanged() +{ + markConversionBuffersDirty(); +} + } // namespace rx diff --git a/src/libANGLE/renderer/vulkan/BufferVk.h b/src/libANGLE/renderer/vulkan/BufferVk.h index cd4a59305..fecf5eb28 100644 --- a/src/libANGLE/renderer/vulkan/BufferVk.h +++ b/src/libANGLE/renderer/vulkan/BufferVk.h @@ -79,6 +79,8 @@ class BufferVk : public BufferImpl GLint64 getSize() const { return mState.getSize(); } + void onDataChanged() override; + const vk::BufferHelper &getBuffer() const { ASSERT(mBuffer.valid()); diff --git a/src/libANGLE/renderer/vulkan/RendererVk.cpp b/src/libANGLE/renderer/vulkan/RendererVk.cpp index c1aeca4b2..11beebe81 100644 --- a/src/libANGLE/renderer/vulkan/RendererVk.cpp +++ b/src/libANGLE/renderer/vulkan/RendererVk.cpp @@ -1123,6 +1123,13 @@ gl::Version RendererVk::getMaxSupportedESVersion() const maxVersion = std::min(maxVersion, gl::Version(3, 0)); } + // ES3.1 requires at least a maximum offset of at least 2047. + // If the Vulkan implementation can't support that, we cannot support 3.1. + if (mPhysicalDeviceProperties.limits.maxVertexInputAttributeOffset < 2047) + { + maxVersion = std::min(maxVersion, gl::Version(3, 0)); + } + // Limit to ES2.0 if there are any blockers for 3.0. // If the command buffer doesn't support queries, we can't support ES3. diff --git a/src/libANGLE/renderer/vulkan/vk_caps_utils.cpp b/src/libANGLE/renderer/vulkan/vk_caps_utils.cpp index 5dac9f0fb..4b79129c9 100644 --- a/src/libANGLE/renderer/vulkan/vk_caps_utils.cpp +++ b/src/libANGLE/renderer/vulkan/vk_caps_utils.cpp @@ -142,8 +142,13 @@ void RendererVk::ensureCapsInitialized() const mNativeCaps.maxVertexAttributes = limitsVk.maxVertexInputAttributes; mNativeCaps.maxVertexAttribBindings = limitsVk.maxVertexInputBindings; - mNativeCaps.maxVertexAttribRelativeOffset = limitsVk.maxVertexInputAttributeOffset; - mNativeCaps.maxVertexAttribStride = limitsVk.maxVertexInputBindingStride; + // Offset and stride are stored as uint16_t in PackedAttribDesc. + mNativeCaps.maxVertexAttribRelativeOffset = + std::min(static_cast(std::numeric_limits::max()), + limitsVk.maxVertexInputAttributeOffset); + mNativeCaps.maxVertexAttribStride = + std::min(static_cast(std::numeric_limits::max()), + limitsVk.maxVertexInputBindingStride); mNativeCaps.maxElementsIndices = std::numeric_limits::max(); mNativeCaps.maxElementsVertices = std::numeric_limits::max(); diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp index 8bc914b97..e55cfeb16 100644 --- a/src/tests/gl_tests/VertexAttributeTest.cpp +++ b/src/tests/gl_tests/VertexAttributeTest.cpp @@ -1358,7 +1358,7 @@ void main() glUseProgram(computeProgram); glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, testBuffer); glDispatchCompute(1, 1, 1); - glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); + glMemoryBarrier(GL_VERTEX_ATTRIB_ARRAY_BARRIER_BIT); // Draw again to verify that testBuffer has been changed. glUseProgram(mProgram); @@ -1449,6 +1449,8 @@ TEST_P(VertexAttributeTestES31, ChangeAttribFormatAfterVertexAttribPointer) // Verify that only updating a binding without updating the bound format won't mess up this draw. TEST_P(VertexAttributeTestES31, OnlyUpdateBindingByBindVertexBuffer) { + ANGLE_SKIP_TEST_IF(IsVulkan()); // anglebug.com/3598 - vertex attrib binding + // Default binding index for test constexpr GLint kTestBinding = 10; initOnlyUpdateBindingTest(kTestBinding); @@ -1486,6 +1488,8 @@ TEST_P(VertexAttributeTestES31, OnlyUpdateBindingByBindVertexBuffer) // Verify that only updating a binding without updating the bound format won't mess up this draw. TEST_P(VertexAttributeTestES31, OnlyUpdateBindingByVertexAttribPointer) { + ANGLE_SKIP_TEST_IF(IsVulkan()); // anglebug.com/3598 - vertex attrib binding + // Default binding index for test constexpr GLint kTestBinding = 10; initOnlyUpdateBindingTest(kTestBinding); @@ -2053,7 +2057,11 @@ ANGLE_INSTANTIATE_TEST(VertexAttributeTestES3, ES3_OPENGLES(), ES3_VULKAN()); -ANGLE_INSTANTIATE_TEST(VertexAttributeTestES31, ES31_D3D11(), ES31_OPENGL(), ES31_OPENGLES()); +ANGLE_INSTANTIATE_TEST(VertexAttributeTestES31, + ES31_D3D11(), + ES31_OPENGL(), + ES31_OPENGLES(), + ES31_VULKAN()); ANGLE_INSTANTIATE_TEST(VertexAttributeCachingTest, ES2_D3D9(),