Skip to content

Commit

Permalink
vulkan: fix taa validations (#6903)
Browse files Browse the repository at this point in the history
- Validation failing due to incorrect layout wrt Blitter
   render pass.
 - Clear colors are also not needed for blitter renderpass.
 - SAMPLEABLE + DEPTH_ATTACHMENT needs to have the correct layout.
  • Loading branch information
poweifeng committed Jun 15, 2023
1 parent b8b1fad commit 432b5d0
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 26 deletions.
41 changes: 25 additions & 16 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ void VulkanBlitter::blitDepth(BlitArgs args) {
assert_invariant(src.texture && dst.texture);

if (src.texture->samples > 1 && dst.texture->samples == 1) {
blitSlowDepth(aspect, args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair,
blitSlowDepth(args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair,
args.dstRectPair);
return;
}
Expand Down Expand Up @@ -259,27 +259,30 @@ void VulkanBlitter::lazyInit() noexcept {
// 2. Begin render pass
// 3. Draw a big triangle
// 4. End render pass.
void VulkanBlitter::blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter,
const VkExtent2D srcExtent, VulkanAttachment src, VulkanAttachment dst,
const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) {
void VulkanBlitter::blitSlowDepth(VkFilter filter, const VkExtent2D srcExtent, VulkanAttachment src,
VulkanAttachment dst, const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) {
lazyInit();

const BlitterUniforms uniforms = {
BlitterUniforms const uniforms = {
.sampleCount = src.texture->samples,
.inverseSampleCount = 1.0f / float(src.texture->samples),
};
mParamsBuffer->loadFromCpu(&uniforms, 0, sizeof(uniforms));

VkImageAspectFlags const aspect = VK_IMAGE_ASPECT_DEPTH_BIT;

// We will transition the src into sampler layout and also keep it in sampler layout for
// consistency.
VulkanLayout const samplerLayout = VulkanLayout::DEPTH_SAMPLER;

// BEGIN RENDER PASS
// -----------------

VulkanLayout const layout = VulkanLayout::DEPTH_ATTACHMENT;

const VulkanFboCache::RenderPassKey rpkey = {
.initialColorLayoutMask = 0,
.initialDepthLayout = VulkanLayout::UNDEFINED,
.renderPassDepthLayout = layout,
.finalDepthLayout = layout,
.renderPassDepthLayout = samplerLayout,
.finalDepthLayout = samplerLayout,
.depthFormat = dst.getFormat(),
.clear = {},
.discardStart = TargetBufferFlags::DEPTH,
Expand All @@ -297,7 +300,7 @@ void VulkanBlitter::blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter,
.samples = rpkey.samples,
.color = {},
.resolve = {},
.depth = dst.getImageView(VK_IMAGE_ASPECT_DEPTH_BIT),
.depth = dst.getImageView(aspect),
};
const VkFramebuffer vkfb = mFramebufferCache.getFramebuffer(fbkey);

Expand All @@ -311,12 +314,18 @@ void VulkanBlitter::blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter,
renderPassInfo.renderArea.extent.width = dstRect[1].x - dstRect[0].x;
renderPassInfo.renderArea.extent.height = dstRect[1].y - dstRect[0].y;

// Even though we don't clear anything, we have to provide a clear value.
VkClearValue clearValues[1] = {};
renderPassInfo.clearValueCount = 1;
renderPassInfo.pClearValues = clearValues;

const VkCommandBuffer cmdbuffer = mCommands->get().cmdbuffer;

// We need to transition the source into a sampler since it'll be sampled in the shader.
const VkImageSubresourceRange srcRange = {
.aspectMask = aspect,
.baseMipLevel = src.level,
.levelCount = 1,
.baseArrayLayer = src.layer,
.layerCount = 1,
};
src.texture->transitionLayout(cmdbuffer, srcRange, samplerLayout);

vkCmdBeginRenderPass(cmdbuffer, &renderPassInfo, VK_SUBPASS_CONTENTS_INLINE);

VkViewport viewport = {
Expand Down Expand Up @@ -378,7 +387,7 @@ void VulkanBlitter::blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter,
samplers[0] = {
.sampler = vksampler,
.imageView = src.getImageView(VK_IMAGE_ASPECT_DEPTH_BIT),
.imageLayout = ImgUtil::getVkLayout(VulkanLayout::DEPTH_ATTACHMENT),
.imageLayout = ImgUtil::getVkLayout(samplerLayout),
};

mPipelineCache.bindSamplers(samplers,
Expand Down
5 changes: 2 additions & 3 deletions filament/backend/src/vulkan/VulkanBlitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ class VulkanBlitter {
private:
void lazyInit() noexcept;

void blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter, const VkExtent2D srcExtent,
VulkanAttachment src, VulkanAttachment dst, const VkOffset3D srcRect[2],
const VkOffset3D dstRect[2]);
void blitSlowDepth(VkFilter filter, const VkExtent2D srcExtent, VulkanAttachment src,
VulkanAttachment dst, const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]);

VulkanBuffer* mTriangleBuffer = nullptr;
VulkanBuffer* mParamsBuffer = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
VkClearValue clearValues[
MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT + MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT +
1] = {};
if (params.flags.clear != TargetBufferFlags::NONE) {
if (clearVal != TargetBufferFlags::NONE) {

// NOTE: clearValues must be populated in the same order as the attachments array in
// VulkanFboCache::getFramebuffer. Values must be provided regardless of whether Vulkan is
Expand Down
5 changes: 2 additions & 3 deletions filament/backend/src/vulkan/VulkanHandles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ VulkanProgram::VulkanProgram(VkDevice device, const Program& builder) noexcept :
// Make a copy of the binding map
samplerGroupInfo = builder.getSamplerGroupInfo();
if constexpr (FILAMENT_VULKAN_VERBOSE) {
utils::slog.d << "Created VulkanProgram " << builder
<< ", shaders = (" << bundle.vertex << ", " << bundle.fragment << ")"
<< utils::io::endl;
utils::slog.d << "Created VulkanProgram " << builder << ", shaders = (" << bundle.vertex
<< ", " << bundle.fragment << ")" << utils::io::endl;
}
}

Expand Down
20 changes: 19 additions & 1 deletion filament/backend/src/vulkan/VulkanImageUtility.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ class VulkanImageUtility {

inline static VulkanLayout getDefaultLayout(TextureUsage usage) {
if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) {
return VulkanLayout::DEPTH_ATTACHMENT;
if (any(usage & TextureUsage::SAMPLEABLE)) {
return VulkanLayout::DEPTH_SAMPLER;
} else {
return VulkanLayout::DEPTH_ATTACHMENT;
}
}

if (any(usage & TextureUsage::COLOR_ATTACHMENT)) {
Expand All @@ -75,6 +79,20 @@ class VulkanImageUtility {
return VulkanLayout::READ_ONLY;
}

inline static VulkanLayout getDefaultLayout(VkImageUsageFlags vkusage) {
TextureUsage usage {};
if (vkusage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
usage = usage | TextureUsage::DEPTH_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
usage = usage | TextureUsage::COLOR_ATTACHMENT;
}
if (vkusage & VK_IMAGE_USAGE_SAMPLED_BIT) {
usage = usage | TextureUsage::SAMPLEABLE;
}
return getDefaultLayout(usage);
}

static VkImageLayout getVkLayout(VulkanLayout layout);

static void transitionLayout(VkCommandBuffer cmdbuffer, VulkanLayoutTransition transition);
Expand Down
8 changes: 6 additions & 2 deletions filament/backend/src/vulkan/VulkanTexture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,12 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
<< "handle = " << utils::io::hex << mTextureImage << utils::io::dec << ", "
<< "extent = " << w << "x" << h << "x"<< depth << ", "
<< "mipLevels = " << int(levels) << ", "
<< "TextureUsage = " << static_cast<int>(usage) << ", "
<< "usage = " << imageInfo.usage << ", "
<< "samples = " << imageInfo.samples << ", "
<< "type = " << imageInfo.imageType << ", "
<< "flags = " << imageInfo.flags << ", "
<< "target = " << static_cast<int>(target) <<", "
<< "format = " << mVkFormat << utils::io::endl;
}
ASSERT_POSTCONDITION(!error, "Unable to create image.");
Expand Down Expand Up @@ -199,11 +201,13 @@ VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice,
// Transition the layout of each image slice that might be used as a render target.
// We do not transition images that are merely SAMPLEABLE, this is deferred until upload time
// because we do not know how many layers and levels will actually be used.
if (any(usage & (TextureUsage::COLOR_ATTACHMENT | TextureUsage::DEPTH_ATTACHMENT))) {
if (imageInfo.usage
& (VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT
| VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
const uint32_t layers = mPrimaryViewRange.layerCount;
VkImageSubresourceRange range = { getImageAspect(), 0, levels, 0, layers };
VkCommandBuffer cmdbuf = mCommands->get().cmdbuffer;
transitionLayout(cmdbuf, range, ImgUtil::getDefaultLayout(usage));
transitionLayout(cmdbuf, range, ImgUtil::getDefaultLayout(imageInfo.usage));
}
}

Expand Down

0 comments on commit 432b5d0

Please sign in to comment.