Fix bugs in fixed division

src/fixed.h

@@ -149,34 +149,39 @@ class fixedf {
 	}
 	friend fixedf operator/(const fixedf &a, const fixedf &b)
 	{
-		// 128-bit divided by 64-bit, to make sure high bits are not lost
-		Sint64 quotient_hi = a.v >> (64 - FRAC);
-		Uint64 quotient_lo = a.v << FRAC;
-		Sint64 d = b.v;
-		int isneg = 0;
-		Sint64 remainder = 0;
-
-		if (d < 0) {
-			d = -d;
-			isneg = 1;
+		Uint64 abs_a = a.v, abs_b = b.v;
+		bool is_neg = false;
+		if (a.v < 0) {
+			abs_a = -abs_a;
+			is_neg = !is_neg;
 		}
-
+		if (b.v < 0) {
+			abs_b = -abs_b;
+			is_neg = !is_neg;
+		}
+		// 128-bit divided by 64-bit, to make sure high bits are not lost
+		Uint64 quotient_hi = abs_a >> (64 - FRAC);
+		Uint64 quotient_lo = abs_a << FRAC;
+		Uint64 remainder = 0;
 		for (int i = 0; i < 128; i++) {
 			Uint64 sbit = (Uint64(1) << 63) & quotient_hi;
+			// The most significant bit of remainder is 0 because abs_b <= (Uint64(1) << 63)
 			remainder <<= 1;
-			if (sbit) remainder |= 1;
+			if (sbit) {
+				remainder |= 1;
+			}
 			// shift quotient left 1
 			{
 				quotient_hi <<= 1;
 				if (quotient_lo & (Uint64(1) << 63)) quotient_hi |= 1;
 				quotient_lo <<= 1;
 			}
-			if (remainder >= d) {
-				remainder -= d;
+			if (remainder >= abs_b) {
+				remainder -= abs_b;
 				quotient_lo |= 1;
 			}
 		}
-		return (isneg ? -Sint64(quotient_lo) : quotient_lo);
+		return is_neg ? -quotient_lo : quotient_lo;
 	}
 	friend bool operator==(const fixedf &a, const fixedf &b) { return a.v == b.v; }
 	friend bool operator!=(const fixedf &a, const fixedf &b) { return a.v != b.v; }

src/test/TestFixed.cpp

@@ -0,0 +1,33 @@
+#include <limits>
+#include "doctest.h"
+#include "fixed.h"
+
+TEST_CASE("Fixed")
+{
+	SUBCASE("operator ==")
+	{
+		// 42.0 == 42.0
+		CHECK(fixed(42, 1) == fixed(42, 1));
+		// 123.0 != 321.0
+		CHECK(fixed(123, 1) != fixed(321, 1));
+		// 7.0 == 7.0
+		CHECK(fixed(0x7'00000000) == fixed(7, 1));
+	}
+
+	SUBCASE("operator /")
+	{
+		// 3.0 / 3.0 == 1.0
+		CHECK(fixed(3, 1) / fixed(3, 1) == fixed(1, 1));
+		// -3.0 / 3.0 == -1.0
+		// There used to be a bug causing this to produce an incorrect result
+		CHECK(fixed(-3'00000000) / fixed(3, 1) == fixed(-1'00000000));
+		// 0x40000000.00000000 / 0x40000000.00000001 == 0x00000000.ffffffff (because we round down)
+		// There used to be a bug causing this to produce an incorrect result
+		CHECK(fixed(0x40000000'00000000) / fixed(0x40000000'00000001) == fixed(0x00000000'ffffffff));
+		// fixed(INT_MIN) / fixed(INT_MIN) used to trigger a signed int overflow
+		Sint64 min = std::numeric_limits<Sint64>::min();
+		CHECK(fixed(min) / fixed(min) == fixed(1, 1));
+		// fixed(INT_MIN) / -1.0 used to trigger a signed int overflow
+		CHECK(fixed(min) / fixed(-1'00000000) == fixed(min) / fixed(-1'00000000));
+	}
+}