Add straight skeleton extrusion (extrude_skeleton) for roof generation - #74
Conversation
Wrap CGAL's extrude_skeleton so a 2D polygon (optionally with holes) becomes a closed 3D roof mesh. extrude() returns a ready-to-display triangulated mesh plus the roof outline (eaves, hips, ridges). Includes example, screenshot, and tests.
540c99b to
0750087
Compare
|
@jf--- could you review the pr? |
|
would have expected this to be trivial but its much more involved that I'd initially expect. all for it. |
jf---
left a comment
There was a problem hiding this comment.
Reviewed the C++ binding, Python wrapper, tests, and docs. This is clean and correct — CGAL usage checks out, the build is wired, and the API fits the module. No blockers; one item worth fixing before merge, the rest are follow-ups.
Worth fixing before merge
Silent maximum_height <= 0 → unbounded roof. extrude() maps maximum_height=None → -1.0 (straight_skeleton_2.py:453) and the binding computes bounded = maximum_height > 0.0 (straight_skeleton_2.cpp:331). So maximum_height=0.0 (or any negative) silently yields an unbounded roof instead of erroring, and -1.0 doubles as both the None-sentinel and a plausible-looking user value. The C++ docstring documents "Values <= 0 mean unbounded" but the Python extrude() docstring (:393) doesn't. Suggest validating maximum_height > 0 in extrude() and raising, rather than overloading a numeric sentinel.
Follow-ups (non-blocking)
- Coplanarity tolerance duplicated across the language boundary. C++ merges patches at
cosine_of_maximum_angle(0.9998)(straight_skeleton_2.cpp:372, ~1.15°); Python filters roof edges atangle_tol=1.0degree (straight_skeleton_2.py:322/:328, ~0.99985). Same near-coplanar threshold as two unlinked magic numbers in two files, and the Python default has no rationale comment. Worth naming it and noting the two are intentionally coupled so they don't drift. - Test gaps (
test_straight_skeleton_2_extrude.py): theweights=path is only tested alongsideangles=to assert exclusivity — nothing drivesweightsalone;_contour_speeds' per-edge / per-contour branches and both itsValueErrorpaths are uncovered; theRuntimeErrorpath (angles ≥ 90 withoutmaximum_height) is untested; outline assertions arelen(...) > 0only. - Angle range not validated. CGAL requires angles strictly within (0, 180); out-of-range values fall through to the generic
"Failed to extrude straight skeleton"RuntimeError (straight_skeleton_2.cpp:348/:353), whose text doesn't mention the range. A range-check inextrude()(or the range in the message) saves a debugging session.
Nits
example_straight_skeleton_2_extrude.mdprose says taperangle(singular); the parameter isangles.- Footprint z-coordinates are dropped (the roof is built on the z=0 plane) — worth one docstring line.
- The
_roof_outlinedocstring uses an rST::literal block; a fenced ```python block renders better under this repo's mkdocstrings.
What's good
- CGAL usage is correct: angles passed in degrees, CCW-boundary / CW-holes orientation auto-fix, and
triangulate_facesbeforeremesh_planar_patches(remesh needs a triangle mesh). - Build wiring is in place:
m.def("extrude_straight_skeleton", …)insideNB_MODULE(_straight_skeleton_2), compiled by CMake, with<nanobind/stl/vector.h>added for thevector<vector<int>>marshaling. - Ear-clipping n-gon roof faces (instead of centroid-fanning) so non-convex faces stay closed is a nice touch.
- API is consistent with the module (polygon-in, high-level-object-out), and the
ValueErrormodel matches the sibling functions.
Happy to re-look once the maximum_height guard is in.
Why this PR:
I received a request from a Rhino user:
Summary
Adds a wrapper around CGAL's
extrude_skeleton(CGAL 6.x 2D Straight Skeleton and Polygon Offsetting: Skeleton Extrusion), so a 2D polygon — a building footprint — can be turned into a closed 3D roof mesh with polygon faces. This directly answers the request to bringextrude_skeletonto COMPAS for roof modelling (e.g. modelling suburbs).Based on the CGAL
extrude_skeletonexample and the improved straight skeleton write-up.Screenshot
Usage
Testing
python -m pytest tests/test_straight_skeleton_2_extrude.py— 7 passing; existing straight-skeleton tests still pass.ruff format/ruff checkclean.