perf(image): release static texture staging after upload (#114) #136

Merged
catbot merged 1 commit from claude/issue-114 into master 2026-06-17 21:41:07 +02:00
4 changed files with 314 additions and 3 deletions
Showing only changes of commit b4fa6e596e - Show all commits

perf(image): release static texture staging after upload (#114)

ImageVulkan kept its host-visible staging `buffer` (sized w*h, persistently
mapped) alive for the whole life of the image and Destroy() never freed it, so
static textures (e.g. Sponza albedo) pinned HOST_VISIBLE / small-BAR memory
forever.

Update now releases the staging via VulkanBuffer::DeferredClear() right after
recording the buffer→image copy — the same fence-keyed deletion queue
(#101/#102) the compressed Mesh path already uses — so it is freed once the
upload submit's frame has cleared. A new `streamed` flag (set by FontAtlas)
keeps the persistent map for images re-uploaded from a CPU-side staging buffer
every frame; its uploads go through UpdateRegion, which never releases. Destroy
now also frees any staging the image still owns, gated on a live handle so a
released static texture can't double-free.

Adds tests/ImageStagingRelease driving the real upload + readback on a headless
device: asserts the staging is enqueued (not pinned), the image reads back
byte-equal (released staging outlived the submit), the entry retires only after
framesInFlight frames, and a streamed image keeps then frees its staging.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
catbot 2026-06-17 19:40:35 +00:00

View file

@ -39,7 +39,11 @@ void FontAtlas::Initialize(GraphicsCommandBuffer cmd) {
VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,
// The atlas is sampled by the UI text *compute* shader, not an RT // The atlas is sampled by the UI text *compute* shader, not an RT
// pipeline — scope the upload barriers to the real consumer. // pipeline — scope the upload barriers to the real consumer.
VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT,
// Streamed: glyphs are rasterized into buffer.value on the CPU and
// re-uploaded every frame via UpdateRegion, so the persistent staging
// map must outlive the first upload rather than being released (#114).
/*streamed*/ true
); );
std::memset(image.buffer.value, 0, kAtlasSize * kAtlasSize); std::memset(image.buffer.value, 0, kAtlasSize * kAtlasSize);
#else #else

View file

@ -90,12 +90,24 @@ export namespace Crafter {
// consumer. Defaults to RT — the font atlas overrides it to COMPUTE, // consumer. Defaults to RT — the font atlas overrides it to COMPUTE,
// since UI text is rendered by a compute shader, not an RT pipeline. // since UI text is rendered by a compute shader, not an RT pipeline.
VkPipelineStageFlags consumerStage = VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR; VkPipelineStageFlags consumerStage = VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR;
// Whether this image is continuously re-uploaded from a persistent
// CPU-side staging map. False (the default) marks a static texture
// uploaded once — e.g. Sponza albedo: the first Update's buffer→image
// copy is the only reader of the host-visible staging `buffer`, so it
// is released to the fence-keyed deletion queue (#101/#102) right after
// the copy instead of pinning HOST_VISIBLE / small-BAR memory for the
// image's whole life (issue #114). True keeps `buffer` alive: a streamed
// image (the FontAtlas) re-fills buffer.value on the CPU and re-uploads
// every frame, so the persistent map must survive — its uploads go
// through UpdateRegion, which never releases the staging.
bool streamed = false;
void Create(std::uint16_t width, std::uint16_t height, std::uint8_t mipLevels, VkCommandBuffer cmd, VkFormat format, VkImageCreateFlags flags, VkImageLayout layout, VkPipelineStageFlags consumerStage = VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR) { void Create(std::uint16_t width, std::uint16_t height, std::uint8_t mipLevels, VkCommandBuffer cmd, VkFormat format, VkImageCreateFlags flags, VkImageLayout layout, VkPipelineStageFlags consumerStage = VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR, bool streamed = false) {
this->width = width; this->width = width;
this->height = height; this->height = height;
this->mipLevels = mipLevels; this->mipLevels = mipLevels;
this->consumerStage = consumerStage; this->consumerStage = consumerStage;
this->streamed = streamed;
buffer.Create( buffer.Create(
VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT,
@ -212,6 +224,7 @@ export namespace Crafter {
} else { } 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); 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);
} }
ReleaseStaging();
} }
// Upload only the sub-rectangle [x, x+w) × [y, y+h) of the staging // Upload only the sub-rectangle [x, x+w) × [y, y+h) of the staging
@ -365,15 +378,37 @@ export namespace Crafter {
} else { } 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); 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);
} }
ReleaseStaging();
} }
void Destroy() { void Destroy() {
vkDestroyImageView(Device::device, imageView, nullptr); vkDestroyImageView(Device::device, imageView, nullptr);
vkDestroyImage(Device::device, image, nullptr); vkDestroyImage(Device::device, image, nullptr);
vkFreeMemory(Device::device, imageMemory, nullptr); vkFreeMemory(Device::device, imageMemory, nullptr);
// Free any staging this image still owns: a streamed image's
// persistent `buffer`, or either staging buffer on an image
// destroyed before its first upload released them (issue #114).
// ReleaseStaging()/Update already null released handles, so these
// are no-ops then; Clear() is gated on a live handle so a static
// texture (whose `buffer` was deferred-cleared) can't double-free.
if (buffer.buffer != VK_NULL_HANDLE) buffer.Clear();
if (compressedStaging.buffer != VK_NULL_HANDLE) compressedStaging.Clear();
} }
private: private:
// Release the host-visible staging `buffer` after a one-shot upload,
// unless this is a streamed image (FontAtlas) whose persistent map must
// survive. The recorded buffer→image copy still references buffer.buffer,
// so route through the fence-keyed deletion queue (#101/#102) instead of
// an immediate Clear(): DeferredClear nulls the handle (so re-entry and
// Destroy treat the staging as already gone) while the allocation
// outlives this submit's frame. Issue #114.
void ReleaseStaging() {
if (!streamed) {
buffer.DeferredClear();
}
}
void TransitionImageLayout(VkCommandBuffer cmd, VkImage image, VkImageLayout oldLayout, VkImageLayout newLayout, VkPipelineStageFlags sourceStage, VkPipelineStageFlags destinationStage, VkAccessFlags srcAccessMask, VkAccessFlags dstAccessMask, std::uint32_t mipLevel, std::uint32_t count) { void TransitionImageLayout(VkCommandBuffer cmd, VkImage image, VkImageLayout oldLayout, VkImageLayout newLayout, VkPipelineStageFlags sourceStage, VkPipelineStageFlags destinationStage, VkAccessFlags srcAccessMask, VkAccessFlags dstAccessMask, std::uint32_t mipLevel, std::uint32_t count) {
VkImageMemoryBarrier barrier = {}; VkImageMemoryBarrier barrier = {};
barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER;

View file

@ -651,6 +651,40 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
msc.GetInterfacesAndImplementations(ifaces, meshStagingImpls); msc.GetInterfacesAndImplementations(ifaces, meshStagingImpls);
cfg.tests.push_back(std::move(meshStagingTest)); cfg.tests.push_back(std::move(meshStagingTest));
// Issue #114: a static ImageVulkan no longer pins its host-visible
// staging `buffer` for the image's life — Update releases it via
// DeferredClear() right after recording the buffer→image copy, so the
// fence-keyed deletion queue (#101/#102) frees it once that submit's
// frame clears, while a `streamed` image (the FontAtlas) keeps its
// persistent map. Drives the real upload path on a headless device (no
// swapchain/window — a buffer→image copy + readback only needs the queue
// + command pool) and asserts the staging is enqueued (not pinned), the
// image reads back byte-equal (the released staging outlived the submit),
// the entry retires only after framesInFlight frames, and a streamed
// image keeps + then frees its staging on Destroy. Shares the native
// build settings.
Test imageStagingTest;
Configuration& isc = imageStagingTest.config;
isc.path = cfg.path;
isc.name = "ImageStagingRelease";
isc.outputName = "ImageStagingRelease";
isc.type = ConfigurationType::Executable;
isc.target = cfg.target;
isc.march = cfg.march;
isc.mtune = cfg.mtune;
isc.debug = cfg.debug;
isc.sysroot = cfg.sysroot;
isc.dependencies = cfg.dependencies;
isc.externalDependencies = cfg.externalDependencies;
isc.compileFlags = cfg.compileFlags;
isc.linkFlags = cfg.linkFlags;
isc.defines = cfg.defines;
isc.cFiles = cfg.cFiles;
std::vector<fs::path> imageStagingImpls(impls.begin(), impls.end());
imageStagingImpls.emplace_back("tests/ImageStagingRelease/main");
isc.GetInterfacesAndImplementations(ifaces, imageStagingImpls);
cfg.tests.push_back(std::move(imageStagingTest));
// Issue #89: Device::PreferDirectDeviceWrite chooses the upload strategy // Issue #89: Device::PreferDirectDeviceWrite chooses the upload strategy
// for a CPU-written, GPU-read buffer — direct HOST_VISIBLE|DEVICE_LOCAL // for a CPU-written, GPU-read buffer — direct HOST_VISIBLE|DEVICE_LOCAL
// map+write on ReBAR/UMA vs. staged-into-pure-DEVICE_LOCAL on a small // map+write on ReBAR/UMA vs. staged-into-pure-DEVICE_LOCAL on a small

View file

@ -0,0 +1,238 @@
/*
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
*/
// Issue #114: a static ImageVulkan used to keep its host-visible staging
// `buffer` (sized w*h, persistently mapped) alive for the whole life of the
// image, pinning HOST_VISIBLE / small-BAR memory long after the one upload that
// reads it has retired — and Destroy() never freed it. Update now releases it
// via VulkanBuffer::DeferredClear() right after recording the buffer→image
// copy, so the fence-keyed deletion queue (#101/#102) frees it once that
// submit's frame has cleared. A `streamed` image (the FontAtlas) keeps its
// staging, since it re-fills the persistent map and re-uploads every frame.
//
// This drives the real upload path on a headless device — no swapchain/window
// needed, a buffer→image copy only touches the queue + command pool — and
// asserts:
// - After a static Update, image.buffer owns no handle (released), and
// exactly that one allocation was handed to the deletion queue tagged with
// the current frame (so it retires later, not now).
// - The upload + readback still complete with ZERO validation errors and the
// image reads back byte-equal to the staged source — proof the staging
// genuinely outlived the submit (the queue had not yet retired it), so
// releasing it is no use-after-free.
// - The deferred entry retires (and frees) only once framesInFlight frames
// elapse, and not before.
// - A `streamed` image keeps its staging across Update and UpdateRegion (the
// FontAtlas contract), and Destroy() then frees it validation-clean.
#include "vulkan/vulkan.h"
#include <cstring>
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;
}
VkCommandBuffer BeginCmd() {
VkCommandBufferAllocateInfo allocInfo {
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO,
.commandPool = Device::commandPool,
.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY,
.commandBufferCount = 1,
};
VkCommandBuffer cmd = VK_NULL_HANDLE;
Device::CheckVkResult(vkAllocateCommandBuffers(Device::device, &allocInfo, &cmd));
VkCommandBufferBeginInfo beginInfo {
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO,
.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT,
};
Device::CheckVkResult(vkBeginCommandBuffer(cmd, &beginInfo));
return cmd;
}
void SubmitWait(VkCommandBuffer cmd) {
Device::CheckVkResult(vkEndCommandBuffer(cmd));
VkSubmitInfo submitInfo {
.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO,
.commandBufferCount = 1,
.pCommandBuffers = &cmd,
};
Device::CheckVkResult(vkQueueSubmit(Device::queue, 1, &submitInfo, VK_NULL_HANDLE));
Device::CheckVkResult(vkQueueWaitIdle(Device::queue));
vkFreeCommandBuffers(Device::device, Device::commandPool, 1, &cmd);
}
// RGBA8 texel as a 4-byte word, matching VK_FORMAT_R8G8B8A8_UNORM.
using Texel = std::uint32_t;
constexpr std::uint16_t kW = 4;
constexpr std::uint16_t kH = 4;
constexpr std::uint32_t kCount = static_cast<std::uint32_t>(kW) * kH;
} // namespace
int main() {
Device::Initialize();
Device::validationErrorCount = 0;
// Control the deferred-deletion clock: enqueue at frame 0, retire after 2.
Device::framesInFlight = 2;
Device::frameCounter = 0;
Device::deletionQueue.clear();
// ── Static texture: staging is released right after the upload copy. ──────
ImageVulkan<Texel> tex;
{
VkCommandBuffer cmd = BeginCmd();
tex.Create(kW, kH, /*mipLevels*/ 1, cmd,
VK_FORMAT_R8G8B8A8_UNORM,
VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT
| VK_IMAGE_USAGE_SAMPLED_BIT,
VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
Check(tex.buffer.buffer != VK_NULL_HANDLE,
"Create allocated the host-visible staging buffer");
Check(!tex.streamed, "a default ImageVulkan is not streamed");
// Fill the staging map with a known pattern, then upload.
for (std::uint32_t i = 0; i < kCount; ++i) {
tex.buffer.value[i] = 0xA0B0C0D0u + i;
}
tex.Update(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
// ── The fix: staging released to the queue at record time, not pinned. ─
Check(tex.buffer.buffer == VK_NULL_HANDLE,
"static Update releases the staging buffer (no handle pinned)");
Check(Device::deletionQueue.size() == 1,
"exactly one allocation (the staging) handed to the deletion queue");
const bool taggedNow = !Device::deletionQueue.empty()
&& Device::deletionQueue.front().retireAfter == 0
&& Device::deletionQueue.front().buffer != VK_NULL_HANDLE;
Check(taggedNow,
"deferred staging is tagged with the enqueue frame and still a live handle");
// Read the image back to prove the copy saw valid staging. The upload's
// recorded copy still references the now-released staging; it must
// outlive THIS submit (queue retires at frame 2, we're at 0). Move the
// image to TRANSFER_SRC and copy into a host-visible readback buffer in
// the same command buffer.
VkImageMemoryBarrier toSrc {
.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER,
.srcAccessMask = VK_ACCESS_SHADER_READ_BIT,
.dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT,
.oldLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,
.newLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.image = tex.image,
.subresourceRange = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1 },
};
vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR,
VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr,
0, nullptr, 1, &toSrc);
VulkanBuffer<Texel, true> readback;
readback.Create(VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, kCount);
VkBufferImageCopy region {};
region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1 };
region.imageExtent = { kW, kH, 1 };
vkCmdCopyImageToBuffer(cmd, tex.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
readback.buffer, 1, &region);
SubmitWait(cmd);
Check(Device::validationErrorCount == 0,
"static upload + readback raised no validation errors");
readback.FlushHost();
bool match = true;
for (std::uint32_t i = 0; i < kCount; ++i) {
if (readback.value[i] != 0xA0B0C0D0u + i) { match = false; break; }
}
Check(match,
"image reads back byte-equal to the staged source "
"(released staging outlived the submit)");
}
// ── Retire timing: the staging frees once framesInFlight frames elapse,
// and not a frame before. We've already wait-idled, so freeing is safe. ─
Device::ReclaimDeletions();
Check(Device::deletionQueue.size() == 1,
"staging is NOT freed before framesInFlight frames have elapsed");
Device::frameCounter = 2; // retireAfter(0) + framesInFlight(2) <= 2
Device::ReclaimDeletions();
Check(Device::deletionQueue.empty(),
"staging is freed once its retire frame is reached");
// Destroying the image after its staging was released must not double-free
// (buffer handle is already null) and must stay validation-clean.
Device::validationErrorCount = 0;
tex.Destroy();
Check(Device::validationErrorCount == 0,
"Destroy after a released-staging upload is validation-clean");
// ── Streamed image: the persistent staging is kept across uploads. ────────
Device::frameCounter = 0;
Device::deletionQueue.clear();
Device::validationErrorCount = 0;
{
ImageVulkan<Texel> atlas;
VkCommandBuffer cmd = BeginCmd();
atlas.Create(kW, kH, /*mipLevels*/ 1, cmd,
VK_FORMAT_R8G8B8A8_UNORM,
VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT,
VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,
VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT,
/*streamed*/ true);
Check(atlas.streamed, "streamed=true is recorded on the image");
for (std::uint32_t i = 0; i < kCount; ++i) atlas.buffer.value[i] = i;
atlas.Update(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
Check(atlas.buffer.buffer != VK_NULL_HANDLE,
"streamed Update keeps the persistent staging buffer");
// UpdateRegion (the FontAtlas re-upload path) must also keep it.
atlas.UpdateRegion(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, 0, 0, kW, kH);
Check(atlas.buffer.buffer != VK_NULL_HANDLE,
"streamed UpdateRegion keeps the persistent staging buffer");
Check(Device::deletionQueue.empty(),
"a streamed image enqueues nothing for deferred deletion");
SubmitWait(cmd);
Check(Device::validationErrorCount == 0,
"streamed upload raised no validation errors");
// Destroy frees the still-live persistent staging (the leak #114 names).
atlas.Destroy();
Check(atlas.buffer.buffer == VK_NULL_HANDLE,
"Destroy frees the streamed image's persistent staging buffer");
Check(Device::validationErrorCount == 0,
"streamed Destroy (freeing live staging) is validation-clean");
}
std::println("{}", failures == 0 ? "ALL PASS" : "FAILURES PRESENT");
return failures == 0 ? 0 : 1;
}