extent1d: make the WR-509 fix the default, drop the opt-in flag

Since #2647 solidcore adds no UpstreamFixes flags. ContainsPeriodicValue now
always moves a value near the top of the period above the start down by the
period, kept where it lies in the interval, instead of setting it to the
start. The flag, the *_with variants and the old branch are gone; every
caller, including the kernel-less ones, gets the fix.

The flag-off halves of the #1518 tests are now the regression tests (they
fail on the old path). Upstream's MyTest for Extent2d::contains_periodic_point
moves from 1.0 to 0.9 (8.9 mod 4 inside [1, 3] +- 0.2); recorded in WR-509.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

solidcore/crates/solidcore/src/curve/ellipse/p2.rs

@@ -2330,6 +2330,40 @@ mod tests {
         assert_eq!(c.base().form(), BSplineCurveForm::EllipticArc);
     }
 
+    #[test]
+    fn issue_1518_short_arc_inverts_each_point_to_itself() {
+        // The 70.9232 x 16 ellipse of #1518 on an arc 7e-5 degrees long, shorter than
+        // the angular tolerance (1e-4 degrees); atan2 puts its points a period up.
+        let k = kernel();
+        let (d0, d1) = (-10.0, -10.0 + 7.0e-5);
+        let e = Ellipse::new(
+            &k,
+            Vec3::ZERO,
+            Vec3::X,
+            Vec3::Y,
+            Extent1d::from_min_max(d0, d1),
+            70.9232,
+            16.0,
+            &EllipseOptions::default(),
+        )
+        .expect("ellipse");
+        let fractions = [0.25, 0.5, 0.75, 1.0];
+        let points: Vec<(f64, Point3)> = fractions
+            .iter()
+            .map(|f| {
+                let a = d0 + f * (d1 - d0);
+                (a, e.evaluate_step_point(a).expect("step point"))
+            })
+            .collect();
+
+        // each point inverts to its own angle, not to the arc's start (the pinned C++
+        // returns -10 for all four)
+        for &(a, p) in &points {
+            let t = e.step_inversion(&k, p, false, 0.0).0;
+            assert!((t - a).abs() <= 1e-9 * a.abs(), "{a}: {t}");
+        }
+    }
+
     #[test]
     fn step_and_nurb_parameters_agree() {
         let k = kernel();

solidcore/crates/solidcore/src/geom/extent1d.rs

@@ -72,6 +72,9 @@
 //!   prefix and zero-fills, unbounded dimensions are not written, counts are
 //!   incremented); arrays shorter than the dimension (undefined behaviour upstream)
 //!   are rejected with [`SolidError::InvalidInput`].
+//! - `ContainsPeriodicValue` moves a value within the tolerance of the top of the period
+//!   above the start down by the period, and keeps it where that lies in the interval,
+//!   instead of setting it to the start (WR-509, #1518; tangency-labs/smlib#46).
 //! - `SM_ASSERT*` debug checks and the error-log side effect of `SE(...)` are not
 //!   ported (they do not change results). The destructor's "undefined" sentinel has
 //!   no Rust counterpart; [`Extent1d::is_defined`] still checks for it.
@@ -844,12 +847,14 @@ impl Extent1d {
 
         contained = map > self.min - zero && map < self.max + zero;
 
-        if !contained
-            && are_same_to_tol(map, ivl.max(), zero)
-            && are_same_to_tol(map - period, self.min, zero)
-        {
-            map = self.min;
-            contained = true;
+        if !contained && are_same_to_tol(map, ivl.max(), zero) {
+            // a period down, kept where it lies in the interval; the C++ sets it to the
+            // start instead (WR-509, #1518; tangency-labs/smlib#46)
+            let down = map - period;
+            if down > self.min - zero && down < self.max + zero {
+                map = down;
+                contained = true;
+            }
         }
 
         let periodic_value =
@@ -1150,6 +1155,27 @@ mod tests {
             .is_err());
     }
 
+    #[test]
+    fn issue_1518_value_past_start_moves_down_a_period() {
+        // An interval shorter than the tolerance (1e-4 degrees), and a value 3e-5 past
+        // its start given a period up: the C++ returns the start (-10).
+        let q = Extent1d::from_min_max(-10.0, -10.0 + 7.0e-5);
+        let v = 350.0 + 3.0e-5;
+        let c = q.contains_periodic_value(v, 360.0, -1.0);
+        assert!(c.contained);
+        assert_eq!(c.periodic_value, v - 360.0);
+        assert!((c.dist_to_min - 3.0e-5).abs() < 1e-12);
+        assert_eq!(q.clamp_periodic_value(v, 360.0, -1.0), v - 360.0);
+
+        // a value within the tolerance below the start keeps its place a period down
+        // (the C++: the start, 0)
+        let r = Extent1d::from_min_max(0.0, 90.0);
+        let w = 360.0 - 5.0e-5;
+        let c = r.contains_periodic_value(w, 360.0, -1.0);
+        assert!(c.contained);
+        assert_eq!(c.periodic_value, w - 360.0);
+    }
+
     #[test]
     fn clamp_vector() {
         let mut nd = ExtentNd::new(3);

solidcore/crates/solidcore/src/geom/extent2d.rs

@@ -1004,10 +1004,12 @@ mod tests {
         );
         assert!(!e1.is_point_on_boundary(p(1.1, 2.1), EFF_ZERO).0);
 
-        assert_eq!(
-            e1.contains_periodic_point(p(8.9, 3.0), 4.0, 0.0, 0.2),
-            (true, p(1.0, 3.0))
-        );
+        // u = 8.9 is 0.9 a period down, within the tolerance of [1, 3]: it keeps 0.9
+        // (WR-509, #1518; upstream's MyTest expects the start, 1.0)
+        let (inside, q) = e1.contains_periodic_point(p(8.9, 3.0), 4.0, 0.0, 0.2);
+        assert!(inside);
+        assert!((q.x - 0.9).abs() <= 1e-12, "{}", q.x);
+        assert_eq!(q.y, 3.0);
         assert!(!e1.contains_periodic_point(p(8.9, 3.0), 4.0, 0.0, 0.0).0);
 
         assert_eq!(e1.evaluate(0.5, 0.5), p(2.0, 3.0));

solidcore/plan/upstream-issues/WR-509--containsperiodicvalue-snaps-a-value-just-past-the-start-to-the-start.md

@@ -0,0 +1,11 @@
+#### WR-509. `SmExtent1d::ContainsPeriodicValue` sets a value just past an interval's start, given a period up, to the start, so a very short ellipse arc inverts every point to its start
+- **Severity:** Wrong result
+- **Area:** Geometry and tolerances
+- **Location:** `source/SMLib/src/SmExtent1d.cpp:1236`, `SmExtent1d::ContainsPeriodicValue` (def. `:1186`).
+- **What happens:** a value outside the interval is wrapped into `[m_dMin, m_dMin + dPeriod]`. When it then lies within the tolerance (`SM_EFF_ZERO_DEG`, 1e-4 degrees, by default) of the top of that period, and a period down is within the tolerance of `m_dMin`, it is set to `m_dMin`, not to its own value a period down. A value up to the tolerance past the start is taken back to the start. `SmEllipse::STEPInversion` takes its angle from `atan2` in `[0, 360)` and maps it through this function, so on an arc whose start is not in `[0, 360)` it returns the start for every point that close to it, and for an arc shorter than the tolerance for every point of it.
+- **Trigger:** the 70.9232 x 16 ellipse where a plane cuts a cylinder, trimmed to 3.39e-6 of its NURBS parameter (8.6e-5 mm of a tube's wall cut by another tube's end plane): `SmEllipse` test 2 fails (2.1e-5 off) though its NURBS lies on the ellipse (#1518). In the port's tests, a 70.9232 x 16 ellipse on `[-10, -10 + 7e-5]` degrees inverts the points at a quarter, half, three quarters and the end of the arc to -10.
+- **How found:** SolidCraft (tangency-labs/solidcore#1518; the fork's fix tangency-labs/smlib#46).
+- **Confirmed:** in the Rust port's old path, which matched the C++ source line for line (`Extent1d::contains_periodic_value`): the regression tests named under Rust port fail on it. Not run against the C++ build.
+- **Fix:** the fork moves the value down by the period and keeps it when that lies in the interval (within the tolerance of either end), as tangency-labs/smlib#46.
+- **Rust port:** fixed, as the fork (tangency-labs/smlib#46), with no flag: `Extent1d::contains_periodic_value` moves the value down by the period and keeps it where that lies in the interval. Every caller gets it, including `clamp_periodic_value`, `is_value_on_periodic_boundary`, `are_disjoint_periodic`, `Extent2d::contains_periodic_point` and the ellipse's `step_inversion`. Regression tests `issue_1518_short_arc_inverts_each_point_to_itself` and `issue_1518_value_past_start_moves_down_a_period` fail on the old path.
+- **Results that change:** a value is moved only when it lies within the tolerance of the top of the period above the interval's start. Every other value gives the same bits as before. Measured in the port's tests: upstream's `MyTest` for `Extent2d::contains_periodic_point` (`geom::extent2d::tests::upstream_my_test_extent2d`), u = 8.9 with period 4 on `[1, 3]` and tolerance 0.2, was 1.0 (the start) and is now 0.9 (8.9 a period down twice, inside the tolerance). The ellipse arc of #1518 on `[-10, -10 + 7e-5]` degrees inverts its points at 1/4, 1/2, 3/4 and the end to their own angles, where before it gave -10 for all four. Difftest cases that move are listed by the CI difftest run of this PR.