Skip to content

S3: Frame3<T> + KinematicChain<T>; Pose3 is a wire format (Drawing 2.0.0.72, epic-129) - #5

Merged
rafalmaciag merged 6 commits into
masterfrom
feature/epic-129-s3-frame3
Oct 24, 2026
Merged

rafalmaciag merged 6 commits into
masterfrom
feature/epic-129-s3-frame3

Conversation

@rafalmaciag

@rafalmaciag rafalmaciag commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Epic-129 (Matrix), slice S3 "one pose algebra" — the Drawing half. Design: docs/epics/epic-129-matrix/design.md § "Drawing: the kinematic chain (feature 001)"; ADR-11 (major version, no forwarding members). Merging with release:major produces 2.0.0.72; the lead publishes 2.0.0.72-preview.1 from this branch head.

API as built

namespace ModelingEvolution.Drawing;

public readonly record struct Frame3<T>(Point3<T> Position, Quaternion<T> Rotation)   // T constrained as Pose3<T>
{
    public static Frame3<T> Identity { get; }
    public Frame3<T> Multiply(in Frame3<T> child);                                    // this · child
    public static Frame3<T> operator *(in Frame3<T> a, in Frame3<T> b);
    public Frame3<T> Inverse();
    public Point3<T> TransformPoint(Point3<T> p);
    public Vector3<T> TransformVector(Vector3<T> v);
    public static Frame3<T> Interpolate(in Frame3<T> a, in Frame3<T> b, T t);          // lerp + slerp (was Pose3.Lerp)
    public static (T Position, T Angle) Distance(in Frame3<T> a, in Frame3<T> b);      // |Δp|, 2·acos(|qa·qb|) rad
    public static Frame3<T> FromSurface(Point3<T> a, Point3<T> b, Point3<T> c);        // moved from Pose3
    public static Frame3<T> FromSurface(Point3<T> a, Point3<T> b, Point3<T> c, Point3<T> h);
    public static Frame3<T> FromSurface(ReadOnlySpan<Point3<T>> points);
    public static Frame3<T> FromPose3(Pose3<T> p, T lengthScale);                      // ZYX degrees → quaternion
    public Pose3<T> ToPose3(T lengthScale);
}

// added to Quaternion<T>
public static Quaternion<T> FromRpy(T roll, T pitch, T yaw);        // Rz·Ry·Rx, radians (Rotation3.ToQuaternion now calls it)
public static Quaternion<T> FromTo(Vector3<T> from, Vector3<T> to);  // shortest arc; π about a perpendicular axis when opposite
public static Quaternion<T> FromAxes(Vector3<T> x, Vector3<T> y, Vector3<T> z);

// pose maths on the path types takes a Frame3 (review item A)
public Polyline3<T>   Polyline3<T>.Transform(in Frame3<T> frame);    // frame.TransformPoint per point
public PosePath3<T>   PosePath3<T>.Transform(in Frame3<T> frame);    // frame · FromPose3(p, 1) → ToPose3(1) per element
public Trajectory3<T> Trajectory3<T>.Transform(in Frame3<T> frame);  // same, timestamps kept
public Frame3<T>      Triangle3<T>.ToFrame();                        // (Centroid, FromSurface(a, b, c).Rotation); was Pose3 ToPose()
public PosePath3Builder<T> PosePath3Builder<T>.Step(in Frame3<T> step);  // previous · step; replaces Delta (Euler components added)

// Q7: one Shepperd, one Euler formula each way, matrix bridge through the quaternion
public static Quaternion<T> Quaternion<T>.FromMatrix3x3(in Matrix3x3<T> m);  // Shepperd, normalized; FromAxes(x,y,z) = FromMatrix3x3(FromColumns(x,y,z))
public Matrix3x3<T>         Quaternion<T>.ToMatrix3x3();
public static Matrix3x3<T>  Matrix3x3<T>.RotationZYX(rx, ry, rz);          // = Quaternion.FromRpy(rad...).ToMatrix3x3()
public (Degree<T>, Degree<T>, Degree<T>) Matrix3x3<T>.ToEulerZYX();        // = Rotation3.FromQuaternion(Quaternion.FromMatrix3x3(this))
// deleted: Matrix3x3<T>.ToQuaternion() (the second Shepperd; use Quaternion.FromMatrix3x3)
public static Matrix3x3<T>  Matrix3x3<T>.SimilarityFromVectors(source, target);  // = |t|/|s| · Quaternion.FromTo(s, t).ToMatrix3x3(); zero source or target throws

// Vector3<T> (round 3): no direction → Euler shortcut; Vector3 and Matrix3x3 take or return no Rotation3/Pose3 (EulerSurfaceTests)
// deleted: Vector3<T>.RotationTo(Vector3<T>)                     → Quaternion<T>.FromTo(from, to)
//          explicit operator Rotation3<T>(Vector3<T>)  (+Z → v)   → Quaternion<T>.FromTo(Vector3<T>.EZ, v)
//          operator +(Vector3<T>, Rotation3<T>) → Pose3<T>        → Pose3<T>.From(v, r) (needed for the surface assert)
//          operator +(Point3<T>, Rotation3<T>) → Pose3<T>         → Pose3<T>.From(p, r) (round 4; Point3 is in EulerSurfaceTests too)
// Quaternion<T>.FromTo, opposite directions (documented, round 4): half-turn about normalize(EX × from), or normalize(EY × from)
//          when from is along X; +X → −X turns about −Z, +Z → −Z about −Y.
// Rotation3<T>.FromQuaternion (round 4): ry = atan2(sin, cos); gimbal branch (rz = 0) only when cos ry < 1e-9; exact for every ry.

// Rotation3<T>: wire/edit only (pinned by Rotation3SurfaceTests)
//   kept:    ctor(Degree, Degree, Degree), FromDegrees, FromRadians, Identity, IsIdentity, Rx/Ry/Rz (get/init), ToRadians,
//            GetEnumerator/Enumerator (collection literals), ToQuaternion (= Quaternion.FromRpy), FromQuaternion,
//            FromMatrix3x3 (= FromQuaternion(Quaternion.FromMatrix3x3(m))), Parse, TryParse, ToString,
//            both tuple conversions, Equals ×2, GetHashCode, ==, !=, Truncating<U>()
//   deleted: Combine, operator +, Inverse, unary operator -, Slerp, Rotate(Vector3), Rotate(Point3), ToMatrix3x3,
//            Negate, explicit operator Vector3 (it called Rotate)
// also deleted: Polyline3<T>.Transform(Rotation3<T>) (use Transform(in Frame3<T>)), PosePath3Builder<T>.Delta

namespace ModelingEvolution.Drawing.Kinematics;

public enum JointType : byte { Revolute, Prismatic }
public readonly record struct Link(string Name, int Parent);
public readonly record struct Joint<T>(string Name, JointType Type, int Child, Frame3<T> Origin, Vector3<T> Axis,
                                       T Min, T Max, T MaxVelocity, T? MaxAcceleration);   // T : struct, …
public readonly record struct ChainFrame<T>(string Name, int Link, Frame3<T> InLink);

public sealed class KinematicChain<T>                                                     // T : struct, …
{
    public KinematicChain(ImmutableArray<Link> links, ImmutableArray<Joint<T>> joints, ImmutableArray<ChainFrame<T>> frames);
    public ImmutableArray<Link> Links { get; }
    public ImmutableArray<Joint<T>> Joints { get; }
    public ImmutableArray<ChainFrame<T>> Frames { get; }
    public int Dof { get; }
    public int FrameIndex(string name);                                                   // -1 when absent
    public void Forward(ReadOnlySpan<T> q, Span<Frame3<T>> links);                         // links[c] = links[parent] · Origin · J(q)
    public Frame3<T> FramePose(ReadOnlySpan<Frame3<T>> links, int frame);
    public void Jacobian(ReadOnlySpan<Frame3<T>> links, int frame, Span<T> j);             // 6 × Dof, column-major, (v, ω)
}

Constructor validation (ArgumentException, message names the link/joint/frame): link 0 is the only root (Parent == -1); every other link has 0 ≤ Parent < index; unique link names; every non-root link is moved by exactly one joint (Child in [1, Links.Length)); unit axis (±1e-6); Min ≤ Max; frames on existing links with unique names. Forward/FramePose/Jacobian validate span lengths and do not allocate (asserted by ForwardTests.ForwardAndJacobian_DoNotAllocate, 0 bytes over 100 iterations of the 1P+6R chain).

Pose3<T> members deleted / moved (no forwarders, no [Obsolete])

Member Now
TransformPoint, TransformVector deleted → Frame3.TransformPoint / TransformVector
Inverse() deleted → Frame3.Inverse()
Multiply(Pose3), operator *(Pose3, Pose3) deleted → Frame3.Multiply / *
operator +(Pose3, Rotation3) deleted → compose Frame3
operator +/-(Pose3, Vector3) deleted → pose with { Position = pose.Position + v } or a Frame3
Lerp(a, b, t) deleted → Frame3.Interpolate
Distance(a, b) deleted → Frame3.Distance (or Point3.Distance for position only)
FromSurface (3 overloads) + private helpers moved → Frame3.FromSurface

Kept (asserted member-by-member by Pose3SurfaceTests): the 3 constructors, Identity, From(Point3, Rotation3), From(Vector3, Rotation3), property getters (+ init setters of Position/Rotation), GetEnumerator/Enumerator (collection literals), Parse, TryParse, ToString, both tuple conversions, both Deconstruct, equality (Equals ×2, GetHashCode, ==, !=), Truncating<U>().

Drawing's own callers moved

  • PosePath3.Length and Trajectory3Builder.MoveTo (×3): Point3.Distance of the positions (they used the position-only Pose3.Distance).
  • PosePath3.Transform, Trajectory3.Transform: now take in Frame3<T>; frame * Frame3.FromPose3(p, 1) → ToPose3(1) per stored element (wire conversion of the elements only).
  • PosePath3 / Trajectory3 operator +/-(…, Vector3): with { Position = … }.
  • Trajectory3.AtTime: Frame3.Interpolate through FromPose3/ToPose3 (scale 1).
  • Triangle3.ToPose() → Frame3<T> ToFrame(): Frame3.FromSurface(a, b, c).Rotation at the centroid (one construction).
  • Polyline3.Transform: now takes in Frame3<T>; frame.TransformPoint per point.
  • README (root and package): Frame3<T>, KinematicChain<T> and the new quaternion factories documented; the Pose3 row says wire and edit format only.

Test ledger — ModelingEvolution.Drawing.Tests (native, net10.0, Release)

Passed Failed Skipped Total
master 3c711a8 (before) 1633 0 0 1633
8f120e0 (first review) 1692 0 0 1692
5217f0e (after review A) 1695 0 0 1695
5e2f1a3 (after Q7) 1706 0 0 1706
30642b8 (round 3, Vector3 Euler leftovers) 1698 0 0 1698
this branch bd027b8 (round 4, gimbal fix, Point3 operator) 1707 0 0 1707

Review A (5217f0e): +3 tests (Polyline3Tests/PosePath3Tests/Trajectory3Tests.Transform_Frame_*: 90° about Z + (10,0,0) applied to two elements, rotation and time checked); the 2 Triangle3Tests.Triangle3_ToPose_* tests renamed to Triangle3_ToFrame_* and now assert the frame (position at the centroid, rotation equal to Frame3.FromSurface, Z axis = normal, X along A→B). No test folded. Run twice in Release, 1695/0/0 both times.

Q7 (5e2f1a3), 1695 → 1706 (run twice in Release, 1706/0/0 both times):

  • Removed (7) — their only subject is deleted: PosePath3BuilderTests.Delta_BeforeStartAt_Throws, Delta_AppendsTranslationToPreviousPosition, Delta_AppendsRotationToPreviousRotation, Chain_OfMultipleDeltas_AccumulatesCorrectly, Construct_WithZeroInitialCapacity_DoesNotInfiniteLoop, CollectionLiteralSyntax_CompilesAndProducesCorrectResult; Rotation3Tests.Rotation3_Negate_NegatesAngles.
  • Added (18): PosePath3BuilderTests.Step_BeforeStartAt_Throws, Step_ComposesInThePreviousPoseFrame, Step_Chain_FromZeroCapacityAndLiteralStart_Grows (keeps the zero-capacity and literal guards); Rotation3SurfaceTests (9 deleted-name cases + exact public member list); Matrix3x3Tests.QuaternionMatrix_RoundTrip_MatrixQuaternionMatrix_10000Random (1e-12), QuaternionMatrix_FromMatrix3x3_AllShepperdBranches, QuaternionMatrix_ToMatrix3x3_ColumnsAreRotatedAxes, RotationZYX_EqualsMasterClosedForm_10000Random (master's closed form copied into the test, 1e-12), ToEulerZYX_EqualsMasterAngles_OutsideGimbalBand_10000Random (master's atan2/asin extraction, 1e-9°).
  • Rewritten on Quaternion/Frame3, same expected numbers: Rotation3Tests — Rotation3_ToQuaternion_Rotate_{XAxis90_RotatesYToZ, YAxis90_RotatesZToX, ZAxis90_RotatesXToY}, Rotation3_ToQuaternion_FrameRotatesPoint, Rotation3_ToQuaternion_InverseReversesRotation, Rotation3_ToQuaternion_ProductOfTwoRotations, Rotation3_QuaternionSlerp_{AtZero_ReturnsFirst, AtOne_ReturnsSecond, AtHalf_ReturnsMidpoint}, Rotation3_QuaternionConjugate_IsInverse (was UnaryMinus_ReturnsInverse), Rotation3_QuaternionInverse_ComposesToIdentity, the 4 Rotation3_ToVector3_* (cast → ToQuaternion().Rotate(EZ)); TorchRotationTests (all 4; Rotate → ToQuaternion().Rotate, Slerp → a test-local quaternion slerp); Vector3Tests 5 RotationTo_* + 5 (Vector3)rotation cases; Matrix3x3Tests.ToQuaternion_FromQuaternion_RoundTrip ×4, ToQuaternion_Identity_IsUnitQuaternion, Rotation3_QuaternionMatrix_RoundTrip ×3, Rotation3_QuaternionRotate_MatchesMatrix_Transform. The existing RotationZYX_*/ToEulerZYX_* expectations are unchanged and pass.
  • Frame3SurfaceTests/Frame3Tests and KinematicChain.Jacobian: frame.Rotation.Rotate(v) → frame.TransformVector(v) (same maths), so G-19d's \.Rotation\.Rotate\( can be 0 lines; the zero-allocation test still passes.

Round 3 (30642b8), 1706 → 1698 (run twice in Release, 1698/0/0 both times; 371 build warnings before and after):

  • Removed (14) — their only subject is deleted: Vector3Tests.Vector3_RotationTo_{EZtoEX_Rotates90DegreesAroundY, EZtoEY_Rotates90DegreesAroundX, ParallelVectors_ReturnsIdentity, AntiParallelVectors_Rotates180Degrees, ArbitraryVectors_AlignsCorrectly, NonUnitVectors_NormalizesFirst} (each case is covered by QuaternionFactoryTests.FromTo_*: random 1000 pairs incl. non-unit, shortest arc, same, opposite ×4); Vector3Tests.Vector3_ToRotation3_{ZAxisDirection_ReturnsZeroRotation, XAxisDirection, YAxisDirection, ArbitraryDirection_RoundTrips, NonUnitVector_NormalizesFirst, ZeroVector_ReturnsIdentity}; Vector3Tests.Vector3_AddRotation_ReturnsPose3; Matrix3x3SimilarityTests.AntiParallel_HandledByRotationTo (replaced below).
  • Added (6): EulerSurfaceTests.Vector3_HasNoMemberTakingOrReturningRotation3OrPose3, Matrix3x3_HasNoMemberTakingOrReturningRotation3OrPose3 (methods, operators, ctors, properties, fields; public and non-public); Matrix3x3SimilarityTests.AntiParallel_IsAHalfTurnMappingSourceToTarget ×3 (+X, +Z, (0.3, −0.4, 0.5); target = −2·source; asserts M·source = target and trace(M/2) = −1, det = 1, within 1e-12 — not a specific axis; FromTo picks normalize(EX × source), or normalize(EY × source) when source ∥ EX, so +X → −X turns about −Z); Matrix3x3SimilarityTests.ZeroTarget_Throws (was already the behaviour; the doc said "zero matrix", now corrected).
  • Rewritten, same expected numbers: Rotation3Tests.Rotation3_ToVector3_RoundTrips_WithVector3 → Rotation3_FromTo_RoundTrips_WithVector3 (Rotation3.FromQuaternion(Quaternion.FromTo(EZ, dir)), then ToQuaternion().Rotate(EZ) = dir within 1e-6). The other 7 Matrix3x3SimilarityTests are unchanged and pass on the FromTo path.

Round 4 (3f3b720 + bd027b8), 1698 → 1707 (run twice in Release, 1707/0/0 both times; build warnings 371 → 370, the deleted operator had no XML doc):

  • Removed (1): Point3Tests.Point3_AddRotation_ReturnsPose3 (its only subject, operator +(Point3, Rotation3), is deleted).
  • Rewritten (1): Matrix3x3Tests.ToEulerZYX_EqualsMasterAngles_OutsideGimbalBand_10000Random → ToEulerZYX_EqualsMasterAngles_10000Random: ry over the full [−90°, 90°]; master's angles (1e-9°) wherever master was exact (|cos ry| > 1e-6), the same matrix (1e-8) inside master's band.
  • Added (10): Matrix3x3Tests.ToEulerZYX_EqualsMasterNearGimbal_10000Random (|ry| ∈ [89.9°, 90°]; master's angles within 1e-6° where master was exact, otherwise the matrix), RotationZYX_ToEulerZYX_RoundTripsNearGimbal_10000Random (every element within 1e-8), RotationZYX_ToEulerZYX_AtExactly90_KeepsTheMatrixWithRzZero ×2 (1e-12, rz = 0); Frame3EulerTests.FromPose3_ToPose3_KeepsTheRotationNearGimbal_10000Random (positions 1e-9 mm, every rotation element 1e-8; observed worst cases are about 4e-11), FromPose3_ToPose3_AtExactly90_KeepsTheRotationWithRzZero ×4 (ry = ±90°, rz = 0 or not, 1e-12); EulerSurfaceTests.Point3_HasNoMemberTakingOrReturningRotation3OrPose3.

Removed from Pose3Tests (45 → 12): 7 maths tests (TransformPoint ×2, TransformVector, Lerp ×3, Distance), replaced by Frame3Tests; 26 FromSurface tests moved unchanged in substance to Frame3SurfaceTests.
New / moved, by class:

  • Frame3Tests (13) — TS-129-01: identity, associativity, TransformPoint over 10 000 random frames (1e-12 m, 1e-9 rad); Interpolate t=0/1/0.5; Distance; plus the concrete transform cases from Pose3Tests.
  • Frame3EulerTests (4) — TS-129-02: FromPose3(p, 0.001).ToPose3(1000) round trip over 10 000 poses (ry ±89.9°) within 1e-9 mm / 1e-9°; FromRpy == Rz·Ry·Rx within 1e-12 per element over 10 000 angle triples; FromRpy is the rule Rotation3 uses.
  • Frame3SurfaceTests (29) — the 26 moved FromSurface tests + 3 TS-129-01 cases: each overload gives, within 1e-12, the master result converted through FromPose3.
  • QuaternionFactoryTests (10) — FromTo (random, shortest arc, same, opposite ×4, zero throws), FromAxes (10 000 random, identity).
  • Pose3SurfaceTests (11) — TS-129-55: 10 deleted names absent (public and non-public), exact public member list.
  • Kinematics.KinematicChainTests (12) — TS-129-03.
  • Kinematics.ForwardTests (7) — TS-129-04 (2R and 1P vs closed form at 1 000 random q within 1e-12; wrong links/q length throws) + zero-allocation check.
  • Kinematics.JacobianTests (6) — TS-129-05: test.arm6 and 1P+6R vs central differences (h = 1e-7) at 1 000 random q within 1e-6; prismatic column = (1,0,0,0,0,0); base frame gives zeros; wrong lengths throw.

Grep gate (TS-129-48), new grep-gate CI job (.github/scripts/grep-gate.sh, one row per line)

$ git grep -nE '\b(TransformPoint|TransformVector|Inverse|Multiply|Lerp|Distance|FromSurface)\s*[<(]|operator\s*[-+*]' -- Sources/ModelingEvolution.Drawing/Pose3.cs
(0 lines)          # master: 17 lines
$ git grep -nE 'Pose3(<[^>]*>)?\.(Lerp|Distance|FromSurface)\b' -- Sources/
(0 lines)          # master: 40 lines (PosePath3 1, Trajectory3 1, Trajectory3Builder 3, Triangle3 1, package README 2, Pose3Tests 32)
$ git grep -nE 'Rotation3(<[^>]*>)?\.(Combine|Slerp)\b|\.Rotation\.(Rotate|Inverse|Combine)\(' -- Sources/
(0 lines)          # G-19d (Q7; the lead renumbers it in the docs)
$ git grep -nE '…|\bRotationTo\s*\(' -- Sources/
(0 lines)          # G-19d extended in round 3 (planted `x.RotationTo(y)` in Vector3.cs → 1 line, gate fails)

The script fails on any non-zero count (checked by planting a Pose3<double>.Lerp line locally).

Pose3<T> parameters that remain in Sources/ModelingEvolution.Drawing (all wire or edit, none composes poses)

Member Why it is wire/edit
Frame3.FromPose3(Pose3, T lengthScale) the one wire → maths conversion
Pose3 ==/!=/Equals, tuple conversion, TryParse(out), Enumerator(Pose3) (internal) equality, parse and deconstruction of the wire type itself
Pose3JsonConverter.Write(…, Pose3, …) JSON serialization
PosePath3(params Pose3[]), (IList<Pose3>), (ReadOnlyMemory<Pose3>); Waypoint3(Pose3, T time); PoseEventArgs(Pose3, …) constructors of containers whose stored element is the wire Pose3
PosePath3Builder.StartAt(Pose3); Trajectory3Builder.Add(Pose3[, time]), MoveTo(Pose3, Speed[, Corner[, Speed]]) ×3 append a taught pose verbatim to a Pose3 buffer (edit). Note: MoveTo with a Corner blends rotations with Rotation3.Slerp — Euler maths outside Pose3, the follow-up the lead is deciding, not a Pose3 operation
PosePath3Extensions.AdaptToEndpoints(this PosePath3, Pose3 adaptedStart, Pose3 adaptedEnd) edit: the two poses replace the path's endpoints verbatim; the maths is a Matrix3x3 similarity on the mid positions only, rotations are copied, no pose is composed. Borderline — flagged to the lead

Q7: Euler rotation algebra deleted, matrix bridge kept through the quaternion

Rotation3<T> is a wire and edit format like Pose3<T>; rotation maths uses Quaternion<T> or Frame3<T>. The matrix ⇄ Euler edge that the OpenCV and solver code in rw2 and the simulator need stays (Matrix3x3.RotationZYX, ToEulerZYX, Rotation3.FromMatrix3x3), routed through the quaternion so there is one Euler → rotation formula (Quaternion.FromRpy), one rotation → Euler formula (Rotation3.FromQuaternion) and one Shepperd (Quaternion.FromMatrix3x3). Round 3 deletes Vector3.RotationTo and the Vector3 → Rotation3 cast (its own direction → Euler formula); Matrix3x3.SimilarityFromVectors composes Quaternion.FromTo. Numbers: RotationZYX equals master's closed form within 1e-12; ToEulerZYX equals master's angles within 1e-9° wherever master was exact (|cos ry| > 1e-6).

Behaviour note — gimbal fix (round 4, replaces the earlier 3(a) note). On master Rotation3.FromQuaternion took its gimbal branch at |sin ry| ≥ 1 − 1e-6 (|ry| ≳ 89.92°) and forced ry = ±90°, rz = 0, which changed the rotation itself by up to 0.08° (0.01° at 89.99°). This PR routes ToEulerZYX, Rotation3.FromMatrix3x3, Frame3.ToPose3 and PosePath3Builder.Step through it, so it is fixed here: ry = atan2(sin ry, cos ry) with cos ry = √(r00² + r10²); the gimbal branch is taken only when cos ry < 1e-9, and there rz = 0, rx = atan2(−r12, r11), exact for any ry. Every Euler result now reproduces its rotation (all elements within 1e-8 over 10 000 samples with |ry| ∈ [89.9°, 90°], 1e-12 at exactly ±90°). Callers that pass ry within 89.92°..90° on master (e.g. rw2 Vision/PoseConversions.cs:48, NbvCalibrationService.cs:229; simulator Metrology/Core/Se3.cs:41, HandEyeGroundTruth.cs:56) now get the exact rotation back instead of the snapped one; at true gimbal lock the split is still rz = 0.

Line endings (round 4, 3f3b720). Earlier rounds converted 11 files from CRLF (as on master) to LF: both READMEs, Matrix3x3.cs, Polyline3.cs, PosePath3.cs, Trajectory3.cs, Trajectory3Builder.cs, Matrix3x3Tests.cs, Polyline3Tests.cs, PosePath3Tests.cs, Trajectory3Tests.cs. 3f3b720 restores CRLF on exactly those (content-only diff is empty). git diff --stat origin/master...HEAD: 13 510 lines (7 239 + / 6 271 −) before, 4 360 (2 664 + / 1 696 −) after — equal to --ignore-cr-at-eol; 4 542 with round 4's code. New files are LF; master itself mixes LF and CRLF files (e.g. Pose3.cs LF, Matrix3x3.cs CRLF).
Consumer impact (Task 2 count, before this PR): rw2 161 sites in 50 files, simulator 5 in 4, SDK 4 in 2. No call to the deleted Matrix3x3.ToQuaternion() found in the three consumers (rw2's m.ToQuaternion() in Robot.Abb.Simulator is on its own Mat4).
Round 3 count (git grep -n on master: simulator 7c75764, rw2 307b2fdd, SDK 46ff3ac): RotationTo( — simulator 0, rw2 0, SDK 1 site in 1 file (csharp/RocketWelder.SDK.Vision/CameraProjector.cs:84, Vector3d.EZ.RotationTo(rayDir) → Quaternion<double>.FromTo(Vector3d.EZ, rayDir)); (Rotation3<…>) cast on a Vector3 — 0 in all three (aliases checked: Vec3, SimVec3, Vector3d). operator +(Vector3, Rotation3) uses were not counted (no reliable grep for an operator).

test.arm6 (for Matrix.Fixtures to copy; metres, radians; Sources/ModelingEvolution.Drawing.Tests/Kinematics/TestChains.cs)

Links: base(-1), link1(0), link2(1), link3(2), link4(3), link5(4), link6(5). All joints revolute, MaxVelocity 3.14; Origin = xyz + rpy (Quaternion.FromRpy), Axis normalized.

joint child origin xyz origin rpy axis min..max MaxAcceleration
j1 link1 (0, 0, 0.180) (0, 0, 0) (0, 0, 1) ±3.05 10
j2 link2 (0.050, −0.030, 0.120) (π/2, 0, 0.1) (0, 0, 1) ±2.60 10
j3 link3 (0.420, 0.010, 0) (0, 0, 0.2) (0, 0, −1) ±2.80 10
j4 link4 (0.380, −0.020, 0.090) (−π/2, 0.05, 0) (0, 0.6, 0.8) ±3.05 null
j5 link5 (0.010, 0.110, 0.030) (π/2, 0, 0) (1, 0, 0) ±2.10 null
j6 link6 (0, 0.020, 0.095) (0, −π/2, 0.3) (0, 0, 1) ±6.28 null

Frames: base on base (identity); flange on link6, xyz (0, 0, 0.045), rpy (π, 0, 0).
Rail variant (1P+6R): base → carriage by prismatic rail (origin (0, 0, 0.25), axis +X, 0..3 m, MaxVelocity 1, no acceleration), then test.arm6 on the carriage with j1's origin xyz (0, 0.10, 0.230).

Design points decided here

  • Joint<T> and KinematicChain<T> add struct to the Pose3 constraint: without it T? MaxAcceleration is only a nullable annotation (double, not double?) and could not be null. Frame3<T> and ChainFrame<T> keep the Pose3 constraint exactly.
  • Every non-root link has exactly one joint (Dof == Links.Length − 1); fixed offsets are ChainFrames or joint origins (the design has no Fixed joint).
  • Distance returns (Position, Angle); the angle is computed as 2·atan2(|v|, |w|) of qa*·qb, equal to the Rules' 2·acos(|qa·qb|) but well conditioned near 0.
  • Transform keeps its Pose3 parameter — reversed by review A: Transform(in Frame3<T>); callers convert at their edge with Frame3.FromPose3(pose, scale).
  • Pose3.From(...) factories kept as named constructors (not in the fate table either way).

🤖 Generated with Claude Code

https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

- Frame3<T> (position + unit quaternion): Multiply/*, Inverse, TransformPoint,
  TransformVector, Interpolate (slerp), Distance (position, angle),
  FromSurface (3 overloads, moved from Pose3), FromPose3/ToPose3 (the one Euler rule).
- Quaternion<T>.FromRpy (Rz*Ry*Rx), FromTo, FromAxes; Rotation3.ToQuaternion
  now delegates to FromRpy.
- ModelingEvolution.Drawing.Kinematics: JointType, Link, Joint<T>, ChainFrame<T>,
  KinematicChain<T> (Forward, FramePose, Jacobian, FrameIndex, Dof); spans in,
  spans out, no allocation.
- Pose3<T>: TransformPoint, TransformVector, Inverse, Multiply, operator * / + / -,
  Lerp, Distance, FromSurface deleted (no forwarders; breaking, ADR-11 major).
- Drawing's callers (PosePath3, Trajectory3, Trajectory3Builder, Triangle3,
  Polyline3) moved to Frame3 / Point3.
- Tests TS-129-01..05 and TS-129-55; grep-gate CI job with rows G-17, G-18.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8
@rafalmaciag rafalmaciag added the release:major release.yml bump on merge: major label Oct 24, 2026

@rafalmaciag rafalmaciag left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES REQUESTED: one change, decision A. Everything else checks out.

Requested (A). Transform(Pose3<T>) takes the wire type and composes pose · element, which is pose maths. The reason given, "rw2 changes fewer files", is a compatibility argument, and design.md rule 2 ("ALL maths composes Frame3; Pose3 is wire and edit only") and the "no compatibility" rule forbid it. The hidden lengthScale = 1 also makes a unit choice inside the library. Change these:

  • Sources/ModelingEvolution.Drawing/Polyline3.cs:328, PosePath3.cs:156, Trajectory3.cs:211: change the signature to Transform(in Frame3<T> frame) and delete the inner FromPose3(pose, T.One). The elements of PosePath3 and Trajectory3 are still Pose3, so their per-element FromPose3(·,1)/ToPose3(1) stays. Callers convert at their edge with Frame3.FromPose3(pose, 1).
  • Triangle3.cs:87: ToPose() builds geometry, so it belongs with FromSurface, which design.md moved to Frame3. Replace it with Frame3<T> ToFrame() (new Frame3<T>(Centroid, Frame3<T>.FromSurface(_a,_b,_c).Rotation)) and update its test.

Verdicts on the other decisions:

  • B. Confirmed. UR base_link→base, wrist_3→flange and flange→tool0 are frames (ChainFrame on the moving link, with the fixed offsets composed into InLink). A fixed body that carries a mesh, such as base_link_inertia, folds into its parent: the next joint's Origin becomes the fixed offset · Origin. This is lossless. The S7 importer must emit a ChainFrame per folded body so TS-129-06 can still compare it. For a rail with an arm (1P+6R), the arm-mount offset goes into the first arm joint's Origin. Forward and Jacobian stay a single pass or walk (RailArm6 is tested). Architecture page 04 (Link(Frame3 Origin, Joint? Joint)) must follow design.md.
  • C. Fine. T? needs where T : struct to become Nullable<T>. Without it, null cannot be stored for double. Frame3 and ChainFrame have no T?, so keeping the Pose3 constraint is right.
  • D. Fine. 2·atan2(|v|,|w|) equals 2·acos(|qa·qb|) for unit quaternions and is well conditioned near 0. Rotation3.ToQuaternion → FromRpy removes a duplicate. The Pose3.From(...) factories and GetEnumerator/IsIdentity are wire/edit constructors and accessors, not maths. They are acceptable, but they go beyond the kept list in TS-129-55, so that list in the docs needs amending (lead).

Standard checks:

  1. The API matches design.md. Multiply is this·child. Forward computes links[c]=links[parent]·Origin·J(q) in one pass. The Jacobian is 6×Dof column-major (v, ω): revolute a×(p−p_joint), prismatic (a,0). FromRpy = Rz·Ry·Rx. Two additions beyond the sketch are fine: Interpolate and Distance, which the fate table calls for.
  2. I ran the suite in Release twice: 1692/0/0 both times. Every random test is seeded with new Random(129). Sample counts and tolerances match TS-129-01/02/04/05. TS-129-03 errors name the culprit. The reflection test for TS-129-55 is exact.
  3. No [Obsolete] and no forwarding members. Forward, FramePose and Jacobian use spans and ItemRef with no LINQ, closures or arrays. The 0 B test warms up first and measures the right calls.
  4. grep-gate.sh: G-17 finds 0 lines and G-18 finds 0 (40 on master). It exits non-zero on any hit, a new check is one row in ROWS, and .github/scripts/** is in the CI paths filter.
  5. Release build: 0 warnings. TreatWarningsAsErrors is not set anywhere in this repo, before or after this PR. The tree is clean after the tests, apart from gitignored skeleton-svg/ and skeleton-performance-results.md, which existing Skeleton tests already wrote on master. The label release:major is present.

Follow-up (not blocking, lead's call). Rotation3 still has a full Euler rotation algebra: Combine/operator+, Inverse/operator-, Slerp, Rotate. Matrix3x3.RotationZYX/ToEulerZYX is a second ZYX path, and PosePath3Builder.Delta adds Euler angles component by component. design.md's fate table covers only Pose3. Because 2.0 is the breaking window (ADR-11), decide now whether rule 2 extends to Rotation3.

🤖 Generated with Claude Code

https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

Rafal Maciag and others added 3 commits October 24, 2026 18:38
Polyline3/PosePath3/Trajectory3.Transform(Pose3) become Transform(in Frame3):
the pose maths composes Frame3 and the hidden lengthScale=1 conversion of the
argument is gone; PosePath3/Trajectory3 keep the per-element FromPose3/ToPose3
because their elements are wire Pose3. Triangle3.ToPose() becomes ToFrame(),
built from Frame3.FromSurface's rotation at the centroid. One test per
Transform added; Triangle3 tests assert the frame.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8
…uaternion (epic-129)

Rotation3 is wire/edit only: Combine, operator +, Inverse, operator -, Slerp,
Rotate(Vector3), Rotate(Point3), ToMatrix3x3, Negate and the explicit cast to
Vector3 are deleted (Rotation3SurfaceTests pins the kept list).
PosePath3Builder.Delta (Euler components added) is replaced by Step(in Frame3),
composing previous · step. Polyline3.Transform(Rotation3) is deleted.
Trajectory3Builder's corner blend slerps quaternions from FromPose3(·,1).

One Shepperd and one Euler formula each way: Quaternion.FromMatrix3x3 /
ToMatrix3x3 added; FromAxes uses FromMatrix3x3; Matrix3x3.ToQuaternion deleted;
RotationZYX = FromRpy(...).ToMatrix3x3(); ToEulerZYX and
Rotation3.FromMatrix3x3 = Rotation3.FromQuaternion(Quaternion.FromMatrix3x3(m));
Vector3.RotationTo's private Euler extraction goes through FromQuaternion.
Tests pin RotationZYX/ToEulerZYX to master's closed form over 10 000 random
rotations and the matrix -> quaternion -> matrix round trip to 1e-12.
Grep gate row G-19d added.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8
- Vector3<T>.RotationTo, the explicit Vector3 → Rotation3 cast (a second
  direction → Euler formula) and operator +(Vector3, Rotation3) → Pose3 are
  deleted; the rotation between two directions is Quaternion.FromTo.
- Matrix3x3.SimilarityFromVectors = |t|/|s| · Quaternion.FromTo(s, t).ToMatrix3x3().
- EulerSurfaceTests: Vector3 and Matrix3x3 take or return no Rotation3/Pose3.
- grep gate G-19d also rejects \bRotationTo\s*\( in Sources/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

@rafalmaciag rafalmaciag left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES REQUESTED (re-review at 30642b8). Item A is fixed, and the Q7 deletion is clean. One correctness regression remains, plus one hygiene item.

1. Blocking: the one Euler extraction is wrong by up to 0.08° near ry = ±90°.
Rotation3.FromQuaternion switches to its gimbal branch when |sinp| ≥ 1−1e-6, which is |ry| ≳ 89.92°. That branch then forces ry = ±90° and rz = 0. This is not "the same matrix, different rx/rz split": the rotation itself changes. At ry = 89.92° the error is 0.080°, at 89.95° it is 0.05°, and at 89.99° it is 0.01°. I checked this by reproducing the formulas.

The defect was already in FromQuaternion on master. This PR routes Matrix3x3.ToEulerZYX and Rotation3.FromMatrix3x3 through it as well, and master's ToEulerZYX was exact there. So the defect now reaches more callers:

  • ToPose3 and PosePath3Builder.Step, which round-trips through Euler on every step;
  • rw2 Vision/PoseConversions.cs:48 and NbvCalibrationService.cs:229;
  • simulator Metrology/Core/Se3.cs:41 and HandEyeGroundTruth.cs:56.

The tests miss it because TS-129-02 samples ry within ±89.9° and ToEulerZYX_EqualsMasterAngles_OutsideGimbalBand skips the band.

Fix in Sources/ModelingEvolution.Drawing/Rotation3.cs FromQuaternion:

var r00 = T.One - Two * (q.Y * q.Y + q.Z * q.Z);
var r10 = Two * (q.W * q.Z + q.X * q.Y);
var cosp = T.Sqrt(r00 * r00 + r10 * r10);
ry = T.Atan2(sinp, cosp);
if (cosp < T.CreateTruncating(1e-9)) {        // true gimbal only
    rz = T.Zero;
    rx = T.Atan2(-Two * (q.Y * q.Z - q.W * q.X), T.One - Two * (q.X * q.X + q.Z * q.Z)); // R = Ry·Rx, exact for any ry
} else {
    rx = T.Atan2(sinrCosp, cosrCosp);
    rz = T.Atan2(r10, r00);
}

I modelled this in Python on 200 000 random rotations with |ry| ∈ [89.9°, 90°]: the worst rotation error is 3e-6°, which is the limit of the measurement. Add a test of FromPose3→ToPose3 and RotationZYX→ToEulerZYX→RotationZYX over ry ∈ [89.9°, 90°]: every matrix element must agree within 1e-8. With that fix, 3(a) has no behaviour left to document.

2. Hygiene: line endings. 11 files went from CRLF to LF: README.md ×2, Matrix3x3.cs, Polyline3.cs, PosePath3.cs, Trajectory3.cs, Trajectory3Builder.cs and four test files. That inflates the round diff from about 540 to 5 100 lines and breaks blame for those files. Restore the original endings, so that git diff --stat matches --ignore-cr-at-eol.

Verified:

  • Survivors of A. The wire members (FromPose3/ToPose3, JSON, parse, equality) are correct. The containers (PosePath3 ctors and indexers, Waypoint3, PoseEventArgs, Trajectory3.AtTime, builders StartAt/Add/MoveTo) store or emit edit data and do their maths through Frame3. AdaptToEndpoints(start, end) is acceptable: it uses the positions only and copies the taught rotations unchanged. No public member composes a Pose3.
  • Q7.
    • One FromRpy and one extraction remain.
    • FromMatrix3x3 covers all four Shepperd branches, and its matrix→quaternion→matrix round trip passes within 1e-12.
    • RotationZYX keeps master's numbers.
    • The Trajectory3Builder corner blend is the same maths: quaternion slerp, then FromQuaternion.
    • The TransformVector rewrites are the same maths, and the 0 B allocation test passes.
    • EulerSurfaceTests is sound.
  • 3(b), antiparallel FromTo. Acceptable. It is a deterministic half-turn, but the axis differs from master's RotationTo, so SimilarityFromVectors and AdaptToEndpoints give different (equally valid) results for reversed endpoints. State the axis rule in the FromTo XML doc.
  • Grep gate. G-17, G-18 and G-19d each find 0 lines. A planted, tracked hit makes the script exit 1.
  • Ledger. Release, run twice: 1698/0/0 both times. Every removed test covered a deleted member. The Delta zero-capacity and collection-literal tests were merged into Step_Chain_FromZeroCapacityAndLiteralStart_Grows with the same expected numbers. The tree is clean after the tests.
  • Minor, lead's call. Point3.operator +(Point3, Rotation3) → Pose3 survives although its Vector3 twin was deleted. Step composes in the previous pose's frame, where Delta added a world-frame offset; neither rw2 nor the simulator calls Delta.

🤖 Generated with Claude Code

https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

Rafal Maciag and others added 2 commits October 24, 2026 19:00
Content unchanged (git diff --ignore-cr-at-eol is empty); undoes the
CRLF → LF conversion so blame and the PR diff show only real changes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8
…pic-129)

- Rotation3.FromQuaternion: ry = atan2(sin, cos) with cos = sqrt(r00² + r10²);
  gimbal branch only when cos ry < 1e-9 (rz = 0, rx = atan2(-r12, r11), exact
  for any ry). The old branch (|sin ry| >= 1 - 1e-6) forced ry = ±90° and
  changed the rotation by up to 0.08°. Reaches ToEulerZYX, FromMatrix3x3,
  Frame3.ToPose3 and PosePath3Builder.Step.
- Tests over |ry| in [89.9°, 90°] (10 000 seeded) and at exactly ±90°:
  matrices within 1e-8 (1e-12 at ±90°); ToEulerZYX compared with master's
  angles over the full ry range where master was exact.
- Point3 operator +(Point3, Rotation3) → Pose3 deleted (use Pose3.From);
  EulerSurfaceTests also checks Point3.
- Quaternion.FromTo doc states the half-turn axis for opposite directions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

@rafalmaciag rafalmaciag left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE (final check at bd027b8).

  • Euler extraction fixed. Rotation3.FromQuaternion now computes ry = atan2(sinp, sqrt(r00²+r10²)). It takes the gimbal branch only when cosp < 1e-9, and there sets rx = atan2(−r12, r11), rz = 0. This is the formula I modelled last round: worst error 3e-6° over 200 000 rotations with |ry| ∈ [89.9°, 90°].
    • The new tests pass: FromPose3_ToPose3_KeepsTheRotationNearGimbal_10000Random, RotationZYX_ToEulerZYX_RoundTripsNearGimbal_10000Random, ToEulerZYX_EqualsMasterNearGimbal_10000Random and the exact ±90° cases.
  • Line endings restored. git diff origin/master...bd027b8 --stat and --ignore-cr-at-eol agree: 46 files, +2797/−1745. No file's line endings differ from master.
  • Other items fixed:
    • operator +(Point3, Rotation3) is gone, and EulerSurfaceTests now covers Point3.
    • The FromTo XML doc states the antiparallel axis rule (+X→−X about −Z, +Z→−Z about −Y), and it matches the code.
  • Suite and gate. Release run: 1707/0/0. Grep gate G-17, G-18 and G-19d: 0 lines each. Tree clean after the tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_014wH5aL3TZhJNvLqa8YTku8

@rafalmaciag
rafalmaciag merged commit 004df0d into master Oct 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:major release.yml bump on merge: major

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant