From 518509aa0e9c3acbedb92dcdabc0dfd34e889002 Mon Sep 17 00:00:00 2001 From: catbot Date: Tue, 16 Jun 2026 18:30:44 +0000 Subject: [PATCH] perf(image): batch final mip-chain layout transition (#70) The mip-generation upload path issued N+1 layout-transition barriers per chain. The final blit's destination level is never read again, so its dedicated TRANSFER_DST -> TRANSFER_SRC barrier was redundant: the level was transitioned to SRC only to keep the final SRC -> consumer transition uniform. Skip that last per-level barrier and fold the final level into the closing transition, which now batches two VkImageMemoryBarrier entries ([0, mipLevels-1) from TRANSFER_SRC, the last level from TRANSFER_DST) into a single vkCmdPipelineBarrier. One fewer barrier call per chain; the N-1 interleaved per-level barriers stay one-at-a-time since each is read by the next blit. Applied to both the host-upload and GPU-decompress Update paths. Adds the MipChainBarrierBatch regression test driving the pure barrier builder (no GPU device needed). Co-Authored-By: Claude Opus 4.8 --- interfaces/Crafter.Graphics-ImageVulkan.cppm | 57 ++++++++- project.cpp | 27 ++++ tests/MipChainBarrierBatch/main.cpp | 122 +++++++++++++++++++ 3 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 tests/MipChainBarrierBatch/main.cpp diff --git a/interfaces/Crafter.Graphics-ImageVulkan.cppm b/interfaces/Crafter.Graphics-ImageVulkan.cppm index 9ba72a6..6630e2c 100644 --- a/interfaces/Crafter.Graphics-ImageVulkan.cppm +++ b/interfaces/Crafter.Graphics-ImageVulkan.cppm @@ -31,6 +31,41 @@ import :Decompress; import :VulkanBuffer; export namespace Crafter { + // Builds the batched final layout transition for a freshly-generated mip + // chain (mipLevels >= 2). After the blit loop, levels [0, mipLevels-1) sit + // in TRANSFER_SRC_OPTIMAL — each was read as a blit source — while the last + // level is still in TRANSFER_DST_OPTIMAL: it was written by the final blit + // and is never read, so it never needed a DST->SRC barrier of its own. + // Both groups move to the consumer `layout` in one vkCmdPipelineBarrier + // (two VkImageMemoryBarrier entries sharing src=TRANSFER / dst=consumer + // stage), shaving one barrier call off the previous N+1-per-chain count. + // The interleaved per-level barriers stay one-at-a-time — each is mandated + // because the next blit reads the level it transitions. Writes the entries + // into `out` and returns the count (always 2). + std::uint32_t BuildMipChainFinalBarriers(VkImage image, VkImageLayout layout, std::uint32_t mipLevels, std::array& out) { + auto fill = [&](VkImageMemoryBarrier& b, VkImageLayout oldLayout, std::uint32_t baseMip, std::uint32_t count, VkAccessFlags srcAccess) { + b = {}; + b.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; + b.oldLayout = oldLayout; + b.newLayout = layout; + b.image = image; + b.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + b.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + b.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + b.subresourceRange.baseMipLevel = baseMip; + b.subresourceRange.levelCount = count; + b.subresourceRange.baseArrayLayer = 0; + b.subresourceRange.layerCount = 1; + b.srcAccessMask = srcAccess; + b.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; + }; + // Levels [0, mipLevels-1): blit sources, currently TRANSFER_SRC. + fill(out[0], VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, 0, mipLevels - 1u, VK_ACCESS_TRANSFER_READ_BIT); + // Final level: the last blit's destination, still TRANSFER_DST. + fill(out[1], VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, mipLevels - 1u, 1u, VK_ACCESS_TRANSFER_WRITE_BIT); + return 2; + } + template class ImageVulkan { public: @@ -160,10 +195,17 @@ export namespace Crafter { blit.dstOffsets[1] = { (int32_t)mipWidth, (int32_t)mipHeight, 1 }; vkCmdBlitImage(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &blit, VK_FILTER_LINEAR); - TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, i, 1); + // The final blit's destination is never read again, so it + // skips the DST->SRC barrier and is taken straight to the + // consumer layout by the batched final transition below. + if (i + 1 < mipLevels) { + TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, i, 1); + } } - TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, layout, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, VK_ACCESS_TRANSFER_READ_BIT, VK_ACCESS_SHADER_READ_BIT, 0, mipLevels); + std::array finalBarriers; + std::uint32_t finalCount = BuildMipChainFinalBarriers(image, layout, mipLevels, finalBarriers); + vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, 0, 0, nullptr, 0, nullptr, finalCount, finalBarriers.data()); } else { TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, layout, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT, 0, mipLevels); } @@ -297,9 +339,16 @@ export namespace Crafter { blit.dstOffsets[0] = { 0, 0, 0 }; blit.dstOffsets[1] = { (int32_t)mipWidth, (int32_t)mipHeight, 1 }; vkCmdBlitImage(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &blit, VK_FILTER_LINEAR); - TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, i, 1); + // The final blit's destination is never read again, so it + // skips the DST->SRC barrier and is taken straight to the + // consumer layout by the batched final transition below. + if (i + 1 < mipLevels) { + TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, i, 1); + } } - TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, layout, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, VK_ACCESS_TRANSFER_READ_BIT, VK_ACCESS_SHADER_READ_BIT, 0, mipLevels); + std::array finalBarriers; + std::uint32_t finalCount = BuildMipChainFinalBarriers(image, layout, mipLevels, finalBarriers); + vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, 0, 0, nullptr, 0, nullptr, finalCount, finalBarriers.data()); } else { TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, layout, VK_PIPELINE_STAGE_TRANSFER_BIT, consumerStage, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT, 0, mipLevels); } diff --git a/project.cpp b/project.cpp index d928210..ad47601 100644 --- a/project.cpp +++ b/project.cpp @@ -415,6 +415,33 @@ extern "C" Configuration CrafterBuildProject(std::span a mc.GetInterfacesAndImplementations(ifaces, memImpls); cfg.tests.push_back(std::move(memTest)); + // Issue #70: ImageVulkan's mip-chain upload folds the never-read final + // blit destination into a single batched final-transition barrier + // instead of giving it a dedicated DST->SRC barrier. The barrier set is + // built by BuildMipChainFinalBarriers, pure CPU logic over mip count and + // layout, so this test drives it directly — no GPU device at runtime. + Test mipBarrierTest; + Configuration& mbc = mipBarrierTest.config; + mbc.path = cfg.path; + mbc.name = "MipChainBarrierBatch"; + mbc.outputName = "MipChainBarrierBatch"; + mbc.type = ConfigurationType::Executable; + mbc.target = cfg.target; + mbc.march = cfg.march; + mbc.mtune = cfg.mtune; + mbc.debug = cfg.debug; + mbc.sysroot = cfg.sysroot; + mbc.dependencies = cfg.dependencies; + mbc.externalDependencies = cfg.externalDependencies; + mbc.compileFlags = cfg.compileFlags; + mbc.linkFlags = cfg.linkFlags; + mbc.defines = cfg.defines; + mbc.cFiles = cfg.cFiles; + std::vector mipBarrierImpls(impls.begin(), impls.end()); + mipBarrierImpls.emplace_back("tests/MipChainBarrierBatch/main"); + mbc.GetInterfacesAndImplementations(ifaces, mipBarrierImpls); + cfg.tests.push_back(std::move(mipBarrierTest)); + // Issue #47: the fused UI uber-kernel (shaders/ui-fused.comp.glsl) and // its C++ push-constant mirror UIFusedHeader. Compiles the real shader // with glslang, validates with spirv-val, and pins the push-constant diff --git a/tests/MipChainBarrierBatch/main.cpp b/tests/MipChainBarrierBatch/main.cpp new file mode 100644 index 0000000..4b12e9e --- /dev/null +++ b/tests/MipChainBarrierBatch/main.cpp @@ -0,0 +1,122 @@ +/* +Crafter®.Graphics +Copyright (C) 2026 Catcrafts® +catcrafts.net + +This library is free software; you can redistribute it and/or +modify it under the terms of the GNU Lesser General Public +License version 3.0 as published by the Free Software Foundation; + +This library is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public +License along with this library; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +*/ + +// Regression test for issue #70: ImageVulkan's mip-chain upload used to issue +// N+1 layout-transition barriers per chain, one vkCmdPipelineBarrier per call. +// The final blit's destination level is never read again, so its dedicated +// TRANSFER_DST -> TRANSFER_SRC barrier is redundant; the chain now folds that +// last level straight into the final transition, which batches two +// VkImageMemoryBarrier entries into a single vkCmdPipelineBarrier: +// - levels [0, mipLevels-1): TRANSFER_SRC_OPTIMAL -> consumer layout +// - level mipLevels-1 : TRANSFER_DST_OPTIMAL -> consumer layout +// BuildMipChainFinalBarriers is the pure builder for that batched barrier, so +// this test drives it directly with a sentinel image handle — no GPU device +// needed at runtime, mirroring MemoryTypeFallback / UploadStrategy. + +#include +#include "vulkan/vulkan.h" + +import Crafter.Graphics; +import std; +using namespace Crafter; + +namespace { + +int failures = 0; + +void Check(bool ok, std::string_view what) { + std::println("{} {}", ok ? "PASS" : "FAIL", what); + if (!ok) ++failures; +} + +// A non-null sentinel so we can assert the builder propagates the image handle +// into every barrier without owning a real VkImage. +VkImage SentinelImage() { + return reinterpret_cast(static_cast(0xC0FFEE)); +} + +// Common invariants every emitted barrier must satisfy regardless of which +// level group it covers. +void CheckCommon(const VkImageMemoryBarrier& b, VkImageLayout layout, std::string_view tag) { + Check(b.sType == VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, std::format("{}: sType", tag)); + Check(b.image == SentinelImage(), std::format("{}: image handle propagated", tag)); + Check(b.newLayout == layout, std::format("{}: newLayout is the consumer layout", tag)); + Check(b.dstAccessMask == VK_ACCESS_SHADER_READ_BIT, std::format("{}: dst access is shader read", tag)); + Check(b.srcQueueFamilyIndex == VK_QUEUE_FAMILY_IGNORED, std::format("{}: src queue ignored", tag)); + Check(b.dstQueueFamilyIndex == VK_QUEUE_FAMILY_IGNORED, std::format("{}: dst queue ignored", tag)); + Check(b.subresourceRange.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT, std::format("{}: color aspect", tag)); + Check(b.subresourceRange.baseArrayLayer == 0, std::format("{}: base array layer 0", tag)); + Check(b.subresourceRange.layerCount == 1, std::format("{}: single array layer", tag)); +} + +} // namespace + +int main() { + constexpr VkImageLayout LAYOUT = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + + for (std::uint32_t mipLevels : {2u, 3u, 5u, 12u}) { + std::array barriers; + std::uint32_t count = BuildMipChainFinalBarriers(SentinelImage(), LAYOUT, mipLevels, barriers); + + // The whole point: the final transition is a single batched call of two + // barriers, never one-per-level. + Check(count == 2, std::format("mip={}: batched into exactly two barriers", mipLevels)); + + const VkImageMemoryBarrier& src = barriers[0]; + const VkImageMemoryBarrier& dst = barriers[1]; + CheckCommon(src, LAYOUT, std::format("mip={} src-group", mipLevels)); + CheckCommon(dst, LAYOUT, std::format("mip={} dst-group", mipLevels)); + + // Group 0: the blit sources, [0, mipLevels-1), coming from TRANSFER_SRC. + Check(src.oldLayout == VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + std::format("mip={}: source group transitions from TRANSFER_SRC", mipLevels)); + Check(src.srcAccessMask == VK_ACCESS_TRANSFER_READ_BIT, + std::format("mip={}: source group src access is transfer read", mipLevels)); + Check(src.subresourceRange.baseMipLevel == 0, + std::format("mip={}: source group starts at level 0", mipLevels)); + Check(src.subresourceRange.levelCount == mipLevels - 1, + std::format("mip={}: source group covers all but the last level", mipLevels)); + + // Group 1: the final blit's destination, still TRANSFER_DST, never read. + Check(dst.oldLayout == VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + std::format("mip={}: last level transitions from TRANSFER_DST", mipLevels)); + Check(dst.srcAccessMask == VK_ACCESS_TRANSFER_WRITE_BIT, + std::format("mip={}: last level src access is transfer write", mipLevels)); + Check(dst.subresourceRange.baseMipLevel == mipLevels - 1, + std::format("mip={}: last level starts at the final mip", mipLevels)); + Check(dst.subresourceRange.levelCount == 1, + std::format("mip={}: last level covers a single mip", mipLevels)); + + // The two groups must tile [0, mipLevels) with no gap and no overlap, or + // some level would be left in a transfer layout when the shader samples. + std::uint32_t covered = src.subresourceRange.levelCount + dst.subresourceRange.levelCount; + Check(covered == mipLevels, + std::format("mip={}: the two groups cover every level exactly once", mipLevels)); + Check(src.subresourceRange.baseMipLevel + src.subresourceRange.levelCount + == dst.subresourceRange.baseMipLevel, + std::format("mip={}: groups are contiguous (no gap/overlap)", mipLevels)); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +} -- 2.54.0