feat: add destroy() methods to classes that need cleanup, address vec…

…3 leaks
isaac-mason · Jun 14, 2023 · 273c456 · 273c456
1 parent 02c9131
commit 273c456
Show file tree

Hide file tree

Showing 11 changed files with 239 additions and 52 deletions.
diff --git a/.changeset/pink-tips-share.md b/.changeset/pink-tips-share.md
@@ -0,0 +1,6 @@
+---
+'@recast-navigation/core': patch
+'@recast-navigation/wasm': patch
+---
+
+feat: add destroy() methods to classes that need cleanup, address vec3 leaks
diff --git a/packages/recast-navigation-core/src/crowd.ts b/packages/recast-navigation-core/src/crowd.ts
@@ -1,7 +1,7 @@
 import type R from '@recast-navigation/wasm';
+import { finalizer } from './finalizer';
 import type { NavMesh } from './nav-mesh';
-import { Vector3, array } from './utils';
-import { vec3 } from './utils';
+import { Vector3, array, emscripten, vec3 } from './utils';
 import { Wasm } from './wasm';
 
 export type CrowdParams = {
@@ -141,9 +141,13 @@ export class Crowd {
    */
   timeFactor = 1;
 
+  private tmpVec1 = new Wasm.Recast.Vec3();
+
   constructor({ maxAgents, maxAgentRadius, navMesh }: CrowdParams) {
     this.navMesh = navMesh;
     this.raw = new Wasm.Recast.Crowd(maxAgents, maxAgentRadius, navMesh.raw);
+
+    finalizer.register(this);
   }
 
   /**
@@ -171,7 +175,10 @@ export class Crowd {
     dtCrowdAgentParams.queryFilterType = params.queryFilterType;
     dtCrowdAgentParams.userData = params.userData;
 
-    const agentId = this.raw.addAgent(vec3.toRaw(position), dtCrowdAgentParams);
+    const agentId = this.raw.addAgent(
+      vec3.toRaw(position, this.tmpVec1),
+      dtCrowdAgentParams
+    );
 
     this.agents.push(agentId);
 
@@ -194,14 +201,14 @@ export class Crowd {
    * Submits a new move request for the specified agent.
    */
   goto(agentIndex: number, position: Vector3) {
-    this.raw.agentGoto(agentIndex, vec3.toRaw(position));
+    this.raw.agentGoto(agentIndex, vec3.toRaw(position, this.tmpVec1));
   }
 
   /**
    * Teleports the agent to the given position.
    */
   teleport(agentIndex: number, position: Vector3) {
-    this.raw.agentTeleport(agentIndex, vec3.toRaw(position));
+    this.raw.agentTeleport(agentIndex, vec3.toRaw(position, this.tmpVec1));
   }
 
   /**
@@ -241,13 +248,6 @@ export class Crowd {
     }
   }
 
-  /**
-   * Destroys the crowd.
-   */
-  destroy() {
-    this.raw.destroy();
-  }
-
   /**
    * Returns the maximum number of agents that can be managed by the crowd.
    */
@@ -273,21 +273,21 @@ export class Crowd {
    * Returns the position of the specified agent.
    */
   getAgentPosition(agentIndex: number): Vector3 {
-    return vec3.fromRaw(this.raw.getAgentPosition(agentIndex));
+    return vec3.fromRaw(this.raw.getAgentPosition(agentIndex), true);
   }
 
   /**
    * Returns the velocity of the specified agent.
    */
   getAgentVelocity(agentIndex: number): Vector3 {
-    return vec3.fromRaw(this.raw.getAgentVelocity(agentIndex));
+    return vec3.fromRaw(this.raw.getAgentVelocity(agentIndex), true);
   }
 
   /**
    * Returns the next target position on the path to the specified agents target.
    */
   getAgentNextTargetPath(agentIndex: number): Vector3 {
-    return vec3.fromRaw(this.raw.getAgentNextTargetPath(agentIndex));
+    return vec3.fromRaw(this.raw.getAgentNextTargetPath(agentIndex), true);
   }
 
   /**
@@ -307,7 +307,9 @@ export class Crowd {
   getAgentCorners(agentIndex: number): Vector3[] {
     const corners = this.raw.getCorners(agentIndex);
 
-    return array((i) => corners.getPoint(i), corners.getPointCount()).map(vec3.fromRaw);
+    return array((i) => corners.getPoint(i), corners.getPointCount()).map(
+      (vec) => vec3.fromRaw(vec)
+    );
   }
 
   /**
@@ -394,6 +396,12 @@ export class Crowd {
    * The default is (1,1,1)
    */
   setDefaultQueryExtent(extent: Vector3): void {
-    this.raw.setDefaultQueryExtent(vec3.toRaw(extent));
+    this.raw.setDefaultQueryExtent(vec3.toRaw(extent, this.tmpVec1));
+  }
+
+  destroy(): void {
+    this.raw.destroy();
+    finalizer.unregister(this);
+    emscripten.destroy(this.raw);
   }
 }
diff --git a/packages/recast-navigation-core/src/finalizer.ts b/packages/recast-navigation-core/src/finalizer.ts
@@ -0,0 +1,35 @@
+const WARNING_MESSAGE =
+  'recast-navigation found a recast object that was not cleaned up properly.\n' +
+  'It was destroyed automatically in this case, but this functionality is not reliable.\n' +
+  'Make sure to call destroy() once you are done with recast objects.';
+
+declare class FinalizationRegistry {
+  constructor(cleanupCallback: (heldValue: any) => void);
+  register(target: object, heldValue: any, unregisterToken?: object): void;
+  unregister(unregisterToken: object): void;
+}
+
+const createNoopFinalizer = () => ({
+  register: () => {},
+  unregister: () => {},
+});
+
+const createFinalizer = () => {
+  const registry = new FinalizationRegistry((callback: () => void) => {
+    console.warn(WARNING_MESSAGE);
+    callback();
+  });
+
+  return {
+    register: (target: { destroy: () => void }) => {
+      registry.register(target, () => target.destroy(), target);
+    },
+    unregister: (target: any) => {
+      registry.unregister(target);
+    },
+  };
+};
+
+export const finalizer = globalThis.FinalizationRegistry
+  ? createFinalizer()
+  : createNoopFinalizer();
diff --git a/packages/recast-navigation-core/src/nav-mesh-builder.ts b/packages/recast-navigation-core/src/nav-mesh-builder.ts
@@ -1,5 +1,7 @@
 import type R from '@recast-navigation/wasm';
 import { Wasm } from './wasm';
+import { emscripten } from './utils';
+import { finalizer } from './finalizer';
 
 export type CreateNavMeshDataParams = {
   verts: number[];
@@ -42,6 +44,8 @@ export class NavMeshBuilder {
 
   constructor() {
     this.raw = new Wasm.Recast.NavMeshBuilder();
+
+    finalizer.register(this);
   }
 
   createNavMeshData(params: CreateNavMeshDataParams): CreateNavMeshDataResult {
@@ -79,6 +83,16 @@ export class NavMeshBuilder {
     navMeshCreateParams.set_ch(params.ch);
     navMeshCreateParams.set_buildBvTree(params.buildBvTree);
 
-    return this.raw.createNavMeshData(navMeshCreateParams);
+    const createNavMeshDataResult =
+      this.raw.createNavMeshData(navMeshCreateParams);
+
+    emscripten.destroy(navMeshCreateParams);
+
+    return createNavMeshDataResult;
+  }
+
+  destroy(): void {
+    finalizer.unregister(this);
+    emscripten.destroy(this.raw);
   }
 }
diff --git a/packages/recast-navigation-core/src/nav-mesh-exporter.ts b/packages/recast-navigation-core/src/nav-mesh-exporter.ts
@@ -2,12 +2,16 @@ import type R from '@recast-navigation/wasm';
 import { NavMesh } from './nav-mesh';
 import { TileCache } from './tile-cache';
 import { Wasm } from './wasm';
+import { emscripten } from './utils';
+import { finalizer } from './finalizer';
 
 export class NavMeshExporter {
   raw: R.NavMeshExporter;
 
   constructor() {
     this.raw = new Wasm.Recast.NavMeshExporter();
+
+    finalizer.register(this);
   }
 
   /**
@@ -29,4 +33,9 @@ export class NavMeshExporter {
 
     return data;
   }
+
+  destroy(): void {
+    finalizer.unregister(this);
+    emscripten.destroy(this.raw);
+  }
 }
diff --git a/packages/recast-navigation-core/src/nav-mesh-generator.ts b/packages/recast-navigation-core/src/nav-mesh-generator.ts
@@ -2,6 +2,8 @@ import type R from '@recast-navigation/wasm';
 import { NavMesh } from './nav-mesh';
 import { TileCache } from './tile-cache';
 import { Wasm } from './wasm';
+import { emscripten } from './utils';
+import { finalizer } from './finalizer';
 
 export type NavMeshConfig = {
   /**
@@ -156,6 +158,8 @@ export class NavMeshGenerator {
 
   constructor() {
     this.raw = new Wasm.Recast.NavMeshGenerator();
+
+    finalizer.register(this);
   }
 
   /**
@@ -207,4 +211,9 @@ export class NavMeshGenerator {
 
     return { success, navMesh, tileCache };
   }
+
+  destroy(): void {
+    finalizer.unregister(this);
+    emscripten.destroy(this.raw);
+  }
 }
diff --git a/packages/recast-navigation-core/src/nav-mesh-importer.ts b/packages/recast-navigation-core/src/nav-mesh-importer.ts
@@ -2,6 +2,8 @@ import type R from '@recast-navigation/wasm';
 import { NavMesh } from './nav-mesh';
 import { TileCache } from './tile-cache';
 import { Wasm } from './wasm';
+import { emscripten } from './utils';
+import { finalizer } from './finalizer';
 
 export type NavMeshImporterResult = {
   navMesh: NavMesh;
@@ -13,6 +15,8 @@ export class NavMeshImporter {
 
   constructor() {
     this.raw = new Wasm.Recast.NavMeshImporter();
+
+    finalizer.register(this);
   }
 
   /**
@@ -46,4 +50,9 @@ export class NavMeshImporter {
 
     return { navMesh, tileCache };
   }
+
+  destroy(): void {
+    finalizer.unregister(this);
+    emscripten.destroy(this.raw);
+  }
 }
diff --git a/packages/recast-navigation-core/src/nav-mesh-query.ts b/packages/recast-navigation-core/src/nav-mesh-query.ts
@@ -1,4 +1,5 @@
 import type R from '@recast-navigation/wasm';
+import { finalizer } from './finalizer';
 import { NavMesh } from './nav-mesh';
 import { array, vec3, Vector3 } from './utils';
 import { Wasm } from './wasm';
@@ -15,39 +16,45 @@ export type NavMeshQueryParams = {
 export class NavMeshQuery {
   raw: R.NavMeshQuery;
 
+  private tmpVec1 = new Wasm.Recast.Vec3();
+  private tmpVec2 = new Wasm.Recast.Vec3();
+
   constructor({ navMesh, maxNodes = 2048 }: NavMeshQueryParams) {
     this.raw = new Wasm.Recast.NavMeshQuery(navMesh.raw, maxNodes);
+
+    finalizer.register(this);
   }
 
   /**
    * Returns the closest point on the NavMesh to the given position.
    */
   getClosestPoint(position: Vector3): Vector3 {
-    const positionRaw = vec3.toRaw(position);
-    const closestPoint = this.raw.getClosestPoint(positionRaw);
+    const positionRaw = vec3.toRaw(position, this.tmpVec1);
+    const closestPointRaw = this.raw.getClosestPoint(positionRaw)
 
-    return vec3.fromRaw(closestPoint);
+    return vec3.fromRaw(closestPointRaw, true);
   }
 
   /**
    * Returns a random point on the NavMesh within the given radius of the given position.
    */
   getRandomPointAround(position: Vector3, radius: number): Vector3 {
-    const positionRaw = vec3.toRaw(position);
-    const randomPoint = this.raw.getRandomPointAround(positionRaw, radius);
+    const positionRaw = vec3.toRaw(position, this.tmpVec1);
 
-    return vec3.fromRaw(randomPoint);
+    return vec3.fromRaw(
+      this.raw.getRandomPointAround(positionRaw, radius),
+      true
+    );
   }
 
   /**
    * Compute the final position from a segment made of destination-position
    */
   moveAlong(position: Vector3, destination: Vector3): Vector3 {
-    const positionRaw = vec3.toRaw(position);
-    const destinationRaw = vec3.toRaw(destination);
-    const movedPosition = this.raw.moveAlong(positionRaw, destinationRaw);
+    const positionRaw = vec3.toRaw(position, this.tmpVec1);
+    const destinationRaw = vec3.toRaw(destination, this.tmpVec2);
 
-    return { x: movedPosition.x, y: movedPosition.y, z: movedPosition.z };
+    return vec3.fromRaw(this.raw.moveAlong(positionRaw, destinationRaw), true);
   }
 
   /**
@@ -56,20 +63,20 @@ export class NavMeshQuery {
    * @returns an array of Vector3 positions that make up the path, or an empty array if no path was found.
    */
   computePath(start: Vector3, end: Vector3): Vector3[] {
-    const startRaw = vec3.toRaw(start);
-    const endRaw = vec3.toRaw(end);
+    const startRaw = vec3.toRaw(start, this.tmpVec1);
+    const endRaw = vec3.toRaw(end, this.tmpVec2);
     const pathRaw = this.raw.computePath(startRaw, endRaw);
 
-    return array((i) => pathRaw.getPoint(i), pathRaw.getPointCount()).map(vec3.fromRaw);
+    return array((i) => pathRaw.getPoint(i), pathRaw.getPointCount()).map(
+      (vec) => vec3.fromRaw(vec)
+    );
   }
 
   /**
    * Gets the Bounding box extent specified by setDefaultQueryExtent
    */
   getDefaultQueryExtent(): Vector3 {
-    const extentRaw = this.raw.getDefaultQueryExtent();
-
-    return { x: extentRaw.x, y: extentRaw.y, z: extentRaw.z };
+    return vec3.fromRaw(this.raw.getDefaultQueryExtent(), true);
   }
 
   /**
@@ -78,14 +85,15 @@ export class NavMeshQuery {
    * The default is (1,1,1)
    */
   setDefaultQueryExtent(extent: Vector3): void {
-    const extentRaw = vec3.toRaw(extent);
+    const extentRaw = vec3.toRaw(extent, this.tmpVec1);
     this.raw.setDefaultQueryExtent(extentRaw);
   }
 
   /**
    * Destroys the NavMeshQuery instance
    */
   destroy(): void {
+    finalizer.unregister(this);
     this.raw.destroy();
   }
 }