diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 81212a6..7be8379 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -330,6 +330,11 @@ class Evaluator { // free function, same reasoning as tagGenerated()/builtinChildren(). void warn(const std::string& message, const oscad::Position* position); + // Apply `!`: replace the whole tree with the first show_only + // subtree, discarding its ancestors and every sibling. See its + // definition for why this cannot be done by tagging bodies. + void rerootAtShowOnly(std::vector>& tree); + // parent_module(idx) support: walks the live module-call stack // (innermost last) and returns the name `idx` levels up from the // current module, or undef if out of range. Public because diff --git a/pyproject.toml b/pyproject.toml index 8540b9f..bc32c55 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.28.0" +version = "0.28.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index 371c1c3..3279c6c 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -1,3 +1,4 @@ +#include #include "openscad_cpp_evaluator/dispatch.hpp" #include "openscad_cpp_evaluator/evaluator.hpp" @@ -255,6 +256,7 @@ std::vector Evaluator::evaluateImpl(const NodeList& nodes, EvalCont using Clock = std::chrono::steady_clock; const Clock::time_point resolveStart = profiling_ ? Clock::now() : Clock::time_point{}; std::vector> tree = resolveTreeImpl(nodes, ctx); + rerootAtShowOnly(tree); const Clock::time_point resolveEnd = profiling_ ? Clock::now() : Clock::time_point{}; std::vector result = generateTree(tree); const Clock::time_point generateEnd = profiling_ ? Clock::now() : Clock::time_point{}; @@ -289,6 +291,44 @@ std::vector Evaluator::evaluateImpl(const NodeList& nodes, EvalCont return result; } + +// `!` does not merely tag what it marks -- it makes that subtree the whole +// model. Everything else goes: siblings, and every operation wrapped around +// it. `translate([50,0,0]) !cube(5);` puts a cube at the origin, and +// `linear_extrude(height=10) !circle(10);` leaves a 2D circle rather than a +// cylinder, because the extrude is an ancestor and ancestors are discarded. +// +// Tagging the bodies and filtering them at the end -- which is what this +// did before -- cannot express that: by the time a body carries the tag, +// every ancestor has already been applied to it. +void Evaluator::rerootAtShowOnly(std::vector>& tree) { + std::vector*> found; + // Pre-order, so `found.front()` is the first one in the source. + std::function>&)> walk = + [&](std::vector>& nodes) { + for (std::unique_ptr& n : nodes) { + if (!n) continue; + if (n->kind == "show_only") found.push_back(&n); + walk(n->children); + } + }; + walk(tree); + if (found.empty()) return; + + if (found.size() > 1) { + // Same rule as the reference: warn, and take the first. + const CSGNode& second = **found[1]; + warn("More than one Root Modifier (!)", + second.node ? &second.node->position() : nullptr); + } + + // Moved out before the rest of the tree is dropped -- its owner is + // somewhere inside what is about to be destroyed. + std::unique_ptr chosen = std::move(*found.front()); + tree.clear(); + tree.push_back(std::move(chosen)); +} + std::vector Evaluator::evaluate(const std::vector>& nodes, EvalContext& ctx, const std::unordered_map& viewportParams) { return evaluateImpl(nodes, ctx, viewportParams); diff --git a/tests/test_booleans.cpp b/tests/test_booleans.cpp index 2ff002a..11d21ca 100644 --- a/tests/test_booleans.cpp +++ b/tests/test_booleans.cpp @@ -177,15 +177,50 @@ TEST(Modifiers, BackgroundIsExcludedFromBooleanMergeButStillReturned) { EXPECT_TRUE(hasBackground); } -TEST(Modifiers, TopLevelShowOnlyFiltersOutEverythingElse) { +TEST(Modifiers, TopLevelShowOnlyLeavesNothingButItsOwnSubtree) { Evaluated e = evaluateSrc("!cube(1); cube(2); #cube(3);"); - // evaluate()'s top-level filter: any show_only body present -> keep - // only show_only + highlight bodies. cube(2) (role=normal) is dropped; - // cube(1) (show_only) and cube(3) (highlight) survive. - ASSERT_EQ(e.bodies.size(), 2u); - for (const ColoredBody& b : e.bodies) { - EXPECT_TRUE(b.role == BodyRole::ShowOnly || b.role == BodyRole::Highlight); - } + // `!` makes its subtree the whole model, so both siblings go -- the + // highlighted one included. This test previously expected cube(3) to + // survive on the strength of its role; the reference implementation + // renders only cube(1) here (checked against 2021.01). + ASSERT_EQ(e.bodies.size(), 1u); + EXPECT_EQ(e.bodies[0].role, BodyRole::ShowOnly); + EXPECT_NEAR(e.bodies[0].body->Volume(), 1.0, 1e-9); +} + +// The reason a body-role filter could not do this: by the time a body is +// tagged, every operation wrapped around it has already been applied. +TEST(Modifiers, ShowOnlyDiscardsTheOperationsWrappedAroundIt) { + Evaluated moved = evaluateSrc("translate([50,0,0]) !cube(5);"); + ASSERT_EQ(moved.bodies.size(), 1u); + const manifold::Box box = moved.bodies[0].body->BoundingBox(); + EXPECT_NEAR(box.min.x, 0.0, 1e-9) << "the translate is an ancestor, so it does not apply"; + EXPECT_NEAR(box.max.x, 5.0, 1e-9); +} + +// The case that prompted this: an extrude is an ancestor like any other, +// so what is left is the 2D circle rather than the cylinder. +TEST(Modifiers, ShowOnlyInsideAnExtrudeLeavesTheProfile) { + Evaluated e = evaluateSrc("linear_extrude(height=10) !circle(10);"); + ASSERT_EQ(e.bodies.size(), 1u); + // What survives is 2D: a section, not a solid. Extruding it is what + // the discarded ancestor would have done. + ASSERT_TRUE(e.bodies[0].section.has_value()) << "extruded into a solid"; + const manifold::Rect bounds = e.bodies[0].section->Bounds(); + EXPECT_NEAR(bounds.max.x, 10.0, 0.1); + EXPECT_NEAR(bounds.min.x, -10.0, 0.1); +} + +TEST(Modifiers, MoreThanOneShowOnlyTakesTheFirstAndWarns) { + std::vector echoed; + Evaluated e = evaluateSrc("translate([50,0,0]) !cube(5);\n!sphere(3);", + [&echoed](const std::string& m) { echoed.push_back(m); }); + ASSERT_EQ(e.bodies.size(), 1u); + const manifold::Box box = e.bodies[0].body->BoundingBox(); + EXPECT_NEAR(box.max.x, 5.0, 1e-9) << "the cube, not the sphere"; + EXPECT_TRUE(std::any_of(echoed.begin(), echoed.end(), [](const std::string& m) { + return m.find("More than one Root Modifier") != std::string::npos; + })) << "no warning in: " << (echoed.empty() ? "(nothing)" : echoed.front()); } TEST(Modifiers, NoShowOnlyMeansAllRolesPassThrough) {