render() in expression position: measure your own geometry - #104
Merged
Conversation
obj = render() { difference() { cube(100); sphere(20); } };
echo(obj.volume, obj.genus, obj.boundingbox);
polyhedron(obj);
OpenSCAD could not previously measure its own geometry: the numbers only
exist after the geometry is built, and the language had no way to reach
them. This adds the evaluator half (the parser half is the submodule bump
in this same commit).
It measures and DISCARDS -- nothing is drawn. That is what makes the
semantics uniform: no side effects, so top level, module bodies, function
bodies, list comprehensions and ternaries all behave identically, and
function purity survives. `function fits(w) = render() { cube(w); }.volume
< 500;` works with no exception carved out for it.
The two-pass problem needed no new pipeline. generatePartialTree() already
generates mid-resolve for the debugger's live render, and documents why it
is safe (the resolve pass never reads CSGNode::bodies). measureCsgSubtree
does the same thing for one subtree, on its own treeStack_ frame, which is
popped and discarded so it can never reach the drawn tree.
Discarding costs one guard. measuring_ suppresses the four writes that
exist solely to describe DRAWN geometry -- idToNode/idToColor in
tagGenerated and tagDisplayOnly, the restampCachedIds call on a cache hit,
and cacheProducer_ -- because those tables are cleared once per pass, so a
leak is permanent and surfaces much later as wrong click-to-source. The
geometry cache itself deliberately stays ON: cacheKey is content-addressed,
so an entry a measurement stores is genuinely reusable by the real render.
The same flag suppresses checkDebug, which would otherwise inject stops at
the paused statement's own callStack_ depth and corrupt
lastStmtByDepth_'s duplicate-collapse state.
Mesh output is VNF-shaped -- [x,y,z] vertices, 0-based faces, CLOCKWISE
seen from outside -- so it feeds polyhedron() and BOSL2 unchanged. Two
details there are load-bearing and silent when wrong: the winding is
reversed on the way out (Manifold's triVerts is CCW), and vertices are
welded by exact position (Manifold splits property-vertices, so without
the weld the round-tripped polyhedron is an OPEN mesh). Both are pinned by
a round-trip test that asserts a POSITIVE volume -- verified empirically
that a reversed mesh comes back as volume -1 with Status()==NoError, so
the sign is the whole signal and abs() would hide it.
polyhedron() and polygon() now accept the object directly, so the round
trip is one call rather than two arguments. Works for any object with the
keys, not just one render() produced.
Measurements come from Manifold (Volume/SurfaceArea/Genus/BoundingBox),
never hand-computed. An empty result reports dim=0 with boundingbox=undef
rather than Manifold's empty Box, which is {+inf,-inf} and would poison
downstream arithmetic. An open surface still returns its mesh, with volume
and genus unavailable.
Not yet done: the bytecode VM currently falls back to the interpreter for
a declaration containing a render expression, via compileExpr's existing
NotCompilable safety net. A real Kind::Measure bracket follows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A declaration containing a render() expression previously fell back to the
interpreter wholesale, via compileExpr's NotCompilable safety net. The
interpreter is many times slower than the VM, so a single measurement
anywhere in a function dragged that whole function onto the slow path.
No new opcode. A new BuiltinWrapSite::Kind::Measure reuses the existing
Op::PushBuiltinWrap/PopBuiltinWrap pair, which is what lets it inherit the
entire bracket lifecycle -- push/pop counting, ctxChain discipline,
exception teardown -- for free. A new op pair would have needed a fourth
parallel per-frame stack, and the teardown's pop ordering would then have
had to satisfy both nesting orders at once.
That ordering turned out to be a non-problem, and the comments claiming
otherwise are corrected here: the three loops in teardownVmCallStackDownTo
are COUNTS, not targeted pops. Popping N off the back of treeStack_ removes
the top N whichever group counted them, so the grouping is bookkeeping, not
sequencing. Only measuring_ needed restoring, which is one line reading
front().savedMeasuring -- recorded by every kind so no kind-inspection is
needed.
The Pop branch's RAII guard deliberately wraps measureCsgSubtree rather
than preceding it: measuring_ must stay true across the generate, since
that flag is what suppresses the four provenance writes. Restoring early
would leave them inert on the VM path only -- no crash, no wrong geometry,
just silently wrong click-to-source. ProvenanceStaysCleanUnderTheVm is the
only thing that catches it.
One real bug this surfaced, and it is why the interpreter landed first:
statement opcodes now run inside FUNCTION chunks for the first time, and a
compiled function keeps its parameters and lets in frame SLOTS that no
EvalContext can see. The children of a render expression resolve names
through the context, so `function f(w) = render() { cube(w); }.volume;`
found no `w` at all and silently measured an undef-sized cube -- returning
volume 0 rather than erroring. Kind::Measure now captures (name, slot) for
every visible local at compile time and republishes them into the
children's context at Push. Caught by the test suite, not by inspection.
Op::PopBuiltinWrap asserts both the operand-stack and treeStack_ depths it
recorded at Push. Every compileOneStatement case is operand-stack-neutral,
but nothing structurally enforces that, and this is the first bracket to
run statements with a non-empty operand stack beneath it.
Tests: 12 scripts asserted identical under both engines, a positive check
that the chunks actually compile (the regression guard against
reintroducing the bail), captured-locals, provenance under the VM, and five
throw paths including one originating deep inside a nested module frame so
the multi-frame teardown loop runs. 953 tests pass under both
OSCAD_BYTECODE_VM=0 and =1, and clean under ASan.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Minor, not patch: a new language construct plus a newly reserved word. The CLAUDE.md section leads with the two things that will actually bite someone -- `render` can no longer be an identifier, and `obj = render() cube(1);` does not parse (the child_statement swallows the semicolon) -- then the non-obvious invariants: winding is reversed on the way out, vertices are welded by exact position, a reversed mesh reports NEGATIVE volume so tests must never use abs(), and genus is Manifold's for the whole result so a sealed internal cavity reports -1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BelfrySCAD/openscad_cpp_parser#5 (render() in expression position) is in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenSCAD could not previously measure its own geometry — the numbers only exist after the geometry is built, and the language had no way to reach them. Requires BelfrySCAD/openscad_cpp_parser#5 (submodule bumped here).
It measures and discards — nothing is drawn
That is what makes the semantics uniform. No side effects, so top level, module bodies, function bodies, list comprehensions and ternaries all behave identically, and function purity survives:
A draw-and-measure design would have had to forbid this.
The two-pass problem needed no new pipeline
generatePartialTree()already generates mid-resolve for the debugger's live render, and its doc comment already reasons about why that is safe (the resolve pass never readsCSGNode::bodies).measureCsgSubtreedoes the same for one subtree, on its owntreeStack_frame, popped and discarded so it can never reach the drawn tree.Discarding costs one guard
measuring_suppresses the four writes that exist solely to describe drawn geometry —idToNode/idToColorintagGeneratedandtagDisplayOnly, therestampCachedIdscall on a cache hit, andcacheProducer_. Those tables are cleared once per pass, so a leak is permanent and surfaces much later as wrong click-to-source. The same flag suppressescheckDebug, which would otherwise inject stops at the paused statement's owncallStack_depth.The geometry cache deliberately stays on:
cacheKeyis content-addressed, so an entry a measurement stores is genuinely reusable by the real render.Real bytecode, not an interpreter bail
A new
BuiltinWrapSite::Kind::Measurereuses the existingOp::PushBuiltinWrap/PopBuiltinWrappair, inheriting the whole bracket lifecycle — push/pop counting,ctxChaindiscipline, exception teardown — for free. No new opcode, and no change to thetreeStack_unwind.The teardown's "respects LIFO order" comments turned out to be misleading and are corrected here: those three loops are counts, not targeted pops. Popping N off the back removes the top N whichever group counted them, so the grouping is bookkeeping, not sequencing.
One real bug this surfaced — and why the interpreter landed first: statement opcodes now run inside function chunks for the first time, and a compiled function keeps parameters in frame slots that no
EvalContextcan see.function f(w) = render() { cube(w); }.volume;found nowand silently measured an undef-sized cube, returning 0 rather than erroring.Kind::Measurenow captures(name, slot)at compile time and republishes into the children's context.Mesh output is VNF-shaped
[x,y,z]vertices, 0-based N-gon faces, clockwise seen from outside — feedspolyhedron()and BOSL2 unchanged.polyhedron()andpolygon()also take the object directly, and accept any object with the right keys.Two details are load-bearing and silent when wrong: winding is reversed on the way out (Manifold's
triVertsis CCW), and vertices are welded by exact position (Manifold splits property-vertices; without the weld the round-trip is an open mesh). Verified empirically that a reversed mesh comes back as negative volume withStatus() == NoError, so tests assert a positive volume and never useabs().Things that will bite
renderis now a reserved keyword — see the parser PR for why LALR(1) leaves no alternative.obj = render() cube(1);does not parse. The child_statement swallows the;. Use the braced form. Asserted in tests.genusis Manifold's, for the whole result. A cube with a sealed cavity reports-1, not0— its boundary has two components.Testing
953 tests pass under both
OSCAD_BYTECODE_VM=0and=1, and clean under ASan. New coverage includes 12 scripts asserted identical across both engines, a positive check that the chunks actually compile (the regression guard against reintroducing the bail), provenance-cleanliness under the VM specifically, and five throw paths including one originating deep inside a nested module frame.Verified end to end through BelfrySCAD's CLI: the round-trip reproduces 908 facets / 458 vertices and the identical surface area and bounding box.
(An unrelated pre-existing ASan heap-buffer-overflow in Manifold's own
sort.cpp:55, hit byMinkowskiDifference.ShrinksANonConvexBodyOnEverySide, reproduces identically on a clean checkout of4f20db3— confirmed by building that worktree separately. Not touched here.)🤖 Generated with Claude Code