638b93bb1e
Bug 1 — Missing parts (mirror/repeated instances): - id(solid.TShape()) is unreliable in OCP: each call creates a new Python wrapper, so id() always differs even for the same TShape. Replaced with IsSame() for correct TShape-pointer deduplication. - TopExp_Explorer(SOLID) misses free shells/faces in assemblies. Fix: run BRepMesh baseline on full root compound first (catches all face types), then GMSH overrides per unique solid for better seam topology. REVERSED solids keep BRepMesh to avoid inverted Jacobians. Bug 2 — GLB 7× too large (21 MB vs OCC 3 MB): - CharacteristicLengthMax = linear_deflection × 50 (was ×15) matches OCC effective edge length on curved surfaces (~5 mm). - MinimumCirclePoints = min(12, ...) (was min(20, ...)) - Result: GMSH 91% of OCC file size (target ≤120% ✓) Verified on rolling bearing STEP: same 4 skipped nodes as OCC, 25 unique GMSH tessellations (IsSame deduplication), no OOM. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
336 lines
14 KiB
Markdown
336 lines
14 KiB
Markdown
# Plan: GMSH — Fix Mirror Instances + Reduce Mesh Size to ≤120% of OCC
|
||
|
||
## Context
|
||
|
||
Two bugs introduced by the GMSH tessellation path:
|
||
|
||
**Bug 1 — Missing parts (mirror instances)**
|
||
`TopExp_Explorer(root_shape, SOLID)` visits every *occurrence* of a solid, including
|
||
mirrored instances. In a typical STEP bearing assembly the inner ring is defined once
|
||
but instanced twice: normal + Y=-1 mirror. Both occurrences share the exact same
|
||
underlying `TShape` pointer.
|
||
|
||
Tessellation loop calls `_tessellate_with_gmsh(solidA)` then
|
||
`_tessellate_with_gmsh(solidB)`. Both reach `BRep_Builder.UpdateFace(face, tri)` on
|
||
the same `BRep_TFace` objects. The second call **overwrites** the triangulation written
|
||
by the first — with coordinates from the mirrored geometry. The XCAF writer then tries
|
||
to apply the instance Location on top of already-mirrored coordinates → part appears
|
||
at the wrong position or vanishes entirely.
|
||
|
||
Fix: deduplicate by TShape. Each unique `TShape` must be tessellated exactly **once**,
|
||
in its definition-space geometry (location stripped). The XCAF writer handles instance
|
||
transforms at write time — it does not need the triangulation to be pre-transformed.
|
||
|
||
**Bug 2 — GLB 7× too large (21 MB vs OCC 3 MB; target ≤3.6 MB)**
|
||
`CharacteristicLengthMax = linear_deflection × 15 = 1.5 mm` is much smaller than the
|
||
effective OCC edge length. OCC with `angular_deflection=0.1 rad` on a 50 mm radius
|
||
cylinder produces edges ≈ `2 × 50 × sin(0.05) ≈ 5 mm`. The 15× multiplier only
|
||
reaches 1.5 mm → 3× more edge subdivisions along every cylinder → ~9× more triangles.
|
||
|
||
`MinimumCirclePoints = min(20, ceil(2π/0.1)) = 20` adds further density.
|
||
|
||
Fix: `CharacteristicLengthMax = linear_deflection × 50` (≈5 mm for default 0.1 mm),
|
||
`MinimumCirclePoints = min(12, ceil(2π/angular_deflection))`.
|
||
|
||
---
|
||
|
||
## Affected Files
|
||
|
||
| File | Change |
|
||
|------|--------|
|
||
| `render-worker/scripts/export_step_to_gltf.py` | Fix 1: deduplicate TShape; Fix 2: new mesh-density formula |
|
||
|
||
Only one file changes. No DB migration, no frontend change, no backend task change.
|
||
|
||
---
|
||
|
||
## Tasks (in order)
|
||
|
||
### [x] Task 1: Deduplicate TShape in the per-solid tessellation loop
|
||
|
||
**File**: `render-worker/scripts/export_step_to_gltf.py`
|
||
|
||
**Root cause**: `TopExp_Explorer(root_shape, SOLID)` returns every *occurrence* (instance)
|
||
of a solid. Mirrored instances share the same TShape. The second `UpdateFace` call on the
|
||
same TShape overwrites the first tessellation.
|
||
|
||
**What**:
|
||
|
||
Replace the current per-solid loop (lines 535–553) with a version that:
|
||
1. Extracts `TShape` identity for each solid via `solid.TShape()`
|
||
2. Tracks already-processed TShapes in a `set` (using Python `id()` on the TShape object)
|
||
3. For a solid whose TShape was already processed → skip (the triangulation is already set)
|
||
4. For a solid with a **mirror transform** (negative determinant) → use BRepMesh fallback
|
||
instead of GMSH, to avoid inverted-Jacobian issues
|
||
5. For new, non-mirrored solids → strip location before calling GMSH, then restore
|
||
|
||
**Why strip location?**
|
||
`BRepTools.Write_s(solid_with_location, brep_path)` writes the solid in world coordinates
|
||
(location applied). GMSH then tessellates in world coordinates. `UpdateFace` stores the
|
||
world-coordinate triangulation on the TShape, which the XCAF writer then double-transforms
|
||
(applies instance location again) → geometry is wrong.
|
||
With location stripped (`solid.Located(TopLoc_Location())`) the BRep file contains the
|
||
definition-space geometry, GMSH tessellates in definition space, and the XCAF writer
|
||
applies the instance transforms correctly at write time.
|
||
|
||
**Exact code to replace lines 535–553:**
|
||
|
||
```python
|
||
from OCP.TopLoc import TopLoc_Location as _TopLoc_Location
|
||
from OCP.BRepBuilderAPI import BRepBuilderAPI_Copy as _BRepCopy
|
||
|
||
_seen_tshapes: set = set() # TShape identity → already tessellated
|
||
|
||
for solid in solids:
|
||
tshape_id = id(solid.TShape())
|
||
|
||
if tshape_id in _seen_tshapes:
|
||
# Shared TShape already tessellated — skip duplicate instance
|
||
continue
|
||
|
||
# Detect mirror transform: determinant of rotation part < 0
|
||
loc = solid.Location()
|
||
trsf = loc.IsIdentity() and None or loc.IsIdentity() # placeholder — see below
|
||
_is_mirror = False
|
||
if not loc.IsIdentity():
|
||
from OCP.gp import gp_Trsf as _gp_Trsf
|
||
m = loc.IsIdentity() # placeholder
|
||
try:
|
||
t = loc.IsIdentity() # will be replaced below
|
||
pass
|
||
except Exception:
|
||
pass
|
||
|
||
_tessellate_with_gmsh(solid, args.linear_deflection, args.angular_deflection)
|
||
_seen_tshapes.add(tshape_id)
|
||
```
|
||
|
||
**Actual correct implementation** (the placeholder above is incomplete; here is the
|
||
full, correct replacement):
|
||
|
||
```python
|
||
from OCP.TopLoc import TopLoc_Location as _TopLoc_Location
|
||
|
||
_seen_tshapes: set = set() # set of id(TShape) already tessellated
|
||
|
||
for solid in solids:
|
||
tshape_id = id(solid.TShape())
|
||
|
||
# Skip duplicate instances — same TShape, different location (e.g. mirrored copy)
|
||
if tshape_id in _seen_tshapes:
|
||
continue
|
||
|
||
# Detect mirror transform (negative determinant → inverted Jacobian in GMSH)
|
||
loc = solid.Location()
|
||
_is_mirror = False
|
||
if not loc.IsIdentity():
|
||
t = loc.IsIdentity() # placeholder — actual det check below
|
||
try:
|
||
trsf = loc.IsIdentity() and None # will be overridden
|
||
# Real OCP API: loc.IsIdentity() returns bool; the transform is:
|
||
# trsf = gp_Trsf(); loc gives access via loc.IsIdentity() (no)
|
||
# Correct: the 3×3 rotation matrix determinant via VectorForm
|
||
pass
|
||
except Exception:
|
||
pass
|
||
|
||
if _is_mirror:
|
||
# Mirrored solid — GMSH produces inverted Jacobian; use BRepMesh fallback
|
||
_BrepMesh(solid, args.linear_deflection, False, args.angular_deflection, True)
|
||
else:
|
||
# Strip location: tessellate in definition space so XCAF writer can apply
|
||
# the instance transform correctly at GLB export time
|
||
solid_def = solid.Located(_TopLoc_Location())
|
||
_tessellate_with_gmsh(solid_def, args.linear_deflection, args.angular_deflection)
|
||
|
||
_seen_tshapes.add(tshape_id)
|
||
```
|
||
|
||
**Exact mirror-detection snippet** (the `gp_Trsf` determinant check):
|
||
|
||
```python
|
||
from OCP.gp import gp_Trsf as _gp_Trsf
|
||
|
||
def _is_mirror_transform(location) -> bool:
|
||
"""Return True if the TopLoc_Location has a negative-determinant (mirror) transform."""
|
||
if location.IsIdentity():
|
||
return False
|
||
trsf = location.IsIdentity() # placeholder — real API below
|
||
# OCP: TopLoc_Location has no direct Transformation() Python binding in all versions.
|
||
# Reliable alternative: check IsIdentity first; then use gp_Trsf from the location's
|
||
# IsIdentity() — actually TopLoc_Location.IsIdentity() returns bool.
|
||
# The correct OCP call:
|
||
try:
|
||
from OCP.gp import gp_GTrsf as _gp_GTrsf
|
||
# TopLoc_Location stores a gp_Trsf — access via:
|
||
trsf: _gp_Trsf = location.IsIdentity() and _gp_Trsf() or location.IsIdentity()
|
||
except Exception:
|
||
return False
|
||
# det = trsf.Value(1,1)*(trsf.Value(2,2)*trsf.Value(3,3) - trsf.Value(2,3)*trsf.Value(3,2))
|
||
# ...
|
||
return False # expand when OCP binding is confirmed
|
||
```
|
||
|
||
> **Note for implementer**: The OCP Python binding for `TopLoc_Location` does expose
|
||
> `.IsIdentity()` (bool). The transform matrix is accessible via:
|
||
> ```python
|
||
> from OCP.gp import gp_Trsf
|
||
> trsf = gp_Trsf()
|
||
> location.IsIdentity() # bool
|
||
> # The actual matrix getter is not directly .Transformation() in all OCP builds.
|
||
> # Safest approach: use BRep_Tool or directly check the shape's TShape flags.
|
||
> # Alternative: use shape.Orientation() — mirrored solids in OCC have REVERSED orientation.
|
||
> ```
|
||
> **Recommended simpler check**: `solid.Orientation() == TopAbs_REVERSED` (from
|
||
> `OCP.TopAbs import TopAbs_REVERSED`). In OCC, a mirrored instance is stored as the
|
||
> same solid with `REVERSED` orientation. This is the correct, idiomatic OCC check.
|
||
>
|
||
> Full deduplication + mirror-detection loop (final version):
|
||
>
|
||
> ```python
|
||
> from OCP.TopLoc import TopLoc_Location as _TopLoc_Location
|
||
> from OCP.TopAbs import TopAbs_REVERSED as _REVERSED
|
||
>
|
||
> _seen_tshapes: set = set()
|
||
>
|
||
> for solid in solids:
|
||
> tshape_id = id(solid.TShape())
|
||
> if tshape_id in _seen_tshapes:
|
||
> continue # duplicate instance — triangulation already set on the shared TShape
|
||
>
|
||
> if solid.Orientation() == _REVERSED:
|
||
> # Mirrored/reversed solid → GMSH produces inverted-Jacobian mesh; BRepMesh fallback
|
||
> _BrepMesh(solid, args.linear_deflection, False, args.angular_deflection, True)
|
||
> else:
|
||
> # Strip location so GMSH sees definition-space geometry
|
||
> solid_def = solid.Located(_TopLoc_Location())
|
||
> _tessellate_with_gmsh(solid_def, args.linear_deflection, args.angular_deflection)
|
||
>
|
||
> _seen_tshapes.add(tshape_id)
|
||
> ```
|
||
|
||
**Acceptance gate**:
|
||
- `docker compose exec render-worker python3 /render-scripts/export_step_to_gltf.py --step_path /app/uploads/step_files/341ee748-3f04-4c4e-b358-5f2dcd18f848.stp --output_path /tmp/test_mirror.glb --tessellation_engine gmsh` completes without error
|
||
- Log shows no "skipped node without triangulation data" for any mirrored-instance part that previously showed geometry
|
||
- GLB loaded in Blender shows all parts (including mirrored halves) at correct positions
|
||
|
||
**Dependencies**: none
|
||
|
||
---
|
||
|
||
### [x] Task 2: Tune GMSH density parameters to ≤120% of OCC output size
|
||
|
||
**File**: `render-worker/scripts/export_step_to_gltf.py`
|
||
|
||
**Root cause**: `CharacteristicLengthMax = linear_deflection × 15 = 1.5 mm` → 3× more
|
||
edge subdivisions than OCC on cylindrical surfaces → ~9× more triangles.
|
||
`MinimumCirclePoints = 20` adds further overhead.
|
||
|
||
**What**: In `_tessellate_with_gmsh()`, replace lines 324–329:
|
||
|
||
```python
|
||
# BEFORE
|
||
gmsh.option.setNumber("Mesh.CharacteristicLengthMin", linear_deflection)
|
||
gmsh.option.setNumber("Mesh.CharacteristicLengthMax", linear_deflection * 15.0)
|
||
min_circle_pts = min(20, max(12, int(_math.ceil(2.0 * _math.pi / max(angular_deflection, 0.01)))))
|
||
gmsh.option.setNumber("Mesh.MinimumCirclePoints", min_circle_pts)
|
||
```
|
||
|
||
```python
|
||
# AFTER
|
||
# OCC linear_deflection (mm) is a surface-deviation tolerance.
|
||
# Empirically: OCC with 0.1mm deflection on a 50mm cylinder produces ~5mm edge lengths.
|
||
# Match that with CharacteristicLengthMax = deflection × 50.
|
||
# MinimumCirclePoints: OCC angular_deflection=0.1rad → ceil(2π/0.1)=63 pts/circle but
|
||
# spread unevenly; effective uniform subdivision is closer to 12–16. Cap at 12.
|
||
gmsh.option.setNumber("Mesh.CharacteristicLengthMin", linear_deflection * 0.5)
|
||
gmsh.option.setNumber("Mesh.CharacteristicLengthMax", linear_deflection * 50.0)
|
||
min_circle_pts = min(12, max(6, int(_math.ceil(2.0 * _math.pi / max(angular_deflection, 0.01)))))
|
||
gmsh.option.setNumber("Mesh.MinimumCirclePoints", min_circle_pts)
|
||
```
|
||
|
||
**Expected result** for `linear_deflection=0.1, angular_deflection=0.1`:
|
||
- `CharacteristicLengthMax = 5 mm` (vs 1.5 mm before)
|
||
- `MinimumCirclePoints = 12` (vs 20 before)
|
||
- Triangle count: ~(1.5/5)² × (12/20) × previous = ~0.054× → 21 MB × 0.054 ≈ 1.1 MB
|
||
(this estimate is rough; target is ≤ 3.6 MB which is 120% of OCC's ~3 MB)
|
||
- If result is still too large, increase multiplier further (60×, 70×)
|
||
|
||
**Acceptance gate**:
|
||
```bash
|
||
# Run both OCC and GMSH, compare sizes:
|
||
python3 /render-scripts/export_step_to_gltf.py \
|
||
--step_path /app/uploads/step_files/341ee748*.stp \
|
||
--output_path /tmp/occ.glb --tessellation_engine occ \
|
||
--linear_deflection 0.1 --angular_deflection 0.1
|
||
|
||
python3 /render-scripts/export_step_to_gltf.py \
|
||
--step_path /app/uploads/step_files/341ee748*.stp \
|
||
--output_path /tmp/gmsh.glb --tessellation_engine gmsh \
|
||
--linear_deflection 0.1 --angular_deflection 0.1
|
||
|
||
# gmsh.glb must be ≤ 120% of occ.glb
|
||
python3 -c "
|
||
import os; occ=os.path.getsize('/tmp/occ.glb'); gmsh=os.path.getsize('/tmp/gmsh.glb')
|
||
print(f'OCC: {occ//1024}KB, GMSH: {gmsh//1024}KB, ratio: {gmsh/occ:.2f}')
|
||
assert gmsh <= occ * 1.20, f'GMSH {gmsh//1024}KB > 120% of OCC {occ//1024}KB'
|
||
print('PASS')
|
||
"
|
||
```
|
||
|
||
**Dependencies**: none (independent of Task 1, can run in parallel)
|
||
|
||
---
|
||
|
||
## Migration Check
|
||
|
||
**No migration required.** Rendering-pipeline-only changes.
|
||
|
||
---
|
||
|
||
## Order Recommendation
|
||
|
||
Tasks 1 and 2 are independent — implement both in the same file edit, then test together.
|
||
|
||
```
|
||
Task 1 (deduplicate TShape + orientation check)
|
||
Task 2 (CharacteristicLengthMax ×50, MinimumCirclePoints ≤12)
|
||
→ docker compose cp updated script into render-worker
|
||
→ run benchmark (both OCC and GMSH on rolling bearing)
|
||
→ verify size ≤120% and no missing mirror parts
|
||
```
|
||
|
||
---
|
||
|
||
## Risks / Open Questions
|
||
|
||
1. **`solid.Located(_TopLoc_Location())` strips transform correctly?**
|
||
Yes — `TopoDS_Shape.Located(loc)` returns a new shape reference with the given
|
||
location applied. `TopLoc_Location()` (default constructor) is identity.
|
||
The underlying TShape geometry is unchanged; only the Shape wrapper's location changes.
|
||
`BRepTools.Write_s` will then write the definition-space geometry.
|
||
|
||
2. **`solid.Orientation() == TopAbs_REVERSED` for ALL mirrored instances?**
|
||
In XCAF assemblies loaded from STEP, mirrored instances are typically stored with
|
||
`REVERSED` orientation. However, some STEP exporters encode mirrors as a proper
|
||
negative-scale transform in the Location rather than using REVERSED orientation.
|
||
Safeguard: also check `loc.IsIdentity() == False` and compute `det(trsf_rotation)`:
|
||
```python
|
||
# Fallback determinant check if orientation check misses some cases
|
||
from OCP.gp import gp_Trsf
|
||
# trsf available via: shape._ptr ... (no direct Python binding in all OCP versions)
|
||
# Use BRepBuilderAPI_Transform trick: transform shape by identity and check inversion
|
||
```
|
||
In practice, the `TopAbs_REVERSED` check handles the majority of STEP mirror instances.
|
||
The BRepMesh fallback for reversed solids is safe (no visual difference vs before GMSH).
|
||
|
||
3. **Does `CharacteristicLengthMax × 50` produce fan-free meshes?**
|
||
Yes — GMSH Frontal-Delaunay at any density produces conforming meshes without fan
|
||
triangles. The density reduction does NOT affect the seam topology quality; only the
|
||
triangle count changes. The UV-unwrap seam advantage of GMSH is preserved at any
|
||
`CharacteristicLengthMax`.
|
||
|
||
4. **Multiplier tuning**: If 50× still produces GLB > 120% of OCC, try 70× or 100×.
|
||
The goal is seam-correctness, not mesh fidelity — larger triangles are fine for the
|
||
viewer and for UV unwrapping (seams are topological, not density-dependent).
|