From 4ee8402169c959f9fccb4e52b5fa83ffb0fd946f Mon Sep 17 00:00:00 2001 From: Mu-Tsun Tsai Date: Tue, 9 Jan 2024 11:11:03 +0800 Subject: [PATCH] Fix collision detection bug related to node removal --- src/core/design/context/tree.ts | 6 ++--- src/core/design/context/treeNode.ts | 6 +++-- src/core/design/layout/partition.ts | 1 + src/core/design/tasks/aabb.ts | 2 +- src/core/design/tasks/junction.ts | 22 +++++++++++----- src/core/design/tasks/roughContour.ts | 2 +- src/core/service/state.ts | 6 ++--- test/specs/tree.spec.ts | 36 ++++++++++++++++++--------- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/core/design/context/tree.ts b/src/core/design/context/tree.ts index 461cf741..f3a9e48a 100644 --- a/src/core/design/context/tree.ts +++ b/src/core/design/context/tree.ts @@ -165,7 +165,7 @@ export class Tree implements ITree, ISerializable { 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 { 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 { private _removeEdgeAndCheckNewFlap(node: TreeNode, partner: TreeNode): void { this.$removeEdge(node.id, partner.id); if(partner.$isLeafLike) { - State.$flapAABBChanged.add(partner); + State.$nodeAABBChanged.add(partner); } } diff --git a/src/core/design/context/treeNode.ts b/src/core/design/context/treeNode.ts index e7cb6b9b..7cdfe4d2 100644 --- a/src/core/design/context/treeNode.ts +++ b/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; diff --git a/src/core/design/layout/partition.ts b/src/core/design/layout/partition.ts index 173d3f5f..712dd099 100644 --- a/src/core/design/layout/partition.ts +++ b/src/core/design/layout/partition.ts @@ -49,6 +49,7 @@ export class Partition implements ISerializable { 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; diff --git a/src/core/design/tasks/aabb.ts b/src/core/design/tasks/aabb.ts index 086e2e3b..1f64a68f 100644 --- a/src/core/design/tasks/aabb.ts +++ b/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 { diff --git a/src/core/design/tasks/junction.ts b/src/core/design/tasks/junction.ts index 63171300..a8006016 100644 --- a/src/core/design/tasks/junction.ts +++ b/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()!; diff --git a/src/core/design/tasks/roughContour.ts b/src/core/design/tasks/roughContour.ts index 1817a21b..2b367fa7 100644 --- a/src/core/design/tasks/roughContour.ts +++ b/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 diff --git a/src/core/service/state.ts b/src/core/service/state.ts index 1bc43889..ae7bfa97 100644 --- a/src/core/service/state.ts +++ b/src/core/service/state.ts @@ -83,8 +83,8 @@ export namespace State { */ export const $subtreeAABBChanged = new Set(); - /** Those flaps that have their {@link ITreeNode.$AABB AABB} changed in the current round. */ - export const $flapAABBChanged = new Set(); + /** Those nodes that have their {@link ITreeNode.$AABB AABB} proactively changed in the current round. */ + export const $nodeAABBChanged = new Set(); /** * 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(); diff --git a/test/specs/tree.spec.ts b/test/specs/tree.spec.ts index 1a438377..9ac5c896 100644 --- a/test/specs/tree.spec.ts +++ b/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() {