Make ! the root of the tree, not a tag on the bodies

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<std::unique_ptr<CSGNode>>& 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

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"

src/csg_resolve.cpp

@@ -1,3 +1,4 @@
+#include <functional>
 #include "openscad_cpp_evaluator/dispatch.hpp"
 #include "openscad_cpp_evaluator/evaluator.hpp"
 
@@ -255,6 +256,7 @@ std::vector<ColoredBody> 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<std::unique_ptr<CSGNode>> tree = resolveTreeImpl(nodes, ctx);
+    rerootAtShowOnly(tree);
     const Clock::time_point resolveEnd = profiling_ ? Clock::now() : Clock::time_point{};
     std::vector<ColoredBody> result = generateTree(tree);
     const Clock::time_point generateEnd = profiling_ ? Clock::now() : Clock::time_point{};
@@ -289,6 +291,44 @@ std::vector<ColoredBody> 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<std::unique_ptr<CSGNode>>& tree) {
+    std::vector<std::unique_ptr<CSGNode>*> found;
+    // Pre-order, so `found.front()` is the first one in the source.
+    std::function<void(std::vector<std::unique_ptr<CSGNode>>&)> walk =
+        [&](std::vector<std::unique_ptr<CSGNode>>& nodes) {
+            for (std::unique_ptr<CSGNode>& 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<CSGNode> chosen = std::move(*found.front());
+    tree.clear();
+    tree.push_back(std::move(chosen));
+}
+
 std::vector<ColoredBody> Evaluator::evaluate(const std::vector<std::unique_ptr<oscad::ASTNode>>& nodes, EvalContext& ctx,
                                               const std::unordered_map<std::string, Value>& viewportParams) {
     return evaluateImpl(nodes, ctx, viewportParams);

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<std::string> 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) {