Skip to content

S
swiftshader
Commit d6dc4b7e by Chris Forbes
Options

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: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarChris Forbes <chrisforbes@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
parent 115cfe7a
Showing with 32 additions and 58 deletions
src/Vulkan/VkCommandBuffer.cpp
......@@ -61,7 +61,7 @@ protected:
{
executionState.renderPass = renderPass;
executionState.renderPassFramebuffer = framebuffer;
renderPass->begin();
executionState.subpassIndex = 0;
framebuffer->clear(executionState.renderPass, clearValueCount, clearValues, renderArea);
}
......@@ -83,17 +83,17 @@ public:
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 @@ protected:
// 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 @@ void CommandBuffer::ExecutionState::bindAttachments(sw::Context& context)
// 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 @@ struct DrawBase : public CommandBuffer::Command
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 @@ struct ClearAttachment : public CommandBuffer::Command
// 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:
......
src/Vulkan/VkCommandBuffer.hpp
......@@ -177,6 +177,8 @@ public:
VertexInputBinding indexBufferBinding;
VkIndexType indexType;
uint32_t subpassIndex = 0;
void bindAttachments(sw::Context& context);
void bindVertexInputs(sw::Context& context, int firstVertex, int firstInstance);
};
......
src/Vulkan/VkFramebuffer.cpp
......@@ -97,20 +97,20 @@ void Framebuffer::clear(const RenderPass* renderPass, uint32_t clearValueCount,
}
}
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 @@ void Framebuffer::clear(const RenderPass* renderPass, const VkClearAttachment& a
}
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 @@ ImageView *Framebuffer::getAttachment(uint32_t index) const
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 @@ void Framebuffer::resolve(const RenderPass* renderPass)
{
if (renderPass->isMultiView())
{
auto viewMask = renderPass->getViewMask();
auto viewMask = renderPass->getViewMask(subpassIndex);
while (viewMask)
{
int view = sw::log2i(viewMask);
......
src/Vulkan/VkFramebuffer.hpp
......@@ -30,11 +30,11 @@ public:
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;
......
src/Vulkan/VkRenderPass.cpp
......@@ -208,22 +208,6 @@ void RenderPass::getRenderAreaGranularity(VkExtent2D* pGranularity) const
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
......
src/Vulkan/VkRenderPass.hpp
......@@ -32,18 +32,14 @@ public:
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 @@ public:
return subpassCount;
}
VkSubpassDescription getSubpass(uint32_t i) const
{
return subpasses[i];
}
VkSubpassDescription getCurrentSubpass() const
VkSubpassDescription const& getSubpass(uint32_t subpassIndex) const
{
return subpasses[currentSubpass];
return subpasses[subpassIndex];
}
uint32_t getDependencyCount() const
......@@ -76,9 +67,9 @@ public:
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 @@ public:
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 @@ private:
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;
......
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