From 07cabaddd8541be322ecc555f186c1d867b506d3 Mon Sep 17 00:00:00 2001 From: angusj Date: Mon, 13 May 2024 15:23:21 +1000 Subject: [PATCH] Updated C# and Delphi code with bugfixed IsCollinear function (#835) --- .../include/clipper2/clipper.core.h | 11 ++--- CSharp/Clipper2Lib/Clipper.Core.cs | 39 ++++++++--------- Delphi/Clipper2Lib/Clipper.Core.pas | 43 ++++++++----------- 3 files changed, 42 insertions(+), 51 deletions(-) diff --git a/CPP/Clipper2Lib/include/clipper2/clipper.core.h b/CPP/Clipper2Lib/include/clipper2/clipper.core.h index 0d82b5c6..4fe8a8cb 100644 --- a/CPP/Clipper2Lib/include/clipper2/clipper.core.h +++ b/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 }; } diff --git a/CSharp/Clipper2Lib/Clipper.Core.cs b/CSharp/Clipper2Lib/Clipper.Core.cs index a4ba38b9..6358aef8 100644 --- a/CSharp/Clipper2Lib/Clipper.Core.cs +++ b/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)] diff --git a/Delphi/Clipper2Lib/Clipper.Core.pas b/Delphi/Clipper2Lib/Clipper.Core.pas index 723607e6..a6ee145c 100644 --- a/Delphi/Clipper2Lib/Clipper.Core.pas +++ b/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;