diff --git a/src/scene/tape_build.cpp b/src/scene/tape_build.cpp index ad0931ff..82b1294f 100644 --- a/src/scene/tape_build.cpp +++ b/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 cull_deformers(const std::vector& 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 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(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 live = cull_deformers(deformers, inv); + const std::vector live = cull_deformers(deformers, inv, repeat); live_chain_ = live; live_chain_src_ = &deformers; tape.params.push_back(static_cast(live.size())); diff --git a/tests/unit/test_deformer_cull.cpp b/tests/unit/test_deformer_cull.cpp index 347788a6..f54c81fe 100644 --- a/tests/unit/test_deformer_cull.cpp +++ b/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); +}