From a7a5640beb6ebde7be5060e2ae81291d5f83d147 Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Sun, 4 Oct 2026 11:35:05 -0400 Subject: [PATCH 1/2] Record a session-fed multires gesture into a clay_multires_delta clay_multires_sculptor_apply_stroke_tx (ABI 0.126.0, #688) took no record, so a multires gesture fed through a stroke session could not enter a host's undo stack, while its mesh and adaptive siblings and the whole-path _apply_stroke_recorded all take one. 0.126.0 is untagged, so the parameter is added in place: a nullable clay_multires_delta* record after defer_normals, as in _apply_stroke_recorded. The record is part of the session's binding, is continued across every call of the gesture (base and active-pass halves), and is checked before the session's stamps are taken, as is the report, so a CLAY_ERROR_SNAPSHOT_MISMATCH or a malformed report loses no stamps. --- bindings/c/clay.h | 34 ++- bindings/c/clay_c.cpp | 21 +- docs/05-claycore-library.md | 2 +- docs/07-brushes-and-features.md | 15 +- .../add-stroke-session/specs/c-abi/spec.md | 12 +- openspec/changes/add-stroke-session/tasks.md | 5 + tests/unit/test_c_multires_delta.cpp | 263 ++++++++++++++++++ tests/unit/test_c_stroke_session.cpp | 20 +- 8 files changed, 348 insertions(+), 24 deletions(-) diff --git a/bindings/c/clay.h b/bindings/c/clay.h index 8407616b..da29c2ee 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -10285,17 +10285,37 @@ clay_result clay_multires_sculptor_apply_stroke(clay_multires_sculptor* sculptor int32_t defer_normals, size_t* out_applied, clay_multires_stamp_report* out_report); -/* clay_multires_sculptor_apply_stroke, fed by a stroke session (ABI +/* clay_multires_sculptor_apply_stroke_recorded, fed by a stroke session (ABI * 0.126.0). The gesture begins the level record MeshBrush::Layer measures * against once, at its first stamp, and carries a grab's region and a * snakehook's anchor to the close, as the whole-path call does within one - * call. `out_report` describes this call. */ + * call. `out_report` describes this call. + * + * `record` (a clay_multires_delta, declared below) may be NULL for an + * unrecorded gesture, which stamps exactly what a recorded one does. Recorded + * or not, and into which record, is part of the binding: a later call naming + * another record, or NULL after a record, is CLAY_ERROR_INVALID_ARGUMENT. + * Every call of the gesture continues the same record, as consecutive + * clay_multires_sculptor_stamp_recorded calls do, so one record across the + * gesture is ONE undo step holding both halves -- the base detail and, with an + * active sculpt pass, that pass -- and equals the record + * clay_multires_sculptor_apply_stroke_recorded makes for the whole path. + * + * The refusals come BEFORE the session's stamps are taken, so a refused call + * loses none of them and the next accepted call applies them: a malformed + * report is CLAY_ERROR_INVALID_ARGUMENT, and a record the hierarchy no longer + * accepts -- another hierarchy, a changed level structure, or a DIFFERENT + * sculpt pass made active mid-gesture while the record holds one -- is + * CLAY_ERROR_SNAPSHOT_MISMATCH, with `*out_applied` 0 and nothing stamped. */ +typedef struct clay_multires_delta clay_multires_delta; clay_result clay_multires_sculptor_apply_stroke_tx(clay_multires_sculptor* sculptor, clay_stroke_tx* tx, const clay_mesh_brush_desc* brush, const clay_mask* mask, const clay_mesh_frame* mesh_to_world, - int32_t defer_normals, size_t* out_applied, + int32_t defer_normals, + clay_multires_delta* record, + size_t* out_applied, clay_multires_stamp_report* out_report); /* -- changed-block transport ------------------------------------------------- @@ -10806,8 +10826,9 @@ clay_result clay_multires_sculpt_layer_stroke_cancel(clay_multires_sculpt_layer_ * same spot record the same entry count and the same encoded bytes. * * THREE WAYS IN, ONE RECORD: - * - clay_multires_sculptor_stamp_recorded and _apply_stroke_recorded: the - * plain sculptor. It writes the stack's ACTIVE sculpt pass when there is + * - clay_multires_sculptor_stamp_recorded, _apply_stroke_recorded and the + * stroke-session consumer _apply_stroke_tx (ABI 0.126.0, the record + * continued across every call of the gesture): the plain sculptor. It writes the stack's ACTIVE sculpt pass when there is * one and the base when there is not, and the record takes whichever half * was written -- a host does not have to know in advance; * - clay_multires_sculpt_layer_stroke_commit_into: the transaction's record, @@ -10867,7 +10888,8 @@ clay_result clay_multires_sculpt_layer_stroke_cancel(clay_multires_sculpt_layer_ * and a capture into a record is a call on that record too. A replay only * READS its record. */ -typedef struct clay_multires_delta clay_multires_delta; +/* typedef struct clay_multires_delta clay_multires_delta; -- declared above + * clay_multires_sculptor_apply_stroke_tx, the first call that takes one. */ /* An empty, unbound record. Destroy with clay_multires_delta_destroy; NULL is * a no-op. */ diff --git a/bindings/c/clay_c.cpp b/bindings/c/clay_c.cpp index 30a116ea..26cbf83e 100644 --- a/bindings/c/clay_c.cpp +++ b/bindings/c/clay_c.cpp @@ -23361,20 +23361,25 @@ clay_result clay_multires_sculptor_apply_stroke_tx(clay_multires_sculptor* sculp const clay_mesh_brush_desc* brush, const clay_mask* mask, const clay_mesh_frame* mesh_to_world, - int32_t defer_normals, size_t* out_applied, + int32_t defer_normals, + clay_multires_delta* record, + size_t* out_applied, clay_multires_stamp_report* out_report) { if (out_applied) *out_applied = 0; if (!sculptor || !sculptor->sculptor) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null multires sculptor"); + clay_result r = probe_multires_report(out_report); + if (r != CLAY_OK) return r; mesh::MultiresSurface* sp = sculptor->owner ? sculptor->owner->target() : nullptr; if (!sp) return fail(CLAY_ERROR_NOT_FOUND, "hierarchy is no longer in its document"); SculptBind b; - clay_result r = read_sculpt_bind(*sculptor, brush, mask, mesh_to_world, defer_normals, &b); + r = read_sculpt_bind(*sculptor, brush, mask, mesh_to_world, defer_normals, &b); if (r != CLAY_OK) return r; MultiresStrokeSink* sink = nullptr; r = bind_stroke_sink( - tx, StrokeSinkKind::Multires, sculptor, {bind_key(mask), bind_key(defer_normals)}, + tx, StrokeSinkKind::Multires, sculptor, + {bind_key(mask), bind_key(defer_normals), bind_key(record)}, [&](std::unique_ptr* made) { *made = std::make_unique(*sculptor, tx->tx.preset(), *sculptor->sculptor, b); @@ -23382,10 +23387,18 @@ clay_result clay_multires_sculptor_apply_stroke_tx(clay_multires_sculptor* sculp }, same_descriptors(b), &sink); if (r != CLAY_OK) return r; + // THE RECORD BEFORE THE STAMPS ARE TAKEN, as on the adaptive consumer: a + // refused call loses none of them, and the next accepted call applies them. + if (!sink->closed) { + r = accept_multires_record(record, *sp); + if (r != CLAY_OK) return r; + } const std::uint32_t level = sp->sculpt_level(); StrokeBatch batch = next_batch(*sink, sink->source(tx->tx)); - const std::size_t applied = sink->gesture.apply(batch.stamps, nullptr); + const std::size_t applied = + sink->gesture.apply(batch.stamps, base_half(record), layer_half(record)); + if (record) record->gesture.bind(*sp); if (out_applied) *out_applied = applied; if (batch.closes) { sink->closed = true; diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 7a8385ee..06e0b490 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -1902,7 +1902,7 @@ Runnable: [`examples/60_surviving_a_crash.py`](../examples/60_surviving_a_crash. - **A rebuild through the document** (`clay_document_voxel_remesh_layer`, `Document.voxel_remesh_layer`): the same operation landing on a LAYER, as one undo step, transactional — a refusal, a validation failure or a cancel leaves the layer byte-identical and adds no step. Its undo record is `session::Step::Kind::MeshReplace`, holding the mesh on each side, because a sparse `VertexDeltas` records no indices by design and cannot express a change that replaces them. A per-layer GEOMETRY REVISION moves for a wholesale replacement and never for a sculpt: that asymmetry is what lets a cached adjacency, BVH or sculptor survive a brush stroke and be invalidated by a rebuild, and it is what makes a late-arriving commit refusable rather than silently winning. It also closes a trap neither of the older checks could see — a replacement landing on the same vertex and index counts passes both the mesh-pointer comparison (a `std::map` node's address is stable) and the sculptor's own count check. - **Welding** (`mesh/weld.h`): merging coincident vertices and removing the triangles that collapses. **The default mesher emits ZERO-AREA TRIANGLES** — 1458 of 70,140 on a plain analytic sphere, two per cent, with two corners at bit-identical positions — and nothing had noticed, for two reasons worth stating: everything downstream tolerates them (an exporter writes them, a BVH holds them, the decimator drops them itself), and `validate` counts them as SLIVERS rather than degenerates, because its `degenerate_triangles` means *repeated indices* and these have distinct indices at identical positions. So a marched mesh reports `clean()` while carrying them. `mesh::DynamicSurface` cannot hold one — a half-edge face with two of the same vertex does not exist — so it refuses, which meant **no mesh this library marched could become an adaptive surface at all** until this shipped. Not the same verb as `Adjacency`'s weld: that groups vertices into classes and leaves the triangle list byte-identical (a seam's duplicates move together under a brush); this merges them and rewrites the triangles. Watertightness survives by construction — a triangle whose corners coincide bounds nothing, so removing it cannot open a hole — a UV seam is preserved by default (duplicated positions with different UVs is how a flat mesh *represents* a seam), quads are dropped on rewrite, every index is in range afterwards, and a mesh with nothing to merge comes back byte-identical. - **Adaptive-surface undo** (`clay_dynamic_delta`, pyclay `TopologyDelta`, ABI 0.118.0): a host-owned record of one adaptive gesture (`mesh::RecordedGesture`, a `TopologyDelta` plus the surface marks at both ends), captured by `clay_dynamic_sculptor_stamp_recorded` and replayed by `clay_dynamic_delta_revert` / `_apply` **through the sculptor**. The host owns the sequence: how deep the stack is, when a record is dropped, how an undo interleaves with its own document edits. The engine owns one thing, the proof that a record matches the surface before it writes. **Records replay last in, first out, and nothing else.** Every surface carries a `{lineage, epoch}` mark, and a replay onto any state other than the record's end state is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written. This is needed even for strokes that never touch, because a later stroke reuses the slots an earlier one freed. Replaying onto the target state is `CLAY_OK` and writes nothing. The sculptor's chunk index and dirty-chunk stream follow the replay incrementally (0.1 ms at 49k faces against 39 ms for `rebuild_index`), so no rebuild is needed; one clears the dirty set and renumbers chunks. A replay restores `to_mesh` and the stored normals byte for byte over live elements, but not `clay_dynamic_surface_serialize` bytes, since dead slots stay. **Budgeting**: `clay_dynamic_delta_stats.resident_bytes` is what a record holds, allocator-dependent, and the memory ledger does not count it. `encoded_bytes` is exact (`56 + 122V + 114H + 42E + 66F`). **Spilling**: `clay_dynamic_delta_serialize` writes a record out and `_deserialize` reads it back to replay onto the same surface handle. It is not crash recovery, because a reloaded surface is a new lineage. One more limit: a replay keeps only the sculptor it runs through in step, so a second sculptor over the same surface is left stale, just as it is by a stamp. See `docs/07-brushes-and-features.md` § 8b. -- **Multiresolution undo** (`clay_multires_delta`, ABI 0.125.0, issue #671): a host-owned record of one gesture on a hierarchy (`mesh::MultiresGesture`: a `MultiresDelta` base half, a `SculptLayerDelta` pass half, and the hierarchy's `structure_revision` plus a per-process origin at capture). Captured by `clay_multires_sculptor_stamp_recorded` / `_apply_stroke_recorded` (the plain sculptor, which writes the active pass when there is one) and by `clay_multires_sculpt_layer_stroke_commit_into`; replayed by `clay_multires_delta_revert` / `_apply` **through the hierarchy**, bit-identically, idempotently, and with the moved patches in `clay_multires_dirty_blocks`. **Bound to its structure**: another hierarchy, a level added or removed (even one later undone), or a pass the stack no longer has is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written; no ordering is enforced beyond that, so overlapping records replay last in, first out by the host. `encoded_bytes` is exact (`40 + 16 + 32d + 28c` for a base half, `+ 24 + 32ld + 16lm` for a pass half); `resident_bytes` is the budget figure and the memory ledger does not count it. Serialize is for spilling within the process. See `docs/07-brushes-and-features.md` § 8d. +- **Multiresolution undo** (`clay_multires_delta`, ABI 0.125.0, issue #671): a host-owned record of one gesture on a hierarchy (`mesh::MultiresGesture`: a `MultiresDelta` base half, a `SculptLayerDelta` pass half, and the hierarchy's `structure_revision` plus a per-process origin at capture). Captured by `clay_multires_sculptor_stamp_recorded` / `_apply_stroke_recorded` / the stroke-session consumer `_apply_stroke_tx` (ABI 0.126.0; one record continued across the gesture's calls, checked before the session's stamps are taken) (the plain sculptor, which writes the active pass when there is one) and by `clay_multires_sculpt_layer_stroke_commit_into`; replayed by `clay_multires_delta_revert` / `_apply` **through the hierarchy**, bit-identically, idempotently, and with the moved patches in `clay_multires_dirty_blocks`. **Bound to its structure**: another hierarchy, a level added or removed (even one later undone), or a pass the stack no longer has is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written; no ordering is enforced beyond that, so overlapping records replay last in, first out by the host. `encoded_bytes` is exact (`40 + 16 + 32d + 28c` for a base half, `+ 24 + 32ld + 16lm` for a pass half); `resident_bytes` is the budget figure and the memory ledger does not count it. Serialize is for spilling within the process. See `docs/07-brushes-and-features.md` § 8d. - **Spatial scalar transfer** (`mesh/transfer.h`): resampling a caller-owned per-vertex array — a mask, a weight — from one mesh onto another by closest point. Mask is not a `Mesh` field and does not become one to suit an operation; this is what carries it across a topology replacement. - **The dual meshers walk the cell range once.** Surface nets, dual contouring and the quad mesher share `dual_grid_mesh`, which used to place every cell's vertex in one pass and then walk the whole range again to emit quads — re-reading four lattice corners it already had, for every cell including the ones that own no vertex and can contribute no quad. It now emits a cell's quads as it places its vertex, which is sound because a quad only ever references cells reached by stepping BACK along the two axes that are not the edge's: the owning cell and three already placed. The sampler is also templated rather than a `std::function`, so the tape meshers' lambda inlines — eight corners a cell is 33M calls on a 0.02 lattice, and none of them was inlining. Together 113 ms → 75 ms serial, output byte-identical across all five entry points (#304). - **Brick meshing runs the march across bricks in parallel and welds serially.** Marching a brick is pure — it reads the cache, which is a const lookup — so bricks fan out over `clay/parallel`. The WELDING cannot: one `Builder` serves every brick precisely so a lattice edge shared by two of them yields one vertex, which is what keeps the sparse set watertight at brick seams. So the march records what each brick would emit and a serial pass replays those recordings through the single builder in key order, calling `edge_vertex` in the same sequence the serial loop did — the result is byte-identical to the serial path by construction, not by tolerance. Measured on a 24-core desktop: 276 bricks 30.9 ms → 7.5 ms. diff --git a/docs/07-brushes-and-features.md b/docs/07-brushes-and-features.md index 54f022f4..6da6527c 100644 --- a/docs/07-brushes-and-features.md +++ b/docs/07-brushes-and-features.md @@ -1189,6 +1189,15 @@ rebuilt descriptor with the same values matches), is refused. The call after gesture is open on its target: a grab's carried region, the deferred normals, and — for SDF, voxel and mask targets — the owning document's undo group, which is what makes a gesture one undo step. +The sculptor consumers take the record their whole-path recorded call takes: a +`clay_mesh_deltas` for a mesh, a `clay_dynamic_delta` for an adaptive surface, +and a `clay_multires_delta` for a hierarchy. Passing one record to every call of +the gesture (NULL for none; which one is part of the binding) gives the record +the whole-path call gives, both of a hierarchy's halves included, so one +`clay_multires_delta_revert` takes the gesture back on every level. The adaptive +and multires records are checked before the call takes its stamps: a refusal +(`CLAY_ERROR_SNAPSHOT_MISMATCH`, e.g. another sculpt pass made active +mid-gesture) loses none of them. The mesh consumers are the same code as their whole-path calls (`MeshStrokeGesture`, `MultiresStrokeGesture`, `DynamicStrokeGesture`): a stroke per call would re-gather a grab's region at every batch and land elsewhere. @@ -2464,8 +2473,10 @@ write: the base half (`MultiresDelta`: cage positions at level 0, a level's own coefficients above it) and the layer half (`SculptLayerDelta`: one pass's coefficients and mask weights). -Three calls capture into it. `clay_multires_sculptor_stamp_recorded` and -`_apply_stroke_recorded` are the plain sculptor's calls with a record; the plain +Four calls capture into it. `clay_multires_sculptor_stamp_recorded`, +`_apply_stroke_recorded` and the stroke-session consumer `_apply_stroke_tx` (§ 5, +one record continued across every call of the gesture) are the plain sculptor's +calls with a record; the plain sculptor writes the **active pass** when there is one, so the record takes whichever half was written and a host does not have to know in advance which. `clay_multires_sculpt_layer_stroke_commit_into` closes a layered transaction and diff --git a/openspec/changes/add-stroke-session/specs/c-abi/spec.md b/openspec/changes/add-stroke-session/specs/c-abi/spec.md index f1bb0f63..2dfefea3 100644 --- a/openspec/changes/add-stroke-session/specs/c-abi/spec.md +++ b/openspec/changes/add-stroke-session/specs/c-abi/spec.md @@ -16,7 +16,7 @@ The C ABI SHALL expose an opaque `clay_stroke_tx` over `brush::StrokeTransaction The first consumer call SHALL bind the session to its target, its handle arguments, its scalars and its descriptors. Every later call SHALL decode its descriptors again and compare them, field by field, with what the bind decoded; padding and the declared `struct_size` are not compared. A later call naming a different target, kind, handle or scalar, or passing a descriptor that is NULL, malformed or decodes differently, SHALL be refused with `CLAY_ERROR_INVALID_ARGUMENT` and apply nothing. -One gesture SHALL be one undo step. The SDF, voxel and mask consumers hold the owning document's undo group open from the bind to the close. A mesh gesture records into the caller's delta record. An adaptive gesture continues one `clay_dynamic_delta` across calls and refuses a call whose record the surface has moved past with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. Destroying a session whose gesture is open SHALL close the gesture without applying the stamps it held back. +One gesture SHALL be one undo step. The SDF, voxel and mask consumers hold the owning document's undo group open from the bind to the close. A mesh gesture records into the caller's delta record. An adaptive gesture continues one `clay_dynamic_delta` across calls and refuses a call whose record the surface has moved past with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. A multiresolution gesture continues one `clay_multires_delta` across calls, capturing both the base half and, with an active sculpt pass, the pass half, so the record equals the one `clay_multires_sculptor_apply_stroke_recorded` makes for the whole path; it refuses a call whose record the hierarchy no longer accepts (another hierarchy, a changed level structure, or a different pass active while the record holds one) with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. For both, the record is part of the binding, and NULL records nothing and stamps what a recorded gesture stamps. Destroying a session whose gesture is open SHALL close the gesture without applying the stamps it held back. The whole-path calls SHALL be unchanged. @@ -36,6 +36,14 @@ The whole-path calls SHALL be unchanged. - **WHEN** an unrecorded stamp moves the surface between two calls of a recorded adaptive gesture - **THEN** the next call returns `CLAY_ERROR_SNAPSHOT_MISMATCH` and applies nothing, and once the record is cleared the following call applies the held stamps +#### Scenario: A multires gesture fed in batches is one record +- **WHEN** a stroke is fed to `clay_multires_sculptor_apply_stroke_tx` in batches with one `clay_multires_delta`, on a hierarchy with and without an active sculpt pass +- **THEN** the positions equal `clay_multires_sculptor_apply_stroke_recorded`'s, the record's entry counts, levels and encoded size equal that call's record, and one `clay_multires_delta_revert` restores every level bit for bit + +#### Scenario: A refused multires call loses no stamps +- **WHEN** another sculpt pass is made active between two calls of a recorded multires gesture +- **THEN** the next call returns `CLAY_ERROR_SNAPSHOT_MISMATCH` and applies nothing, and once the first pass is active again the following call applies the held stamps and the gesture ends where the whole-path stroke does + #### Scenario: The binding is checked -- **WHEN** a bound session is passed another layer, another item, a mask it was not bound with, another scalar, another kind of target, a NULL brush descriptor, a brush descriptor with another strength, another topology or another frame +- **WHEN** a bound session is passed another layer, another item, a mask it was not bound with, another scalar, another kind of target, a NULL brush descriptor, a brush descriptor with another strength, another topology, another frame, or another record than the one it was bound with (NULL included) - **THEN** each call returns `CLAY_ERROR_INVALID_ARGUMENT` and applies nothing diff --git a/openspec/changes/add-stroke-session/tasks.md b/openspec/changes/add-stroke-session/tasks.md index 7835e8be..599b2a8d 100644 --- a/openspec/changes/add-stroke-session/tasks.md +++ b/openspec/changes/add-stroke-session/tasks.md @@ -22,3 +22,8 @@ ## Docs - [x] `docs/07-brushes-and-features.md`: the session, its rule, and the cross-binding table. + +## Follow-up before 0.126.0 is tagged +- [x] `clay_multires_sculptor_apply_stroke_tx` takes a nullable `clay_multires_delta* record`, after `defer_normals` as in `clay_multires_sculptor_apply_stroke_recorded`. Changed in place: 0.126.0 is not tagged, so the ABI line stays 0.126.0. The record is part of the binding, is continued across the gesture's calls (both halves), and is checked, with the report, before the session's stamps are taken. +- [x] `tests/unit/test_c_multires_delta.cpp`: a session-fed stroke records the whole-path call's record and reverts bit for bit, with and without an active pass; a NULL record stamps the same; the record is part of the binding; a refusal (another pass active, a malformed report) loses no stamps. Mutation check: ignoring the record, checking it after the stamps are taken, dropping it from the binding, and checking the report afterwards each fail a new case. +- [x] `docs/05`, `docs/07` § 5 and § 8d. diff --git a/tests/unit/test_c_multires_delta.cpp b/tests/unit/test_c_multires_delta.cpp index b5dc1a4f..20cccc9c 100644 --- a/tests/unit/test_c_multires_delta.cpp +++ b/tests/unit/test_c_multires_delta.cpp @@ -17,6 +17,7 @@ #include +#include #include #include #include @@ -634,3 +635,265 @@ TEST_CASE("c multires delta: commit_into refuses a record that already holds a g clay_multires_delta_destroy(held); clay_multires_delta_destroy(fresh); } + +// -- a gesture fed by a stroke session (add-stroke-session, #670) ------------- +// +// clay_multires_sculptor_apply_stroke_tx is the whole-path stroke delivered in +// pieces, so its record has to be the whole-path call's record: one gesture, +// both halves, continued across every call, and refused BEFORE the session's +// stamps are taken so that a refusal loses none of them. + +namespace { + +// A drag across the cage in more samples than one batch, so the gesture is +// applied over several calls. +std::vector session_path() { + std::vector out; + const int n = 24; + for (int i = 0; i < n; ++i) { + const float t = static_cast(i) / static_cast(n - 1); + clay_stroke_sample_full s{}; + s.position[0] = -0.6f + 1.2f * t; + s.position[2] = 0.15f * t * t; + s.pressure = 0.6f + 0.4f * t; + out.push_back(s); + } + return out; +} + +std::vector packed(const std::vector& samples) { + std::vector out; + for (const clay_stroke_sample_full& s : samples) + out.insert(out.end(), {s.position[0], s.position[1], s.position[2], s.pressure, s.tilt}); + return out; +} + +// Feed `samples` in batches of `batch`, applying after every append and once +// after the end. Returns the stamps applied; `*writing_calls` counts the calls +// that applied any. +size_t feed_session(clay_multires_sculptor* sculptor, clay_stroke_tx* tx, + const std::vector& samples, size_t batch, + const clay_mesh_brush_desc& brush, clay_multires_delta* record, + size_t* writing_calls) { + size_t applied = 0; + *writing_calls = 0; + auto apply = [&] { + size_t n = 0; + clay_multires_stamp_report report{}; + report.struct_size = sizeof(report); + REQUIRE(clay_multires_sculptor_apply_stroke_tx(sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, &report) == CLAY_OK); + applied += n; + if (n > 0) ++*writing_calls; + }; + for (size_t done = 0; done < samples.size(); done += batch) { + const size_t n = std::min(batch, samples.size() - done); + REQUIRE(clay_stroke_tx_append(tx, samples.data() + done, n, nullptr, nullptr) == CLAY_OK); + apply(); + } + REQUIRE(clay_stroke_tx_end(tx) == CLAY_OK); + apply(); + return applied; +} + +clay_stroke_tx* session(const clay_stroke_preset& preset) { + clay_stroke_tx* tx = nullptr; + REQUIRE(clay_stroke_tx_begin(&preset, &tx) == CLAY_OK); + return tx; +} + +void check_same_record(const clay_multires_delta* a, const clay_multires_delta* b) { + const clay_multires_delta_stats sa = stats_of(a), sb = stats_of(b); + CHECK(sa.detail_entries == sb.detail_entries); + CHECK(sa.cage_entries == sb.cage_entries); + CHECK(sa.layer_detail_entries == sb.layer_detail_entries); + CHECK(sa.layer_mask_entries == sb.layer_mask_entries); + CHECK(sa.encoded_bytes == sb.encoded_bytes); + CHECK(levels_of(a) == levels_of(b)); +} + +} // namespace + +TEST_CASE( + "c multires delta: a session-fed stroke records the whole-path " + "stroke's record") { + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + const std::vector flat = packed(samples); + for (bool with_pass : {true, false}) { + CAPTURE(with_pass); + Fixture whole(6, 2), pieces(6, 2); + const uint64_t layer = with_pass ? whole.add_layer() : CLAY_NO_SCULPT_LAYER; + if (with_pass) REQUIRE(pieces.add_layer() == layer); + const Snapshot before = snapshot(pieces.surface); + + clay_multires_delta* expected = clay_multires_delta_create(); + size_t whole_applied = 0; + REQUIRE(clay_multires_sculptor_apply_stroke_recorded( + whole.sculptor, flat.data(), samples.size(), &preset, &brush, nullptr, nullptr, + 1, expected, &whole_applied, nullptr) == CLAY_OK); + REQUIRE(whole_applied > 4); + + clay_multires_delta* record = clay_multires_delta_create(); + clay_stroke_tx* tx = session(preset); + size_t writing_calls = 0; + const size_t applied = + feed_session(pieces.sculptor, tx, samples, 4, brush, record, &writing_calls); + clay_stroke_tx_destroy(tx); + CHECK(applied == whole_applied); + // The record is CONTINUED: several calls wrote into it, and it is one + // gesture rather than the last call's. + CHECK(writing_calls > 2); + const Snapshot after = snapshot(pieces.surface); + CHECK(after == snapshot(whole.surface)); + + const clay_multires_delta_stats s = stats_of(record); + CHECK(s.sculpt_layer == layer); + if (with_pass) + CHECK(s.layer_detail_entries > 0); + else + CHECK(s.detail_entries > 0); + check_same_record(record, expected); + + // ONE revert takes the whole gesture back, on every level, bit for bit. + check_round_trip(pieces.surface, record, before, after); + clay_multires_delta_destroy(record); + clay_multires_delta_destroy(expected); + } +} + +TEST_CASE( + "c multires delta: a session with a NULL record records nothing and " + "stamps the same") { + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + const std::vector flat = packed(samples); + Fixture plain(6, 2), unrecorded(6, 2), recorded(6, 2); + plain.add_layer(); + unrecorded.add_layer(); + recorded.add_layer(); + REQUIRE(clay_multires_sculptor_apply_stroke(plain.sculptor, flat.data(), samples.size(), + &preset, &brush, nullptr, nullptr, 1, nullptr, + nullptr) == CLAY_OK); + size_t calls = 0; + clay_stroke_tx* tx = session(preset); + feed_session(unrecorded.sculptor, tx, samples, 4, brush, nullptr, &calls); + clay_stroke_tx_destroy(tx); + clay_multires_delta* record = clay_multires_delta_create(); + tx = session(preset); + feed_session(recorded.sculptor, tx, samples, 4, brush, record, &calls); + clay_stroke_tx_destroy(tx); + CHECK(snapshot(unrecorded.surface) == snapshot(plain.surface)); + CHECK(snapshot(recorded.surface) == snapshot(plain.surface)); + clay_multires_delta_destroy(record); +} + +TEST_CASE("c multires delta: a session's record is part of its binding") { + // Recorded or not, and into which record, is fixed by the first call, as + // it is for the adaptive consumer: half a gesture in one record and half + // in another would be two undo steps that each restore a torn stroke. + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + Fixture f(6, 2); + f.add_layer(); + clay_multires_delta* record = clay_multires_delta_create(); + clay_multires_delta* other = clay_multires_delta_create(); + clay_stroke_tx* tx = session(preset); + REQUIRE(clay_stroke_tx_append(tx, samples.data(), 12, nullptr, nullptr) == CLAY_OK); + size_t n = 0; + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + REQUIRE(n > 0); + REQUIRE(clay_stroke_tx_append(tx, samples.data() + 12, 12, nullptr, nullptr) == CLAY_OK); + const Snapshot held = snapshot(f.surface); + for (clay_multires_delta* wrong : {other, static_cast(nullptr)}) { + n = 7; + CHECK(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + wrong, &n, + nullptr) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(n == 0); + } + CHECK(snapshot(f.surface) == held); + CHECK(stats_of(other).encoded_bytes == 40); + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + CHECK(n > 0); + clay_stroke_tx_destroy(tx); + clay_multires_delta_destroy(other); + clay_multires_delta_destroy(record); +} + +TEST_CASE( + "c multires delta: a refused session call loses none of the " + "session's stamps") { + // The record is checked BEFORE the stamps are taken. Mid-gesture the host + // makes another pass active: continuing the record would put that pass's + // coefficients under the first pass's id, so the call is refused with + // CLAY_ERROR_SNAPSHOT_MISMATCH and stamps nothing. Once the first pass is + // active again the held stamps apply, and the gesture ends exactly where + // the whole-path stroke does, as one record. A malformed report is refused + // before the stamps too. + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + const std::vector flat = packed(samples); + + Fixture whole(6, 2), f(6, 2); + int32_t err = -1; + const uint64_t first = f.add_layer(); + const uint64_t second = f.add_layer(); + REQUIRE(whole.add_layer() == first); + REQUIRE(whole.add_layer() == second); + REQUIRE(clay_multires_set_active_sculpt_layer(f.surface, first, &err) == CLAY_OK); + REQUIRE(clay_multires_set_active_sculpt_layer(whole.surface, first, &err) == CLAY_OK); + const Snapshot before = snapshot(f.surface); + clay_multires_delta* expected = clay_multires_delta_create(); + REQUIRE(clay_multires_sculptor_apply_stroke_recorded( + whole.sculptor, flat.data(), samples.size(), &preset, &brush, nullptr, nullptr, 1, + expected, nullptr, nullptr) == CLAY_OK); + + clay_multires_delta* record = clay_multires_delta_create(); + clay_stroke_tx* tx = session(preset); + REQUIRE(clay_stroke_tx_append(tx, samples.data(), 12, nullptr, nullptr) == CLAY_OK); + size_t n = 0; + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + REQUIRE(n > 0); + REQUIRE(stats_of(record).sculpt_layer == first); + REQUIRE(clay_stroke_tx_append(tx, samples.data() + 12, 12, nullptr, nullptr) == CLAY_OK); + REQUIRE(clay_stroke_tx_end(tx) == CLAY_OK); + + REQUIRE(clay_multires_set_active_sculpt_layer(f.surface, second, &err) == CLAY_OK); + const Snapshot held = snapshot(f.surface); + n = 7; + CHECK(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, + nullptr) == CLAY_ERROR_SNAPSHOT_MISMATCH); + CHECK(n == 0); + CHECK(snapshot(f.surface) == held); + CHECK(stats_of(record).sculpt_layer == first); + + REQUIRE(clay_multires_set_active_sculpt_layer(f.surface, first, &err) == CLAY_OK); + clay_multires_stamp_report broken{}; + broken.struct_size = 3; + n = 7; + CHECK(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, + &broken) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(n == 0); + CHECK(snapshot(f.surface) == held); + + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + CHECK(n > 0); + clay_stroke_tx_destroy(tx); + const Snapshot after = snapshot(f.surface); + CHECK(after == snapshot(whole.surface)); + check_same_record(record, expected); + check_round_trip(f.surface, record, before, after); + clay_multires_delta_destroy(record); + clay_multires_delta_destroy(expected); +} diff --git a/tests/unit/test_c_stroke_session.cpp b/tests/unit/test_c_stroke_session.cpp index 23c1e0b4..b7efb4be 100644 --- a/tests/unit/test_c_stroke_session.cpp +++ b/tests/unit/test_c_stroke_session.cpp @@ -928,7 +928,8 @@ TEST_CASE("c stroke session: a multires gesture is the whole-path gesture") { clay_multires_stamp_report report{}; report.struct_size = sizeof(report); REQUIRE(clay_multires_sculptor_apply_stroke_tx(pieces.sculptor, tx, &brush, nullptr, - nullptr, 1, &n, &report) == CLAY_OK); + nullptr, 1, nullptr, &n, + &report) == CLAY_OK); CHECK(report.level == 2); applied += n; }); @@ -1042,28 +1043,29 @@ TEST_CASE("c stroke session: a sculptor's descriptors are part of the binding") clay_stroke_tx* tx = begin(preset); REQUIRE(clay_stroke_tx_append(tx, samples.data(), 15, nullptr, nullptr) == CLAY_OK); std::size_t n = 0; - REQUIRE(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &brush, nullptr, nullptr, - 1, &n, nullptr) == CLAY_OK); + REQUIRE(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &brush, nullptr, nullptr, 1, + nullptr, &n, nullptr) == CLAY_OK); REQUIRE(n > 0); REQUIRE(clay_stroke_tx_append(tx, samples.data() + 15, 15, nullptr, nullptr) == CLAY_OK); const std::vector held = h.level(); clay_mesh_brush_desc stronger = brush; stronger.strength = 0.8f; n = 7; - CHECK(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &stronger, nullptr, nullptr, - 1, &n, nullptr) == - CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &stronger, nullptr, nullptr, 1, + nullptr, &n, + nullptr) == CLAY_ERROR_INVALID_ARGUMENT); CHECK(n == 0); clay_mesh_frame frame{}; frame.struct_size = sizeof(frame); frame.rotation[3] = 1.0f; frame.scale = 2.0f; CHECK(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &brush, nullptr, &frame, 1, - &n, nullptr) == CLAY_ERROR_INVALID_ARGUMENT); + nullptr, &n, + nullptr) == CLAY_ERROR_INVALID_ARGUMENT); CHECK(n == 0); CHECK(same_floats(h.level(), held)); - REQUIRE(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &brush, nullptr, nullptr, - 1, &n, nullptr) == CLAY_OK); + REQUIRE(clay_multires_sculptor_apply_stroke_tx(h.sculptor, tx, &brush, nullptr, nullptr, 1, + nullptr, &n, nullptr) == CLAY_OK); CHECK(n > 0); clay_stroke_tx_destroy(tx); } From 2429b2fe8a0ae3b1560d5af726ed2d9ac62333ca Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Sun, 4 Oct 2026 11:47:20 -0400 Subject: [PATCH 2/2] Bind a session's multires record only in a call that took stamps The consumer re-bound its record on every call, including a no-op call after the close, which skips the record check. A host calling after every append could therefore stamp a record from before a level change with the hierarchy's current structure_revision, and a redo after a level was removed and added back was accepted instead of refused with SNAPSHOT_MISMATCH. Bind only when stamps were taken, which is always after the check accepted the record. The adaptive consumer does not re-mark on an empty batch (DynamicStrokeGesture::apply_recorded returns before begin_capture); a subcase now pins that. Also states in clay.h that a pass activated mid-gesture joins a base-only record, with a test, and rewords the docs/05 capture sentence. --- bindings/c/clay.h | 18 +++-- bindings/c/clay_c.cpp | 9 ++- docs/05-claycore-library.md | 2 +- .../add-stroke-session/specs/c-abi/spec.md | 6 +- openspec/changes/add-stroke-session/tasks.md | 2 + tests/unit/test_c_multires_delta.cpp | 80 +++++++++++++++++++ tests/unit/test_c_stroke_session.cpp | 38 +++++++++ 7 files changed, 147 insertions(+), 8 deletions(-) diff --git a/bindings/c/clay.h b/bindings/c/clay.h index da29c2ee..fcf0c49c 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -10291,10 +10291,12 @@ clay_result clay_multires_sculptor_apply_stroke(clay_multires_sculptor* sculptor * snakehook's anchor to the close, as the whole-path call does within one * call. `out_report` describes this call. * - * `record` (a clay_multires_delta, declared below) may be NULL for an - * unrecorded gesture, which stamps exactly what a recorded one does. Recorded - * or not, and into which record, is part of the binding: a later call naming - * another record, or NULL after a record, is CLAY_ERROR_INVALID_ARGUMENT. + * `record` is a clay_multires_delta. Its typedef is just below; the record's + * own calls are documented further down, under clay_multires_delta. It may be + * NULL for an unrecorded gesture, which stamps exactly what a recorded one + * does. Recorded or not, and into which record, is part of the binding: a + * later call naming another record, or NULL after a record, is + * CLAY_ERROR_INVALID_ARGUMENT. * Every call of the gesture continues the same record, as consecutive * clay_multires_sculptor_stamp_recorded calls do, so one record across the * gesture is ONE undo step holding both halves -- the base detail and, with an @@ -10306,7 +10308,13 @@ clay_result clay_multires_sculptor_apply_stroke(clay_multires_sculptor* sculptor * report is CLAY_ERROR_INVALID_ARGUMENT, and a record the hierarchy no longer * accepts -- another hierarchy, a changed level structure, or a DIFFERENT * sculpt pass made active mid-gesture while the record holds one -- is - * CLAY_ERROR_SNAPSHOT_MISMATCH, with `*out_applied` 0 and nothing stamped. */ + * CLAY_ERROR_SNAPSHOT_MISMATCH, with `*out_applied` 0 and nothing stamped. A + * pass made active mid-gesture while the record holds only base entries is + * ACCEPTED: a base half and a pass half are separate storage, so the record + * then holds both and one revert still takes the whole gesture back. + * + * A call with nothing to apply -- before anything settles, or after the + * close -- leaves the record exactly as it was, binding included. */ typedef struct clay_multires_delta clay_multires_delta; clay_result clay_multires_sculptor_apply_stroke_tx(clay_multires_sculptor* sculptor, clay_stroke_tx* tx, diff --git a/bindings/c/clay_c.cpp b/bindings/c/clay_c.cpp index 26cbf83e..1239fee8 100644 --- a/bindings/c/clay_c.cpp +++ b/bindings/c/clay_c.cpp @@ -23398,7 +23398,14 @@ clay_result clay_multires_sculptor_apply_stroke_tx(clay_multires_sculptor* sculp StrokeBatch batch = next_batch(*sink, sink->source(tx->tx)); const std::size_t applied = sink->gesture.apply(batch.stamps, base_half(record), layer_half(record)); - if (record) record->gesture.bind(*sp); + // ONLY A CALL THAT TOOK STAMPS BINDS, and it took them after the check + // above accepted the record. A call with nothing to apply -- above all + // one after the close, which skips the check -- must leave the binding + // alone: re-binding there would stamp a record from before a level change + // with the hierarchy's current structure and let it replay onto a + // hierarchy it does not describe. An empty record needs no bind, and a + // non-empty one was bound by the call that captured into it. + if (record && !batch.stamps.empty()) record->gesture.bind(*sp); if (out_applied) *out_applied = applied; if (batch.closes) { sink->closed = true; diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 06e0b490..366ce0c6 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -1902,7 +1902,7 @@ Runnable: [`examples/60_surviving_a_crash.py`](../examples/60_surviving_a_crash. - **A rebuild through the document** (`clay_document_voxel_remesh_layer`, `Document.voxel_remesh_layer`): the same operation landing on a LAYER, as one undo step, transactional — a refusal, a validation failure or a cancel leaves the layer byte-identical and adds no step. Its undo record is `session::Step::Kind::MeshReplace`, holding the mesh on each side, because a sparse `VertexDeltas` records no indices by design and cannot express a change that replaces them. A per-layer GEOMETRY REVISION moves for a wholesale replacement and never for a sculpt: that asymmetry is what lets a cached adjacency, BVH or sculptor survive a brush stroke and be invalidated by a rebuild, and it is what makes a late-arriving commit refusable rather than silently winning. It also closes a trap neither of the older checks could see — a replacement landing on the same vertex and index counts passes both the mesh-pointer comparison (a `std::map` node's address is stable) and the sculptor's own count check. - **Welding** (`mesh/weld.h`): merging coincident vertices and removing the triangles that collapses. **The default mesher emits ZERO-AREA TRIANGLES** — 1458 of 70,140 on a plain analytic sphere, two per cent, with two corners at bit-identical positions — and nothing had noticed, for two reasons worth stating: everything downstream tolerates them (an exporter writes them, a BVH holds them, the decimator drops them itself), and `validate` counts them as SLIVERS rather than degenerates, because its `degenerate_triangles` means *repeated indices* and these have distinct indices at identical positions. So a marched mesh reports `clean()` while carrying them. `mesh::DynamicSurface` cannot hold one — a half-edge face with two of the same vertex does not exist — so it refuses, which meant **no mesh this library marched could become an adaptive surface at all** until this shipped. Not the same verb as `Adjacency`'s weld: that groups vertices into classes and leaves the triangle list byte-identical (a seam's duplicates move together under a brush); this merges them and rewrites the triangles. Watertightness survives by construction — a triangle whose corners coincide bounds nothing, so removing it cannot open a hole — a UV seam is preserved by default (duplicated positions with different UVs is how a flat mesh *represents* a seam), quads are dropped on rewrite, every index is in range afterwards, and a mesh with nothing to merge comes back byte-identical. - **Adaptive-surface undo** (`clay_dynamic_delta`, pyclay `TopologyDelta`, ABI 0.118.0): a host-owned record of one adaptive gesture (`mesh::RecordedGesture`, a `TopologyDelta` plus the surface marks at both ends), captured by `clay_dynamic_sculptor_stamp_recorded` and replayed by `clay_dynamic_delta_revert` / `_apply` **through the sculptor**. The host owns the sequence: how deep the stack is, when a record is dropped, how an undo interleaves with its own document edits. The engine owns one thing, the proof that a record matches the surface before it writes. **Records replay last in, first out, and nothing else.** Every surface carries a `{lineage, epoch}` mark, and a replay onto any state other than the record's end state is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written. This is needed even for strokes that never touch, because a later stroke reuses the slots an earlier one freed. Replaying onto the target state is `CLAY_OK` and writes nothing. The sculptor's chunk index and dirty-chunk stream follow the replay incrementally (0.1 ms at 49k faces against 39 ms for `rebuild_index`), so no rebuild is needed; one clears the dirty set and renumbers chunks. A replay restores `to_mesh` and the stored normals byte for byte over live elements, but not `clay_dynamic_surface_serialize` bytes, since dead slots stay. **Budgeting**: `clay_dynamic_delta_stats.resident_bytes` is what a record holds, allocator-dependent, and the memory ledger does not count it. `encoded_bytes` is exact (`56 + 122V + 114H + 42E + 66F`). **Spilling**: `clay_dynamic_delta_serialize` writes a record out and `_deserialize` reads it back to replay onto the same surface handle. It is not crash recovery, because a reloaded surface is a new lineage. One more limit: a replay keeps only the sculptor it runs through in step, so a second sculptor over the same surface is left stale, just as it is by a stamp. See `docs/07-brushes-and-features.md` § 8b. -- **Multiresolution undo** (`clay_multires_delta`, ABI 0.125.0, issue #671): a host-owned record of one gesture on a hierarchy (`mesh::MultiresGesture`: a `MultiresDelta` base half, a `SculptLayerDelta` pass half, and the hierarchy's `structure_revision` plus a per-process origin at capture). Captured by `clay_multires_sculptor_stamp_recorded` / `_apply_stroke_recorded` / the stroke-session consumer `_apply_stroke_tx` (ABI 0.126.0; one record continued across the gesture's calls, checked before the session's stamps are taken) (the plain sculptor, which writes the active pass when there is one) and by `clay_multires_sculpt_layer_stroke_commit_into`; replayed by `clay_multires_delta_revert` / `_apply` **through the hierarchy**, bit-identically, idempotently, and with the moved patches in `clay_multires_dirty_blocks`. **Bound to its structure**: another hierarchy, a level added or removed (even one later undone), or a pass the stack no longer has is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written; no ordering is enforced beyond that, so overlapping records replay last in, first out by the host. `encoded_bytes` is exact (`40 + 16 + 32d + 28c` for a base half, `+ 24 + 32ld + 16lm` for a pass half); `resident_bytes` is the budget figure and the memory ledger does not count it. Serialize is for spilling within the process. See `docs/07-brushes-and-features.md` § 8d. +- **Multiresolution undo** (`clay_multires_delta`, ABI 0.125.0, issue #671): a host-owned record of one gesture on a hierarchy (`mesh::MultiresGesture`: a `MultiresDelta` base half, a `SculptLayerDelta` pass half, and the hierarchy's `structure_revision` plus a per-process origin at capture). Captured by the plain sculptor's recorded calls, which write the active pass when there is one: `clay_multires_sculptor_stamp_recorded`, `_apply_stroke_recorded`, and the stroke-session consumer `_apply_stroke_tx` (ABI 0.126.0), which continues one record across the gesture's calls and checks it before the session's stamps are taken. Also captured by `clay_multires_sculpt_layer_stroke_commit_into`. Replayed by `clay_multires_delta_revert` / `_apply` **through the hierarchy**, bit-identically, idempotently, and with the moved patches in `clay_multires_dirty_blocks`. **Bound to its structure**: another hierarchy, a level added or removed (even one later undone), or a pass the stack no longer has is `CLAY_ERROR_SNAPSHOT_MISMATCH` with nothing written; no ordering is enforced beyond that, so overlapping records replay last in, first out by the host. `encoded_bytes` is exact (`40 + 16 + 32d + 28c` for a base half, `+ 24 + 32ld + 16lm` for a pass half); `resident_bytes` is the budget figure and the memory ledger does not count it. Serialize is for spilling within the process. See `docs/07-brushes-and-features.md` § 8d. - **Spatial scalar transfer** (`mesh/transfer.h`): resampling a caller-owned per-vertex array — a mask, a weight — from one mesh onto another by closest point. Mask is not a `Mesh` field and does not become one to suit an operation; this is what carries it across a topology replacement. - **The dual meshers walk the cell range once.** Surface nets, dual contouring and the quad mesher share `dual_grid_mesh`, which used to place every cell's vertex in one pass and then walk the whole range again to emit quads — re-reading four lattice corners it already had, for every cell including the ones that own no vertex and can contribute no quad. It now emits a cell's quads as it places its vertex, which is sound because a quad only ever references cells reached by stepping BACK along the two axes that are not the edge's: the owning cell and three already placed. The sampler is also templated rather than a `std::function`, so the tape meshers' lambda inlines — eight corners a cell is 33M calls on a 0.02 lattice, and none of them was inlining. Together 113 ms → 75 ms serial, output byte-identical across all five entry points (#304). - **Brick meshing runs the march across bricks in parallel and welds serially.** Marching a brick is pure — it reads the cache, which is a const lookup — so bricks fan out over `clay/parallel`. The WELDING cannot: one `Builder` serves every brick precisely so a lattice edge shared by two of them yields one vertex, which is what keeps the sparse set watertight at brick seams. So the march records what each brick would emit and a serial pass replays those recordings through the single builder in key order, calling `edge_vertex` in the same sequence the serial loop did — the result is byte-identical to the serial path by construction, not by tolerance. Measured on a 24-core desktop: 276 bricks 30.9 ms → 7.5 ms. diff --git a/openspec/changes/add-stroke-session/specs/c-abi/spec.md b/openspec/changes/add-stroke-session/specs/c-abi/spec.md index 2dfefea3..34f30fb2 100644 --- a/openspec/changes/add-stroke-session/specs/c-abi/spec.md +++ b/openspec/changes/add-stroke-session/specs/c-abi/spec.md @@ -16,7 +16,7 @@ The C ABI SHALL expose an opaque `clay_stroke_tx` over `brush::StrokeTransaction The first consumer call SHALL bind the session to its target, its handle arguments, its scalars and its descriptors. Every later call SHALL decode its descriptors again and compare them, field by field, with what the bind decoded; padding and the declared `struct_size` are not compared. A later call naming a different target, kind, handle or scalar, or passing a descriptor that is NULL, malformed or decodes differently, SHALL be refused with `CLAY_ERROR_INVALID_ARGUMENT` and apply nothing. -One gesture SHALL be one undo step. The SDF, voxel and mask consumers hold the owning document's undo group open from the bind to the close. A mesh gesture records into the caller's delta record. An adaptive gesture continues one `clay_dynamic_delta` across calls and refuses a call whose record the surface has moved past with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. A multiresolution gesture continues one `clay_multires_delta` across calls, capturing both the base half and, with an active sculpt pass, the pass half, so the record equals the one `clay_multires_sculptor_apply_stroke_recorded` makes for the whole path; it refuses a call whose record the hierarchy no longer accepts (another hierarchy, a changed level structure, or a different pass active while the record holds one) with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. For both, the record is part of the binding, and NULL records nothing and stamps what a recorded gesture stamps. Destroying a session whose gesture is open SHALL close the gesture without applying the stamps it held back. +One gesture SHALL be one undo step. The SDF, voxel and mask consumers hold the owning document's undo group open from the bind to the close. A mesh gesture records into the caller's delta record. An adaptive gesture continues one `clay_dynamic_delta` across calls and refuses a call whose record the surface has moved past with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. A multiresolution gesture continues one `clay_multires_delta` across calls, capturing both the base half and, with an active sculpt pass, the pass half, so the record equals the one `clay_multires_sculptor_apply_stroke_recorded` makes for the whole path; it refuses a call whose record the hierarchy no longer accepts (another hierarchy, a changed level structure, or a different pass active while the record holds one) with `CLAY_ERROR_SNAPSHOT_MISMATCH`, taking no stamps from the session. A pass made active mid-gesture while the record holds only base entries joins it, and the record then holds both halves. For both, the record is part of the binding, NULL records nothing and stamps what a recorded gesture stamps, and a call that applies nothing (after the close in particular) leaves the record and its binding untouched. Destroying a session whose gesture is open SHALL close the gesture without applying the stamps it held back. The whole-path calls SHALL be unchanged. @@ -44,6 +44,10 @@ The whole-path calls SHALL be unchanged. - **WHEN** another sculpt pass is made active between two calls of a recorded multires gesture - **THEN** the next call returns `CLAY_ERROR_SNAPSHOT_MISMATCH` and applies nothing, and once the first pass is active again the following call applies the held stamps and the gesture ends where the whole-path stroke does +#### Scenario: A call after the close leaves a multires record alone +- **WHEN** a recorded multires gesture has closed, the record is reverted, the top level is removed and added back, and the host makes one more consumer call +- **THEN** that call returns `CLAY_OK` and applies nothing, and the record is still refused by `clay_multires_delta_apply` and `_revert` with `CLAY_ERROR_SNAPSHOT_MISMATCH` + #### Scenario: The binding is checked - **WHEN** a bound session is passed another layer, another item, a mask it was not bound with, another scalar, another kind of target, a NULL brush descriptor, a brush descriptor with another strength, another topology, another frame, or another record than the one it was bound with (NULL included) - **THEN** each call returns `CLAY_ERROR_INVALID_ARGUMENT` and applies nothing diff --git a/openspec/changes/add-stroke-session/tasks.md b/openspec/changes/add-stroke-session/tasks.md index 599b2a8d..a257c123 100644 --- a/openspec/changes/add-stroke-session/tasks.md +++ b/openspec/changes/add-stroke-session/tasks.md @@ -27,3 +27,5 @@ - [x] `clay_multires_sculptor_apply_stroke_tx` takes a nullable `clay_multires_delta* record`, after `defer_normals` as in `clay_multires_sculptor_apply_stroke_recorded`. Changed in place: 0.126.0 is not tagged, so the ABI line stays 0.126.0. The record is part of the binding, is continued across the gesture's calls (both halves), and is checked, with the report, before the session's stamps are taken. - [x] `tests/unit/test_c_multires_delta.cpp`: a session-fed stroke records the whole-path call's record and reverts bit for bit, with and without an active pass; a NULL record stamps the same; the record is part of the binding; a refusal (another pass active, a malformed report) loses no stamps. Mutation check: ignoring the record, checking it after the stamps are taken, dropping it from the binding, and checking the report afterwards each fail a new case. - [x] `docs/05`, `docs/07` § 5 and § 8d. +- [x] Review fix: the multires consumer re-bound its record on every call, including a no-op call after the close, which stamped a stale record with the hierarchy's current structure and let it replay after a level was removed and added back. It now binds only in a call that took stamps. Regression: `c multires delta: a call after a session closes does not re-bind its record` (fails before the fix: `_apply` and `_revert` returned `CLAY_OK` instead of `CLAY_ERROR_SNAPSHOT_MISMATCH`). The adaptive consumer did not have the bug (`DynamicStrokeGesture::apply_recorded` returns before moving the marks when it has no stamps); a subcase in `test_c_stroke_session.cpp` now pins that, and fails when a post-close call is made to re-mark. +- [x] `c multires delta: a pass made active mid-session joins a base-only record`. diff --git a/tests/unit/test_c_multires_delta.cpp b/tests/unit/test_c_multires_delta.cpp index 20cccc9c..25d5c4d1 100644 --- a/tests/unit/test_c_multires_delta.cpp +++ b/tests/unit/test_c_multires_delta.cpp @@ -897,3 +897,83 @@ TEST_CASE( clay_multires_delta_destroy(record); clay_multires_delta_destroy(expected); } + +TEST_CASE("c multires delta: a pass made active mid-session joins a base-only record") { + // The record holds only base entries when the host activates a pass; the + // rest of the gesture writes that pass. The two halves are separate + // storage, so the call is accepted and one revert takes back both. + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + Fixture f(6, 2); + const Snapshot before = snapshot(f.surface); + clay_multires_delta* record = clay_multires_delta_create(); + clay_stroke_tx* tx = session(preset); + REQUIRE(clay_stroke_tx_append(tx, samples.data(), 12, nullptr, nullptr) == CLAY_OK); + size_t n = 0; + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + REQUIRE(n > 0); + REQUIRE(stats_of(record).detail_entries > 0); + REQUIRE(stats_of(record).layer_detail_entries == 0); + + const uint64_t layer = f.add_layer(); + REQUIRE(clay_stroke_tx_append(tx, samples.data() + 12, 12, nullptr, nullptr) == CLAY_OK); + REQUIRE(clay_stroke_tx_end(tx) == CLAY_OK); + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, nullptr, 1, + record, &n, nullptr) == CLAY_OK); + CHECK(n > 0); + clay_stroke_tx_destroy(tx); + const clay_multires_delta_stats s = stats_of(record); + CHECK(s.detail_entries > 0); + CHECK(s.layer_detail_entries > 0); + CHECK(s.sculpt_layer == layer); + // The added pass is empty in `before` and still exists, so a revert puts + // its coefficients back to zero and the base back as it was. + const Snapshot after = snapshot(f.surface); + REQUIRE(clay_multires_delta_revert(record, f.surface) == CLAY_OK); + const Snapshot reverted = snapshot(f.surface); + CHECK(reverted.detail == before.detail); + CHECK(reverted.positions == before.positions); + REQUIRE(clay_multires_delta_apply(record, f.surface) == CLAY_OK); + CHECK(snapshot(f.surface) == after); + clay_multires_delta_destroy(record); +} + +TEST_CASE("c multires delta: a call after a session closes does not re-bind its record") { + // Calling after every append is the intended rhythm, so a host can make a + // call after the gesture closed: it applies nothing and returns CLAY_OK. + // It must not touch the record either. Re-binding there would stamp the + // record with the hierarchy's CURRENT structure, and a record from before + // a level was removed and added back would then be replayed onto a + // hierarchy it does not describe. + const clay_stroke_preset preset = stroke_preset(); + const clay_mesh_brush_desc brush = draw_at(0.0f, 0.0f, 0.4f, 0.3f); + const std::vector samples = session_path(); + for (bool lagging_call : {false, true}) { + CAPTURE(lagging_call); + Fixture f(6, 2); + clay_multires_delta* record = clay_multires_delta_create(); + clay_stroke_tx* tx = session(preset); + size_t calls = 0; + REQUIRE(feed_session(f.sculptor, tx, samples, 4, brush, record, &calls) > 0); + REQUIRE(clay_multires_delta_revert(record, f.surface) == CLAY_OK); + + int32_t err = -1; + REQUIRE(clay_multires_remove_highest_level(f.surface, &err) == CLAY_OK); + REQUIRE(clay_multires_add_level(f.surface, nullptr, &err) == CLAY_OK); + if (lagging_call) { + size_t n = 7; + REQUIRE(clay_multires_sculptor_apply_stroke_tx(f.sculptor, tx, &brush, nullptr, + nullptr, 1, record, &n, + nullptr) == CLAY_OK); + CHECK(n == 0); + } + const Snapshot relevelled = snapshot(f.surface); + CHECK(clay_multires_delta_apply(record, f.surface) == CLAY_ERROR_SNAPSHOT_MISMATCH); + CHECK(clay_multires_delta_revert(record, f.surface) == CLAY_ERROR_SNAPSHOT_MISMATCH); + CHECK(snapshot(f.surface) == relevelled); + clay_stroke_tx_destroy(tx); + clay_multires_delta_destroy(record); + } +} diff --git a/tests/unit/test_c_stroke_session.cpp b/tests/unit/test_c_stroke_session.cpp index b7efb4be..f0a2eedc 100644 --- a/tests/unit/test_c_stroke_session.cpp +++ b/tests/unit/test_c_stroke_session.cpp @@ -864,6 +864,44 @@ TEST_CASE("c stroke session: an adaptive gesture is the whole-path gesture, reco clay_dynamic_sculptor_destroy(sculptor); clay_dynamic_surface_destroy(surface); } + + SUBCASE("a call after the close leaves the record's marks alone") { + // The multires consumer once re-bound its record on a call after the + // close. The adaptive one must not re-mark either: a record that the + // surface has moved past stays refused, however many no-op calls the + // host makes after the gesture ended. + clay_stroke_preset preset = defaults(); + preset.radius = 0.3f; + std::vector samples = path(20, -0.4f, 0.4f, false); + for (clay_stroke_sample_full& s : samples) s.position[2] = 1.0f; + clay_mesh_brush_desc brush = mesh_brush(CLAY_MESH_BRUSH_DRAW, 0.3f, 0.4f); + clay_dynamic_surface* surface = sphere_surface(12); + clay_dynamic_sculptor* sculptor = nullptr; + REQUIRE(clay_dynamic_sculptor_create(surface, &sculptor) == CLAY_OK); + clay_dynamic_delta* record = clay_dynamic_delta_create(); + clay_stroke_tx* tx = begin(preset); + std::size_t applied = 0; + run_session(tx, samples, 5, [&] { + std::size_t n = 0; + REQUIRE(clay_dynamic_sculptor_apply_stroke_tx(sculptor, tx, &brush, nullptr, nullptr, + 0, record, &n, nullptr) == CLAY_OK); + applied += n; + }); + REQUIRE(applied > 0); + clay_mesh_brush_desc stray = brush; + stray.center[2] = 1.0f; + REQUIRE(clay_dynamic_sculptor_stamp(sculptor, &stray, nullptr, nullptr, nullptr) == + CLAY_OK); + std::size_t n = 7; + REQUIRE(clay_dynamic_sculptor_apply_stroke_tx(sculptor, tx, &brush, nullptr, nullptr, 0, + record, &n, nullptr) == CLAY_OK); + CHECK(n == 0); + CHECK(clay_dynamic_delta_revert(record, sculptor) == CLAY_ERROR_SNAPSHOT_MISMATCH); + clay_stroke_tx_destroy(tx); + clay_dynamic_delta_destroy(record); + clay_dynamic_sculptor_destroy(sculptor); + clay_dynamic_surface_destroy(surface); + } } namespace {