Store subpass index in ExecutionState, not in RenderPass
A single RenderPass object may be used to record multiple command
buffers simultaneously. Storing the subpass index there produces
crosstalk between the command buffer recordings, and incorrect
rendering.
Bug: b/139824232
Change-Id: Ib8689a47733a991c61785d2d6332584f7a96f05d
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35468
Presubmit-Ready: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Chris Forbes <chrisforbes@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Vulkan/VkCommandBuffer.cpp b/src/Vulkan/VkCommandBuffer.cpp
index fde05d6..bad25fc 100644
--- a/src/Vulkan/VkCommandBuffer.cpp
+++ b/src/Vulkan/VkCommandBuffer.cpp
@@ -61,7 +61,7 @@
{
executionState.renderPass = renderPass;
executionState.renderPassFramebuffer = framebuffer;
- renderPass->begin();
+ executionState.subpassIndex = 0;
framebuffer->clear(executionState.renderPass, clearValueCount, clearValues, renderArea);
}
@@ -83,17 +83,17 @@
protected:
void play(CommandBuffer::ExecutionState& executionState) override
{
- bool hasResolveAttachments = (executionState.renderPass->getCurrentSubpass().pResolveAttachments != nullptr);
+ bool hasResolveAttachments = (executionState.renderPass->getSubpass(executionState.subpassIndex).pResolveAttachments != nullptr);
if(hasResolveAttachments)
{
// FIXME(sugoi): remove the following lines and resolve in Renderer::finishRendering()
// for a Draw command or after the last command of the current subpass
// which modifies pixels.
executionState.renderer->synchronize();
- executionState.renderPassFramebuffer->resolve(executionState.renderPass);
+ executionState.renderPassFramebuffer->resolve(executionState.renderPass, executionState.subpassIndex);
}
- executionState.renderPass->nextSubpass();
+ ++executionState.subpassIndex;
}
};
@@ -114,9 +114,7 @@
// FIXME(sugoi): remove the following line and resolve in Renderer::finishRendering()
// for a Draw command or after the last command of the current subpass
// which modifies pixels.
- executionState.renderPassFramebuffer->resolve(executionState.renderPass);
-
- executionState.renderPass->end();
+ executionState.renderPassFramebuffer->resolve(executionState.renderPass, executionState.subpassIndex);
executionState.renderPass = nullptr;
executionState.renderPassFramebuffer = nullptr;
}
@@ -434,16 +432,18 @@
// there is too much stomping of the renderer's state by setContext() in
// draws.
- for (auto i = 0u; i < renderPass->getCurrentSubpass().colorAttachmentCount; i++)
+ auto const & subpass = renderPass->getSubpass(subpassIndex);
+
+ for (auto i = 0u; i < subpass.colorAttachmentCount; i++)
{
- auto attachmentReference = renderPass->getCurrentSubpass().pColorAttachments[i];
+ auto attachmentReference = subpass.pColorAttachments[i];
if (attachmentReference.attachment != VK_ATTACHMENT_UNUSED)
{
context.renderTarget[i] = renderPassFramebuffer->getAttachment(attachmentReference.attachment);
}
}
- auto attachmentReference = renderPass->getCurrentSubpass().pDepthStencilAttachment;
+ auto attachmentReference = subpass.pDepthStencilAttachment;
if (attachmentReference && attachmentReference->attachment != VK_ATTACHMENT_UNUSED)
{
auto attachment = renderPassFramebuffer->getAttachment(attachmentReference->attachment);
@@ -606,7 +606,7 @@
context.instanceID = instance;
// FIXME: reconsider instances/views nesting.
- auto viewMask = executionState.renderPass->getViewMask();
+ auto viewMask = executionState.renderPass->getViewMask(executionState.subpassIndex);
while (viewMask)
{
context.viewID = sw::log2i(viewMask);
@@ -869,7 +869,7 @@
// however, we don't do the clear through the rasterizer, so need to ensure prior drawing
// has completed first.
executionState.renderer->synchronize();
- executionState.renderPassFramebuffer->clear(executionState.renderPass, attachment, rect);
+ executionState.renderPassFramebuffer->clearAttachment(executionState.renderPass, executionState.subpassIndex, attachment, rect);
}
private:
diff --git a/src/Vulkan/VkCommandBuffer.hpp b/src/Vulkan/VkCommandBuffer.hpp
index 8c4351b..b592add 100644
--- a/src/Vulkan/VkCommandBuffer.hpp
+++ b/src/Vulkan/VkCommandBuffer.hpp
@@ -177,6 +177,8 @@
VertexInputBinding indexBufferBinding;
VkIndexType indexType;
+ uint32_t subpassIndex = 0;
+
void bindAttachments(sw::Context& context);
void bindVertexInputs(sw::Context& context, int firstVertex, int firstInstance);
};
diff --git a/src/Vulkan/VkFramebuffer.cpp b/src/Vulkan/VkFramebuffer.cpp
index f9c0716..a879a78 100644
--- a/src/Vulkan/VkFramebuffer.cpp
+++ b/src/Vulkan/VkFramebuffer.cpp
@@ -97,20 +97,20 @@
}
}
-void Framebuffer::clear(const RenderPass* renderPass, const VkClearAttachment& attachment, const VkClearRect& rect)
+void Framebuffer::clearAttachment(const RenderPass* renderPass, uint32_t subpassIndex, const VkClearAttachment& attachment, const VkClearRect& rect)
{
+ VkSubpassDescription subpass = renderPass->getSubpass(subpassIndex);
+
if(attachment.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT)
{
if(attachment.colorAttachment != VK_ATTACHMENT_UNUSED)
{
- VkSubpassDescription subpass = renderPass->getCurrentSubpass();
-
ASSERT(attachment.colorAttachment < subpass.colorAttachmentCount);
ASSERT(subpass.pColorAttachments[attachment.colorAttachment].attachment < attachmentCount);
if (renderPass->isMultiView())
{
- auto viewMask = renderPass->getViewMask();
+ auto viewMask = renderPass->getViewMask(subpassIndex);
while (viewMask)
{
int view = sw::log2i(viewMask);
@@ -127,13 +127,11 @@
}
else if(attachment.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))
{
- VkSubpassDescription subpass = renderPass->getCurrentSubpass();
-
ASSERT(subpass.pDepthStencilAttachment->attachment < attachmentCount);
if (renderPass->isMultiView())
{
- auto viewMask = renderPass->getViewMask();
+ auto viewMask = renderPass->getViewMask(subpassIndex);
while (viewMask)
{
int view = sw::log2i(viewMask);
@@ -153,9 +151,9 @@
return attachments[index];
}
-void Framebuffer::resolve(const RenderPass* renderPass)
+void Framebuffer::resolve(const RenderPass* renderPass, uint32_t subpassIndex)
{
- VkSubpassDescription subpass = renderPass->getCurrentSubpass();
+ auto const& subpass = renderPass->getSubpass(subpassIndex);
if(subpass.pResolveAttachments)
{
for(uint32_t i = 0; i < subpass.colorAttachmentCount; i++)
@@ -165,7 +163,7 @@
{
if (renderPass->isMultiView())
{
- auto viewMask = renderPass->getViewMask();
+ auto viewMask = renderPass->getViewMask(subpassIndex);
while (viewMask)
{
int view = sw::log2i(viewMask);
diff --git a/src/Vulkan/VkFramebuffer.hpp b/src/Vulkan/VkFramebuffer.hpp
index cfc5e07..0c4853b 100644
--- a/src/Vulkan/VkFramebuffer.hpp
+++ b/src/Vulkan/VkFramebuffer.hpp
@@ -30,11 +30,11 @@
void destroy(const VkAllocationCallbacks* pAllocator);
void clear(const RenderPass* renderPass, uint32_t clearValueCount, const VkClearValue* pClearValues, const VkRect2D& renderArea);
- void clear(const RenderPass* renderPass, const VkClearAttachment& attachment, const VkClearRect& rect);
+ void clearAttachment(const RenderPass* renderPass, uint32_t subpassIndex, const VkClearAttachment& attachment, const VkClearRect& rect);
static size_t ComputeRequiredAllocationSize(const VkFramebufferCreateInfo* pCreateInfo);
ImageView *getAttachment(uint32_t index) const;
- void resolve(const RenderPass* renderPass);
+ void resolve(const RenderPass* renderPass, uint32_t subpassIndex);
private:
uint32_t attachmentCount = 0;
diff --git a/src/Vulkan/VkRenderPass.cpp b/src/Vulkan/VkRenderPass.cpp
index 852679b..10682ef 100644
--- a/src/Vulkan/VkRenderPass.cpp
+++ b/src/Vulkan/VkRenderPass.cpp
@@ -208,22 +208,6 @@
pGranularity->height = 1;
}
-void RenderPass::begin()
-{
- currentSubpass = 0;
-}
-
-void RenderPass::nextSubpass()
-{
- ++currentSubpass;
- ASSERT(currentSubpass < subpassCount);
-}
-
-void RenderPass::end()
-{
- currentSubpass = 0;
-}
-
void RenderPass::MarkFirstUse(int attachment, int subpass)
{
// FIXME: we may not actually need to track attachmentFirstUse if we're going to eagerly
diff --git a/src/Vulkan/VkRenderPass.hpp b/src/Vulkan/VkRenderPass.hpp
index 2d0841f..338c8aa 100644
--- a/src/Vulkan/VkRenderPass.hpp
+++ b/src/Vulkan/VkRenderPass.hpp
@@ -32,18 +32,14 @@
void getRenderAreaGranularity(VkExtent2D* pGranularity) const;
- void begin();
- void nextSubpass();
- void end();
-
uint32_t getAttachmentCount() const
{
return attachmentCount;
}
- VkAttachmentDescription getAttachment(uint32_t i) const
+ VkAttachmentDescription getAttachment(uint32_t attachmentIndex) const
{
- return attachments[i];
+ return attachments[attachmentIndex];
}
uint32_t getSubpassCount() const
@@ -51,14 +47,9 @@
return subpassCount;
}
- VkSubpassDescription getSubpass(uint32_t i) const
+ VkSubpassDescription const& getSubpass(uint32_t subpassIndex) const
{
- return subpasses[i];
- }
-
- VkSubpassDescription getCurrentSubpass() const
- {
- return subpasses[currentSubpass];
+ return subpasses[subpassIndex];
}
uint32_t getDependencyCount() const
@@ -76,9 +67,9 @@
return attachmentFirstUse[i] >= 0;
}
- uint32_t getViewMask() const
+ uint32_t getViewMask(uint32_t subpassIndex) const
{
- return viewMasks ? viewMasks[currentSubpass] : 1;
+ return viewMasks ? viewMasks[subpassIndex] : 1;
}
bool isMultiView() const
@@ -86,9 +77,9 @@
return viewMasks != nullptr;
}
- uint32_t getAttachmentViewMask(uint32_t i) const
+ uint32_t getAttachmentViewMask(uint32_t attachmentIndex) const
{
- return attachmentViewMasks[i];
+ return attachmentViewMasks[attachmentIndex];
}
private:
@@ -98,7 +89,6 @@
VkSubpassDescription* subpasses = nullptr;
uint32_t dependencyCount = 0;
VkSubpassDependency* dependencies = nullptr;
- uint32_t currentSubpass = 0;
int* attachmentFirstUse = nullptr;
uint32_t* viewMasks = nullptr;
uint32_t* attachmentViewMasks = nullptr;