diff --git a/docs/roadmap.md b/docs/roadmap.md index 25b5c33..7d371e1 100644 --- a/docs/roadmap.md +++ b/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()` 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) diff --git a/tests/fixtures/headless/polyhedron_nonplanar_scale.scad b/tests/fixtures/headless/polyhedron_nonplanar_scale.scad new file mode 100644 index 0000000..8dbc722 --- /dev/null +++ b/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]] +); diff --git a/tests/test_headless_build.cpp b/tests/test_headless_build.cpp index 336a440..91bff88 100644 --- a/tests/test_headless_build.cpp +++ b/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() 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() 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; };