Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vk: layout transition clean up #7217

Merged
merged 3 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
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
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
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
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
Loading