Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added Alternative DependencySorter to deal with cycles #219

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added Alternative DependencySorter to deal with cycles #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aad97b3
Added an alternative dependency sorter that will attempt to return a …
mjtalbot Jan 8, 2020
af6a7da
Replaced dependency sorter with a sorter using Tarjans algorithm to d…
mjtalbot Jan 9, 2020
8aad656
cleaned up tests, and clearly highlight how isolated nodes will be tr…
mjtalbot Jan 9, 2020
5451394
Minor tweak to keep the default sorter simpler and renamed the test f…
luigi-rosso Jan 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions flare_dart/lib/actor_component.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ abstract class ActorComponent {
return _name;
}

set name(String name) {
_name = name;
}

void resolveComponentIndices(List<ActorComponent> components) {
ActorNode node = components[_parentIdx] as ActorNode;
if (node != null) {
Expand Down
62 changes: 61 additions & 1 deletion flare_dart/lib/dependency_sorter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import "dart:collection";

import "package:graphs/graphs.dart";

import "actor_component.dart";

class DependencySorter {
Expand All @@ -24,7 +27,7 @@ class DependencySorter {
return true;
}
if (_temp.contains(n)) {
print("Dependency cycle!");
// cycle detected!
return false;
}

Expand All @@ -44,3 +47,60 @@ class DependencySorter {
return true;
}
}

/// Sorts dependencies for Actors even when cycles are present
///
/// Any nodes that form part of a cycle can be found in `cycleNodes` after
/// `sort`. NOTE: Nodes isolated by cycles will not be found in `_order` or
/// `cycleNodes` e.g. `A -> B <-> C -> D` isolates D when running a sort based
/// on A
mjtalbot marked this conversation as resolved.
Show resolved Hide resolved
class TarjansDependencySorter extends DependencySorter {
HashSet<ActorComponent> _cycleNodes;
HashSet<ActorComponent> get cycleNodes => _cycleNodes;

TarjansDependencySorter() {
_perm = HashSet<ActorComponent>();
_temp = HashSet<ActorComponent>();
_cycleNodes = HashSet<ActorComponent>();
}

@override
bool visit(ActorComponent n) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh this is nicer!

if (cycleNodes.contains(n)) {
// skip any nodes on a known cycle.
return true;
}

return super.visit(n);
}

@override
List<ActorComponent> sort(ActorComponent root) {
_order = <ActorComponent>[];

if (!visit(root)) {
// if we detect cycles, go find them all
_perm.clear();
_temp.clear();
_cycleNodes.clear();
_order.clear();

var cycles = stronglyConnectedComponents<ActorComponent>(
[root], (ActorComponent node) => node.dependents);

cycles.forEach((cycle) {
// cycles of len 1 are not cycles.
if (cycle.length > 1) {
cycle.forEach((cycleMember) {
_cycleNodes.add(cycleMember);
});
}
});

// revisit the tree, skipping nodes on any cycle.
visit(root);
}

return _order;
}
}
5 changes: 4 additions & 1 deletion flare_dart/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@ author: "Rive Team <[email protected]>"
homepage: https://github.com/2d-inc/Flare-Flutter
environment:
sdk: ">=2.1.0 <3.0.0"

dependencies:
graphs: ^0.2.0
dev_dependencies:
test: ^1.9.4
259 changes: 259 additions & 0 deletions flare_dart/test/dependency_sorter_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
import 'dart:typed_data';

import 'package:flare_dart/actor_artboard.dart';
import 'package:flare_dart/actor_color.dart';
import 'package:flare_dart/actor_component.dart';
import 'package:flare_dart/actor_drop_shadow.dart';
import 'package:flare_dart/actor_inner_shadow.dart';
import 'package:flare_dart/actor_layer_effect_renderer.dart';
import 'package:flare_dart/actor_node.dart';
import 'package:flare_dart/actor.dart';
import 'package:flare_dart/dependency_sorter.dart';

import 'package:test/test.dart';

class DummyActor extends Actor {
@override
Future<bool> loadAtlases(List<Uint8List> rawAtlases) {
throw UnimplementedError();
}

@override
ColorFill makeColorFill() {
throw UnimplementedError();
}

@override
ColorStroke makeColorStroke() {
throw UnimplementedError();
}

@override
ActorDropShadow makeDropShadow() {
throw UnimplementedError();
}

@override
GradientFill makeGradientFill() {
throw UnimplementedError();
}

@override
GradientStroke makeGradientStroke() {
throw UnimplementedError();
}

@override
ActorInnerShadow makeInnerShadow() {
throw UnimplementedError();
}

@override
ActorLayerEffectRenderer makeLayerEffectRenderer() {
throw UnimplementedError();
}

@override
RadialGradientFill makeRadialFill() {
throw UnimplementedError();
}

@override
RadialGradientStroke makeRadialStroke() {
throw UnimplementedError();
}

@override
Future<Uint8List> readOutOfBandAsset(String filename, context) {
throw UnimplementedError();
}
}

String orderString(List<ActorComponent> order) {
String output = order.fold(
'',
(previousValue, actorComponent) =>
previousValue + ' ' + actorComponent.name);
return output.trim();
}

void main() {
group("Simple Cycle:", () {
ActorArtboard artboard;
final actor = DummyActor();
artboard = ActorArtboard(actor);
final nodeA = ActorNode()..name = 'A';
final nodeB = ActorNode()..name = 'B';
final nodeC = ActorNode()..name = 'C';
final nodeD = ActorNode()..name = 'D';

///
/// [root] <- [A] <- [B] <- [D]
/// A | A
/// | +------+
/// [C]
artboard.addDependency(nodeA, artboard.root);
artboard.addDependency(nodeB, nodeA);
artboard.addDependency(nodeC, nodeA);
artboard.addDependency(nodeD, nodeB);
artboard.addDependency(nodeB, nodeD);

test("DependencySorter cannot order", () {
expect(DependencySorter().sort(artboard.root), equals(null));
});

test("TarjansDependencySorter orders", () {
var order = TarjansDependencySorter().sort(artboard.root);
expect(order.length, equals(3));
expect(orderString(order), equals('Unnamed A C'));
});
});
group("No cycle:", () {
final actor = DummyActor();
final artboard = ActorArtboard(actor);
final nodeA = ActorNode()..name = 'A';
final nodeB = ActorNode()..name = 'B';
final nodeC = ActorNode()..name = 'C';
final nodeD = ActorNode()..name = 'D';

///
/// [root] <- [A] <- [B] <- [D]
/// A A
/// | |
/// [C]-----+
artboard.addDependency(nodeA, artboard.root);
artboard.addDependency(nodeB, nodeA);
artboard.addDependency(nodeC, nodeA);
artboard.addDependency(nodeD, nodeB);
artboard.addDependency(nodeC, nodeB);

test("DependencySorter orders", () {
var order = DependencySorter().sort(artboard.root);
expect(order, isNotNull);
expect(orderString(order), 'Unnamed A B C D');
});

test("DependencySorter orders", () {
var order = TarjansDependencySorter().sort(artboard.root);
expect(order, isNotNull);
expect(orderString(order), 'Unnamed A B C D');
});
});

group("Complex Cycle A:", () {
final actor = DummyActor();
final artboard = ActorArtboard(actor);
final nodeA = ActorNode()..name = 'A';
final nodeB = ActorNode()..name = 'B';
final nodeC = ActorNode()..name = 'C';
final nodeD = ActorNode()..name = 'D';

///
/// +------+
/// | v
/// [root] <- [A] <- [B] <- [D]
/// A |
/// | |
/// [C]<-----------+
///
artboard.addDependency(nodeA, artboard.root);
artboard.addDependency(nodeB, nodeA);
artboard.addDependency(nodeD, nodeB);
artboard.addDependency(nodeB, nodeD);
artboard.addDependency(nodeC, nodeA);
artboard.addDependency(nodeD, nodeC);

test("DependencySorter cannot order", () {
expect(DependencySorter().sort(artboard.root), equals(null));
});

test("TarjansDependencySorter orders", () {
var order = TarjansDependencySorter().sort(artboard.root);
expect(order, isNotNull);
expect(orderString(order), equals('Unnamed A C'));
});
});

group("Complex Cycle B, F is isolated:", () {
ActorArtboard artboard;

final actor = DummyActor();
artboard = ActorArtboard(actor);
final nodeA = ActorNode()..name = 'A';
final nodeB = ActorNode()..name = 'B';
final nodeC = ActorNode()..name = 'C';
final nodeD = ActorNode()..name = 'D';
final nodeE = ActorNode()..name = 'E';
final nodeF = ActorNode()..name = 'F';

///
/// +-------------+
/// | |
/// | [F] |
/// | v v
/// [root] <- [A] <- [B] <- [C] <- [D]
/// A |
/// | |
/// [E]<-----------+
///
artboard.addDependency(nodeA, artboard.root);
artboard.addDependency(nodeB, nodeA);
artboard.addDependency(nodeC, nodeB);
artboard.addDependency(nodeD, nodeC);
artboard.addDependency(nodeB, nodeD);
artboard.addDependency(nodeE, nodeA);
artboard.addDependency(nodeC, nodeE);
artboard.addDependency(nodeF, nodeC);

test("TarjansDependencySorter orders", () {
var dependencySorter = TarjansDependencySorter();
var order = dependencySorter.sort(artboard.root);
expect(orderString(order), equals('Unnamed A E'));
expect(dependencySorter.cycleNodes, containsAll([nodeB, nodeC, nodeD]));
expect(dependencySorter.cycleNodes, contains(nodeF),
skip: "Node F is isolated by a cycle, and does not "
" exist in 'order' or in 'cycleNodes'");
});
});

group("Complex Cycle C, F is not isolated:", () {
ActorArtboard artboard;

final actor = DummyActor();
artboard = ActorArtboard(actor);
final nodeA = ActorNode()..name = 'A';
final nodeB = ActorNode()..name = 'B';
final nodeC = ActorNode()..name = 'C';
final nodeD = ActorNode()..name = 'D';
final nodeE = ActorNode()..name = 'E';
final nodeF = ActorNode()..name = 'F';

///
/// +---------------+
/// | |
/// | [F]---+ |
/// | v | v
/// [root] <- [A] <- [B] <- [C] <-+- [D]
/// A | |
/// | | |
/// [E]<-----------+ |
/// A |
/// +------------------+
artboard.addDependency(nodeA, artboard.root);
artboard.addDependency(nodeB, nodeA);
artboard.addDependency(nodeC, nodeB);
artboard.addDependency(nodeD, nodeC);
artboard.addDependency(nodeB, nodeD);
artboard.addDependency(nodeE, nodeA);
artboard.addDependency(nodeC, nodeE);
artboard.addDependency(nodeF, nodeC);
artboard.addDependency(nodeF, nodeE);

test("TarjansDependencySorter orders", () {
var dependencySorter = TarjansDependencySorter();
var order = dependencySorter.sort(artboard.root);
expect(orderString(order), equals('Unnamed A E F'));
expect(dependencySorter.cycleNodes, containsAll([nodeB, nodeC, nodeD]));
});
});
}