diff --git a/CMakeLists.txt b/CMakeLists.txt index 59710e18..0657db08 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -490,6 +490,23 @@ if(CLAY_BUILD_BENCHMARKS) target_include_directories(declared_vs_actual_probe PRIVATE bindings/c) clay_apply_warnings(declared_vs_actual_probe) + # Also not gated: what undoing ONE Move segment costs a host against the + # length of the chain it sits on (issue #639) -- the bricks the undo bound + # marks, and the per-brick price that still grows with the chain. Drives the + # C ABI only, so one source builds against any revision for an A/B. + add_executable(undo_grab_bound_probe benchmarks/undo_grab_bound_probe.cpp) + target_link_libraries(undo_grab_bound_probe PRIVATE claycore) + target_include_directories(undo_grab_bound_probe PRIVATE bindings/c) + clay_apply_warnings(undo_grab_bound_probe) + + # Also not gated: the undo bound's never-tighter property on random documents + # (issue #639) -- refill only the reported bound, compare with a rebuild on + # a copy, and optionally against the raw field. C ABI only. + add_executable(undo_bound_oracle_probe benchmarks/undo_bound_oracle_probe.cpp) + target_link_libraries(undo_bound_oracle_probe PRIVATE claycore) + target_include_directories(undo_bound_oracle_probe PRIVATE bindings/c) + clay_apply_warnings(undo_bound_oracle_probe) + add_executable(grab_jacobian_chain_probe benchmarks/grab_jacobian_chain_probe.cpp) target_link_libraries(grab_jacobian_chain_probe PRIVATE claycore) target_include_directories(grab_jacobian_chain_probe PRIVATE bindings/c) diff --git a/benchmarks/undo_bound_oracle_probe.cpp b/benchmarks/undo_bound_oracle_probe.cpp new file mode 100644 index 00000000..6e0fe9d4 --- /dev/null +++ b/benchmarks/undo_bound_oracle_probe.cpp @@ -0,0 +1,580 @@ +// The never-tighter property of the undo bound, re-derived on random +// documents (issue #639). Drives the C ABI only, so one source builds against +// any revision for an A/B. +// +// Each trial builds a document -- a base layer composed through a smooth or +// subtracting fold, a mirrored or radial layer, nested blended groups, items +// with blends, subtracts, rotations and (with "rich") intersects, per-axis +// scales, rounding, repeats, a moved layer and an instancing layer -- puts a +// chain of grab / magnify / blob links on one node, sometimes ahead of a +// twist, and then takes ONE step: a link added at the front, a link removed, +// or a host Move segment (clay_layer_move_surface). It undoes the step and +// redoes it, and for each direction refills ONLY the reported bound and +// compares every brick's state and fp16 payload with a cache rebuilt from +// nothing on a saved COPY of the document (a same-document rebuild resumes +// from the seeds a too-narrow bound failed to drop, and agrees with it). +// +// undo_bound_oracle_probe FIRST COUNT [rich] +// +// Environment switches: +// RV_RAWBOUND also check the bound against the RAW field (clay_eval_points): +// every sample whose value moved within the band must lie in +// the bound dilated by the band. This separates a bound that is +// too tight from a full rebuild that disagrees with the field. +// RV_RAWCHECK instead compare each document's full brick build with its raw +// field at in-band samples (a property of the engine, not of +// the undo bound). +// RV_FORWARD instead check the FORWARD Move: dirty exactly the regions +// clay_layer_move_surface_regions reports, compare with a rebuild. +// RV_VERBOSE trace the fixture and every stale brick. +// +// Measured with it, see openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/ +// tasks.md ("What review found"). +#include +#include +#include +#include +#include +#include +#include + +#include "clay.h" + +namespace { + +constexpr float kVoxel = 0.05f; +constexpr int kDim = 8; +constexpr std::size_t kSamples = 8 * 8 * 8; +constexpr float kWorld = 2.4f; +bool g_rich = false; +bool g_verbose = false; +bool g_rawbound = false; +size_t g_raw_violations = 0, g_raw_violation_trials = 0; +#define V(...) do { if (g_verbose) std::printf(__VA_ARGS__); } while (0) + +#define OK(x) \ + do { \ + clay_result r_ = (x); \ + if (r_ != CLAY_OK) { \ + std::fprintf(stderr, "FAIL %s -> %d line %d\n", #x, (int)r_, __LINE__); \ + std::exit(2); \ + } \ + } while (0) + +struct Box { + float lo[3] = {0, 0, 0}, hi[3] = {0, 0, 0}; + int32_t has = 0, inf = 0; +}; + +clay_brick_cache* make_cache() { + clay_brick_config cfg; + cfg.struct_size = sizeof cfg; + OK(clay_brick_config_defaults(&cfg)); + cfg.dim = kDim; + cfg.voxel_size = kVoxel; + cfg.band_voxels = 3; + cfg.memory_budget = 0; + return clay_brick_cache_create(&cfg); +} + +size_t refill(clay_brick_cache* c, const clay_document* d) { + const size_t chunk = 64; + std::vector reqs(chunk); + std::vector vals(chunk * kSamples); + std::vector res(chunk); + size_t n = 0; + for (;;) { + size_t count = chunk, rem = 0; + OK(clay_brick_cache_take_dirty(c, reqs.data(), &count, &rem)); + if (!count) break; + OK(clay_brick_cache_eval_requests(d, nullptr, reqs.data(), count, vals.data(), + count * kSamples, nullptr, 0)); + size_t acc = 0; + OK(clay_brick_cache_submit(c, reqs.data(), count, vals.data(), count * kSamples, nullptr, + 0, res.data(), &acc)); + n += count; + if (!rem) break; + } + return n; +} + +std::vector world_keys() { + const float b = kVoxel * kDim; + const int lo = (int)std::floor(-kWorld / b), hi = (int)std::floor(kWorld / b); + std::vector k; + for (int z = lo; z <= hi; ++z) + for (int y = lo; y <= hi; ++y) + for (int x = lo; x <= hi; ++x) { + k.push_back(x); + k.push_back(y); + k.push_back(z); + } + return k; +} + +struct Snap { + std::vector st; + std::vector h; +}; + +Snap snap(clay_brick_cache* c, const std::vector& keys) { + size_t n = keys.size() / 3; + Snap s; + s.st.assign(n, -1); + s.h.assign(n * kSamples, 0); + OK(clay_brick_cache_read_bricks(c, 0, keys.data(), n, 0, s.st.data(), s.h.data(), + n * kSamples, nullptr, 0)); + return s; +} + +size_t diff(const Snap& a, const Snap& b) { + size_t d = 0; + for (size_t i = 0; i < a.st.size(); ++i) + if (a.st[i] != b.st[i] || + std::memcmp(&a.h[i * kSamples], &b.h[i * kSamples], kSamples * 2) != 0) + ++d; + return d; +} + +void mark_world(clay_brick_cache* c) { + const float lo[3] = {-kWorld, -kWorld, -kWorld}, hi[3] = {kWorld, kWorld, kWorld}; + OK(clay_brick_cache_mark_dirty(c, lo, hi)); +} + +Snap rebuilt(clay_document* d, const std::vector& keys) { + clay_blob* blob = nullptr; + OK(clay_document_save_memory(d, &blob)); + clay_document* copy = nullptr; + OK(clay_document_load_memory(clay_blob_data(blob), clay_blob_size(blob), ©)); + clay_blob_destroy(blob); + clay_brick_cache* c = make_cache(); + mark_world(c); + refill(c, copy); + Snap s = snap(c, keys); + clay_brick_cache_destroy(c); + clay_document_destroy(copy); + return s; +} + +struct Rng { + std::mt19937 g; + explicit Rng(unsigned s) : g(s) {} + float u(float a, float b) { return std::uniform_real_distribution(a, b)(g); } + int i(int a, int b) { return std::uniform_int_distribution(a, b)(g); } + bool p(float q) { return u(0, 1) < q; } +}; + +clay_node_id add_item(clay_document* d, clay_layer_id l, Rng& r, clay_node_id group, bool in_group) { + clay_item* it = nullptr; + if (r.p(0.7f)) { + float rad = r.u(0.25f, 0.8f); + it = clay_item_create(CLAY_PRIM_SPHERE, &rad, 1); + } else { + float bx[3] = {r.u(0.2f, 0.6f), r.u(0.2f, 0.6f), r.u(0.2f, 0.6f)}; + it = clay_item_create(CLAY_PRIM_BOX, bx, 3); + } + float pos[3] = {r.u(-0.9f, 0.9f), r.u(-0.9f, 0.9f), r.u(-0.9f, 0.9f)}; + OK(clay_item_set_position(it, pos)); + V("item pos %.3f %.3f %.3f in_group=%d group=%u\n", pos[0], pos[1], pos[2], (int)in_group, (unsigned)group); + if (r.p(0.4f)) { + float ax[3] = {r.u(-1, 1), r.u(-1, 1), r.u(0.1f, 1)}; + OK(clay_item_set_rotation(it, ax, r.u(-2.0f, 2.0f))); + } + if (r.p(0.3f)) OK(clay_item_set_scale(it, r.u(0.7f, 1.3f))); + if (r.p(0.6f)) { float k = r.u(0.05f, 0.3f); V(" blend %.3f\n", k); OK(clay_item_set_blend(it, CLAY_BLEND_QUADRATIC, k)); } + if (r.p(0.15f)) { V(" subtract\n"); OK(clay_item_set_op(it, CLAY_OP_SUBTRACT)); } + if (g_rich) { + if (r.p(0.3f)) { + float sc[3] = {r.u(0.6f, 1.4f), r.u(0.6f, 1.4f), r.u(0.6f, 1.4f)}; + OK(clay_item_set_scale_nonuniform(it, sc)); + } + if (r.p(0.3f)) OK(clay_item_set_rounding(it, r.u(0.0f, 0.08f))); + if (r.p(0.1f)) OK(clay_item_set_op(it, CLAY_OP_INTERSECT)); + if (r.p(0.15f)) OK(clay_item_set_repeat_radial(it, r.i(2, 4), r.u(0.2f, 0.6f))); + else if (r.p(0.1f)) { + const float s1 = r.u(0.6f, 1.0f); float sp[3] = {s1, s1, s1}; + float cn[3] = {1, 0, 1}; + OK(clay_item_set_repeat_grid(it, sp, cn)); + } + if (r.p(0.15f)) OK(clay_item_set_mirror(it, 0)); + } + clay_node_id n = 0; + if (in_group) + OK(clay_layer_add_item_in_group(d, l, group, -1, it, &n)); + else + OK(clay_layer_add_item(d, l, it, &n)); + clay_item_destroy(it); + return n; +} + +// A random finite-support link at the front; params in the node's frame. +void add_head_link(clay_document* d, clay_layer_id l, clay_node_id n, Rng& r, float kind_bias) { + const float c[3] = {r.u(-0.9f, 0.9f), r.u(-0.9f, 0.9f), r.u(-0.9f, 0.9f)}; + const float rad = r.u(0.08f, 0.5f); + const float k = r.u(0, 1); + const int32_t ease = r.p(0.2f) ? 1 : 0; + V("link node=%u c=%.3f %.3f %.3f r=%.3f kind=%.2f<%.2f ease=%d\n", (unsigned)n, c[0], c[1], c[2], rad, k, kind_bias, ease); + if (k < kind_bias) { + float pr[8] = {c[0], c[1], c[2], rad, r.u(-0.4f, 0.4f), r.u(-0.4f, 0.4f), + r.u(-0.4f, 0.4f), r.p(0.3f) ? 1.0f : 0.0f}; + V(" grab pull %.3f %.3f %.3f front %.0f\n", pr[4], pr[5], pr[6], pr[7]); + OK(clay_layer_add_deformer(d, l, n, CLAY_DEFORM_GRAB, pr, 8, ease, 1)); + } else if (k < (1 + kind_bias) / 2) { + float pr[5] = {c[0], c[1], c[2], rad, r.u(-0.6f, 0.6f)}; + OK(clay_layer_add_deformer(d, l, n, CLAY_DEFORM_MAGNIFY, pr, 5, ease, 1)); + } else { + float pr[9] = {c[0], c[1], c[2], rad, r.u(0.02f, 0.1f), 6.0f, 3.0f, 0.5f, 7.0f}; + OK(clay_layer_add_deformer(d, l, n, CLAY_DEFORM_BLOB, pr, 9, ease, 1)); + } +} + +struct Trial { + clay_document* d = nullptr; + clay_layer_id layer = 0; + clay_node_id target = 0; +}; + +Trial build(unsigned seed, Rng& r) { + (void)seed; + Trial t; + t.d = clay_document_create(); + OK(clay_add_sdf_layer(t.d, "body", &t.layer)); + OK(clay_document_enable_undo(t.d)); + if (r.p(0.5f)) { // a base layer under, composed through a fold + clay_layer_id base = 0; + OK(clay_add_sdf_layer(t.d, "base", &base)); + OK(clay_document_move_layer(t.d, base, 0)); + const int32_t op = r.p(0.7f) ? CLAY_OP_ADD : CLAY_OP_SUBTRACT; + V("BASE LAYER op=%d (items below are base until 'body')\n", op); + OK(clay_document_set_layer_composition(t.d, t.layer, op, CLAY_BLEND_QUADRATIC, + r.u(0.05f, 0.3f), 0.0f)); + for (int i = 0, n = r.i(1, 2); i < n; ++i) add_item(t.d, base, r, 0, false); + } + V("body\n"); + if (r.p(0.4f)) { + const int mx = r.p(0.7f), my = r.p(0.3f), mz = r.p(0.3f); + const float mk = r.u(0.0f, 0.2f); + V("mirror %d %d %d k=%.3f\n", mx, my, mz, mk); + OK(clay_set_layer_mirror(t.d, t.layer, mx, my, mz, mk)); + } else if (r.p(0.2f)) { + const int ax = r.i(0, 2), cnt = r.i(2, 5); + const float rk = r.u(0.0f, 0.2f); + V("radial axis %d count %d k=%.3f\n", ax, cnt, rk); + OK(clay_set_layer_radial(t.d, t.layer, ax, cnt, rk)); + } + if (g_rich && r.p(0.35f)) { + const float pos[3] = {r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f)}; + const float ax[3] = {r.u(-1, 1), r.u(0.1f, 1), r.u(-1, 1)}; + const float sc[3] = {r.u(0.7f, 1.3f), r.u(0.7f, 1.3f), r.u(0.7f, 1.3f)}; + OK(clay_document_set_layer_transform_nonuniform(t.d, t.layer, pos, ax, r.u(-1.5f, 1.5f), sc)); + } + clay_node_id group = 0; + const bool use_group = r.p(0.5f); + if (use_group) { + clay_node_id parent = 0; + if (r.p(0.4f)) { // nested group + OK(clay_layer_add_group(t.d, t.layer, 0, -1, CLAY_OP_ADD, CLAY_BLEND_QUADRATIC, + r.u(0.05f, 0.3f), 0.0f, &parent)); + add_item(t.d, t.layer, r, parent, true); + } + OK(clay_layer_add_group(t.d, t.layer, parent, -1, CLAY_OP_ADD, CLAY_BLEND_QUADRATIC, + r.u(0.05f, 0.3f), r.p(0.3f) ? r.u(0.0f, 0.05f) : 0.0f, &group)); + } + std::vector nodes; + for (int i = 0, n = r.i(2, 4); i < n; ++i) + nodes.push_back(add_item(t.d, t.layer, r, group, use_group && r.p(0.7f))); + t.target = nodes[r.i(0, (int)nodes.size() - 1)]; + V("target %u\n", (unsigned)t.target); + // A tail: sometimes a twist behind everything, so head links sit ahead of it. + if (r.p(0.25f)) { + float k = r.u(-1.0f, 1.0f); + OK(clay_layer_add_deformer(t.d, t.layer, t.target, CLAY_DEFORM_TWIST, &k, 1, 0, 0)); + } + for (int i = 0, n = r.i(0, 8); i < n; ++i) add_head_link(t.d, t.layer, t.target, r, 0.7f); + if (g_rich && r.p(0.3f)) { + clay_layer_id inst = 0; + OK(clay_document_instance_layer(t.d, t.layer, "inst", &inst)); + const float pos[3] = {r.u(-0.8f, 0.8f), r.u(-0.8f, 0.8f), r.u(-0.8f, 0.8f)}; + const float ax[3] = {0, 1, 0}; + OK(clay_document_set_layer_transform(t.d, inst, pos, ax, r.u(-1.5f, 1.5f), r.u(0.7f, 1.2f))); + } + return t; +} + +// The step under test, recorded on the undo stack. +int do_step(Trial& t, Rng& r) { + const int kind = r.i(0, 3); + if (kind == 0 || kind == 1) { + add_head_link(t.d, t.layer, t.target, r, 0.8f); + return 0; + } + if (kind == 2) { // remove a link at a random index (0 is the head) + add_head_link(t.d, t.layer, t.target, r, 0.8f); + if (clay_layer_remove_deformer(t.d, t.layer, t.target, (size_t)r.i(0, 1)) != CLAY_OK) + OK(clay_layer_remove_deformer(t.d, t.layer, t.target, 0)); + return 2; + } + // A host Move segment through the surface-level gesture. + clay_move_params p; + std::memset(&p, 0, sizeof p); + p.struct_size = sizeof p; + p.radius = r.u(0.1f, 0.4f); + const float c[3] = {r.u(-1, 1), r.u(-1, 1), r.u(-1, 1)}; + const float pull[3] = {r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f)}; + size_t applied = 0; + const clay_result mr = clay_layer_move_surface(t.d, t.layer, c, pull, &p, &applied); + V("move c=%.3f %.3f %.3f pull %.3f %.3f %.3f r=%.3f applied=%zu rc=%d\n", c[0], c[1], c[2], pull[0], pull[1], pull[2], p.radius, applied, (int)mr); + if (mr != CLAY_OK || applied == 0) { + add_head_link(t.d, t.layer, t.target, r, 1.0f); + return 0; + } + return 3; +} + +Box node_bound(Trial& t) { + Box b; + OK(clay_layer_node_influence_bound(t.d, t.layer, t.target, b.lo, b.hi, &b.has, &b.inf)); + return b; +} + +bool contains(const Box& outer, const Box& inner) { + if (!inner.has) return true; + if (!outer.has) return false; + if (outer.inf) return true; + if (inner.inf) return false; + for (int a = 0; a < 3; ++a) + if (inner.lo[a] < outer.lo[a] - 1e-6f || inner.hi[a] > outer.hi[a] + 1e-6f) return false; + return true; +} + +Box join(const Box& a, const Box& b) { + if (!a.has) return b; + if (!b.has) return a; + Box o = a; + o.inf = a.inf || b.inf; + for (int i = 0; i < 3; ++i) { + o.lo[i] = std::fmin(a.lo[i], b.lo[i]); + o.hi[i] = std::fmax(a.hi[i], b.hi[i]); + } + return o; +} + +struct Stats { + size_t trials = 0, stale_trials = 0, larger = 0, visible = 0, narrowed = 0; + size_t refilled = 0, node_refill = 0; +}; + +// One direction: returns stale bricks. +std::vector brick_points(const std::vector& keys) { + std::vector pts; + const float b = kVoxel * kDim; + for (size_t i = 0; i < keys.size() / 3; ++i) + for (int z = 0; z < kDim; ++z) + for (int y = 0; y < kDim; ++y) + for (int x = 0; x < kDim; ++x) { + pts.push_back(keys[3 * i] * b + x * kVoxel); + pts.push_back(keys[3 * i + 1] * b + y * kVoxel); + pts.push_back(keys[3 * i + 2] * b + z * kVoxel); + } + return pts; +} + +std::vector eval_all(clay_document* d, const std::vector& pts) { + std::vector out(pts.size() / 3); + OK(clay_eval_points(d, nullptr, pts.data(), out.size(), out.data(), nullptr)); + return out; +} + +float half_to_float(uint16_t h) { + const int e = (h >> 10) & 31, m = h & 1023; + float v = e == 0 ? std::ldexp((float)m, -24) : std::ldexp(1.0f + m / 1024.0f, e - 15); + return (h & 0x8000) ? -v : v; +} + +size_t check_dir(Trial& t, bool undo, const std::vector& keys, Stats& s, unsigned seed, + int kind) { + clay_brick_cache* kept = make_cache(); + mark_world(kept); + refill(kept, t.d); + const Snap before = snap(kept, keys); + std::vector pts, raw_before; + if (g_verbose || g_rawbound) { pts = brick_points(keys); raw_before = eval_all(t.d, pts); } + const Box nb_before = node_bound(t); + Box b; + int32_t done = 0; + if (undo) + OK(clay_document_undo_bound(t.d, &done, b.lo, b.hi, &b.has, &b.inf)); + else + OK(clay_document_redo_bound(t.d, &done, b.lo, b.hi, &b.has, &b.inf)); + const Box nb_after = node_bound(t); + V("node bound before [%.3f %.3f %.3f]-[%.3f %.3f %.3f] has=%d inf=%d\n", nb_before.lo[0], nb_before.lo[1], nb_before.lo[2], nb_before.hi[0], nb_before.hi[1], nb_before.hi[2], nb_before.has, nb_before.inf); + V("node bound after [%.3f %.3f %.3f]-[%.3f %.3f %.3f] has=%d inf=%d\n", nb_after.lo[0], nb_after.lo[1], nb_after.lo[2], nb_after.hi[0], nb_after.hi[1], nb_after.hi[2], nb_after.has, nb_after.inf); + // Node-bound-of-target (what main reported for a pure single-node step). + const Box nb = join(nb_before, nb_after); + if (kind != 3 && !contains(nb, b)) { + ++s.larger; + std::printf("seed %u %s: bound LARGER than node bound\n", seed, undo ? "undo" : "redo"); + } + if (g_rawbound) { + // THE DIRECT CHECK: every sample whose raw value moved, and is within + // the band on either side, lies inside the reported bound dilated by + // the band (what mark_dirty adds). + const std::vector raw_after = eval_all(t.d, pts); + const float band = 0.15f, eps = 1e-4f; + size_t v = 0; + for (size_t i = 0; i < raw_after.size(); ++i) { + if (raw_after[i] == raw_before[i]) continue; + if (std::fabs(raw_after[i]) >= band && std::fabs(raw_before[i]) >= band) continue; + bool inside = b.has && (b.inf || true); + if (b.has && !b.inf) + for (int a = 0; a < 3; ++a) + if (pts[3 * i + a] < b.lo[a] - band - eps || pts[3 * i + a] > b.hi[a] + band + eps) inside = false; + if (!inside) { + if (v < 3) std::printf("seed %u %s: raw moved OUTSIDE bound+band at %.3f %.3f %.3f (%.5f -> %.5f)\n", seed, undo ? "undo" : "redo", pts[3*i], pts[3*i+1], pts[3*i+2], raw_before[i], raw_after[i]); + ++v; + } + } + g_raw_violations += v; + if (v) ++g_raw_violation_trials; + } + if (b.has && !b.inf) OK(clay_brick_cache_mark_dirty(kept, b.lo, b.hi)); + if (b.has && b.inf) mark_world(kept); + s.refilled += refill(kept, t.d); + const Snap got = snap(kept, keys); + const Snap want = rebuilt(t.d, keys); + const size_t stale = diff(got, want); + if (g_verbose && stale) { + const std::vector raw_after = eval_all(t.d, pts); + size_t moved = 0, moved_band = 0; float lo[3] = {1e9f, 1e9f, 1e9f}, hi[3] = {-1e9f, -1e9f, -1e9f}; + for (size_t i = 0; i < raw_after.size(); ++i) + if (raw_after[i] != raw_before[i]) { + ++moved; + if (std::fabs(raw_after[i]) < 0.16f || std::fabs(raw_before[i]) < 0.16f) { + ++moved_band; + for (int a = 0; a < 3; ++a) { lo[a] = std::fmin(lo[a], pts[3*i+a]); hi[a] = std::fmax(hi[a], pts[3*i+a]); } + } + } + V(" raw field moved at %zu samples, %zu in band, band-moved box [%.3f %.3f %.3f]-[%.3f %.3f %.3f]\n", moved, moved_band, lo[0], lo[1], lo[2], hi[0], hi[1], hi[2]); + } + if (g_verbose) + for (size_t i = 0; i < got.st.size(); ++i) + if (got.st[i] != want.st[i] || std::memcmp(&got.h[i * kSamples], &want.h[i * kSamples], kSamples * 2) != 0) { + int maxd = 0; float gv = 0, wv = 0; size_t mj = 0; + for (size_t j = 0; j < kSamples; ++j) { int d = std::abs((int)got.h[i*kSamples+j] - (int)want.h[i*kSamples+j]); if (d > maxd) { maxd = d; gv = half_to_float(got.h[i*kSamples+j]); wv = half_to_float(want.h[i*kSamples+j]); mj = j; } } + if (!raw_before.empty()) { + const std::vector ra = eval_all(t.d, std::vector(pts.begin() + 3 * (i * kSamples + mj), pts.begin() + 3 * (i * kSamples + mj) + 3)); + V(" sample %zu: kept %.5f rebuilt %.5f raw_before %.5f raw_after %.5f\n", mj, gv, wv, raw_before[i * kSamples + mj], ra[0]); + } + V(" stale key %d %d %d state %d vs %d max half-diff %d (%g vs %g raw)\n", keys[3*i], keys[3*i+1], keys[3*i+2], got.st[i], want.st[i], maxd, gv, wv); + } + if (diff(before, want) > 0) ++s.visible; + if (kind != 3 && contains(nb, b) && !(contains(b, nb))) ++s.narrowed; + if (stale) + std::printf("seed %u kind %d %s: STALE %zu bricks bound=[%.3f %.3f %.3f]-[%.3f %.3f %.3f] has=%d inf=%d\n", + seed, kind, undo ? "undo" : "redo", stale, b.lo[0], b.lo[1], b.lo[2], b.hi[0], + b.hi[1], b.hi[2], b.has, b.inf); + clay_brick_cache_destroy(kept); + return stale; +} + +} // namespace + +// The FORWARD Move's own oracle: build the trial, and instead of the random +// step do the kind-3 move with clay_layer_move_surface_regions, dirty exactly +// the regions it reports, and compare with a rebuild. +size_t forward_move_check(unsigned seed, const std::vector& keys) { + Rng r(seed); + Trial t = build(seed, r); + const int kind = r.i(0, 3); + if (kind != 3) { clay_document_destroy(t.d); return 0; } + clay_brick_cache* kept = make_cache(); + mark_world(kept); + refill(kept, t.d); + clay_move_params p; + std::memset(&p, 0, sizeof p); + p.struct_size = sizeof p; + p.radius = r.u(0.1f, 0.4f); + const float c[3] = {r.u(-1, 1), r.u(-1, 1), r.u(-1, 1)}; + const float pull[3] = {r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f), r.u(-0.3f, 0.3f)}; + size_t applied = 0, nbox = 0; + std::vector boxes(6 * 64); + if (clay_layer_move_surface_regions(t.d, t.layer, c, pull, &p, &applied, boxes.data(), 64, &nbox) != CLAY_OK || applied == 0) { + clay_brick_cache_destroy(kept); clay_document_destroy(t.d); return 0; + } + for (size_t i = 0; i < nbox; ++i) OK(clay_brick_cache_mark_dirty(kept, &boxes[6 * i], &boxes[6 * i + 3])); + refill(kept, t.d); + const size_t stale = diff(snap(kept, keys), rebuilt(t.d, keys)); + std::printf("forward seed %u applied %zu boxes %zu: stale %zu\n", seed, applied, nbox, stale); + clay_brick_cache_destroy(kept); + clay_document_destroy(t.d); + return stale; +} + +// A document's full brick build against its raw field, at in-band samples. +float build_vs_raw(clay_document* d, const std::vector& keys, size_t* bad) { + clay_brick_cache* c = make_cache(); + mark_world(c); + refill(c, d); + const Snap s = snap(c, keys); + const std::vector pts = brick_points(keys); + const std::vector raw = eval_all(d, pts); + float worst = 0; + *bad = 0; + for (size_t i = 0; i < raw.size(); ++i) { + if (s.st[i / kSamples] != 2 || std::fabs(raw[i]) > 0.12f) continue; + const float e = std::fabs(half_to_float(s.h[i]) - raw[i]); + if (e > 2e-3f) ++*bad; + worst = std::fmax(worst, e); + } + clay_brick_cache_destroy(c); + return worst; +} + +int main(int argc, char** argv) { + if (std::getenv("RV_RAWCHECK")) { + g_rich = argc > 3 && argv[3][0] == 'r'; + const std::vector keys = world_keys(); + const unsigned first = (unsigned)std::atoi(argv[1]), count = (unsigned)std::atoi(argv[2]); + size_t docs_bad = 0; + for (unsigned seed = first; seed < first + count; ++seed) { + Rng r(seed); + Trial t = build(seed, r); + size_t bad = 0; + const float w = build_vs_raw(t.d, keys, &bad); + if (bad) { ++docs_bad; std::printf("seed %u: %zu in-band samples off by > 2e-3, worst %.5f\n", seed, bad, w); } + clay_document_destroy(t.d); + } + std::printf("raw check: %zu of %u documents\n", docs_bad, count); + return 0; + } + if (std::getenv("RV_FORWARD")) { + g_rich = argc > 3 && argv[3][0] == 'r'; + const std::vector keys = world_keys(); + const unsigned first = (unsigned)std::atoi(argv[1]), count = (unsigned)std::atoi(argv[2]); + size_t bad = 0; + for (unsigned s = first; s < first + count; ++s) bad += forward_move_check(s, keys) ? 1 : 0; + std::printf("forward stale trials %zu\n", bad); + return 0; + } + const unsigned first = argc > 1 ? (unsigned)std::atoi(argv[1]) : 1; + const unsigned count = argc > 2 ? (unsigned)std::atoi(argv[2]) : 50; + g_rich = argc > 3 && argv[3][0] == 'r'; + g_verbose = std::getenv("RV_VERBOSE") != nullptr; + g_rawbound = std::getenv("RV_RAWBOUND") != nullptr; + const std::vector keys = world_keys(); + Stats s; + for (unsigned seed = first; seed < first + count; ++seed) { + Rng r(seed); + Trial t = build(seed, r); + const int kind = do_step(t, r); + const size_t su = check_dir(t, true, keys, s, seed, kind); + const size_t sr = check_dir(t, false, keys, s, seed, kind); + ++s.trials; + if (su || sr) ++s.stale_trials; + clay_document_destroy(t.d); + } + if (g_rawbound) std::printf("raw-bound violations: %zu samples in %zu directions\n", g_raw_violations, g_raw_violation_trials); + std::printf("trials=%zu stale_trials=%zu larger_than_node=%zu edit_visible_dirs=%zu narrowed_dirs=%zu refilled=%zu\n", + s.trials, s.stale_trials, s.larger, s.visible, s.narrowed, s.refilled); + return s.stale_trials || s.larger ? 1 : 0; +} diff --git a/benchmarks/undo_grab_bound_probe.cpp b/benchmarks/undo_grab_bound_probe.cpp new file mode 100644 index 00000000..7cd11c1f --- /dev/null +++ b/benchmarks/undo_grab_bound_probe.cpp @@ -0,0 +1,152 @@ +// What undoing ONE Move segment costs a host, against the length of the chain +// it sits on (issue #639). NOT a gated benchmark. +// +// THE SHAPE. A sphere node, `chain` grabs spread around its equator -- what a +// long Move session leaves on a node -- and one small grab on its pole, put at +// the HEAD of the chain as a Move puts it. The probe undoes that one grab and +// does what the header tells a host to: hand clay_document_undo_bound's box to +// clay_brick_cache_mark_dirty and refill what it marked. Then it redoes, and +// refills again, so every sample starts from the same document. +// +// WHAT IT REPORTS, per run: the bricks the undo marked -- a COUNT, the same on +// every machine, and the thing issue #639 is about -- and the median wall time +// of the undo (undo_bound + mark + refill) over `samples` repeats after five +// discarded ones, with that time divided by the brick count. The first is the +// node-size factor, the last is the per-brick price, which still grows with +// the chain after this change and is reported so that it is not hidden. +// +// Drives the C ABI only, so one source builds against any revision: +// +// undo_grab_bound_probe [chain=10] [samples=200] [node_radius=1.5] +// +// Exits non-zero if the undo does not report a finite box, so it cannot +// measure a refill of nothing. + +#include +#include +#include +#include +#include +#include +#include + +#include "clay.h" + +namespace { + +constexpr int kDim = 8; +constexpr std::size_t kSamples = 8 * 8 * 8; + +void check(clay_result r, const char* what) { + if (r == CLAY_OK) return; + std::fprintf(stderr, "%s failed: %d %s\n", what, static_cast(r), clay_last_error()); + std::exit(1); +} + +struct Refill { + std::vector reqs = std::vector(256); + std::vector values = std::vector(256 * kSamples); + std::vector results = std::vector(256); + + // Drain, evaluate, submit until nothing is dirty; returns bricks evaluated. + std::size_t run(clay_brick_cache* cache, const clay_document* doc) { + std::size_t evaluated = 0; + for (;;) { + std::size_t count = reqs.size(), remaining = 0; + check(clay_brick_cache_take_dirty(cache, reqs.data(), &count, &remaining), "take"); + if (count == 0) break; + check(clay_brick_cache_eval_requests(doc, nullptr, reqs.data(), count, values.data(), + count * kSamples, nullptr, 0), + "eval"); + std::size_t accepted = 0; + check(clay_brick_cache_submit(cache, reqs.data(), count, values.data(), + count * kSamples, nullptr, 0, results.data(), &accepted), + "submit"); + evaluated += count; + if (remaining == 0) break; + } + return evaluated; + } +}; + +void add_grab(clay_document* d, clay_layer_id layer, clay_node_id node, const float p[8]) { + check(clay_layer_add_deformer(d, layer, node, CLAY_DEFORM_GRAB, p, 8, CLAY_EASE_LINEAR, 1), + "add grab"); +} + +// The document: a sphere, `chain` grabs around its equator, one on its pole. +clay_document* build(int chain, float node_r, clay_layer_id* out_layer) { + clay_document* d = clay_document_create(); + check(clay_add_sdf_layer(d, "body", out_layer), "layer"); + check(clay_document_enable_undo(d), "undo"); + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, &node_r, 1); + clay_node_id node = 0; + check(clay_layer_add_item(d, *out_layer, item, &node), "item"); + clay_item_destroy(item); + for (int i = 0; i < chain; ++i) { + const float t = 6.2831853f * static_cast(i) / static_cast(chain); + const float c = std::cos(t), s = std::sin(t); + const float p[8] = {node_r * c, node_r * s, 0.0f, 0.2f, 0.08f * c, 0.08f * s, 0.0f, 0.0f}; + add_grab(d, *out_layer, node, p); + } + const float pole[8] = {0.0f, 0.0f, node_r, 0.15f, 0.0f, 0.0f, 0.08f, 0.0f}; + add_grab(d, *out_layer, node, pole); + return d; +} + +// One step through the reporting entry point, marked and refilled. +std::size_t step(clay_document* d, clay_brick_cache* cache, Refill* refill, bool undo) { + float lo[3], hi[3]; + std::int32_t done = 0, has = 0, inf = 0; + check(undo ? clay_document_undo_bound(d, &done, lo, hi, &has, &inf) + : clay_document_redo_bound(d, &done, lo, hi, &has, &inf), + undo ? "undo" : "redo"); + if (!done || !has || inf) { + std::fprintf(stderr, "the step reported no finite box\n"); + std::exit(1); + } + check(clay_brick_cache_mark_dirty(cache, lo, hi), "mark"); + return refill->run(cache, d); +} + +double median(std::vector v) { + std::sort(v.begin(), v.end()); + return v[v.size() / 2]; +} + +} // namespace + +int main(int argc, char** argv) { + const int chain = argc > 1 ? std::atoi(argv[1]) : 10; + const int samples = argc > 2 ? std::max(1, std::atoi(argv[2])) : 200; + const float node_r = argc > 3 ? static_cast(std::atof(argv[3])) : 1.5f; + + clay_layer_id layer = 0; + clay_document* d = build(chain, node_r, &layer); + clay_brick_config cfg; + cfg.struct_size = sizeof cfg; + check(clay_brick_config_defaults(&cfg), "config"); + cfg.dim = kDim; + cfg.voxel_size = 0.05f; + clay_brick_cache* cache = clay_brick_cache_create(&cfg); + Refill refill; + check(clay_brick_cache_mark_dirty_layer(cache, d, layer), "mark layer"); + refill.run(cache, d); + + std::vector undo_ms; + std::size_t bricks = 0; + for (int i = 0; i < samples + 5; ++i) { + const auto t0 = std::chrono::steady_clock::now(); + bricks = step(d, cache, &refill, /*undo=*/true); + const auto t1 = std::chrono::steady_clock::now(); + if (i >= 5) undo_ms.push_back(std::chrono::duration(t1 - t0).count()); + step(d, cache, &refill, /*undo=*/false); + } + const double p50 = median(undo_ms); + std::printf("chain=%d node_r=%.2f samples=%d bricks=%zu undo_p50_ms=%.3f per_brick_us=%.2f\n", + chain, static_cast(node_r), samples, bricks, p50, + 1000.0 * p50 / static_cast(bricks)); + clay_brick_cache_destroy(cache); + clay_document_destroy(d); + return 0; +} diff --git a/bindings/c/clay.h b/bindings/c/clay.h index dbf44b7d..029d1552 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -1279,6 +1279,38 @@ clay_result clay_document_redo(clay_document* doc, int32_t* out_redone); * layer sharing it. A step that cannot change the field — a rename — reports * no bounds rather than the layer. * + * A DEFORMER STEP REPORTS THE DEFORMER, NOT ITS NODE (issue #639). A Move + * segment is a grab put at the head of a node's chain, and through 0.120.0 its + * undo reported the node's whole bound — which every grab also dilates by its + * pull — so undoing one segment cost bricks-in-the-node times chain length. + * When the chains before and after a command differ only in a HEAD of grab, + * magnify, blob or alpha links, that command now reports those links' balls + * (a grab's at its centre AND at its displaced end), placed where the item is + * — every mirror and radial copy, every instancing layer — dilated by each + * enclosing group's blend support and by every layer fold above, and clamped + * into the node's bound, so it is never larger than what it replaces. CLAMPED, + * NOT INTERSECTED: the node's bound leaves the band to you (mark_dirty adds + * it), so a ball that misses the node's box by less than a band still changes + * the field, and reports the face of that box nearest it rather than nothing. + * Those links are exactly the identity outside their balls, so the raw field + * there is bit-identical and nothing outside needs a refill. Measured on a radius-1.5 + * node, 0.05 voxels, one grab of radius 0.15 undone: 12 bricks at every chain + * length, against 1,000 / 1,440 / 4,000 at 1 / 10 / 40 grabs before. + * + * WHAT IT DOES NOT NARROW, and reports the node's bound for as before: a + * RADIAL POSE (its kernel is not the identity past its ball, by an ulp); a + * grab behind a twist or any other whole-item deformer, whose ball that + * deformer has moved — a grab ADDED AT THE FRONT of such a chain narrows, one + * appended behind it does not; an easing whose value at the rim is not exactly + * zero on every backend (the sines, out_expo, the circs, in_bounce, + * in_out_bounce); an infinite repeat grid; a morph or hidden group above the + * item. Other commands in the same step report their own bounds as always. + * + * WHAT IT DOES NOT MAKE CHEAP: the price of ONE refilled brick still rises + * with the chain's length -- on the fixture above, 7.4 us a brick at 1 grab and + * 28.1 us at 160, none of which reach those bricks. This removes the node-size + * factor, not that one. + * * The three states are clay_layer_node_influence_bound's, and they line up * with what mark_dirty takes: * *out_has_bounds 0 nothing to dirty; out_min/out_max untouched diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 7b19fb03..0abf4c9a 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -3055,7 +3055,7 @@ Authoring reaches what `pyclay` reaches: an opaque **item builder** (`clay_item_ Two consequences worth knowing at the call. Both C transform setters write the WHOLE transform, so `clay_layer_set_transform` means "this node's scale is uniform s" and **collapses** a per-axis one; and `clay_layer_node_transform`, the single-factor reader from 0.53.0, **refuses** a node carrying a non-uniform scale rather than reporting one of the three — the same rule #317 was filed over, that a reader which cannot express what is there must not answer. pyclay needs neither: `scale=` takes one number or three in the same argument, and its partial updates leave a squash alone unless you name it. A LAYER's scale stayed uniform at 0.54.0 — `layer.xform * node.xform` is consumed as a rigid frame by the move brush and the lattice gizmo, and what those should do with a per-axis one was deferred as its own question. **#373 answered it in 0.78.0**: `Layer::scale_axes` composes innermost in the layer's own frame, `brush::lattice_gizmo` refuses rather than approximating on a squashed layer, and the scene and `.clayspace` formats move to minor 16. See [Scaling a subtool per axis](#scaling-a-subtool-per-axis). -**The incremental path is reachable too** (`clay_brick_cache_*`, ABI minor 24): the brick cache of §8 is an opaque handle created from a `clay_brick_config` and never bound to a document, with exactly one refill loop — `mark_dirty` → `take_dirty` → `eval_requests` → `submit` — plus brick readback in the stored fp16 bits at a fixed `dim³` stride, statistics, LOD mips, `mesh_bricks` and `raycast_bricks`. The mip is meshable as well as readable (`clay_brick_cache_mesh_lod`): a coarse brick is the cache's own lattice at twice the spacing, so a level changes the spacing and the meaning of the key list and nothing else — and because an empty mesh already means "no surface bricks", a level nobody built is `CLAY_ERROR_NOT_FOUND` rather than an empty one. It mirrors `brick::BrickCache` member for member and adds no policy: no owned thread, no lock, no refill driver, no ordering or eviction knob, because the consumer owns queues and scheduling. (One batched const query, `clay_brick_cache_raycast_many`, fans its rays out across the engine's shared worker pool — the same pool the CPU backend evaluates points with — and returns only when every ray is done; each slot is byte-identical to the single-ray call's answer, so this is a latency detail, not a policy.) A request is a plain value whose layout *is* `brick::BrickRequest` (asserted with `offsetof`), so a drain is a `memcpy` and a request may be queued, copied and evaluated on any thread. It carries its lattice *and its band*, because the tape must be culled against the brick dilated by the band — a sample keeps its true distance whenever that distance is within the band, so an item a band outside the brick still decides samples inside it, and a band passed separately is a band that will one day arrive as zero. Three supporting calls come with it, none of which existed before: `clay_eval_grid` (dense lattice evaluation with an optional cull region — the per-brick fill primitive), and `clay_layer_node_influence_bound` / `clay_layer_influence_bound`, which report the box an edit must *dirty* (wider than `clay_layer_bounds`, and flagged when unbounded). Undo joins them at ABI minor 40: `clay_document_undo_bound` / `clay_document_redo_bound` are the plain undo and redo plus the influence bound of what the step applied, in the same three-state shape, because the engine holds the list of commands it replayed and a host outside it can only guess — the layer, or a node diff that misses every in-place edit. Marking is validated in 64-bit before the engine converts the region: a span above `CLAY_MAX_BATCH` or a brick coordinate outside `int32` is refused with the cache untouched, since three caller floats can otherwise name 1019 bricks. +**The incremental path is reachable too** (`clay_brick_cache_*`, ABI minor 24): the brick cache of §8 is an opaque handle created from a `clay_brick_config` and never bound to a document, with exactly one refill loop — `mark_dirty` → `take_dirty` → `eval_requests` → `submit` — plus brick readback in the stored fp16 bits at a fixed `dim³` stride, statistics, LOD mips, `mesh_bricks` and `raycast_bricks`. The mip is meshable as well as readable (`clay_brick_cache_mesh_lod`): a coarse brick is the cache's own lattice at twice the spacing, so a level changes the spacing and the meaning of the key list and nothing else — and because an empty mesh already means "no surface bricks", a level nobody built is `CLAY_ERROR_NOT_FOUND` rather than an empty one. It mirrors `brick::BrickCache` member for member and adds no policy: no owned thread, no lock, no refill driver, no ordering or eviction knob, because the consumer owns queues and scheduling. (One batched const query, `clay_brick_cache_raycast_many`, fans its rays out across the engine's shared worker pool — the same pool the CPU backend evaluates points with — and returns only when every ray is done; each slot is byte-identical to the single-ray call's answer, so this is a latency detail, not a policy.) A request is a plain value whose layout *is* `brick::BrickRequest` (asserted with `offsetof`), so a drain is a `memcpy` and a request may be queued, copied and evaluated on any thread. It carries its lattice *and its band*, because the tape must be culled against the brick dilated by the band — a sample keeps its true distance whenever that distance is within the band, so an item a band outside the brick still decides samples inside it, and a band passed separately is a band that will one day arrive as zero. Three supporting calls come with it, none of which existed before: `clay_eval_grid` (dense lattice evaluation with an optional cull region — the per-brick fill primitive), and `clay_layer_node_influence_bound` / `clay_layer_influence_bound`, which report the box an edit must *dirty* (wider than `clay_layer_bounds`, and flagged when unbounded). Undo joins them at ABI minor 40: `clay_document_undo_bound` / `clay_document_redo_bound` are the plain undo and redo plus the influence bound of what the step applied, in the same three-state shape, because the engine holds the list of commands it replayed and a host outside it can only guess — the layer, or a node diff that misses every in-place edit. A step whose deformer chains differ only in a head of finite-support links — a Move segment's grabs, magnify, blob, alpha — reports those links' balls (a grab's at both ends), placed through every symmetry copy and instancing layer, dilated by the enclosing groups' blend supports and the layer folds above, and clamped into the node's bound — per axis the overlap, or the face nearest the ball where they miss, because the band a consumer adds can reach past the node's box (issue #639); a radial pose, a head behind a whole-item deformer, or an easing that is not exactly zero at its rim keeps the node's bound. Marking is validated in 64-bit before the engine converts the region: a span above `CLAY_MAX_BATCH` or a brick coordinate outside `int32` is refused with the cache untouched, since three caller floats can otherwise name 1019 bricks. **And the live preview is incremental** (ABI 0.60.0): `clay_sdf_smooth_preview_item` copies the whole working volume, which is the right shape for a host joining mid-gesture and the wrong one for a per-frame loop — a dab moves a ball of bricks and the host re-uploads the model. `clay_sdf_smooth_preview_delta_info` and `clay_sdf_smooth_preview_delta_take` hand over exactly the bricks whose bytes are new: the ones a dab materialized and the ones its relax moved. Ask, size the buffers, take — the caller owns the memory it uploads from, which is the convention `clay_brick_cache_take_dirty` and `clay_voxel_take_dirty_chunks` already set. A short buffer takes **nothing** and reports what it needs, because taking is what clears the delta and a partial drain would strand bricks no later call reports. The delta ACCUMULATES until taken and is deduplicated by brick — a dab materializes a brick and then relaxes it, so the same coordinate arrives twice by construction — so a host that skips a frame loses nothing. `generation` is bumped by every update that changed the preview and by nothing else, which is how a host tells a duplicate read from a skipped frame and drops an upload begun against an older one; taking clears the delta and not the generation, because the generation names the state the caller now holds. `clay_sdf_preview_brick` carries no `struct_size`: it is an array element like `clay_brick_request`, received in hundreds and read rather than filled in. diff --git a/docs/06-host-gpu-previews.md b/docs/06-host-gpu-previews.md index d7da9384..dd2ebc57 100644 --- a/docs/06-host-gpu-previews.md +++ b/docs/06-host-gpu-previews.md @@ -434,6 +434,11 @@ clay_brick_cache_read_bricks(cache, /*lod*/ 0, keys, key_count, /*apron*/ 1, 207 ms to undo it). Do NOT try to work it out by diffing the layer's nodes across the call: an undone move, resize or colour edit keeps its node id, the diff sees nothing, and under-dirtying leaves stale bricks at a blend seam. + Undoing one Move segment reports the segment's grab, not the node it hangs + off (issue #639): a step whose deformer chains differ only in a head of grab, + magnify, blob or alpha links reports their balls, placed and dilated as the + item is and clamped into the node's bound. The price of each refilled brick + still grows with the chain's length; only the node-size factor is gone. **Formats.** `r16float` for distance, `rgba8unorm` for colour. Both are hardware-filterable in WebGPU, so the trilinear step is free and the shader is a diff --git a/include/clay/scene/bounds.h b/include/clay/scene/bounds.h index 03ee3301..280247ec 100644 --- a/include/clay/scene/bounds.h +++ b/include/clay/scene/bounds.h @@ -646,6 +646,45 @@ math::Aabb node_influence_bound_in_document(const Document& doc, const SdfConten std::optional item_geometry_reach_in_document(const Document& doc, const SdfContent& content, NodeId id); +// WHERE A CHANGE TO THE HEAD OF ONE ITEM'S DEFORMER CHAIN LANDS IN THE +// DOCUMENT (issue #639): the item going from chain `before` to chain `after`. +// +// A Move is a grab put at the head of a node's chain, and undoing one used to +// report the node's whole influence bound -- the command's target -- so the +// host refilled every brick of the node for an edit that moved one ball of it. +// This answers the narrower question when it can. +// +// The chains are compared from the TAIL; what differs is a head on each side. +// When every link in both heads is a FINITE-SUPPORT link whose weight is +// exactly zero at its rim, and whose kernel returns its input untouched past +// it -- grab, magnify, blob, alpha; NOT radial pose, see bounds.cpp -- the +// field is bit-identical outside the union of their balls, and that union +// (with a grab's displaced end) is placed the way the item is: its transform, +// every symmetry copy, its rounding and combine support +// (the geometry bound's own placement), dilated once per enclosing group by +// that group's blend support, and carried up by layer_reach_in_document -- +// unioned over every layer sharing the content. +// +// Those dilations are not what makes it sound -- the identity is exact, and a +// combine is pointwise -- but they are the dilations every other reach takes, +// and taking them keeps the answer inside the band-clamped argument the rest of +// this file makes rather than beside it. The price is a blend radius or two. +// +// AN EMPTY BOX when the chains are the same. NULLOPT, and the caller keeps the +// node's influence bound, for: a missing node or a group; any other kind in +// either head (a twist ahead of a grab moves the point before the grab's ball +// is tested); an easing whose rim value is not exactly zero on every backend; +// an infinite repetition grid; a morph or hidden group above; an unbounded +// result. +// +// It is a REGION OF CHANGE, not an influence bound, and it is not clamped into +// the node's bound here: `UndoStack::replay` does that (`head_within`), so the +// undo bound is never larger than what it replaces. +std::optional deformer_head_reach_in_document(const Document& doc, + const SdfContent& content, NodeId id, + const std::vector& before, + const std::vector& after); + // Whole-layer bound (union of root node bounds), IN THE LAYER'S OWN FIELD. It // takes a Layer and not a Document, so it cannot answer where a change to this // layer reaches the DOCUMENT: the folds above it are a property of the stack diff --git a/include/clay/scene/commands.h b/include/clay/scene/commands.h index c28d6eea..24a71fff 100644 --- a/include/clay/scene/commands.h +++ b/include/clay/scene/commands.h @@ -328,6 +328,25 @@ math::Aabb command_influence_bound(const Document& doc, const Command& cmd, // geometry with nothing on the host's side to point at. std::optional command_surface_delta_bound(const Document& doc, const Command& cmd); +// WHERE REPLACING A DEFORMER CHAIN'S HEAD CAN CHANGE THE FIELD (issue #639), +// taken on the side BEFORE the apply -- the only side that holds both chains, +// the node's current one and the command's. +// +// For a SetDeformersCmd whose old and new chains differ only in a head of +// finite-support links, the region is those links' balls, placed and dilated +// by `deformer_head_reach_in_document`, which is where the argument and its +// refusals are written. That is what a Move leaves behind -- one grab at the +// head of the chain per segment -- and what undoing or redoing one replays. +// +// nullopt for every other command kind and wherever that function refuses. A +// caller CLAMPS this into the command's influence bound on both sides, never +// replaces it, so the answer can only narrow the old one: an undo bound is then +// never larger than the node's, and never tighter than what changed. Clamped, +// not intersected: that bound is reported without the band its consumers add, +// so a ball that misses it by less than a band still changes the field, and +// the intersection would be empty (UndoStack::replay's `head_within`). +std::optional command_head_delta_bound(const Document& doc, const Command& cmd); + // What an AddLayerCmd that REINSERTS an existing layer must name as its // content source: the first OTHER layer in stack order holding the same edit // list, or 0 when this layer holds it alone. diff --git a/openspec/ROADMAP.md b/openspec/ROADMAP.md index e1cffe61..1c7e934b 100644 --- a/openspec/ROADMAP.md +++ b/openspec/ROADMAP.md @@ -156,16 +156,23 @@ because none has a proposal yet: - #634: device watch item. `stroke_build` read 0.298 -> 0.488 ms (1.64x) across iOS 26.5.2 -> 27.0, and cannot be attributed until a same-OS gate runs. -## Where the engine is (2026-09-22, v0.120.0) +## Where the engine is (2026-09-23, v0.120.0) -21 capabilities, 235 archived changes, 41 still open. v0.120.0 was published +21 capabilities, 238 archived changes, 41 still open. v0.120.0 was published 2026-09-18. On 2026-09-23 `unify-the-undo-history` finished — voxel sculpt-layer operations became undo steps, the mid-session enable was decided, -and the device half of its measurement was re-scoped to #644 — and archived, -followed by `record-the-layer-a-crossing-creates`, which had waited on it since -2026-09-06 because its `scene-model` delta modifies a requirement the first -change adds. Each of the 41 still open has at least -one open task. Complete enough that the +and the device half of its measurement was re-scoped to #644 — and archived +(#647), followed by `record-the-layer-a-crossing-creates`, which had waited on +it since 2026-09-06 because its `scene-model` delta modifies a requirement the +first change adds. The same day #646 finished and archived +`finish-regional-multires` (the layered smooths average over the whole rim +ring; the mixed-depth export's C +entry point moved to the new `expose-the-mixed-depth-export`), #645 archived +`report-an-unconfined-lattice-as-unbounded` (an unconfined infinite grid now +reports unbounded `tape.bounds`, #640), and #648 archived +`bound-an-undone-grab-by-its-support` (an undone grab-family deformer step +reports the grab's support rather than its whole node, #639). Each of the 41 +still open has at least one open task. Complete enough that the gaps below are about *sculpting affordances*, not about the field engine — and as of the 2026-09-06 reconciliation below, about what a HOST can reach rather than about what the engine can do. diff --git a/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/design.md b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/design.md new file mode 100644 index 00000000..5f9f3527 --- /dev/null +++ b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/design.md @@ -0,0 +1,178 @@ +## Context + +`clay_document_undo_bound` / `_redo_bound` report, per command in the step, +`command_influence_bound` taken before and after the replay, unioned +(`UndoStack::replay`, `src/scene/commands.cpp`). For a `SetDeformersCmd` that +target is the NODE: `node_command_bound` -> `node_influence_bound_in_document`. +A Move segment is exactly that command -- `brush::moved_chain` puts one grab per +drag image at the HEAD of every chain the drag reached -- so undoing it reports +the node's whole bound. The node's bound also grows with the chain: +`deformed_local_bounds` dilates the whole local box by every grab's pull. + +## D1. The argument: a head of exact-identity links changes nothing outside its balls + +The chain runs `p -> d0 -> d1 -> ... -> prim` (`ctape_prim_local`), accumulating +`offset += deform_offset(link, wp)` and `wp = deform_point(link, wp)` per link. + +Take the two chains a step goes between. Strip their longest common TAIL (links +compared bit for bit on every field the kernel reads -- the record, the alpha +stamp, a bend curve's guide, a lattice's cage and its placement -- so a lattice +or a curve BEHIND the head is stripped like any other link; `gesture_id` is host +bookkeeping and is not read). What is left is a HEAD on each side. Suppose every +link in both heads is the identity outside its own ball -- returns `wp` +untouched (a warp) or adds exactly `0.0f` (an offset) wherever its weight is +zero. Then for a point `p` outside every one of those balls, link 0 hands `p` on +unchanged, so link 1 sees `p` and hands it on unchanged, and so on: both heads +deliver the same point and the same offset (`x + 0.0f == x`) to the same tail. +The item's raw field is BIT-IDENTICAL outside the union of the balls. + +Every combine above the item -- its own op, a group's, the layer mirror's seam, +a layer's composition -- is a function of its operands at `p`. So the document's +raw field is bit-identical outside the union of the balls' images too. This is a +stronger statement than the band-clamped one every other reach in `bounds.cpp` +makes, and it is what `test_deformer_head_reach.cpp` checks, bit for bit, on the +reference evaluator. + +A twist (or any whole-item link) in either HEAD breaks it: it moves the point +before a later link's ball is tested, so the region is that link's PREIMAGE, not +its ball. A whole-item link in the common TAIL costs nothing -- it sees the same +point on both sides. So a grab added at the FRONT of a twisted chain narrows, and +one appended BEHIND the twist does not. + +## D2. Which deformers qualify, and why these four + +The candidates are `link_support`'s finite set -- grab, magnify, radial pose, +blob, alpha -- all gated by `cregion_weight`. Finite weight is necessary and NOT +sufficient; what the argument needs is that the KERNEL returns its input +untouched where the weight is zero. + +| kind | kernel past the rim | qualifies | +|---|---|---| +| grab | `if (w == 0.0f) return p;` | yes | +| magnify | `if (w == 0.0f) return p;` | yes | +| blob (offset) | `if (w == 0.0f) return 0.0f;` | yes | +| alpha (offset) | `if (weight <= 0.0f) return 0.0f;` | yes | +| radial pose | no early-out: `centre + rotate(p - centre, -angle * 0)` | **no** | + +Radial pose was on the list in the first draft of this design. The raw check +refuted it: `centre + (p - centre)` is not `p` in float, so the field moves by an +ulp wherever the item is -- 1,917 of 68,796 lattice points outside the ball moved +on the test fixture. An ulp is enough to flip a stored fp16 brick. Pose keeps the +node's bound; `test_deformer_head_reach.cpp` pins the reason, so the day pose +gains an early-out the test says it may join. + +THE EASING, second condition. The weight at the rim is `cease(ease, 0)`. +`ease_out_sine` computes `cos(pi/2)` there, which is not zero in float, and +`bounds.cpp` already records a curve that returns 5.96e-08 at its zero end: a +grab under such a curve is a rigid translation by a hair EVERYWHERE, and has no +support at all. And the host computing zero is not enough -- a backend with fast +math or fused multiply-add may not. So `ease_is_zero_at_rim` refuses the +families whose rim runs through a transcendental or a multi-term polynomial (the +sines, `out_expo`, the circs, `in_bounce`, `in_out_bounce`) whatever the host +computes, and accepts the rest only when the host computes exactly zero. The +accepted ones reach the rim through a product with `t = 0`, a guarded branch, or +`1 - 1*1*...*1`, which every IEEE backend evaluates exactly. + +## D3. The dilation is shared, not restated + +The balls are in the item's chain frame. They are placed by the functions that +place the item's own geometry, taken rather than re-spelled: + +- **Placement** -- `placed_local_bound`, which is the body `geometry_bound` had, + with the local box made a parameter. `geometry_bound` now calls it with + `item_local_bounds`; the head reach calls it with the balls. One body, so the + item transform, the per-axis scale, EVERY mirror and radial copy, the seam + supports, the rounding and the item's own combine support cannot drift apart + between "where the item is" and "where its head can change it". +- **Repetition** -- `repeated_local_bounds` on the balls, because repetition + folds the point before the chain sees it. An infinite grid is refused. +- **Groups** -- `dilate_by_ancestors`, the walk `item_geometry_reach_in_document` + already uses: node_reach_bound's per-group support with its #515 predicate, + refusing a morph or hidden group. +- **Folds** -- `layer_reach_in_document`, i.e. `folds_from_layer_support`, the + one definition every other reach goes through. +- **Instancing** -- the union over every layer sharing the content, exactly as + `node_influence_bound_in_document` takes it. + +None of these dilations is what makes the answer sound -- D1 is exact -- but +they are the ones every other reach takes, and taking them keeps this answer +inside the band-clamped framework the rest of `bounds.cpp` is written in rather +than beside it. They cost a blend radius or two per side. The mutation checks +show the fold and the displaced end are pinned by the box test; the mirror copy +is pinned by the brick oracle. + +## D4. The displaced end is margin, kept on purpose + +D1 needs the ball at the grab's centre only: the kernel reads the weight at the +sample point. The ball at `centre + pull` is margin. It stays, because clay.h +has always promised a move's two ends, and because the live Move reports the +same segment as its ball dilated by the pull (`move_surface_impl`): an undo that +reported LESS than the gesture that made the edit is a surprise no host could +debug. It costs the pull's length along one axis. + +## D5. Clamped into the node's bound, never substituted + +`apply_bounded` still takes `command_influence_bound` on both sides and then +CLAMPS the head's box into it, corner by corner (`head_within`): per axis, the +overlap where the two overlap, and the face of the node's box nearest the ball +where they miss. + +Not the intersection, which is what this first shipped with and which a +randomized oracle refuted. The node's bound is reported WITHOUT the band; +every consumer adds it (`BrickCache::mark_dirty` dilates by it, the seed store +by band + pad). So the node can change the field within a band OUTSIDE its +box, and a ball sitting there changes it: a magnify just past a node's face, +undone, intersected to nothing, the host dirtied nothing and one brick was left +stale. Per axis, for a ball `[a, b]` and a node box `[c, d]`, what can change +is `[a, b]` within the band `e` of `[c, d]`; the clamped interval dilated by +`e` covers that for every `e` -- the overlap `[max(a,c), min(b,d)]` when they +overlap, the face `[c, c]` when `b < c` -- and the intersection dilated by `e` +does not when it is empty. Tested: `a ball that misses the node's box but lies +within the band of it is still refilled`. + +Two consequences, both wanted: + +- The undo bound is never larger than the node's -- the acceptance's "no larger + than the node's bound" -- even for a ball that pokes out of the node's box. +- Every refusal falls back to exactly the old answer: nullopt means no clamp. + +## D6. Mixed steps + +The narrowing is per COMMAND, inside `UndoStack::replay`. A step holding a +grab and a transform of another node reports the grab's clamped box unioned +with the transform's two ends. A Move commit that also ran the complexity +policy -- a consolidation inside the same step -- reports that consolidation's +own bound beside the grabs'. Tested: `a step that also carries another command +still covers that command`. + +## D7. Where it is NOT applied + +`apply_edit` -- the forward path of `clay_layer_add_deformer` and of a Move +commit -- is unchanged. The live Move states its own reach (`move_surface_impl`), +and a host adding one deformer by hand dirties with `mark_dirty_nodes`, which is +the node's bound by contract. The rule is one function +(`command_head_delta_bound`), so applying it forward later is one call site. + +## D8. The oracle had to be taken on a COPY of the document + +The first brick oracle built the "fresh" cache on the same document after the +undo, the shape `test_intersect_delta_oracle.cpp` uses. With the mirror image +deliberately dropped from the bound, it reported ZERO stale bricks. The seed +store belongs to the document (clay.h, `clay_resume_stats`), and the refill +resumed from the very seeds the too-narrow bound had failed to drop, reproducing +the stale values and agreeing with them. The oracle here rebuilds from a +`clay_document_save_memory` / `clay_document_load_memory` copy, which has no seeds; with that, +the same mutation reads 8 stale bricks. `test_intersect_delta_oracle.cpp` has +the same shape and is not changed here -- it is noted as a gate that may not be +able to fire on its own failure mode. + +## D9. What this does NOT fix + +The per-brick price. The cost of one refilled brick still grows with the +chain's length -- 7.4 us at 1 grab, 28.1 us at 160 on the probe's fixture, +with none of those grabs reaching the refilled bricks -- and where that cost +goes is not measured here. This change removes the node-extent factor only +(numbers in proposal.md). The issue's +second route -- a refill over a baked volume costing close to an analytic +item's (~430 us vs ~7 us a brick, measured by the host) -- is a separate problem +and is not attempted. diff --git a/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/proposal.md b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/proposal.md new file mode 100644 index 00000000..7f37260f --- /dev/null +++ b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/proposal.md @@ -0,0 +1,120 @@ +## Why + +Undoing a Move on a field layer costs more the longer the layer's chain of +edits is, at a flat triangle count. ClaySpaceDesktop measured it against +v0.120.0 (issue #639): **20 ms -> 4,536 ms** over 50 undo steps in a live +session, and **61 ms at gesture 11 against 362 ms at gesture 40** in a host +test. + +A host Move is a grab put at the HEAD of a node's deformer chain, one per +segment (`brush::moved_chain`). The host dirties what `clay_document_undo_bound` +reports, and that is, per command, the union of what the command TARGETS before +and after — for a `SetDeformersCmd`, the NODE. So undoing one segment refills +the node's whole influence bound, and that bound itself grows with every grab +on the chain, because `deformed_local_bounds` dilates the node's whole box by +each grab's pull. Undo cost ~= bricks-in-the-node x chain length: the first +factor is fixed by the node (and grows with the chain), the second grows per +gesture. + +## Reproduced on this engine + +Probe: `benchmarks/undo_grab_bound_probe.cpp`, C ABI only. A sphere node of +radius 1.5, N grabs of radius 0.2 around its equator, one grab of radius 0.15 on +its pole at the head of the chain; undo it, mark the reported bound, refill. +Bricks of 8^3 at voxel 0.05. On origin/main at `49f4418a` (ABI 0.120.0): + +| grabs on the chain | bricks the undo marks | the grab's own ball marks | +|---:|---:|---:| +| 1 | **1,000** | 12 | +| 10 | **1,440** | 12 | +| 40 | **4,000** | 12 | + +The undo's count IS the node's (`clay_layer_node_influence_bound` marks the +same 1,000 / 1,440 / 4,000), and it grows with the chain although the edit is +the same 0.15 ball every time. The acceptance test +`undoing one grab marks a count independent of the node's size and its chain` +reads the same shape at other sizes: 1,000 bricks for a radius-1.5 node, 32,768 +for radius 6, 61,952 for radius 6 with 40 grabs — for an edit whose ball marks +12. + +## Measured after + +Same probe, both arms built from the same source with `clang++ -O2` against +each tree's `cpu-only` Release `libclaycore.a`; Mac (M-series), CPU backend; +200 timed undos per point after five discarded, medians, the two binaries +interleaved three times per point after one throwaway run of each. The three +repeats agree to within 0.1 ms (main) and 0.005 ms (branch); the middle one +is shown. + +| grabs | bricks main -> branch | undo p50, main -> branch | per refilled brick, main -> branch | +|---:|---:|---:|---:| +| 1 | 1,000 -> **12** | 1.665 -> **0.089 ms** (18.7x) | 1.66 -> 7.42 us | +| 10 | 1,440 -> **12** | 2.481 -> **0.086 ms** (28.8x) | 1.72 -> 7.17 us | +| 40 | 4,000 -> **12** | 7.415 -> **0.115 ms** (64.5x) | 1.85 -> 9.59 us | +| 80 | -- -> 12 | -- -> 0.158 ms | -- -> 13.18 us | +| 160 | -- -> 12 | -- -> 0.337 ms | -- -> 28.12 us | + +**The count is flat and asserted; the clock is not the claim.** The brick count +is what the tests pin (`undoing one grab marks a count independent of the +node's size and its chain`: 12 bricks at node radius 1.5 and 6, at 1 and 40 +grabs, where main marks 1,000 / 32,768 / 61,952). + +**The chain-length factor is still there, and this is it.** On the branch the +same 12 bricks cost 7.4 us each at 1 grab and 28.1 us at 160, although none of +the equator grabs reaches the pole the bricks sit on. The issue's second factor +-- the price of one refilled brick grows with the chain -- is untouched by this +change, and WHERE that per-brick cost goes on this fixture (compile, cull, or +evaluation) is not measured here. What is removed is the first factor, the +node's extent. The per-brick column is not comparable across the arms: most of +main's 1,000-4,000 bricks are proven uniform or empty and cost almost nothing, +while the branch's 12 are the surface bricks the grab actually moved. + +## What Changes + +- **A deformer step reports the deformer's support, clamped into the node's bound.** + When the chains before and after a `SetDeformersCmd` differ only in a HEAD of + links that are exactly the identity outside their own ball — grab, magnify, + blob, alpha, under an easing that is exactly zero at the rim on every backend + — `UndoStack::replay` clamps those balls into that command's before/after + bound (a grab's at its centre and its displaced end), placed as the item is + placed (every mirror and radial copy, every instancing layer), dilated per + enclosing group and per layer fold. One rule: `scene::command_head_delta_bound` + over `scene::deformer_head_reach_in_document` (design.md D1-D5). +- The placement is `placed_local_bound`, which is `geometry_bound`'s body with + the local box made a parameter — not a second spelling of it (D3). +- `clay_document_undo_bound` / `_redo_bound` keep their signatures. **No new + symbol, no version bump**: ABI stays 0.120.0. What they return for a deformer + step is now tighter, and never tighter than what changed. +- `clay.h`, `docs/05`, `docs/06` say what a deformer step reports and what it + does NOT promise. + +## What is NOT changed, and why + +- **The per-brick price.** Each refilled brick is still evaluated through the + whole chain, so one brick costs more on a longer chain. This removes the + node-extent factor only, and the measurement above shows both. +- **Radial pose**, which has finite weight and is still not the identity past + its ball (D2), and any head containing a whole-item deformer, keep the node's + bound. +- **The forward path** (`apply_edit`) is unchanged (D7). +- **The issue's second route** — make a refill over a baked volume cost close to + an analytic item's (~430 us against ~7 us a brick, the host's numbers) — is a + separate problem and is not attempted here. + +## Capabilities + +### Modified Capabilities + +- `c-abi`: "Undo reports the region it changed" gains the deformer-head + narrowing and five scenarios. + +## Impact + +- `src/scene/bounds.cpp`, `include/clay/scene/bounds.h` — the head reach and the + extracted placement. +- `src/scene/commands.cpp`, `include/clay/scene/commands.h` — the command-level + rule and the replay clip. +- Tests: `test_c_undo_bound_grab_support.cpp` (C ABI), `test_deformer_head_reach.cpp`. +- `benchmarks/undo_grab_bound_probe.cpp` — not gated. +- A host that dirties by the undo bound refills the grab's ball instead of the + node. A host that ALSO widened it by hand keeps working; it just pays for it. diff --git a/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/specs/c-abi/spec.md b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/specs/c-abi/spec.md new file mode 100644 index 00000000..bbc6f4fa --- /dev/null +++ b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/specs/c-abi/spec.md @@ -0,0 +1,94 @@ +## MODIFIED Requirements + +### Requirement: Undo reports the region it changed +The C API SHALL offer undo and redo entry points that additionally report the world-space INFLUENCE bound of what they applied, in the three-state shape the influence-bound queries already use: nothing changed, a finite box, or unbounded. The bound SHALL be usable directly as the region argument of the brick cache's dirty marking, with the unbounded state spelled the way that call already spells it. + +Without this the narrowest region a host can honestly dirty after an undo is the whole layer, because nothing in the ABI says which nodes the step touched. Every alternative available to the host is worse: diffing the layer's nodes across the call misses an in-place change — an undone move, resize or colour edit keeps its node id — and under-dirtying leaves stale bricks at a blend seam, which is silent and on screen. + +The bound SHALL be the union, over every command in the step, of what that command targets BEFORE it is applied and AFTER it is applied. One side alone cannot see a move (which has two ends), a removal (whose node is gone afterwards) or an add (whose node was not there before). + +The bound MAY be larger than the region that actually changed and SHALL NOT be smaller. Where being tight would cost correctness it SHALL be conservative, and the two places that costs something are stated rather than left to the implementation: + +A node inside a group SHALL report the bound of the NODE THE COMMAND NAMES, dilated by the blend support of each group on the path from that node up to its root — the group's blend spreads a child's influence past the child's own box, and that spread is what the ancestors' supports measure. It SHALL NOT report the whole root subtree's bound: a sibling's geometry is not something the edit can reach, and reporting it makes the region grow with the size of the group rather than with the size of the edit. + +A command on content shared by instanced layers SHALL report the union over every layer that shares that content. + +A node whose subtree combines non-locally — an intersect, an unbounded primitive, an infinite grid repeat — SHALL report the unbounded state, exactly as an influence-bound query does for the same node. The path dilation applies to the local case, and it SHALL NOT be used to turn a non-local subtree into a finite box. The one exception is the deformer-head narrowing below, whose argument does not rest on locality. + +A command that replaces an item's deformer chain SHALL contribute, when the old and new chains differ only in a HEAD of links that are exactly the identity outside their own ball — grab, magnify, blob and alpha, under an easing whose value at the rim is exactly zero on every backend — the node's target bound INTERSECTED with those links' balls (a grab's at its centre and at its displaced end), placed as the item is placed including every symmetry copy, dilated once per enclosing group by that group's blend support and by every layer fold from the item's layer up, and unioned over every layer sharing the content. This is what one Move segment is, and reporting the node's whole bound for it made the cost of undoing one segment grow with the size of the node and with the length of its chain (issue #639). A radial pose, a whole-item deformer anywhere in either head, an easing whose rim is not exactly zero, an infinite grid repeat, or a morph or hidden group above the item SHALL keep the node's bound. The narrowing SHALL only ever shrink the node's bound, never replace it with something larger, and SHALL apply per command, so a step that also carries other commands still reports their bounds. + +A command that cannot change what the document evaluates to SHALL contribute nothing to the bound, so a step made only of such commands reports that there is nothing to dirty rather than reporting the layer. + +The existing undo and redo entry points SHALL keep their signatures and their behaviour, and the reporting variants SHALL agree with them on what was undone and on the resulting document. + +#### Scenario: Undoing a dab dirties the dab +- **WHEN** an item is added to a populated layer and the add is undone through the reporting variant +- **THEN** the reported bound contains the added item's influence bound and is strictly smaller than the layer's + +#### Scenario: A move is bounded at both ends +- **WHEN** an item is transformed far from where it was and the transform is undone +- **THEN** the reported bound contains both the position it was moved to and the position it was moved back to + +#### Scenario: An undone removal is bounded by what came back +- **WHEN** a node is removed and the removal is undone, restoring it +- **THEN** the reported bound contains the restored node's influence bound + +#### Scenario: A group's blend is not cut off +- **WHEN** a child of a smooth-blended group is edited and the edit is undone +- **THEN** the reported bound reaches past the child's own box by the group's blend support, is never smaller than the child's own influence bound, and band-clamped values outside it are unchanged + +#### Scenario: A sibling's geometry does not widen the bound +- **GIVEN** a group holding one small child and a large one far from it +- **WHEN** the small child is transformed and the transform is undone +- **THEN** the reported bound is strictly inside the group's own influence bound on the side facing the sibling, and it does not contain the far sibling's geometry + +#### Scenario: Nested groups each contribute their support +- **GIVEN** a child inside a smooth-blended group which is itself inside another smooth-blended group +- **WHEN** the child is edited and the edit is undone +- **THEN** the reported bound is the child's bound dilated by both groups' blend supports, and band-clamped values outside it are unchanged + +#### Scenario: A non-local subtree is still unbounded +- **WHEN** a child of a group whose combine is an intersect is edited and the edit is undone +- **THEN** the call reports the unbounded state rather than a finite box + +#### Scenario: An instanced layer is not missed +- **WHEN** a layer is instanced, an edit is made through one instance, and that edit is undone +- **THEN** the reported bound covers where the change lands in every layer sharing the content, not only the layer named by the command + +#### Scenario: Unbounded influence is expressible +- **WHEN** the undone step touched a node whose influence has no finite extent +- **THEN** the call reports the unbounded state rather than a finite box, and the host's honest response is to dirty everything + +#### Scenario: An edit that changes no field dirties nothing +- **WHEN** a layer rename is undone +- **THEN** the call reports that there is nothing to dirty + +#### Scenario: Nothing to undo is still not an error +- **WHEN** the reporting variant is called on a document with nothing to undo +- **THEN** it reports that nothing was undone, reports nothing to dirty, and returns success + +#### Scenario: Undoing one grab reports the grab, not its node +- **GIVEN** a large node carrying a chain of grabs, under a layer composed onto the one beneath with a smooth fold +- **WHEN** the grab at the head of the chain is undone through the reporting variant +- **THEN** the reported bound contains that grab's ball at its centre and at its displaced end, each dilated by the fold's support, as far as the node's own bound reaches, and it is contained in the node's bound and a small fraction of it + +#### Scenario: The bricks one grab's undo marks do not depend on the node +- **WHEN** one small grab is undone on nodes of different sizes carrying chains of different lengths +- **THEN** the number of bricks the reported bound marks is the same in every case, and it is the grab's ball's count rather than the node's + +#### Scenario: Refilling a grab's undo bound equals a rebuild +- **WHEN** a grab is undone or redone at a smooth-union seam, on a mirrored layer, inside a blended group, under a smooth layer fold, on an intersecting item, or with its ball crossing the node's own box, and a brick cache refills only the reported bound +- **THEN** every brick's state and stored payload equal those of a cache rebuilt from nothing on a copy of the document + +#### Scenario: Redo reports what undo did +- **WHEN** a grab's undo is redone +- **THEN** the reported bound equals the one the undo reported + +#### Scenario: A grab ahead of a whole-item link in the common tail is narrowed +- **GIVEN** a node whose chain ends in a twist, a lattice or a bend curve +- **WHEN** a grab added at the front of that chain is undone through the reporting variant +- **THEN** the reported bound marks a small fraction of the node's bricks, and refilling only it equals a rebuild + +#### Scenario: A head the argument does not cover keeps the node's bound +- **WHEN** the step adds a radial pose at the head of a chain, or a grab behind a twist +- **THEN** the reported bound marks exactly the bricks the node's own bound marks diff --git a/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/tasks.md b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/tasks.md new file mode 100644 index 00000000..ac3975cf --- /dev/null +++ b/openspec/changes/archive/2026-09-23-bound-an-undone-grab-by-its-support/tasks.md @@ -0,0 +1,182 @@ +## 1. Reproduce on this engine, before building + +- [x] 1.1 Probe `benchmarks/undo_grab_bound_probe.cpp` (C ABI only, so one + source builds against main and against this branch): a sphere node of + radius 1.5, N grabs of radius 0.2 around its equator, one grab of radius + 0.15 on its pole; undo the pole grab, mark the reported bound, refill it. + On origin/main the bound IS the node's: 1,000 / 1,440 / 4,000 bricks at N = 1 / 10 + / 40 -- growing with the chain, because every grab dilates the node's box + by its pull. +- [x] 1.2 The acceptance tests written first and run against main: the box and + count tests FAIL there (5 of 14 cases, 10 assertions: 1,000 / 32,768 / 61,952 bricks + for one 12-brick grab on a radius-1.5 node, a radius-6 node and a radius-6 + node with 40 grabs; magnify 384 = node, blob 216 = node, alpha 1,000 = + node, the host's own Move 1,440 = node); the brick oracles PASS there, + as they must -- the node's bound is loose, not tight, and the oracles are + the never-tighter guard this change has to keep passing. + +## 2. The rule, in the scene layer + +- [x] 2.1 `placed_local_bound`: `geometry_bound`'s body with the local box as a + parameter; `geometry_bound` calls it with `item_local_bounds` +- [x] 2.2 `deformer_head_reach_in_document` (bounds.h/.cpp): tail stripping, + the qualifying kinds, the easing rim, repetition, placement, groups, + folds, sharers +- [x] 2.3 `command_head_delta_bound` (commands.h/.cpp), and `UndoStack::replay` + clamping it into each command's before/after union +- [x] 2.4 Radial pose REMOVED from the qualifying list after the raw check + found it moves outside its ball (design.md D2) + +## 3. Tests + +- [x] 3.1 `tests/unit/test_c_undo_bound_grab_support.cpp` -- through the C ABI: + the box (both ends, fold support, inside the node), redo symmetry, the + brick COUNT independent of node size and chain length, the brick oracle + (seam, mirror, group, fold, intersect, ball crossing the node's box, + magnify, blob, alpha, a mixed step, the host's own Move), and the + refusals marking exactly the node's bricks +- [x] 3.2 `tests/unit/test_deformer_head_reach.cpp` -- the raw field is + bit-identical outside the box on the reference evaluator, and every + refusal +- [x] 3.3 The oracle rebuilds on a COPY of the document (design.md D8) + +## 4. Mutation checks + +- [x] 4.1 Drop the fold dilation (`layer_reach_in_document` skipped) -> fails + the C box test and the C++ group-and-fold test (2 cases) +- [x] 4.2 Drop the mirror copies (`with_copies=false`) -> fails the C mirror + oracle (stale bricks on the reflected side) and the C++ raw check (2 cases) +- [x] 4.3 Report only the centre ball -> fails the C box test, the C++ + displaced-end test and the group-and-fold extent (3 cases). No brick + oracle can catch this one, by design: the far end is margin (design.md D4) + +## 5. Documentation + +- [x] 5.1 `bindings/c/clay.h` above `clay_document_undo_bound`: what a deformer + step reports, and what it does not promise +- [x] 5.2 `docs/05-claycore-library.md`, `docs/06-host-gpu-previews.md` +- [x] 5.3 No pyclay docstring states the rule (checked); no new symbol, so no + version bump -- ABI stays 0.120.0 + +## 6. Gates + +- [x] 6.1 ctest, fresh `cpu-only` with tests and pyclay BUILT: 11/11 (the four + unit shards, pyclay_pytest, the rest), before and after merging #637; the + new files add 29 cases +- [x] 6.2 `check_c_abi.py build/cpu-only/libclay_shared.dylib`: OK (hygiene + + ctypes FFI); `check_binding_parity.py --require-import`: OK, `imported + .../pyclay.cpython-311-darwin.so`; `check_task_symbols.py`: OK; + `check_test_shards.py`: 2,885 cases over 4 shards +- [x] 6.3 `openspec@1.12.0 validate --all --strict`: 65 passed after the + merge (66 before; #637 archived its change) +- [x] 6.4 `release_check.py --skip-slow`: every row PASS except `device`, which + is stale by construction for a `src/` change (and was already stale on + origin/main: `clay.h` and `pyclay_module.cpp` changed since the gate ran + at `704f2d4af`). Mac numbers are not device numbers; nothing here claims + one +- [x] 6.5 Cognitive complexity, clang-tidy against this tree's + compile_commands.json: every new or changed function <= 15 -- + `deformer_head_reach_in_document` 14, `same_link` 10, `chain_head_change` + 8, `replay` 6; `placed_local_bound` 5 after the symmetry copies moved into + two helpers (the same body, measured before that split, scored 16); + test helpers and doctest bodies <= 15 + +## What building it found + +- **Radial pose does not qualify, although `link_support` lists it as finite.** + The first design had it. The raw bit-for-bit check found 1,917 of 68,796 + lattice points outside its ball moving: `cpose_point` has no zero-weight + early-out, so past the ball it returns `centre + (p - centre)` rotated by + zero, which is not `p` in float. Grab and magnify early-out to `p`; blob and + alpha add exactly 0. Finite WEIGHT is not the property; an untouched RETURN + is. +- **The easing is half the condition.** `ease_out_sine` is not zero at the rim + on the host, and a transcendental rim is not guaranteed zero on a GPU even + where it is on the host. The sines, `out_expo`, the circs, `in_bounce` and + `in_out_bounce` keep the node's bound. +- **A same-document brick oracle cannot see this failure.** The first oracle + rebuilt the "fresh" cache on the undone document itself. With the mirror + image dropped from the bound it reported ZERO stale bricks: the seed store is + the document's, and the rebuild resumed from the very seeds the narrow bound + had failed to drop. Rebuilding on a `clay_document_save_memory` / `clay_document_load_memory` copy, the same + mutation reads 8 stale bricks. `test_intersect_delta_oracle.cpp` (#471) + rebuilds on the same document and so has the same blind spot; it is noted, + not changed, here. +- **The acceptance's two box clauses conflict for a ball that pokes out of the + node.** "Covers the support at both ends, dilated by the folds" and "no + larger than the node's bound" cannot both hold when the grab's ball reaches + past the node's influence bound. The bound is clamped into the node's (see + "What review found" for why clamped and not intersected), and the test + asserts coverage of the support INTERSECTED with the node's bound. +- **The displaced end and the dilations are margin, not soundness.** The raw + identity argument needs only the centre ball; they are kept so an undo never + reports less than the live Move reported for the same segment, and so the + answer stays inside the band-clamped framework the rest of bounds.cpp uses. +- **The chain-length factor stays, measured:** 7.4 -> 28.1 us per refilled + brick from 1 to 160 grabs on the probe's fixture. Not attempted here. +- **Merge chain:** the 5-hour wait for #635, #636, #638 and #637 hit its cap + with #637 (per-operator bound narrowing) still open, so this branch started + from origin/main at `49f4418a` (#635, #636, #638 merged). #637 merged minutes + later and was merged in (`git merge origin/main`, no conflicts): it narrows + `tape.bounds` (`combine_extent`), not the reach functions this change uses, + so the rules stay one rule. Every gate below was re-run after that merge. +- **The task-symbol gate caught this file.** Writing the seed-store finding as + the short names save_memory / load_memory failed `check_task_symbols.py` (2 unresolved); + the full names resolve. + +## 7. Independent review + +- [x] 7.1 Randomized oracle, `benchmarks/undo_bound_oracle_probe.cpp`: random + documents, one deformer step or host Move segment, undo and redo, refill + ONLY the reported bound, compare with a rebuild on a saved copy +- [x] 7.2 The same trials checked against the RAW field (`RV_RAWBOUND`): every + sample that moved within the band must lie in the bound dilated by the + band +- [x] 7.3 Clamp the head's box into the node's bound instead of intersecting + (`head_within`), with a regression test that fails on the intersecting + code and passes on main +- [x] 7.4 `same_link` compares a bend curve's guide and a lattice's cage and + placement, so a grab ahead of one in the common tail narrows +- [x] 7.5 Mutations re-run by hand, library AND test binaries rebuilt each time + +## What review found + +- **The intersection was unsound; the clamp is not.** The first randomized run + (1,500 trials) left a brick stale where the step reported NO bounds: a + magnify whose ball missed the node's box by less than the band. The node's + bound is reported without the band (every consumer adds it), so the field can + change there, and intersecting the ball with that box gave nothing. Fixed in + `head_within` (design.md D5); test `a ball that misses the node's box but + lies within the band of it is still refilled`. +- **Against the raw field the rule now holds everywhere it was exercised.** 800 + trials under `RV_RAWBOUND` (400 plain, 400 with per-axis scales, repeats, + intersects, instancing and moved layers): 1,489 undo or redo directions that + changed the field, 1,190 of them narrowed, and ZERO samples that moved + outside the bound plus the band beyond what main shows. Main shows one: seed + 5128 (a grab on a node in a blended group) moves the raw field by an ulp + outside the node's own influence bound, on main and on this branch alike. + That is the node bound's, not this change's. +- **Some refill-vs-rebuild disagreements remain, and they are the rebuild's.** + Of 3,000 trials (1,500 plain, 1,500 rich), five leave 1 to 8 bricks + different from a full rebuild that main's node bound would have refilled + (a sixth, 5128, leaves 13 on main as well). At every one, the bricks the bound kept match the raw field (`clay_eval_points`) + to the fp16 step and the REBUILD does not: a full brick build of the same + document disagrees with its own raw field at in-band samples on main too (by + up to 0.0105 on seed 933 and 0.189 on seed 5111). main's forward Move -- + dirtying exactly what `clay_layer_move_surface_regions` reports -- leaves the + same 8 bricks on seed 933 and 6 on seed 5229. Undo used to hide that by + refilling the whole node; it now refills what the Move itself refilled. The + build's in-band disagreement with the raw field is a separate defect, not + fixed here. +- **The lattice and bend-curve payloads were not compared**, so either link + anywhere in a chain ended the common tail and the step fell back to the node: + 216 of 216 bricks for a grab ahead of a lattice, 420 of 420 ahead of a curve. +- **Mutations** (each rebuilt, library and tests; probe 200 plain + 200 rich + trials): dropping the fold dilation fails the C box test and the C++ fold + extent; dropping the mirror copies fails the C mirror oracle, the C++ raw + check and 17 + 14 probe trials; dropping the displaced end fails the C box + test and two C++ cases; dropping the group dilation fails two C++ cases (no C + oracle and no probe trial catches it -- margin, as D1 says); comparing no + cage fails the C++ payload refusal. None of the dilation drops adds a raw + or oracle failure over the unmutated baseline, which is what "margin, not + soundness" predicts. diff --git a/openspec/specs/c-abi/spec.md b/openspec/specs/c-abi/spec.md index c3a014e1..49a1ab3c 100644 --- a/openspec/specs/c-abi/spec.md +++ b/openspec/specs/c-abi/spec.md @@ -1583,7 +1583,9 @@ A node inside a group SHALL report the bound of the NODE THE COMMAND NAMES, dila A command on content shared by instanced layers SHALL report the union over every layer that shares that content. -A node whose subtree combines non-locally — an intersect, an unbounded primitive, an infinite grid repeat — SHALL report the unbounded state, exactly as an influence-bound query does for the same node. The path dilation applies to the local case, and it SHALL NOT be used to turn a non-local subtree into a finite box. +A node whose subtree combines non-locally — an intersect, an unbounded primitive, an infinite grid repeat — SHALL report the unbounded state, exactly as an influence-bound query does for the same node. The path dilation applies to the local case, and it SHALL NOT be used to turn a non-local subtree into a finite box. The one exception is the deformer-head narrowing below, whose argument does not rest on locality. + +A command that replaces an item's deformer chain SHALL contribute, when the old and new chains differ only in a HEAD of links that are exactly the identity outside their own ball — grab, magnify, blob and alpha, under an easing whose value at the rim is exactly zero on every backend — the node's target bound INTERSECTED with those links' balls (a grab's at its centre and at its displaced end), placed as the item is placed including every symmetry copy, dilated once per enclosing group by that group's blend support and by every layer fold from the item's layer up, and unioned over every layer sharing the content. This is what one Move segment is, and reporting the node's whole bound for it made the cost of undoing one segment grow with the size of the node and with the length of its chain (issue #639). A radial pose, a whole-item deformer anywhere in either head, an easing whose rim is not exactly zero, an infinite grid repeat, or a morph or hidden group above the item SHALL keep the node's bound. The narrowing SHALL only ever shrink the node's bound, never replace it with something larger, and SHALL apply per command, so a step that also carries other commands still reports their bounds. A command that cannot change what the document evaluates to SHALL contribute nothing to the bound, so a step made only of such commands reports that there is nothing to dirty rather than reporting the layer. @@ -1635,6 +1637,32 @@ The existing undo and redo entry points SHALL keep their signatures and their be - **WHEN** the reporting variant is called on a document with nothing to undo - **THEN** it reports that nothing was undone, reports nothing to dirty, and returns success +#### Scenario: Undoing one grab reports the grab, not its node +- **GIVEN** a large node carrying a chain of grabs, under a layer composed onto the one beneath with a smooth fold +- **WHEN** the grab at the head of the chain is undone through the reporting variant +- **THEN** the reported bound contains that grab's ball at its centre and at its displaced end, each dilated by the fold's support, as far as the node's own bound reaches, and it is contained in the node's bound and a small fraction of it + +#### Scenario: The bricks one grab's undo marks do not depend on the node +- **WHEN** one small grab is undone on nodes of different sizes carrying chains of different lengths +- **THEN** the number of bricks the reported bound marks is the same in every case, and it is the grab's ball's count rather than the node's + +#### Scenario: Refilling a grab's undo bound equals a rebuild +- **WHEN** a grab is undone or redone at a smooth-union seam, on a mirrored layer, inside a blended group, under a smooth layer fold, on an intersecting item, or with its ball crossing the node's own box, and a brick cache refills only the reported bound +- **THEN** every brick's state and stored payload equal those of a cache rebuilt from nothing on a copy of the document + +#### Scenario: Redo reports what undo did +- **WHEN** a grab's undo is redone +- **THEN** the reported bound equals the one the undo reported + +#### Scenario: A grab ahead of a whole-item link in the common tail is narrowed +- **GIVEN** a node whose chain ends in a twist, a lattice or a bend curve +- **WHEN** a grab added at the front of that chain is undone through the reporting variant +- **THEN** the reported bound marks a small fraction of the node's bricks, and refilling only it equals a rebuild + +#### Scenario: A head the argument does not cover keeps the node's bound +- **WHEN** the step adds a radial pose at the head of a chain, or a grab behind a twist +- **THEN** the reported bound marks exactly the bricks the node's own bound marks + ### Requirement: The full mesh validation report crosses the ABI The C API SHALL expose every quantity `mesh::ValidationReport` computes, through a versioned output descriptor rather than through individual out-parameters. diff --git a/src/scene/bounds.cpp b/src/scene/bounds.cpp index fe1e1c43..f44d7aea 100644 --- a/src/scene/bounds.cpp +++ b/src/scene/bounds.cpp @@ -937,6 +937,43 @@ Aabb item_local_bounds(const Node& item) { namespace { +// The copies the layer's MIRROR emits of `local`, and the seam's support. +// The copy's map is on the order emit_item uses: the reflection acts in the +// layer's LOCAL space, so the layer's per-axis scale is outside it, and the +// item's per-axis scale is innermost. +void expand_by_mirror_copies(const Node& item, const Layer& layer, const Aabb& local, + const math::cfloat4x4& lm, Aabb* bound) { + if (layer.mirror_axes == 0) return; + const math::cfloat4x4 axes = math::scale_matrix(item.scale_axes); + for (int axis = 0; axis < 3; ++axis) { + if (!(layer.mirror_axes & (1u << axis))) continue; + math::cfloat4x4 m = math::mul( + lm, math::mul(math::reflection_matrix(axis), math::mul(item.xform.matrix(), axes))); + bound->expand(local.transformed(m)); + } + *bound = bound->dilated(kernel::csmin_quadratic_support(layer.mirror_k)); +} + +// Every copy the RADIAL mode emits, for the same reason: a bound that misses a +// copy lets the cull drop an item that is on screen. A rotated box is not +// axis-aligned, so each copy contributes the AABB OF the rotated box -- it +// over-covers, which costs cull precision and never correctness. +void expand_by_radial_copies(const Node& item, const Layer& layer, const Aabb& local, + const math::cfloat4x4& lm, Aabb* bound) { + if (layer.radial_count <= 1) return; + const math::cfloat4x4 axes = math::scale_matrix(item.scale_axes); + const int axis = layer.radial_axis < 3 ? layer.radial_axis : 1; + const int count = static_cast(layer.radial_count); + for (int k = 1; k < count; ++k) { + const float angle = + 6.2831853071795864769f * static_cast(k) / static_cast(count); + math::cfloat4x4 m = math::mul(lm, math::mul(math::rotation_matrix(axis, angle), + math::mul(item.xform.matrix(), axes))); + bound->expand(local.transformed(m)); + } + *bound = bound->dilated(kernel::csmin_quadratic_support(layer.radial_k)); +} + // The geometry bound, with or without the copies the layer's symmetry emits. // One body for both readings so the dilations cannot drift apart: the // every-copy bound is what culling and invalidation consult, the item-alone @@ -946,9 +983,14 @@ namespace { // matrix for every item in a layer and building it is 7.4 ns of this function's // 45.3 -- so a caller looping over a layer's items, which is what // `layer_influence_extent` is, was rebuilding it once per item for no reason. -Aabb geometry_bound(const Node& item, const Layer& layer, bool with_copies, - const math::cfloat4x4* layer_m = nullptr) { - Aabb local = item_local_bounds(item); +// +// `local` is the box in the item's own frame that is being placed. It is the +// item's whole local bound for a geometry bound, and a smaller box -- a +// deformer's own ball -- for `deformer_head_reach_in_document`, which places +// a region of the item rather than all of it and must take every copy and +// every dilation this does, not a second spelling of them. +Aabb placed_local_bound(const Node& item, const Layer& layer, const Aabb& local, bool with_copies, + const math::cfloat4x4* layer_m = nullptr) { if (local.empty()) return local; const math::cfloat4x4 lm = layer_m ? *layer_m : layer_matrix(layer); @@ -957,36 +999,10 @@ Aabb geometry_bound(const Node& item, const Layer& layer, bool with_copies, // multiplies the result before the layer's placement does. A bound that // missed either would be tight around a shape the item no longer is, and // the cull would drop a squashed cylinder that is on screen. - const math::cfloat4x4 axes = math::scale_matrix(item.scale_axes); Aabb bound = local.transformed(math::mul(lm, item_matrix(item))); - if (with_copies && item.mirror && layer.mirror_axes != 0) { - for (int axis = 0; axis < 3; ++axis) { - if (!(layer.mirror_axes & (1u << axis))) continue; - // The copy's map, on the order emit_item uses: the reflection acts - // in the layer's LOCAL space, so the layer's per-axis scale is - // outside it. - math::cfloat4x4 m = math::mul( - lm, math::mul(math::reflection_matrix(axis), math::mul(item.xform.matrix(), axes))); - bound.expand(local.transformed(m)); - } - bound = bound.dilated(kernel::csmin_quadratic_support(layer.mirror_k)); - } - // Every copy the radial mode emits, for the same reason: a bound that - // misses a copy lets the cull drop an item that is on screen. A rotated box - // is not axis-aligned, so each copy contributes the AABB OF the rotated box - // — it over-covers, which costs cull precision and never correctness. - if (with_copies && item.mirror && layer.radial_count > 1) { - const int axis = layer.radial_axis < 3 ? layer.radial_axis : 1; - const int count = static_cast(layer.radial_count); - for (int k = 1; k < count; ++k) { - const float angle = - 6.2831853071795864769f * static_cast(k) / static_cast(count); - math::cfloat4x4 m = - math::mul(lm, math::mul(math::rotation_matrix(axis, angle), - math::mul(item.xform.matrix(), axes))); - bound.expand(local.transformed(m)); - } - bound = bound.dilated(kernel::csmin_quadratic_support(layer.radial_k)); + if (with_copies && item.mirror) { + expand_by_mirror_copies(item, layer, local, lm, &bound); + expand_by_radial_copies(item, layer, local, lm, &bound); } // Rounding is authored in item-local units (tape emits round*scale); // erosion (negative rounding) shrinks the surface, never the bound. @@ -1005,6 +1021,11 @@ Aabb geometry_bound(const Node& item, const Layer& layer, bool with_copies, return bound.dilated(kernel::cmax(round_world, 0.0f) + combine); } +Aabb geometry_bound(const Node& item, const Layer& layer, bool with_copies, + const math::cfloat4x4* layer_m = nullptr) { + return placed_local_bound(item, layer, item_local_bounds(item), with_copies, layer_m); +} + } // namespace Aabb item_geometry_bound(const Node& item, const Layer& layer) { @@ -1931,6 +1952,197 @@ std::optional item_geometry_reach_in_document(const Document& doc, return out; } +// -- where a change at the HEAD of a deformer chain lands (issue #639) -------- + +namespace { + +// Does this easing curve return EXACTLY zero at the rim of a region, on every +// backend? `cregion_weight` is `cease(ease, clamp(1 - d/r, 0, 1))`, so outside +// the ball the weight is `cease(ease, 0)` -- and the whole argument below is +// that it is zero there, so the link is the identity. +// +// Two conditions, and the second is the one that is easy to miss. The HOST +// must compute zero -- `ease_out_sine` is `cos(pi/2)` there, which is not +// zero in float, and bounds.cpp already records one easing that returns +// 5.96e-08 at its zero end. And the value must not depend on HOW a backend +// evaluates it: a transcendental (sin, cos, exp2, sqrt) or a multi-term +// polynomial at the rim can round differently under a GPU's fast math or a +// fused multiply-add, and the host's zero would then say nothing about the +// device's. So the families whose rim runs through one are refused outright, +// whatever the host happens to compute; the rest reach the rim through a +// product with t = 0, a guarded branch, or `1 - 1*1*...`, which every IEEE +// backend evaluates exactly. +bool ease_is_zero_at_rim(std::uint8_t ease) { + switch (ease) { + case kernel::ease_in_sine: + case kernel::ease_out_sine: + case kernel::ease_in_out_sine: + case kernel::ease_out_expo: + case kernel::ease_in_circ: + case kernel::ease_out_circ: + case kernel::ease_in_out_circ: + case kernel::ease_in_bounce: + case kernel::ease_in_out_bounce: + return false; + default: + return ease < kernel::ease_count && kernel::cease(ease, 0.0f) == 0.0f; + } +} + +// Where one link can change the chain's output, in the chain's own frame -- +// or nullopt when that is not a ball. +// +// The links that qualify are the ones whose KERNEL returns its input untouched +// where the weight is zero: grab and magnify early-out to `p` (a warp), blob +// and alpha to an offset of exactly 0. That is `link_support`'s finite set +// less RADIAL POSE, which has the same finite weight and no early-out -- +// outside its ball it returns `centre + (p - centre)` turned by zero, and that +// is not `p` in float. An ulp is enough to move a stored fp16 brick, so pose +// keeps the node's bound. Everything else acts everywhere. +// +// A GRAB ALSO REPORTS ITS DISPLACED END. The kernel's weight is read at the +// sample point, so the identity argument needs only the ball at the centre; +// the far end is margin, and it is the margin clay.h has always promised for a +// move ("two ends"), and what the live Move reports for the same segment +// (`move_surface_impl` dilates its ball by the pull). Undo reporting less than +// the gesture that made the edit would be a surprise nobody could debug. +// +// The rim margin covers `1 - d/r` rounding to a hair above zero for a point a +// hair outside the ball. +bool link_is_exact_identity_outside(const Deformer& d) { + return d.type == kernel::cdeform_grab || d.type == kernel::cdeform_magnify || + d.type == kernel::cdeform_blob || d.type == kernel::cdeform_alpha; +} + +std::optional link_change_support(const Deformer& d) { + if (!link_is_exact_identity_outside(d) || !ease_is_zero_at_rim(d.ease)) return std::nullopt; + const LinkSupport s = link_support(d); + if (!s.finite) return std::nullopt; + const float r = s.radius * 1.001f + 1e-5f; + Aabb b{s.centre - cf3(r, r, r), s.centre + cf3(r, r, r)}; + if (d.type == kernel::cdeform_grab) { + const cfloat3 end = s.centre + cf3(d.ext[0], d.ext[1], d.ext[2]); + b.expand(Aabb{end - cf3(r, r, r), end + cf3(r, r, r)}); + } + return b; +} + +bool same_bits(float a, float b) { + return std::bit_cast(a) == std::bit_cast(b); +} + +bool same_point(const cfloat3& a, const cfloat3& b) { + return same_bits(a.x, b.x) && same_bits(a.y, b.y) && same_bits(a.z, b.z); +} + +// The payloads that do not fit the record: a bend_curve's guide, a lattice's +// cage and its placement. Compared in full, so a lattice or a curve in the +// COMMON TAIL -- a grab put at the front of a chain that ends in one -- is +// stripped like any other link rather than ending the tail early and refusing. +bool same_guide(const std::vector& a, const std::vector& b) { + if (a.size() != b.size()) return false; + for (std::size_t i = 0; i < a.size(); ++i) { + if (!same_point(a[i].pos, b[i].pos) || !same_bits(a[i].radius, b[i].radius) || + a[i].type != b[i].type || !same_point(a[i].in_handle, b[i].in_handle) || + !same_point(a[i].out_handle, b[i].out_handle)) + return false; + } + return true; +} + +bool same_cage(const Deformer& a, const Deformer& b) { + if (a.cage.size() != b.cage.size()) return false; + for (std::size_t i = 0; i < a.cage.size(); ++i) + if (!same_point(a.cage[i], b.cage[i])) return false; + const math::Transform& ta = a.cage_xform; + const math::Transform& tb = b.cage_xform; + return same_point(ta.position, tb.position) && same_bits(ta.scale, tb.scale) && + same_bits(ta.rotation.x, tb.rotation.x) && same_bits(ta.rotation.y, tb.rotation.y) && + same_bits(ta.rotation.z, tb.rotation.z) && same_bits(ta.rotation.w, tb.rotation.w); +} + +bool same_stamp(const AlphaStamp& sa, const AlphaStamp& sb) { + return sa.width == sb.width && sa.height == sb.height && same_bits(sa.extent, sb.extent) && + same_bits(sa.radius, sb.radius) && same_bits(sa.amplitude, sb.amplitude) && + sa.samples == sb.samples; +} + +// Two links the kernel evaluates identically: every field it reads, bit for +// bit (`gesture_id` is a host's bookkeeping and is never evaluated). +bool same_link(const Deformer& a, const Deformer& b) { + if (a.type != b.type || a.ease != b.ease) return false; + if (!same_bits(a.k, b.k) || !same_bits(a.a, b.a) || !same_bits(a.b, b.b) || + !same_bits(a.c, b.c)) + return false; + for (int i = 0; i < 6; ++i) + if (!same_bits(a.ext[i], b.ext[i])) return false; + return same_guide(a.guide, b.guide) && same_cage(a, b) && same_stamp(a.stamp, b.stamp); +} + +// The region, in the chain's frame, outside which two chains agree. +// +// The chain runs p -> d0 -> d1 -> ... -> prim. Strip the longest common TAIL: +// what is left is a head on each side. A finite-support link is the identity +// outside its ball, so a head made only of them hands the tail the point +// UNCHANGED wherever the point is outside every one of their balls (link 0 +// leaves it alone, so link 1 sees it where it was, and so on) -- and an offset +// link adds exactly zero there. Both chains then evaluate the same tail at the +// same point: the field is bit-identical outside the union of the balls. +// +// A head that is empty on both sides changes nothing (an empty box). A head +// holding any other kind is nullopt: a twist ahead of a grab moves the point +// before the grab's ball is tested, so the region is the twist's preimage of +// the ball and not the ball. +std::optional chain_head_change(const std::vector& before, + const std::vector& after) { + std::size_t i = before.size(), j = after.size(); + while (i > 0 && j > 0 && same_link(before[i - 1], after[j - 1])) { + --i; + --j; + } + Aabb local; + for (const auto& [chain, head] : {std::pair{&before, i}, std::pair{&after, j}}) { + for (std::size_t k = 0; k < head; ++k) { + const std::optional s = link_change_support((*chain)[k]); + if (!s) return std::nullopt; + local.expand(*s); + } + } + return local; +} + +} // namespace + +std::optional deformer_head_reach_in_document(const Document& doc, + const SdfContent& content, NodeId id, + const std::vector& before, + const std::vector& after) { + const Node* n = content.find(id); + if (!n || n->is_group) return std::nullopt; + std::optional local = chain_head_change(before, after); + if (!local || local->empty()) return local; + // Repetition folds the point BEFORE the chain sees it, so the ball recurs + // in every cell -- the sweep item_local_bounds applies to the whole item. + if (n->repeat.is_infinite_grid()) return std::nullopt; + if (n->repeat.active()) local = repeated_local_bounds(*local, n->repeat); + + // Per placement, the three dilations every other reach takes, from the + // functions that define them: the item's own placement, symmetry copies, + // rounding and combine support (placed_local_bound, the geometry bound's + // own body); once per enclosing group (dilate_by_ancestors, node_reach_bound's + // walk); and every fold from the layer up (layer_reach_in_document). + Aabb out; + for (const Layer& l : doc.layers) { + if (l.sdf.get() != &content) continue; + Aabb b = placed_local_bound(*n, l, *local, /*with_copies=*/true); + if (!dilate_by_ancestors(content, id, l, &b)) return std::nullopt; + out.expand(layer_reach_in_document(doc, l.id, b)); + } + if (out.empty()) return out; + if (out.is_infinite() || !box_is_sane(out)) return std::nullopt; + return out; +} + Aabb layer_reach_in_document(const Document& doc, LayerId layer_id, const Aabb& in_layer) { // Empty stays empty (nothing changed, so nothing reaches) and infinite // stays infinite (dilating FLT_MAX overflows to the same claim, badly). diff --git a/src/scene/commands.cpp b/src/scene/commands.cpp index ab6a4276..2e100628 100644 --- a/src/scene/commands.cpp +++ b/src/scene/commands.cpp @@ -546,6 +546,16 @@ std::optional command_surface_delta_bound(const Document& doc, const return item_geometry_reach_in_document(doc, *l->sdf, c->node); } +std::optional command_head_delta_bound(const Document& doc, const Command& cmd) { + const auto* c = std::get_if(&cmd); + if (!c) return std::nullopt; + const Layer* l = doc.find_layer(c->layer); + if (!l || l->kind != LayerKind::Sdf || !l->sdf) return std::nullopt; + const Node* n = l->sdf->find(c->node); + if (!n) return std::nullopt; + return deformer_head_reach_in_document(doc, *l->sdf, c->node, n->deformers, c->deformers); +} + math::Aabb command_influence_bound(const Document& doc, const Command& cmd, LayerExtent* extent) { return std::visit( @@ -1833,25 +1843,57 @@ bool UndoStack::perform(Document& doc, const Command& cmd) { namespace { -// Both sides of one command, unioned into the step's bound. Unioning with -// math::Aabb::infinite() yields it back — it is maximal on every axis — so -// "one command was unbounded" needs no case of its own. A null bound is a -// caller that did not ask, and costs nothing. -void widen(math::Aabb* bound, const Document& doc, const Command& cmd) { - if (bound) bound->expand(command_influence_bound(doc, cmd)); +// `head` pulled into `reach`, corner by corner: each corner clamped into +// `reach`'s box. Where the two overlap on an axis that is their overlap; where +// they MISS on an axis it is the face of `reach` nearest `head` -- NOT nothing. +// +// Not the intersection, and the difference is a correctness one. `reach` is +// reported without the band, which every consumer adds (mark_dirty dilates by +// it), so the node can change the field anywhere within a band OUTSIDE its box +// -- and a head's ball sitting there changes it. The intersection is empty for +// such a ball and the host dirtied nothing: a magnify just past a node's face, +// undone, left a brick stale. The clamped box, dilated by any band, covers the +// part of the ball within that band of `reach`, whatever the band is; and it +// is still inside `reach`, so never larger than the node's bound. An infinite +// `reach` leaves `head` as it is; an empty one, or an empty head, is nothing. +math::Aabb head_within(const math::Aabb& reach, const math::Aabb& head) { + if (reach.empty() || head.empty()) return math::Aabb{}; + return math::Aabb{kernel::cmin(kernel::cmax(head.min, reach.min), reach.max), + kernel::cmax(kernel::cmin(head.max, reach.max), reach.min)}; +} + +// Apply one command and return what it touched: its influence bound on both +// sides, unioned -- an add's node is not there before, a removal's is not +// there after, and a move has two ends. Unioning with math::Aabb::infinite() +// yields it back, so "one command was unbounded" needs no case of its own. +// +// NARROWED for a deformer chain whose head changed (issue #639): the target of +// that command is the node, whose whole bound is what this used to report for +// one Move segment. `command_head_delta_bound` is where the field can actually +// have changed, and it is clamped into the node's bound (`head_within`) rather +// than substituted, so the result is never larger than the node's bound either. +math::Aabb apply_bounded(Document& doc, const Command& cmd, std::optional* inverse) { + math::Aabb reach = command_influence_bound(doc, cmd); + const std::optional head = command_head_delta_bound(doc, cmd); + *inverse = scene::apply(doc, cmd); + reach.expand(command_influence_bound(doc, cmd)); + return head ? head_within(reach, *head) : reach; } } // namespace // Replay one stack entry onto the document, collecting the inverses for the // opposite stack and the region the whole step touched. Undo and redo differ -// only in which stack they came from, so they share this. +// only in which stack they came from, so they share this. A null bound is a +// caller that did not ask, and costs nothing. UndoStack::Entry UndoStack::replay(Document& doc, const Entry& entry, math::Aabb* bound) { Entry opposite; for (auto it = entry.inverses.rbegin(); it != entry.inverses.rend(); ++it) { - widen(bound, doc, *it); - std::optional inverse = scene::apply(doc, *it); - widen(bound, doc, *it); + std::optional inverse; + if (bound) + bound->expand(apply_bounded(doc, *it, &inverse)); + else + inverse = scene::apply(doc, *it); if (inverse) opposite.inverses.push_back(std::move(*inverse)); } return opposite; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5e9faaa6..a50041ca 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -203,6 +203,8 @@ add_executable(clay_unit_tests unit/test_c_uniform_gate.cpp unit/test_c_brick.cpp unit/test_c_undo_bound.cpp + unit/test_c_undo_bound_grab_support.cpp + unit/test_deformer_head_reach.cpp unit/test_c_out_descriptors.cpp unit/test_c_brick_atlas.cpp unit/test_c_brick_lod_mesh.cpp diff --git a/tests/unit/test_c_undo_bound_grab_support.cpp b/tests/unit/test_c_undo_bound_grab_support.cpp new file mode 100644 index 00000000..28aa37c4 --- /dev/null +++ b/tests/unit/test_c_undo_bound_grab_support.cpp @@ -0,0 +1,860 @@ +#include + +#include +#include +#include +#include +#include +#include + +#include "clay.h" + +// What undoing ONE grab of a Move must refill (issue #639). +// +// A host Move is a grab deformer put at the head of a node's chain, one per +// segment, and the host dirties what clay_document_undo_bound reports. Through +// 0.120.0 that was the NODE's whole influence bound -- the command's target -- +// so undoing one segment refilled every brick of the node, and the node's +// bound itself grows with every grab (each one dilates it by its pull). Undo +// cost was bricks-in-the-node times chain length. +// +// A grab is the identity outside its own ball, so the field cannot change +// anywhere else. This file pins both halves of that through the C ABI, which is +// the path the host takes: +// +// - the COUNT: undoing one small grab on a large node marks roughly the +// grab's ball, and the same number whatever the node's size and however +// long its chain; +// - the ORACLE: a cache refilled over that bound is bit-identical to a cache +// built from nothing on the undone document -- at a smooth-union seam, on +// a mirrored layer, inside a blended group, under a smooth fold, and for a +// grab whose ball crosses the node's own box. That is the never-tighter +// contract, checked on the stored payload rather than argued. + +namespace { + +constexpr float kVoxel = 0.05f; +constexpr int kDim = 8; +constexpr std::size_t kSamples = 8 * 8 * 8; +constexpr int kBandVoxels = 3; + +struct Doc { + clay_document* d = nullptr; + clay_layer_id layer = 0; + Doc() { + d = clay_document_create(); + REQUIRE(d != nullptr); + REQUIRE(clay_add_sdf_layer(d, "body", &layer) == CLAY_OK); + REQUIRE(clay_document_enable_undo(d) == CLAY_OK); + } + ~Doc() { clay_document_destroy(d); } + Doc(const Doc&) = delete; + Doc& operator=(const Doc&) = delete; +}; + +struct Cache { + clay_brick_cache* c = nullptr; + Cache() { + clay_brick_config cfg; + cfg.struct_size = sizeof(cfg); + REQUIRE(clay_brick_config_defaults(&cfg) == CLAY_OK); + cfg.dim = kDim; + cfg.voxel_size = kVoxel; + cfg.band_voxels = kBandVoxels; + cfg.memory_budget = 0; + c = clay_brick_cache_create(&cfg); + REQUIRE(c != nullptr); + } + ~Cache() { clay_brick_cache_destroy(c); } + Cache(const Cache&) = delete; + Cache& operator=(const Cache&) = delete; +}; + +struct Box { + float lo[3] = {0, 0, 0}; + float hi[3] = {0, 0, 0}; + std::int32_t has = 0; + std::int32_t infinite = 0; + float volume() const { return (hi[0] - lo[0]) * (hi[1] - lo[1]) * (hi[2] - lo[2]); } + bool contains(const Box& o) const { + for (int a = 0; a < 3; ++a) + if (o.lo[a] < lo[a] || o.hi[a] > hi[a]) return false; + return true; + } +}; + +Box ball_box(const float c[3], float r) { + Box b; + b.has = 1; + for (int a = 0; a < 3; ++a) { + b.lo[a] = c[a] - r; + b.hi[a] = c[a] + r; + } + return b; +} + +Box hull(const Box& a, const Box& b) { + Box out = a; + for (int i = 0; i < 3; ++i) { + out.lo[i] = std::fmin(a.lo[i], b.lo[i]); + out.hi[i] = std::fmax(a.hi[i], b.hi[i]); + } + return out; +} + +Box clip(Box b, const Box& to) { + for (int a = 0; a < 3; ++a) { + b.lo[a] = std::fmax(b.lo[a], to.lo[a]); + b.hi[a] = std::fmin(b.hi[a], to.hi[a]); + } + return b; +} + +bool same_box(const Box& a, const Box& b) { + for (int i = 0; i < 3; ++i) + if (a.lo[i] != b.lo[i] || a.hi[i] != b.hi[i]) return false; + return a.has == b.has && a.infinite == b.infinite; +} + +Box dilated(Box b, float r) { + for (int a = 0; a < 3; ++a) { + b.lo[a] -= r; + b.hi[a] += r; + } + return b; +} + +// One assertion site for every C call a helper makes. Doctest's macros each +// expand to a branch, so a helper spelling REQUIRE per call reads as far more +// complex than it is. +void ok(clay_result r) { REQUIRE(r == CLAY_OK); } + +clay_node_id add_sphere(Doc& doc, float radius, const float pos[3], float blend_k = 0.0f, + const clay_node_id* group = nullptr) { + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, &radius, 1); + REQUIRE(item != nullptr); + ok(clay_item_set_position(item, pos)); + if (blend_k > 0.0f) ok(clay_item_set_blend(item, CLAY_BLEND_QUADRATIC, blend_k)); + clay_node_id node = 0; + const clay_result r = group ? clay_layer_add_item_in_group(doc.d, doc.layer, *group, -1, item, + &node) + : clay_layer_add_item(doc.d, doc.layer, item, &node); + clay_item_destroy(item); + ok(r); + return node; +} + +// A "base" layer UNDER the document's own, holding one sphere at `under`, and +// the document's layer composed onto it with a quadratic smooth union of +// radius `k` -- so the edited layer's value passes through one fold. +void fold_onto_base(Doc& doc, float k, const float under[3]) { + clay_layer_id base = 0; + ok(clay_add_sdf_layer(doc.d, "base", &base)); + ok(clay_document_move_layer(doc.d, base, 0)); + ok(clay_document_set_layer_composition(doc.d, doc.layer, CLAY_OP_ADD, CLAY_BLEND_QUADRATIC, k, + 0.0f)); + const clay_layer_id keep = doc.layer; + doc.layer = base; + add_sphere(doc, 0.5f, under); + doc.layer = keep; +} + +// One Move segment, the way the host records it: a grab at the HEAD of the +// node's chain. Centre and pull are in the node's frame. +struct Grab { + float centre[3]; + float radius; + float pull[3]; +}; + +void add_grab(Doc& doc, clay_node_id node, const Grab& g) { + const float params[8] = {g.centre[0], g.centre[1], g.centre[2], g.radius, + g.pull[0], g.pull[1], g.pull[2], 0.0f}; + REQUIRE(clay_layer_add_deformer(doc.d, doc.layer, node, CLAY_DEFORM_GRAB, params, 8, + CLAY_EASE_LINEAR, 1) == CLAY_OK); +} + +// `count` grabs spread around the equator of a sphere node of the given +// radius, in its own frame and well away from the pole the test's own grab +// sits on -- the chain a long Move session leaves behind. +void add_chain(Doc& doc, clay_node_id node, float node_radius, int count) { + for (int i = 0; i < count; ++i) { + const float t = 6.2831853f * static_cast(i) / static_cast(count); + Grab g{{node_radius * std::cos(t), node_radius * std::sin(t), 0.0f}, + 0.2f, + {0.1f * std::cos(t), 0.1f * std::sin(t), 0.0f}}; + add_grab(doc, node, g); + } +} + +Box undo_bound(Doc& doc) { + Box b; + std::int32_t undone = 0; + REQUIRE(clay_document_undo_bound(doc.d, &undone, b.lo, b.hi, &b.has, &b.infinite) == CLAY_OK); + REQUIRE(undone == 1); + return b; +} + +Box redo_bound(Doc& doc) { + Box b; + std::int32_t redone = 0; + REQUIRE(clay_document_redo_bound(doc.d, &redone, b.lo, b.hi, &b.has, &b.infinite) == CLAY_OK); + REQUIRE(redone == 1); + return b; +} + +Box node_bound(const Doc& doc, clay_node_id node) { + Box b; + REQUIRE(clay_layer_node_influence_bound(doc.d, doc.layer, node, b.lo, b.hi, &b.has, + &b.infinite) == CLAY_OK); + return b; +} + +std::uint64_t dirty_count(const Cache& cache) { + clay_brick_stats stats; + std::memset(&stats, 0, sizeof stats); + stats.struct_size = sizeof stats; + REQUIRE(clay_brick_cache_stats(cache.c, &stats) == CLAY_OK); + return stats.dirty_bricks; +} + +// How many bricks a box marks in a fresh cache -- the count a host pays. +std::uint64_t bricks_marked(const Box& b) { + REQUIRE(b.has == 1); + REQUIRE(b.infinite == 0); + Cache cache; + REQUIRE(clay_brick_cache_mark_dirty(cache.c, b.lo, b.hi) == CLAY_OK); + return dirty_count(cache); +} + +// Mark what a step reported. "No bounds" marks nothing -- which is what a +// host does with it, and what the oracle then has to catch if it was wrong. +void mark(const Cache& cache, const Box& b) { + REQUIRE(b.infinite == 0); + if (b.has == 1) ok(clay_brick_cache_mark_dirty(cache.c, b.lo, b.hi)); +} + +// The host frame loop: drain, evaluate, submit. +std::size_t refill(const Cache& cache, const clay_document* doc) { + constexpr std::size_t chunk = 64; + std::vector reqs(chunk); + std::vector values(chunk * kSamples); + std::vector results(chunk); + std::size_t evaluated = 0; + for (;;) { + std::size_t count = chunk, remaining = 0; + ok(clay_brick_cache_take_dirty(cache.c, reqs.data(), &count, &remaining)); + if (count == 0) break; + ok(clay_brick_cache_eval_requests(doc, nullptr, reqs.data(), count, values.data(), + count * kSamples, nullptr, 0)); + std::size_t accepted = 0; + ok(clay_brick_cache_submit(cache.c, reqs.data(), count, values.data(), count * kSamples, + nullptr, 0, results.data(), &accepted)); + evaluated += count; + if (remaining == 0) break; + } + return evaluated; +} + +using Key = std::tuple; + +// Every brick key inside `world`, as brick coordinates. +std::vector keys_in(const Box& world) { + const float brick = kVoxel * static_cast(kDim); + int lo[3], hi[3]; + for (int a = 0; a < 3; ++a) { + lo[a] = static_cast(std::floor(world.lo[a] / brick)); + hi[a] = static_cast(std::floor(world.hi[a] / brick)); + } + std::vector keys; + for (int z = lo[2]; z <= hi[2]; ++z) + for (int y = lo[1]; y <= hi[1]; ++y) + for (int x = lo[0]; x <= hi[0]; ++x) { + keys.push_back(x); + keys.push_back(y); + keys.push_back(z); + } + return keys; +} + +// Every brick's state and, for a surface brick, its stored fp16 payload. +struct Snapshot { + std::vector states; + std::vector halves; +}; + +Snapshot snapshot(const Cache& cache, const std::vector& keys) { + const std::size_t n = keys.size() / 3; + Snapshot s; + s.states.assign(n, -1); + s.halves.assign(n * kSamples, 0); + REQUIRE(clay_brick_cache_read_bricks(cache.c, 0, keys.data(), n, 0, s.states.data(), + s.halves.data(), n * kSamples, nullptr, 0) == CLAY_OK); + return s; +} + +// Bricks whose state or payload differ between two snapshots of one key list. +std::size_t differing_bricks(const Snapshot& a, const Snapshot& b) { + std::size_t diff = 0; + for (std::size_t i = 0; i < a.states.size(); ++i) { + const bool same_state = a.states[i] == b.states[i]; + const bool same_values = + std::memcmp(&a.halves[i * kSamples], &b.halves[i * kSamples], + kSamples * sizeof(std::uint16_t)) == 0; + if (!same_state || !same_values) ++diff; + } + return diff; +} + +// A cache built from nothing on a COPY of the document. +// +// A copy and not the document itself, and that is load-bearing: the seed store +// belongs to the DOCUMENT (clay.h, clay_resume_stats), and a refill resumes +// from any seed the invalidation left alone. So a cache rebuilt on the same +// document after a too-narrow bound resumes from the very seeds the bound +// failed to drop, reproduces the stale bricks exactly, and agrees with them. +// Measured while writing this file: with the mirror image dropped from the +// bound, the same-document oracle reported zero stale bricks. +Snapshot rebuilt(const Doc& doc, const Box& world, const std::vector& keys) { + clay_blob* blob = nullptr; + ok(clay_document_save_memory(doc.d, &blob)); + clay_document* copy = nullptr; + const clay_result r = clay_document_load_memory(clay_blob_data(blob), clay_blob_size(blob), + ©); + clay_blob_destroy(blob); + ok(r); + Cache fresh; + mark(fresh, world); + refill(fresh, copy); + Snapshot s = snapshot(fresh, keys); + clay_document_destroy(copy); + return s; +} + +// THE ORACLE, for one step in one direction. `world` must hold everything the +// document draws; both caches are marked over it so their key sets agree. +struct OracleResult { + std::size_t stale = 0; // bricks the bound-refill got wrong + std::size_t edit_visible = 0; // bricks the step changed at all + std::size_t refilled = 0; // bricks the bound made the host refill + std::size_t tracked = 0; // bricks in `world` +}; + +template +OracleResult check_step(Doc& doc, const Box& world, Step step) { + const std::vector keys = keys_in(world); + Cache kept; + mark(kept, world); + refill(kept, doc.d); + const Snapshot before = snapshot(kept, keys); + + mark(kept, step(doc)); + OracleResult r; + r.refilled = refill(kept, doc.d); + + const Snapshot oracle = rebuilt(doc, world, keys); + r.stale = differing_bricks(snapshot(kept, keys), oracle); + r.edit_visible = differing_bricks(before, oracle); + r.tracked = keys.size() / 3; + return r; +} + +// One step, checked: it changed something (or the pass would be for the wrong +// reason) and the bound-refill agrees with the rebuild everywhere. +template +void check_exact(Doc& doc, const Box& world, Step step) { + const OracleResult r = check_step(doc, world, step); + CAPTURE(r.refilled); + CAPTURE(r.tracked); + REQUIRE(r.edit_visible > 0); + CHECK(r.stale == 0); +} + +// Undo, then redo, each checked against its own full rebuild. +void check_undo_and_redo(Doc& doc, const Box& world) { + check_exact(doc, world, undo_bound); + check_exact(doc, world, redo_bound); +} + +} // namespace + +// --------------------------------------------------------------------------- +// the box +// --------------------------------------------------------------------------- + +TEST_CASE("undoing one grab reports its ball at both ends, dilated by the folds above") { + Doc doc; + // A base layer beneath and the edited layer composed onto it with a SMOOTH + // union, so the edited layer's value passes through one fold -- a + // quadratic blend of radius k, whose support is 4k (kernel/ops.h) -- and + // the bound must carry it. + const float fold_k = 0.1f; + const float fold_support = 4.0f * fold_k; + const float under[3] = {0.0f, -6.0f, 0.0f}; + fold_onto_base(doc, fold_k, under); + const float at[3] = {0.0f, 0.0f, 0.0f}; + const float node_radius = 3.0f; + const clay_node_id node = add_sphere(doc, node_radius, at); + add_chain(doc, node, node_radius, 8); + // A sideways pull at the pole, so both ends of it lie well inside the + // node's box on the axis they differ along. + const Grab g{{0.0f, 0.0f, node_radius}, 0.25f, {0.3f, 0.0f, 0.0f}}; + add_grab(doc, node, g); + const Box node_before = node_bound(doc, node); + + const Box b = undo_bound(doc); + REQUIRE(b.has == 1); + REQUIRE(b.infinite == 0); + + // Both ends of the pull -- the centre's ball and the displaced end's -- + // each dilated by the fold's whole support, as far as the node's own + // bound reaches: past it the node's field is beyond the band on both sides + // of the undo, which is the contract every influence bound already makes. + // The node sits at the origin with no rotation or scale, so its frame is + // the world's. + const float end[3] = {g.centre[0] + g.pull[0], g.centre[1] + g.pull[1], + g.centre[2] + g.pull[2]}; + const Box ends = hull(ball_box(g.centre, g.radius), ball_box(end, g.radius)); + CHECK(b.contains(clip(dilated(ends, fold_support), node_before))); + + // ...and NO LARGER than the node's own bound: this narrows the command's + // target, it never widens it. + CHECK(node_before.contains(b)); + // The point of the change: a small fraction of the node, not the node. + CHECK(b.volume() < 0.05f * node_before.volume()); +} + +TEST_CASE("redo of one grab reports the same box its undo did") { + Doc doc; + const float at[3] = {0.0f, 0.0f, 0.0f}; + const clay_node_id node = add_sphere(doc, 2.0f, at); + add_chain(doc, node, 2.0f, 4); + add_grab(doc, node, Grab{{0.0f, 0.0f, 2.0f}, 0.25f, {0.0f, 0.0f, 0.2f}}); + const Box u = undo_bound(doc); + const Box r = redo_bound(doc); + CHECK(same_box(u, r)); +} + +// --------------------------------------------------------------------------- +// the count +// --------------------------------------------------------------------------- + +namespace { + +// The issue's shape: a node of radius `node_radius` whose TOP sits at z = 1.5 +// whatever its size, carrying `chain` grabs around its equator, and one more +// small grab on that top. Returns the bricks undoing that one grab marks. +struct UndoCount { + std::uint64_t by_undo = 0; + std::uint64_t by_node = 0; +}; + +UndoCount undo_one_grab(float node_radius, int chain) { + Doc doc; + const float at[3] = {0.0f, 0.0f, 1.5f - node_radius}; + const clay_node_id node = add_sphere(doc, node_radius, at); + add_chain(doc, node, node_radius, chain); + add_grab(doc, node, Grab{{0.0f, 0.0f, node_radius}, 0.2f, {0.0f, 0.0f, 0.1f}}); + UndoCount c; + c.by_node = bricks_marked(node_bound(doc, node)); + c.by_undo = bricks_marked(undo_bound(doc)); + return c; +} + +} // namespace + +TEST_CASE("undoing one grab marks a count independent of the node's size and its chain") { + // The grab's own ball at both ends, marked directly: the floor. + const float c[3] = {0.0f, 0.0f, 1.5f}; + const float e[3] = {0.0f, 0.0f, 1.6f}; + const std::uint64_t ball = bricks_marked(hull(ball_box(c, 0.2f), ball_box(e, 0.2f))); + + const UndoCount small = undo_one_grab(1.5f, 1); + const UndoCount large = undo_one_grab(6.0f, 1); + const UndoCount long_chain = undo_one_grab(6.0f, 40); + CAPTURE(ball); + CAPTURE(small.by_undo); + CAPTURE(small.by_node); + CAPTURE(large.by_undo); + CAPTURE(large.by_node); + CAPTURE(long_chain.by_undo); + CAPTURE(long_chain.by_node); + + // The node's own count grows with the node and with the chain: this is the + // cost the issue measured, and the fixture has to show it or the equalities + // below would hold for the wrong reason. + REQUIRE(large.by_node > 4 * small.by_node); + REQUIRE(long_chain.by_node > large.by_node); + + // The undo marks the SAME bricks at every size and chain length... + CHECK(large.by_undo == small.by_undo); + CHECK(long_chain.by_undo == small.by_undo); + // ...and that number is the grab's ball, not the node. + CHECK(small.by_undo >= ball); + CHECK(small.by_undo <= 2 * ball); + CHECK(long_chain.by_undo * 20 < long_chain.by_node); +} + +// --------------------------------------------------------------------------- +// the oracle: never tighter than what changed +// --------------------------------------------------------------------------- + +TEST_CASE("a grab undone at a smooth-union seam refills exactly what a rebuild does") { + Doc doc; + const float a[3] = {-0.45f, 0.0f, 0.0f}; + const float b[3] = {0.45f, 0.0f, 0.0f}; + add_sphere(doc, 0.5f, a); + const clay_node_id node = add_sphere(doc, 0.5f, b, 0.3f); + add_chain(doc, node, 0.5f, 3); + // On the seam (world x = 0), pulling across it. + add_grab(doc, node, Grab{{-0.45f, 0.0f, 0.3f}, 0.3f, {0.0f, 0.0f, 0.2f}}); + Box world; + world.has = 1; + for (int i = 0; i < 3; ++i) { + world.lo[i] = -1.4f; + world.hi[i] = 1.4f; + } + check_undo_and_redo(doc, world); +} + +TEST_CASE("a grab undone on a mirrored layer refills its reflection as well") { + Doc doc; + REQUIRE(clay_set_layer_mirror(doc.d, doc.layer, 1, 0, 0, 0.1f) == CLAY_OK); + // Off the plane, so the item and its reflection are apart and a bound + // covering only the item's own side would leave the reflection stale. + const float at[3] = {0.8f, 0.0f, 0.0f}; + const clay_node_id node = add_sphere(doc, 0.5f, at); + add_chain(doc, node, 0.5f, 3); + add_grab(doc, node, Grab{{0.0f, 0.0f, 0.5f}, 0.25f, {0.0f, 0.0f, 0.2f}}); + Box world; + world.has = 1; + for (int i = 0; i < 3; ++i) { + world.lo[i] = -1.6f; + world.hi[i] = 1.6f; + } + check_undo_and_redo(doc, world); +} + +TEST_CASE("a grab undone inside a blended group refills exactly what a rebuild does") { + Doc doc; + // A root node beneath the group, so the group's blend really combines + // (issue #515) and its support is a term. + const float under[3] = {0.0f, -0.9f, 0.0f}; + add_sphere(doc, 0.5f, under); + clay_node_id group = 0; + REQUIRE(clay_layer_add_group(doc.d, doc.layer, 0, -1, CLAY_OP_ADD, CLAY_BLEND_QUADRATIC, 0.3f, + 0.0f, &group) == CLAY_OK); + const float sib[3] = {-0.6f, 0.0f, 0.0f}; + add_sphere(doc, 0.4f, sib, 0.0f, &group); + const float at[3] = {0.3f, 0.0f, 0.0f}; + const clay_node_id node = add_sphere(doc, 0.5f, at, 0.0f, &group); + add_chain(doc, node, 0.5f, 3); + // On top of the seam with the sibling (world x = -0.2), pulling up. + add_grab(doc, node, Grab{{-0.5f, 0.3f, 0.0f}, 0.3f, {0.0f, 0.2f, 0.0f}}); + Box world; + world.has = 1; + for (int i = 0; i < 3; ++i) { + world.lo[i] = -1.6f; + world.hi[i] = 1.6f; + } + check_undo_and_redo(doc, world); +} + +TEST_CASE("a grab undone under a smooth fold refills exactly what a rebuild does") { + Doc doc; + const float under[3] = {0.0f, 0.0f, -0.7f}; + fold_onto_base(doc, 0.3f, under); + const float at[3] = {0.0f, 0.0f, 0.2f}; + const clay_node_id node = add_sphere(doc, 0.5f, at); + add_chain(doc, node, 0.5f, 3); + // On the neck the fold welds between the two layers, pulling outward. + add_grab(doc, node, Grab{{0.45f, 0.0f, -0.3f}, 0.3f, {0.2f, 0.0f, 0.0f}}); + Box world; + world.has = 1; + for (int i = 0; i < 3; ++i) { + world.lo[i] = -1.4f; + world.hi[i] = 1.4f; + } + check_undo_and_redo(doc, world); +} + +TEST_CASE("a grab whose ball crosses the node's own box refills exactly what a rebuild does") { + Doc doc; + const float at[3] = {0.0f, 0.0f, 0.0f}; + const clay_node_id node = add_sphere(doc, 0.5f, at); + // Centred OUTSIDE the node's box and reaching into it: the part of the ball + // beyond the box sees no material before the pull and some after. + add_grab(doc, node, Grab{{0.0f, 0.0f, 0.75f}, 0.5f, {0.0f, 0.0f, -0.3f}}); + add_grab(doc, node, Grab{{0.6f, 0.0f, 0.0f}, 0.4f, {0.3f, 0.0f, 0.0f}}); + Box world; + world.has = 1; + for (int i = 0; i < 3; ++i) { + world.lo[i] = -1.4f; + world.hi[i] = 1.4f; + } + // Both grabs, newest first -- each reaches past the node's box -- then both + // back again. + for (int i = 0; i < 2; ++i) check_exact(doc, world, undo_bound); + for (int i = 0; i < 2; ++i) check_exact(doc, world, redo_bound); +} + + +// --------------------------------------------------------------------------- +// the other finite-support kinds, the host's own Move, and mixed steps +// --------------------------------------------------------------------------- + +namespace { + +Box cube(float half) { + Box b; + b.has = 1; + for (int i = 0; i < 3; ++i) { + b.lo[i] = -half; + b.hi[i] = half; + } + return b; +} + +void add_deformer(Doc& doc, clay_node_id node, int32_t kind, const std::vector& params, + int32_t ease = CLAY_EASE_LINEAR) { + REQUIRE(clay_layer_add_deformer(doc.d, doc.layer, node, kind, params.data(), params.size(), + ease, 1) == CLAY_OK); +} + +// The undo marked something, and at least `factor` times fewer bricks than +// the node's bound. +void check_narrowed(const UndoCount& c, std::uint64_t factor) { + CAPTURE(c.by_undo); + CAPTURE(c.by_node); + CHECK((c.by_undo > 0 && c.by_undo * factor < c.by_node)); +} + +// A sphere at the origin carrying one alpha stamp on its +z pole. +clay_node_id add_alpha_sphere(Doc& doc, float r) { + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, &r, 1); + REQUIRE(item != nullptr); + std::vector stamp(16 * 16); + for (std::size_t i = 0; i < stamp.size(); ++i) stamp[i] = static_cast(i % 7) / 7.0f; + const float centre[3] = {0.0f, 0.0f, r}; + const float dir[3] = {0.0f, 0.0f, 1.0f}; + const float tangent[3] = {1.0f, 0.0f, 0.0f}; + ok(clay_item_add_alpha(item, stamp.data(), 16, 16, centre, dir, tangent, 0.4f, 0.3f, 0.05f, + CLAY_EASE_LINEAR)); + clay_node_id node = 0; + const clay_result added = clay_layer_add_item(doc.d, doc.layer, item, &node); + clay_item_destroy(item); + ok(added); + return node; +} + +// One segment of the host's Move: a clay_layer_move_surface call that reaches +// exactly one item. +void move_segment(Doc& doc, const float centre[3], const float pull[3], float radius) { + clay_move_params p; + std::memset(&p, 0, sizeof p); + p.struct_size = sizeof p; + p.radius = radius; + std::size_t applied = 0; + ok(clay_layer_move_surface(doc.d, doc.layer, centre, pull, &p, &applied)); + CHECK(applied == 1); +} + +// Undo one step and report both the bricks it marked and the node's. +UndoCount count_undo(Doc& doc, clay_node_id node) { + UndoCount c; + c.by_node = bricks_marked(node_bound(doc, node)); + c.by_undo = bricks_marked(undo_bound(doc)); + return c; +} + +} // namespace + +TEST_CASE("magnify and blob at the head are narrowed too, and stay exact") { + const float at[3] = {0.0f, 0.0f, 0.0f}; + const std::vector>> heads = { + {CLAY_DEFORM_MAGNIFY, {0.0f, 0.0f, 0.6f, 0.3f, 0.5f}}, + {CLAY_DEFORM_BLOB, {0.0f, 0.0f, 0.6f, 0.3f, 0.08f, 6.0f, 3.0f, 0.5f, 7.0f}}, + }; + for (const auto& [kind, params] : heads) { + CAPTURE(kind); + Doc doc; + const clay_node_id node = add_sphere(doc, 0.6f, at); + add_chain(doc, node, 0.6f, 3); + add_deformer(doc, node, kind, params); + check_undo_and_redo(doc, cube(1.4f)); + // Narrowed, and not to nothing: the step is undone again for the count. + check_narrowed(count_undo(doc, node), 2); + } +} + +TEST_CASE("an alpha taken off the head of a chain is narrowed and stays exact") { + // The one kind clay_layer_add_deformer does not take: it rides the item + // builder, and a placed node loses it through clay_layer_remove_deformer, + // which is a SetDeformersCmd like any other. + Doc doc; + const clay_node_id node = add_alpha_sphere(doc, 1.5f); + add_chain(doc, node, 1.5f, 3); + // The alpha is at the TAIL now; take off the three grabs ahead of it and + // then the alpha itself, which is then the head. + for (int i = 0; i < 4; ++i) ok(clay_layer_remove_deformer(doc.d, doc.layer, node, 0)); + check_undo_and_redo(doc, cube(2.0f)); + check_narrowed(count_undo(doc, node), 10); +} + +TEST_CASE("a head the argument does not cover keeps the node's bound") { + const float at[3] = {0.0f, 0.0f, 0.0f}; + SUBCASE("a radial pose, which is not the identity outside its ball") { + Doc doc; + const clay_node_id node = add_sphere(doc, 0.6f, at); + add_chain(doc, node, 0.6f, 3); + add_deformer(doc, node, CLAY_DEFORM_POSE, {0.0f, 0.0f, 0.6f, 0.3f, 0.0f, 1.0f, 0.0f, 0.5f}); + const UndoCount c = count_undo(doc, node); + CHECK(c.by_undo == c.by_node); + } + SUBCASE("a grab behind a twist, whose ball the twist has moved") { + Doc doc; + const clay_node_id node = add_sphere(doc, 0.6f, at); + add_deformer(doc, node, CLAY_DEFORM_TWIST, {0.5f}); + const float p[8] = {0.0f, 0.0f, 0.6f, 0.3f, 0.0f, 0.0f, 0.2f, 0.0f}; + REQUIRE(clay_layer_add_deformer(doc.d, doc.layer, node, CLAY_DEFORM_GRAB, p, 8, + CLAY_EASE_LINEAR, /*at_front=*/0) == CLAY_OK); + const UndoCount c = count_undo(doc, node); + CHECK(c.by_undo == c.by_node); + } +} + +TEST_CASE("a grab on an intersecting item is narrowed and stays exact") { + // An intersect's influence is its layer's extent, not its own box. The + // grab's argument does not care -- the item's own field is untouched + // outside the ball and max() is pointwise -- and the oracle says so. + Doc doc; + const float body[3] = {0.0f, 0.0f, 0.0f}; + add_sphere(doc, 0.7f, body); + const float cut_at[3] = {0.0f, 0.0f, 0.3f}; + float cr = 0.7f; + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, &cr, 1); + REQUIRE(clay_item_set_position(item, cut_at) == CLAY_OK); + REQUIRE(clay_item_set_op(item, CLAY_OP_INTERSECT) == CLAY_OK); + clay_node_id cutter = 0; + REQUIRE(clay_layer_add_item(doc.d, doc.layer, item, &cutter) == CLAY_OK); + clay_item_destroy(item); + add_grab(doc, cutter, Grab{{0.0f, 0.0f, -0.7f}, 0.3f, {0.0f, 0.0f, -0.15f}}); + check_undo_and_redo(doc, cube(1.4f)); +} + +TEST_CASE("a step that also carries another command still covers that command") { + Doc doc; + const float a_at[3] = {-0.6f, 0.0f, 0.0f}; + const float b_at[3] = {0.7f, 0.0f, 0.0f}; + const clay_node_id a = add_sphere(doc, 0.4f, a_at); + const clay_node_id b = add_sphere(doc, 0.3f, b_at); + REQUIRE(clay_document_begin_undo_group(doc.d) == CLAY_OK); + add_grab(doc, a, Grab{{0.0f, 0.4f, 0.0f}, 0.2f, {0.0f, 0.15f, 0.0f}}); + const float axis[3] = {0.0f, 1.0f, 0.0f}; + const float moved[3] = {0.7f, 0.0f, 0.6f}; + REQUIRE(clay_layer_set_transform(doc.d, doc.layer, b, moved, axis, 0.0f, 1.0f) == CLAY_OK); + REQUIRE(clay_document_end_undo_group(doc.d) == CLAY_OK); + // The transform's two ends are nowhere near the grab's ball: a bound that + // narrowed the whole step to the grab would leave them stale. + check_undo_and_redo(doc, cube(1.4f)); +} + +TEST_CASE("undoing one segment of the host's Move marks the segment, not the node") { + // The path the issue's host takes: clay_layer_move_surface, one call per + // segment, each leaving grabs at the head of the chains it reached. + Doc doc; + const float at[3] = {0.0f, 0.0f, 0.0f}; + const float node_radius = 1.5f; + const clay_node_id node = add_sphere(doc, node_radius, at); + for (int i = 0; i < 12; ++i) { + const float t = 6.2831853f * static_cast(i) / 12.0f; + const float c[3] = {node_radius * std::cos(t), node_radius * std::sin(t), 0.0f}; + const float d[3] = {0.08f * std::cos(t), 0.08f * std::sin(t), 0.0f}; + move_segment(doc, c, d, 0.2f); + } + const float top[3] = {0.0f, 0.0f, node_radius}; + const float up[3] = {0.0f, 0.0f, 0.08f}; + move_segment(doc, top, up, 0.2f); + + const float end[3] = {top[0] + up[0], top[1] + up[1], top[2] + up[2]}; + const std::uint64_t ball = bricks_marked(hull(ball_box(top, 0.2f), ball_box(end, 0.2f))); + const UndoCount c = count_undo(doc, node); + CAPTURE(ball); + CHECK((c.by_undo >= ball && c.by_undo <= 2 * ball)); + check_narrowed(c, 20); + redo_bound(doc); + check_undo_and_redo(doc, cube(2.0f)); +} + +namespace { + +enum class Tail { lattice, curve, twist }; + +// A sphere whose chain ends in a whole-item link -- a lattice cage, a bend +// curve or a twist -- with a short grab chain added AHEAD of it, the way a +// Move lands on a node that already carries one. +clay_node_id add_tailed_sphere(Doc& doc, Tail tail) { + const float at[3] = {0.0f, 0.0f, 0.0f}; + const clay_node_id node = add_sphere(doc, 0.6f, at); + if (tail == Tail::twist) { + const float k = 0.5f; + ok(clay_layer_add_deformer(doc.d, doc.layer, node, CLAY_DEFORM_TWIST, &k, 1, + CLAY_EASE_LINEAR, 0)); + } else if (tail == Tail::lattice) { + const float lo[3] = {-0.7f, -0.7f, -0.7f}; + const float hi[3] = {0.7f, 0.7f, 0.7f}; + std::vector offsets(2 * 2 * 2 * 3, 0.0f); + offsets[0] = 0.1f; // one corner dragged, so the cage is not the identity + ok(clay_layer_add_lattice(doc.d, doc.layer, node, lo, hi, 2, 2, 2, offsets.data(), 0)); + } else { + const float guide[9] = {0.0f, -0.7f, 0.0f, 0.1f, 0.0f, 0.0f, 0.0f, 0.7f, 0.0f}; + ok(clay_layer_add_bend_curve(doc.d, doc.layer, node, guide, 3, 0, 0.0f, 1.0f, 0)); + } + add_chain(doc, node, 0.6f, 3); + add_grab(doc, node, Grab{{0.0f, 0.0f, 0.6f}, 0.2f, {0.0f, 0.0f, 0.1f}}); + return node; +} + +} // namespace + +TEST_CASE("a grab ahead of a twist, a lattice or a bend curve in the common tail is narrowed") { + // The tail is the same on both sides of the step and sees the same point, + // whatever it does with it; a whole-item link there -- a payload link + // included -- must be stripped like any other rather than refuse. + for (const Tail tail : {Tail::lattice, Tail::curve, Tail::twist}) { + CAPTURE(static_cast(tail)); + Doc doc; + const clay_node_id node = add_tailed_sphere(doc, tail); + check_undo_and_redo(doc, cube(1.4f)); + check_narrowed(count_undo(doc, node), 4); + } +} + +TEST_CASE("a ball that misses the node's box but lies within the band of it is still refilled") { + // The node's bound is reported WITHOUT the band -- every consumer adds it + // (mark_dirty dilates by the band). So the field can change in the band + // OUTSIDE that box, and a link whose ball sits there changes it. Cutting + // the ball down to the part inside the box leaves nothing to dirty; the + // band the consumer adds must be able to reach the ball from what is + // reported. Found by a randomized oracle: a magnify just past a node's + // face, undone, reported no bounds and left one brick stale. + const float at[3] = {0.0f, 0.0f, 0.0f}; + for (const int32_t kind : {CLAY_DEFORM_MAGNIFY, CLAY_DEFORM_GRAB}) { + CAPTURE(kind); + Doc doc; + const clay_node_id node = add_sphere(doc, 0.5f, at); + const Box before = node_bound(doc, node); + if (kind == CLAY_DEFORM_MAGNIFY) + add_deformer(doc, node, kind, {0.0f, 0.0f, 0.62f, 0.1f, -0.6f}); + else + add_deformer(doc, node, kind, {0.0f, 0.0f, 0.66f, 0.08f, 0.0f, 0.0f, -0.03f, 0.0f}); + // The fixture's premise: the ball is clear of the node's box on z, and + // within the band (3 voxels, 0.15) of it. + const float ball_lo = kind == CLAY_DEFORM_MAGNIFY ? 0.52f : 0.58f; + const float top = node_bound(doc, node).hi[2]; + CAPTURE(top); + REQUIRE((top < ball_lo && before.hi[2] < ball_lo)); + REQUIRE(ball_lo - top < 0.15f); + check_undo_and_redo(doc, cube(1.0f)); + } +} diff --git a/tests/unit/test_deformer_head_reach.cpp b/tests/unit/test_deformer_head_reach.cpp new file mode 100644 index 00000000..9785f17f --- /dev/null +++ b/tests/unit/test_deformer_head_reach.cpp @@ -0,0 +1,416 @@ +#include + +#include +#include +#include +#include + +#include "clay/kernel/ease.h" +#include "clay/scene/bounds.h" +#include "clay/scene/commands.h" +#include "clay/scene/document.h" +#include "kernel_utils.h" +#include "scene_utils.h" + +// Where changing the HEAD of a deformer chain can change the field (#639). +// +// `deformer_head_reach_in_document` claims more than a band-clamped bound: a +// finite-support link is the identity outside its ball, so the RAW field is +// bit-identical outside the box it reports. That is the claim this file tests, +// against the reference evaluator, on the raw value -- a stronger check than +// the brick oracle in test_c_undo_bound_grab_support.cpp, which sees only what +// the cache stores. And it tests the refusals, which are what keep the answer +// from ever being a guess. + +using namespace clay; +using kernel::cf3; +using kernel::cfloat3; + +namespace { + +struct Fixture { + scene::Document doc; + scene::LayerId layer_id = 0; + scene::NodeId node = 0; + + scene::Layer& layer() { return *doc.find_layer(layer_id); } + scene::SdfContent& content() { return *layer().sdf; } + std::vector& chain() { return content().find_mut(node)->deformers; } +}; + +// A sphere of radius 0.6 at `at` in a fresh layer, carrying `chain`. +Fixture sphere_with(std::vector chain, cfloat3 at = cf3(0, 0, 0)) { + Fixture f; + scene::Layer& l = f.doc.add_sdf_layer("l"); + f.layer_id = l.id; + scene::Node n = clay_test::item(scene::Prim::sphere(0.6f), at); + n.deformers = std::move(chain); + f.node = l.sdf->insert(n); + return f; +} + +std::optional reach(Fixture& f, const std::vector& before, + const std::vector& after) { + return scene::deformer_head_reach_in_document(f.doc, f.content(), f.node, before, after); +} + +std::vector lattice(float extent, int side) { + std::vector pts; + for (int i = 0; i < side; ++i) + for (int j = 0; j < side; ++j) + for (int k = 0; k < side; ++k) { + auto f = [&](int n) { + return -extent + 2.0f * extent * static_cast(n) / + static_cast(side - 1); + }; + pts.push_back(cf3(f(i), f(j), f(k))); + } + return pts; +} + +struct Sampled { + std::size_t outside = 0; // lattice points outside the box + std::size_t moved = 0; // of those, how many changed at all + std::size_t changed = 0; // points anywhere whose value changed +}; + +// Swap the node's chain from `before` to `after` and compare the RAW field, +// bit for bit, at every lattice point outside `box`. +Sampled sample_outside(Fixture& f, const math::Aabb& box, const std::vector& before, + const std::vector& after) { + const std::vector pts = lattice(2.5f, 41); + f.chain() = before; + std::vector a; + for (const cfloat3& p : pts) a.push_back(clay_test::ref_eval_document(f.doc, p).d); + f.chain() = after; + Sampled s; + for (std::size_t i = 0; i < pts.size(); ++i) { + const float b = clay_test::ref_eval_document(f.doc, pts[i]).d; + const bool same = std::bit_cast(a[i]) == std::bit_cast(b); + if (!same) ++s.changed; + if (box.contains(pts[i])) continue; + ++s.outside; + if (!same) ++s.moved; + } + return s; +} + +// A finite, non-empty box, or the test has nothing to compare against. +math::Aabb finite_reach(Fixture& f, const std::vector& before, + const std::vector& after) { + const std::optional box = reach(f, before, after); + REQUIRE(box.has_value()); + REQUIRE((!box->empty() && !box->is_infinite())); + return *box; +} + +// The claim, for one fixture: the edit is visible somewhere (so a pass is not +// vacuous), the box leaves something outside it to compare, and the raw field +// is untouched everywhere out there. +void check_exact_outside(Fixture& f, const std::vector& before, + const std::vector& after) { + const Sampled s = sample_outside(f, finite_reach(f, before, after), before, after); + CAPTURE(s.changed); + CAPTURE(s.outside); + REQUIRE((s.changed > 0 && s.outside > 0)); + CHECK(s.moved == 0); +} + +scene::Deformer grab_at(cfloat3 c, float r, cfloat3 d) { return scene::Deformer::grab(c, r, d); } + +std::vector with_head(scene::Deformer head, std::vector tail) { + tail.insert(tail.begin(), head); + return tail; +} + +const std::vector kTail = { + grab_at(cf3(0.6f, 0, 0), 0.25f, cf3(0.1f, 0, 0)), + grab_at(cf3(-0.6f, 0, 0), 0.25f, cf3(-0.1f, 0, 0)), +}; + +} // namespace + +TEST_CASE("head reach: a grab at the head changes the raw field only inside its box") { + Fixture f = sphere_with(kTail); + const auto after = with_head(grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), kTail); + check_exact_outside(f, kTail, after); // redo: the grab appears + check_exact_outside(f, after, kTail); // undo: the grab goes +} + +TEST_CASE("head reach: magnify and blob qualify beside grab, and each is exact outside") { + const std::vector heads = { + scene::Deformer::magnify(cf3(0, 0.6f, 0), 0.3f, 0.5f), + scene::Deformer::blob(cf3(0, 0.6f, 0), 0.3f, 0.08f), + }; + for (const scene::Deformer& head : heads) { + CAPTURE(static_cast(head.type)); + Fixture f = sphere_with(kTail); + check_exact_outside(f, kTail, with_head(head, kTail)); + } +} + +TEST_CASE("head reach: a grab reports its displaced end as well as its centre") { + Fixture f = sphere_with({}); + const auto after = std::vector{grab_at(cf3(0, 0.6f, 0), 0.2f, cf3(0.5f, 0, 0))}; + const std::optional box = reach(f, {}, after); + REQUIRE(box.has_value()); + CHECK(box->contains(cf3(0.5f + 0.19f, 0.6f, 0))); // the far end's ball + CHECK(box->contains(cf3(-0.19f, 0.6f, 0))); // the centre's ball +} + +TEST_CASE("head reach: a radial pose is refused, because it is not the identity outside") { + // Finite support by its weight, and still not the identity past it: + // cpose_point has no zero-weight early-out, so outside the ball it returns + // `centre + (p - centre)` rotated by zero, which is NOT p in float. The + // field moves by an ulp everywhere the item is. Found by the raw check + // below: 1,917 of 68,796 lattice points outside the ball moved. + const auto pose = scene::Deformer::pose(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0, 1), 0.6f); + Fixture f = sphere_with(kTail); + CHECK_FALSE(reach(f, kTail, with_head(pose, kTail)).has_value()); + const Sampled s = sample_outside(f, math::Aabb{cf3(-0.3f, 0.3f, -0.3f), cf3(0.3f, 0.9f, 0.3f)}, + kTail, with_head(pose, kTail)); + CHECK(s.moved > 0); // the reason, pinned: if this ever reads 0, pose can join the list +} + +TEST_CASE("head reach: the same chain on both sides changes nothing") { + Fixture f = sphere_with(kTail); + const std::optional box = reach(f, kTail, kTail); + REQUIRE(box.has_value()); + CHECK(box->empty()); +} + +TEST_CASE("head reach: a link removed from the MIDDLE counts the links ahead of it too") { + // The common tail stops at the removed link, so the links in front of it + // are part of both heads: a point one of them moves can be moved INTO the + // removed link's ball, and the union of all their balls covers that. + const auto before = std::vector{grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), + grab_at(cf3(0, -0.6f, 0), 0.3f, cf3(0, -0.2f, 0)), + kTail[0]}; + const auto after = std::vector{before[0], before[2]}; + Fixture f = sphere_with(before); + const std::optional box = reach(f, before, after); + REQUIRE(box.has_value()); + CHECK(box->contains(cf3(0, 0.6f, 0))); + CHECK(box->contains(cf3(0, -0.6f, 0))); + check_exact_outside(f, before, after); +} + +TEST_CASE("head reach: a whole-item deformer anywhere in the head is refused") { + // A twist ahead of the grab turns the point before the grab tests its + // ball, so the region is the twist's preimage of the ball, not the ball. + Fixture f = sphere_with({}); + const auto after = std::vector{scene::Deformer::twist(0.5f), + grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0))}; + CHECK_FALSE(reach(f, {}, after).has_value()); + // ...and a grab appended BEHIND an existing twist is the same case. + const auto twisted = std::vector{scene::Deformer::twist(0.5f)}; + const auto appended = std::vector{twisted[0], grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0))}; + CHECK_FALSE(reach(f, twisted, appended).has_value()); + // A whole-item deformer in the common TAIL costs nothing: it sees the + // point the head hands it, which is unchanged outside the ball. + const auto head_on_twist = + std::vector{grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), twisted[0]}; + CHECK(reach(f, twisted, head_on_twist).has_value()); +} + +TEST_CASE("head reach: an easing that is not exactly zero at the rim is refused") { + // ease_out_sine is 1 - (1 - cos(pi/2)) at the rim, which is not zero in + // float: a grab under it translates the whole item by a hair, so its + // "support" is everywhere. + REQUIRE(kernel::cease(kernel::ease_out_sine, 0.0f) != 0.0f); + Fixture f = sphere_with({}); + auto g = grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)); + g.ease = kernel::ease_out_sine; + CHECK_FALSE(reach(f, {}, std::vector{g}).has_value()); + // ...and the refusal is the ease's, not the grab's. + g.ease = kernel::ease_smoothstep; + CHECK(reach(f, {}, std::vector{g}).has_value()); +} + +TEST_CASE("head reach: every easing it accepts is exactly zero at the rim") { + // The list in bounds.cpp refuses the families whose rim runs through a + // transcendental; everything it accepts must at least be zero on the host. + for (int e = 0; e < kernel::ease_count; ++e) { + CAPTURE(e); + Fixture f = sphere_with({}); + auto g = grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)); + g.ease = static_cast(e); + if (reach(f, {}, std::vector{g}).has_value()) + CHECK(kernel::cease(e, 0.0f) == 0.0f); + } +} + +TEST_CASE("head reach: the ball is placed where the item is -- transform, mirror, repeat") { + SUBCASE("a rotated, scaled item under a moved layer") { + Fixture f = sphere_with(kTail, cf3(0.3f, -0.2f, 0.1f)); + scene::Node& n = *f.content().find_mut(f.node); + n.xform.rotation = math::Quat::from_axis_angle(cf3(0, 0, 1), 0.7f); + n.xform.scale = 1.3f; + f.layer().xform.position = cf3(-0.2f, 0.1f, 0); + check_exact_outside(f, kTail, with_head(grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), + kTail)); + } + SUBCASE("a mirrored layer, where the ball has a reflection") { + Fixture f = sphere_with(kTail, cf3(0.9f, 0, 0)); + f.layer().mirror_axes = scene::kMirrorX; + f.layer().mirror_k = 0.05f; + check_exact_outside(f, kTail, with_head(grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), + kTail)); + } + SUBCASE("a finite repeat grid, where the ball recurs in every cell") { + Fixture f = sphere_with({}); + scene::Node& n = *f.content().find_mut(f.node); + n.prim = scene::Prim::sphere(0.3f); + n.repeat.type = kernel::crepeat_grid_finite; + n.repeat.spacing = cf3(1.0f, 1.0f, 1.0f); + n.repeat.counts = cf3(1, 0, 0); + check_exact_outside(f, {}, std::vector{grab_at(cf3(0, 0.3f, 0), 0.2f, cf3(0, 0.15f, 0))}); + } +} + +TEST_CASE("head reach: an infinite repeat grid is refused") { + Fixture f = sphere_with({}); + scene::Node& n = *f.content().find_mut(f.node); + n.repeat.type = kernel::crepeat_grid_infinite; + n.repeat.spacing = cf3(2.0f, 2.0f, 2.0f); + CHECK_FALSE(reach(f, {}, std::vector{grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0))}) + .has_value()); +} + +TEST_CASE("head reach: dilated once per enclosing group and by every fold above") { + Fixture f; + scene::Layer& base = f.doc.add_sdf_layer("base"); + base.sdf->insert(clay_test::item(scene::Prim::sphere(0.5f), cf3(0, -3.0f, 0))); + scene::Layer& l = f.doc.add_sdf_layer("l"); + f.layer_id = l.id; + l.composition.op = scene::Op::Add; + l.composition.blend.profile = scene::BlendProfile::Quadratic; + l.composition.blend.k = 0.05f; + // A node beneath the group so its combine really combines (#515). + l.sdf->insert(clay_test::item(scene::Prim::sphere(0.2f), cf3(0, 2.0f, 0))); + scene::Node g; + g.is_group = true; + g.op = scene::Op::Add; + g.blend.profile = scene::BlendProfile::Quadratic; + g.blend.k = 0.1f; + const scene::NodeId group = l.sdf->insert(g); + f.node = l.sdf->insert(clay_test::item(scene::Prim::sphere(0.6f), cf3(0, 0, 0)), group); + + const auto head = std::vector{grab_at(cf3(0, 0.6f, 0), 0.2f, cf3(0, 0.1f, 0))}; + const std::optional box = reach(f, {}, head); + REQUIRE(box.has_value()); + // The ball (radius 0.2 about y = 0.6, and about y = 0.7 at the displaced + // end), then the group's support (4 * 0.1) and the fold's (4 * 0.05): + // 0.2 + 0.4 + 0.2 on every side of the two balls' hull. + const float grow = 0.2f + 0.4f + 0.2f; + CHECK(box->max.y == doctest::Approx(0.7f + grow).epsilon(0.01)); + CHECK(box->min.y == doctest::Approx(0.6f - grow).epsilon(0.01)); + CHECK(box->min.x == doctest::Approx(-grow).epsilon(0.01)); + check_exact_outside(f, {}, head); +} + +TEST_CASE("head reach: a morph group above is refused, a hidden one too") { + scene::Node g; + g.is_group = true; + g.op = scene::Op::TransitionLinear; + Fixture m; + scene::Layer& l = m.doc.add_sdf_layer("l"); + m.layer_id = l.id; + l.sdf->insert(clay_test::item(scene::Prim::sphere(0.2f), cf3(0, 2.0f, 0))); + const scene::NodeId group = l.sdf->insert(g); + m.node = l.sdf->insert(clay_test::item(scene::Prim::sphere(0.6f), cf3(0, 0, 0)), group); + const auto head = std::vector{grab_at(cf3(0, 0.6f, 0), 0.2f, cf3(0, 0.1f, 0))}; + CHECK_FALSE(reach(m, {}, head).has_value()); + l.sdf->find_mut(group)->op = scene::Op::Add; + CHECK(reach(m, {}, head).has_value()); + l.sdf->find_mut(group)->visible = false; + CHECK_FALSE(reach(m, {}, head).has_value()); +} + +TEST_CASE("head reach: an instanced layer places the ball once per sharer") { + Fixture f = sphere_with(kTail); + scene::Layer* copy = f.doc.instance_layer(f.layer_id, "copy"); + REQUIRE(copy != nullptr); + copy->xform.position = cf3(3.0f, 0, 0); + const auto after = with_head(grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), kTail); + const std::optional box = reach(f, kTail, after); + REQUIRE(box.has_value()); + CHECK(box->contains(cf3(0, 0.6f, 0))); + CHECK(box->contains(cf3(3.0f, 0.6f, 0))); +} + +TEST_CASE("head reach: the command side reads the node's chain before the apply") { + Fixture f = sphere_with(kTail); + const auto after = with_head(grab_at(cf3(0, 0.6f, 0), 0.3f, cf3(0, 0.2f, 0)), kTail); + const scene::Command cmd{scene::SetDeformersCmd{f.layer_id, f.node, after}}; + const std::optional box = scene::command_head_delta_bound(f.doc, cmd); + REQUIRE(box.has_value()); + CHECK(box->contains(cf3(0, 0.6f, 0))); + // Another command kind is never narrowed here. + const scene::Command other{scene::SetColorCmd{f.layer_id, f.node, cf3(1, 0, 0)}}; + CHECK_FALSE(scene::command_head_delta_bound(f.doc, other).has_value()); +} + +namespace { + +// A 2x2x2 cage about the sphere with one corner dragged, so it is not the +// identity and a change to it is visible. +scene::Deformer dragged_lattice(float corner_dx) { + scene::Deformer d = scene::Deformer::lattice(cf3(-0.7f, -0.7f, -0.7f), cf3(0.7f, 0.7f, 0.7f), + 2, 2, 2); + d.cage[0] = cf3(corner_dx, 0, 0); + return d; +} + +std::vector guide_through(float bow) { + std::vector g(3); + g[0].pos = cf3(0, -0.7f, 0); + g[1].pos = cf3(bow, 0, 0); + g[2].pos = cf3(0, 0.7f, 0); + return g; +} + +} // namespace + +TEST_CASE("head reach: a lattice or a bend curve in the common tail is stripped") { + // The tail sees the same point on both sides, whatever it does with it. + const auto head = grab_at(cf3(0, 0.6f, 0), 0.25f, cf3(0, 0.15f, 0)); + const std::vector> tails = { + {dragged_lattice(0.1f)}, + {scene::Deformer::bend_curve(guide_through(0.1f), 0.0f, 1.0f)}, + }; + for (const auto& tail : tails) { + CAPTURE(static_cast(tail[0].type)); + Fixture f = sphere_with(tail); + check_exact_outside(f, tail, with_head(head, tail)); + } +} + +TEST_CASE("head reach: a payload link that differs only in its payload is refused") { + // Same type, same record: only the cage or the guide moved. Comparing the + // record alone would call them one link, strip them as common tail, and + // report the grab's ball for a change that reaches the whole item. + const auto head = grab_at(cf3(0, 0.6f, 0), 0.25f, cf3(0, 0.15f, 0)); + SUBCASE("the cage") { + const std::vector before = {head, dragged_lattice(0.1f)}; + const std::vector after = {head, dragged_lattice(0.2f)}; + Fixture f = sphere_with(before); + CHECK_FALSE(reach(f, before, after).has_value()); + } + SUBCASE("the cage's placement") { + scene::Deformer moved = dragged_lattice(0.1f); + moved.cage_xform.position = cf3(0.05f, 0, 0); + const std::vector before = {head, dragged_lattice(0.1f)}; + const std::vector after = {head, moved}; + Fixture f = sphere_with(before); + CHECK_FALSE(reach(f, before, after).has_value()); + } + SUBCASE("the guide") { + const std::vector before = { + head, scene::Deformer::bend_curve(guide_through(0.1f), 0.0f, 1.0f)}; + const std::vector after = { + head, scene::Deformer::bend_curve(guide_through(0.2f), 0.0f, 1.0f)}; + Fixture f = sphere_with(before); + CHECK_FALSE(reach(f, before, after).has_value()); + } +}