1 changed files with 38 additions and 0 deletions
Issue #110 reported `crafter-build test MeshDecompressStagingRelease` failing
deterministically on the GPU decompress path: the "exactly one allocation
handed to the deletion queue" assertion saw more than one entry.
Root cause (confirmed at Crafter.Graphics-Mesh.cpp:175-176): a static
(allowUpdate=false) Build cannot refit, so it DeferredClear()s its now-dead
per-mesh BLAS scratch in addition to the compressed staging (#67) — two
deferred allocations, not one. The original test built statically yet asserted
size==1, so the scratch was the unaccounted-for second entry. The behaviour
was already corrected on master by PR #109 (the #73 merge, ed9b3f6), which
switched the assertion's build to allowUpdate=true so the scratch is retained.
This hardens that fix: it adds a sibling case that Builds with allowUpdate=false
and asserts the queue holds exactly TWO entries (staging + dead scratch),
builds with zero validation errors, and that both retire on schedule. The count
the #67 assertion relies on is now a named, explicitly-tested quantity rather
than an implicit comment, so a future change to scratch deferral can't silently
shift it back.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
commit
3655cd636c
|
|
@ -39,6 +39,13 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
|
|||
// - The deferred entry actually retires (and frees) once framesInFlight frames
|
||||
// elapse, and not before — the whole point of routing through the queue.
|
||||
//
|
||||
// As a regression guard for issue #110 it then repeats the Build with
|
||||
// allowUpdate=false and asserts the queue holds exactly TWO entries — the
|
||||
// compressed staging PLUS the dead per-mesh BLAS scratch a static build can no
|
||||
// longer refit. That extra scratch was the unaccounted-for entry behind #110
|
||||
// (the original test did a static build yet asserted size==1); pinning the
|
||||
// breakdown keeps the count above a known, named quantity.
|
||||
//
|
||||
// On hardware without VK_EXT_memory_decompression the compressed Build takes the
|
||||
// CPU-decode fallback, which never allocates compressedStaging, so the new
|
||||
// behavior is moot — the test reports that and skips the GPU-path assertions.
|
||||
|
|
@ -217,6 +224,37 @@ int main() {
|
|||
Check(Device::deletionQueue.empty(),
|
||||
"staging is freed once its retire frame is reached");
|
||||
|
||||
// ── Regression guard for issue #110 ──────────────────────────────────────
|
||||
// The "exactly one allocation" assertion above holds ONLY because the
|
||||
// refit-capable build (allowUpdate=true) RETAINS its per-mesh BLAS scratch
|
||||
// for in-place updates (#66). A static (allowUpdate=false) build cannot
|
||||
// refit, so it hands its now-dead scratch to the deletion queue too — the
|
||||
// SAME Build then lands TWO deferred allocations: the compressed staging
|
||||
// (#67) AND the dead scratch. Issue #110 was the original test doing a
|
||||
// static build and asserting size==1: the scratch was the unaccounted-for
|
||||
// second entry. Pin that breakdown down explicitly so a future change to
|
||||
// scratch deferral can't silently shift the count the #67 assertion relies
|
||||
// on, and so the "extra deferred allocation" stays a known, named quantity.
|
||||
Device::frameCounter = 0;
|
||||
Device::deletionQueue.clear();
|
||||
Mesh staticMesh;
|
||||
VkCommandBuffer scmd = BeginCmd();
|
||||
staticMesh.Build(asset, scmd, RTBuildOptions{ .allowUpdate = false });
|
||||
Check(staticMesh.compressedStaging.buffer == VK_NULL_HANDLE,
|
||||
"static Build also releases compressedStaging (#67 is allowUpdate-independent)");
|
||||
Check(Device::deletionQueue.size() == 2,
|
||||
"static Build enqueues staging + dead BLAS scratch (the #110 extra deletion)");
|
||||
SubmitWait(scmd);
|
||||
Check(staticMesh.blasAddr != 0, "static compressed Build produced a non-zero BLAS address");
|
||||
Check(Device::validationErrorCount == 0,
|
||||
"static decompress + BLAS build raised no validation errors");
|
||||
// Retire both entries (enqueued at frame 0, retire at framesInFlight) so the
|
||||
// staticMesh's scratch/staging are freed and nothing leaks past this test.
|
||||
Device::frameCounter = Device::framesInFlight;
|
||||
Device::ReclaimDeletions();
|
||||
Check(Device::deletionQueue.empty(),
|
||||
"static Build's staging + scratch both retire once their frame elapses");
|
||||
|
||||
std::println("{}", failures == 0 ? "ALL PASS" : "FAILURES PRESENT");
|
||||
return failures == 0 ? 0 : 1;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue