diff --git a/implementations/Crafter.Graphics-FontAtlas.cpp b/implementations/Crafter.Graphics-FontAtlas.cpp index 44179c0..180a402 100644 --- a/implementations/Crafter.Graphics-FontAtlas.cpp +++ b/implementations/Crafter.Graphics-FontAtlas.cpp @@ -39,13 +39,17 @@ void FontAtlas::Initialize(GraphicsCommandBuffer cmd) { VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL ); std::memset(image.buffer.value, 0, kAtlasSize * kAtlasSize); - dirty = true; #else (void)cmd; staging.assign(kAtlasSize * kAtlasSize, 0); textureHandle = WebGPU::wgpuCreateAtlasTexture(kAtlasSize, kAtlasSize); - dirty = true; #endif + // The freshly-zeroed atlas backs an image whose contents are undefined + // (Vulkan) / unwritten (WebGPU). Mark the whole extent dirty so the + // first Update clears it once; thereafter only glyph sub-rects flip + // dirty, and every untouched texel stays the zero it was uploaded as — + // so partial uploads keep the image byte-identical to the staging copy. + MarkDirty(0, 0, kAtlasSize, kAtlasSize); } bool FontAtlas::ShelfPlace(int w, int h, int& outX, int& outY) { @@ -112,7 +116,7 @@ bool FontAtlas::Ensure(Font& font, std::uint32_t codepoint) { g.v0 = static_cast(py) / kAtlasSize; g.u1 = static_cast(px + sw) / kAtlasSize; g.v1 = static_cast(py + sh) / kAtlasSize; - dirty = true; + MarkDirty(px, py, sw, sh); } cache_.emplace(key, g); @@ -126,16 +130,28 @@ const Glyph* FontAtlas::Lookup(Font& font, std::uint32_t codepoint) const { void FontAtlas::Update(GraphicsCommandBuffer cmd) { if (!dirty) return; + + // Clamp the accumulated box to the atlas and convert to (origin, + // extent). Add() works in glyph coordinates that are always in-bounds, + // but clamping keeps the copy extent provably valid. The `dirty` guard + // above guarantees the rect is non-empty, so UploadBox always fills the + // outputs. + std::uint32_t x = 0, y = 0, w = 0, h = 0; + dirtyRect.UploadBox(kAtlasSize, x, y, w, h); + #ifndef CRAFTER_GRAPHICS_WINDOW_DOM - image.Update(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); + image.UpdateRegion(cmd, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, x, y, w, h); #else (void)cmd; - // Full-atlas upload. Future: track dirty region. + // The staging buffer keeps the full atlas row stride, so srcBytesPerRow + // stays kAtlasSize and (dstX, dstY) index the sub-rect's first texel. WebGPU::wgpuWriteAtlasRegion( textureHandle, staging.data(), kAtlasSize, kAtlasSize, kAtlasSize, - 0, 0, kAtlasSize, kAtlasSize + static_cast(x), static_cast(y), + static_cast(w), static_cast(h) ); #endif dirty = false; + dirtyRect.Reset(); } diff --git a/interfaces/Crafter.Graphics-FontAtlas.cppm b/interfaces/Crafter.Graphics-FontAtlas.cppm index f80e9c6..c695f65 100644 --- a/interfaces/Crafter.Graphics-FontAtlas.cppm +++ b/interfaces/Crafter.Graphics-FontAtlas.cppm @@ -51,13 +51,62 @@ export namespace Crafter { static constexpr int kOnEdgeValue = 128; static constexpr float kPixelDistScale = 32.0f; + // Bounding box of the atlas pixels touched since the last upload, + // used to copy only the sub-rect that actually changed instead of + // the whole 1 MiB atlas. Half-open: covers [minX, maxX) × [minY, + // maxY). `active` distinguishes "nothing dirty" from a zero-origin + // rect. + struct DirtyRect { + int minX = 0, minY = 0, maxX = 0, maxY = 0; + bool active = false; + + bool Empty() const noexcept { return !active; } + void Reset() noexcept { active = false; } + + // Grow the box to include [x, x+w) × [y, y+h). No-op for an + // empty rect (a glyph that rasterized to nothing marks nothing). + void Add(int x, int y, int w, int h) noexcept { + if (w <= 0 || h <= 0) return; + if (!active) { + minX = x; minY = y; maxX = x + w; maxY = y + h; + active = true; + } else { + minX = std::min(minX, x); + minY = std::min(minY, y); + maxX = std::max(maxX, x + w); + maxY = std::max(maxY, y + h); + } + } + + // Clamp the box to [0, limit] on both axes and emit it as an + // (origin, extent) pair for a GPU copy. Returns false when + // empty, leaving the outputs untouched. + bool UploadBox(int limit, std::uint32_t& x, std::uint32_t& y, std::uint32_t& w, std::uint32_t& h) const noexcept { + if (!active) return false; + const int x0 = std::clamp(minX, 0, limit); + const int y0 = std::clamp(minY, 0, limit); + const int x1 = std::clamp(maxX, 0, limit); + const int y1 = std::clamp(maxY, 0, limit); + x = static_cast(x0); + y = static_cast(y0); + w = static_cast(x1 - x0); + h = static_cast(y1 - y0); + return true; + } + }; + #ifndef CRAFTER_GRAPHICS_WINDOW_DOM ImageVulkan image; #else WebGPUTextureRef textureHandle = 0; std::vector staging; #endif + // `dirty` stays the cheap "is there anything to flush?" flag the + // renderer polls each frame; `dirtyRect` carries the bounds Update + // copies. The two are always set and cleared together (dirty == + // !dirtyRect.Empty()). bool dirty = false; + DirtyRect dirtyRect; void Initialize(GraphicsCommandBuffer cmd); @@ -69,6 +118,13 @@ export namespace Crafter { const Glyph* Lookup(Font& font, std::uint32_t codepoint) const; void Update(GraphicsCommandBuffer cmd); + // Flag the [x, x+w) × [y, y+h) atlas region as needing re-upload, + // keeping `dirty` in lockstep with the accumulated `dirtyRect`. + void MarkDirty(int x, int y, int w, int h) noexcept { + dirtyRect.Add(x, y, w, h); + dirty = !dirtyRect.Empty(); + } + private: struct Shelf { int y = 0; int height = 0; int cursorX = 0; }; std::vector shelves_; diff --git a/interfaces/Crafter.Graphics-ImageVulkan.cppm b/interfaces/Crafter.Graphics-ImageVulkan.cppm index 46d8539..7d6a12b 100644 --- a/interfaces/Crafter.Graphics-ImageVulkan.cppm +++ b/interfaces/Crafter.Graphics-ImageVulkan.cppm @@ -162,6 +162,34 @@ export namespace Crafter { } } + // Upload only the sub-rectangle [x, x+w) × [y, y+h) of the staging + // buffer into the image, instead of the whole extent. The staging + // buffer keeps the full image row stride, so bufferRowLength stays + // `width` and bufferOffset just points at the rect's first texel — + // imageOffset/imageExtent then carve out the rect on the GPU side. + // Layout transitions stay whole-image (cheap); only the copy extent + // shrinks. Single mip level only: the partial-upload path is for + // CPU-streamed atlases (FontAtlas) which carry no mips, so there is + // no blit chain to regenerate. + void UpdateRegion(VkCommandBuffer cmd, VkImageLayout layout, std::uint32_t x, std::uint32_t y, std::uint32_t w, std::uint32_t h) { + buffer.FlushDevice(cmd, VK_ACCESS_MEMORY_READ_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT); + TransitionImageLayout(cmd, image, layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_SHADER_READ_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, 0, mipLevels); + + VkBufferImageCopy region{}; + region.bufferOffset = (static_cast(y) * width + x) * sizeof(PixelType); + region.bufferRowLength = width; + region.bufferImageHeight = 0; + region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + region.imageSubresource.mipLevel = 0; + region.imageSubresource.baseArrayLayer = 0; + region.imageSubresource.layerCount = 1; + region.imageOffset = { static_cast(x), static_cast(y), 0 }; + region.imageExtent = { w, h, 1 }; + vkCmdCopyBufferToImage(cmd, buffer.buffer, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, ®ion); + + TransitionImageLayout(cmd, image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, layout, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT, 0, mipLevels); + } + // GPU compressed-asset Update: stage compressed bytes, decompress // into `buffer` via VK_EXT_memory_decompression, then copy buffer→image // and transition to `layout`. Falls back to CPU decode + the existing diff --git a/project.cpp b/project.cpp index 7d45dd3..a70e5b6 100644 --- a/project.cpp +++ b/project.cpp @@ -295,6 +295,33 @@ extern "C" Configuration CrafterBuildProject(std::span a blasImpls.emplace_back("tests/BLASBuildOptions/main"); bc.GetInterfacesAndImplementations(ifaces, blasImpls); cfg.tests.push_back(std::move(blasTest)); + + // Issue #51: FontAtlas only re-uploads the dirty sub-rect now, + // tracked via FontAtlas::DirtyRect. The accumulation/clamp math is + // pure CPU, so this test drives it directly — no GPU device needed + // at runtime (the real copy params are covered by HelloUI rendering + // text on both backends). + Test atlasTest; + Configuration& ac = atlasTest.config; + ac.path = cfg.path; + ac.name = "FontAtlasDirtyRect"; + ac.outputName = "FontAtlasDirtyRect"; + ac.type = ConfigurationType::Executable; + ac.target = cfg.target; + ac.march = cfg.march; + ac.mtune = cfg.mtune; + ac.debug = cfg.debug; + ac.sysroot = cfg.sysroot; + ac.dependencies = cfg.dependencies; + ac.externalDependencies = cfg.externalDependencies; + ac.compileFlags = cfg.compileFlags; + ac.linkFlags = cfg.linkFlags; + ac.defines = cfg.defines; + ac.cFiles = cfg.cFiles; + std::vector atlasImpls(impls.begin(), impls.end()); + atlasImpls.emplace_back("tests/FontAtlasDirtyRect/main"); + ac.GetInterfacesAndImplementations(ifaces, atlasImpls); + cfg.tests.push_back(std::move(atlasTest)); } return cfg; diff --git a/tests/FontAtlasDirtyRect/main.cpp b/tests/FontAtlasDirtyRect/main.cpp new file mode 100644 index 0000000..c136bbe --- /dev/null +++ b/tests/FontAtlasDirtyRect/main.cpp @@ -0,0 +1,143 @@ +/* +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 #51: FontAtlas::Update used to re-upload the +// whole 1024×1024 atlas every time a single glyph was rasterized. It now +// accumulates a dirty bounding box (FontAtlas::DirtyRect) and copies only +// that sub-rect. The bookkeeping is pure CPU math, so this test drives it +// directly — no Vulkan device or WebGPU context needed: +// +// - a fresh rect is empty; the first Add seeds exact bounds +// - further Adds grow the half-open union, never shrink it +// - degenerate glyphs (w<=0 / h<=0) leave the rect untouched +// - UploadBox converts (min,max) → (origin, extent) and clamps to the +// atlas, and reports emptiness so Update can early-out +// - FontAtlas::MarkDirty keeps the public `dirty` flag in lockstep with +// the rect, and Reset() clears both notions of dirtiness +// +// The actual GPU copy params (bufferOffset / bufferRowLength on Vulkan, +// dst origin / bytesPerRow on WebGPU) are exercised end-to-end by rendering +// text in the HelloUI example on both backends. + +#include + +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; +} + +// Read UploadBox into locals and compare against an expected (x,y,w,h). +bool BoxIs(const FontAtlas::DirtyRect& r, int limit, + std::uint32_t ex, std::uint32_t ey, std::uint32_t ew, std::uint32_t eh) { + std::uint32_t x = 99999, y = 99999, w = 99999, h = 99999; + if (!r.UploadBox(limit, x, y, w, h)) return false; + return x == ex && y == ey && w == ew && h == eh; +} + +} // namespace + +int main() { + // --- DirtyRect: empty / seed / grow ------------------------------------ + { + FontAtlas::DirtyRect r; + Check(r.Empty(), "fresh rect is empty"); + std::uint32_t x, y, w, h; + Check(!r.UploadBox(1024, x, y, w, h), "UploadBox on empty rect returns false"); + + r.Add(10, 20, 4, 6); + Check(!r.Empty(), "rect is non-empty after first Add"); + Check(BoxIs(r, 1024, 10, 20, 4, 6), "first Add seeds exact (origin, extent)"); + + // A box fully inside the current one must not change anything. + r.Add(11, 21, 1, 1); + Check(BoxIs(r, 1024, 10, 20, 4, 6), "contained Add does not shrink or move the box"); + + // Grow up-left and down-right; result is the union. + r.Add(5, 8, 2, 2); // extends min corner to (5,8) + r.Add(100, 200, 10, 10); // extends max corner to (110,210) + Check(BoxIs(r, 1024, 5, 8, 105, 202), "Adds grow to the half-open union"); + } + + // --- DirtyRect: degenerate glyphs are no-ops --------------------------- + { + FontAtlas::DirtyRect r; + r.Add(3, 3, 0, 5); + r.Add(3, 3, 5, 0); + r.Add(3, 3, -1, -1); + Check(r.Empty(), "zero/negative-extent Adds leave the rect empty"); + + r.Add(3, 3, 5, 5); + r.Add(50, 50, 0, 0); // must not move the max corner out + Check(BoxIs(r, 1024, 3, 3, 5, 5), "degenerate Add after a real one is ignored"); + } + + // --- DirtyRect: clamp to atlas extent ---------------------------------- + { + FontAtlas::DirtyRect r; + r.Add(-4, -4, 8, 8); // straddles the top-left corner + Check(BoxIs(r, 1024, 0, 0, 4, 4), "negative origin clamps to 0, extent trimmed"); + + FontAtlas::DirtyRect r2; + r2.Add(1020, 1020, 16, 16); // straddles the bottom-right corner + Check(BoxIs(r2, 1024, 1020, 1020, 4, 4), "over-extent box clamps to the atlas edge"); + } + + // --- FontAtlas::MarkDirty keeps `dirty` and the rect in sync ----------- + { + FontAtlas atlas; + Check(!atlas.dirty && atlas.dirtyRect.Empty(), "default FontAtlas is clean"); + + atlas.MarkDirty(0, 0, FontAtlas::kAtlasSize, FontAtlas::kAtlasSize); + Check(atlas.dirty, "MarkDirty sets the public dirty flag"); + Check(BoxIs(atlas.dirtyRect, FontAtlas::kAtlasSize, + 0, 0, FontAtlas::kAtlasSize, FontAtlas::kAtlasSize), + "full-atlas MarkDirty covers the whole extent (the Initialize clear)"); + + atlas.MarkDirty(8, 8, 16, 16); // contained — full extent already dirty + Check(BoxIs(atlas.dirtyRect, FontAtlas::kAtlasSize, + 0, 0, FontAtlas::kAtlasSize, FontAtlas::kAtlasSize), + "subsequent contained MarkDirty keeps the full extent"); + + // Simulate the reset Update performs after a successful upload. + atlas.dirty = false; + atlas.dirtyRect.Reset(); + Check(!atlas.dirty && atlas.dirtyRect.Empty(), "post-upload reset clears dirtiness"); + + // After reset, a single glyph marks only its own rect. + atlas.MarkDirty(64, 128, 12, 20); + Check(atlas.dirty, "a glyph after reset re-arms dirty"); + Check(BoxIs(atlas.dirtyRect, FontAtlas::kAtlasSize, 64, 128, 12, 20), + "post-reset MarkDirty tracks just the new glyph, not the old full extent"); + } + + if (failures != 0) { + std::println("{} check(s) failed", failures); + return EXIT_FAILURE; + } + std::println("all checks passed"); + return EXIT_SUCCESS; +}