Updated C# and Delphi code with bugfixed IsCollinear function (#835)

CPP/Clipper2Lib/include/clipper2/clipper.core.h

@@ -664,30 +664,27 @@ namespace Clipper2Lib
     return (x > 0) - (x < 0); 
   }
 
-  struct MultiplicationResult
+  struct MultiplyUInt64Result
   {
     const uint64_t result = 0;
     const uint64_t carry = 0;
 
-    bool operator==(const MultiplicationResult& other) const
+    bool operator==(const MultiplyUInt64Result& other) const
     {
       return result == other.result && carry == other.carry;
     };
   };
 
-  inline MultiplicationResult Multiply(uint64_t a, uint64_t b) // #834
+  inline MultiplyUInt64Result Multiply(uint64_t a, uint64_t b) // #834, #835
   {
     const auto lo = [](uint64_t x) { return x & 0xFFFFFFFF; };
     const auto hi = [](uint64_t x) { return x >> 32; };
 
-    // https://stackoverflow.com/a/1815371/1158913
     const uint64_t x1 = lo(a) * lo(b);
     const uint64_t x2 = hi(a) * lo(b) + hi(x1);
     const uint64_t x3 = lo(a) * hi(b) + lo(x2);
-    const uint64_t x4 = hi(a) * hi(b) + hi(x2) + hi(x3);
-
     const uint64_t result = lo(x3) << 32 | lo(x1);
-    const uint64_t carry  = x4;
+    const uint64_t carry = hi(a) * hi(b) + hi(x2) + hi(x3);
 
     return { result, carry };
   }

CSharp/Clipper2Lib/Clipper.Core.cs

@@ -1,6 +1,6 @@
 /*******************************************************************************
 * Author    :  Angus Johnson                                                   *
-* Date      :  12 May 2024                                                     *
+* Date      :  13 May 2024                                                     *
 * Website   :  http://www.angusj.com                                           *
 * Copyright :  Angus Johnson 2010-2024                                         *
 * Purpose   :  Core structures and functions for the Clipper Library           *
@@ -616,20 +616,22 @@ internal static int TriSign(long x) // returns 0, 1 or -1
       else return 0;
     }
 
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    internal static ulong CalcOverflowCarry(ulong a, ulong b) // #834
+    public struct MultiplyUInt64Result
     {
-      // given aLo = (a & 0xFFFFFFFF) and
-      // aHi = (a & 0xFFFFFFFF00000000) and similarly with b, then
-      // a * b == (aHi + aLo) * (bHi + bLo)
-      // a * b == (aHi * bHi) + (aHi * bLo) + (aLo * bHi) + (aLo * bLo)
+      public ulong lo64;
+      public ulong hi64;
+    }
 
-      ulong aLo = a & 0xFFFFFFFF;
-      ulong aHi = a >> 32;
-      ulong bLo = b & 0xFFFFFFFF;
-      ulong bHi = b >> 32;
-      // integer overflow of multiplying the unsigned 64bits a and b ==>
-      return aHi * bHi + ((aHi * bLo) >> 32) + ((bHi * aLo) >> 32);
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static MultiplyUInt64Result MultiplyUInt64(ulong a, ulong b) // #834,#835
+    {
+      ulong x1 = (a & 0xFFFFFFFF) * (b & 0xFFFFFFFF);
+      ulong x2 = (a >> 32) * (b & 0xFFFFFFFF) + (x1 >> 32);
+      ulong x3 = (a & 0xFFFFFFFF) * (b >> 32) + (x2 & 0xFFFFFFFF);
+      MultiplyUInt64Result result; 
+      result.lo64 = (x3 & 0xFFFFFFFF) << 32 | (x1 & 0xFFFFFFFF);
+      result.hi64 = (a >> 32) * (b >> 32) + (x2 >> 32) + (x3 >> 32);
+      return result;
     }
 
     // returns true if (and only if) a * b == c * d
@@ -640,18 +642,15 @@ internal static bool ProductsAreEqual(long a, long b, long c, long d)
       ulong absB = (ulong) Math.Abs(b);
       ulong absC = (ulong) Math.Abs(c);
       ulong absD = (ulong) Math.Abs(d);
-      // the multiplications here can potentially overflow, but
-      // any overflows will be compared using CalcOverflowCarry()
-      ulong abs_ab = absA * absB;
-      ulong abs_cd = absC * absD;
+
+      MultiplyUInt64Result mul_ab = MultiplyUInt64(absA, absB);
+      MultiplyUInt64Result mul_cd = MultiplyUInt64(absC, absD);
 
       // nb: it's important to differentiate 0 values here from other values
       int sign_ab = TriSign(a) * TriSign(b);
       int sign_cd = TriSign(c) * TriSign(d);
 
-      ulong carry_ab = CalcOverflowCarry(absA, absB);
-      ulong carry_cd = CalcOverflowCarry(absC, absD);
-      return abs_ab == abs_cd && sign_ab == sign_cd && carry_ab == carry_cd;
+      return mul_ab.lo64 == mul_cd.lo64 && mul_ab.hi64 == mul_cd.hi64 && sign_ab == sign_cd;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]

Delphi/Clipper2Lib/Clipper.Core.pas

@@ -2,7 +2,7 @@
 
 (*******************************************************************************
 * Author    :  Angus Johnson                                                   *
-* Date      :  12 May 2024                                                     *
+* Date      :  13 May 2024                                                     *
 * Website   :  http://www.angusj.com                                           *
 * Copyright :  Angus Johnson 2010-2024                                         *
 * Purpose   :  Core Clipper Library module                                     *
@@ -1872,30 +1872,29 @@ function TriSign(val: Int64): integer; // returns 0, 1 or -1
 end;
 //------------------------------------------------------------------------------
 
-function CalcOverflowCarry(a, b: UInt64): UInt64; // #834
+type
+  TMultiplyUInt64Result = record
+    lo64: UInt64;
+    hi64 : UInt64;
+  end;
+
+function MultiplyUInt64(a, b: UInt64): TMultiplyUInt64Result; // #834, #835
 {$IFDEF INLINING} inline; {$ENDIF}
 var
-  aLo, aHi, bLo, bHi: UInt64;
+  x1, x2, x3: UInt64;
 begin
-  // given aLo = (a and $FFFFFFFF) and
-  // aHi = (a and $FFFFFFFF00000000) and similarly with b, then
-  // a * b == (aHi + aLo) * (bHi + bLo)
-  // a * b == (aHi * bHi) + (aHi * bLo) + (aLo * bHi) + (aLo * bLo)
-  aLo := a and $FFFFFFFF;
-  aHi := a shr 32;  // this avoids multiple shifts
-  bLo := b and $FFFFFFFF;
-  bHi := b shr 32;
-  //integer overflow of multiplying the unsigned 64bits a and b ==>
-  Result := (aHi * bHi) + ((aHi * bLo) shr 32) + ((bHi * aLo) shr 32);
+  x1 := (a and $FFFFFFFF) * (b and $FFFFFFFF);
+  x2 := (a shr 32) * (b and $FFFFFFFF) + (x1 shr 32);
+  x3 := (a and $FFFFFFFF) * (b shr 32) + (x2 and $FFFFFFFF);
+  Result.lo64 := ((x3 and $FFFFFFFF) shl 32) or (x1 and $FFFFFFFF);
+  Result.hi64 := hi(a shr 32) * (b shr 32) + (x2 shr 32) + (x3 shr 32);
 end;
 //------------------------------------------------------------------------------
 
-{$OVERFLOWCHECKS OFF}
 function ProductsAreEqual(a, b, c, d: Int64): Boolean;
 var
   absA,absB,absC,absD: UInt64;
-  absAB, absCD       : UInt64;
-  carryAB, carryCD   : UInt64;
+  absAB, absCD       : TMultiplyUInt64Result;
   signAB, signCD     : integer;
 begin
   // nb: unsigned values will be needed for CalcOverflowCarry()
@@ -1904,20 +1903,16 @@ function ProductsAreEqual(a, b, c, d: Int64): Boolean;
   absC := UInt64(Abs(c));
   absD := UInt64(Abs(d));
 
-  // the multiplications here can potentially overflow, but
-  // any overflows will be compared using CalcOverflowCarry()
-  absAB := absA * absB;
-  absCD := absC * absD;
+  absAB := MultiplyUInt64(absA, absB);
+  absCD := MultiplyUInt64(absC, absD);
 
   // nb: it's important to differentiate 0 values here from other values
   signAB := TriSign(a) * TriSign(b);
   signCD := TriSign(c) * TriSign(d);
 
-  carryAB := CalcOverflowCarry(absA, absB);
-  carryCD := CalcOverflowCarry(absC, absD);
-  Result := (absAB = absCD) and (signAB = signCD) and (carryAB = carryCD);
+  Result := (absAB.lo64 = absCD.lo64) and
+    (absAB.hi64 = absCD.hi64) and (signAB = signCD);
 end;
-{$OVERFLOWCHECKS ON}
 //------------------------------------------------------------------------------
 
 function IsCollinear(const pt1, sharedPt, pt2: TPoint64): Boolean;