Skip to content

Commit

Permalink
Revert "vk: remove subpasses to simplify descriptor set refactor (#7592…
Browse files Browse the repository at this point in the history 
…)" (#7630)

This reverts commit a9793b3.

Due to change in output for swiftshader
  • Loading branch information
@poweifeng
poweifeng committed Mar 6, 2024
1 parent 04c7f84 commit 4e648b2
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 50 deletions.
12 changes: 7 additions & 5 deletions filament/backend/src/vulkan/VulkanConstants.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
// Usefaul default combinations
#define FVK_DEBUG_EVERYTHING 0xFFFFFFFF
#define FVK_DEBUG_PERFORMANCE \
FVK_DEBUG_SYSTRACE
FVK_DEBUG_SYSTRACE | \
FVK_DEBUG_GROUP_MARKERS

#define FVK_DEBUG_CORRECTNESS \
FVK_DEBUG_VALIDATION | \
Expand All @@ -79,17 +80,18 @@
FVK_DEBUG_PRINT_GROUP_MARKERS

#ifndef NDEBUG
#define FVK_DEBUG_FLAGS FVK_DEBUG_PERFORMANCE
#define FVK_DEBUG_FLAGS (FVK_DEBUG_PERFORMANCE | FVK_DEBUG_DEBUG_UTILS | FVK_DEBUG_VALIDATION)
#else
#define FVK_DEBUG_FLAGS 0
#endif

#define FVK_ENABLED(flags) (((FVK_DEBUG_FLAGS) & (flags)) == (flags))
#define FVK_ENABLED_BOOL(flags) FVK_ENABLED(flags)
#define FVK_ENABLED(flags) ((FVK_DEBUG_FLAGS) & (flags))
#define FVK_ENABLED_BOOL(flags) ((bool) FVK_ENABLED(flags))


// Group marker only works only if validation or debug utils is enabled since it uses
// vkCmd(Begin/End)DebugUtilsLabelEXT or vkCmdDebugMarker(Begin/End)EXT
#if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS)
#if FVK_ENABLED(FVK_DEBUG_PRINT_GROUP_MARKERS)
static_assert(FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) || FVK_ENABLED(FVK_DEBUG_VALIDATION));
#endif

Expand Down
1 change: 1 addition & 0 deletions filament/backend/src/vulkan/VulkanContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ struct VulkanRenderPass {
VulkanRenderTarget* renderTarget;
VkRenderPass renderPass;
RenderPassParams params;
int currentSubpass;
};

// This is a collection of immutable data about the vulkan context. This actual handles to the
Expand Down
34 changes: 31 additions & 3 deletions filament/backend/src/vulkan/VulkanDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ bool VulkanDriver::isRenderTargetFormatSupported(TextureFormat format) {
}

bool VulkanDriver::isFrameBufferFetchSupported() {
return false;
return true;
}

bool VulkanDriver::isFrameBufferFetchMultiSampleSupported() {
Expand Down Expand Up @@ -1247,7 +1247,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
}

VkRenderPass renderPass = mFramebufferCache.getRenderPass(rpkey);
mPipelineCache.bindRenderPass(renderPass);
mPipelineCache.bindRenderPass(renderPass, 0);

// Create the VkFramebuffer or fetch it from cache.
VulkanFboCache::FboKey fbkey {
Expand Down Expand Up @@ -1372,6 +1372,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
.renderTarget = rt,
.renderPass = renderPassInfo.renderPass,
.params = params,
.currentSubpass = 0,
};
FVK_SYSTRACE_END();
}
Expand Down Expand Up @@ -1414,13 +1415,40 @@ void VulkanDriver::endRenderPass(int) {
0, 1, &barrier, 0, nullptr, 0, nullptr);
}

if (mCurrentRenderPass.currentSubpass > 0) {
for (uint32_t i = 0; i < VulkanPipelineCache::INPUT_ATTACHMENT_COUNT; i++) {
mPipelineCache.bindInputAttachment(i, {});
}
mCurrentRenderPass.currentSubpass = 0;
}
mCurrentRenderPass.renderTarget = nullptr;
mCurrentRenderPass.renderPass = VK_NULL_HANDLE;
FVK_SYSTRACE_END();
}

void VulkanDriver::nextSubpass(int) {
PANIC_POSTCONDITION("Subpasses are unsupported");
ASSERT_PRECONDITION(mCurrentRenderPass.currentSubpass == 0,
"Only two subpasses are currently supported.");

VulkanRenderTarget* renderTarget = mCurrentRenderPass.renderTarget;
assert_invariant(renderTarget);
assert_invariant(mCurrentRenderPass.params.subpassMask);

vkCmdNextSubpass(mCommands->get().buffer(), VK_SUBPASS_CONTENTS_INLINE);

mPipelineCache.bindRenderPass(mCurrentRenderPass.renderPass,
++mCurrentRenderPass.currentSubpass);

for (uint32_t i = 0; i < VulkanPipelineCache::INPUT_ATTACHMENT_COUNT; i++) {
if ((1 << i) & mCurrentRenderPass.params.subpassMask) {
VulkanAttachment subpassInput = renderTarget->getColor(i);
VkDescriptorImageInfo info = {
.imageView = subpassInput.getImageView(VK_IMAGE_ASPECT_COLOR_BIT),
.imageLayout = ImgUtil::getVkLayout(subpassInput.getLayout()),
};
mPipelineCache.bindInputAttachment(i, info);
}
}
}

void VulkanDriver::setRenderPrimitiveBuffer(Handle<HwRenderPrimitive> rph, PrimitiveType pt,
Expand Down
65 changes: 56 additions & 9 deletions filament/backend/src/vulkan/VulkanFboCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept {
iter.value().timestamp = mCurrentTime;
return iter->second.handle;
}
const bool hasSubpasses = config.subpassMask != 0;

// Set up some const aliases for terseness.
const VkAttachmentLoadOp kClear = VK_ATTACHMENT_LOAD_OP_CLEAR;
Expand All @@ -140,39 +141,56 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept {
// In Vulkan, the subpass desc specifies the layout to transition to at the start of the render
// pass, and the attachment description specifies the layout to transition to at the end.

VkAttachmentReference inputAttachmentRef[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT] = {};
VkAttachmentReference colorAttachmentRefs[2][MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT] = {};
VkAttachmentReference resolveAttachmentRef[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT] = {};
VkAttachmentReference depthAttachmentRef = {};

const bool hasDepth = config.depthFormat != VK_FORMAT_UNDEFINED;

VkSubpassDescription subpasses[1] = {{
VkSubpassDescription subpasses[2] = {{
.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
.pInputAttachments = nullptr,
.pColorAttachments = colorAttachmentRefs[0],
.pResolveAttachments = resolveAttachmentRef,
.pDepthStencilAttachment = hasDepth ? &depthAttachmentRef : nullptr
},
{
.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
.pInputAttachments = inputAttachmentRef,
.pColorAttachments = colorAttachmentRefs[1],
.pResolveAttachments = resolveAttachmentRef,
.pDepthStencilAttachment = hasDepth ? &depthAttachmentRef : nullptr
}};

// The attachment list contains: Color Attachments, Resolve Attachments, and Depth Attachment.
// For simplicity, create an array that can hold the maximum possible number of attachments.
// Note that this needs to have the same ordering as the corollary array in getFramebuffer.
VkAttachmentDescription attachments[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT + MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT + 1] = {};

// We support 2 subpasses, which means we need to supply 1 dependency struct.
VkSubpassDependency dependencies[1] = {{
.srcSubpass = 0,
.dstSubpass = 1,
.srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
.dstStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
.dstAccessMask = VK_ACCESS_SHADER_READ_BIT,
.dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT,
}};

VkRenderPassCreateInfo renderPassInfo {
.sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO,
.attachmentCount = 0u,
.pAttachments = attachments,
.subpassCount = 1u,
.subpassCount = hasSubpasses ? 2u : 1u,
.pSubpasses = subpasses,
.dependencyCount = 0u,
.pDependencies = nullptr,
.dependencyCount = hasSubpasses ? 1u : 0u,
.pDependencies = dependencies
};

int attachmentIndex = 0;

assert_invariant(config.subpassMask == 0 && "Subpass is not supported");

// Populate the Color Attachments.
for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) {
if (config.colorFormat[i] == VK_FORMAT_UNDEFINED) {
Expand All @@ -181,9 +199,36 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept {
const VkImageLayout subpassLayout = ImgUtil::getVkLayout(VulkanLayout::COLOR_ATTACHMENT);
uint32_t index;

index = subpasses[0].colorAttachmentCount++;
colorAttachmentRefs[0][index].layout = subpassLayout;
colorAttachmentRefs[0][index].attachment = attachmentIndex;
if (!hasSubpasses) {
index = subpasses[0].colorAttachmentCount++;
colorAttachmentRefs[0][index].layout = subpassLayout;
colorAttachmentRefs[0][index].attachment = attachmentIndex;
} else {

// The Driver API consolidates all color attachments from the first and second subpasses
// into a single list, and uses a bitmask to mark attachments that belong only to the
// second subpass and should be available as inputs. All color attachments in the first
// subpass are automatically made available to the second subpass.

// If there are subpasses, we require the input attachment to be the first attachment.
// Breaking this assumption would likely require enhancements to the Driver API in order
// to supply Vulkan with all the information needed.
assert_invariant(config.subpassMask == 1);

if (config.subpassMask & (1 << i)) {
index = subpasses[0].colorAttachmentCount++;
colorAttachmentRefs[0][index].layout = subpassLayout;
colorAttachmentRefs[0][index].attachment = attachmentIndex;

index = subpasses[1].inputAttachmentCount++;
inputAttachmentRef[index].layout = subpassLayout;
inputAttachmentRef[index].attachment = attachmentIndex;
}

index = subpasses[1].colorAttachmentCount++;
colorAttachmentRefs[1][index].layout = subpassLayout;
colorAttachmentRefs[1][index].attachment = attachmentIndex;
}

const TargetBufferFlags flag = TargetBufferFlags(int(TargetBufferFlags::COLOR0) << i);
const bool clear = any(config.clear & flag);
Expand All @@ -207,6 +252,8 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept {
if (subpasses[0].colorAttachmentCount == 0) {
subpasses[0].pColorAttachments = nullptr;
subpasses[0].pResolveAttachments = nullptr;
subpasses[1].pColorAttachments = nullptr;
subpasses[1].pResolveAttachments = nullptr;
}

// Populate the Resolve Attachments.
Expand Down
5 changes: 4 additions & 1 deletion filament/backend/src/vulkan/VulkanHandles.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,10 @@ uint8_t VulkanRenderTarget::getColorTargetCount(const VulkanRenderPass& pass) co
if (!mColor[i].texture) {
continue;
}
count++;
// NOTE: This must be consistent with VkRenderPass construction (see VulkanFboCache).
if (!(pass.params.subpassMask & (1 << i)) || pass.currentSubpass == 1) {
count++;
}
}
return count;
}
Expand Down
85 changes: 74 additions & 11 deletions filament/backend/src/vulkan/VulkanPipelineCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ VulkanPipelineCache::VulkanPipelineCache(VulkanResourceAllocator* allocator)
mDummyBufferWriteInfo.pImageInfo = nullptr;
mDummyBufferWriteInfo.pBufferInfo = &mDummyBufferInfo;
mDummyBufferWriteInfo.pTexelBufferView = nullptr;

mDummyTargetInfo.imageLayout = VulkanImageUtility::getVkLayout(VulkanLayout::READ_ONLY);
mDummyTargetWriteInfo.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
mDummyTargetWriteInfo.pNext = nullptr;
mDummyTargetWriteInfo.dstArrayElement = 0;
mDummyTargetWriteInfo.descriptorCount = 1;
mDummyTargetWriteInfo.descriptorType = VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT;
mDummyTargetWriteInfo.pImageInfo = &mDummyTargetInfo;
mDummyTargetWriteInfo.pBufferInfo = nullptr;
mDummyTargetWriteInfo.pTexelBufferView = nullptr;
}

VulkanPipelineCache::~VulkanPipelineCache() {
Expand Down Expand Up @@ -246,7 +256,9 @@ VulkanPipelineCache::DescriptorCacheEntry* VulkanPipelineCache::createDescriptor
// Rewrite every binding in the new descriptor sets.
VkDescriptorBufferInfo descriptorBuffers[UBUFFER_BINDING_COUNT];
VkDescriptorImageInfo descriptorSamplers[SAMPLER_BINDING_COUNT];
VkWriteDescriptorSet descriptorWrites[UBUFFER_BINDING_COUNT + SAMPLER_BINDING_COUNT];
VkDescriptorImageInfo descriptorInputAttachments[INPUT_ATTACHMENT_COUNT];
VkWriteDescriptorSet descriptorWrites[UBUFFER_BINDING_COUNT + SAMPLER_BINDING_COUNT +
INPUT_ATTACHMENT_COUNT];
uint32_t nwrites = 0;
VkWriteDescriptorSet* writes = descriptorWrites;
nwrites = 0;
Expand Down Expand Up @@ -296,6 +308,23 @@ VulkanPipelineCache::DescriptorCacheEntry* VulkanPipelineCache::createDescriptor
writeInfo.dstBinding = binding;
}
}
for (uint32_t binding = 0; binding < INPUT_ATTACHMENT_COUNT; binding++) {
if (mDescriptorRequirements.inputAttachments[binding].imageView) {
VkWriteDescriptorSet& writeInfo = writes[nwrites++];
VkDescriptorImageInfo& imageInfo = descriptorInputAttachments[binding];
imageInfo = mDescriptorRequirements.inputAttachments[binding];
writeInfo.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
writeInfo.pNext = nullptr;
writeInfo.dstArrayElement = 0;
writeInfo.descriptorCount = 1;
writeInfo.descriptorType = VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT;
writeInfo.pImageInfo = &imageInfo;
writeInfo.pBufferInfo = nullptr;
writeInfo.pTexelBufferView = nullptr;
writeInfo.dstSet = descriptorCacheEntry.handles[2];
writeInfo.dstBinding = binding;
}
}

vkUpdateDescriptorSets(mDevice, nwrites, writes, 0, nullptr);

Expand Down Expand Up @@ -373,15 +402,15 @@ VulkanPipelineCache::PipelineCacheEntry* VulkanPipelineCache::createPipeline() n

const bool hasFragmentShader = shaderStages[1].module != VK_NULL_HANDLE;

VkGraphicsPipelineCreateInfo pipelineCreateInfo = {
.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO,
.stageCount = hasFragmentShader ? SHADER_MODULE_COUNT : 1,
.pStages = shaderStages,
.pVertexInputState = &vertexInputState,
.pInputAssemblyState = &inputAssemblyState,
.layout = layout->handle,
.renderPass = mPipelineRequirements.renderPass,
};
VkGraphicsPipelineCreateInfo pipelineCreateInfo = {};
pipelineCreateInfo.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO;
pipelineCreateInfo.layout = layout->handle;
pipelineCreateInfo.renderPass = mPipelineRequirements.renderPass;
pipelineCreateInfo.subpass = mPipelineRequirements.subpassIndex;
pipelineCreateInfo.stageCount = hasFragmentShader ? SHADER_MODULE_COUNT : 1;
pipelineCreateInfo.pStages = shaderStages;
pipelineCreateInfo.pVertexInputState = &vertexInputState;
pipelineCreateInfo.pInputAssemblyState = &inputAssemblyState;

VkPipelineRasterizationStateCreateInfo vkRaster = {};
vkRaster.sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO;
Expand Down Expand Up @@ -506,6 +535,18 @@ VulkanPipelineCache::PipelineLayoutCacheEntry* VulkanPipelineCache::getOrCreateP
dlinfo.pBindings = sbindings;
vkCreateDescriptorSetLayout(mDevice, &dlinfo, VKALLOC, &cacheEntry.descriptorSetLayouts[1]);

// Next create the descriptor set layout for input attachments.
VkDescriptorSetLayoutBinding tbindings[INPUT_ATTACHMENT_COUNT];
binding.descriptorType = VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT;
binding.stageFlags = VK_SHADER_STAGE_FRAGMENT_BIT;
for (uint32_t i = 0; i < INPUT_ATTACHMENT_COUNT; i++) {
binding.binding = i;
tbindings[i] = binding;
}
dlinfo.bindingCount = INPUT_ATTACHMENT_COUNT;
dlinfo.pBindings = tbindings;
vkCreateDescriptorSetLayout(mDevice, &dlinfo, VKALLOC, &cacheEntry.descriptorSetLayouts[2]);

// Create VkPipelineLayout based on how to resources are bounded.
VkPipelineLayoutCreateInfo pPipelineLayoutCreateInfo = {};
pPipelineLayoutCreateInfo.sType = VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO;
Expand All @@ -528,8 +569,9 @@ void VulkanPipelineCache::bindRasterState(const RasterState& rasterState) noexce
mPipelineRequirements.rasterState = rasterState;
}

void VulkanPipelineCache::bindRenderPass(VkRenderPass renderPass) noexcept {
void VulkanPipelineCache::bindRenderPass(VkRenderPass renderPass, int subpassIndex) noexcept {
mPipelineRequirements.renderPass = renderPass;
mPipelineRequirements.subpassIndex = subpassIndex;
}

void VulkanPipelineCache::bindPrimitiveTopology(VkPrimitiveTopology topology) noexcept {
Expand Down Expand Up @@ -577,6 +619,11 @@ void VulkanPipelineCache::unbindImageView(VkImageView imageView) noexcept {
sampler = {};
}
}
for (auto& target : mDescriptorRequirements.inputAttachments) {
if (target.imageView == imageView) {
target = {};
}
}
}

void VulkanPipelineCache::bindUniformBufferObject(uint32_t bindingIndex,
Expand Down Expand Up @@ -615,6 +662,14 @@ void VulkanPipelineCache::bindSamplers(VkDescriptorImageInfo samplers[SAMPLER_BI
mPipelineRequirements.layout = flags;
}

void VulkanPipelineCache::bindInputAttachment(uint32_t bindingIndex,
VkDescriptorImageInfo targetInfo) noexcept {
ASSERT_POSTCONDITION(bindingIndex < INPUT_ATTACHMENT_COUNT,
"Input attachment bindings overflow: index = %d, capacity = %d.",
bindingIndex, INPUT_ATTACHMENT_COUNT);
mDescriptorRequirements.inputAttachments[bindingIndex] = targetInfo;
}

void VulkanPipelineCache::terminate() noexcept {
// Symmetric to createLayoutsAndDescriptors.
destroyLayoutsAndDescriptors();
Expand Down Expand Up @@ -738,6 +793,8 @@ VkDescriptorPool VulkanPipelineCache::createDescriptorPool(uint32_t size) const
poolSizes[0].descriptorCount = poolInfo.maxSets * UBUFFER_BINDING_COUNT;
poolSizes[1].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
poolSizes[1].descriptorCount = poolInfo.maxSets * SAMPLER_BINDING_COUNT;
poolSizes[2].type = VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT;
poolSizes[2].descriptorCount = poolInfo.maxSets * INPUT_ATTACHMENT_COUNT;

VkDescriptorPool pool;
const UTILS_UNUSED VkResult result = vkCreateDescriptorPool(mDevice, &poolInfo, VKALLOC, &pool);
Expand Down Expand Up @@ -848,6 +905,12 @@ bool VulkanPipelineCache::DescEqual::operator()(const DescriptorKey& k1,
return false;
}
}
for (uint32_t i = 0; i < INPUT_ATTACHMENT_COUNT; i++) {
if (k1.inputAttachments[i].imageView != k2.inputAttachments[i].imageView ||
k1.inputAttachments[i].imageLayout != k2.inputAttachments[i].imageLayout) {
return false;
}
}
return true;
}

Expand Down
Loading

0 comments on commit 4e648b2

Please sign in to comment.