Merge pull request #2032 from altvnk/fix/wr5-circumscribe-axis-map

fix: opt in to reading the circumscribe axis map by basis (WR-5)

crates/solidcore/src/geom/extent3d.rs

@@ -55,8 +55,8 @@
 //! | [`Extent3d::u_interval`], [`Extent3d::v_interval`], [`Extent3d::w_interval`] | `GetUInterval`, `GetVInterval`, `GetWInterval` |
 //! | [`Extent3d::set_u_interval`], [`Extent3d::set_v_interval`] | `SetUInterval`, `SetVInterval` |
 //! | [`Extent3d::approximate_unbounded`], [`Extent3d::approximate_unbounded_expanded`] | `ApproximateUnbounded` |
-//! | [`Extent3d::from_pseudo_box`]          | `SmExtent3d(const SmPseudoBox&)`               |
-//! | [`Extent3d::circumscribe`]             | `Circumscribe`                                 |
+//! | [`Extent3d::from_pseudo_box`], [`Extent3d::from_pseudo_box_with`] | `SmExtent3d(const SmPseudoBox&)` |
+//! | [`Extent3d::circumscribe`], [`Extent3d::circumscribe_with`] | `Circumscribe`                       |
 //! | [`BoundaryType`]                       | `SmBoundaryType` (`SmTypes.h`)                 |
 //!
 //! Deviations:
@@ -78,15 +78,18 @@
 //! - `SM_ASSERT*` debug checks are not ported (they do not change results).
 //! - Upstream bug kept: `ContainsLineSeg3d` returns true when *either* end point is
 //!   inside (it ORs the two tests). See `plan/findings.md`.
-//! - Upstream quirk kept: `Circumscribe` of an axis-aligned pseudo box reads the
-//!   `IsAxisAligned` map (basis -> axis) as if it were axis -> basis, so a basis that is
-//!   a non-involutive permutation of the axes (e.g. `y, z, x`) gets its intervals on the
-//!   wrong axes. See `plan/findings.md`.
+//! - Upstream quirk kept by default: `Circumscribe` of an axis-aligned pseudo box
+//!   reads the `IsAxisAligned` map (basis -> axis) as if it were axis -> basis, so a
+//!   basis that is a non-involutive permutation of the axes (e.g. `y, z, x`) gets its
+//!   intervals on the wrong axes. [`Extent3d::circumscribe_with`] reads it by basis
+//!   behind `Kernel::fixes.circumscribe_axis_map_by_basis` (WR-5;
+//!   `plan/upstream-issues/WR-5--addendum-2026-10-31-opt-in-axis-map-by-basis.md`).
 
 use crate::error::{Result, SolidError};
 use crate::geom::extent1d::Extent1d;
 use crate::geom::pseudo_box::PseudoBox;
 use crate::geom::vector3d::{asin, sqrt};
+use crate::kernel::Kernel;
 use crate::math::{
     is_infinite_param, max2, max3, min2, min3, Point3, Vec3, BIG_DOUBLE, EFF_ZERO, EFF_ZERO_SQ,
     INFINITE_PARAMETER, PI, UNDEF_DOUBLE,
@@ -324,36 +327,71 @@ impl Extent3d {
         }
     }
 
-    /// The box circumscribing `pb` (`SmExtent3d(const SmPseudoBox&)`); see
-    /// [`Extent3d::circumscribe`].
+    /// The box circumscribing `pb` with a default kernel
+    /// (`SmExtent3d(const SmPseudoBox&)`); see [`Extent3d::from_pseudo_box_with`].
     #[must_use]
     pub fn from_pseudo_box(pb: &PseudoBox) -> Self {
+        Self::from_pseudo_box_with(pb, &Kernel::default())
+    }
+
+    /// The box circumscribing `pb` with `k`'s opt-in fixes
+    /// (`SmExtent3d(const SmPseudoBox&)`); see [`Extent3d::circumscribe_with`].
+    #[must_use]
+    pub fn from_pseudo_box_with(pb: &PseudoBox, k: &Kernel) -> Self {
         let mut e = Self::new();
-        e.circumscribe(pb);
+        e.circumscribe_with(pb, k);
         e
     }
 
+    /// Becomes the box circumscribing `pb` with a default kernel (`Circumscribe`); see
+    /// [`Extent3d::circumscribe_with`].
+    pub fn circumscribe(&mut self, pb: &PseudoBox) {
+        self.circumscribe_with(pb, &Kernel::default());
+    }
+
     /// Becomes the box circumscribing `pb` (`Circumscribe`): the same size for an
     /// axis-aligned pseudo box (copied from its intervals, so partially unbounded
     /// pseudo boxes stay partially unbounded), fully unbounded for an unaligned
-    /// unbounded one, else the bounding box of its corners.
-    pub fn circumscribe(&mut self, pb: &PseudoBox) {
+    /// unbounded one, else the bounding box of its corners. With
+    /// `Kernel::fixes.circumscribe_axis_map_by_basis` the axis-aligned map is read by
+    /// basis (WR-5); otherwise its entries are read as bases, as upstream does.
+    pub fn circumscribe_with(&mut self, pb: &PseudoBox, k: &Kernel) {
         // low work - axis aligned PseudoBox
         if let Some(axis_map) = pb.is_axis_aligned() {
-            for (ii, &m) in axis_map.iter().enumerate() {
-                // upstream indexes the basis with the axis map entry of axis `ii`
-                let lx = (m.unsigned_abs() as usize).wrapping_sub(1);
-                let ivl = pb.interval(lx).unwrap_or_default();
-                let (lo, hi) = if m > 0 {
-                    (ivl.min(), ivl.max())
-                } else {
-                    (-ivl.max(), -ivl.min())
-                };
-                if let Ok(c) = self.min.component_mut(ii) {
-                    *c = lo;
+            if k.fixes.circumscribe_axis_map_by_basis {
+                // Basis `ii` is parallel to axis `|axis_map[ii]| - 1`: its interval
+                // belongs on that axis, negated when the basis is reversed.
+                for (ii, &m) in axis_map.iter().enumerate() {
+                    let ivl = pb.interval(ii).unwrap_or_default();
+                    let (lo, hi) = if m < 0 {
+                        (-ivl.max(), -ivl.min())
+                    } else {
+                        (ivl.min(), ivl.max())
+                    };
+                    let axis = (m.unsigned_abs() as usize).wrapping_sub(1);
+                    if let Ok(c) = self.min.component_mut(axis) {
+                        *c = lo;
+                    }
+                    if let Ok(c) = self.max.component_mut(axis) {
+                        *c = hi;
+                    }
                 }
-                if let Ok(c) = self.max.component_mut(ii) {
-                    *c = hi;
+            } else {
+                for (ii, &m) in axis_map.iter().enumerate() {
+                    // upstream indexes the basis with the axis map entry of axis `ii`
+                    let lx = (m.unsigned_abs() as usize).wrapping_sub(1);
+                    let ivl = pb.interval(lx).unwrap_or_default();
+                    let (lo, hi) = if m > 0 {
+                        (ivl.min(), ivl.max())
+                    } else {
+                        (-ivl.max(), -ivl.min())
+                    };
+                    if let Ok(c) = self.min.component_mut(ii) {
+                        *c = lo;
+                    }
+                    if let Ok(c) = self.max.component_mut(ii) {
+                        *c = hi;
+                    }
                 }
             }
             return;

crates/solidcore/src/geom/pseudo_box.rs

@@ -1221,6 +1221,7 @@ fn add_rotated_edge_extrema(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::kernel::Kernel;
 
     fn rotated_basis() -> [Vec3; 3] {
         let s = std::f64::consts::FRAC_1_SQRT_2;
@@ -1295,7 +1296,8 @@ mod tests {
     }
 
     /// Upstream quirk kept by `Extent3d::circumscribe`: the basis -> axis map is
-    /// read as axis -> basis.
+    /// read as axis -> basis. `Extent3d::circumscribe_with` reads it by basis behind
+    /// `Kernel::fixes.circumscribe_axis_map_by_basis` (WR-5).
     #[test]
     fn circumscribe_axis_cycle_quirk() {
         let ivl = Extent1d::from_min_max;
@@ -1304,9 +1306,39 @@ mod tests {
             [ivl(1.0, 2.0), ivl(3.0, 5.0), ivl(-7.0, -6.0)],
         );
         assert_eq!(pb.is_axis_aligned(), Some([2, 3, 1]));
-        let e = Extent3d::from_pseudo_box(&pb);
-        assert_eq!(e.min(), Vec3::new(3.0, -7.0, 1.0));
-        assert_eq!(e.max(), Vec3::new(5.0, -6.0, 2.0));
+
+        // Pinned default: the map is read as axis -> basis, so basis 0's y interval
+        // lands on x, basis 1's z interval on y and basis 2's x interval on z.
+        let pinned = Extent3d::from_pseudo_box(&pb);
+        assert_eq!(pinned.min(), Vec3::new(3.0, -7.0, 1.0));
+        assert_eq!(pinned.max(), Vec3::new(5.0, -6.0, 2.0));
+
+        let mut k = Kernel::default();
+        k.fixes.circumscribe_axis_map_by_basis = true;
+        let fixed = Extent3d::from_pseudo_box_with(&pb, &k);
+        assert_eq!(fixed.min(), Vec3::new(-7.0, 1.0, 3.0));
+        assert_eq!(fixed.max(), Vec3::new(-6.0, 2.0, 5.0));
+
+        // Controls: an identity basis and a pure sign flip read the same either way.
+        for (basis, intervals, map) in [
+            (
+                [Vec3::X, Vec3::Y, Vec3::Z],
+                [ivl(1.0, 2.0), ivl(3.0, 5.0), ivl(-7.0, -6.0)],
+                [1, 2, 3],
+            ),
+            (
+                [-Vec3::X, Vec3::Y, -Vec3::Z],
+                [ivl(1.0, 2.0), ivl(3.0, 5.0), ivl(-7.0, -6.0)],
+                [-1, 2, -3],
+            ),
+        ] {
+            let control = PseudoBox::from_basis_intervals(basis, intervals);
+            assert_eq!(control.is_axis_aligned(), Some(map));
+            let pinned = Extent3d::from_pseudo_box(&control);
+            let fixed = Extent3d::from_pseudo_box_with(&control, &k);
+            assert_eq!(fixed.min(), pinned.min());
+            assert_eq!(fixed.max(), pinned.max());
+        }
     }
 
     /// A singular (uninitialised) basis with huge intervals reaches the failing

crates/solidcore/src/kernel.rs

@@ -305,6 +305,13 @@ pub struct UpstreamFixes {
     /// Volume line-domain checks using `SmExtent3d::ContainsLineSeg3d` require
     /// both endpoints inside the natural parameter box instead of either (WR-3).
     pub extent3d_contains_both_segment_endpoints: bool,
+    /// `SmExtent3d::Circumscribe` reads `SmPseudoBox::IsAxisAligned`'s map by basis:
+    /// basis `ii`'s interval lands on axis `|map[ii]| - 1`, negated when the basis is
+    /// reversed, so a basis that is a non-involutive permutation of the axes (e.g.
+    /// `y, z, x`) keeps its intervals on their own axes. Upstream indexes the basis
+    /// with the axis map entry of axis `ii`, which is right for swaps and sign flips
+    /// but cycles a 3-cycle (WR-5). Off by default for pinned C++ parity.
+    pub circumscribe_axis_map_by_basis: bool,
     /// Perspective culling projects each sphere's own radius offsets about its center (WR-608, #525).
     pub perspective_sphere_offsets: bool,
     /// `SmCone::RebuildSTEPFromNURBParameters` gives a cylinder the mean of near-equal radii (semi-angle 0) (WR-385, #713).

plan/upstream-issues/WR-5--addendum-2026-10-31-opt-in-axis-map-by-basis.md

@@ -0,0 +1,27 @@
+Updates: WR-5
+
+#### WR-5 (update, 2026-10-31). The axis map is read by basis behind an opt-in flag
+
+- **What changed:** `SmExtent3d::Circumscribe`'s axis-aligned branch takes the interval
+  at `|alAxisMap[ii]| - 1` and writes it to component `ii`. `SmPseudoBox::IsAxisAligned`
+  fills `alAxisMap[ii]` for basis `ii` with the axis that basis is parallel to, so basis
+  `ii`'s interval belongs on axis `|alAxisMap[ii]| - 1`, negated when the entry is
+  negative.
+- **Trigger:** basis `(y, z, x)` with intervals `[1,2]`, `[3,5]`, `[-7,-6]`: pinned gives
+  min `(3, -7, 1)` and max `(5, -6, 2)`; reading the map by basis gives min
+  `(-7, 1, 3)` and max `(-6, 2, 5)`.
+- **Confirmed by the fork:** the map is read by basis. The doc comment at
+  `SmPseudoBox.cpp:1737-1776` (`alAxisMap[0] = index+1 of basis parallel to X`) describes
+  axis -> basis, but the code fills `alAxisMap[ii]` for basis `ii`; the code is
+  authoritative. Swaps and sign flips agree either way; only a non-involutive
+  permutation such as the 3-cycle above differs.
+- **Opt-in:** `Kernel::fixes.circumscribe_axis_map_by_basis`, default `false` (pinned C++
+  parity). `Extent3d::circumscribe_with` and `Extent3d::from_pseudo_box_with` take the
+  kernel and read the map by basis when it is set; `Extent3d::circumscribe` and
+  `Extent3d::from_pseudo_box` delegate with `Kernel::default()` and are unchanged. Test
+  `geom::pseudo_box::tests::circumscribe_axis_cycle_quirk` asserts the pinned result, the
+  corrected result, and identity and sign-flip controls.
+- **Rust port:** still reproduces the pinned result by default; no difftest change on the
+  default path.
+
+addendum_options: Kernel::fixes.circumscribe_axis_map_by_basis