Skip to content

Commit

Permalink
vk: layout transition clean up (#7217)
Browse files Browse the repository at this point in the history 
- Return the correct SubresourceRange for depth attachments
 - Fix transition for when one layout within mulitple mip-levels
   is different
 - Use implicit layout transition for renderpasses
 - Fix access mask for sampler in vertex shaders
 - Use unordered_map for VkSubresourceRange in VulkanTexture
  • Loading branch information
@poweifeng
poweifeng authored Oct 2, 2023
1 parent 976f304 commit df50f4c
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 116 deletions.
64 changes: 34 additions & 30 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,20 @@ namespace {
inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect, VkFilter filter,
const VkExtent2D srcExtent, VulkanAttachment src, VulkanAttachment dst,
const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) {
const VkImageBlit blitRegions[1] = {{.srcSubresource = {aspect, src.level, src.layer, 1},
const VkImageBlit blitRegions[1] = {{
.srcSubresource = {aspect, src.level, src.layer, 1},
.srcOffsets = {srcRect[0], srcRect[1]},
.dstSubresource = {aspect, dst.level, dst.layer, 1},
.dstOffsets = {dstRect[0], dstRect[1]}}};
.dstOffsets = {dstRect[0], dstRect[1]},
}};

const VkImageResolve resolveRegions[1] = {{.srcSubresource = {aspect, src.level, src.layer, 1},
const VkImageResolve resolveRegions[1] = {{
.srcSubresource = {aspect, src.level, src.layer, 1},
.srcOffset = srcRect[0],
.dstSubresource = {aspect, dst.level, dst.layer, 1},
.dstOffset = dstRect[0],
.extent = {srcExtent.width, srcExtent.height, 1}}};
.extent = {srcExtent.width, srcExtent.height, 1},
}};

const VkImageSubresourceRange srcRange = {
.aspectMask = aspect,
Expand All @@ -69,11 +73,14 @@ inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect,

if constexpr (FVK_ENABLED(FVK_DEBUG_BLITTER)) {
utils::slog.d << "Fast blit from=" << src.texture->getVkImage() << ",level=" << (int) src.level
<< "layout=" << src.getLayout()
<< " layout=" << src.getLayout()
<< " to=" << dst.texture->getVkImage() << ",level=" << (int) dst.level
<< "layout=" << dst.getLayout() << utils::io::endl;
<< " layout=" << dst.getLayout() << utils::io::endl;
}

VulkanLayout oldSrcLayout = src.getLayout();
VulkanLayout oldDstLayout = dst.getLayout();

src.texture->transitionLayout(cmdbuffer, srcRange, VulkanLayout::TRANSFER_SRC);
dst.texture->transitionLayout(cmdbuffer, dstRange, VulkanLayout::TRANSFER_DST);

Expand All @@ -91,17 +98,14 @@ inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect,
1, blitRegions, filter);
}

VulkanLayout newSrcLayout = ImgUtil::getDefaultLayout(src.texture->usage);
VulkanLayout const newDstLayout = ImgUtil::getDefaultLayout(dst.texture->usage);

// In the case of blitting the depth attachment, we transition the source into GENERAL (for
// sampling) and set the copy as ATTACHMENT_OPTIMAL (to be set as the attachment).
if (any(src.texture->usage & TextureUsage::DEPTH_ATTACHMENT)) {
newSrcLayout = VulkanLayout::DEPTH_SAMPLER;
if (oldSrcLayout == VulkanLayout::UNDEFINED) {
oldSrcLayout = ImgUtil::getDefaultLayout(src.texture->usage);
}

src.texture->transitionLayout(cmdbuffer, srcRange, newSrcLayout);
dst.texture->transitionLayout(cmdbuffer, dstRange, newDstLayout);
if (oldDstLayout == VulkanLayout::UNDEFINED) {
oldDstLayout = ImgUtil::getDefaultLayout(dst.texture->usage);
}
src.texture->transitionLayout(cmdbuffer, srcRange, oldSrcLayout);
dst.texture->transitionLayout(cmdbuffer, dstRange, oldDstLayout);
}

struct BlitterUniforms {
Expand Down Expand Up @@ -190,19 +194,19 @@ void VulkanBlitter::blitDepth(BlitArgs args) {
}

void VulkanBlitter::terminate() noexcept {
if (mDevice) {
if (mDepthResolveProgram) {
delete mDepthResolveProgram;
mDepthResolveProgram = nullptr;
}

if (mTriangleBuffer) {
delete mTriangleBuffer;
mTriangleBuffer = nullptr;
}
if (mTriangleBuffer) {
delete mTriangleBuffer;
mTriangleBuffer = nullptr;
}

if (mParamsBuffer) {
delete mParamsBuffer;
mParamsBuffer = nullptr;
}
if (mParamsBuffer) {
delete mParamsBuffer;
mParamsBuffer = nullptr;
}
}

Expand Down Expand Up @@ -239,11 +243,11 @@ void VulkanBlitter::lazyInit() noexcept {
mDepthResolveProgram->samplerGroupInfo[0].samplers.reserve(1);
mDepthResolveProgram->samplerGroupInfo[0].samplers.resize(1);

if constexpr (FVK_ENABLED(FVK_DEBUG_BLITTER)) {
utils::slog.d << "Created Shader Module for VulkanBlitter "
<< "shaders = (" << vertexShader << ", " << fragmentShader << ")"
<< utils::io::endl;
}
#if FVK_ENABLED(FVK_DEBUG_BLITTER)
utils::slog.d << "Created Shader Module for VulkanBlitter "
<< "shaders = (" << vertexShader << ", " << fragmentShader << ")"
<< utils::io::endl;
#endif

static const float kTriangleVertices[] = {
-1.0f, -1.0f,
Expand Down
13 changes: 2 additions & 11 deletions filament/backend/src/vulkan/VulkanContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,12 @@ VkImageView VulkanAttachment::getImageView(VkImageAspectFlags aspect) {

VkImageSubresourceRange VulkanAttachment::getSubresourceRange(VkImageAspectFlags aspect) const {
assert_invariant(texture);
uint32_t levelCount = 1;
uint32_t layerCount = 1;
// For depth attachments, we consider all the subresource range since layout transitions of
// depth and stencil attachments should always be carried out for all subresources.
if (aspect & VK_IMAGE_ASPECT_DEPTH_BIT) {
auto range = texture->getPrimaryRange();
levelCount = range.levelCount;
layerCount = range.layerCount;
}
return {
.aspectMask = aspect,
.baseMipLevel = uint32_t(level),
.levelCount = levelCount,
.levelCount = 1,
.baseArrayLayer = uint32_t(layer),
.layerCount = layerCount,
.layerCount = 1,
};
}

Expand Down
58 changes: 31 additions & 27 deletions filament/backend/src/vulkan/VulkanDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,6 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
// If that's the case, we need to change the layout of the texture to DEPTH_SAMPLER, which is a
// more general layout. Otherwise, we prefer the DEPTH_ATTACHMENT layout, which is optimal for
// the non-sampling case.
bool samplingDepthAttachment = false;
VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer const cmdbuffer = commands.buffer();

Expand All @@ -1041,29 +1040,24 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
if (!any(texture->usage & TextureUsage::DEPTH_ATTACHMENT)) {
continue;
}
samplingDepthAttachment
= depth.texture && texture->getVkImage() == depth.texture->getVkImage();
if (texture->getPrimaryImageLayout() == VulkanLayout::DEPTH_SAMPLER) {
continue;
}
VkImageSubresourceRange const subresources{
.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT,
.baseMipLevel = 0,
.levelCount = texture->levels,
.baseArrayLayer = 0,
.layerCount = texture->depth,
};
commands.acquire(texture);
texture->transitionLayout(cmdbuffer, subresources, VulkanLayout::DEPTH_SAMPLER);

// Transition the primary view, which is the sampler's view into the right layout.
texture->transitionLayout(cmdbuffer, texture->getPrimaryViewRange(),
VulkanLayout::DEPTH_SAMPLER);
break;
}
}
}
// currentDepthLayout tracks state of the layout after the (potential) transition in the above block.
VulkanLayout currentDepthLayout = depth.getLayout();
VulkanLayout const renderPassDepthLayout = samplingDepthAttachment
? VulkanLayout::DEPTH_SAMPLER
: VulkanLayout::DEPTH_ATTACHMENT;

VulkanLayout const currentDepthLayout = depth.getLayout();
VulkanLayout const renderPassDepthLayout = VulkanLayout::DEPTH_ATTACHMENT;
// We need to keep the final layout as an attachment because the implicit transition does not
// have any barrier guarrantees, meaning that if we want to sample from the output in the next
// pass, then we'd have a race-condition/validation error.
VulkanLayout const finalDepthLayout = renderPassDepthLayout;

TargetBufferFlags clearVal = params.flags.clear;
Expand All @@ -1073,11 +1067,8 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
discardEndVal &= ~TargetBufferFlags::DEPTH;
clearVal &= ~TargetBufferFlags::DEPTH;
}
if (currentDepthLayout != renderPassDepthLayout) {
depth.texture->transitionLayout(cmdbuffer,
depth.getSubresourceRange(VK_IMAGE_ASPECT_DEPTH_BIT), renderPassDepthLayout);
currentDepthLayout = renderPassDepthLayout;
}
auto const attachmentSubresourceRange = depth.getSubresourceRange(VK_IMAGE_ASPECT_DEPTH_BIT);
depth.texture->setLayout(attachmentSubresourceRange, VulkanLayout::DEPTH_ATTACHMENT);
}

// Create the VkRenderPass or fetch it from cache.
Expand Down Expand Up @@ -1271,12 +1262,11 @@ void VulkanDriver::endRenderPass(int) {
// NOTE: ideally dstStageMask would merely be VERTEX_SHADER_BIT | FRAGMENT_SHADER_BIT, but this
// seems to be insufficient on Mali devices. To work around this we are adding a more aggressive
// TOP_OF_PIPE barrier.

if (!rt->isSwapChain()) {
VkMemoryBarrier barrier {
.sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER,
.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
.dstAccessMask = VK_ACCESS_SHADER_READ_BIT,
VkMemoryBarrier barrier{
.sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER,
.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
.dstAccessMask = VK_ACCESS_SHADER_READ_BIT,
};
VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
if (rt->hasDepth()) {
Expand Down Expand Up @@ -1592,6 +1582,7 @@ void VulkanDriver::draw(PipelineState pipelineState, Handle<HwRenderPrimitive> r

if (UTILS_LIKELY(boundSampler->t)) {
VulkanTexture* texture = mResourceAllocator.handle_cast<VulkanTexture*>(boundSampler->t);
VkImageViewType const expectedType = texture->getViewType();

// TODO: can this uninitialized check be checked in a higher layer?
// This fallback path is very flaky because the dummy texture might not have
Expand All @@ -1610,9 +1601,22 @@ void VulkanDriver::draw(PipelineState pipelineState, Handle<HwRenderPrimitive> r

usage = VulkanPipelineCache::getUsageFlags(sampler.binding, samplerGroup.stageFlags, usage);

VkImageView imageView = VK_NULL_HANDLE;
VkImageSubresourceRange const range = texture->getPrimaryViewRange();
if (any(texture->usage & TextureUsage::DEPTH_ATTACHMENT)
&& expectedType == VK_IMAGE_VIEW_TYPE_2D) {
// If the sampler is part of a mipmapped depth texture, where one of the level
// *can* be an attachment, then the sampler for this texture has the same view
// properties as a view for an attachment. Therefore, we can use
// getAttachmentView to get a corresponding VkImageView.
imageView = texture->getAttachmentView(range);
} else {
imageView = texture->getViewForType(range, expectedType);
}

samplerInfo[sampler.binding] = {
.sampler = vksampler,
.imageView = texture->getPrimaryImageView(),
.imageView = imageView,
.imageLayout = ImgUtil::getVkLayout(texture->getPrimaryImageLayout())
};
samplerTextures[sampler.binding] = texture;
Expand Down
17 changes: 8 additions & 9 deletions filament/backend/src/vulkan/VulkanImageUtility.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ getVkTransition(const VulkanLayoutTransition& transition) {

switch (transition.oldLayout) {
case VulkanLayout::UNDEFINED:
srcAccessMask = 0;
srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT;
srcAccessMask = VK_ACCESS_MEMORY_READ_BIT;
srcStage = VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT;
break;
case VulkanLayout::COLOR_ATTACHMENT:
srcAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT
Expand All @@ -93,8 +93,7 @@ getVkTransition(const VulkanLayoutTransition& transition) {
srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT;
break;
case VulkanLayout::DEPTH_ATTACHMENT:
srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT
| VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
srcStage = VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT;
break;
case VulkanLayout::DEPTH_SAMPLER:
Expand Down Expand Up @@ -125,7 +124,7 @@ getVkTransition(const VulkanLayoutTransition& transition) {
break;
case VulkanLayout::READ_ONLY:
dstAccessMask = VK_ACCESS_SHADER_READ_BIT;
dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT;
dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_VERTEX_SHADER_BIT;
break;
case VulkanLayout::TRANSFER_SRC:
dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT;
Expand All @@ -141,10 +140,10 @@ getVkTransition(const VulkanLayoutTransition& transition) {
dstStage = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT;
break;
case VulkanLayout::DEPTH_SAMPLER:
dstAccessMask
= VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT
| VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT;
dstAccessMask =
VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT |
VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT;
break;
case VulkanLayout::PRESENT:
case VulkanLayout::COLOR_ATTACHMENT_RESOLVE:
Expand Down
Loading

0 comments on commit df50f4c

Please sign in to comment.