Commit 9e8fea5b by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix desc set cache bug with xfb offset

Prior to this change, the transform feedback buffer offset was not stored in the descriptor set key, so if the offset changed, stale descriptor sets could be used. Bug: angleproject:5963 Change-Id: I3dec4ab9fa82092a65e9a75bdd19c5f2cf49521c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2894513Reviewed-by: 's avatarCharlie Lao <cclao@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent 3ff08c44
...@@ -80,7 +80,8 @@ void TransformFeedbackVk::initializeXFBBuffersDesc(ContextVk *contextVk, size_t ...@@ -80,7 +80,8 @@ void TransformFeedbackVk::initializeXFBBuffersDesc(ContextVk *contextVk, size_t
} }
mXFBBuffersDesc.updateTransformFeedbackBuffer( mXFBBuffersDesc.updateTransformFeedbackBuffer(
bufferIndex, mBufferHelpers[bufferIndex]->getBufferSerial()); bufferIndex, mBufferHelpers[bufferIndex]->getBufferSerial(),
mBufferOffsets[bufferIndex]);
} }
} }
...@@ -178,7 +179,8 @@ angle::Result TransformFeedbackVk::pause(const gl::Context *context) ...@@ -178,7 +179,8 @@ angle::Result TransformFeedbackVk::pause(const gl::Context *context)
for (size_t xfbIndex = 0; xfbIndex < xfbBufferCount; ++xfbIndex) for (size_t xfbIndex = 0; xfbIndex < xfbBufferCount; ++xfbIndex)
{ {
mXFBBuffersDesc.updateTransformFeedbackBuffer(xfbIndex, emptyBuffer.getBufferSerial()); mXFBBuffersDesc.updateTransformFeedbackBuffer(xfbIndex, emptyBuffer.getBufferSerial(),
0);
} }
} }
......
...@@ -3006,13 +3006,19 @@ UniformsAndXfbDescriptorDesc &UniformsAndXfbDescriptorDesc::operator=( ...@@ -3006,13 +3006,19 @@ UniformsAndXfbDescriptorDesc &UniformsAndXfbDescriptorDesc::operator=(
size_t UniformsAndXfbDescriptorDesc::hash() const size_t UniformsAndXfbDescriptorDesc::hash() const
{ {
return angle::ComputeGenericHash(&mBufferSerials, sizeof(BufferSerial) * mBufferCount); ASSERT(mBufferCount > 0);
return angle::ComputeGenericHash(&mBufferSerials, sizeof(mBufferSerials[0]) * mBufferCount) ^
angle::ComputeGenericHash(
&mXfbBufferOffsets,
sizeof(mXfbBufferOffsets[0]) * (mBufferCount - kDefaultUniformBufferCount));
} }
void UniformsAndXfbDescriptorDesc::reset() void UniformsAndXfbDescriptorDesc::reset()
{ {
mBufferCount = 0; mBufferCount = 0;
memset(&mBufferSerials, 0, sizeof(BufferSerial) * kMaxBufferCount); memset(&mBufferSerials, 0, sizeof(mBufferSerials));
memset(&mXfbBufferOffsets, 0, sizeof(mXfbBufferOffsets));
} }
bool UniformsAndXfbDescriptorDesc::operator==(const UniformsAndXfbDescriptorDesc &other) const bool UniformsAndXfbDescriptorDesc::operator==(const UniformsAndXfbDescriptorDesc &other) const
...@@ -3022,7 +3028,12 @@ bool UniformsAndXfbDescriptorDesc::operator==(const UniformsAndXfbDescriptorDesc ...@@ -3022,7 +3028,12 @@ bool UniformsAndXfbDescriptorDesc::operator==(const UniformsAndXfbDescriptorDesc
return false; return false;
} }
return memcmp(&mBufferSerials, &other.mBufferSerials, sizeof(BufferSerial) * mBufferCount) == 0; ASSERT(mBufferCount > 0);
return memcmp(&mBufferSerials, &other.mBufferSerials,
sizeof(mBufferSerials[0]) * mBufferCount) == 0 &&
memcmp(&mXfbBufferOffsets, &other.mXfbBufferOffsets,
sizeof(mXfbBufferOffsets[0]) * (mBufferCount - kDefaultUniformBufferCount)) == 0;
} }
// ShaderBuffersDescriptorDesc implementation. // ShaderBuffersDescriptorDesc implementation.
...@@ -3054,12 +3065,6 @@ bool ShaderBuffersDescriptorDesc::operator==(const ShaderBuffersDescriptorDesc & ...@@ -3054,12 +3065,6 @@ bool ShaderBuffersDescriptorDesc::operator==(const ShaderBuffersDescriptorDesc &
return mPayload == other.mPayload; return mPayload == other.mPayload;
} }
void ShaderBuffersDescriptorDesc::append64BitValue(uint64_t value)
{
mPayload.push_back(static_cast<uint32_t>(value & (angle::Bit<uint64_t>(32u) - 1u)));
mPayload.push_back(value >> 32);
}
// FramebufferDesc implementation. // FramebufferDesc implementation.
FramebufferDesc::FramebufferDesc() FramebufferDesc::FramebufferDesc()
......
...@@ -1109,11 +1109,18 @@ class UniformsAndXfbDescriptorDesc ...@@ -1109,11 +1109,18 @@ class UniformsAndXfbDescriptorDesc
mBufferSerials[kDefaultUniformBufferIndex] = bufferSerial; mBufferSerials[kDefaultUniformBufferIndex] = bufferSerial;
mBufferCount = std::max(mBufferCount, static_cast<uint32_t>(1)); mBufferCount = std::max(mBufferCount, static_cast<uint32_t>(1));
} }
void updateTransformFeedbackBuffer(size_t xfbIndex, BufferSerial bufferSerial) void updateTransformFeedbackBuffer(size_t xfbIndex,
BufferSerial bufferSerial,
VkDeviceSize bufferOffset)
{ {
uint32_t bufferIndex = static_cast<uint32_t>(xfbIndex) + 1; uint32_t bufferIndex = static_cast<uint32_t>(xfbIndex) + 1;
mBufferSerials[bufferIndex] = bufferSerial; mBufferSerials[bufferIndex] = bufferSerial;
mBufferCount = std::max(mBufferCount, (bufferIndex + 1));
ASSERT(static_cast<uint64_t>(bufferOffset) <=
static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()));
mXfbBufferOffsets[xfbIndex] = static_cast<uint32_t>(bufferOffset);
mBufferCount = std::max(mBufferCount, (bufferIndex + 1));
} }
size_t hash() const; size_t hash() const;
void reset(); void reset();
...@@ -1124,8 +1131,11 @@ class UniformsAndXfbDescriptorDesc ...@@ -1124,8 +1131,11 @@ class UniformsAndXfbDescriptorDesc
uint32_t mBufferCount; uint32_t mBufferCount;
// The array index 0 is used for default uniform buffer // The array index 0 is used for default uniform buffer
static constexpr size_t kDefaultUniformBufferIndex = 0; static constexpr size_t kDefaultUniformBufferIndex = 0;
static constexpr size_t kMaxBufferCount = 1 + gl::IMPLEMENTATION_MAX_TRANSFORM_FEEDBACK_BUFFERS; static constexpr size_t kDefaultUniformBufferCount = 1;
static constexpr size_t kMaxBufferCount =
kDefaultUniformBufferCount + gl::IMPLEMENTATION_MAX_TRANSFORM_FEEDBACK_BUFFERS;
std::array<BufferSerial, kMaxBufferCount> mBufferSerials; std::array<BufferSerial, kMaxBufferCount> mBufferSerials;
std::array<uint32_t, gl::IMPLEMENTATION_MAX_TRANSFORM_FEEDBACK_BUFFERS> mXfbBufferOffsets;
}; };
class ShaderBuffersDescriptorDesc class ShaderBuffersDescriptorDesc
...@@ -1148,8 +1158,6 @@ class ShaderBuffersDescriptorDesc ...@@ -1148,8 +1158,6 @@ class ShaderBuffersDescriptorDesc
} }
ANGLE_INLINE void append32BitValue(uint32_t value) { mPayload.push_back(value); } ANGLE_INLINE void append32BitValue(uint32_t value) { mPayload.push_back(value); }
void append64BitValue(uint64_t value);
private: private:
// After a preliminary minimum size, use heap memory. // After a preliminary minimum size, use heap memory.
static constexpr size_t kFastBufferWordLimit = 32; static constexpr size_t kFastBufferWordLimit = 32;
......
...@@ -2814,6 +2814,84 @@ TEST_P(TransformFeedbackWithDepthBufferTest, RecordAndDrawWithDepthWriteEnabled) ...@@ -2814,6 +2814,84 @@ TEST_P(TransformFeedbackWithDepthBufferTest, RecordAndDrawWithDepthWriteEnabled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
} }
// Test that changing the transform feedback binding offset works.
TEST_P(TransformFeedbackTest, RecordTwiceWithBindingOffsetChange)
{
constexpr char kVS[] = R"(
varying vec4 v;
void main()
{
v = vec4(0.25, 0.5, 0.75, 1.0);
})";
constexpr char kFS[] = R"(
precision mediump float;
void main()
{
gl_FragColor = vec4(0);
})";
// Capture the varying "v"
const std::vector<std::string> tfVaryings = {"v"};
ANGLE_GL_PROGRAM_TRANSFORM_FEEDBACK(program, kVS, kFS, tfVaryings, GL_INTERLEAVED_ATTRIBS);
EXPECT_GL_NO_ERROR();
glUseProgram(program);
constexpr std::array<GLenum, 3> kUsages = {GL_STATIC_DRAW, GL_STREAM_DRAW, GL_DYNAMIC_DRAW};
constexpr uint32_t kVaryingSize = 4;
constexpr uint32_t kFirstOffset = 8;
constexpr uint32_t kSecondOffset = 24;
constexpr uint32_t kBufferSize = 40;
const std::vector<float> initialData(kBufferSize, 0);
std::vector<float> expectedData = initialData;
expectedData[kFirstOffset + 0] = expectedData[kSecondOffset + 0] = 0.25f;
expectedData[kFirstOffset + 1] = expectedData[kSecondOffset + 1] = 0.5f;
expectedData[kFirstOffset + 2] = expectedData[kSecondOffset + 2] = 0.75f;
expectedData[kFirstOffset + 3] = expectedData[kSecondOffset + 3] = 1.0f;
for (GLenum usage : kUsages)
{
GLTransformFeedback xfb;
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, xfb);
GLBuffer xfbBuffer;
glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, xfbBuffer);
glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kBufferSize * sizeof(float), initialData.data(),
GL_DYNAMIC_DRAW);
// Record into first offset
glBindBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer, kFirstOffset * sizeof(float),
kVaryingSize * sizeof(float));
glBeginTransformFeedback(GL_POINTS);
glDrawArrays(GL_POINTS, 0, 1);
glEndTransformFeedback();
// Record into second offset
glBindBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer, kSecondOffset * sizeof(float),
kVaryingSize * sizeof(float));
glBeginTransformFeedback(GL_POINTS);
glDrawArrays(GL_POINTS, 0, 1);
glEndTransformFeedback();
const float *bufferData = static_cast<float *>(glMapBufferRange(
GL_TRANSFORM_FEEDBACK_BUFFER, 0, kBufferSize * sizeof(float), GL_MAP_READ_BIT));
EXPECT_GL_NO_ERROR();
for (uint32_t index = 0; index < kBufferSize; ++index)
{
EXPECT_NEAR(bufferData[index], expectedData[index], 1e-6)
<< "index: " << index << " usage: " << usage;
}
glUnmapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER);
}
}
class TransformFeedbackTestES32 : public TransformFeedbackTest class TransformFeedbackTestES32 : public TransformFeedbackTest
{}; {};
......
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