From d697d42c4a608ff97bc7c7e769417fbfc29a9eed Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Mon, 29 Apr 2024 11:32:04 -0700 Subject: [PATCH] vk: finer-grain barrier at renderpass end (#7808) - At the end of a renderpass we use a more fine-grained barrier for each of the attachments in the render target - Make sure that buffer update are barrier'd from previous reads - Remove previous Mali workaround barriers. Seems to be fine without them on pixels + Mali. --- filament/backend/src/vulkan/VulkanBuffer.cpp | 60 ++++++--- filament/backend/src/vulkan/VulkanBuffer.h | 3 +- filament/backend/src/vulkan/VulkanDriver.cpp | 115 +++++++++++------- filament/backend/src/vulkan/VulkanDriver.h | 21 ++++ .../backend/src/vulkan/VulkanFboCache.cpp | 10 +- filament/backend/src/vulkan/VulkanFboCache.h | 17 +-- filament/backend/src/vulkan/VulkanHandles.cpp | 12 +- filament/backend/src/vulkan/VulkanHandles.h | 9 +- 8 files changed, 165 insertions(+), 82 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBuffer.cpp b/filament/backend/src/vulkan/VulkanBuffer.cpp index 1b5f59eafd..bc09c7f521 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.cpp +++ b/filament/backend/src/vulkan/VulkanBuffer.cpp @@ -27,8 +27,8 @@ VulkanBuffer::VulkanBuffer(VmaAllocator allocator, VulkanStagePool& stagePool, VkBufferUsageFlags usage, uint32_t numBytes) : mAllocator(allocator), mStagePool(stagePool), - mUsage(usage) { - + mUsage(usage), + mUpdatedBytes(0) { // for now make sure that only 1 bit is set in usage // (because loadFromCpu() assumes that somewhat) assert_invariant(usage && !(usage & (usage - 1))); @@ -49,7 +49,7 @@ VulkanBuffer::~VulkanBuffer() { } void VulkanBuffer::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint32_t byteOffset, - uint32_t numBytes) const { + uint32_t numBytes) { assert_invariant(byteOffset == 0); VulkanStage const* stage = mStagePool.acquireStage(numBytes); void* mapped; @@ -58,15 +58,47 @@ void VulkanBuffer::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint vmaUnmapMemory(mAllocator, stage->memory); vmaFlushAllocation(mAllocator, stage->memory, byteOffset, numBytes); + // If there was a previous update, then we need to make sure the following write is properly + // synced with the previous read. + if (mUpdatedBytes > 0) { + VkAccessFlags srcAccess = 0; + VkPipelineStageFlags srcStage = 0; + if (mUsage & VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT) { + srcAccess = VK_ACCESS_SHADER_READ_BIT; + srcStage = VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; + } else if (mUsage & VK_BUFFER_USAGE_VERTEX_BUFFER_BIT) { + srcAccess = VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT; + srcStage = VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; + } else if (mUsage & VK_BUFFER_USAGE_INDEX_BUFFER_BIT) { + srcAccess = VK_ACCESS_INDEX_READ_BIT; + srcStage = VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; + } + + VkBufferMemoryBarrier barrier{ + .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, + .srcAccessMask = srcAccess, + .dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .buffer = mGpuBuffer, + .size = mUpdatedBytes, + }; + vkCmdPipelineBarrier(cmdbuf, srcStage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 1, + &barrier, 0, nullptr); + } + VkBufferCopy region{ .size = numBytes }; vkCmdCopyBuffer(cmdbuf, stage->buffer, mGpuBuffer, 1, ®ion); + mUpdatedBytes = numBytes; + // Firstly, ensure that the copy finishes before the next draw call. // Secondly, in case the user decides to upload another chunk (without ever using the first one) // we need to ensure that this upload completes first (hence // dstStageMask=VK_PIPELINE_STAGE_TRANSFER_BIT). VkAccessFlags dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; VkPipelineStageFlags dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; + if (mUsage & VK_BUFFER_USAGE_VERTEX_BUFFER_BIT) { dstAccessMask |= VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT; dstStageMask |= VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; @@ -75,26 +107,24 @@ void VulkanBuffer::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint dstStageMask |= VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; } else if (mUsage & VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT) { dstAccessMask |= VK_ACCESS_UNIFORM_READ_BIT; - // NOTE: ideally dstStageMask would include VERTEX_SHADER_BIT | FRAGMENT_SHADER_BIT, but - // this seems to be insufficient on Mali devices. To work around this we are using a more - // aggressive ALL_GRAPHICS_BIT barrier. - dstStageMask |= VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT; + dstStageMask |= + (VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT); } else if (mUsage & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT) { // TODO: implement me } VkBufferMemoryBarrier barrier{ - .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, - .srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, - .dstAccessMask = dstAccessMask, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .buffer = mGpuBuffer, - .size = VK_WHOLE_SIZE, + .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, + .srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, + .dstAccessMask = dstAccessMask, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .buffer = mGpuBuffer, + .size = VK_WHOLE_SIZE, }; vkCmdPipelineBarrier(cmdbuf, VK_PIPELINE_STAGE_TRANSFER_BIT, dstStageMask, 0, 0, nullptr, 1, - &barrier, 0, nullptr); + &barrier, 0, nullptr); } } // namespace filament::backend diff --git a/filament/backend/src/vulkan/VulkanBuffer.h b/filament/backend/src/vulkan/VulkanBuffer.h index db24edf394..e496f930db 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.h +++ b/filament/backend/src/vulkan/VulkanBuffer.h @@ -30,7 +30,7 @@ public: uint32_t numBytes); ~VulkanBuffer(); void loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint32_t byteOffset, - uint32_t numBytes) const; + uint32_t numBytes); VkBuffer getGpuBuffer() const { return mGpuBuffer; } @@ -42,6 +42,7 @@ private: VmaAllocation mGpuMemory = VK_NULL_HANDLE; VkBuffer mGpuBuffer = VK_NULL_HANDLE; VkBufferUsageFlags mUsage = {}; + uint32_t mUpdatedBytes = 0; }; } // namespace filament::backend diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 07df284726..49e6f581c9 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1236,12 +1236,7 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP } } - 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; + VulkanLayout currentDepthLayout = depth.getLayout(); TargetBufferFlags clearVal = params.flags.clear; TargetBufferFlags discardEndVal = params.flags.discardEnd; @@ -1250,16 +1245,20 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP discardEndVal &= ~TargetBufferFlags::DEPTH; clearVal &= ~TargetBufferFlags::DEPTH; } - auto const attachmentSubresourceRange = depth.getSubresourceRange(); - depth.texture->setLayout(attachmentSubresourceRange, VulkanLayout::DEPTH_ATTACHMENT); + // If the depth attachment texture was previously sampled, then we need to manually + // transition it to an attachment. This is necessary to also set up a barrier between the + // previous read and the potentially coming write. + if (currentDepthLayout == VulkanLayout::DEPTH_SAMPLER) { + depth.texture->transitionLayout(cmdbuffer, depth.getSubresourceRange(), + VulkanLayout::DEPTH_ATTACHMENT); + currentDepthLayout = VulkanLayout::DEPTH_ATTACHMENT; + } } // Create the VkRenderPass or fetch it from cache. VulkanFboCache::RenderPassKey rpkey = { .initialColorLayoutMask = 0, .initialDepthLayout = currentDepthLayout, - .renderPassDepthLayout = renderPassDepthLayout, - .finalDepthLayout = finalDepthLayout, .depthFormat = depth.getFormat(), .clear = clearVal, .discardStart = discardStart, @@ -1296,19 +1295,30 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP .layers = 1, .samples = rpkey.samples, }; + auto& renderPassAttachments = mRenderPassFboInfo.attachments; for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { if (!rt->getColor(i).texture) { fbkey.color[i] = VK_NULL_HANDLE; fbkey.resolve[i] = VK_NULL_HANDLE; } else if (fbkey.samples == 1) { - fbkey.color[i] = rt->getColor(i).getImageView(); + auto& colorAttachment = rt->getColor(i); + renderPassAttachments.insert(colorAttachment); + fbkey.color[i] = colorAttachment.getImageView(); fbkey.resolve[i] = VK_NULL_HANDLE; assert_invariant(fbkey.color[i]); } else { - fbkey.color[i] = rt->getMsaaColor(i).getImageView(); - VulkanTexture* texture = (VulkanTexture*) rt->getColor(i).texture; + auto& msaaColorAttachment = rt->getMsaaColor(i); + renderPassAttachments.insert(msaaColorAttachment); + + auto& colorAttachment = rt->getColor(i); + fbkey.color[i] = msaaColorAttachment.getImageView(); + + VulkanTexture* texture = colorAttachment.texture; if (texture->samples == 1) { - fbkey.resolve[i] = rt->getColor(i).getImageView(); + mRenderPassFboInfo.hasColorResolve = true; + + renderPassAttachments.insert(colorAttachment); + fbkey.resolve[i] = colorAttachment.getImageView(); assert_invariant(fbkey.resolve[i]); } assert_invariant(fbkey.color[i]); @@ -1317,6 +1327,10 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP if (depth.texture) { fbkey.depth = depth.getImageView(); assert_invariant(fbkey.depth); + renderPassAttachments.insert(depth); + + UTILS_UNUSED_IN_RELEASE bool const depthDiscardEnd = + any(rpkey.discardEnd & TargetBufferFlags::DEPTH); // Vulkan 1.1 does not support multisampled depth resolve, so let's check here // and assert if this is requested. (c.f. isAutoDepthResolveSupported) @@ -1325,7 +1339,7 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP // - If the RT is MS then all SS attachments are auto resolved if not discarded. assert_invariant(!(rt->getSamples() > 1 && rt->getDepth().texture->samples == 1 && - !any(rpkey.discardEnd & TargetBufferFlags::DEPTH))); + !depthDiscardEnd)); } VkFramebuffer vkfb = mFramebufferCache.getFramebuffer(fbkey); @@ -1338,16 +1352,10 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP } #endif - // The current command buffer now owns a reference to the render target and its attachments. - // Note that we must acquire parent textures, not sidecars. + // The current command buffer now has references to the render target and its attachments. commands.acquire(rt); - if (depth.texture) { - commands.acquire((VulkanTexture*) depth.texture); - } - for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { - if (rt->getColor(i).texture) { - commands.acquire(rt->getColor(i).texture); - } + for (auto const& attachment: renderPassAttachments) { + commands.acquire(attachment.texture); } // Populate the structures required for vkCmdBeginRenderPass. @@ -1433,27 +1441,51 @@ void VulkanDriver::endRenderPass(int) { // issue several of them when considering MRT. This would be very complex to set up and would // require more state tracking, so we've chosen to use a memory barrier for simplicity and // correctness. - - // 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, - }; - VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - if (rt->hasDepth()) { - barrier.srcAccessMask |= VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - srcStageMask |= VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + for (auto const& attachment: mRenderPassFboInfo.attachments) { + bool const isDepth = attachment.isDepth(); + VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; + + // This is a workaround around a validation issue (might not be an actual driver issue). + if (mRenderPassFboInfo.hasColorResolve && !isDepth) { + srcStageMask = VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT; + } + + VkPipelineStageFlags dstStageMask = + VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; + VkAccessFlags srcAccess = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; + VkAccessFlags dstAccess = VK_ACCESS_SHADER_READ_BIT; + VulkanLayout layout = VulkanFboCache::FINAL_COLOR_ATTACHMENT_LAYOUT; + if (isDepth) { + srcAccess = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + dstAccess = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; + srcStageMask = VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + dstStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT; + layout = VulkanFboCache::FINAL_DEPTH_ATTACHMENT_LAYOUT; + } + + auto const vkLayout = imgutil::getVkLayout(layout); + auto const& range = attachment.getSubresourceRange(); + VkImageMemoryBarrier barrier = { + .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, + .pNext = nullptr, + .srcAccessMask = srcAccess, + .dstAccessMask = dstAccess, + .oldLayout = vkLayout, + .newLayout = vkLayout, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .image = attachment.getImage(), + .subresourceRange = range, + }; + + attachment.texture->setLayout(range, layout); + vkCmdPipelineBarrier(cmdbuffer, srcStageMask, dstStageMask, 0, 0, nullptr, 0, nullptr, + 1, &barrier); } - vkCmdPipelineBarrier(cmdbuffer, srcStageMask, - VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT | // <== For Mali - VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, - 0, 1, &barrier, 0, nullptr, 0, nullptr); } + mRenderPassFboInfo.clear(); mDescriptorSetManager.clearState(); mCurrentRenderPass.renderTarget = nullptr; mCurrentRenderPass.renderPass = VK_NULL_HANDLE; @@ -1818,7 +1850,6 @@ void VulkanDriver::bindPipeline(PipelineState pipelineState) { } VkSampler const vksampler = mSamplerCache.getSampler(boundSampler->s); - #if FVK_ENABLED_DEBUG_SAMPLER_NAME VulkanDriver::DebugUtils::setName(VK_OBJECT_TYPE_SAMPLER, reinterpret_cast(vksampler), bindingToName[binding].c_str()); diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index fca5c45c5b..8de0ae4a26 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -42,6 +42,25 @@ namespace filament::backend { class VulkanPlatform; struct VulkanSamplerGroup; +// The maximum number of attachments for any renderpass (color + resolve + depth) +constexpr uint8_t MAX_RENDERTARGET_ATTACHMENT_TEXTURES = + MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT * 2 + 1; + +// We need to store information about a render pass to enable better barriers at the end of a +// renderpass. +struct RenderPassFboBundle { + using AttachmentArray = + CappedArray; + + AttachmentArray attachments; + bool hasColorResolve = false; + + void clear() { + attachments.clear(); + hasColorResolve = false; + } +}; + class VulkanDriver final : public DriverBase { public: static Driver* create(VulkanPlatform* platform, VulkanContext const& context, @@ -141,6 +160,8 @@ private: VulkanDescriptorSetManager::GetPipelineLayoutFunction mGetPipelineFunction; + RenderPassFboBundle mRenderPassFboInfo; + bool const mIsSRGBSwapChainSupported; }; diff --git a/filament/backend/src/vulkan/VulkanFboCache.cpp b/filament/backend/src/vulkan/VulkanFboCache.cpp index d8dc804fa0..c754fe57e7 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.cpp +++ b/filament/backend/src/vulkan/VulkanFboCache.cpp @@ -33,8 +33,6 @@ bool VulkanFboCache::RenderPassEq::operator()(const RenderPassKey& k1, const RenderPassKey& k2) const { if (k1.initialColorLayoutMask != k2.initialColorLayoutMask) return false; if (k1.initialDepthLayout != k2.initialDepthLayout) return false; - if (k1.renderPassDepthLayout != k2.renderPassDepthLayout) return false; - if (k1.finalDepthLayout != k2.finalDepthLayout) return false; for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { if (k1.colorFormat[i] != k2.colorFormat[i]) return false; } @@ -243,7 +241,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { .initialLayout = ((!discard && config.initialColorLayoutMask & (1 << i)) || clear) ? imgutil::getVkLayout(VulkanLayout::COLOR_ATTACHMENT) : imgutil::getVkLayout(VulkanLayout::UNDEFINED), - .finalLayout = imgutil::getVkLayout(VulkanLayout::COLOR_ATTACHMENT), + .finalLayout = imgutil::getVkLayout(FINAL_COLOR_ATTACHMENT_LAYOUT), }; } @@ -281,7 +279,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { .stencilLoadOp = kDontCare, .stencilStoreOp = kDisableStore, .initialLayout = imgutil::getVkLayout(VulkanLayout::COLOR_ATTACHMENT), - .finalLayout = imgutil::getVkLayout(VulkanLayout::COLOR_ATTACHMENT), + .finalLayout = imgutil::getVkLayout(FINAL_COLOR_ATTACHMENT_LAYOUT), }; } @@ -290,7 +288,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { const bool clear = any(config.clear & TargetBufferFlags::DEPTH); const bool discardStart = any(config.discardStart & TargetBufferFlags::DEPTH); const bool discardEnd = any(config.discardEnd & TargetBufferFlags::DEPTH); - depthAttachmentRef.layout = imgutil::getVkLayout(config.renderPassDepthLayout); + depthAttachmentRef.layout = imgutil::getVkLayout(VulkanLayout::DEPTH_ATTACHMENT); depthAttachmentRef.attachment = attachmentIndex; attachments[attachmentIndex++] = { .format = config.depthFormat, @@ -300,7 +298,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { .stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE, .stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE, .initialLayout = imgutil::getVkLayout(config.initialDepthLayout), - .finalLayout = imgutil::getVkLayout(config.finalDepthLayout), + .finalLayout = imgutil::getVkLayout(FINAL_DEPTH_ATTACHMENT_LAYOUT), }; } renderPassInfo.attachmentCount = attachmentIndex; diff --git a/filament/backend/src/vulkan/VulkanFboCache.h b/filament/backend/src/vulkan/VulkanFboCache.h index cefdae2995..2f4b1a9101 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.h +++ b/filament/backend/src/vulkan/VulkanFboCache.h @@ -35,24 +35,25 @@ namespace filament::backend { // class VulkanFboCache { public: + constexpr static VulkanLayout FINAL_COLOR_ATTACHMENT_LAYOUT = VulkanLayout::COLOR_ATTACHMENT; + constexpr static VulkanLayout FINAL_RESOLVE_ATTACHMENT_LAYOUT = VulkanLayout::COLOR_ATTACHMENT; + constexpr static VulkanLayout FINAL_DEPTH_ATTACHMENT_LAYOUT = VulkanLayout::DEPTH_ATTACHMENT; + // RenderPassKey is a small POD representing the immutable state that is used to construct // a VkRenderPass. It is hashed and used as a lookup key. - // TODO: This struct can be reduced in size by using a subset of formats instead of VkFormat - // and removing the "finalDepthLayout" field. struct alignas(8) RenderPassKey { // For each target, we need to know three image layouts: the layout BEFORE the pass, the // layout DURING the pass, and the layout AFTER the pass. Here are the rules: // - For depth, we explicitly specify all three layouts. // - Color targets have their initial image layout specified with a bitmask. // - For each color target, the pre-existing layout is either UNDEFINED (0) or GENERAL (1). - // - The render pass and final images layout for color buffers is always GENERAL. + // - The render pass and final images layout for color buffers is always + // VulkanLayout::COLOR_ATTACHMENT. uint8_t initialColorLayoutMask; // Note that if VulkanLayout grows beyond 16, we'd need to up this. - VulkanLayout initialDepthLayout : 4; - VulkanLayout renderPassDepthLayout : 4; - VulkanLayout finalDepthLayout : 4; - uint8_t padding0 : 4; + VulkanLayout initialDepthLayout : 8; + uint8_t padding0; uint8_t padding1; VkFormat colorFormat[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT]; // 32 bytes @@ -63,7 +64,7 @@ public: uint8_t samples; // 1 byte uint8_t needsResolveMask; // 1 byte uint8_t subpassMask; // 1 byte - bool padding2; // 1 byte + uint8_t padding2; // 1 byte }; struct RenderPassVal { VkRenderPass handle; diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 5c951ed5b2..0dd11ce8d1 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -285,13 +285,13 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica // Constrain the sample count according to both kinds of sample count masks obtained from // VkPhysicalDeviceProperties. This is consistent with the VulkanTexture constructor. - const auto& limits = context.getPhysicalDeviceLimits(); + auto const& limits = context.getPhysicalDeviceLimits(); mSamples = samples = reduceSampleCount(samples, limits.framebufferDepthSampleCounts & limits.framebufferColorSampleCounts); // Create sidecar MSAA textures for color attachments if they don't already exist. for (int index = 0; index < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; index++) { - const VulkanAttachment& spec = color[index]; + VulkanAttachment const& spec = color[index]; VulkanTexture* texture = (VulkanTexture*) spec.texture; if (texture && texture->samples == 1) { auto msTexture = texture->getSidecar(); @@ -354,19 +354,19 @@ VkExtent2D VulkanRenderTarget::getExtent() const { return {width, height}; } -VulkanAttachment VulkanRenderTarget::getColor(int target) const { +VulkanAttachment& VulkanRenderTarget::getColor(int target) { return mColor[target]; } -VulkanAttachment VulkanRenderTarget::getMsaaColor(int target) const { +VulkanAttachment& VulkanRenderTarget::getMsaaColor(int target) { return mMsaaAttachments[target]; } -VulkanAttachment VulkanRenderTarget::getDepth() const { +VulkanAttachment& VulkanRenderTarget::getDepth() { return mDepth; } -VulkanAttachment VulkanRenderTarget::getMsaaDepth() const { +VulkanAttachment& VulkanRenderTarget::getMsaaDepth() { return mMsaaDepthAttachment; } diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index 111e1299a3..bb20097a47 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -274,10 +274,11 @@ struct VulkanRenderTarget : private HwRenderTarget, VulkanResource { void transformClientRectToPlatform(VkRect2D* bounds) const; void transformClientRectToPlatform(VkViewport* bounds) const; VkExtent2D getExtent() const; - VulkanAttachment getColor(int target) const; - VulkanAttachment getMsaaColor(int target) const; - VulkanAttachment getDepth() const; - VulkanAttachment getMsaaDepth() const; + // We return references in the following methods to avoid a copy. + VulkanAttachment& getColor(int target); + VulkanAttachment& getMsaaColor(int target); + VulkanAttachment& getDepth(); + VulkanAttachment& getMsaaDepth(); uint8_t getColorTargetCount(const VulkanRenderPass& pass) const; uint8_t getSamples() const { return mSamples; } bool hasDepth() const { return mDepth.texture; }