Keep a grab the deformer cull cannot judge in the brick's tape (#649)

src/scene/tape_build.cpp

@@ -192,15 +192,27 @@ struct Compiler {
     // the next one may be tested against the same region. A warp that is KEPT
     // may move the point, so the region is dilated by the most that warp can
     // move it before the next test — conservative, and it is what keeps this
-    // exact rather than nearly.
+    // exact rather than nearly. Only a grab's move is bounded here, so the
+    // first link of any other kind stops the test (below).
     //
     // GRAB ONLY, for now. `pose` and `magnify` share `cregion_weight` and could
     // join this on the same terms; `pose_line`, the noise and the lattices have
     // no finite support and never can. One verb at a time, each with the test
     // that shows the field did not move.
+    //
+    // NOT ACROSS A REPEAT (issue #649). The interpreter folds the local point
+    // into its repetition cell BEFORE the chain runs (kernel/tape.h,
+    // ctape_repeat_point), so the chain sees the folded point, and the region
+    // below is the UNFOLDED one: a copy away from the source cell samples its
+    // grab at points this test never looked at. Measured on random documents
+    // with a radial or grid repeat carrying a grab: in-band brick samples off
+    // the raw field by up to 0.26, the copies' grab simply missing from every
+    // brick that did not overlap the source cell. A repeated item keeps its
+    // whole chain; a region test in the folded frame would have to cover every
+    // cell the brick reaches, which is the item's whole cell for most of them.
     std::vector<Deformer> cull_deformers(const std::vector<Deformer>& deformers,
-                                         const cfloat4x4& inv) const {
-        if (!cull || deformers.empty()) return deformers;
+                                         const cfloat4x4& inv, const Repeat& repeat) const {
+        if (!cull || deformers.empty() || repeat.active()) return deformers;
         bool any_finite = false;
         for (const Deformer& d : deformers)
             if (d.type == kernel::cdeform_grab) any_finite = true;
@@ -214,10 +226,20 @@ struct Compiler {
 
         std::vector<Deformer> kept;
         kept.reserve(deformers.size());
-        for (const Deformer& d : deformers) {
+        for (std::size_t i = 0; i < deformers.size(); ++i) {
+            const Deformer& d = deformers[i];
+            // Any other link stays, and ENDS the test (issue #649): it may move
+            // the point by an amount this loop has no bound for, and the
+            // induction above holds only while the region is dilated by every
+            // move made so far. A magnify ahead of a grab pulled samples 0.1
+            // into a grab whose ball the region missed, and the culled tape
+            // measured 0.038 off the whole one. So the rest of the chain is
+            // kept as it is. The grabs a Move records sit at the HEAD of a
+            // chain, ahead of anything else, and are still tested.
             if (d.type != kernel::cdeform_grab) {
-                kept.push_back(d);
-                continue;  // no finite support to test; it stays and may move the point
+                kept.insert(kept.end(), deformers.begin() + static_cast<std::ptrdiff_t>(i),
+                            deformers.end());
+                break;
             }
             const kernel::cfloat3 centre = kernel::cf3(d.k, d.a, d.b);
             const float radius = d.c;
@@ -262,7 +284,7 @@ struct Compiler {
         tape.params.push_back(repeat.counts.x);
         tape.params.push_back(repeat.counts.y);
         tape.params.push_back(repeat.counts.z);
-        const std::vector<Deformer> live = cull_deformers(deformers, inv);
+        const std::vector<Deformer> live = cull_deformers(deformers, inv, repeat);
         live_chain_ = live;
         live_chain_src_ = &deformers;
         tape.params.push_back(static_cast<float>(live.size()));

tests/unit/test_deformer_cull.cpp

@@ -250,3 +250,99 @@ TEST_CASE("deformer cull: a culled march agrees with a dense sign-change truth")
     REQUIRE(truth < kSide * kSide);
     CHECK(marched == truth);
 }
+
+// ACROSS A REPEAT (issue #649). The interpreter folds the local point into its
+// repetition cell BEFORE the chain runs, so every copy samples the one chain at
+// folded points -- and the region test above was made in the UNFOLDED frame. A
+// brick over a copy other than the source cell found the grab out of reach,
+// dropped it, and filled that copy undeformed: random documents measured
+// in-band brick samples off the raw field by up to 0.26.
+namespace {
+
+// A sphere under `repeat` whose one grab sits over the SOURCE cell only -- the
+// canonical +X sector of a radial array, the centre cell of a grid -- so a
+// region over any other copy never overlaps the grab's ball.
+scene::Document repeated_ball(const scene::Repeat& repeat, const cfloat3* grab_at) {
+    scene::Document doc;
+    scene::Layer& l = doc.add_sdf_layer("body");
+    scene::Node n;
+    n.id = l.sdf->reserve_id();
+    n.prim = scene::Prim::sphere(0.4f);
+    n.repeat = repeat;
+    if (grab_at)
+        n.deformers.push_back(scene::Deformer::grab(*grab_at, 0.25f, cf3(0.0f, 0.12f, 0.0f)));
+    l.sdf->insert(n);
+    return doc;
+}
+
+// Largest |whole - culled| over a region, and how many of its samples the
+// grab actually moved -- so a case cannot pass because the grab reaches
+// nothing there.
+struct CullAgreement {
+    float worst = 0.0f;
+    int moved = 0;
+};
+
+CullAgreement agreement(const scene::Repeat& repeat, cfloat3 grab_at, const math::Aabb& region) {
+    const scene::Document doc = repeated_ball(repeat, &grab_at);
+    const scene::Tape whole = scene::compile_document(doc);
+    scene::CullRegion cull{region};
+    const scene::Tape culled = scene::compile_document(doc, &cull);
+    const scene::Tape undeformed = scene::compile_document(repeated_ball(repeat, nullptr));
+    CullAgreement a;
+    for (cfloat3 p : points_in(region, 9)) {
+        const float w = whole.eval(p).d;
+        a.worst = std::fmax(a.worst, std::fabs(w - culled.eval(p).d));
+        if (std::fabs(w - undeformed.eval(p).d) > 1e-3f) ++a.moved;
+    }
+    return a;
+}
+
+}  // namespace
+
+TEST_CASE("deformer cull: a radial copy keeps the grab its source cell carries") {
+    // Four sectors about Y; the grab on the ball's +X side, which is the
+    // canonical sector. A region on the -X side never contains the grab's
+    // ball, yet every sample there is rotated into the +X sector before the
+    // chain runs, and is pulled by it.
+    const math::Aabb region{cf3(-0.6f, -0.2f, -0.2f), cf3(-0.2f, 0.2f, 0.2f)};
+    const CullAgreement a =
+        agreement(scene::Repeat::radial(4, 0.0f), cf3(0.4f, 0.0f, 0.0f), region);
+    REQUIRE(a.moved > 0);
+    CHECK(a.worst == 0.0f);
+}
+
+TEST_CASE("deformer cull: a grid copy keeps the grab its source cell carries") {
+    // Five copies along X, one unit apart; the grab over the centre copy's top
+    // and a region over the copy at x = +2.
+    const math::Aabb region{cf3(1.6f, 0.1f, -0.3f), cf3(2.4f, 0.7f, 0.3f)};
+    const CullAgreement a = agreement(scene::Repeat::grid_finite(1.0f, cf3(2.0f, 0.0f, 0.0f)),
+                                      cf3(0.0f, 0.4f, 0.0f), region);
+    REQUIRE(a.moved > 0);
+    CHECK(a.worst == 0.0f);
+}
+
+TEST_CASE("deformer cull: a grab behind another warp is not tested against the bare region") {
+    // The induction dilates the region by every move a kept link can make, and
+    // only a grab's move is bounded. A magnify ahead of a grab pulls these
+    // samples 0.1 and more toward its centre, into a grab whose ball the region
+    // itself never touches -- dropping the grab there was 0.038 off (#649).
+    scene::Document doc;
+    scene::Layer& l = doc.add_sdf_layer("body");
+    scene::Node n;
+    n.id = l.sdf->reserve_id();
+    n.prim = scene::Prim::sphere(1.0f);
+    n.deformers.push_back(scene::Deformer::magnify(cf3(0.0f, 0.0f, 1.0f), 0.5f, 0.9f));
+    n.deformers.push_back(scene::Deformer::grab(cf3(0.0f, 0.0f, 1.0f), 0.15f, cf3(0, 0, 0.1f)));
+    l.sdf->insert(n);
+
+    const math::Aabb region{cf3(0.2f, -0.05f, 0.85f), cf3(0.35f, 0.05f, 1.1f)};
+    const math::Aabb grab_ball{cf3(-0.15f, -0.15f, 0.85f), cf3(0.15f, 0.15f, 1.15f)};
+    REQUIRE_FALSE(grab_ball.intersects(region));
+
+    const scene::Tape whole = scene::compile_document(doc);
+    scene::CullRegion cull{region};
+    const scene::Tape culled = scene::compile_document(doc, &cull);
+    CHECK(deformer_count(culled) == 2);
+    for (cfloat3 p : points_in(region, 7)) CHECK(whole.eval(p).d == culled.eval(p).d);
+}