Commit 34f66126 by Courtney Goeltzenleuchter Committed by Commit Bot

Vulkan: Check that its okay to add commands

It can be hard to tell sometimes when the mRenderPassCommands or mOutSideRenderPassCommands command buffers have changed and there have been some issues with code that locally caches a pointer to a commandBuffer that then becomes invalid. This change adds checking so that if a command is being added to a commandBuffer that's been closed (e.g. submitted for processing) then we hit an assert. Bug: b/168144059 Change-Id: If5d37c462e3bcb51f6ec2ca44c27a2fad4e57c19 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2405812Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Courtney Goeltzenleuchter <courtneygo@google.com>
parent 463e02e6
...@@ -4726,6 +4726,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass() ...@@ -4726,6 +4726,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
if (mRenderer->getFeatures().enableCommandProcessingThread.enabled) if (mRenderer->getFeatures().enableCommandProcessingThread.enabled)
{ {
mRenderPassCommands->markClosed();
vk::CommandProcessorTask task = {this, &mPrimaryCommands, mRenderPassCommands}; vk::CommandProcessorTask task = {this, &mPrimaryCommands, mRenderPassCommands};
ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::flushInsideRenderPassCommands"); ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::flushInsideRenderPassCommands");
queueCommandsToWorker(task); queueCommandsToWorker(task);
...@@ -4768,6 +4769,7 @@ void ContextVk::getNextAvailableCommandBuffer(vk::CommandBufferHelper **commandB ...@@ -4768,6 +4769,7 @@ void ContextVk::getNextAvailableCommandBuffer(vk::CommandBufferHelper **commandB
mAvailableCommandBuffers.pop(); mAvailableCommandBuffers.pop();
lock.unlock(); lock.unlock();
(*commandBuffer)->setHasRenderPass(hasRenderPass); (*commandBuffer)->setHasRenderPass(hasRenderPass);
(*commandBuffer)->markOpen();
} }
void ContextVk::recycleCommandBuffer(vk::CommandBufferHelper *commandBuffer) void ContextVk::recycleCommandBuffer(vk::CommandBufferHelper *commandBuffer)
...@@ -4865,6 +4867,7 @@ angle::Result ContextVk::flushOutsideRenderPassCommands() ...@@ -4865,6 +4867,7 @@ angle::Result ContextVk::flushOutsideRenderPassCommands()
if (mRenderer->getFeatures().enableCommandProcessingThread.enabled) if (mRenderer->getFeatures().enableCommandProcessingThread.enabled)
{ {
mOutsideRenderPassCommands->markClosed();
vk::CommandProcessorTask task = {this, &mPrimaryCommands, mOutsideRenderPassCommands}; vk::CommandProcessorTask task = {this, &mPrimaryCommands, mOutsideRenderPassCommands};
ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::flushOutsideRenderPassCommands"); ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::flushOutsideRenderPassCommands");
queueCommandsToWorker(task); queueCommandsToWorker(task);
......
...@@ -688,6 +688,9 @@ class SecondaryCommandBuffer final : angle::NonCopyable ...@@ -688,6 +688,9 @@ class SecondaryCommandBuffer final : angle::NonCopyable
reinterpret_cast<CommandHeader *>(mCurrentWritePointer)->id = CommandID::Invalid; reinterpret_cast<CommandHeader *>(mCurrentWritePointer)->id = CommandID::Invalid;
} }
void open() { mIsOpen = true; }
void close() { mIsOpen = false; }
void reset() void reset()
{ {
mCommands.clear(); mCommands.clear();
...@@ -716,6 +719,7 @@ class SecondaryCommandBuffer final : angle::NonCopyable ...@@ -716,6 +719,7 @@ class SecondaryCommandBuffer final : angle::NonCopyable
template <class StructType> template <class StructType>
ANGLE_INLINE StructType *commonInit(CommandID cmdID, size_t allocationSize) ANGLE_INLINE StructType *commonInit(CommandID cmdID, size_t allocationSize)
{ {
ASSERT(mIsOpen);
mCurrentBytesRemaining -= allocationSize; mCurrentBytesRemaining -= allocationSize;
CommandHeader *header = reinterpret_cast<CommandHeader *>(mCurrentWritePointer); CommandHeader *header = reinterpret_cast<CommandHeader *>(mCurrentWritePointer);
...@@ -798,6 +802,9 @@ class SecondaryCommandBuffer final : angle::NonCopyable ...@@ -798,6 +802,9 @@ class SecondaryCommandBuffer final : angle::NonCopyable
return writePointer + sizeInBytes; return writePointer + sizeInBytes;
} }
// flag to indicate that commandBuffer is open for new commands
bool mIsOpen;
std::vector<CommandHeader *> mCommands; std::vector<CommandHeader *> mCommands;
// Allocator used by this class. If non-null then the class is valid. // Allocator used by this class. If non-null then the class is valid.
......
...@@ -957,6 +957,18 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -957,6 +957,18 @@ class CommandBufferHelper : angle::NonCopyable
void executeBarriers(ContextVk *contextVk, PrimaryCommandBuffer *primary); void executeBarriers(ContextVk *contextVk, PrimaryCommandBuffer *primary);
void setHasRenderPass(bool hasRenderPass) { mIsRenderPassCommandBuffer = hasRenderPass; } void setHasRenderPass(bool hasRenderPass) { mIsRenderPassCommandBuffer = hasRenderPass; }
// The markOpen and markClosed functions are to aid in proper use of the CommandBufferHelper.
// saw invalid use due to threading issues that can be easily caught by marking when it's safe
// (open) to write to the commandbuffer.
#if defined(ANGLE_ENABLE_ASSERTS)
void markOpen() { mCommandBuffer.open(); }
void markClosed() { mCommandBuffer.close(); }
#else
void markOpen() {}
void markClosed() {}
#endif
void reset(); void reset();
void releaseToContextQueue(ContextVk *contextVk); void releaseToContextQueue(ContextVk *contextVk);
......
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