Vulkan: fix glGetQueryObject not flushing
glGetQueryObject* requires forward progress in the queue regardless of
whether we are waiting on the result or busy-looping over whether the
results are available. This commit calls flush() if the query has
pending work.
Additionally, this fixes a race condition where glGetQueryObject* may be
accessing a query whose corresponding batch has been submitted but not
yet executed. In such a case, the GPU may not have already reset the
query, so we have to wait on the fence of that batch to make sure the
query results are reliably available.
Bug: angleproject:2855
Change-Id: I977909c6526c0778a13722a8b8b73e54ad0202f6
Reviewed-on: https://chromium-review.googlesource.com/c/1279125
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/CommandGraph.h b/src/libANGLE/renderer/vulkan/CommandGraph.h
index 49769f6..f598fde 100644
--- a/src/libANGLE/renderer/vulkan/CommandGraph.h
+++ b/src/libANGLE/renderer/vulkan/CommandGraph.h
@@ -193,12 +193,13 @@
// Called when 'this' object changes, but we'd like to start a new command buffer later.
void finishCurrentCommands(RendererVk *renderer);
+ // Get the current queue serial for this resource. Used to release resources, and for
+ // queries, to know if the queue they are submitted on has finished execution.
+ Serial getStoredQueueSerial() const;
+
protected:
explicit CommandGraphResource(CommandGraphResourceType resourceType);
- // Get the current queue serial for this resource. Only used to release resources.
- Serial getStoredQueueSerial() const;
-
private:
void startNewCommands(RendererVk *renderer, CommandGraphNodeFunction function);
diff --git a/src/libANGLE/renderer/vulkan/QueryVk.cpp b/src/libANGLE/renderer/vulkan/QueryVk.cpp
index a8b3e9a..d6e11df 100644
--- a/src/libANGLE/renderer/vulkan/QueryVk.cpp
+++ b/src/libANGLE/renderer/vulkan/QueryVk.cpp
@@ -10,6 +10,7 @@
#include "libANGLE/renderer/vulkan/QueryVk.h"
#include "libANGLE/Context.h"
#include "libANGLE/renderer/vulkan/ContextVk.h"
+#include "libANGLE/renderer/vulkan/RendererVk.h"
#include "common/debug.h"
@@ -66,6 +67,28 @@
}
ContextVk *contextVk = vk::GetImpl(context);
+ RendererVk *renderer = contextVk->getRenderer();
+
+ // glGetQueryObject* requires an implicit flush of the command buffers to guarantee execution in
+ // finite time.
+ if (mQueryHelper.hasPendingWork(renderer))
+ {
+ ANGLE_TRY_HANDLE(context, renderer->flush(contextVk));
+ ASSERT(!mQueryHelper.hasPendingWork(renderer));
+ }
+
+ // If the command buffer this query is being written to is still in flight, its reset command
+ // may not have been performed by the GPU yet. To avoid a race condition in this case, wait
+ // for the batch to finish first before querying (or return not-ready if not waiting).
+ ANGLE_TRY(renderer->checkCompletedCommands(contextVk));
+ if (mQueryHelper.isResourceInUse(renderer))
+ {
+ if (!wait)
+ {
+ return angle::Result::Continue();
+ }
+ ANGLE_TRY(renderer->finishToSerial(contextVk, mQueryHelper.getStoredQueueSerial()));
+ }
VkQueryResultFlags flags = (wait ? VK_QUERY_RESULT_WAIT_BIT : 0) | VK_QUERY_RESULT_64_BIT;
@@ -74,23 +97,28 @@
sizeof(mCachedResult), flags);
ANGLE_TRY(result);
- if (result == angle::Result::Continue())
+ // If the results are not ready, do nothing. mCachedResultValid remains false.
+ if (result == angle::Result::Incomplete())
{
- mCachedResultValid = true;
-
- switch (getType())
- {
- case gl::QueryType::AnySamples:
- case gl::QueryType::AnySamplesConservative:
- // OpenGL query result in these cases is binary
- mCachedResult = !!mCachedResult;
- break;
- default:
- UNREACHABLE();
- break;
- }
+ // If VK_QUERY_RESULT_WAIT_BIT was given, Incomplete() cannot have been returned.
+ ASSERT(!wait);
+ return angle::Result::Continue();
}
+ // Fix up the results to what OpenGL expects.
+ switch (getType())
+ {
+ case gl::QueryType::AnySamples:
+ case gl::QueryType::AnySamplesConservative:
+ // OpenGL query result in these cases is binary
+ mCachedResult = !!mCachedResult;
+ break;
+ default:
+ UNREACHABLE();
+ break;
+ }
+
+ mCachedResultValid = true;
return angle::Result::Continue();
}
@@ -124,18 +152,6 @@
gl::Error QueryVk::isResultAvailable(const gl::Context *context, bool *available)
{
- ContextVk *contextVk = vk::GetImpl(context);
-
- // Make sure the command buffer for this query is submitted. If not, *available should always
- // be false. This is because the reset command is not yet executed (it's only put in the command
- // graph), so actually checking the results may return "true" because of a previous submission.
-
- if (mQueryHelper.hasPendingWork(contextVk->getRenderer()))
- {
- *available = false;
- return gl::NoError();
- }
-
ANGLE_TRY(getResult(context, false));
*available = mCachedResultValid;
diff --git a/src/libANGLE/renderer/vulkan/RendererVk.cpp b/src/libANGLE/renderer/vulkan/RendererVk.cpp
index bdd8ac7..e14b877 100644
--- a/src/libANGLE/renderer/vulkan/RendererVk.cpp
+++ b/src/libANGLE/renderer/vulkan/RendererVk.cpp
@@ -881,17 +881,17 @@
mGarbage.clear();
}
-angle::Result RendererVk::checkInFlightCommands(vk::Context *context)
+angle::Result RendererVk::checkCompletedCommands(vk::Context *context)
{
int finishedCount = 0;
for (CommandBatch &batch : mInFlightCommands)
{
- VkResult result = batch.fence.getStatus(mDevice);
- if (result == VK_NOT_READY)
+ angle::Result result = batch.fence.getStatus(context);
+ ANGLE_TRY(result);
+ if (result == angle::Result::Incomplete())
break;
- ANGLE_VK_TRY(context, result);
ASSERT(batch.serial > mLastCompletedQueueSerial);
mLastCompletedQueueSerial = batch.serial;
@@ -949,7 +949,7 @@
// TODO(jmadill): Overflow check.
mCurrentQueueSerial = mQueueSerialFactory.generate();
- ANGLE_TRY(checkInFlightCommands(context));
+ ANGLE_TRY(checkCompletedCommands(context));
// Simply null out the command buffer here - it was allocated using the command pool.
commandBuffer.releaseHandle();
@@ -971,6 +971,40 @@
return serial > mLastCompletedQueueSerial;
}
+angle::Result RendererVk::finishToSerial(vk::Context *context, Serial serial)
+{
+ if (!isSerialInUse(serial) || mInFlightCommands.empty())
+ {
+ return angle::Result::Continue();
+ }
+
+ // Find the first batch with serial equal to or bigger than given serial (note that
+ // the batch serials are unique, otherwise upper-bound would have been necessary).
+ size_t batchIndex = mInFlightCommands.size() - 1;
+ for (size_t i = 0; i < mInFlightCommands.size(); ++i)
+ {
+ if (mInFlightCommands[i].serial >= serial)
+ {
+ batchIndex = i;
+ break;
+ }
+ }
+ const CommandBatch &batch = mInFlightCommands[batchIndex];
+
+ // Wait for it finish
+ constexpr uint64_t kMaxFenceWaitTimeNs = 10'000'000'000llu;
+ angle::Result result = batch.fence.wait(context, kMaxFenceWaitTimeNs);
+ if (result == angle::Result::Incomplete())
+ {
+ // Wait a maximum of 10s. If that times out, we declare it a failure.
+ result = angle::Result::Stop();
+ }
+ ANGLE_TRY(result);
+
+ // Clean up finished batches.
+ return checkCompletedCommands(context);
+}
+
angle::Result RendererVk::getCompatibleRenderPass(vk::Context *context,
const vk::RenderPassDesc &desc,
vk::RenderPass **renderPassOut)
diff --git a/src/libANGLE/renderer/vulkan/RendererVk.h b/src/libANGLE/renderer/vulkan/RendererVk.h
index 5a930d3..2e88087 100644
--- a/src/libANGLE/renderer/vulkan/RendererVk.h
+++ b/src/libANGLE/renderer/vulkan/RendererVk.h
@@ -101,6 +101,13 @@
}
}
+ // Check to see which batches have finished completion (forward progress for
+ // mLastCompletedQueueSerial, for example for when the application busy waits on a query
+ // result).
+ angle::Result checkCompletedCommands(vk::Context *context);
+ // Wait for completion of batches until (at least) batch with given serial is finished.
+ angle::Result finishToSerial(vk::Context *context, Serial serial);
+
uint32_t getQueueFamilyIndex() const { return mCurrentQueueFamilyIndex; }
const vk::MemoryProperties &getMemoryProperties() const { return mMemoryProperties; }
@@ -180,7 +187,6 @@
angle::Result submitFrame(vk::Context *context,
const VkSubmitInfo &submitInfo,
vk::CommandBuffer &&commandBuffer);
- angle::Result checkInFlightCommands(vk::Context *context);
void freeAllInFlightResources();
angle::Result flushCommandGraph(vk::Context *context, vk::CommandBuffer *commandBatch);
void initFeatures();
diff --git a/src/libANGLE/renderer/vulkan/vk_utils.cpp b/src/libANGLE/renderer/vulkan/vk_utils.cpp
index 0454a41..b041bff 100644
--- a/src/libANGLE/renderer/vulkan/vk_utils.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_utils.cpp
@@ -1013,9 +1013,15 @@
vkCreateFence(context->getDevice(), &createInfo, nullptr, &mHandle));
}
-VkResult Fence::getStatus(VkDevice device) const
+angle::Result Fence::getStatus(Context *context) const
{
- return vkGetFenceStatus(device, mHandle);
+ ANGLE_VK_TRY_RETURN_ALLOW_NOT_READY(context, vkGetFenceStatus(context->getDevice(), mHandle));
+}
+
+angle::Result Fence::wait(Context *context, uint64_t timeout) const
+{
+ ANGLE_VK_TRY_RETURN_ALLOW_TIMEOUT(
+ context, vkWaitForFences(context->getDevice(), 1, &mHandle, true, timeout));
}
// MemoryProperties implementation.
diff --git a/src/libANGLE/renderer/vulkan/vk_utils.h b/src/libANGLE/renderer/vulkan/vk_utils.h
index dc5fbcc..7a6fbdd 100644
--- a/src/libANGLE/renderer/vulkan/vk_utils.h
+++ b/src/libANGLE/renderer/vulkan/vk_utils.h
@@ -607,7 +607,8 @@
using WrappedObject::operator=;
angle::Result init(Context *context, const VkFenceCreateInfo &createInfo);
- VkResult getStatus(VkDevice device) const;
+ angle::Result getStatus(Context *context) const;
+ angle::Result wait(Context *context, uint64_t timeout) const;
};
// Similar to StagingImage, for Buffers.
@@ -906,4 +907,7 @@
#define ANGLE_VK_TRY_RETURN_ALLOW_NOT_READY(context, command) \
ANGLE_VK_TRY_RETURN_ALLOW_OTHER(context, command, VK_NOT_READY)
+#define ANGLE_VK_TRY_RETURN_ALLOW_TIMEOUT(context, command) \
+ ANGLE_VK_TRY_RETURN_ALLOW_OTHER(context, command, VK_TIMEOUT)
+
#endif // LIBANGLE_RENDERER_VULKAN_VK_UTILS_H_