From ce0d99a4d1ff4ba602f31693cda3f1ec8a8cc39c Mon Sep 17 00:00:00 2001 From: Roger Maitland Date: Mon, 2 Mar 2026 11:44:25 -0500 Subject: [PATCH] Fixed Issue #1095 --- src/build123d/topology/one_d.py | 239 ++++++++++++++----------- tests/test_direct_api/test_edge.py | 77 ++++++++ tests/test_direct_api/test_mixin1_d.py | 9 + tests/test_direct_api/test_wire.py | 79 +++++++- 4 files changed, 296 insertions(+), 108 deletions(-) diff --git a/src/build123d/topology/one_d.py b/src/build123d/topology/one_d.py index 9bfc075..a515fec 100644 --- a/src/build123d/topology/one_d.py +++ b/src/build123d/topology/one_d.py @@ -53,6 +53,7 @@ from __future__ import annotations import copy import warnings +from bisect import bisect_right from collections.abc import Iterable, Sequence from itertools import combinations from math import atan2, ceil, copysign, cos, floor, inf, isclose, pi, radians @@ -88,6 +89,7 @@ from OCP.BRepOffsetAPI import BRepOffsetAPI_MakeOffset from OCP.BRepPrimAPI import BRepPrimAPI_MakeHalfSpace from OCP.BRepProj import BRepProj_Projection from OCP.BRepTools import BRepTools, BRepTools_WireExplorer +from OCP.Extrema import Extrema_ExtPC from OCP.GC import ( GC_MakeArcOfCircle, GC_MakeArcOfEllipse, @@ -97,6 +99,7 @@ from OCP.GC import ( from OCP.GCPnts import ( GCPnts_AbscissaPoint, GCPnts_QuasiUniformDeflection, + GCPnts_TangentialDeflection, GCPnts_UniformDeflection, ) from OCP.GProp import GProp_GProps @@ -756,13 +759,9 @@ class Mixin1D(Shape[TOPODS]): # 1D + 1D: Common (collinear overlap) + Section (crossing vertices) elif isinstance(other, (Edge, Wire)): - common = self._bool_op_list( - (self,), (other,), BRepAlgoAPI_Common() - ) + common = self._bool_op_list((self,), (other,), BRepAlgoAPI_Common()) results.extend(common.expand()) - section = self._bool_op_list( - (self,), (other,), BRepAlgoAPI_Section() - ) + section = self._bool_op_list((self,), (other,), BRepAlgoAPI_Section()) # Extract vertices from section (edges already in Common for wires) for shape in section: if isinstance(shape, Vertex) and not shape.is_null: @@ -786,7 +785,6 @@ class Mixin1D(Shape[TOPODS]): distance: float, position_mode: PositionMode = PositionMode.PARAMETER, frame_method: FrameMethod = FrameMethod.FRENET, - planar: bool | None = None, x_dir: VectorLike | None = None, ) -> Location: """Locations along curve @@ -802,15 +800,10 @@ class Mixin1D(Shape[TOPODS]): spots. The CORRECTED frame behaves more like a “camera dolly” or sweep profile would — it's smoother and more stable. Defaults to FrameMethod.FRENET. - planar (bool, optional): planar mode. Defaults to None. x_dir (VectorLike, optional): override the x_dir to help with plane - creation along a 1D shape. Must be perpendicalar to shapes tangent. + creation along a 1D shape. Must be perpendicular to shapes tangent. Defaults to None. - .. deprecated:: - The `planar` parameter is deprecated and will be removed in a future release. - Use `x_dir` to specify orientation instead. - Returns: Location: A Location object representing local coordinate system at the specified distance. @@ -823,10 +816,25 @@ class Mixin1D(Shape[TOPODS]): else: distance = self.length - distance - if position_mode == PositionMode.PARAMETER: - param = self.param_at(distance) - else: - param = self.param_at(distance / self.length) + if isinstance(self, Wire): + if frame_method == FrameMethod.CORRECTED: + # BRep_CompCurve parameter + param = self.param_at(distance / self.length) + else: + # A BRep_Curve parameter taken from a Edge based curve + if position_mode == PositionMode.PARAMETER: + curve, param, _ = self._occt_param_at( + distance * self.length, PositionMode.LENGTH + ) + else: + curve, param, _ = self._occt_param_at(distance, PositionMode.LENGTH) + + else: # Edge + if position_mode == PositionMode.PARAMETER: + param = self.param_at(distance) + else: + param = self.param_at(distance / self.length) + curve = self.geom_adaptor() law: GeomFill_TrihedronLaw if frame_method == FrameMethod.FRENET: @@ -842,18 +850,7 @@ class Mixin1D(Shape[TOPODS]): pnt = curve.Value(param) transformation = gp_Trsf() - if planar is not None: - warnings.warn( - "The 'planar' parameter is deprecated and will be removed in a future version. " - "Use 'x_dir' to control orientation instead.", - DeprecationWarning, - stacklevel=2, - ) - if planar is not None and planar: - transformation.SetTransformation( - gp_Ax3(pnt, gp_Dir(0, 0, 1), gp_Dir(normal.XYZ())), gp_Ax3() - ) - elif x_dir is not None: + if x_dir is not None: try: transformation.SetTransformation( @@ -881,7 +878,6 @@ class Mixin1D(Shape[TOPODS]): distances: Iterable[float], position_mode: PositionMode = PositionMode.PARAMETER, frame_method: FrameMethod = FrameMethod.FRENET, - planar: bool | None = None, x_dir: VectorLike | None = None, ) -> list[Location]: """Locations along curve @@ -894,22 +890,16 @@ class Mixin1D(Shape[TOPODS]): Defaults to PositionMode.PARAMETER. frame_method (FrameMethod, optional): moving frame calculation method. Defaults to FrameMethod.FRENET. - planar (bool, optional): planar mode. Defaults to False. x_dir (VectorLike, optional): override the x_dir to help with plane - creation along a 1D shape. Must be perpendicalar to shapes tangent. + creation along a 1D shape. Must be perpendicular to shapes tangent. Defaults to None. - .. deprecated:: - The `planar` parameter is deprecated and will be removed in a future release. - Use `x_dir` to specify orientation instead. - Returns: list[Location]: A list of Location objects representing local coordinate systems at the specified distances. """ return [ - self.location_at(d, position_mode, frame_method, planar, x_dir) - for d in distances + self.location_at(d, position_mode, frame_method, x_dir) for d in distances ] def normal(self) -> Vector: @@ -1041,42 +1031,6 @@ class Mixin1D(Shape[TOPODS]): offset_edges = offset_wire.edges() return offset_edges[0] if len(offset_edges) == 1 else offset_wire - def param_at(self, position: float) -> float: - """ - Map a normalized arc-length position to the underlying OCCT parameter. - - The meaning of the returned parameter depends on the type of self: - - - **Edge**: Returns the native OCCT curve parameter corresponding to the - given normalized `position` (0.0 → start, 1.0 → end). For closed/periodic - edges, OCCT may return a value **outside** the edge's nominal parameter - range `[param_min, param_max]` (e.g., by adding/subtracting multiples of - the period). If you require a value folded into the edge's range, apply a - modulo with the parameter span. - - - **Wire**: Returns a *composite* parameter encoding both the edge index - and the position within that edge: the **integer part** is the zero-based - count of fully traversed edges, and the **fractional part** is the - normalized position in `[0.0, 1.0]` along the current edge. - - Args: - position (float): Normalized arc-length position along the shape, - where `0.0` is the start and `1.0` is the end. Values outside - `[0.0, 1.0]` are not validated and yield OCCT-dependent results. - - Returns: - float: OCCT parameter (for edges) **or** composite “edgeIndex + fraction” - parameter (for wires), as described above. - - """ - - curve = self.geom_adaptor() - - length = GCPnts_AbscissaPoint.Length_s(curve) - return GCPnts_AbscissaPoint( - curve, length * position, curve.FirstParameter() - ).Parameter() - def perpendicular_line( self, length: float, u_value: float, plane: Plane = Plane.XY ) -> Edge: @@ -1372,22 +1326,6 @@ class Mixin1D(Shape[TOPODS]): """ return self.derivative_at(position, 1, position_mode).normalized() - # def vertex(self) -> Vertex | None: - # """Return the Vertex""" - # return Shape.get_single_shape(self, "Vertex") - - # def vertices(self) -> ShapeList[Vertex]: - # """vertices - all the vertices in this Shape""" - # return Shape.get_shape_list(self, "Vertex") - - # def wire(self) -> Wire | None: - # """Return the Wire""" - # return Shape.get_single_shape(self, "Wire") - - # def wires(self) -> ShapeList[Wire]: - # """wires - all the wires in this Shape""" - # return Shape.get_shape_list(self, "Wire") - class Edge(Mixin1D[TopoDS_Edge]): """An Edge in build123d is a fundamental element in the topological data structure @@ -1855,12 +1793,13 @@ class Edge(Mixin1D[TopoDS_Edge]): direction: VectorLike | None = None, ) -> ShapeList[Edge]: """ - Create all planar line(s) on the XY plane tangent to one curve and passing - through a fixed point. + Create all planar line(s) on the XY plane tangent to one curve with a + fixed orientation, defined either by an angle measured from a reference + axis or by a direction vector. Args: tangency_one (Edge): edge that line will be tangent to - tangency_two (Axis): axis that angle will be measured against + tangency_two (Axis): reference axis from which the angle is measured angle : float, optional Line orientation in degrees (measured CCW from the X-axis). direction : VectorLike, optional @@ -2808,6 +2747,35 @@ class Edge(Mixin1D[TopoDS_Edge]): ).Parameter() return comp_curve, occt_param, self.is_forward + def param_at(self, position: float) -> float: + """ + Map a normalized arc-length position to the underlying OCCT parameter. + + Returns the native OCCT curve parameter corresponding to the + given normalized `position` (0.0 → start, 1.0 → end). For closed/periodic + edges, OCCT may return a value **outside** the edge's nominal parameter + range `[param_min, param_max]` (e.g., by adding/subtracting multiples of + the period). If you require a value folded into the edge's range, apply a + modulo with the parameter span. + + Args: + position (float): Normalized arc-length position along the shape, + where `0.0` is the start and `1.0` is the end. Values outside + `[0.0, 1.0]` are not validated and yield OCCT-dependent results. + + Returns: + float: OCCT parameter (for edges) **or** composite “edgeIndex + fraction” + parameter (for wires), as described above. + + """ + + curve = self.geom_adaptor() + + length = GCPnts_AbscissaPoint.Length_s(curve) + return GCPnts_AbscissaPoint( + curve, length * position, curve.FirstParameter() + ).Parameter() + def param_at_point(self, point: VectorLike) -> float: """ Return the normalized parameter (∈ [0.0, 1.0]) of the location on this edge @@ -3870,6 +3838,40 @@ class Wire(Mixin1D[TopoDS_Wire]): for e1, e2 in zip(edges1, edges2) ) + def param_at(self, position: float) -> float: + """ + Return the OCCT comp-curve parameter corresponding to the given wire position. + This is *not* the edge composite parameter; it is the parameter of the wire’s + BRepAdaptor_CompCurve. + """ + curve = self.geom_adaptor() + + # Compute the correct target point along the wire + target_pnt = self.position_at(position) + + # Project to comp-curve parameter + extrema = Extrema_ExtPC(target_pnt.to_pnt(), curve) + if not extrema.IsDone() or extrema.NbExt() == 0: + raise RuntimeError("Failed to find point on curve") + + min_dist = float("inf") + closest_pnt = None + closest_param = None + for i in range(1, extrema.NbExt() + 1): + dist = extrema.SquareDistance(i) + if dist < min_dist: + min_dist = dist + closest_pnt = extrema.Point(i).Value() + closest_param = extrema.Point(i).Parameter() + + if ( + closest_pnt is None + or (Vector(tcast(gp_Pnt, closest_pnt)) - target_pnt).length > TOLERANCE + ): + raise RuntimeError("Failed to find point on curve") + + return tcast(float, closest_param) + def param_at_point(self, point: VectorLike) -> float: """ Return the normalized wire parameter for the point closest to this wire. @@ -3992,28 +3994,51 @@ class Wire(Mixin1D[TopoDS_Wire]): at the given position, the corresponding OCCT parameter on that edge and if edge is_forward. """ - wire_curve_adaptor = self.geom_adaptor() - + # Normalize to absolute distance along the wire if position_mode == PositionMode.PARAMETER: if not self.is_forward: - position = 1 - position - occt_wire_param = self.param_at(position) + position = 1.0 - position + distance = position * self.length else: if not self.is_forward: position = self.length - position - occt_wire_param = self.param_at(position / self.length) + distance = position - topods_edge_at_position = TopoDS_Edge() - occt_edge_params = wire_curve_adaptor.Edge( - occt_wire_param, topods_edge_at_position - ) - edge_curve_adaptor = BRepAdaptor_Curve(topods_edge_at_position) + # Build ordered edges and cumulative lengths + self_edges = self.edges() + edge_lengths = [e.length for e in self_edges] + cumulative_lengths = [] + total = 0.0 + for edge_length in edge_lengths: + total += edge_length + cumulative_lengths.append(total) - return ( - edge_curve_adaptor, - occt_edge_params[0], - topods_edge_at_position.Orientation() == TopAbs_Orientation.TopAbs_FORWARD, + # Clamp distance + if distance <= 0.0: + edge_idx = 0 + local_dist = 0.0 + elif distance >= total: + edge_idx = len(self_edges) - 1 + local_dist = edge_lengths[edge_idx] + else: + edge_idx = bisect_right(cumulative_lengths, distance) + prev_cum = cumulative_lengths[edge_idx - 1] if edge_idx > 0 else 0.0 + local_dist = distance - prev_cum + + target_edge = self_edges[edge_idx] + + # Convert local distance to edge fraction + local_frac = ( + 0.0 if target_edge.length == 0 else (local_dist / target_edge.length) ) + if not target_edge.is_forward: + local_frac = 1 - local_frac + + # Use edge param_at to get native OCCT parameter + occt_edge_param = target_edge.param_at(local_frac) + + edge_curve_adaptor = target_edge.geom_adaptor() + return edge_curve_adaptor, occt_edge_param, target_edge.is_forward def project_to_shape( self, diff --git a/tests/test_direct_api/test_edge.py b/tests/test_direct_api/test_edge.py index 6f06f68..63081ae 100644 --- a/tests/test_direct_api/test_edge.py +++ b/tests/test_direct_api/test_edge.py @@ -444,5 +444,82 @@ class TestEdge(unittest.TestCase): line.geom_adaptor() +class TestEdgeParamAt(unittest.TestCase): + """Edge.param_at regression tests (Issue #1095).""" + + def test_param_at_line_midpoint(self): + """ + On a straight line, arc length is linear in parameter. + param_at(0.5) should correspond to the geometric midpoint. + """ + edge = Edge.make_line(Vector(0, 0, 0), Vector(4, 0, 0)) + u = edge.param_at(0.5) + mid = Vector(edge.geom_adaptor().Value(u)) + self.assertAlmostEqual(mid.X, 2.0, places=6) + self.assertAlmostEqual(mid.Y, 0.0, places=6) + self.assertAlmostEqual(mid.Z, 0.0, places=6) + + def test_param_at_arc_quarter_point(self): + """ + Unit semicircle (0° → 180°). Arc-length quarter point is at 45°. + """ + edge = Edge.make_circle(1.0, Plane.XY, 0, 180) + u = edge.param_at(0.25) + pt = Vector(edge.geom_adaptor().Value(u)) + expected_x = math.cos(math.radians(45)) + expected_y = math.sin(math.radians(45)) + self.assertAlmostEqual(pt.X, expected_x, places=5) + self.assertAlmostEqual(pt.Y, expected_y, places=5) + + def test_param_at_ellipse_is_not_linear_in_parameter(self): + """ + On an ellipse, a naive linear parameter map does NOT follow arc length. + This test verifies param_at uses arc-length based mapping. + """ + edge = Edge.make_ellipse(4.0, 2.0, Plane.XY, 0, 180) + # Arc-length quarter point (0.25 of length) + u_len = edge.param_at(0.25) + pt_len = Vector(edge.geom_adaptor().Value(u_len)) + + # Naive "parameter space" quarter (just interpolate between start/end) + # Equivalent to assuming parameter == normalized length. + u_naive = edge.geom_adaptor().FirstParameter() + 0.25 * ( + edge.geom_adaptor().LastParameter() - edge.geom_adaptor().FirstParameter() + ) + pt_naive = Vector(edge.geom_adaptor().Value(u_naive)) + + # These should differ measurably on an ellipse. + self.assertGreater( + (pt_len - pt_naive).length, + 1e-3, + msg="Ellipse arc-length mapping appears linear in parameter (unexpected).", + ) + + def test_param_at_arc_endpoints_are_exact(self): + """param_at(0.0) and param_at(1.0) must map to arc start/end.""" + edge = Edge.make_circle(3.0, Plane.XY, 30, 150) + + u0 = edge.param_at(0.0) + u1 = edge.param_at(1.0) + p0 = Vector(edge.geom_adaptor().Value(u0)) + p1 = Vector(edge.geom_adaptor().Value(u1)) + + expected_start = Vector( + 3.0 * math.cos(math.radians(30)), + 3.0 * math.sin(math.radians(30)), + 0.0, + ) + expected_end = Vector( + 3.0 * math.cos(math.radians(150)), + 3.0 * math.sin(math.radians(150)), + 0.0, + ) + + self.assertAlmostEqual(p0.X, expected_start.X, places=5) + self.assertAlmostEqual(p0.Y, expected_start.Y, places=5) + self.assertAlmostEqual(p1.X, expected_end.X, places=5) + self.assertAlmostEqual(p1.Y, expected_end.Y, places=5) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_direct_api/test_mixin1_d.py b/tests/test_direct_api/test_mixin1_d.py index 864711b..8504df4 100644 --- a/tests/test_direct_api/test_mixin1_d.py +++ b/tests/test_direct_api/test_mixin1_d.py @@ -275,6 +275,15 @@ class TestMixin1D(unittest.TestCase): self.assertAlmostEqual(loc.position, (0, 1, 0), 5) self.assertAlmostEqual(loc.orientation, (0, -90, -90), 5) + path = Polyline((0, 0), (10, 0), (10, 10), (0, 10)) + loc = path.location_at(0.75) + self.assertAlmostEqual(loc.position, (7.5, 10, 0), 5) + self.assertAlmostEqual(loc.orientation, (0, -90, 180), 5) + + loc = path.location_at(0.75 * 30, position_mode=PositionMode.LENGTH) + self.assertAlmostEqual(loc.position, (7.5, 10, 0), 5) + self.assertAlmostEqual(loc.orientation, (0, -90, 180), 5) + def test_location_at_x_dir(self): path = Polyline((-50, -40), (50, -40), (50, 40), (-50, 40), close=True) l1 = path.location_at(0) diff --git a/tests/test_direct_api/test_wire.py b/tests/test_direct_api/test_wire.py index bbfb6fc..b9f069b 100644 --- a/tests/test_direct_api/test_wire.py +++ b/tests/test_direct_api/test_wire.py @@ -29,11 +29,12 @@ license: import math import random import unittest +from unittest.mock import patch, MagicMock import numpy as np from build123d.topology.shape_core import TOLERANCE -from build123d.build_enums import GeomType, Side +from build123d.build_enums import GeomType, PositionMode, Side from build123d.build_line import BuildLine from build123d.geometry import Axis, Color, Location, Plane, Vector from build123d.objects_curve import Curve, Line, PolarLine, Polyline, Spline @@ -41,6 +42,7 @@ from build123d.objects_sketch import Circle, Rectangle, RegularPolygon from build123d.operations_generic import fillet from build123d.topology import Edge, Face, Wire from OCP.BRepAdaptor import BRepAdaptor_CompCurve +from OCP.gp import gp_Pnt class TestWire(unittest.TestCase): @@ -355,5 +357,80 @@ class TestWireToBSpline(unittest.TestCase): self.assertTrue(0.0 < u1 < 1.0) +class TestWireParamAtLengthMode(unittest.TestCase): + """ + Regression test for Issue #1095: PositionMode.LENGTH is incorrect on + some composite wires due to non-linear distance→parameter mapping. + """ + + def _make_two_arc_wire(self): + """ + Composite wire from two semicircular arcs with different radii. + Total length = 3π. The arc-length midpoint (position=0.5) falls + into the second arc at 25% of its length. + + Expected midpoint: + (1 - sqrt(2), sqrt(2), 0) + """ + arc1 = Edge.make_circle( + 1.0, Plane.XY, 0, 180 + ) # center (0,0), from (1,0) to (-1,0) + + # arc2: radius 2, center shifted to (1,0), spans 180° → 360° + arc2 = Edge.make_circle(2.0, Plane.XY, 180, 360) + arc2 = arc2.moved(Location(Vector(1, 0, 0))) + + wire = Wire([arc1, arc2]) + return wire + + def test_wire_two_arcs_midpoint(self): + wire = self._make_two_arc_wire() + pt = wire.position_at(0.5, position_mode=PositionMode.PARAMETER) + + expected_x = 1.0 - math.sqrt(2) # ≈ -0.4142 + expected_y = -math.sqrt(2) # ≈ -1.4142 + expected_z = 0.0 + + tol = 1e-4 + self.assertAlmostEqual(pt.X, expected_x, delta=tol) + self.assertAlmostEqual(pt.Y, expected_y, delta=tol) + self.assertAlmostEqual(pt.Z, expected_z, delta=tol) + + +class TestWireParamAtExceptions(unittest.TestCase): + def test_param_at_raises_when_no_extrema(self): + wire = Wire([Edge.make_line((0, 0), (1, 0))]) + + # Mock Extrema_ExtPC to simulate NbExt() == 0 + mock_extrema = MagicMock() + mock_extrema.IsDone.return_value = True + mock_extrema.NbExt.return_value = 0 + + with patch("build123d.topology.one_d.Extrema_ExtPC", return_value=mock_extrema): + with self.assertRaises(RuntimeError) as ctx: + wire.param_at(0.5) + self.assertIn("Failed to find point on curve", str(ctx.exception)) + + def test_param_at_raises_when_projection_too_far(self): + wire = Wire([Edge.make_line((0, 0), (1, 0))]) + + # Mock Extrema_ExtPC to return a point far from the target + mock_extrema = MagicMock() + mock_extrema.IsDone.return_value = True + mock_extrema.NbExt.return_value = 1 + + # Return a point far away and a dummy parameter + mock_point = MagicMock() + mock_point.Value.return_value = gp_Pnt(1000, 1000, 0) + mock_point.Parameter.return_value = 0.123 + mock_extrema.Point.return_value = mock_point + mock_extrema.SquareDistance.return_value = 1000.0 * 1000.0 + + with patch("build123d.topology.one_d.Extrema_ExtPC", return_value=mock_extrema): + with self.assertRaises(RuntimeError) as ctx: + wire.param_at(0.5) + self.assertIn("Failed to find point on curve", str(ctx.exception)) + + if __name__ == "__main__": unittest.main()