diff --git a/src/ada/occ/geom/curves.py b/src/ada/occ/geom/curves.py index 5a7d49a55..4a2d02612 100644 --- a/src/ada/occ/geom/curves.py +++ b/src/ada/occ/geom/curves.py @@ -67,13 +67,27 @@ def _points_equal(a, b, tol=1e-6): p2 = point3d(edge.end) edge_maker = BRepBuilderAPI_MakeEdge(p1, p2) elif isinstance(curve_geom, geo_cu.Circle): - # Use circle geometry - circle_origin = gp_Ax2(gp_Pnt(*curve_geom.position.location), gp_Dir(*curve_geom.position.axis)) + # Use circle geometry. The OCC point-trim overload always walks the + # circle's POSITIVE parametric direction from P1 to P2, so the + # occupied arc must be expressed in that form: the EdgeCurve's own + # endpoints traversed positively iff ``same_sense``; for a + # reversed-sense arc, reversing the circle's axis flips its + # parametric direction so the same overload picks the correct + # (complementary) arc. Using the OrientedEdge's endpoints here was + # wrong twice over — readers differ on whether they pre-swap them, + # and the arc point-set never depends on traversal orientation. + axis_dir = gp_Dir(*curve_geom.position.axis) + ec_same_sense = bool(getattr(edge_element, "same_sense", True)) + if not ec_same_sense: + axis_dir = axis_dir.Reversed() + circle_origin = gp_Ax2(gp_Pnt(*curve_geom.position.location), axis_dir) circle = gp_Circ(circle_origin, curve_geom.radius) + arc_start = getattr(edge_element, "start", edge.start) + arc_end = getattr(edge_element, "end", edge.end) # If start and end are equal (full circle), create a full circle edge. # Otherwise create an arc between start and end. - if _points_equal(edge.start, edge.end): + if _points_equal(arc_start, arc_end): edge_maker = BRepBuilderAPI_MakeEdge(circle) else: # Prefer the SAT-recorded parametric trim values @@ -87,9 +101,14 @@ def _points_equal(a, b, tol=1e-6): t_start = getattr(edge, "t_start", None) t_end = getattr(edge, "t_end", None) if t_start is not None and t_end is not None: - edge_maker = BRepBuilderAPI_MakeEdge(circle, float(t_start), float(t_end)) + # SAT params are canonical w.r.t. the UNREVERSED curve. + circle_fwd = gp_Circ( + gp_Ax2(gp_Pnt(*curve_geom.position.location), gp_Dir(*curve_geom.position.axis)), + curve_geom.radius, + ) + edge_maker = BRepBuilderAPI_MakeEdge(circle_fwd, float(t_start), float(t_end)) else: - edge_maker = BRepBuilderAPI_MakeEdge(circle, point3d(edge.start), point3d(edge.end)) + edge_maker = BRepBuilderAPI_MakeEdge(circle, point3d(arc_start), point3d(arc_end)) elif isinstance(curve_geom, (geo_cu.BSplineCurveWithKnots, geo_cu.RationalBSplineCurveWithKnots)): # Build an OCC BSpline curve from the adapy representation try: diff --git a/src/ada/occ/geom/surfaces.py b/src/ada/occ/geom/surfaces.py index fb2d4ebad..6a56fe510 100644 --- a/src/ada/occ/geom/surfaces.py +++ b/src/ada/occ/geom/surfaces.py @@ -1,4 +1,5 @@ import math +from collections import Counter from OCC.Core.Bnd import Bnd_Box from OCC.Core.BRep import BRep_Builder, BRep_Tool @@ -790,9 +791,16 @@ def _sample_edge_points(oe): if _points_close(oe.start, oe.end): a0, a1 = 0.0, 2.0 * math.pi else: - # Partial arc: sample between the endpoint angles. The traversal sense - # isn't recovered here, so the complement arc may be sampled instead — - # harmless for parameter-extent estimation (it can only widen coverage). + # Partial arc: sample between the endpoint angles ALONG THE OCCUPIED ARC. + # The arc's point-set is fully determined by the EdgeCurve alone: from + # start to end going in the circle's positive parametric direction when + # ``same_sense``, negative otherwise. Crucially this ignores + # ``OrientedEdge.orientation`` — readers differ on whether they pre-swap + # the oriented edge's endpoints (the STEP stream reader does, the SAT + # converter doesn't), but every producer authors EdgeCurve(start, end, + # same_sense) with the same semantics. Sampling the complement arc here + # widened the parameter extent to the full period, so a small wedge face + # rebuilt as a spurious full cylinder/cone band. def _ang(p): d = (p[0] - c[0], p[1] - c[1], p[2] - c[2]) return math.atan2( @@ -800,9 +808,16 @@ def _ang(p): d[0] * xd[0] + d[1] * xd[1] + d[2] * xd[2], ) - a0, a1 = _ang(pts[0]), _ang(pts[1]) - if a1 <= a0: - a1 += 2.0 * math.pi + same_sense = bool(getattr(ec, "same_sense", True)) + ec_start = getattr(ec, "start", oe.start) + ec_end = getattr(ec, "end", oe.end) + a0, a1 = _ang(ec_start), _ang(ec_end) + if same_sense: + if a1 <= a0: + a1 += 2.0 * math.pi + else: + if a1 >= a0: + a1 -= 2.0 * math.pi for k in range(13): a = a0 + (a1 - a0) * k / 12.0 ca, sa = math.cos(a), math.sin(a) @@ -838,15 +853,77 @@ def _param_extent(vals: list[float], periodic: bool, period: float) -> tuple[flo return s[best_i + 1], s[best_i] + period +# Fidelity accounting for the param-extent repair paths. Consumed (return-and-reset) +# by the streaming conversion so its summary can quantify how much geometry was +# rebuilt/approximated/dropped — see consume_param_rebuild_stats(). +PARAM_REBUILD_STATS: Counter = Counter() + + +def consume_param_rebuild_stats() -> dict[str, int]: + """Return and reset the per-process rebuild counters (picked up after each solid + by the streaming tessellation worker).""" + out = dict(PARAM_REBUILD_STATS) + PARAM_REBUILD_STATS.clear() + return out + + +# A rebuilt face whose area exceeds this multiple of (boundary-sample bbox diagonal)^2 +# is over-covering its own boundary evidence and gets dropped instead. Legitimate +# closed revolution faces stay far below: a full cylinder's worst area/diag^2 ratio is +# ~1.11 (at h = 2*sqrt(2)*r), a torus tube ~1.23 — 4.0 leaves >3x margin, while the +# failure mode (full-period band rebuilt from a small wedge's evidence) overshoots by +# ~period/arc_span. +_REBUILD_AREA_GATE = 4.0 + + +def _is_closure_bound(fb, face_surface) -> bool: + """True when every edge of the bound is a full circle concentric with the + revolution surface's axis — the bound that *defines* a closed face's extent + (e.g. a full cylinder's top/bottom rims, which often arrive as two separate + bounds). Such a bound is no hole and must never be re-added as an inner wire.""" + pos = getattr(face_surface, "Position", None) + if pos is None: + return False + ax = pos().Axis() + a_loc, a_dir = ax.Location(), ax.Direction() + edges = getattr(getattr(fb, "bound", None), "edge_list", None) or [] + if not edges: + return False + for oe in edges: + ec = getattr(oe, "edge_element", oe) + g = getattr(ec, "edge_geometry", None) + if not isinstance(g, geo_cu.Circle) or not _points_close(oe.start, oe.end): + return False + c = [float(x) for x in g.position.location] + z = [float(x) for x in g.position.axis] + r = float(g.radius) + # circle axis parallel to the surface axis + dot = abs(z[0] * a_dir.X() + z[1] * a_dir.Y() + z[2] * a_dir.Z()) + zlen = math.sqrt(z[0] ** 2 + z[1] ** 2 + z[2] ** 2) or 1.0 + if dot / zlen < 1.0 - 1e-6: + return False + # circle centre on the surface axis (within tol*r) + dx = c[0] - a_loc.X() + dy = c[1] - a_loc.Y() + dz = c[2] - a_loc.Z() + along = dx * a_dir.X() + dy * a_dir.Y() + dz * a_dir.Z() + off2 = dx * dx + dy * dy + dz * dz - along * along + if off2 > (1e-4 * max(r, 1.0)) ** 2: + return False + return True + + def _make_face_from_param_extent(advanced_face: geo_su.AdvancedFace, face_surface): """Build a face directly from the projected parameter extent of its boundary samples — for faces whose boundary wire cannot trim the surface (closed - revolution faces, seam-crossing arc wires). Inner bounds are filled (the face - over-covers slightly). Returns a face or None.""" + revolution faces, seam-crossing arc wires). Hole bounds are re-added when their + wires build; a rebuild whose area overruns its own boundary evidence is rejected + (None) — a dropped face beats a spurious full cone. Returns a face or None.""" # Recover (u, v) parameter ranges by projecting the boundary samples onto the # surface; a closed direction snaps to the full period via _param_extent. us: list[float] = [] vs: list[float] = [] + sample_pts: list[tuple] = [] for fb in advanced_face.bounds: for oe in getattr(getattr(fb, "bound", None), "edge_list", []): for p in _sample_edge_points(oe): @@ -855,6 +932,7 @@ def _make_face_from_param_extent(advanced_face: geo_su.AdvancedFace, face_surfac u, v = proj.LowerDistanceParameters() us.append(u) vs.append(v) + sample_pts.append(p) if len(us) < 3: return None @@ -867,7 +945,69 @@ def _make_face_from_param_extent(advanced_face: geo_su.AdvancedFace, face_surfac mk = BRepBuilderAPI_MakeFace(face_surface, umin, umax, vmin, vmax, 1e-6) if not mk.IsDone(): return None - return mk.Face() + face = mk.Face() + + # Area sanity gate: the boundary samples are world points ON the face's rim, so + # the face cannot legitimately dwarf their bounding box. Catching it here (rather + # than emitting) is what turns "spurious giant cone obfuscating the model" into a + # small hole. + lo = [min(p[i] for p in sample_pts) for i in range(3)] + hi = [max(p[i] for p in sample_pts) for i in range(3)] + diag2 = (hi[0] - lo[0]) ** 2 + (hi[1] - lo[1]) ** 2 + (hi[2] - lo[2]) ** 2 + area = _face_area(face) + if area > _REBUILD_AREA_GATE * max(diag2, 1e-12): + PARAM_REBUILD_STATS["area_gate_dropped"] += 1 + logger.debug( + "param-extent rebuild over-covers boundary evidence (area %.3g > %.0fx diag^2 %.3g); dropping face", + area, + _REBUILD_AREA_GATE, + diag2, + ) + return None + + # Re-add hole bounds. ``bounds[0]`` is the outer boundary (the same assumption + # the normal wire path makes) and is already represented by the parameter + # extent — punching it as a hole would consume the face. Of the rest, a closure + # bound (full concentric circles — e.g. a closed cylinder's second rim) defines + # the extent and is NOT a hole; anything else is a cutout the plain param-range + # face would otherwise fill. Each hole must shrink the area — a wire that + # doesn't (wrong orientation / off-surface) is skipped rather than risked. + holes = [fb for fb in advanced_face.bounds[1:] if not _is_closure_bound(fb, face_surface)] + for fb in holes: + try: + wire = make_wire_from_face_bound(fb) + except Exception as ex: # noqa: BLE001 - holes are best-effort fidelity + logger.debug("param-extent rebuild: inner bound wire failed (%s)", ex) + PARAM_REBUILD_STATS["inner_bound_dropped"] += 1 + continue + # A hole wire must wind opposite to the outer boundary; source files (and + # make_wire_from_face_bound) don't guarantee which sense arrives, so try + # both and keep whichever SHRINKS the area without zeroing it. The added + # wire has no pcurves on this parametric face, so ShapeFix_Face computes + # them (safe here: the face is BOUNDED — the known ShapeFix segfaults are + # on faces still carrying infinite natural bounds). + added = False + for candidate in (wire, wire.Reversed()): + try: + mk2 = BRepBuilderAPI_MakeFace(face) + mk2.Add(candidate) + if not mk2.IsDone(): + continue + fixer = ShapeFix_Face(mk2.Face()) + fixer.Perform() + fixed = fixer.Face() + if 1e-12 < _face_area(fixed) < area: + face = fixed + area = _face_area(face) + PARAM_REBUILD_STATS["inner_bound_readded"] += 1 + added = True + break + except Exception as ex: # noqa: BLE001 + logger.debug("param-extent rebuild: inner bound add failed (%s)", ex) + if not added: + PARAM_REBUILD_STATS["inner_bound_dropped"] += 1 + + return face def _try_make_closed_revolution_face(advanced_face: geo_su.AdvancedFace, face_surface): @@ -881,7 +1021,10 @@ def _try_make_closed_revolution_face(advanced_face: geo_su.AdvancedFace, face_su return None if not _has_full_circle_edge(advanced_face): return None - return _make_face_from_param_extent(advanced_face, face_surface) + face = _make_face_from_param_extent(advanced_face, face_surface) + if face is not None: + PARAM_REBUILD_STATS["closed_revolution_rebuilt"] += 1 + return face _UV_FINITE_LIM = 1.0e50 # OCC marks an untrimmed natural bound as ±Precision::Infinite (~1e100) @@ -1118,6 +1261,7 @@ def _env_truthy(name: str, default: bool) -> bool: raise UnableToCreateTesselationFromSolidOCCGeom( f"unbounded face after wire trim on {type(advanced_face.face_surface).__name__}; skipping" ) + PARAM_REBUILD_STATS["unbounded_rebuilt"] += 1 face = rebuilt # ShapeFix runs only on the regenerative-pcurve path — the diff --git a/src/ada/visit/scene_handling/scene_from_step_stream.py b/src/ada/visit/scene_handling/scene_from_step_stream.py index c45b96780..373be764a 100644 --- a/src/ada/visit/scene_handling/scene_from_step_stream.py +++ b/src/ada/visit/scene_handling/scene_from_step_stream.py @@ -69,14 +69,28 @@ def _maybe_trim() -> None: pass +def _rebuild_stats(): + """This solid's face-repair counters from the OCC backend (param-extent rebuilds, + re-added/dropped holes, area-gate drops), or None on backends without the module + (adacpp-only installs have no ada.occ).""" + try: + from ada.occ.geom.surfaces import consume_param_rebuild_stats + except ImportError: + return None + stats = consume_param_rebuild_stats() + return stats or None + + def _tessellate_geom_worker(geom): """Pool subprocess entry: build + tessellate ONE solid and return raw mesh arrays plus the solid's colour — the parent assigns a consistent material id (each subprocess has its own material store). Returns - ``(status, gid, color, positions, indices, normals, transforms, paths)`` where - ``status`` is "ok", "degenerate", "empty" or "error:"; ``transforms`` is the - solid's list of world-placement matrices (or None) — the parent meshes once and - places per matrix — and ``paths`` the matching per-instance assembly paths. + ``(status, gid, color, positions, indices, normals, transforms, paths, rebuilds)`` + where ``status`` is "ok", "degenerate", "empty" or "error:"; ``transforms`` is + the solid's list of world-placement matrices (or None) — the parent meshes once and + places per matrix — ``paths`` the matching per-instance assembly paths, and + ``rebuilds`` the per-solid face-repair counters (param-extent rebuilds, dropped + holes, area-gate drops) so the conversion summary can quantify fidelity loss. OCC isn't thread-safe, so the unit of parallelism is a process.""" import numpy as np @@ -104,14 +118,14 @@ def _tessellate_geom_worker(geom): except Exception: diag = 0.0 if diag < 1e-7: - return ("degenerate", gid, geom.color, None, None, None, None, None) + return ("degenerate", gid, geom.color, None, None, None, None, None, _rebuild_stats()) mesh = be.tessellate(occ) idx = getattr(mesh, "indices", None) if idx is None: idx = getattr(mesh, "faces", None) pos = getattr(mesh, "positions", None) if pos is None or idx is None or len(idx) == 0: - return ("empty", gid, geom.color, None, None, None, None, None) + return ("empty", gid, geom.color, None, None, None, None, None, _rebuild_stats()) nrm = getattr(mesh, "normals", None) # ascontiguousarray(+dtype) COPIES, so pos/idx/nrm no longer reference any OCC # buffer — the shape + its triangulation can be dropped immediately below. @@ -121,9 +135,9 @@ def _tessellate_geom_worker(geom): # The shell is tessellated ONCE in its local frame. The world-placement matrices # (one per STEP assembly instance) ride back to the parent, which applies each to # this single local mesh — so a part instanced N times still meshes once. - return ("ok", gid, geom.color, pos, idx, nrm, geom.transforms, geom.instance_paths) + return ("ok", gid, geom.color, pos, idx, nrm, geom.transforms, geom.instance_paths, _rebuild_stats()) except Exception as exc: # noqa: BLE001 - report and skip; one bad solid mustn't abort - return (f"error:{type(exc).__name__}", gid, None, None, None, None, None, None) + return (f"error:{type(exc).__name__}", gid, None, None, None, None, None, None, _rebuild_stats()) finally: # Purge this task's OCC instances now (refcount-0 frees the native shape + # triangulation) rather than relying on end-of-call scope cleanup — keeps a @@ -255,6 +269,7 @@ def _tessellate_stream(source: StepStreamSource, graph, bt, sink) -> dict: on_progress = source.on_progress reasons: collections.Counter = collections.Counter() + rebuild_totals: collections.Counter = collections.Counter() skipped_ids: list[str] = [] n_total = 0 n_roots = {"total": 0} @@ -319,7 +334,9 @@ def _build(gid, color, pos, idx, nrm, transform=None, path=None) -> None: def _handle(result) -> None: nonlocal n_total - status, gid, color, pos, idx, nrm, transforms, paths = result + status, gid, color, pos, idx, nrm, transforms, paths, rebuilds = result + if rebuilds: + rebuild_totals.update(rebuilds) i = n_total n_total += 1 gid = gid or f"solid_{i}" @@ -448,7 +465,17 @@ def _spawn(wid, result_q): busy -= 1 slots[i] = _spawn(i, result_q) _handle( - ("error:WorkerCrashed (native crash in OCC)", gid, None, None, None, None, None, None) + ( + "error:WorkerCrashed (native crash in OCC)", + gid, + None, + None, + None, + None, + None, + None, + None, + ) ) continue if slot["since"] and (now - slot["since"]) > timeout_s: @@ -467,6 +494,7 @@ def _spawn(wid, result_q): None, None, None, + None, ) ) finally: @@ -492,6 +520,8 @@ def _spawn(wid, result_q): ", ".join(skipped_ids), more, ) + if rebuild_totals: + logger.info("scene_from_step_stream: %s — face repairs %s", source.path, dict(rebuild_totals)) logger.info( "scene_from_step_stream: %s — meshed %d/%d solids into %d material group(s)", source.path, @@ -505,6 +535,7 @@ def _spawn(wid, result_q): "skipped": n_skipped, "materials": len(bt.material_store), "reasons": dict(reasons), + "rebuilds": dict(rebuild_totals), } diff --git a/src/frontend/package.json b/src/frontend/package.json index 20b03ca0c..933f3c89d 100644 --- a/src/frontend/package.json +++ b/src/frontend/package.json @@ -11,7 +11,7 @@ "build:serve": "vite build --config vite.config.serve.ts --emptyOutDir", "build:embed": "vite build --config vite.config.embed.ts", "dev": "vite --force", - "test": "node --import tsx --test src/__tests__/services/feaManifestPoll.test.ts src/__tests__/services/feaFieldBlob.test.ts src/__tests__/services/feaElemFieldBlob.test.ts src/__tests__/services/feaMeshEdges.test.ts src/__tests__/services/feaMeshElements.test.ts src/__tests__/services/feaBeamSolidsWarp.test.ts src/__tests__/utils/edgeShaderHelper.test.ts" + "test": "node --import tsx --test src/__tests__/services/feaManifestPoll.test.ts src/__tests__/services/feaFieldBlob.test.ts src/__tests__/services/feaElemFieldBlob.test.ts src/__tests__/services/feaMeshEdges.test.ts src/__tests__/services/feaMeshElements.test.ts src/__tests__/services/feaBeamSolidsWarp.test.ts src/__tests__/utils/edgeShaderHelper.test.ts src/__tests__/utils/rangeIndex.test.ts" }, "devDependencies": { "@tailwindcss/postcss": "^4.3.0", diff --git a/src/frontend/src/__tests__/utils/rangeIndex.test.ts b/src/frontend/src/__tests__/utils/rangeIndex.test.ts new file mode 100644 index 000000000..a275a463a --- /dev/null +++ b/src/frontend/src/__tests__/utils/rangeIndex.test.ts @@ -0,0 +1,43 @@ +/** buildRangeIndex: selection-sync identity must be (model_key, rangeId) — the + * unique numeric node id per loaded model — never the display name, which repeats + * thousands of times in real CAD models (and rangeIds restart at 0 per GLB). */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { buildRangeIndex } from "../../state/treeViewStore"; +import type { TreeNodeData } from "../../components/tree_view/CustomNode"; + +function node( + id: string, + name: string, + extra: Partial = {}, + children: TreeNodeData[] = [], +): TreeNodeData { + return { id, name, children, ...extra }; +} + +test("duplicate names across two models resolve per (model_key, rangeId)", () => { + // Two loaded models, both numbering nodes from 0 and both full of nodes + // named "Solid1". + const modelA = node("1", "a.glb", { model_key: null }, [ + node("2", "Solid1", { model_key: "A", rangeId: "0", node_name: "node0" }), + node("3", "Solid1", { model_key: "A", rangeId: "1", node_name: "node0" }), + ]); + const modelB = node("4", "b.glb", { model_key: null }, [ + node("5", "Solid1", { model_key: "B", rangeId: "0", node_name: "node0" }), + ]); + const root = node("0", "__roots__", {}, [modelA, modelB]); + + const index = buildRangeIndex(root); + + // Group/container nodes (no rangeId/model_key) are not indexed. + assert.equal(index.size, 3); + + // Same display name, same rangeId, different models — distinct tree rows. + assert.equal(index.get("A|0")!.id, "2"); + assert.equal(index.get("A|1")!.id, "3"); + assert.equal(index.get("B|0")!.id, "5"); + assert.equal(index.get("A|2"), undefined); + + // Values are live references, not copies. + assert.equal(index.get("A|0"), modelA.children[0]); +}); diff --git a/src/frontend/src/state/treeViewStore.ts b/src/frontend/src/state/treeViewStore.ts index 3e9b75984..8df7820cb 100644 --- a/src/frontend/src/state/treeViewStore.ts +++ b/src/frontend/src/state/treeViewStore.ts @@ -8,11 +8,35 @@ export interface TreeNode { children: TreeNode[]; } +/** Reverse index ``"|" -> tree node`` over a (container) tree. + * + * Selection sync must resolve a picked (mesh, rangeId) to its tree row by the + * model's UNIQUE numeric node id — display names repeat thousands of times in + * real CAD models, and rangeIds restart at 0 in every loaded GLB, so only the + * composite key is unique. Values are references to the live TreeNodeData + * objects (no copies); ~24k entries cost a couple of MB and one O(1) lookup + * replaces an O(n) recursive name scan plus a worker round-trip per range. */ +export function buildRangeIndex(root: TreeNodeData): Map { + const index = new Map(); + const stack: TreeNodeData[] = [root]; + while (stack.length) { + const n = stack.pop()!; + if (n.rangeId != null && n.model_key != null) { + index.set(`${n.model_key}|${n.rangeId}`, n); + } + if (Array.isArray(n.children)) for (const c of n.children) stack.push(c); + } + return index; +} + export interface TreeViewState { treeData: TreeNodeData | null; tree: TreeApi | null; setTreeData: (data: TreeNodeData) => void; clearTreeData: () => void; + /** O(1) selection-sync lookup by the only globally-unique identity a picked + * range has: the loaded model's key plus the GLB's numeric node id. */ + findNodeByRangeId: (modelKey: string, rangeId: string) => TreeNodeData | null; isTreeCollapsed: boolean; setIsTreeCollapsed: (collapsed: boolean) => void; setTree: (tree: TreeApi) => void; @@ -33,6 +57,8 @@ export interface TreeViewState { setMaxId(max_id: number): void; } +let rangeIndex: Map = new Map(); + export const useTreeViewStore = create((set) => ({ treeData: null, tree: null, @@ -43,8 +69,15 @@ export const useTreeViewStore = create((set) => ({ max_id: 0, setSearchTerm: (searchTerm) => set({searchTerm: searchTerm}), setTree: (tree) => set({tree: tree}), - setTreeData: (data) => set({treeData: data}), - clearTreeData: () => set({treeData: null}), + setTreeData: (data) => { + rangeIndex = buildRangeIndex(data); + set({treeData: data}); + }, + clearTreeData: () => { + rangeIndex = new Map(); + set({treeData: null}); + }, + findNodeByRangeId: (modelKey, rangeId) => rangeIndex.get(`${modelKey}|${rangeId}`) ?? null, isTreeCollapsed: true, setIsTreeCollapsed: (collapsed) => set({isTreeCollapsed: collapsed}), treeViewWidth: 256, diff --git a/src/frontend/src/utils/mesh_select/getDrawRangeByName.ts b/src/frontend/src/utils/mesh_select/getDrawRangeByName.ts deleted file mode 100644 index 296ed2be0..000000000 --- a/src/frontend/src/utils/mesh_select/getDrawRangeByName.ts +++ /dev/null @@ -1,32 +0,0 @@ -import {useModelState} from "@/state/modelState"; - -export function getDrawRangeByName(name: string) { - let userdata = useModelState.getState().userdata - if (!userdata) { - return null; - } - let hierarchy: Record = userdata["id_hierarchy"]; - - let rangeId: string | null = null; - - for (let key in hierarchy) { - if (hierarchy[key][0] === name) { - rangeId = key - break - } - } - - if (!rangeId) { - return null - } - - for (let key in userdata) { - if (key.includes("draw_ranges")) { - // if rangeId is found in the keys - - if (userdata[key].hasOwnProperty(rangeId)) { - return [key, rangeId, userdata[key][rangeId][0], userdata[key][rangeId][1]]; - } - } - } -} \ No newline at end of file diff --git a/src/frontend/src/utils/mesh_select/handleClickMesh.ts b/src/frontend/src/utils/mesh_select/handleClickMesh.ts index 7b287acc5..374df1b03 100644 --- a/src/frontend/src/utils/mesh_select/handleClickMesh.ts +++ b/src/frontend/src/utils/mesh_select/handleClickMesh.ts @@ -8,7 +8,6 @@ import {perform_selection} from "./perform_selection"; import {query_ws_server_mesh_info} from "./handlers/send_mesh_selected_info_callback"; import {useTreeViewStore} from "@/state/treeViewStore"; import {useSelectedObjectStore} from "@/state/useSelectedObjectStore"; -import {findNodeById} from "../tree_view/findNodeById"; import {simulationDataRef} from "@/state/refs"; import {SimulationDataExtensionMetadata} from "@/extensions/design_and_analysis_extension"; @@ -88,14 +87,15 @@ export async function handleClickMesh( const lookupKey: string | undefined = (m as any).unique_key ?? (m.userData ? m.userData['unique_hash'] : undefined); if (!lookupKey) continue; for (const rid of selectedRanges) { - const nodeName = await queryNameFromRangeId(lookupKey, rid); - if (!nodeName) continue; - const node = findNodeById(treeViewStore.treeData, nodeName); + // Resolve by (model_key, rangeId) — the unique numeric node id — + // never by display name, which repeats thousands of times in + // real CAD models and made the tree highlight the wrong row. + const node = treeViewStore.findNodeByRangeId(lookupKey, rid); if (node) node_ids.push(node.id); } } - const lastNode = findNodeById(treeViewStore.treeData, last_selected_name); + const lastNode = treeViewStore.findNodeByRangeId(mesh.unique_key, rangeId); treeViewStore.tree.setSelection({ ids: node_ids, mostRecent: lastNode, diff --git a/src/frontend/src/utils/mesh_select/handleClickPoints.ts b/src/frontend/src/utils/mesh_select/handleClickPoints.ts index 3d486efd0..8ca60894b 100644 --- a/src/frontend/src/utils/mesh_select/handleClickPoints.ts +++ b/src/frontend/src/utils/mesh_select/handleClickPoints.ts @@ -9,7 +9,6 @@ import {gpuPointPicker} from "./GpuPointPicker"; import {useSelectedObjectStore} from "@/state/useSelectedObjectStore"; import {queryNameFromRangeId, queryPointDrawRange, queryPointRangeByRangeId} from "./queryMeshDrawRange"; import {useTreeViewStore} from "@/state/treeViewStore"; -import {findNodeById} from "../tree_view/findNodeById"; import {perform_selection} from "./perform_selection"; export async function handleClickPoints( @@ -143,14 +142,13 @@ export async function handleClickPoints( const lookupKey: string | undefined = (o as any).unique_key ?? (o.userData ? o.userData['unique_hash'] : undefined); if (!lookupKey) continue; for (const rid of selectedRanges) { - const nodeName = await queryNameFromRangeId(lookupKey, rid); - if (!nodeName) continue; - const node = findNodeById(treeViewStore.treeData, nodeName); + // Unique (model_key, rangeId) lookup — see handleClickMesh. + const node = treeViewStore.findNodeByRangeId(lookupKey, rid); if (node) node_ids.push(node.id); } } - const lastNode = findNodeById(treeViewStore.treeData, selected); + const lastNode = treeViewStore.findNodeByRangeId(hash, rangeId); treeViewStore.tree.setSelection({ ids: node_ids, mostRecent: lastNode, diff --git a/src/frontend/src/utils/scene/animations/AnimationController.ts b/src/frontend/src/utils/scene/animations/AnimationController.ts index 4378068dd..ce3f7da8d 100644 --- a/src/frontend/src/utils/scene/animations/AnimationController.ts +++ b/src/frontend/src/utils/scene/animations/AnimationController.ts @@ -47,6 +47,10 @@ export class AnimationController { for (const node_name of node_names) { const searchName = node_name.replace('.', ''); + // TODO: name-based lookup mis-resolves on duplicate display names — + // key by the GLB node index instead (channel.target.node is available + // in mapAnimationTargets but currently discarded). FEA animation + // meshes have unique names in practice, so this is not yet load-bearing. const mesh = scene.getObjectByName(searchName) as THREE.Mesh; if (mesh) { diff --git a/src/frontend/src/utils/scene/crossModelSelect.ts b/src/frontend/src/utils/scene/crossModelSelect.ts index 20ee2ad69..d0802565d 100644 --- a/src/frontend/src/utils/scene/crossModelSelect.ts +++ b/src/frontend/src/utils/scene/crossModelSelect.ts @@ -1,4 +1,4 @@ -import {getDrawRangeByName} from '@/utils/mesh_select/getDrawRangeByName'; +import {modelStore} from '@/state/model_worker/modelStore'; import {useModelState} from '@/state/modelState'; import {useSelectedObjectStore} from '@/state/useSelectedObjectStore'; import {useObjectInfoStore} from '@/state/objectInfoStore'; @@ -22,11 +22,9 @@ type Args = { * cross-model select work correctly when multiple files are overlaid. * * Limitation: the target file must already be loaded (overlay or - * single-active). If it isn't, ``getDrawRangeByName`` will fail - * because the active model's userdata is queried for the lookup. We - * surface a console warning rather than auto-loading — auto-load is a - * follow-up feature once the storage browser exposes the necessary - * hook. + * single-active); we surface a console warning rather than auto-loading + * — auto-load is a follow-up feature once the storage browser exposes + * the necessary hook. */ export async function selectInOtherModel({file, nodeNames}: Args): Promise { const loadedSourceName = useModelState.getState().loadedSourceName; @@ -60,34 +58,43 @@ export async function selectInOtherModel({file, nodeNames}: Args): Promise } // Build a local name→mesh index for this root only, so overlays of - // different files don't poison the lookup. + // different files don't poison the lookup — and recover the TARGET model's + // cache key from its meshes (the old code queried the ACTIVE model's + // userdata, which mis-resolved whenever the link target wasn't active). const meshByName = new Map(); + let modelKey: string | null = null; root.traverse((obj: THREE.Object3D) => { if ((obj as any).isMesh) { meshByName.set(obj.name, obj as unknown as CustomBatchedMesh); + modelKey ??= (obj as any).unique_key ?? obj.userData?.['unique_hash'] ?? null; } }); + if (!modelKey) { + console.warn(`crossModelSelect: no model key recoverable from root of "${file}"`); + return; + } - let firstSelected: string | null = null; - for (const nodeName of nodeNames) { - const lookup = getDrawRangeByName(nodeName); - if (!lookup) { - console.warn(`crossModelSelect: no draw range for "${nodeName}" in active model`); - continue; - } - const [bufferKey, rangeId] = lookup; - const meshName = bufferKey.replace(/^draw_ranges_/, ''); + // One batched worker query resolves every member name to (meshName, rangeId) + // against the TARGET model's hierarchy — all matches, not first-match-wins, + // and the O(hierarchy) scan runs off the main thread. + const pairs = await modelStore.getDrawRangesByMemberNames(modelKey, nodeNames); + if (pairs.length === 0) { + console.warn(`crossModelSelect: no draw ranges for ${nodeNames.length} member name(s) in "${file}"`); + return; + } + let selected = 0; + for (const [meshName, rangeId] of pairs) { const mesh = meshByName.get(meshName); if (!mesh) { console.warn(`crossModelSelect: mesh "${meshName}" not found in target root`); continue; } selectionStore.addSelectedObject(mesh, String(rangeId)); - if (firstSelected === null) firstSelected = nodeName; + selected++; } - if (firstSelected !== null) { - useObjectInfoStore.getState().setName(firstSelected); + if (selected > 0) { + useObjectInfoStore.getState().setName(nodeNames[0]); useObjectInfoStore.getState().setFileName(file); } } diff --git a/src/frontend/src/utils/tree_view/findNodeById.ts b/src/frontend/src/utils/tree_view/findNodeById.ts deleted file mode 100644 index 1563c5318..000000000 --- a/src/frontend/src/utils/tree_view/findNodeById.ts +++ /dev/null @@ -1,17 +0,0 @@ -import {TreeNode} from "@/state/treeViewStore"; - -export function findNodeById(node: TreeNode, id: string): TreeNode | null { - if (node.name === id) { - return node; - } - if (!Array.isArray(node.children)) { - return null; - } - for (let child of node.children) { - const result = findNodeById(child, id); - if (result) { - return result; - } - } - return null -} \ No newline at end of file diff --git a/src/frontend/src/utils/tree_view/handleClickedNode.ts b/src/frontend/src/utils/tree_view/handleClickedNode.ts index 388233def..bdaea5ebd 100644 --- a/src/frontend/src/utils/tree_view/handleClickedNode.ts +++ b/src/frontend/src/utils/tree_view/handleClickedNode.ts @@ -25,13 +25,13 @@ async function get_mesh_and_draw_ranges(nodes: NodeApi[]) { console.warn("No scene found for node:", node); continue; } - let mesh = scene.getObjectByName(node_name) as CustomBatchedMesh; + // node_name (the merged-mesh name from the worker's elementToMesh map) is + // authoritative; null means this row is a pure group node with no draw + // range. The old display-name fallback could only ever bind a WRONG + // same-named mesh — display names repeat thousands of times in real models. + const mesh = node_name ? (scene.getObjectByName(node_name) as CustomBatchedMesh) : undefined; if (!mesh) { - mesh = scene.getObjectByName(node.data.name.replace("/", "")) as CustomBatchedMesh; - if (!mesh) { - // console.warn("No mesh found for node:", node_name, "in scene:", scene); - continue; - } + continue; } meshes_and_ranges.push([mesh, rangeId]); } diff --git a/tests/core/geom/test_param_extent_rebuild.py b/tests/core/geom/test_param_extent_rebuild.py new file mode 100644 index 000000000..529297c04 --- /dev/null +++ b/tests/core/geom/test_param_extent_rebuild.py @@ -0,0 +1,216 @@ +"""Regression: the param-extent face rebuild (the repair path for faces whose wire +fails to trim an infinite cylinder/cone) must not OVER-COVER. Pre-fix, +``_sample_edge_points`` always sampled circular arcs in the circle's positive +parametric direction, so the complement arc could be sampled, the u-extent snapped +to the full period, and a small wedge face rebuilt as a spurious full revolution +band — large converted models showed phantom cones/cylinders obfuscating the view. + +The arc's occupied point-set is determined by the EdgeCurve alone (start -> end, +positive direction iff ``same_sense``); ``OrientedEdge.orientation`` only reverses +traversal and is reader-convention-dependent, so sampling must ignore it.""" + +import math + +import pytest + +from ada.geom.curves import Circle, EdgeCurve, OrientedEdge +from ada.geom.placement import Axis2Placement3D +from ada.geom.points import Point + +R = 10.0 + + +def _pt(angle_deg: float, z: float = 0.0) -> Point: + a = math.radians(angle_deg) + return Point(R * math.cos(a), R * math.sin(a), z) + + +def _arc_edge(p_start: Point, p_end: Point, *, same_sense: bool, orientation: bool) -> OrientedEdge: + """An arc on the canonical z-axis circle. ``p_start``/``p_end`` are the + EdgeCurve's OWN endpoints; the OrientedEdge endpoints follow ``orientation`` + the way the STEP stream reader builds them (swapped when reversed).""" + circ = Circle(position=Axis2Placement3D(location=Point(0, 0, 0)), radius=R) + ec = EdgeCurve(start=p_start, end=p_end, edge_geometry=circ, same_sense=same_sense) + s, e = (p_start, p_end) if orientation else (p_end, p_start) + return OrientedEdge(start=s, end=e, edge_element=ec, orientation=orientation) + + +def _sampled_angles(oe) -> list[float]: + pytest.importorskip("OCC", reason="samples via the pythonocc face-rebuild helpers") + from ada.occ.geom.surfaces import _sample_edge_points + + out = [] + for p in _sample_edge_points(oe): + out.append(math.degrees(math.atan2(p[1], p[0])) % 360.0) + return out + + +def _assert_within_first_quadrant(angles: list[float]) -> None: + # The physical arc is 0..90 deg; tolerate endpoint rounding. + for a in angles: + ok = a <= 90.5 or a >= 359.5 + assert ok, f"sample at {a:.1f} deg lies on the complement arc: {angles}" + + +def test_arc_sampling_follows_edge_curve_sense_not_orientation(): + """The same physical 90-degree arc (first quadrant), authored three ways, must + sample the same angular window — never the 270-degree complement.""" + a, b = _pt(0), _pt(90) + + # 1) Plain forward: EdgeCurve 0->90 with same_sense, traversed forward. + _assert_within_first_quadrant(_sampled_angles(_arc_edge(a, b, same_sense=True, orientation=True))) + + # 2) STEP-reader form: same EdgeCurve, OrientedEdge reversed (endpoints + # pre-swapped by the reader). Occupied arc unchanged. + _assert_within_first_quadrant(_sampled_angles(_arc_edge(a, b, same_sense=True, orientation=False))) + + # 3) Reversed-sense authoring: EdgeCurve 90->0 with same_sense=False — the + # occupied arc still goes 90 -> 0 the NEGATIVE way through the first + # quadrant (pre-fix this sampled 90 -> 360 (the complement)). + _assert_within_first_quadrant(_sampled_angles(_arc_edge(b, a, same_sense=False, orientation=True))) + + +def test_full_circle_still_samples_whole_period(): + pytest.importorskip("OCC", reason="samples via the pythonocc face-rebuild helpers") + angles = _sampled_angles(_arc_edge(_pt(0), _pt(0), same_sense=True, orientation=True)) + # Samples must cover all four quadrants. + quadrants = {int(a // 90) % 4 for a in angles} + assert quadrants == {0, 1, 2, 3}, angles + + +# --------------------------------------------------------------------------- # +# Param-extent rebuild: closure faces survive, holes are re-added, the area +# gate rejects rebuilds that over-cover their own boundary evidence. +# --------------------------------------------------------------------------- # +def _cyl_pt(angle_deg: float, z: float, r: float = R) -> Point: + a = math.radians(angle_deg) + return Point(r * math.cos(a), r * math.sin(a), z) + + +def _full_circle(z: float) -> OrientedEdge: + p = _cyl_pt(0, z) + circ = Circle(position=Axis2Placement3D(location=Point(0, 0, z)), radius=R) + ec = EdgeCurve(start=p, end=p, edge_geometry=circ, same_sense=True) + return OrientedEdge(start=p, end=p, edge_element=ec, orientation=True) + + +def _arc(a_deg: float, b_deg: float, z: float, *, reverse_traversal: bool = False) -> OrientedEdge: + """Arc occupying a_deg..b_deg (positive direction). ``reverse_traversal`` + authors it the way STEP writes a wire walking the arc backwards: the SAME + EdgeCurve wrapped in ORIENTED_EDGE(.F.) with swapped traversal endpoints.""" + p1, p2 = _cyl_pt(a_deg, z), _cyl_pt(b_deg, z) + circ = Circle(position=Axis2Placement3D(location=Point(0, 0, z)), radius=R) + ec = EdgeCurve(start=p1, end=p2, edge_geometry=circ, same_sense=True) + if reverse_traversal: + return OrientedEdge(start=p2, end=p1, edge_element=ec, orientation=False) + return OrientedEdge(start=p1, end=p2, edge_element=ec, orientation=True) + + +def _vline(angle_deg: float, z1: float, z2: float) -> OrientedEdge: + from ada.geom.curves import Line + from ada.geom.direction import Direction + + p1, p2 = _cyl_pt(angle_deg, z1), _cyl_pt(angle_deg, z2) + ec = EdgeCurve( + start=p1, end=p2, edge_geometry=Line(pnt=p1, dir=Direction(0, 0, 1 if z2 > z1 else -1)), same_sense=True + ) + return OrientedEdge(start=p1, end=p2, edge_element=ec, orientation=True) + + +def _cyl_surface() -> "object": + from ada.geom.surfaces import CylindricalSurface + + return CylindricalSurface(position=Axis2Placement3D(location=Point(0, 0, 0)), radius=R) + + +def _make_advanced_face(bounds) -> "object": + from ada.geom.surfaces import AdvancedFace + + return AdvancedFace(bounds=bounds, face_surface=_cyl_surface(), same_sense=True) + + +def _bound(edges, orientation=True): + from ada.geom.curves import EdgeLoop + from ada.geom.surfaces import FaceBound + + return FaceBound(bound=EdgeLoop(edge_list=edges), orientation=orientation) + + +def _meshed_area(face) -> float: + pytest.importorskip("OCC") + from ada.occ.geom.surfaces import _face_area + + return _face_area(face) + + +H = 20.0 + + +def test_full_cylinder_survives_area_gate(): + """A genuinely closed cylinder band (two full-circle rim bounds) must rebuild + to ~2*pi*r*h — the gate must not reject legitimate closure faces.""" + pytest.importorskip("OCC") + from ada.occ.geom.surfaces import make_face_from_geom + + af = _make_advanced_face([_bound([_full_circle(0.0)]), _bound([_full_circle(H)])]) + face = make_face_from_geom(af) + area = _meshed_area(face) + expect = 2 * math.pi * R * H + assert abs(area - expect) / expect < 0.1, f"area {area} vs expected {expect}" + + +def test_inner_hole_bound_is_readded(): + """A non-closure bound on a closed cylinder is a cutout: the rebuilt face's + area must be ~2*pi*r*h minus the hole patch.""" + pytest.importorskip("OCC") + from ada.occ.geom import surfaces as su + + hole = _bound( + [ + _arc(30, 60, 8.0), + _vline(60, 8.0, 12.0), + _arc(30, 60, 12.0, reverse_traversal=True), + _vline(30, 12.0, 8.0), + ], + orientation=False, + ) + af = _make_advanced_face([_bound([_full_circle(0.0)]), _bound([_full_circle(H)]), hole]) + su.consume_param_rebuild_stats() # reset + face = su.make_face_from_geom(af) + stats = su.consume_param_rebuild_stats() + area = _meshed_area(face) + full = 2 * math.pi * R * H + hole_area = (math.radians(30) * R) * 4.0 # 30deg arc span * 4 height + assert stats.get("inner_bound_readded", 0) == 1, stats + assert abs(area - (full - hole_area)) / full < 0.1, f"area {area}, expected ~{full - hole_area}" + + +def test_area_gate_drops_forced_overcover(monkeypatch): + """Force the u-extent to the full period on a small wedge: the rebuilt band + over-covers the wedge's boundary evidence and must be rejected (None).""" + pytest.importorskip("OCC") + from ada.occ.geom import surfaces as su + + h = 2.0 + wedge = _bound( + [ + _arc(0, 20, 0.0), + _vline(20, 0.0, h), + _arc(0, 20, h, reverse_traversal=True), + _vline(0, h, 0.0), + ] + ) + af = _make_advanced_face([wedge]) + occ_surf = su.make_surface_from_geom(af.face_surface) + + def _force_full(vals, periodic, period): + if periodic: + return 0.0, period + return min(vals), max(vals) + + monkeypatch.setattr(su, "_param_extent", _force_full) + su.consume_param_rebuild_stats() + face = su._make_face_from_param_extent(af, occ_surf) + stats = su.consume_param_rebuild_stats() + assert face is None + assert stats.get("area_gate_dropped", 0) == 1, stats