add polyhedron support.

[julialongtin]

No description provided.

[Copilot]

Pull request overview

Adds initial polyhedron support across the core symbolic object model, SDF evaluation, OpenSCAD-like frontend, and OpenSCAD exporter.

Changes:

• Introduces Polyhedron as a new SymbolicObj3 primitive and exposes a polyhedron constructor in Graphics.Implicit.Primitives.
• Adds triangle/mesh utilities (Graphics.Implicit.TriUtil) and implements polyhedron distance evaluation and bounding-box calculation.
• Extends the ExtOpenScad pipeline (primitives, OVal definitions, statement eval, docgen) to support polyhedron modules and suite-vs-no-suite module variants.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/Graphics/Implicit/Test/Instances.hs	Adds Arbitrary ℕ to support new polyhedron-related types in tests.
programs/docgen.hs	Updates doc generation to handle the new ONModuleWithSuite variant.
implicit.cabal	Registers new internal module Graphics.Implicit.TriUtil and adds build-option comments.
Makefile	Improves examples target logging by echoing full command lines.
Graphics/Implicit/TriUtil.hs	New triangle utilities: closest-feature, point-to-triangle distance, triangle normal, etc.
Graphics/Implicit/Primitives.hs	Exports and defines the polyhedron primitive constructor.
Graphics/Implicit/ObjectUtil/GetImplicit3.hs	Implements SDF evaluation for Polyhedron using triangle distances and inside/outside classification.
Graphics/Implicit/ObjectUtil/GetBox3.hs	Implements bounding box computation for Polyhedron.
Graphics/Implicit/ExtOpenScad/Util/OVal.hs	Adds type-string support for ONModuleWithSuite.
Graphics/Implicit/ExtOpenScad/Primitives.hs	Adds polyhedron module and refactors built-in modules into suite vs non-suite variants.
Graphics/Implicit/ExtOpenScad/Eval/Statement.hs	Updates module call evaluation to support ONModuleWithSuite.
Graphics/Implicit/ExtOpenScad/Definitions.hs	Extends OVal with ONModuleWithSuite and updates Show/exports accordingly.
Graphics/Implicit/Export/TextBuilderUtils.hs	Adds bℕ helper for emitting ℕ values in exporters.
Graphics/Implicit/Export/SymbolicFormats.hs	Adds OpenSCAD export support for Polyhedron.
Graphics/Implicit/Definitions.hs	Adds Polyhedron constructor to SymbolicObj3 and its Show instance.
Graphics/Implicit/Canon.hs	Extends canonicalization/equality machinery to include Polyhedron.
Examples/example26-Polyhedron.scad	Adds an example demonstrating polyhedron usage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When denom == 0, closestFeatureToTriangle returns (FeatFace, p), which makes distancePointToTriangle report zero distance even if the triangle is degenerate (or the point is not actually on the triangle). Handle denom == 0 by falling back to a segment/vertex distance computation for degenerate triangles instead of returning the input point.

[Copilot]

The Arbitrary ℕ instance is recursively defined: getPositive <$> arbitrary requires Arbitrary (Positive ℕ), which in turn requires Arbitrary ℕ, causing infinite recursion when generating values. Generate a base numeric type (e.g., Integer/Int) and convert with toℕ, or use choose to build a non-negative ℕ directly.

Suggested change

	arbitrary = do
	n <- getPositive <$> arbitrary
	return n
	arbitrary =
	(fromInteger . getNonNegative) <$> (arbitrary :: Gen (NonNegative Integer))

[Copilot]

There is a stray let with no bindings before the when statement in the ONModuleWithSuite branch, which is a parse/compile error. Remove the let or add the missing bindings.

[Copilot]

The generated OpenSCAD for polyhedron concatenates points and faces into a single argument string without a separating comma, which will produce invalid OpenSCAD (arguments must be comma-separated). Pass points=... and faces=... as separate entries in the argument list (or insert a comma between them).

Suggested change

	buildS3 _ (Polyhedron points tris) = callNaked "polyhedron" ["points = [" <> (fold $ intersperse "," $ renderPoint <$> points) <> "] faces = [" <> (fold $ intersperse "," $ renderTri <$> tris) <> "]" ] []
	buildS3 _ (Polyhedron points tris) =
	callNaked "polyhedron"
	[ "points = [" <> (fold $ intersperse "," $ renderPoint <$> points) <> "]"
	, "faces = [" <> (fold $ intersperse "," $ renderTri <$> tris) <> "]"
	] []

[Copilot]

pointOnOutside rebuilds triSeq, triByEdge, triByVertex, triangles, etc. on every distance query, even though they only depend on points/tris. Since getImplicit3’s result is called many times during meshing, this will be a major slowdown. Precompute these structures once (outside the returned \point -> ...) and reuse them per query.

[Copilot]

In the ONModule branch, the suite argument from the AST is ignored. This changes behavior: providing a suite to a module that doesn't accept one (often a missing semicolon) will no longer error. Add an explicit check that suite is empty and emit an error when it is not.

[Copilot]

This polyhedron bounding-box computation uses lazy foldl, which can build large thunks for big meshes and lead to high memory use. Use a strict fold (e.g., foldl') for accumulating min/max.

[Copilot]

The polyhedron example references vertex index 5, but the example points list has only 5 elements (indices 0..4). This makes the example invalid and can confuse users/doc generation; update the example indices to match the points list.

Suggested change

	example "polyhedron(points=[[0,0,0], [2,0,0], [2,2,0], [0,2,0], [1, 1, 2]], faces=[[0,1,2,3], [0,5,1], [1,5,2], [2,5,3], [3,5,4], [4,5,0]]);"
	example "polyhedron(points=[[0,0,0], [2,0,0], [2,2,0], [0,2,0], [1,1,2]], faces=[[0,1,2,3], [0,4,1], [1,4,2], [2,4,3], [3,4,0]]);"

[Copilot]

polyhedron defaults points/faces to empty lists and still returns Prim.polyhedron points woundTris, which will later crash in getImplicit3 with a runtime error. Validate points/faces (and that generated tris is non-empty) here and report the issue via errorC so the user gets a source-positioned error instead of a later runtime exception.

Suggested change

	-- A tri is constructed of three indexes into the points.
	tris <- fmap concat $ mapM (trianglesFromFace sourcePos) faces
	woundTris <- reWindTriangles sourcePos points tris
	pure [OObj3 $ Prim.polyhedron points woundTris]
	-- Helper: conditionally run an action in the StateC monad.
	let whenState cond action = if cond then action else pure ()
	-- Validate that we actually have points and faces before proceeding.
	whenState (null points) $
	errorC sourcePos "polyhedron: no points were specified; 'points' cannot be an empty list.\n"
	whenState (null faces) $
	errorC sourcePos "polyhedron: no faces were specified; 'faces' cannot be an empty list.\n"
	-- A tri is constructed of three indexes into the points.
	tris <- fmap concat $ mapM (trianglesFromFace sourcePos) faces
	-- If we could not generate any triangles, abort with a clear error instead
	-- of failing later inside getImplicit3.
	if null tris
	then do
	errorC sourcePos "polyhedron: no valid triangles could be generated from the given faces.\n"
	pure []
	else do
	woundTris <- reWindTriangles sourcePos points tris
	pure [OObj3 $ Prim.polyhedron points woundTris]

[Copilot]

reWindTriangles computes safeTri using genericIndex points p1/p2/p3 without validating that face indices are in bounds. Invalid indices will throw a runtime exception without source position. Add explicit bounds checks (or reuse findTriangle which already checks indices) and surface failures through errorC sourcePos.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

Graphics/Implicit/ExtOpenScad/Eval/Statement.hs:152

• In the ONModuleWithSuite branch, when the suite is missing you call errorC but still proceed to run the module with an empty suiteResults. Because errorC doesn’t abort, this can cascade into additional errors or unexpected output. Consider returning pure [] immediately when null vals to stop evaluation after reporting the missing-suite error.

              vals <- runSuiteCapture varlookup suite
              suiteResults <- do
                              when (null vals) (errorC sourcePos "Suite required, but none provided.")
                              pure vals
              -- Run the module.
              let
                argsMapped = argMap evaluatedArgs $ implementation sourcePos suiteResults
              for_ (pack <$> snd argsMapped) $ errorC sourcePos

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.