fix: wasm addFunction tests (kripken#332)

* Migrates tests to global Ammo, initialized once per file

  Fixes concurrency issues when testing wasm ammo

* Provide some addFunction wasm signature constants

* Adds a test helper to create discrete dynamics worlds

* Adds test coverage for all contact callbacks

* Builds and tests addFunction in the ci workflow

* Enables Ammo.addFunction by default

* Removes addFunction detection and test skipping

* Replaces RESERVED_FUNCTION_POINTERS with ALLOW_TABLE_GROWTH

CMakeLists.txt

@@ -40,7 +40,9 @@ set(EMCC_ARGS
   --post-js ${AMMO_ONLOAD_FILE}
   -O3
   -s ALLOW_MEMORY_GROWTH=${ALLOW_MEMORY_GROWTH}
+  -s ALLOW_TABLE_GROWTH=1
   -s EXPORTED_RUNTIME_METHODS=["UTF8ToString"]
+  -s EXTRA_EXPORTED_RUNTIME_METHODS=["addFunction"]
   -s EXPORT_NAME="Ammo"
   -s MODULARIZE=1
   -s NO_EXIT_RUNTIME=1
@@ -58,12 +60,6 @@ else()
     -s NO_DYNAMIC_EXECUTION=1)
 endif()
 
-if(${ADD_FUNCTION_SUPPORT})
-  LIST(APPEND EMCC_ARGS
-    -s RESERVED_FUNCTION_POINTERS=20
-    -s EXTRA_EXPORTED_RUNTIME_METHODS=["addFunction"])
-endif()
-
 set(EMCC_JS_ARGS ${EMCC_ARGS}
   -s AGGRESSIVE_VARIABLE_ELIMINATION=1
   -s ELIMINATE_DUPLICATE_FUNCTIONS=1

README.markdown

@@ -143,7 +143,6 @@ configuration, for example:
   $ cmake -B builds -DCLOSURE=1                # compile with closure
   $ cmake -B builds -DTOTAL_MEMORY=268435456   # allocate a 256MB heap
   $ cmake -B builds -DALLOW_MEMORY_GROWTH=1    # enable a resizable heap
-  $ cmake -B builds -DADD_FUNCTION_SUPPORT=1   # enable Ammo.addFunction()
   ```
 
 On windows, you can build using cmake's

onload.js

@@ -1,2 +1,7 @@
+Module['CONTACT_ADDED_CALLBACK_SIGNATURE'] = 'iiiiiiii';
+Module['CONTACT_DESTROYED_CALLBACK_SIGNATURE'] = 'ii';
+Module['CONTACT_PROCESSED_CALLBACK_SIGNATURE'] = 'iiii';
+Module['INTERNAL_TICK_CALLBACK_SIGNATURE'] = 'vif';
+
 // Reassign global Ammo to the loaded module:
 this['Ammo'] = Module;

tests/2.js

@@ -1,8 +1,10 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('ClosestRayResultCallback', async t => {
-  const Ammo = await loadAmmo();
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('ClosestRayResultCallback', t => {
 
   var rayCallback = new Ammo.ClosestRayResultCallback(new Ammo.btVector3(0, 0, 0), new Ammo.btVector3(1, 3, 17));
   t.log(rayCallback.hasHit());

tests/3.js

@@ -1,8 +1,10 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('Issue 3: Ammo.btSweepAxis3 doesn\'t seem to work', async t => {
-  const Ammo = await loadAmmo();
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('Issue 3: Ammo.btSweepAxis3 doesn\'t seem to work', t => {
 
   var collisionConfiguration = new Ammo.btDefaultCollisionConfiguration();
   var dispatcher = new Ammo.btCollisionDispatcher(collisionConfiguration);

tests/add-function.js

@@ -1,59 +1,38 @@
 const test = require('ava')
 const createUnitBox = require('./helpers/create-unit-box.js')
+const createDiscreteDynamicsWorld = require('./helpers/create-discrete-dynamics-world.js')
 const loadAmmo = require('./helpers/load-ammo.js')
 
 const CF_CUSTOM_MATERIAL_CALLBACK = 8
 
+// Load global Ammo once for all tests:
+test.before(async t => await loadAmmo())
 
-function shouldTest(t, Ammo) {
-  if (Ammo.addFunction) return true
-  t.log(`Passed '${t.title}' without testing (requires ammo with addFunction support)`)
-  t.pass()
-  return false
-}
 
+// Test contact callbacks serially:
+// Each btManifoldResult callback is a global extern in bullet, however ammo
+// has made these available via instance method on btDiscreteDynamicsWorld
+// Since all instances share the same global, concurrent testing is unsafe.
+const testContactCallback = test.serial.cb
 
-test('Ammo.addFunction should return a function pointer', async t => {
-  const Ammo = await loadAmmo()
+testContactCallback('btDiscreteDynamicsWorld.prototype.setContactAddedCallback should work like a charm', t => {
 
-  if (!shouldTest(t, Ammo)) return
-
-  const functi0n = () => true
-  const pointer = Ammo.addFunction(functi0n)
-  t.assert(pointer)
-})
-
-
-test('btDiscreteDynamicsWorld.prototype.setContactAddedCallback should work like a charm', async t => {
-  const Ammo = await loadAmmo()
-
-  if (!shouldTest(t, Ammo)) return
-
-  const collisionConfiguration = new Ammo.btDefaultCollisionConfiguration()
-  const dispatcher = new Ammo.btCollisionDispatcher(collisionConfiguration)
-  const broadphase = new Ammo.btDbvtBroadphase()
-  const solver = new Ammo.btSequentialImpulseConstraintSolver()
-  const dynamicsWorld = new Ammo.btDiscreteDynamicsWorld(
-    dispatcher,
-    broadphase,
-    solver,
-    collisionConfiguration
-  )
+  const world = createDiscreteDynamicsWorld()
+  const timestep = 1
+  const maxSubsteps = 0
 
   // Place two boxes at the origin to generate a collision:
   const bodyA = createUnitBox()
   bodyA.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
-  dynamicsWorld.addRigidBody(bodyA)
+  world.addRigidBody(bodyA)
 
   const bodyB = createUnitBox()
   bodyB.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
-  dynamicsWorld.addRigidBody(bodyB)
+  world.addRigidBody(bodyB)
 
-  // Set up the callback:
-  let callbackInvocationCount = 0
+  // Set up the callback
+  let expectedInvocations = 4
   const callback = (cp, colObj0Wrap, partId0, index0, colObj1Wrap, partId1, index1) => {
-    callbackInvocationCount++
-
     t.not(cp, undefined)
     t.not(colObj0Wrap, undefined)
     t.not(partId0, undefined)
@@ -69,46 +48,125 @@ test('btDiscreteDynamicsWorld.prototype.setContactAddedCallback should work like
     colObj1Wrap = Ammo.wrapPointer(colObj1Wrap, Ammo.btCollisionObjectWrapper)
     colObj1 = colObj1Wrap.getCollisionObject()
     t.assert(Ammo.compare(colObj1, bodyB))
+
+    if (--expectedInvocations < 1) t.end()
   }
-  const callbackPointer = Ammo.addFunction(callback)
-  dynamicsWorld.setContactAddedCallback(callbackPointer)
+  callback.pointer = Ammo.addFunction(callback, Ammo.CONTACT_ADDED_CALLBACK_SIGNATURE)
+  world.setContactAddedCallback(callback.pointer)
+  t.teardown(() => world.setContactAddedCallback(null))
 
   // Step the simulation and assert the behavior:
-  dynamicsWorld.stepSimulation(1)
-  t.is(callbackInvocationCount, 4)
+  world.stepSimulation(timestep, maxSubsteps)
 })
 
 
-test('btDynamicsWorld.prototype.setInternalTickCallback should work like a charm', async t => {
-  const Ammo = await loadAmmo()
+testContactCallback('btDiscreteDynamicsWorld.prototype.setContactDestroyedCallback should work like a charm', t => {
+
+  const world = createDiscreteDynamicsWorld()
+  const timestep = 1
+  const maxSubsteps = 0
+
+  // Place two boxes at the origin to generate a collision
+  const bodyA = createUnitBox()
+  bodyA.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
+  world.addRigidBody(bodyA)
+
+  const bodyB = createUnitBox()
+  bodyB.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
+  world.addRigidBody(bodyB)
+
+  // Set up the callback:
+  const expectedUserPersistentData = []
+  let expectedInvocations = 4
+  const callback = (userPersistentData) => {
+    t.assert(expectedUserPersistentData.includes(userPersistentData))
+    if (--expectedInvocations < 1) t.end()
+  }
+  callback.pointer = Ammo.addFunction(callback, Ammo.CONTACT_DESTROYED_CALLBACK_SIGNATURE)
+  world.setContactDestroyedCallback(callback.pointer)
+  t.teardown(() => world.setContactDestroyedCallback(null))
+
+  // Step the simulation to start the collision:
+  world.stepSimulation(timestep, maxSubsteps)
+
+  // Assign user persistent data to trigger the destroy callback:
+  // In bullet, contact manifold user persistent data can be any void*
+  // ammo does not have a way to create pointers to javascript data,
+  // but you can abuse the void* to store a simple int value.
+  // Here we create some random integers for testing the callback:
+  const dispatcher = world.getDispatcher()
+  for (let i = 0, manifold; i < dispatcher.getNumManifolds(); i++) {
+    manifold = dispatcher.getManifoldByIndexInternal(i)
+    for (let j = 0, point; j < manifold.getNumContacts(); j++) {
+      point = manifold.getContactPoint(j)
+      point.m_userPersistentData = Math.floor(Math.random() * 100)
+      expectedUserPersistentData.push(point.m_userPersistentData)
+    }
+  }
+
+  // Separate the boxes to end the collision:
+  bodyA.getWorldTransform().getOrigin().setValue(10, 10, 10)
+
+  // Step the simulation and assert the behavior:
+  world.stepSimulation(timestep, maxSubsteps)
+})
+
+
+testContactCallback('btDiscreteDynamicsWorld.prototype.setContactProcessedCallback should work like a charm', t => {
+
+  const world = createDiscreteDynamicsWorld()
+  const timestep = 1
+  const maxSubsteps = 0
+
+  // Place two boxes at the origin to generate a collision:
+  const bodyA = createUnitBox()
+  bodyA.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
+  world.addRigidBody(bodyA)
+
+  const bodyB = createUnitBox()
+  bodyB.setCollisionFlags(CF_CUSTOM_MATERIAL_CALLBACK)
+  world.addRigidBody(bodyB)
+
+  // Set up the callback:
+  let expectedInvocations = 4
+  const callback = (cp, body0, body1) => {
+    t.not(cp, undefined)
+
+    body0 = Ammo.wrapPointer(body0, Ammo.btCollisionObject)
+    t.assert(Ammo.compare(body0, bodyA))
+
+    body1 = Ammo.wrapPointer(body1, Ammo.btCollisionObject)
+    t.assert(Ammo.compare(body1, bodyB))
+
+    if (--expectedInvocations < 1) t.end()
+  }
+  const callbackPointer = Ammo.addFunction(callback, Ammo.CONTACT_PROCESSED_CALLBACK_SIGNATURE)
+  world.setContactProcessedCallback(callbackPointer)
+  t.teardown(() => world.setContactProcessedCallback(null))
+
+  // Step the simulation and assert the behavior:
+  world.stepSimulation(timestep, maxSubsteps)
+})
+
 
-  if (!shouldTest(t, Ammo)) return
+test.cb('btDynamicsWorld.prototype.setInternalTickCallback should work like a charm', t => {
 
-  const collisionConfiguration = new Ammo.btDefaultCollisionConfiguration()
-  const dispatcher = new Ammo.btCollisionDispatcher(collisionConfiguration)
-  const broadphase = new Ammo.btDbvtBroadphase()
-  const solver = new Ammo.btSequentialImpulseConstraintSolver()
-  const dynamicsWorld = new Ammo.btDiscreteDynamicsWorld(
-    dispatcher,
-    broadphase,
-    solver,
-    collisionConfiguration
-  )
+  const world = createDiscreteDynamicsWorld()
   const timestep = 1 // integer is easy to assert
-  const maxSubSteps = 0 // required for a stable callback timestep
+  const maxSubsteps = 0 // required for a stable callback timestep
 
   // Set up the callback:
-  let callbackInvocationCount = 0
+  let expectedInvocations = 1
   const callback = (_world, _timestep) => {
-    callbackInvocationCount++
     _world = Ammo.wrapPointer(_world, Ammo.btDiscreteDynamicsWorld)
-    t.assert(Ammo.compare(_world, dynamicsWorld))
+    t.assert(Ammo.compare(_world, world))
     t.is(_timestep, timestep)
+    if (--expectedInvocations < 1) t.end()
   }
-  const callbackPointer = Ammo.addFunction(callback)
-  dynamicsWorld.setInternalTickCallback(callbackPointer)
+  callback.pointer = Ammo.addFunction(callback, Ammo.INTERNAL_TICK_CALLBACK_SIGNATURE)
+  world.setInternalTickCallback(callback.pointer)
+  t.teardown(() => world.setInternalTickCallback(null))
 
   // Step the simulation and assert the behavior:
-  dynamicsWorld.stepSimulation(timestep, maxSubSteps)
-  t.is(callbackInvocationCount, 1)
+  world.stepSimulation(timestep, maxSubsteps)
 })

tests/basics.js

@@ -1,8 +1,10 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('basics', async t => {
-  const Ammo = await loadAmmo()
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('basics', t => {
 
   var vec = new Ammo.btVector3(4, 5, 6);
   t.is([vec.x(), vec.y(), vec.z()].toString(), '4,5,6');

tests/compoundShape.js

@@ -1,8 +1,10 @@
 const test = require('ava')
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('compound shape', async t => {
-  const Ammo = await loadAmmo()
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('compound shape', t => {
 
   var compoundShape = new Ammo.btCompoundShape();
 

tests/constraint.js

@@ -1,8 +1,10 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('constraint', async t => {
-  const Ammo = await loadAmmo();
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('constraint', t => {
 
   function testConstraint() {
 

tests/helpers/create-discrete-dynamics-world.js

@@ -0,0 +1,15 @@
+function createDiscreteDynamicsWorld() {
+  const collisionConfiguration = new Ammo.btDefaultCollisionConfiguration()
+  const dispatcher = new Ammo.btCollisionDispatcher(collisionConfiguration)
+  const broadphase = new Ammo.btDbvtBroadphase()
+  const solver = new Ammo.btSequentialImpulseConstraintSolver()
+  const world = new Ammo.btDiscreteDynamicsWorld(
+    dispatcher,
+    broadphase,
+    solver,
+    collisionConfiguration
+  )
+  return world
+}
+
+module.exports = createDiscreteDynamicsWorld

tests/stress.js

@@ -2,8 +2,10 @@ const test = require('ava');
 const getClosureMapping = require('./helpers/get-closure-mapping.js');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('stress', async t => {
-  const Ammo = await loadAmmo();
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('stress', t => {
 
   var TEST_MEMORY = 0;
 

tests/userData.js

@@ -1,9 +1,11 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
 // Skipped to reflect current state
-test.skip('userData', async t => {
-  const Ammo = await loadAmmo();
+test.skip('userData', t => {
 
   var transform = new Ammo.btTransform();
   transform.setIdentity();

tests/wrapping.js

@@ -1,8 +1,10 @@
 const test = require('ava');
 const loadAmmo = require('./helpers/load-ammo.js');
 
-test('tests for caching, comparing, wrapping, etc.', async t => {
-  const Ammo = await loadAmmo();
+// Initialize global Ammo once for all tests:
+test.before(async t => loadAmmo())
+
+test('tests for caching, comparing, wrapping, etc.', t => {
 
   var vec1 = new Ammo.btVector3(0, 0, 0);
   var vec2 = new Ammo.btVector3(1, 3, 17);