From cdf3a1fc9f74cf4adb9a08a564cf1811eeca581c Mon Sep 17 00:00:00 2001 From: AdityaJagtap18 Date: Sun, 6 Sep 2026 18:24:47 +0530 Subject: [PATCH 1/3] Fix Quaternion.rotate()/rotated() computing the inverse rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quaternion.rotate() (and the rotated() wrapper) implemented `conjugate(this) * [v,0] * this` instead of the standard sandwich product `this * [v,0] * conjugate(this)`, so every quaternion rotation in both the 32-bit (vector_math) and 64-bit (vector_math_64) variants rotated vectors by the inverse of the intended rotation. For example, a 90-degree rotation about +Z applied to (1,0,0) must give (0,1,0) by the right-hand rule (and matches Matrix4.rotationZ(pi/2) and Quaternion.asRotationMatrix()), but the old code returned (0,-1,0). Verified by: - Deriving the correct sandwich-product formula by hand and comparing against Matrix4.rotationZ/asRotationMatrix() ground truth. - Adding test/fuzz_invariants_test.dart, a randomized invariant fuzzer covering normalize, inverse round-trips for Matrix2/3/4 and Quaternion, and — the check that surfaced this bug — cross-verifying Quaternion.rotated() against Quaternion.asRotationMatrix() on the same vector. - Correcting the 12 hardcoded expected vectors in quaternion_test.dart's testQuaternionNormalize(), which encoded the old (buggy) rotation direction; new values were independently derived from the standard rotation-matrix formulas. - Running the full vector_math test suite (261 tests) after the fix: all pass. Co-Authored-By: Claude Sonnet 5 --- .../lib/src/vector_math/quaternion.dart | 20 +- .../lib/src/vector_math_64/quaternion.dart | 20 +- .../test/fuzz_invariants_test.dart | 202 ++++++++++++++++++ .../vector_math/test/quaternion_test.dart | 16 +- 4 files changed, 226 insertions(+), 32 deletions(-) create mode 100644 packages/vector_math/test/fuzz_invariants_test.dart diff --git a/packages/vector_math/lib/src/vector_math/quaternion.dart b/packages/vector_math/lib/src/vector_math/quaternion.dart index 3a1c9c9f..edc63894 100644 --- a/packages/vector_math/lib/src/vector_math/quaternion.dart +++ b/packages/vector_math/lib/src/vector_math/quaternion.dart @@ -337,22 +337,18 @@ class Quaternion { /// Rotates [v] by this. Vector3 rotate(Vector3 v) { - // conjugate(this) * [v,0] * this + // this * [v,0] * conjugate(this) final double w = _qStorage[3]; final double z = _qStorage[2]; final double y = _qStorage[1]; final double x = _qStorage[0]; - final tiw = w; - final double tiz = -z; - final double tiy = -y; - final double tix = -x; - final double tx = tiw * v.x + tix * 0.0 + tiy * v.z - tiz * v.y; - final double ty = tiw * v.y + tiy * 0.0 + tiz * v.x - tix * v.z; - final double tz = tiw * v.z + tiz * 0.0 + tix * v.y - tiy * v.x; - final double tw = tiw * 0.0 - tix * v.x - tiy * v.y - tiz * v.z; - final double resultX = tw * x + tx * w + ty * z - tz * y; - final double resultY = tw * y + ty * w + tz * x - tx * z; - final double resultZ = tw * z + tz * w + tx * y - ty * x; + final double tx = w * v.x + y * v.z - z * v.y; + final double ty = w * v.y + z * v.x - x * v.z; + final double tz = w * v.z + x * v.y - y * v.x; + final double tw = -x * v.x - y * v.y - z * v.z; + final double resultX = -tw * x + tx * w - ty * z + tz * y; + final double resultY = -tw * y + ty * w - tz * x + tx * z; + final double resultZ = -tw * z + tz * w - tx * y + ty * x; final Float32List vStorage = v.storage; vStorage[2] = resultZ; vStorage[1] = resultY; diff --git a/packages/vector_math/lib/src/vector_math_64/quaternion.dart b/packages/vector_math/lib/src/vector_math_64/quaternion.dart index a1f72b51..9596834d 100644 --- a/packages/vector_math/lib/src/vector_math_64/quaternion.dart +++ b/packages/vector_math/lib/src/vector_math_64/quaternion.dart @@ -337,22 +337,18 @@ class Quaternion { /// Rotates [v] by this. Vector3 rotate(Vector3 v) { - // conjugate(this) * [v,0] * this + // this * [v,0] * conjugate(this) final double w = _qStorage[3]; final double z = _qStorage[2]; final double y = _qStorage[1]; final double x = _qStorage[0]; - final tiw = w; - final double tiz = -z; - final double tiy = -y; - final double tix = -x; - final double tx = tiw * v.x + tix * 0.0 + tiy * v.z - tiz * v.y; - final double ty = tiw * v.y + tiy * 0.0 + tiz * v.x - tix * v.z; - final double tz = tiw * v.z + tiz * 0.0 + tix * v.y - tiy * v.x; - final double tw = tiw * 0.0 - tix * v.x - tiy * v.y - tiz * v.z; - final double resultX = tw * x + tx * w + ty * z - tz * y; - final double resultY = tw * y + ty * w + tz * x - tx * z; - final double resultZ = tw * z + tz * w + tx * y - ty * x; + final double tx = w * v.x + y * v.z - z * v.y; + final double ty = w * v.y + z * v.x - x * v.z; + final double tz = w * v.z + x * v.y - y * v.x; + final double tw = -x * v.x - y * v.y - z * v.z; + final double resultX = -tw * x + tx * w - ty * z + tz * y; + final double resultY = -tw * y + ty * w - tz * x + tx * z; + final double resultZ = -tw * z + tz * w - tx * y + ty * x; final Float64List vStorage = v.storage; vStorage[2] = resultZ; vStorage[1] = resultY; diff --git a/packages/vector_math/test/fuzz_invariants_test.dart b/packages/vector_math/test/fuzz_invariants_test.dart new file mode 100644 index 00000000..11854753 --- /dev/null +++ b/packages/vector_math/test/fuzz_invariants_test.dart @@ -0,0 +1,202 @@ +// Fuzz-check basic mathematical invariants across vector_math's core +// operations, looking for a genuine correctness bug (not covered by the +// existing hand-written unit tests). +import 'dart:math' as math; + +import 'package:test/test.dart'; +import 'package:vector_math/vector_math_64.dart'; + +final math.Random rng = math.Random(42); + +double rd() => (rng.nextDouble() - 0.5) * 20; + +Vector2 randV2() => Vector2(rd(), rd()); +Vector3 randV3() => Vector3(rd(), rd(), rd()); +Vector4 randV4() => Vector4(rd(), rd(), rd(), rd()); + +Matrix2 randM2() => Matrix2(rd(), rd(), rd(), rd()); +Matrix3 randM3() => Matrix3(rd(), rd(), rd(), rd(), rd(), rd(), rd(), rd(), rd()); +Matrix4 randM4() => Matrix4( + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), + rd(), +); + +Quaternion randQuat() { + final q = Quaternion(rd(), rd(), rd(), rd()); + q.normalize(); + return q; +} + +void main() { + const n = 300; + const eps = 1e-8; + + test('Vector2/3/4 normalize produces unit length (unless zero)', () { + for (var i = 0; i < n; i++) { + final Vector2 v2 = randV2(); + if (v2.length >= 1e-6) { + final Vector2 copy2 = v2.clone()..normalize(); + expect(copy2.length, closeTo(1.0, 1e-6), reason: 'v2=$v2'); + } + final Vector3 v3 = randV3(); + if (v3.length >= 1e-6) { + final Vector3 copy3 = v3.clone()..normalize(); + expect(copy3.length, closeTo(1.0, 1e-6), reason: 'v3=$v3'); + } + final Vector4 v4 = randV4(); + if (v4.length >= 1e-6) { + final Vector4 copy4 = v4.clone()..normalize(); + expect(copy4.length, closeTo(1.0, 1e-6), reason: 'v4=$v4'); + } + } + }); + + test('normalizeInto matches normalize on a copy', () { + for (var i = 0; i < n; i++) { + final Vector3 v3 = randV3(); + if (v3.length >= 1e-6) { + final Vector3 viaCopy = v3.clone()..normalize(); + final out = Vector3.zero(); + v3.normalizeInto(out); + expect(out.x, closeTo(viaCopy.x, eps), reason: 'v3=$v3'); + expect(out.y, closeTo(viaCopy.y, eps), reason: 'v3=$v3'); + expect(out.z, closeTo(viaCopy.z, eps), reason: 'v3=$v3'); + } + + final Vector4 v4 = randV4(); + if (v4.length >= 1e-6) { + final Vector4 viaCopy4 = v4.clone()..normalize(); + final out4 = Vector4.zero(); + v4.normalizeInto(out4); + expect(out4.x, closeTo(viaCopy4.x, eps)); + expect(out4.y, closeTo(viaCopy4.y, eps)); + expect(out4.z, closeTo(viaCopy4.z, eps)); + expect(out4.w, closeTo(viaCopy4.w, eps)); + } + } + }); + + test('Vector3.min/max/mix match componentwise reference impl', () { + for (var i = 0; i < n; i++) { + final Vector3 a = randV3(); + final Vector3 b = randV3(); + final out = Vector3.zero(); + + Vector3.min(a, b, out); + expect(out.x, math.min(a.x, b.x)); + expect(out.y, math.min(a.y, b.y)); + expect(out.z, math.min(a.z, b.z)); + + Vector3.max(a, b, out); + expect(out.x, math.max(a.x, b.x)); + expect(out.y, math.max(a.y, b.y)); + expect(out.z, math.max(a.z, b.z)); + + final double t = rng.nextDouble(); + Vector3.mix(a, b, t, out); + expect(out.x, closeTo(a.x + (b.x - a.x) * t, eps)); + expect(out.y, closeTo(a.y + (b.y - a.y) * t, eps)); + expect(out.z, closeTo(a.z + (b.z - a.z) * t, eps)); + } + }); + + test('Matrix4 * Matrix4.inverted() ~= identity', () { + for (var i = 0; i < n; i++) { + final Matrix4 m = randM4(); + if (m.determinant().abs() < 1e-6) { + continue; + } + final inv = Matrix4.copy(m)..invert(); + final product = (m * inv) as Matrix4; + final identity = Matrix4.identity(); + for (var r = 0; r < 4; r++) { + for (var c = 0; c < 4; c++) { + expect(product.entry(r, c), closeTo(identity.entry(r, c), 1e-4), reason: 'm=$m'); + } + } + } + }); + + test('Matrix3 * Matrix3.inverted() ~= identity', () { + for (var i = 0; i < n; i++) { + final Matrix3 m = randM3(); + if (m.determinant().abs() < 1e-6) { + continue; + } + final inv = Matrix3.copy(m)..invert(); + final product = (m * inv) as Matrix3; + final identity = Matrix3.identity(); + for (var r = 0; r < 3; r++) { + for (var c = 0; c < 3; c++) { + expect(product.entry(r, c), closeTo(identity.entry(r, c), 1e-4), reason: 'm=$m'); + } + } + } + }); + + test('Quaternion * its inverse ~= identity rotation', () { + for (var i = 0; i < n; i++) { + final Quaternion q = randQuat(); + final Quaternion inv = q.inverted(); + final Quaternion product = q * inv; + expect(product.x, closeTo(0.0, 1e-6)); + expect(product.y, closeTo(0.0, 1e-6)); + expect(product.z, closeTo(0.0, 1e-6)); + expect(product.w.abs(), closeTo(1.0, 1e-6)); + } + }); + + test('Quaternion.asRotationMatrix rotates vectors the same as direct quat rotation', () { + for (var i = 0; i < n; i++) { + final Quaternion q = randQuat(); + final Vector3 v = randV3(); + final Vector3 viaQuat = q.rotated(v); + final Matrix3 m = q.asRotationMatrix(); + final Vector3 viaMatrix = m.transformed(v.clone()); + expect(viaMatrix.x, closeTo(viaQuat.x, 1e-6), reason: 'q=$q v=$v'); + expect(viaMatrix.y, closeTo(viaQuat.y, 1e-6), reason: 'q=$q v=$v'); + expect(viaMatrix.z, closeTo(viaQuat.z, 1e-6), reason: 'q=$q v=$v'); + } + }); + + test('cross product is orthogonal to both operands', () { + for (var i = 0; i < n; i++) { + final Vector3 a = randV3(); + final Vector3 b = randV3(); + final Vector3 c = a.cross(b); + expect(c.dot(a), closeTo(0.0, 1e-6), reason: 'a=$a b=$b'); + expect(c.dot(b), closeTo(0.0, 1e-6), reason: 'a=$a b=$b'); + } + }); + + test('Matrix2 * Matrix2.inverted() ~= identity', () { + for (var i = 0; i < n; i++) { + final Matrix2 m = randM2(); + if (m.determinant().abs() < 1e-6) { + continue; + } + final inv = Matrix2.copy(m)..invert(); + final product = (m * inv) as Matrix2; + final identity = Matrix2.identity(); + for (var r = 0; r < 2; r++) { + for (var c = 0; c < 2; c++) { + expect(product.entry(r, c), closeTo(identity.entry(r, c), 1e-4), reason: 'm=$m'); + } + } + } + }); +} diff --git a/packages/vector_math/test/quaternion_test.dart b/packages/vector_math/test/quaternion_test.dart index d3d14400..1f49fc12 100644 --- a/packages/vector_math/test/quaternion_test.dart +++ b/packages/vector_math/test/quaternion_test.dart @@ -143,7 +143,7 @@ void testQuaternionNormalize() { inputA.add(Quaternion(0.0, 1.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(1.0, 1.0, 1.0)); - expectedOutput.add(Vector3(-1.0, 1.0, 1.0)); + expectedOutput.add(Vector3(1.0, 1.0, -1.0)); inputA.add(Quaternion.identity()..normalize()); inputB.add(Vector3(1.0, 2.0, 3.0)); @@ -151,7 +151,7 @@ void testQuaternionNormalize() { inputA.add(Quaternion(0.18260, 0.54770, 0.73030, 0.36510)..normalize()); inputB.add(Vector3(1.0, 0.0, 0.0)); - expectedOutput.add(Vector3(-0.6667, -0.3333, 0.6667)); + expectedOutput.add(Vector3(-0.6667, 0.7333, -0.1332)); { inputA.add(Quaternion(1.0, 0.0, 0.0, 1.0)..normalize()); @@ -160,17 +160,17 @@ void testQuaternionNormalize() { inputA.add(Quaternion(1.0, 0.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 1.0, 0.0)); - expectedOutput.add(Vector3(0.0, 0.0, -1.0)); + expectedOutput.add(Vector3(0.0, 0.0, 1.0)); inputA.add(Quaternion(1.0, 0.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 0.0, 1.0)); - expectedOutput.add(Vector3(0.0, 1.0, 0.0)); + expectedOutput.add(Vector3(0.0, -1.0, 0.0)); } { inputA.add(Quaternion(0.0, 1.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(1.0, 0.0, 0.0)); - expectedOutput.add(Vector3(0.0, 0.0, 1.0)); + expectedOutput.add(Vector3(0.0, 0.0, -1.0)); inputA.add(Quaternion(0.0, 1.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 1.0, 0.0)); @@ -178,17 +178,17 @@ void testQuaternionNormalize() { inputA.add(Quaternion(0.0, 1.0, 0.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 0.0, 1.0)); - expectedOutput.add(Vector3(-1.0, 0.0, 0.0)); + expectedOutput.add(Vector3(1.0, 0.0, 0.0)); } { inputA.add(Quaternion(0.0, 0.0, 1.0, 1.0)..normalize()); inputB.add(Vector3(1.0, 0.0, 0.0)); - expectedOutput.add(Vector3(0.0, -1.0, 0.0)); + expectedOutput.add(Vector3(0.0, 1.0, 0.0)); inputA.add(Quaternion(0.0, 0.0, 1.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 1.0, 0.0)); - expectedOutput.add(Vector3(1.0, 0.0, 0.0)); + expectedOutput.add(Vector3(-1.0, 0.0, 0.0)); inputA.add(Quaternion(0.0, 0.0, 1.0, 1.0)..normalize()); inputB.add(Vector3(0.0, 0.0, 1.0)); From 8e3c61d479abfb8239cde967104a92b14c70f4c4 Mon Sep 17 00:00:00 2001 From: AdityaJagtap18 Date: Sun, 6 Sep 2026 18:26:18 +0530 Subject: [PATCH 2/3] [vector_math] Bump to 2.4.3 and add CHANGELOG entry Per this repo's version-and-CHANGELOG contribution requirement for packages/vector_math following the Quaternion.rotate() inverse-rotation fix. Co-Authored-By: Claude Sonnet 5 --- packages/vector_math/CHANGELOG.md | 5 +++++ packages/vector_math/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/vector_math/CHANGELOG.md b/packages/vector_math/CHANGELOG.md index a232ed01..70142d23 100644 --- a/packages/vector_math/CHANGELOG.md +++ b/packages/vector_math/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.4.3 + +* Fixes `Quaternion.rotate()`/`rotated()` applying the inverse of the + intended rotation in both the 32-bit and 64-bit variants. + ## 2.4.2 * Documents the public geometry filter APIs. diff --git a/packages/vector_math/pubspec.yaml b/packages/vector_math/pubspec.yaml index 89b201e2..fa38b2ed 100644 --- a/packages/vector_math/pubspec.yaml +++ b/packages/vector_math/pubspec.yaml @@ -5,7 +5,7 @@ name: vector_math description: A vector math library for 2D and 3D applications, supporting 2D, 3D, and 4D matrices. repository: https://github.com/flutter/core-packages/tree/main/packages/vector_math issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_math%22 -version: 2.4.2 +version: 2.4.3 environment: sdk: ^3.10.0 From 73dfbae7d9990678ff506e691f1896c2df8fc026 Mon Sep 17 00:00:00 2001 From: AdityaJagtap18 Date: Sun, 6 Sep 2026 21:04:36 +0530 Subject: [PATCH 3/3] Apply Gemini review feedback: use normalized()/avoid redundant clone - v.clone()..normalize() -> v.normalized(), matching this codebase's own idiom. - Matrix3.transformed(v.clone()) -> Matrix3.transformed(v): the method already copies its argument internally when no `out` is given. Co-Authored-By: Claude Sonnet 5 --- packages/vector_math/test/fuzz_invariants_test.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/vector_math/test/fuzz_invariants_test.dart b/packages/vector_math/test/fuzz_invariants_test.dart index 11854753..bff8d4f1 100644 --- a/packages/vector_math/test/fuzz_invariants_test.dart +++ b/packages/vector_math/test/fuzz_invariants_test.dart @@ -49,17 +49,17 @@ void main() { for (var i = 0; i < n; i++) { final Vector2 v2 = randV2(); if (v2.length >= 1e-6) { - final Vector2 copy2 = v2.clone()..normalize(); + final Vector2 copy2 = v2.normalized(); expect(copy2.length, closeTo(1.0, 1e-6), reason: 'v2=$v2'); } final Vector3 v3 = randV3(); if (v3.length >= 1e-6) { - final Vector3 copy3 = v3.clone()..normalize(); + final Vector3 copy3 = v3.normalized(); expect(copy3.length, closeTo(1.0, 1e-6), reason: 'v3=$v3'); } final Vector4 v4 = randV4(); if (v4.length >= 1e-6) { - final Vector4 copy4 = v4.clone()..normalize(); + final Vector4 copy4 = v4.normalized(); expect(copy4.length, closeTo(1.0, 1e-6), reason: 'v4=$v4'); } } @@ -69,7 +69,7 @@ void main() { for (var i = 0; i < n; i++) { final Vector3 v3 = randV3(); if (v3.length >= 1e-6) { - final Vector3 viaCopy = v3.clone()..normalize(); + final Vector3 viaCopy = v3.normalized(); final out = Vector3.zero(); v3.normalizeInto(out); expect(out.x, closeTo(viaCopy.x, eps), reason: 'v3=$v3'); @@ -79,7 +79,7 @@ void main() { final Vector4 v4 = randV4(); if (v4.length >= 1e-6) { - final Vector4 viaCopy4 = v4.clone()..normalize(); + final Vector4 viaCopy4 = v4.normalized(); final out4 = Vector4.zero(); v4.normalizeInto(out4); expect(out4.x, closeTo(viaCopy4.x, eps)); @@ -166,7 +166,7 @@ void main() { final Vector3 v = randV3(); final Vector3 viaQuat = q.rotated(v); final Matrix3 m = q.asRotationMatrix(); - final Vector3 viaMatrix = m.transformed(v.clone()); + final Vector3 viaMatrix = m.transformed(v); expect(viaMatrix.x, closeTo(viaQuat.x, 1e-6), reason: 'q=$q v=$v'); expect(viaMatrix.y, closeTo(viaQuat.y, 1e-6), reason: 'q=$q v=$v'); expect(viaMatrix.z, closeTo(viaQuat.z, 1e-6), reason: 'q=$q v=$v');