diff --git a/include/openscad_cpp_evaluator/bytecode.hpp b/include/openscad_cpp_evaluator/bytecode.hpp index c8205ba..0311736 100644 --- a/include/openscad_cpp_evaluator/bytecode.hpp +++ b/include/openscad_cpp_evaluator/bytecode.hpp @@ -261,7 +261,9 @@ enum class Op { CallModule, // -- Builtin-wrap compilation (closes the "NativeStatement gap" for ---- - // -- translate/rotate/scale/mirror/multmatrix/resize/color/#/%/!) ------ + // -- translate/rotate/scale/mirror/multmatrix/resize/color/#/%/!, ------ + // -- hull/minkowski/render/linear_extrude/rotate_extrude/projection/ --- + // -- offset/roof) -------------------------------------------------------- // A builtin-with-children statement that isn't a "call" the way // Op::CallModule's target is (no callStack_/profiling participation, // no named scope with upvalue semantics -- see CompiledChunk:: @@ -284,31 +286,41 @@ enum class Op { // (mirrors Evaluator::buildTreeNode's own ordering exactly -- doing // this AFTER would silently drop uncacheable/ManifoldCache taint // tracking for a rands() call embedded in the wrapper's own - // arguments), computes this site's own params (and, for Transform/ - // Color kinds, a possibly-$-scoped child EvalContext, pushed onto - // f.ctxChain unconditionally -- Modifier kind needs neither params - // nor a ctx push), pushes a fresh ev.treeStack_ frame, and stashes - // {params, randsBefore, siteIdx} onto VmFrame::builtinWrapStack (a - // real per-frame LIFO stack, not a single slot: Push/PopBuiltinWrap - // pairs can nest or sequence within one frame's own instruction - // stream, e.g. `translate(a) translate(b) recur();`). The compiler - // always emits a plain Op::CheckDebugStatement immediately before - // this (see emitBuiltinWrap, bytecode_compiler.cpp), mirroring - // ModularEcho/ModularAssert's own pattern -- NOT skipped the way - // Op::CallModule's own call site skips one: this is a genuine - // statement doing real work here, unlike a module call (which - // transfers control to a declaration whose OWN body statements each - // get their own check instead). + // arguments), computes this site's own params for every kind except + // Modifier (empty, no ctx push) and Roof (deferred to Pop -- see + // BuiltinWrapSite's own doc comment) via that kind's own native + // compute function (computeTransformParams/computeColorParams/etc., + // builtins.hpp) -- always a possibly-$-scoped child EvalContext, + // pushed onto f.ctxChain unconditionally for every non-Modifier kind + // (Roof pushes it too, params or not) -- pushes a fresh ev.treeStack_ + // frame, and stashes {params, randsBefore, siteIdx, (Roof only) + // deferredArgs} onto VmFrame::builtinWrapStack (a real per-frame LIFO + // stack, not a single slot: Push/PopBuiltinWrap pairs can nest or + // sequence within one frame's own instruction stream, e.g. + // `translate(a) translate(b) recur();`). The compiler always emits a + // plain Op::CheckDebugStatement immediately before this (see + // emitBuiltinWrap, bytecode_compiler.cpp), mirroring ModularEcho/ + // ModularAssert's own pattern -- NOT skipped the way Op::CallModule's + // own call site skips one: this is a genuine statement doing real + // work here, unlike a module call (which transfers control to a + // declaration whose OWN body statements each get their own check + // instead). PushBuiltinWrap, // Closes the matching Op::PushBuiltinWrap bracket: pops // VmFrame::builtinWrapStack's own top entry FIRST (before anything // that can itself throw, e.g. Evaluator::setTreeDepthOrThrow below -- // so the exception-teardown path's own pending count is already - // correct if THIS throws), pops the ctx push Push made (Transform/ - // Color kinds only), pops ev.treeStack_ to retrieve the children this - // bracket's own body produced, and builds the tagged CSGNode exactly - // like Evaluator::buildTreeNode's own post-resolveBody() half does + // correct if THIS throws); for Roof only, computes `pending.params` + // HERE via computeRoofParams(ev, pending.deferredArgs, ctx) -- the one + // kind whose params computation has an observable side effect + // (ev.warn()) that must stay ordered AFTER children, unlike every + // other kind's params (computed at Push, before children, since + // nothing else in this group has an order-sensitive side effect); pops + // the ctx Push made (every kind except Modifier); pops ev.treeStack_ + // to retrieve the children this bracket's own body produced, and + // builds the tagged CSGNode exactly like Evaluator::buildTreeNode's + // own post-resolveBody() half does // (kind/node/isBuiltin=true/children/params/uncacheable/ // setTreeDepthOrThrow), pushing it onto the new top of treeStack_. a // = index into CompiledChunk::builtinWrapSites (same site Push used). @@ -465,8 +477,9 @@ enum class Op { // A single "native passthrough" statement -- intersection_for, every // OTHER builtin module call not covered by Op::PushBuiltinWrap/ // Op::PushCsgWrap (see those ops' own doc comments for exactly which - // builtins ARE covered -- cube/sphere/hull/linear_extrude/etc., the - // ones that never wrap a recursive call in idiomatic OpenSCAD), the `*` + // builtins ARE covered -- cube/sphere/import/text/surface/etc., the + // true LEAVES that never take a `children` block at all, so wrapping a + // recursive call inside one isn't even syntactically possible), the `*` // modifier's own no-op case, or a user-module call that didn't // resolve at compile time (shadowed, forward-declared, or otherwise // not statically known) -- anything compileStatementList doesn't give @@ -488,7 +501,18 @@ enum class Op { // fell here too, and were the last remaining REAL native-reentry risk // (bespoke group_sizes bookkeeping meant they couldn't just reuse // PushBuiltinWrap's own bracket) -- they now have Op::PushCsgWrap, - // above.) + // above. hull()/minkowski()/render()/linear_extrude()/rotate_extrude()/ + // projection()/offset()/roof() fell here too, closing out every + // remaining non-leaf builtin whose OWN children can wrap a recursive + // call in an idiomatic script (a rounded-hull chain, a recursive + // extrude, etc.) -- they now share Op::PushBuiltinWrap's own bracket + // via 6 new Kind values (Passthrough/LinearExtrude/RotateExtrude/ + // Projection/Offset/Roof), reusing the exact same mechanism rather + // than inventing another. Only intersection_for remains -- its + // group_sizes are keyed by RUNTIME loop iteration, not a static + // source-statement list the way union/difference/intersection's are, + // so it can't reuse either existing bracket shape without new loop- + // compilation machinery; still deliberately native as of this writing.) // a = index into CompiledChunk::nativeStatements. Runtime just does // what Evaluator::evalChildren's own per-statement loop already does // for one node: derive childCtx via ctx.withScope(...), checkDebug, @@ -813,8 +837,8 @@ struct CompiledChunk { // replay the native param-resolution step and rebuild the tagged // CSGNode at Pop time. // - // `node`'s concrete type depends on `kind`: Transform/Color sites are - // genuine `ModularCall`s (down-cast to read `.name`/`.arguments`/ + // `node`'s concrete type depends on `kind`: every kind except Modifier + // is a genuine `ModularCall` (down-cast to read `.name`/`.arguments`/ // `.children`); Modifier sites are the wrapper node itself // (`ModularModifierHighlight`/`Background`/`ShowOnly` -- `!`/`#`/`%`; // `*`/Disable never reaches here, its child never evaluates at all, @@ -826,10 +850,30 @@ struct CompiledChunk { // union of concrete pointers) because that's all Op::PopBuiltinWrap // itself ever needs `node` for: the resulting CSGNode's own // `node`/error-position field. + // + // Passthrough/LinearExtrude/RotateExtrude/Projection/Offset/Roof + // (added for hull()/minkowski()/render()/linear_extrude()/ + // rotate_extrude()/projection()/offset()/roof()) share Transform/ + // Color's exact push-time-computed-params shape, just via a different + // native compute function each (Op::PushBuiltinWrap's own runtime + // handler switches on `kind` to pick it) -- see that op's own doc + // comment for the one exception (Roof, whose params computation has + // to run at POP time instead, after children, to preserve an + // observable warn() ordering). struct BuiltinWrapSite { - enum class Kind { Transform, Color, Modifier }; + enum class Kind { + Transform, + Color, + Modifier, + Passthrough, // hull()/minkowski()/render() -- empty params, no compute function needed + LinearExtrude, + RotateExtrude, + Projection, + Offset, + Roof, // computed at POP time, not PUSH -- see this struct's own doc comment + }; Kind kind; - std::string tagName; // "translate"/"rotate"/.../"color"/"highlight"/"background"/"show_only" + std::string tagName; // "translate"/"rotate"/.../"color"/"highlight"/"background"/"show_only"/"hull"/... const oscad::ASTNode* node = nullptr; }; diff --git a/include/openscad_cpp_evaluator/bytecode_vm.hpp b/include/openscad_cpp_evaluator/bytecode_vm.hpp index 1ac73ae..256e632 100644 --- a/include/openscad_cpp_evaluator/bytecode_vm.hpp +++ b/include/openscad_cpp_evaluator/bytecode_vm.hpp @@ -2,6 +2,7 @@ #include "openscad_cpp_evaluator/bound_args.hpp" #include "openscad_cpp_evaluator/bytecode.hpp" +#include "openscad_cpp_evaluator/call_args.hpp" #include "openscad_cpp_evaluator/csg_node.hpp" #include "openscad_cpp_evaluator/eval_context.hpp" #include "openscad_cpp_evaluator/value.hpp" @@ -28,6 +29,15 @@ struct PendingBuiltinWrap { CSGParams params; std::uint64_t randsBefore = 0; int siteIdx = -1; + // Only populated for BuiltinWrapSite::Kind::Roof -- the already- + // resolved call arguments, retained across the whole bracket so + // Op::PopBuiltinWrap's handler can compute roof()'s own params AFTER + // children finish (computeRoofParams needs them; re-running + // resolveCallArgs at Pop time instead would re-evaluate every argument + // expression a second time -- double rands()/side effects). Every + // other kind computes its params at Push time and leaves this default- + // empty. + CallArgs deferredArgs; }; // One still-open Op::PushCsgWrap bracket's own state -- see that op's own diff --git a/pyproject.toml b/pyproject.toml index 3d5066b..3d5abb8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.13.5" +version = "0.13.6" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/builtins/builtins.hpp b/src/builtins/builtins.hpp index 1bfa32c..9241345 100644 --- a/src/builtins/builtins.hpp +++ b/src/builtins/builtins.hpp @@ -1,5 +1,6 @@ #pragma once +#include "openscad_cpp_evaluator/call_args.hpp" #include "openscad_cpp_evaluator/dispatch.hpp" #include @@ -153,21 +154,30 @@ std::vector generateMinkowski(Evaluator& ev, const CSGParams& param const oscad::ASTNode& node); // linear_extrude()/rotate_extrude()/projection() -- extrude.cpp. +// computeXParams: the pure "resolve args, build params" half, split out +// like computeTransformParams/computeColorParams so Op::PushBuiltinWrap's +// own runtime handler can call it directly (bytecode_vm.cpp) instead of +// through the whole resolve function, whose OTHER half (evalChildren) is +// exactly the native reentry that split exists to avoid. +BuiltinWrapParams computeLinearExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); CSGParams resolveLinearExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); std::vector generateLinearExtrude(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node); +BuiltinWrapParams computeRotateExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); CSGParams resolveRotateExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); std::vector generateRotateExtrude(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node); +BuiltinWrapParams computeProjectionParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); CSGParams resolveProjection(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); std::vector generateProjection(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node); +BuiltinWrapParams computeOffsetParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); CSGParams resolveOffset(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); std::vector generateOffset(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node); @@ -176,6 +186,13 @@ std::vector generateOffset(Evaluator& ev, const CSGParams& params, // contour polygons) + Tier 3 (SDF/level-set fallback) only, per the plan -- // Tier 2 (general multi-contour/hole straight skeleton) is a named, // documented follow-up, not silently dropped (see CLAUDE.md). +// computeRoofParams: UNLIKE computeLinearExtrudeParams and friends above, +// this is the params-computation half taken from AFTER evalChildren (it +// calls ev.warn(), an observable side effect that must stay ordered after +// any child's own echo/warn output) -- takes already-resolved CallArgs, +// not a raw node+ctx, since Op::PushBuiltinWrap's own runtime handler +// calls this at POP time. See its own doc comment (roof.cpp). +CSGParams computeRoofParams(Evaluator& ev, const CallArgs& args, EvalContext& effCtx); CSGParams resolveRoof(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx); std::vector generateRoof(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node); diff --git a/src/builtins/extrude.cpp b/src/builtins/extrude.cpp index c5bfd5e..2b532d8 100644 --- a/src/builtins/extrude.cpp +++ b/src/builtins/extrude.cpp @@ -13,10 +13,21 @@ namespace oscadeval { // mirrors _resolve_linear_extrude/_generate_linear_extrude. `scale` is // either a single number (applied to both X/Y) or a [x,y] pair for the top // face; the bottom face is always full-size. - -CSGParams resolveLinearExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { +// +// Split into computeLinearExtrudeParams (pure, no children-evaluation side +// effect) + resolveLinearExtrude (adds the evalChildren call) so Op:: +// PushBuiltinWrap's own runtime handler (bytecode_vm.cpp) can call the pure +// half directly instead of going through the whole resolve function -- +// exactly the same split computeTransformParams/computeColorParams already +// use, extended to this builtin. Params computation here has no observable +// side effect of its own (no warn()/echo()/rands() call), so moving it +// before evalChildren (unlike this function's own former single-body +// shape, which computed params AFTER children like every other builtin +// still not covered by PushBuiltinWrap) is behaviorally unobservable -- +// see computeRoofParams's own doc comment (roof.cpp) for the one builtin +// in this group where that ISN'T true and a different split was needed. +BuiltinWrapParams computeLinearExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); - ev.evalChildren(node.children, effCtx); const double height = toDoubleLenient(getArg(args, 0, "height", Value{1.0})); const bool center = truthy(getArg(args, std::nullopt, "center", Value{false})); @@ -40,7 +51,13 @@ CSGParams resolveLinearExtrude(Evaluator& ev, const oscad::ModularCall& node, Ev params["scale_x"] = Value{scaleX}; params["scale_y"] = Value{scaleY}; params["color"] = colorToValue(effCtx.color); - return params; + return BuiltinWrapParams{std::move(params), std::move(effCtx)}; +} + +CSGParams resolveLinearExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { + BuiltinWrapParams result = computeLinearExtrudeParams(ev, node, ctx); + ev.evalChildren(node.children, result.ctx); + return std::move(result.params); } std::vector generateLinearExtrude(Evaluator& ev, const CSGParams& params, @@ -65,9 +82,10 @@ std::vector generateLinearExtrude(Evaluator& ev, const CSGParams& p // so resolve only caches $fn/$fa/$fs for generate to call fnSegments() // with once the real max-x bound exists. -CSGParams resolveRotateExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { +// Split the same way as computeLinearExtrudeParams, above -- see its own +// doc comment. +BuiltinWrapParams computeRotateExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); - ev.evalChildren(node.children, effCtx); const double angle = toDoubleLenient(getArg(args, 0, "angle", Value{360.0})); @@ -84,7 +102,13 @@ CSGParams resolveRotateExtrude(Evaluator& ev, const oscad::ModularCall& node, Ev params["fa"] = Value{dynOr("$fa", 12.0)}; params["fs"] = Value{dynOr("$fs", 2.0)}; params["color"] = colorToValue(effCtx.color); - return params; + return BuiltinWrapParams{std::move(params), std::move(effCtx)}; +} + +CSGParams resolveRotateExtrude(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { + BuiltinWrapParams result = computeRotateExtrudeParams(ev, node, ctx); + ev.evalChildren(node.children, result.ctx); + return std::move(result.params); } std::vector generateRotateExtrude(Evaluator& ev, const CSGParams& params, @@ -108,12 +132,19 @@ std::vector generateRotateExtrude(Evaluator& ev, const CSGParams& p // FillRule::Positive to clean up self-intersections Manifold's own // Project() can produce. -CSGParams resolveProjection(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { +// Split the same way as computeLinearExtrudeParams, above -- see its own +// doc comment. +BuiltinWrapParams computeProjectionParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); - ev.evalChildren(node.children, effCtx); CSGParams params; params["cut"] = Value{truthy(getArg(args, std::nullopt, "cut", Value{false}))}; - return params; + return BuiltinWrapParams{std::move(params), std::move(effCtx)}; +} + +CSGParams resolveProjection(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { + BuiltinWrapParams result = computeProjectionParams(ev, node, ctx); + ev.evalChildren(node.children, result.ctx); + return std::move(result.params); } std::vector generateProjection(Evaluator&, const CSGParams& params, @@ -145,9 +176,10 @@ std::vector generateProjection(Evaluator&, const CSGParams& params, // (JoinType::Square, or Miter if chamfer=true) are mutually exclusive; // neither given passes the first child through unchanged. -CSGParams resolveOffset(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { +// Split the same way as computeLinearExtrudeParams, above -- see its own +// doc comment. +BuiltinWrapParams computeOffsetParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); - ev.evalChildren(node.children, effCtx); const Value rArg = getArg(args, std::nullopt, "r", Value{}); const Value deltaArg = getArg(args, std::nullopt, "delta", Value{}); @@ -161,7 +193,13 @@ CSGParams resolveOffset(Evaluator& ev, const oscad::ModularCall& node, EvalConte params["segs"] = Value{static_cast(fnSegmentsFromCtx(effCtx, std::fabs(toDoubleLenient(rArg))))}; } params["color"] = colorToValue(effCtx.color); - return params; + return BuiltinWrapParams{std::move(params), std::move(effCtx)}; +} + +CSGParams resolveOffset(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { + BuiltinWrapParams result = computeOffsetParams(ev, node, ctx); + ev.evalChildren(node.children, result.ctx); + return std::move(result.params); } std::vector generateOffset(Evaluator&, const CSGParams& params, const std::vector>& children, diff --git a/src/builtins/roof.cpp b/src/builtins/roof.cpp index d04fa50..f729477 100644 --- a/src/builtins/roof.cpp +++ b/src/builtins/roof.cpp @@ -417,10 +417,21 @@ manifold::Manifold voronoiRoof(const manifold::CrossSection& cs, double fa, doub // construction here -- this port has no CGAL-based "straight" method (see // the file header comment and CLAUDE.md). Mirrors _resolve_roof. -CSGParams resolveRoof(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { - auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); - ev.evalChildren(node.children, effCtx); - +// Split like computeLinearExtrudeParams (extrude.cpp) -- but UNLIKE that +// group, this one genuinely can't move before evalChildren: the "Unknown +// roof method" warning below is an observable side effect (an echo/warn +// message), and native resolveRoof always evaluates children FIRST, so +// this warning fires AFTER any echo()/warn() a child produces. Op:: +// PushBuiltinWrap's own runtime handler (bytecode_vm.cpp) therefore calls +// this at POP time (after children finish -- VmFrame::builtinWrapStack +// retains `args` for exactly this) rather than at PUSH time the way +// computeLinearExtrudeParams/computeTransformParams/computeColorParams are +// called -- see Op::PushBuiltinWrap's own Roof-kind doc comment +// (bytecode.hpp) for the full contract. Takes the already-resolved +// `CallArgs`/`EvalContext` directly (not the raw node+ctx) since by POP +// time the argument expressions have already run once and must not +// re-run (double rands()/side effects). +CSGParams computeRoofParams(Evaluator& ev, const CallArgs& args, EvalContext& effCtx) { Value methodArg = getArg(args, std::nullopt, "method", Value{std::string("voronoi")}); std::string method = std::holds_alternative(methodArg) ? std::get(methodArg) : "voronoi"; if (method != "voronoi" && method != "straight") { @@ -445,6 +456,12 @@ CSGParams resolveRoof(Evaluator& ev, const oscad::ModularCall& node, EvalContext return params; } +CSGParams resolveRoof(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) { + auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx); + ev.evalChildren(node.children, effCtx); + return computeRoofParams(ev, args, effCtx); +} + std::vector generateRoof(Evaluator& ev, const CSGParams& params, const std::vector>& children, const oscad::ASTNode& node) { const std::optional cs = toCrossSection(flattenCsgTree(children)); diff --git a/src/bytecode_compiler.cpp b/src/bytecode_compiler.cpp index 9dc4277..e102545 100644 --- a/src/bytecode_compiler.cpp +++ b/src/bytecode_compiler.cpp @@ -1428,17 +1428,17 @@ class Compiler { {Op::CallModule, static_cast(chunk_.moduleCallSites.size()) - 1, 0, &call.position()}); return; } - // A user module never shadows a builtin transform/color - // name in a way that reaches here -- the `resolved` check - // above already ruled that out (isBuiltin per - // evalModularCall's own comment). So a name found in - // resolveDispatch() here is unambiguously the real C++ - // builtin. Detected by FUNCTION-POINTER identity against - // &resolveTransform/&resolveColor, not a second, - // independently-maintained name list -- stays in sync - // with registry.cpp automatically if a 7th transform name - // is ever added there. See Op::PushBuiltinWrap's own doc - // comment (bytecode.hpp) for why this specific subset + // A user module never shadows a builtin name in a way that + // reaches here -- the `resolved` check above already ruled + // that out (isBuiltin per evalModularCall's own comment). + // So a name found in resolveDispatch() here is + // unambiguously the real C++ builtin. Detected by + // FUNCTION-POINTER identity against &resolveTransform/ + // &resolveColor/&resolveHull/etc., not a second, + // independently-maintained name list -- stays in sync with + // registry.cpp automatically if a name is ever added to + // one of these groups there. See Op::PushBuiltinWrap's own + // doc comment (bytecode.hpp) for why this specific subset // gets real bytecode instead of falling to // Op::NativeStatement below like every other builtin // still does. @@ -1446,10 +1446,27 @@ class Compiler { auto dispatchIt = dispatch.find(call.name->name); std::optional wrapKind; if (dispatchIt != dispatch.end()) { + using Kind = CompiledChunk::BuiltinWrapSite::Kind; if (dispatchIt->second == &resolveTransform) { - wrapKind = CompiledChunk::BuiltinWrapSite::Kind::Transform; + wrapKind = Kind::Transform; } else if (dispatchIt->second == &resolveColor) { - wrapKind = CompiledChunk::BuiltinWrapSite::Kind::Color; + wrapKind = Kind::Color; + } else if (dispatchIt->second == &resolveHull || dispatchIt->second == &resolveMinkowski || + dispatchIt->second == &resolveRender) { + // Empty params, no compute function needed -- see + // BuiltinWrapSite::Kind::Passthrough's own doc + // comment (bytecode.hpp). + wrapKind = Kind::Passthrough; + } else if (dispatchIt->second == &resolveLinearExtrude) { + wrapKind = Kind::LinearExtrude; + } else if (dispatchIt->second == &resolveRotateExtrude) { + wrapKind = Kind::RotateExtrude; + } else if (dispatchIt->second == &resolveProjection) { + wrapKind = Kind::Projection; + } else if (dispatchIt->second == &resolveOffset) { + wrapKind = Kind::Offset; + } else if (dispatchIt->second == &resolveRoof) { + wrapKind = Kind::Roof; } } if (wrapKind) { diff --git a/src/bytecode_vm.cpp b/src/bytecode_vm.cpp index 8567ea8..700d45e 100644 --- a/src/bytecode_vm.cpp +++ b/src/bytecode_vm.cpp @@ -1056,6 +1056,7 @@ Value driveVm(Evaluator& ev, size_t floor) { // rands() call embedded in the wrapper's own args). const std::uint64_t randsBefore = ev.randsCallCount(); CSGParams params; + CallArgs deferredArgs; // Roof only -- see PendingBuiltinWrap's own doc comment switch (site.kind) { case CompiledChunk::BuiltinWrapSite::Kind::Transform: { BuiltinWrapParams result = @@ -1076,9 +1077,63 @@ Value driveVm(Evaluator& ev, size_t floor) { // own native shape runs its single child // against the untouched ctx too. break; + case CompiledChunk::BuiltinWrapSite::Kind::Passthrough: { + // hull()/minkowski()/render() -- resolve args + // for their own $-propagation-into-children + // side effect only (mirrors resolveHull/ + // resolveMinkowski/resolveRender's own + // `(void)args;`); no params of their own ever. + auto [args, effCtx] = + resolveCallArgs(ev, static_cast(*site.node).arguments, ctx); + (void)args; + f.ctxChain.push_back(std::move(effCtx)); + break; + } + case CompiledChunk::BuiltinWrapSite::Kind::LinearExtrude: { + BuiltinWrapParams result = computeLinearExtrudeParams( + ev, static_cast(*site.node), ctx); + params = std::move(result.params); + f.ctxChain.push_back(std::move(result.ctx)); + break; + } + case CompiledChunk::BuiltinWrapSite::Kind::RotateExtrude: { + BuiltinWrapParams result = computeRotateExtrudeParams( + ev, static_cast(*site.node), ctx); + params = std::move(result.params); + f.ctxChain.push_back(std::move(result.ctx)); + break; + } + case CompiledChunk::BuiltinWrapSite::Kind::Projection: { + BuiltinWrapParams result = + computeProjectionParams(ev, static_cast(*site.node), ctx); + params = std::move(result.params); + f.ctxChain.push_back(std::move(result.ctx)); + break; + } + case CompiledChunk::BuiltinWrapSite::Kind::Offset: { + BuiltinWrapParams result = + computeOffsetParams(ev, static_cast(*site.node), ctx); + params = std::move(result.params); + f.ctxChain.push_back(std::move(result.ctx)); + break; + } + case CompiledChunk::BuiltinWrapSite::Kind::Roof: { + // params stays empty here -- computed at Pop + // instead (computeRoofParams calls ev.warn(), + // which must stay ordered AFTER children; see + // this Kind's own doc comment, bytecode.hpp). + // `args` must be retained, not re-resolved at + // Pop, or its argument expressions would run + // twice. + auto [args, effCtx] = + resolveCallArgs(ev, static_cast(*site.node).arguments, ctx); + deferredArgs = std::move(args); + f.ctxChain.push_back(std::move(effCtx)); + break; + } } ev.treeStack_.emplace_back(); - f.builtinWrapStack.push_back({std::move(params), randsBefore, ins.a}); + f.builtinWrapStack.push_back({std::move(params), randsBefore, ins.a, std::move(deferredArgs)}); ++f.pc; break; } @@ -1093,6 +1148,14 @@ Value driveVm(Evaluator& ev, size_t floor) { f.builtinWrapStack.pop_back(); const CompiledChunk::BuiltinWrapSite& site = f.chunk->builtinWrapSites[static_cast(pending.siteIdx)]; + // Roof's params computation is deferred to here (see + // this Kind's own doc comment, bytecode.hpp) -- ctx is + // still on top of f.ctxChain, not yet popped below, so + // computeRoofParams sees the exact same effCtx the + // children just ran against. + if (site.kind == CompiledChunk::BuiltinWrapSite::Kind::Roof) { + pending.params = computeRoofParams(ev, pending.deferredArgs, f.ctxChain.back()); + } if (site.kind != CompiledChunk::BuiltinWrapSite::Kind::Modifier) f.ctxChain.pop_back(); std::vector> children = std::move(ev.treeStack_.back()); ev.treeStack_.pop_back(); diff --git a/tests/test_bytecode_compiler.cpp b/tests/test_bytecode_compiler.cpp index bd9a508..4e4d2fd 100644 --- a/tests/test_bytecode_compiler.cpp +++ b/tests/test_bytecode_compiler.cpp @@ -1514,6 +1514,120 @@ TEST(ModuleBodyCompiles, CompiledCsgWrapEvaluatesAllAssignmentsBeforeAnyGeometry EXPECT_NEAR(e.bodies[0].body->Volume(), 1.0, 1e-6); } +// -- hull()/minkowski()/render()/linear_extrude()/rotate_extrude()/ -------- +// -- projection()/offset()/roof() -- closing the last real native-reentry -- +// -- gap, via 6 new BuiltinWrapSite::Kind values sharing Op::PushBuiltinWrap +// -- itself rather than a new bracket (see that op's own doc comment). ------ +// Every test here mirrors UnionWrappedRecursionSucceedsWellPastTheOldNative +// ReentryLimit's own shape/depth (1500 -- comfortably past the old 40-level +// native ceiling, comfortably under kMaxCsgTreeDepth=2000). Dimension- +// mismatched constructs (linear_extrude/rotate_extrude/projection/offset/ +// roof all expect 2D children, but nesting one inside itself recursively +// means every non-leaf level's own "child" is really the 3D/2D RESULT of +// the next level down, not raw 2D input) intentionally don't assert a +// specific body count/volume -- recursively self-nesting one of these +// isn't an idiomatic real pattern the way union()/hull()/translate() are; +// the only thing worth proving here is that deep recursion no longer +// throws "Recursion too deep (native call stack)". + +TEST(ModuleBodyCompiles, HullWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + Evaluated e = evalSrc("module recur(n) { hull() { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\n" + "recur(1500);"); + ASSERT_EQ(e.bodies.size(), 1u); +} + +TEST(ModuleBodyCompiles, MinkowskiWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + Evaluated e = evalSrc("module recur(n) { minkowski() { cube(0.1); recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\n" + "recur(1500);"); + ASSERT_EQ(e.bodies.size(), 1u); +} + +TEST(ModuleBodyCompiles, RenderWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + Evaluated e = evalSrc("module recur(n) { render() { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\n" + "recur(1500);"); + ASSERT_EQ(e.bodies.size(), 1u); +} + +TEST(ModuleBodyCompiles, LinearExtrudeWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + evalSrc("module recur(n) { linear_extrude(1) { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { square(1); } }\n" + "recur(1500);"); +} + +TEST(ModuleBodyCompiles, RotateExtrudeWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + evalSrc("module recur(n) { rotate_extrude() { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { translate([1,0]) square(1); } }\n" + "recur(1500);"); +} + +TEST(ModuleBodyCompiles, ProjectionWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + evalSrc("module recur(n) { projection() { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { cube(1); } }\n" + "recur(1500);"); +} + +// offset() is 2D->2D -- unlike the dimension-changing group above, nesting +// it inside itself really does feed each level's REAL 2D output into the +// next level's real offsetting work at generate time, unlike linear_ +// extrude/rotate_extrude/projection/roof (whose intermediate levels see a +// dimension-mismatched, effectively-empty child). offset(r=1) with a +// nonzero radius compounds: each level's rounded-join offsetting grows +// both the shape's size AND its vertex count (more arc segments per +// corner, applied to an already-larger corner count from the level below) +// -- genuinely exponential blowup by depth 1500, confirmed by hanging for +// minutes before this test was rewritten. delta=0 (Square/Miter join, +// never the Round path that adds arc segments) is a real geometric no-op +// at every level -- exercises the exact same compiled opcode path with +// none of the compounding cost. +TEST(ModuleBodyCompiles, OffsetWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + evalSrc("module recur(n) { offset(delta=0) { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { square(1); } }\n" + "recur(1500);"); +} + +TEST(ModuleBodyCompiles, RoofWrappedRecursionSucceedsWellPastTheOldNativeReentryLimit) { + ScopedVm vm(true); + evalSrc("module recur(n) { roof() { recur2(n); } }\n" + "module recur2(n) { if (n > 0) { recur(n - 1); } else { square(4, center=true); } }\n" + "recur(1500);"); +} + +// $-named-argument propagation into a compiled Passthrough-kind wrap's +// children -- mirrors DollarArgPropagatesIntoCompiledCsgWrapChildren, above, +// for the OTHER kind (Passthrough/LinearExtrude/etc.) that also pushes a +// possibly-$-scoped ctx unconditionally. +TEST(ModuleBodyCompiles, DollarArgPropagatesIntoCompiledHullWrapChildren) { + ScopedVm vm(true); + EXPECT_EQ(runCapturingEcho("module m() { hull($fn = 9) { echo($fn); cube(1); } }\n" + "m();"), + "ECHO: 9"); +} + +// The ONE behavioral subtlety Op::PushBuiltinWrap's Roof kind introduces: +// computeRoofParams's own "Unknown roof method" warning is computed at POP +// time (after children), deliberately, so it stays ordered AFTER any +// echo()/warn() a child produces -- exactly matching native resolveRoof's +// own evalChildren-then-compute-params order. Pins that ordering under the +// compiled path specifically (every other kind in this group computes its +// params at PUSH time instead, since none of them has an order-sensitive +// side effect to preserve). +TEST(ModuleBodyCompiles, CompiledRoofUnknownMethodWarningStaysOrderedAfterChildrensOwnEcho) { + ScopedVm vm(true); + EXPECT_EQ(runCapturingEcho("module m() { roof(method=\"bogus\") { echo(\"child\"); square(4, center=true); } }\n" + "m();"), + "ECHO: \"child\"\nWARNING: Unknown roof method 'bogus'. Using 'voronoi'."); +} + // The BOSL2 attachable() shape this whole effort targets: a wrapper module // whose body is just `children();`, applied at every level of a recursive // chain. children() used to fall to Op::NativeStatement -- one genuine