fix: bound runaway faces in streaming STEP tessellation#217
Merged
Conversation
A cylinder/cone face whose boundary circles are split into arcs crossing the parametric seam kept the surface's natural (infinite, or absurdly large) bounds after BRepBuilderAPI_MakeFace(surface, wire). BRepMesh then emitted vertices millions of units out, so a handful of such faces exploded the converted model's bounding box (assembly-placed STEP read via the streaming reader; the placement transforms themselves were verified correct). make_face_from_geom now detects the runaway by comparing the built face's geometric extent against its own boundary wire (a trimmed face cannot legitimately overrun its boundary 10x) and rebuilds the face from the boundary's projected parameter extent (arc-aware sampling); if that fails too the face is dropped — a hole beats a kilometres-long sliver. The check is gated to cylinder/cone faces, the only surfaces here with an infinite direction. ShapeFix_Face was deliberately NOT used as the repair: it segfaults on many real-world unbounded cone faces. Those segfaults also exposed a pool blind spot: a tessellation worker that dies mid-solid (uncatchable native crash) produced no result and its slot sat blocked for the full 120 s per-solid timeout — hundreds of such solids turned an 18-minute conversion into 90+ minutes. The pool loop now checks worker liveness each poll and replaces a dead worker immediately, recording the solid as a native-crash skip. Regression test constructs the seam-crossing arc wire from raw geom objects and asserts the meshed face stays inside the cylinder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🚀 Profiling Results (Top 20 most expensive calls)
📈 Performance Impact per Commit
|
Completes the ada files trio (list/download/upload): upload a local file to any scope the token can address, presign-first so large files go S3-direct without pinning an API worker (then POST upload-complete for the audit row + auto-conversion enqueue), falling back to the API-tunneled PUT on local-storage deployments. Used in anger to push an 800 MB GLB to a personal scope; previously only the ada-build artefact flow could upload at all, and only to project scopes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Added a second commit: |
The end-to-end assertion now goes through active_backend() build + tessellate, so the adacpp integration leg runs it too (verified passing there) instead of failing on a module-level OCC import. Only the bug-trigger precondition and the triangle-soup filter unit test remain pythonocc-gated — they exercise pythonocc internals that adacpp replaces wholesale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Krande
force-pushed
the
fix/unbounded-analytic-face
branch
from
3e3a72f to
af3f128
Compare
Two viewer-facing gaps in the streaming STEP->GLB path: 1. Units. glTF mandates metres but STEP files are very often authored in millimetres — the OCC reader converts via xstep.cascade.unit, while the kernel-free stream path emitted raw file units. A mm model rendered 1000x too big and kilometres off-centre, and the viewer's depth precision collapsed into z-fighting that read as broken geometry. detect_step_length_unit_scale() reads the LENGTH_UNIT record (SI prefixes, one- and two-arg SI_UNIT forms, and CONVERSION_BASED_UNIT names like MILLIMETRE/INCH) and the scene path scales positions once, after instance placement. 2. Hierarchy. Every instance node hung flat under root, so a big assembly was an unfoldable list of thousands of solids. The transform walk now carries each instance's placement chain as (rep_id, product_name) levels (products resolved via SHAPE_DEFINITION_REPRESENTATION), and the scene graph nests instances under lazily-created group nodes keyed by rep-id prefix — the GLB id_hierarchy mirrors the STEP product tree, so good/bad regions can be folded and inspected per sub-assembly. Also fixes an evaluation-order bug the hierarchy work exposed: the leaf node id was computed before the group chain existed, so the first group node silently evicted the leaf's id from the graph (cyclic-looking id_hierarchy). Both backends green (115 pythonocc / 112+3-skip adacpp). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Two more commits after viewer inspection of the converted model:
Verified on the reference model: extent ±46 m centered at origin, 0 out-of-extent nodes at a 60 m threshold, 24k-node hierarchy up to 10 levels deep. Both backends green. |
Loading a multi-hundred-MB GLB spiked past 5 GB of browser memory and took minutes. Four compounding causes, all on the load path: 1. CustomBatchedMesh cloned the entire source geometry (the original mesh is discarded right after), permanently doubling every vertex/index buffer. It now shares the source geometry. 2. The GLTFLoader parser (including the raw GLB body ArrayBuffer) was stashed on userData for lineage registration and never released. It is now dropped as soon as lineage finishes. 3. buildEdgeGeometryWithRangeIds built a THREE sub-geometry per draw range (Array.from boxing every index into JS numbers, a full EdgesGeometry per range, then mergeGeometries over tens of thousands of geometries). Rewritten as a single typed-array pass with the same EdgesGeometry semantics: per-range position welding (the streamed GLB is a triangle soup), boundary edges always, shared edges past the dihedral threshold, per-vertex rangeId. Above 5M indices the 30-degree feature-edge threshold is forced — at the 1-degree default a curved 30M-tri model emits a line pair for nearly every triangle edge (multi-GB overlay). 4. The GPU picker only used its layout cost model when the flat-picker toggle was on; it now always auto-picks the cheaper layout (~0.5 GB on a 30M-tri merged CAD mesh) and the toggle becomes a force-flat override. Frontend typecheck clean; 32 unit tests pass incl. a new edge-builder test (welded soup, crease vs coplanar suppression, rangeId attribution). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👋 Hi there! I have checked your PR and found no issues. Thanks for your contribution! PR Review:I found no pr-related issues.
Python Review:I found no python-related issues. Python Linting results:
Python Packaging results:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Converting a large assembly-placed STEP model produced an "exploded" GLB: the model spanned hundreds of kilometres and sat far off-center in the viewer, even though the assembly-placement transforms (#213) were verified correct.
Root causes
BRepBuilderAPI_MakeFace(surface, wire). BRepMesh then emits vertices millions of units out — a single such face explodes the whole model's bounding box.Fixes
make_face_from_geom: detect a runaway by comparing the built face's geometric extent against its own boundary wire (a trimmed face cannot legitimately overrun its boundary 10x); rebuild from the boundary's projected parameter extent (arc-aware sampling); drop the face if that fails too. Gated to cylinder/cone faces — the only surfaces here with an infinite direction.ShapeFix_Faceis deliberately not used as the repair: it segfaults on many real-world unbounded cone faces.tessellate_shape: safety net for everything else — drop any triangle with a vertex outside the shape's edge hull inflated 10x (edges stay finite even when a face's trim fails; sphere/torus surface extents are included for edge-poor faces).error:WorkerCrashed) instead of burning the per-solid timeout.Results on the reference model (~7k solids, 779 MB STEP)
Existing STEP/geom/visual-parity suites green (115 tests); two new regression tests (synthetic seam-crossing arc wire reproduces the unbounded-face precondition on stock OCC; triangle-soup filter unit test).
🤖 Generated with Claude Code