Resolve #86: non-planar polyhedron() mismatch was already fixed, add regression test

docs/roadmap.md

@@ -371,11 +371,8 @@ exactly once unblocked.
 Confirmed via the same corpus subdirectory (`3D/features`) but **not yet
 fixed**:
 
-- [ ] **Non-planar polyhedron faces** (issue #86) (`polyhedron-nonplanar-tests`) still
-  mismatch after the winding fix (`2.94` vs `1.29`) — a real but distinct
-  issue: fan-triangulating a non-planar quad from vertex 0 picks a different
-  diagonal than whatever OpenSCAD/CGAL does, producing a different (smaller)
-  volume. Needs its own investigation, not just a winding flip.
+- [x] **Non-planar polyhedron faces** (issue #86) (`polyhedron-nonplanar-tests`) —
+  turned out **not** to be a fan-triangulation bug at all; see v3.11 below.
 - [x] **`linear_extrude()`'s default height was 1, not OpenSCAD's actual
   default of 100.** Found bisecting `linear_extrude-tests.scad` line by
   line: `linear_extrude(v=[3,2,5]) square([10,10])` (no `height=` given)
@@ -582,6 +579,54 @@ been checked (matching v3.9's existing note); `2D`, `bugs`, `bugs2D`,
 `misc`, `issues` are still unexamined and may turn up further parse
 failures of their own.
 
+## v3.11 — issue #86 (non-planar polyhedron() faces) re-investigated and closed
+
+This environment turned out to have real `apt` network access after all
+(unlike the v3.10 pass): `apt-get install openscad` gives a live 2021.01
+oracle, and Manifold v3.5.2 builds from source in ~20s exactly as
+`tests/tools/README.md` describes, so this pass could go back to the actual
+volumetric `scad_to_stl`/`stl_diff` methodology instead of language-level
+checks only.
+
+- [x] **Issue #86 was misdiagnosed — not a fan-triangulation bug.**
+  `polyhedron-nonplanar-tests.scad`'s reported `2.94` (real OpenSCAD) vs.
+  `1.29` (ChiselCAD) mismatch was bisected by isolating each of the file's
+  three `polyhedron()` calls into its own `.scad` file and diffing each
+  individually against a live OpenSCAD 2021.01 + Manifold v3.5.2 oracle.
+  Two of the three (the slightly-non-planar hexahedron and the small
+  heptagon-pyramid) already matched to floating-point noise (`rel_error` ~
+  1e-7) — meaning the fan-triangulation-from-vertex-0 approach
+  `CsgEvaluator::evalPolyhedron` uses was never actually wrong for this
+  corpus file. The third — a real-world truncated icosidodecahedron wrapped
+  in `scale(0.02) polyhedron(...)` — reproduced the mismatch in isolation
+  (`volume_a=1.65443 volume_b=0`, confirmed by rebuilding at the exact
+  commit (`696522c`) the issue was filed against). The actual cause: `scale(
+  0.02)` is a **bare scalar**, and at that commit `CsgEvaluator::makeMatrix`
+  still had the pre-v3.10 `scale(<scalar>)` bug (see v3.10's `for`/`rotate`/
+  `scale` fixes) that scaled only the Z axis — turning this polyhedron into
+  a wildly non-uniform, self-intersecting mesh that Manifold's `Status()`
+  still reported as `NoError` but whose true volume collapsed to ~0 after
+  boolean cleanup. That bug was already fixed in commit `18f7a54` (filed
+  under issue #90, not #86) as an incidental side effect of unrelated
+  parser work — nobody had re-run the v3.9 volumetric corpus check
+  afterward to notice it also closed #86. Re-running it now:
+  `polyhedron-nonplanar-tests.scad`'s full file matches the oracle at
+  `rel_error=3.5e-6`, and the previously-broken `scale(0.02)` case alone
+  matches at `rel_error=1.4e-6` — both floating-point noise, no code change
+  needed to `CsgEvaluator::evalPolyhedron`/`PrimitiveGen` itself. Added
+  `tests/fixtures/headless/polyhedron_nonplanar_scale.scad` (the exact
+  reproducing case, straight from the OpenSCAD corpus) and a
+  `chiselcad_tests` regression pinning its volume to the oracle's
+  `1.6544281372`, so this doesn't silently regress independently of the
+  `scale(scalar)` unit test that already covers the actual root cause.
+- Takeaway for future corpus-mismatch triage: a mismatch reported against
+  a file with multiple unrelated constructs (here, both a genuinely
+  non-planar `polyhedron()` *and* a bare-scalar `scale()`) doesn't
+  necessarily implicate the construct the file is named after — isolating
+  each construct individually (as this pass and the original v3.9 harness-
+  bug triage both did) is worth doing before writing a fix for the wrong
+  code path.
+
 ## v4 — Tooling & Visual Quality
 
 - [ ] VS Code LSP extension (syntax highlighting, error squiggles, completions)

tests/fixtures/headless/polyhedron_nonplanar_scale.scad

@@ -0,0 +1,135 @@
+// Real-world non-planar polyhedron() example (truncated icosidodecahedron)
+// from OpenSCAD's own test corpus
+// (tests/data/scad/3D/features/polyhedron-nonplanar-tests.scad upstream),
+// wrapped in scale(0.02) — a bare scalar, not a [x,y,z] vector. See
+// docs/roadmap.md v3.9/v3.10 and issue #86.
+translate([4.5,0.5,0.5]) scale(0.02) polyhedron(points = [
+[-10., -13.090169943749475, -34.270509831248425],
+[-10., -13.090169943749475, 34.270509831248425],
+[-10., 13.090169943749475, -34.270509831248425],
+[-10., 13.090169943749475, 34.270509831248425],
+[-5., -5., -37.3606797749979], [-5., -5., 37.3606797749979],
+[-5., 5., -37.3606797749979], [-5., 5., 37.3606797749979],
+[-5., -37.3606797749979, -5.], [-5., -37.3606797749979, 5.],
+[-5., -21.18033988749895, -31.18033988749895],
+[-5., -21.18033988749895, 31.18033988749895], [-5., 37.3606797749979, -5.],
+[-5., 37.3606797749979, 5.], [-5., 21.18033988749895, -31.18033988749895],
+[-5., 21.18033988749895, 31.18033988749895], [5., -5., -37.3606797749979],
+[5., -5., 37.3606797749979], [5., 5., -37.3606797749979],
+[5., 5., 37.3606797749979], [5., -37.3606797749979, -5.],
+[5., -37.3606797749979, 5.], [5., -21.18033988749895, -31.18033988749895],
+[5., -21.18033988749895, 31.18033988749895], [5., 37.3606797749979, -5.],
+[5., 37.3606797749979, 5.], [5., 21.18033988749895, -31.18033988749895],
+[5., 21.18033988749895, 31.18033988749895], [10., -13.090169943749475,
+ -34.270509831248425], [10., -13.090169943749475, 34.270509831248425],
+[10., 13.090169943749475, -34.270509831248425],
+[10., 13.090169943749475, 34.270509831248425],
+[-34.270509831248425, -10., -13.090169943749475],
+[-34.270509831248425, -10., 13.090169943749475],
+[-34.270509831248425, 10., -13.090169943749475],
+[-34.270509831248425, 10., 13.090169943749475],
+[-29.270509831248425, -18.090169943749473, -16.18033988749895],
+[-29.270509831248425, -18.090169943749473, 16.18033988749895],
+[-29.270509831248425, 18.090169943749473, -16.18033988749895],
+[-29.270509831248425, 18.090169943749473, 16.18033988749895],
+[-18.090169943749473, -16.18033988749895, -29.270509831248425],
+[-18.090169943749473, -16.18033988749895, 29.270509831248425],
+[-18.090169943749473, 16.18033988749895, -29.270509831248425],
+[-18.090169943749473, 16.18033988749895, 29.270509831248425],
+[-13.090169943749475, -34.270509831248425, -10.],
+[-13.090169943749475, -34.270509831248425, 10.],
+[-13.090169943749475, -24.270509831248425, -26.18033988749895],
+[-13.090169943749475, -24.270509831248425, 26.18033988749895],
+[-13.090169943749475, 24.270509831248425, -26.18033988749895],
+[-13.090169943749475, 24.270509831248425, 26.18033988749895],
+[-13.090169943749475, 34.270509831248425, -10.],
+[-13.090169943749475, 34.270509831248425, 10.],
+[-26.18033988749895, -13.090169943749475, -24.270509831248425],
+[-26.18033988749895, -13.090169943749475, 24.270509831248425],
+[-26.18033988749895, 13.090169943749475, -24.270509831248425],
+[-26.18033988749895, 13.090169943749475, 24.270509831248425],
+[-37.3606797749979, -5., -5.], [-37.3606797749979, -5., 5.],
+[-37.3606797749979, 5., -5.], [-37.3606797749979, 5., 5.],
+[-16.18033988749895, -29.270509831248425, -18.090169943749473],
+[-16.18033988749895, -29.270509831248425, 18.090169943749473],
+[-16.18033988749895, 29.270509831248425, -18.090169943749473],
+[-16.18033988749895, 29.270509831248425, 18.090169943749473],
+[-31.18033988749895, -5., -21.18033988749895],
+[-31.18033988749895, -5., 21.18033988749895],
+[-31.18033988749895, 5., -21.18033988749895],
+[-31.18033988749895, 5., 21.18033988749895],
+[-21.18033988749895, -31.18033988749895, -5.],
+[-21.18033988749895, -31.18033988749895, 5.],
+[-21.18033988749895, 31.18033988749895, -5.],
+[-21.18033988749895, 31.18033988749895, 5.],
+[-24.270509831248425, -26.18033988749895, -13.090169943749475],
+[-24.270509831248425, -26.18033988749895, 13.090169943749475],
+[-24.270509831248425, 26.18033988749895, -13.090169943749475],
+[-24.270509831248425, 26.18033988749895, 13.090169943749475],
+[16.18033988749895, -29.270509831248425, -18.090169943749473],
+[16.18033988749895, -29.270509831248425, 18.090169943749473],
+[16.18033988749895, 29.270509831248425, -18.090169943749473],
+[16.18033988749895, 29.270509831248425, 18.090169943749473],
+[24.270509831248425, -26.18033988749895, -13.090169943749475],
+[24.270509831248425, -26.18033988749895, 13.090169943749475],
+[24.270509831248425, 26.18033988749895, -13.090169943749475],
+[24.270509831248425, 26.18033988749895, 13.090169943749475],
+[37.3606797749979, -5., -5.], [37.3606797749979, -5., 5.],
+[37.3606797749979, 5., -5.], [37.3606797749979, 5., 5.],
+[21.18033988749895, -31.18033988749895, -5.],
+[21.18033988749895, -31.18033988749895, 5.],
+[21.18033988749895, 31.18033988749895, -5.],
+[21.18033988749895, 31.18033988749895, 5.],
+[13.090169943749475, -34.270509831248425, -10.],
+[13.090169943749475, -34.270509831248425, 10.],
+[13.090169943749475, -24.270509831248425, -26.18033988749895],
+[13.090169943749475, -24.270509831248425, 26.18033988749895],
+[13.090169943749475, 24.270509831248425, -26.18033988749895],
+[13.090169943749475, 24.270509831248425, 26.18033988749895],
+[13.090169943749475, 34.270509831248425, -10.],
+[13.090169943749475, 34.270509831248425, 10.],
+[26.18033988749895, -13.090169943749475, -24.270509831248425],
+[26.18033988749895, -13.090169943749475, 24.270509831248425],
+[26.18033988749895, 13.090169943749475, -24.270509831248425],
+[26.18033988749895, 13.090169943749475, 24.270509831248425],
+[31.18033988749895, -5., -21.18033988749895],
+[31.18033988749895, -5., 21.18033988749895],
+[31.18033988749895, 5., -21.18033988749895],
+[31.18033988749895, 5., 21.18033988749895],
+[18.090169943749473, -16.18033988749895, -29.270509831248425],
+[18.090169943749473, -16.18033988749895, 29.270509831248425],
+[18.090169943749473, 16.18033988749895, -29.270509831248425],
+[18.090169943749473, 16.18033988749895, 29.270509831248425],
+[29.270509831248425, -18.090169943749473, -16.18033988749895],
+[29.270509831248425, -18.090169943749473, 16.18033988749895],
+[29.270509831248425, 18.090169943749473, -16.18033988749895],
+[29.270509831248425, 18.090169943749473, 16.18033988749895],
+[34.270509831248425, -10., -13.090169943749475],
+[34.270509831248425, -10., 13.090169943749475],
+[34.270509831248425, 10., -13.090169943749475],
+[34.270509831248425, 10., 13.090169943749475]],faces =
+[[41, 53, 65, 67, 55, 43, 3, 7, 5, 1], [100, 104, 106, 102, 110, 30, 18, 16,
+ 28, 108], [11, 1, 5, 17, 29, 23], [18, 30, 26, 14, 2, 6],
+[33, 37, 73, 69, 68, 72, 36, 32, 56, 57], [91, 90, 82, 114, 118, 86, 87,
+ 119, 115, 83], [81, 113, 117, 85, 84, 116, 112, 80, 88, 89],
+[59, 58, 34, 38, 74, 70, 71, 75, 39, 35], [0, 10, 22, 28, 16, 4],
+[15, 27, 31, 19, 7, 3], [64, 52, 40, 0, 4, 6, 2, 42, 54, 66],
+[19, 31, 111, 103, 107, 105, 101, 109, 29, 17], [96, 110, 102, 114, 82, 78],
+[53, 41, 47, 61, 73, 37], [43, 49, 15, 3], [94, 108, 28, 22],
+[23, 29, 109, 95], [2, 14, 48, 42], [36, 72, 60, 46, 40, 52],
+[79, 83, 115, 103, 111, 97], [69, 45, 9, 8, 44, 68],
+[24, 98, 90, 91, 99, 25], [77, 95, 109, 101, 113, 81],
+[42, 48, 62, 74, 38, 54], [40, 46, 10, 0], [97, 111, 31, 27],
+[44, 8, 20, 92, 76, 94, 22, 10, 46, 60], [63, 51, 13, 25, 99, 79, 97, 27,
+ 15, 49], [26, 30, 110, 96], [1, 11, 47, 41], [55, 39, 75, 63, 49, 43],
+[80, 112, 100, 108, 94, 76], [48, 14, 26, 96, 78, 98, 24, 12, 50, 62],
+[61, 47, 11, 23, 95, 77, 93, 21, 9, 45], [71, 70, 50, 12, 13, 51],
+[93, 89, 88, 92, 20, 21], [102, 106, 118, 114], [65, 53, 37, 33],
+[74, 62, 50, 70], [77, 81, 89, 93], [101, 105, 117, 113], [66, 54, 38, 34],
+[73, 61, 45, 69], [78, 82, 90, 98], [32, 36, 52, 64], [115, 119, 107, 103],
+[92, 88, 80, 76], [71, 51, 63, 75], [56, 32, 64, 66, 34, 58],
+[107, 119, 87, 85, 117, 105], [35, 39, 55, 67], [112, 116, 104, 100],
+[99, 91, 83, 79], [68, 44, 60, 72], [57, 59, 35, 67, 65, 33],
+[116, 84, 86, 118, 106, 104], [4, 16, 18, 6], [7, 19, 17, 5],
+[12, 24, 25, 13], [9, 21, 20, 8], [56, 58, 59, 57], [85, 87, 86, 84]]
+);

tests/test_headless_build.cpp

@@ -109,6 +109,28 @@ TEST_CASE("runBuild: linear_extrude()'s default height is 100, not 1",
     CHECK(result.volume == Approx(10.0 * 10.0 * 100.0).margin(1e-6));
 }
 
+TEST_CASE("runBuild: non-planar polyhedron() under scale(<bare scalar>) matches real OpenSCAD",
+          "[headless][v39][bugfix]") {
+    // Issue #86 reported this file (a real-world truncated-icosidodecahedron
+    // polyhedron() from OpenSCAD's own test corpus, wrapped in scale(0.02))
+    // as a "fan-triangulation of non-planar faces" bug: volume 1.2887 here
+    // vs. real OpenSCAD's 1.6544. Bisecting individual polyhedron() calls
+    // from that corpus file against a live OpenSCAD 2021.01 + Manifold
+    // v3.5.2 oracle (see tests/tools/README.md) shows the fan-triangulated
+    // mesh itself was never the problem — the *actual* cause was the
+    // already-fixed scale(<bare scalar>) bug (see "CsgEval:scale(scalar)
+    // broadcasts to all three axes" in test_csg_evaluator.cpp): scale(0.02)
+    // was scaling only the Z axis, badly distorting this polyhedron into a
+    // self-intersecting mesh that Manifold's boolean cleanup collapsed to
+    // near-zero volume. With that fix in place this file already matches
+    // the oracle's 1.6544281 to ~1e-6 relative error; this test pins that
+    // so the two don't silently regress independently of each other again.
+    chisel::csg::MeshCache cache;
+    BuildResult result = runBuild(fixture("headless/polyhedron_nonplanar_scale.scad"), {}, {}, cache);
+    REQUIRE(result.ok());
+    CHECK(result.volume == Approx(1.6544281372).margin(1e-5));
+}
+
 TEST_CASE("runBuild honors AbortFn by returning early", "[headless]") {
     chisel::csg::MeshCache cache;
     auto alwaysAbort = [] { return true; };