diff --git a/packages/exojs-physics/src/ContactGraph.ts b/packages/exojs-physics/src/ContactGraph.ts index 159f87db..b57d6b50 100644 --- a/packages/exojs-physics/src/ContactGraph.ts +++ b/packages/exojs-physics/src/ContactGraph.ts @@ -79,7 +79,7 @@ export class ContactGraph { } const isSensor = a.isSensor || b.isSensor; - const key = pairKey(a, b); + const key = pairKey(a.id, b.id); const existing = this._records.get(key); const record = existing ?? createRecord(a, b, isSensor); const touching = isSensor ? testOverlap(a, b) : collide(a, b, record.manifold); @@ -158,8 +158,21 @@ export class ContactGraph { } } -/** Integer key for an unordered collider pair (`a.id < b.id` guaranteed by the broad phase). */ -const pairKey = (a: Collider, b: Collider): number => (a.id << 16) | b.id; +/** + * Stride for packing two collider ids into one pair key. Multiplying by this + * (rather than a 32-bit `<<`) keeps the key collision-free up to ~67M (2^26) + * ids per world, within JS's 2^53 safe-integer range. + * @internal + */ +export const pairKeyStride = 0x4000000; // 2^26 + +/** + * Integer key for an unordered collider pair (`aId < bId` is guaranteed by the + * broad phase). The previous `(aId << 16) | bId` silently collided once any id + * reached 65536, because JS bitwise operators wrap at 32 bits. + * @internal + */ +export const pairKey = (aId: number, bId: number): number => aId * pairKeyStride + bId; const createRecord = (a: Collider, b: Collider, isSensor: boolean): ContactRecord => ({ a, diff --git a/packages/exojs-physics/test/pair-key.test.ts b/packages/exojs-physics/test/pair-key.test.ts new file mode 100644 index 00000000..c4a99a66 --- /dev/null +++ b/packages/exojs-physics/test/pair-key.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest'; + +import { pairKey,pairKeyStride } from '../src/ContactGraph'; + +/** + * A4: the contact pair key packs two collider ids into one integer. The old + * `(aId << 16) | bId` overflowed once any id reached 65536 (JS bitwise ops are + * 32-bit), silently colliding distinct pairs. Ids are allocated monotonically + * and never recycled, so a long-running world with heavy spawn/destroy churn + * can reach that limit. + */ +describe('ContactGraph pairKey (A4 16-bit overflow fix)', () => { + it('produces a distinct key for every distinct ordered id pair, including ids past 65535', () => { + const ids = [1, 2, 65_535, 65_536, 65_537, 100_000, 1_000_000]; + const keys = new Set(); + + for (let i = 0; i < ids.length; i++) { + for (let j = i + 1; j < ids.length; j++) { + keys.add(pairKey(ids[i], ids[j])); + } + } + + // C(7,2) = 21 pairs, all keys distinct. + expect(keys.size).toBe(21); + }); + + it('stays within the safe-integer range for large ids', () => { + const key = pairKey(1_000_000, 2_000_000); + + expect(Number.isSafeInteger(key)).toBe(true); + expect(key).toBeLessThanOrEqual(Number.MAX_SAFE_INTEGER); + }); + + it('regression: the old 16-bit scheme collided where the new one does not', () => { + const oldKey = (aId: number, bId: number): number => (aId << 16) | bId; + + // id 65536 wraps to 0 under `<< 16`, colliding pair (65536, 5) with (0, 5). + expect(oldKey(65_536, 5)).toBe(oldKey(0, 5)); + expect(pairKey(65_536, 5)).not.toBe(pairKey(0, 5)); + expect(pairKeyStride).toBe(2 ** 26); + }); +});