Fix collision detection bug related to node removal

src/core/design/context/tree.ts

@@ -165,7 +165,7 @@ export class Tree implements ITree, ISerializable<TreeData> {
 		State.$lengthChanged.add(node);
 		State.$treeStructureChanged = true;
 		if(node.$isLeaf) {
-			State.$flapAABBChanged.add(node);
+			State.$nodeAABBChanged.add(node);
 		}
 	}
 
@@ -231,7 +231,7 @@ export class Tree implements ITree, ISerializable<TreeData> {
 			State.$lengthChanged.delete(node);
 			State.$parentChanged.delete(node);
 			State.$childrenChanged.delete(node);
-			State.$flapAABBChanged.delete(node);
+			State.$nodeAABBChanged.delete(node);
 
 			delete this._nodes[id];
 			State.$updateResult.remove.nodes.push(id);
@@ -250,7 +250,7 @@ export class Tree implements ITree, ISerializable<TreeData> {
 	private _removeEdgeAndCheckNewFlap(node: TreeNode, partner: TreeNode): void {
 		this.$removeEdge(node.id, partner.id);
 		if(partner.$isLeafLike) {
-			State.$flapAABBChanged.add(partner);
+			State.$nodeAABBChanged.add(partner);
 		}
 	}
 

src/core/design/context/treeNode.ts

@@ -107,7 +107,7 @@ export class TreeNode implements ITreeNode {
 
 	/** For testing purpose. */
 	public $setAABB(top: number, right: number, bottom: number, left: number): void {
-		State.$flapAABBChanged.add(this);
+		State.$nodeAABBChanged.add(this);
 		this.$AABB.$update(top, right, bottom, left);
 	}
 
@@ -124,7 +124,9 @@ export class TreeNode implements ITreeNode {
 	public $cut(): void {
 		if(this.$parent) {
 			this.$parent.$children.$remove(this);
-			this.$parent.$AABB.$removeChild(this.$AABB);
+			if(this.$parent.$AABB.$removeChild(this.$AABB)) {
+				State.$nodeAABBChanged.add(this.$parent);
+			}
 			State.$childrenChanged.add(this.$parent);
 		}
 		this.$parent = undefined;

src/core/design/layout/partition.ts

@@ -49,6 +49,7 @@ export class Partition implements ISerializable<JPartition> {
 	constructor(config: Configuration, data: JPartition) {
 		this.$configuration = config;
 		this.$overlaps = data.overlaps;
+		if(data.overlaps[0].ox < 0) debugger;
 		this.$devices = new Store(deviceGenerator(data, config));
 		this._strategy = data.strategy;
 

src/core/design/tasks/aabb.ts

@@ -14,7 +14,7 @@ import type { ITreeNode } from "../context";
 export const AABBTask = new Task(aabb, junctionTask, roughContourTask);
 
 function aabb(): void {
-	climb(updater, State.$lengthChanged, State.$flapAABBChanged, State.$parentChanged);
+	climb(updater, State.$lengthChanged, State.$nodeAABBChanged, State.$parentChanged);
 }
 
 function updater(node: ITreeNode): boolean {

src/core/design/tasks/junction.ts

@@ -52,24 +52,30 @@ function getCollisionOfLCA(lca: ITreeNode): void {
  * @param distChanged Indicating that the distance have changed along one of the branches,
  * in which case the comparison cannot be skipped and must carry on.
  */
-function compare(a: ITreeNode, b: ITreeNode, lca: ITreeNode, distChanged: boolean): void {
+function compare(A: ITreeNode, B: ITreeNode, lca: ITreeNode, distChanged: boolean): void {
 	// If both subtrees haven't changed, there's no need to re-compare.
-	if(!distChanged && !State.$subtreeAABBChanged.has(a) && !State.$subtreeAABBChanged.has(b)) return;
+	if(!distChanged && !State.$subtreeAABBChanged.has(A) && !State.$subtreeAABBChanged.has(B)) return;
 
 	// Test for collision
-	if(!a.$AABB.$intersects(b.$AABB, dist(a, b, lca))) {
-		clearJunctions(a, b);
+	if(!intersects(A, B, lca)) {
+		clearJunctions(A, B);
 		return;
 	}
 
 	const ctx: NontrivialDescendantContext = { distChanged };
-	a = getNontrivialDescendant(a, ctx);
-	b = getNontrivialDescendant(b, ctx);
+	const a = getNontrivialDescendant(A, ctx);
+	const b = getNontrivialDescendant(B, ctx);
 	distChanged = ctx.distChanged;
 
 	const n = a.$children.$size;
 	const m = b.$children.$size;
 	if(n === 0 && m === 0) {
+		// Since a and b are the only descendants of A and B, respectively,
+		// A intersecting B must imply that a intersects b.
+		// If that is not the case, the rest of the algorithm can go very wrong.
+		// Uncomment the next line to debug this.
+		// if(!intersects(a, b, lca)) debugger;
+
 		// Leaves found; add it to the output
 		State.$junctions.set(a.id, b.id, createJunction(a, b, lca));
 	} else {
@@ -86,6 +92,10 @@ interface NontrivialDescendantContext {
 	distChanged: boolean;
 }
 
+function intersects(a: ITreeNode, b: ITreeNode, lca: ITreeNode): boolean {
+	return a.$AABB.$intersects(b.$AABB, dist(a, b, lca));
+}
+
 function getNontrivialDescendant(node: ITreeNode, ctx?: NontrivialDescendantContext): ITreeNode {
 	while(node.$children.$size === 1) {
 		node = node.$children.$get()!;

src/core/design/tasks/roughContour.ts

@@ -28,7 +28,7 @@ export const roughContourTask = new Task(roughContour, traceContourTask);
 
 function roughContour(): void {
 	climb(updater,
-		State.$flapAABBChanged,
+		State.$nodeAABBChanged,
 		State.$parentChanged,
 		State.$childrenChanged,
 		State.$lengthChanged

src/core/service/state.ts

@@ -83,8 +83,8 @@ export namespace State {
 	 */
 	export const $subtreeAABBChanged = new Set<ITreeNode>();
 
-	/** Those flaps that have their {@link ITreeNode.$AABB AABB} changed in the current round. */
-	export const $flapAABBChanged = new Set<ITreeNode>();
+	/** Those nodes that have their {@link ITreeNode.$AABB AABB} proactively changed in the current round. */
+	export const $nodeAABBChanged = new Set<ITreeNode>();
 
 	/**
 	 * Those flaps that have any type of changes in the current round,
@@ -141,7 +141,7 @@ export namespace State {
 		$parentChanged.clear();
 		$lengthChanged.clear();
 		$subtreeAABBChanged.clear();
-		$flapAABBChanged.clear();
+		$nodeAABBChanged.clear();
 		$flapChanged.clear();
 		$newRepositories.clear();
 		$repoToProcess.clear();

test/specs/tree.spec.ts

@@ -1,7 +1,7 @@
 import { expect } from "chai";
 
 import { TreeController } from "core/controller/treeController";
-import { getDist } from "core/design/context/tree";
+import { Tree, getDist } from "core/design/context/tree";
 import { heightTask } from "core/design/tasks/height";
 import { Processor } from "core/service/processor";
 import { createTree, node, id0, id1, id2, id3, id4, id6, parseTree } from "../utils/tree";
@@ -107,18 +107,30 @@ describe("Tree", function() {
 		expect(JSON.stringify(tree.toJSON().edges)).to.equal(json);
 	});
 
-	it("Keeps a record of AABB", function() {
-		parseTree("(0,1,1),(1,2,2),(0,3,3),(3,4,4)", "(2,8,8,0,0),(4,5,2,0,0)");
-		const [n0, n1, n2, n3, n4] = [0, 1, 2, 3, 4].map(id => node(id)!);
-		expect(n2.$AABB.$toArray()).to.eql([10, 10, 6, 6]);
-		expect(n1.$AABB.$toArray()).to.eql([11, 11, 5, 5]);
-		expect(n4.$AABB.$toArray()).to.eql([6, 9, -2, 1]);
-		expect(n3.$AABB.$toArray()).to.eql([9, 12, -5, -2]);
-		expect(n0.$AABB.$toArray()).to.eql([11, 12, -5, -2]);
+	describe("AABB record", function() {
+		it("Updates AABB when a child updates", function() {
+			parseTree("(0,1,1),(1,2,2),(0,3,3),(3,4,4)", "(2,8,8,0,0),(4,5,2,0,0)");
+			const [n0, n1, n2, n3, n4] = [0, 1, 2, 3, 4].map(id => node(id)!);
+			expect(n2.$AABB.$toArray()).to.eql([10, 10, 6, 6]);
+			expect(n1.$AABB.$toArray()).to.eql([11, 11, 5, 5]);
+			expect(n4.$AABB.$toArray()).to.eql([6, 9, -2, 1]);
+			expect(n3.$AABB.$toArray()).to.eql([9, 12, -5, -2]);
+			expect(n0.$AABB.$toArray()).to.eql([11, 12, -5, -2]);
+
+			n2.$setAABB(0, 0, 0, 0);
+			Processor.$run(heightTask);
+			expect(n0.$AABB.$toArray()).to.eql([9, 12, -5, -3]);
+		});
 
-		n2.$setAABB(0, 0, 0, 0);
-		Processor.$run(heightTask);
-		expect(n0.$AABB.$toArray()).to.eql([9, 12, -5, -3]);
+		it("Updates AABB when a child node is removed", function() {
+			parseTree("(0,1,1),(1,2,1),(2,3,1),(2,4,1),(0,5,1),(5,6,1)", "(6,0,0,0,0),(3,3,2,0,0),(4,5,2,0,0)");
+			const n1 = node(1)!;
+			expect(n1.$AABB.$toArray()).to.eql([5, 8, -1, 0]);
+
+			// Deleting n3 should propagate changes to n2 and then to n1
+			TreeController.removeLeaf([id3], []);
+			expect(n1.$AABB.$toArray()).to.eql([5, 8, -1, 2]);
+		});
 	});
 
 	describe("Edge joining", function() {