diff --git a/src/layouter/state_machine/sm_layouter.ts b/src/layouter/state_machine/sm_layouter.ts index e82e4e2c..36e3c576 100644 --- a/src/layouter/state_machine/sm_layouter.ts +++ b/src/layouter/state_machine/sm_layouter.ts @@ -360,21 +360,25 @@ export class SMLayouter { throw new Error('Failed to determine exit.'); } - // TODO: Not sure about this, this may fail in situations where the loop - // is executed conditionally and contains breaks and returns. - let subtr = 0; - for (const s of exitCandidates) - subtr += this.allDom.get(s)?.size ?? 0; - const exitRank = rank + ( - (this.allDom.get(node)?.size ?? 0) - subtr - ); - for (const s of exitCandidates) - q.push([s, exitRank]); - - // Add all successors that are not the exit candidate. - for (const n of successors) - if (!exitCandidates.has(n)) + if (exitCandidates.size > 1) { + throw new Error('Undetermined number of possible exit states.'); + } else if (exitCandidates.size === 1) { + let loopSize = 0; + const exitNode = Array.from(exitCandidates)[0]; + for (const s of successors) { + if (s !== exitNode) { + loopSize += (this.allDom.get(s)?.size ?? 0) + 1; + q.push([s, rank + 1]); + } + } + const exitRank = rank + loopSize + 1; + for (const s of exitCandidates) + q.push([s, exitRank]); + } else { + // Add all successors that are not the exit candidate. + for (const n of successors) q.push([n, rank + 1]); + } } /** @@ -386,20 +390,16 @@ export class SMLayouter { const q: [string, number][] = [[this.startNode, 0]]; const scopes: [ScopeType, number, number][] = []; - const visited = new Set(); const rankings = new Map(); while (q.length > 0) { const [node, rank] = q.shift()!; - if (!visited.has(node)) { - const backedges = this.backedgesDstDict.get(node) ?? new Set(); + if (rankings.has(node)) { + rankings.set(node, Math.max(rankings.get(node)!, rank)); + } else { + rankings.set(node, rank); - // Assign the rank for the current node (passed along in the - // queue). - if (rankings.has(node)) - rankings.set(node, Math.max(rankings.get(node)!, rank)); - else - rankings.set(node, rank); + const backedges = this.backedgesDstDict.get(node) ?? new Set(); // Gather all successors that are not reached through backedges. const successors: string[] = []; @@ -443,8 +443,6 @@ export class SMLayouter { } else { this.propagate(node, successors, rank, q, scopes); } - - visited.add(node); } } diff --git a/tests/unit/layouter/state_machine/sm_layouter.test.ts b/tests/unit/layouter/state_machine/sm_layouter.test.ts index c620a04b..4c76bc7f 100644 --- a/tests/unit/layouter/state_machine/sm_layouter.test.ts +++ b/tests/unit/layouter/state_machine/sm_layouter.test.ts @@ -218,7 +218,7 @@ function testSelfLoop(): void { expect(graph2.get('3')?.rank).toBe(3); } -function testMultiInvertedSkipLops(): void { +function testMultiInvertedSkipLoops(): void { const graph = new DiGraph(); // Construct graph. @@ -274,6 +274,36 @@ function testMultiInvertedSkipLops(): void { expect(graph.get('17')?.rank).toBe(9); } +function testSkipLoopWithBreakReturn(): void { + const graph = new DiGraph(); + + // Construct graph. + + constructEdge(graph, '0', '1'); + constructEdge(graph, '1', '2'); + constructEdge(graph, '1', '6'); + constructEdge(graph, '2', '3'); + constructEdge(graph, '2', '6'); + constructEdge(graph, '3', '4'); + constructEdge(graph, '3', '6'); + constructEdge(graph, '4', '5'); + constructEdge(graph, '4', '7'); + constructEdge(graph, '5', '2'); + constructEdge(graph, '6', '7'); + + const layouter = new SMLayouter(graph); + layouter.doLayout(); + + expect(graph.get('0')?.rank).toBe(0); + expect(graph.get('1')?.rank).toBe(1); + expect(graph.get('2')?.rank).toBe(2); + expect(graph.get('3')?.rank).toBe(3); + expect(graph.get('4')?.rank).toBe(4); + expect(graph.get('5')?.rank).toBe(5); + expect(graph.get('6')?.rank).toBe(6); + expect(graph.get('7')?.rank).toBe(7); +} + describe('Test vertical state machine layout ranking', () => { test('Basic branching', testBasicBranching); test('Nested branching', testNestedBranching); @@ -284,7 +314,11 @@ describe('Test vertical state machine layout ranking', () => { test('Test self loops', testSelfLoop); test( 'Test multiple inverted loops with skip edges', - testMultiInvertedSkipLops + testMultiInvertedSkipLoops + ); + test( + 'Test conditionally skipped loop with break and returns', + testSkipLoopWithBreakReturn ); });