Skip to content

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

Merged
particlesector merged 2 commits into
mainfrom
claude/issue-list-review-5as3na
Jul 30, 2026
Merged

Resolve #86: non-planar polyhedron() mismatch was already fixed, add regression test#99
particlesector merged 2 commits into
mainfrom
claude/issue-list-review-5as3na

Conversation

@particlesector

Copy link
Copy Markdown
Owner

Closes #86

Summary

  • Re-investigated issue [Medium] Non-planar polyhedron() faces triangulate differently than OpenSCAD, wrong volume #86 (polyhedron-nonplanar-tests.scad reportedly producing volume 1.29 vs. real OpenSCAD's 2.94) using a live oracle: apt-installed OpenSCAD 2021.01 + a from-source Manifold v3.5.2 build (per tests/tools/README.md).
  • Bisecting the file's three polyhedron() calls individually showed CsgEvaluator::evalPolyhedron's fan-triangulation was never actually wrong — two of the three already matched the oracle to floating-point noise.
  • The real culprit was the third case: scale(0.02) polyhedron(...). scale(0.02) is a bare scalar, and at the commit the issue was filed against, CsgEvaluator::makeMatrix still had the pre-v3.10 scale(<scalar>) bug that scaled only the Z axis, distorting the mesh into a self-intersecting shape whose volume collapsed to ~0. That bug was already fixed in 18f7a54 under unrelated issue [Medium] Parser fails on several 3D/features corpus files (for-loop / transform-argument grammar edge cases) #90 — nobody re-ran the volumetric corpus check afterward to notice it also closed [Medium] Non-planar polyhedron() faces triangulate differently than OpenSCAD, wrong volume #86.
  • No change needed to CsgEvaluator::evalPolyhedron/PrimitiveGen; this file now matches the oracle at floating-point-noise level (rel_error ~1e-6) on main.
  • Added a regression test locking this in, so it can't silently regress independently of the existing scale(scalar) unit test that covers the actual root cause.
  • Documented the investigation in docs/roadmap.md (v3.11).

Test plan

  • Added tests/fixtures/headless/polyhedron_nonplanar_scale.scad (the exact reproducing case from OpenSCAD's own test corpus)
  • Added a chiselcad_tests case in tests/test_headless_build.cpp pinning its volume to the oracle's measured 1.6544281372
  • Ran the full test suite against a real CMake + Manifold v3.5.2 build: 625 test cases / all passing (up from 624 baseline)
  • Verified via scad_to_stl/stl_diff against a live OpenSCAD 2021.01 oracle: full file rel_error=3.5e-6, isolated scale(0.02) case rel_error=1.4e-6

🤖 Generated with Claude Code


Generated by Claude Code

…regression test

Bisected polyhedron-nonplanar-tests.scad's reported "2.94 vs 1.29" volume
mismatch against a live OpenSCAD 2021.01 + Manifold v3.5.2 oracle, isolating
each of its three polyhedron() calls individually. The fan-triangulation
CsgEvaluator::evalPolyhedron uses was never actually wrong here (two of the
three cases already matched to floating-point noise). The real culprit was
the third case's scale(0.02) — a bare scalar, which pre-v3.10 only scaled
the Z axis, distorting that polyhedron into a self-intersecting mesh whose
volume collapsed to ~0. That scale(<scalar>) bug was already fixed in
18f7a54 (filed under issue #90), as an incidental side effect of unrelated
parser work, so this file already matches the oracle at floating-point-noise
level with no CsgEvaluator/PrimitiveGen change needed.

Adds the exact reproducing case as a fixture plus a chiselcad_tests
regression pinning its volume to the oracle's measured value, so this
doesn't silently regress independently of the existing scale(scalar) unit
test that covers the actual root cause. Verified against the full 625-case
suite (real CMake + Manifold v3.5.2 build, not the language-only ad hoc
build prior sessions were limited to).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EpFgmMhwNWZHubDQG2doqQ

@particlesector particlesector left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. The root-cause finding checks out: CsgEvaluator's bare-scalar scale(n) broadcast (line ~547 in src/csg/CsgEvaluator.cpp, on main) does correctly expand to {n,n,n}, confirming the pre-v3.10 Z-only-scale bug is what actually caused issue #86's mismatch, not fan-triangulation. Roadmap edit is accurate and well-placed (new ## v3.11 section slots cleanly between v3.10 and v4), and the new regression test locks in the right invariant. One minor nit left inline on the test margin.


Generated by Claude Code


Generated by Claude Code

Comment thread tests/test_headless_build.cpp Outdated
Measured rel_error for this case was ~1.4e-6; the margin(1e-3) left in
place was ~1000x looser than that and could have missed a real
regression. Tightened to margin(1e-5), ~7x above the measured noise.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EpFgmMhwNWZHubDQG2doqQ
@particlesector
particlesector merged commit ba94410 into main Jul 30, 2026
6 checks passed
@particlesector
particlesector deleted the claude/issue-list-review-5as3na branch July 30, 2026 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Medium] Non-planar polyhedron() faces triangulate differently than OpenSCAD, wrong volume

2 participants