Commit 6b275406 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Workaround vertex attributes vs stride issue on AMD

Under robustBufferAccess, Vulkan states that: Vertex input attributes are considered out of bounds if the offset of the attribute in the bound vertex buffer range plus the size of the attribute is greater than either: - vertexBufferRangeSize, if bindingStride == 0; or - (vertexBufferRangeSize - (vertexBufferRangeSize % bindingStride)) The latter implies that if the buffer size is not a multiple of the vertex attribute stride, what lies beyond the last multiple of stride is considered out of bounds. It also says: Out-of-bounds buffer loads will return any of the following values: - Values from anywhere within the memory range(s) bound to the buffer (possibly including bytes of memory past the end of the buffer, up to the end of the bound range). - Zero values, or (0,0,0,x) vectors for vector reads where x is a valid value represented in the type of the vector components and may be any of ... The first bullet point indicates that the driver is allowed to load the attribute values from the buffer if its range still lies within the buffer size. Take the following example: - Buffer size = 12 - Attribute stride = 8 - Attribute offset = 0 - Attribute size = 4 Basically the buffer is thus laid out as follows: attr stride _________/\_________ / \ +----------+----------+----------+ | vertex 0 | padding | vertex 1 | +----------+----------+----------+ \___ ____/ V attr size In the above example, the attribute for vertex 1 is considered out of bounds, but the driver is allowed to either read it correctly, or return (0, 0, 0, 1) for it. Most drivers implement the former, while AMD implements the latter. This change introduces a workaround for AMD where GL_MAX_VERTEX_ATTRIB_STRIDE is limited to 2048 (the common value for it according to gpuinfo.org) and conservatively rounds up every buffer allocation to that size. While technically, this workaround should be applied on any device with the robustBufferAccess feature enabled, it is currently limited to AMD to avoid the inefficiency. A possible future revision of Vulkan may relax the above restrictions. Bug: angleproject:2848 Change-Id: Ida5ae5d777da10f22ce8be5a09a7644b5bbd778e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1991709Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent 8fde1151
......@@ -213,6 +213,25 @@ struct FeaturesVk : FeatureSetBase
"RewriteStructSamplers behavior, which produces fewer.",
&members, "http://anglebug.com/2703"};
// If the robustBufferAccess feature is enabled, Vulkan considers vertex attribute accesses only
// valid up to the last multiple of stride. If a vertex's attribute range is such that it falls
// within the range of the buffer, but beyond the last multiple of stride, the driver is allowed
// to either read that range from the buffer anyway, or to return (0, 0, 0, 1). Most drivers
// implement the former, while amdvlk on Linux and AMD's windows driver implement the latter.
// For the latter, this workaround limits GL_MAX_VERTEX_ATTRIB_STRIDE to a reasonable value, and
// rounds up every buffer allocation size to be a multiple of that.
// http://anglebug.com/2514
Feature roundUpBuffersToMaxVertexAttribStride = {
"round_up_buffers_to_max_vertex_attrib_stride", FeatureCategory::VulkanWorkarounds,
"If the robustBufferAccess feature is enabled, Vulkan considers vertex attribute accesses "
"only valid up to the last multiple of stride. If a vertex's attribute range is such that "
"it falls within the range of the buffer, but beyond the last multiple of stride, the "
"driver is allowed to either read that range from the buffer anyway, or to return "
"(0, 0, 0, 1). Most drivers implement the former, while some drivers the latter. For the "
"latter, this workaround limits GL_MAX_VERTEX_ATTRIB_STRIDE to a reasonable value, and "
"rounds up every buffer allocation size to be a multiple of that.",
&members, "http://anglebug.com/2848"};
// Whether the VkDevice supports the VK_EXT_swapchain_colorspace extension
// http://anglebug.com/2514
Feature supportsSwapchainColorspace = {
......
......@@ -77,6 +77,10 @@ enum
namespace limits
{
// Almost all drivers use 2048 (the minimum value) as GL_MAX_VERTEX_ATTRIB_STRIDE. ANGLE advertizes
// the same limit.
constexpr uint32_t kMaxVertexAttribStride = 2048;
// Some of the minimums required by GL, used to detect if the backend meets the minimum requirement.
// Currently, there's no need to separate these values per spec version.
constexpr uint32_t kMinimumComputeStorageBuffers = 4;
......
......@@ -3559,6 +3559,8 @@ void Context::initCaps()
// Apply/Verify implementation limits
ANGLE_LIMIT_CAP(mState.mCaps.maxVertexAttributes, MAX_VERTEX_ATTRIBS);
ANGLE_LIMIT_CAP(mState.mCaps.maxVertexAttribStride,
static_cast<GLint>(limits::kMaxVertexAttribStride));
ASSERT(mState.mCaps.minAliasedPointSize >= 1.0f);
......
......@@ -549,6 +549,7 @@ RendererVk::RendererVk()
mQueue(VK_NULL_HANDLE),
mCurrentQueueFamilyIndex(std::numeric_limits<uint32_t>::max()),
mMaxVertexAttribDivisor(1),
mMaxVertexAttribStride(0),
mDevice(VK_NULL_HANDLE),
mLastCompletedQueueSerial(mQueueSerialFactory.generate()),
mCurrentQueueSerial(mQueueSerialFactory.generate()),
......@@ -1460,6 +1461,14 @@ void RendererVk::initFeatures(const ExtensionNameList &deviceExtensionNames)
// Disabled on AMD/windows due to buggy behavior.
ANGLE_FEATURE_CONDITION((&mFeatures), disallowSeamfulCubeMapEmulation, IsWindows() && isAMD);
// Round up buffer sizes in the presence of robustBufferAccess to emulate GLES behavior when
// vertex attributes are fetched from beyond the last multiple of the stride. Currently, only
// AMD is known to refuse reading these attributes.
ANGLE_FEATURE_CONDITION((&mFeatures), roundUpBuffersToMaxVertexAttribStride,
isAMD && mPhysicalDeviceFeatures.robustBufferAccess);
mMaxVertexAttribStride = std::min(static_cast<uint32_t>(gl::limits::kMaxVertexAttribStride),
mPhysicalDeviceProperties.limits.maxVertexInputBindingStride);
ANGLE_FEATURE_CONDITION((&mFeatures), forceD16TexFilter, IsAndroid() && isQualcomm);
ANGLE_FEATURE_CONDITION((&mFeatures), disableFlippingBlitWithCommand,
......
......@@ -146,6 +146,7 @@ class RendererVk : angle::NonCopyable
return mFeatures;
}
uint32_t getMaxVertexAttribDivisor() const { return mMaxVertexAttribDivisor; }
VkDeviceSize getMaxVertexAttribStride() const { return mMaxVertexAttribStride; }
bool isMockICDEnabled() const { return mEnabledICD == vk::ICD::Mock; }
......@@ -260,6 +261,7 @@ class RendererVk : angle::NonCopyable
VkQueue mQueue;
uint32_t mCurrentQueueFamilyIndex;
uint32_t mMaxVertexAttribDivisor;
VkDeviceSize mMaxVertexAttribStride;
VkDevice mDevice;
AtomicSerialFactory mQueueSerialFactory;
AtomicSerialFactory mShaderSerialFactory;
......
......@@ -1434,9 +1434,11 @@ BufferHelper::BufferHelper()
BufferHelper::~BufferHelper() = default;
angle::Result BufferHelper::init(ContextVk *contextVk,
const VkBufferCreateInfo &createInfo,
const VkBufferCreateInfo &requestedCreateInfo,
VkMemoryPropertyFlags memoryPropertyFlags)
{
RendererVk *rendererVk = contextVk->getRenderer();
// TODO: Remove with anglebug.com/2162: Vulkan: Implement device memory sub-allocation
// Check if we have too many resources allocated already and need to free some before allocating
// more and (possibly) exceeding the device's limits.
......@@ -1445,8 +1447,21 @@ angle::Result BufferHelper::init(ContextVk *contextVk,
ANGLE_TRY(contextVk->flushImpl(nullptr));
}
mSize = createInfo.size;
ANGLE_VK_TRY(contextVk, mBuffer.init(contextVk->getDevice(), createInfo));
mSize = requestedCreateInfo.size;
VkBufferCreateInfo modifiedCreateInfo;
const VkBufferCreateInfo *createInfo = &requestedCreateInfo;
if (rendererVk->getFeatures().roundUpBuffersToMaxVertexAttribStride.enabled)
{
const VkDeviceSize maxVertexAttribStride = rendererVk->getMaxVertexAttribStride();
ASSERT(maxVertexAttribStride);
modifiedCreateInfo = requestedCreateInfo;
modifiedCreateInfo.size = roundUp(modifiedCreateInfo.size, maxVertexAttribStride);
createInfo = &modifiedCreateInfo;
}
ANGLE_VK_TRY(contextVk, mBuffer.init(contextVk->getDevice(), *createInfo));
ANGLE_TRY(AllocateBufferMemory(contextVk, memoryPropertyFlags, &mMemoryPropertyFlags, nullptr,
&mBuffer, &mDeviceMemory));
mCurrentQueueFamilyIndex = contextVk->getRenderer()->getQueueFamilyIndex();
......
......@@ -222,8 +222,6 @@ TEST_P(IndexedPointsTestUByte, VertexWithColorUnsignedByteOffset0)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(0, true);
}
......@@ -232,8 +230,6 @@ TEST_P(IndexedPointsTestUByte, VertexWithColorUnsignedByteOffset1)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(1, true);
}
......@@ -242,8 +238,6 @@ TEST_P(IndexedPointsTestUByte, VertexWithColorUnsignedByteOffset2)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(2, true);
}
......@@ -252,8 +246,6 @@ TEST_P(IndexedPointsTestUByte, VertexWithColorUnsignedByteOffset3)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(3, true);
}
......@@ -292,8 +284,6 @@ TEST_P(IndexedPointsTestUShort, VertexWithColorUnsignedShortOffset0)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(0, true);
}
......@@ -302,8 +292,6 @@ TEST_P(IndexedPointsTestUShort, VertexWithColorUnsignedShortOffset1)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(1, true);
}
......@@ -312,8 +300,6 @@ TEST_P(IndexedPointsTestUShort, VertexWithColorUnsignedShortOffset2)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(2, true);
}
......@@ -322,8 +308,6 @@ TEST_P(IndexedPointsTestUShort, VertexWithColorUnsignedShortOffset3)
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(3, true);
}
......@@ -332,8 +316,6 @@ TEST_P(IndexedPointsTestUShort, VertexWithColorUnsignedShortOffsetChangingIndice
{
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
// TODO(fjhenigman): Figure out why this fails on Ozone Intel.
ANGLE_SKIP_TEST_IF(IsOzone() && IsIntel() && IsOpenGLES());
......@@ -398,8 +380,6 @@ TEST_P(IndexedPointsTestUInt, VertexWithColorUnsignedIntOffset0)
}
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(0, true);
}
......@@ -411,8 +391,6 @@ TEST_P(IndexedPointsTestUInt, VertexWithColorUnsignedIntOffset1)
}
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(1, true);
}
......@@ -424,8 +402,6 @@ TEST_P(IndexedPointsTestUInt, VertexWithColorUnsignedIntOffset2)
}
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(2, true);
}
......@@ -437,8 +413,6 @@ TEST_P(IndexedPointsTestUInt, VertexWithColorUnsignedIntOffset3)
}
// http://anglebug.com/4092
ANGLE_SKIP_TEST_IF(isSwiftshader());
// Fix with buffer offset http://anglebug.com/2848
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD());
runTest(3, true);
}
......@@ -448,4 +422,4 @@ TEST_P(IndexedPointsTestUInt, VertexWithColorUnsignedIntOffset3)
// TODO(geofflang): Figure out why this test fails on Intel OpenGL
ANGLE_INSTANTIATE_TEST_ES2(IndexedPointsTestUByte);
ANGLE_INSTANTIATE_TEST_ES2(IndexedPointsTestUShort);
ANGLE_INSTANTIATE_TEST_ES2(IndexedPointsTestUInt);
\ No newline at end of file
ANGLE_INSTANTIATE_TEST_ES2(IndexedPointsTestUInt);
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