Skip to content

Commit 4bbc33d

Browse files
committed
Fix node sorting algorithm producing poor results. Avoid needless computation of global outer boxes when a layout is performed.
1 parent 8e74b09 commit 4bbc33d

File tree

2 files changed

+30
-24
lines changed

2 files changed

+30
-24
lines changed

lib/src/api/node_processor.dart

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,18 @@ class NodeProcessor {
247247
return parents;
248248
}
249249

250-
/// Sorted nodes is a list of nodes that go from parent -> child.
250+
/// Sorted nodes is a list of nodes that go from parent -> child using
251+
/// a merge sort with memoization.
251252
static void sortNodes(List<BaseNode> nodes) {
252253
// We cache the parent chains for each node to avoid
253254
// re-computing them multiple times as an optimization.
254-
final Map<BaseNode, List<String>> parentChains = {};
255+
// This is also known as memoization.
256+
final Map<BaseNode, List<String>> parentChains = {
257+
for (final node in nodes)
258+
if (node.id != kRootNode) node: _allParentsOfNode(node),
259+
};
255260

256-
nodes.sort((a, b) {
261+
mergeSort(nodes, compare: (a, b) {
257262
if (a.id == kRootNode) return -1;
258263
if (b.id == kRootNode) return 1;
259264

@@ -265,23 +270,18 @@ class NodeProcessor {
265270
bParent.childrenOrEmpty.indexOf(b.id);
266271
}
267272

268-
parentChains.putIfAbsent(a, () => _allParentsOfNode(a));
269-
parentChains.putIfAbsent(b, () => _allParentsOfNode(b));
273+
final List<String> aParents = parentChains[a]!;
274+
final List<String> bParents = parentChains[b]!;
270275

271-
final List<String> aParents = parentChains[a] ?? const [];
272-
final List<String> bParents = parentChains[b] ?? const [];
276+
final bool aParentOfB = bParents.contains(a.id);
277+
final bool bParentOfA = aParents.contains(b.id);
273278

274-
final int aIndex = bParents.indexOf(a.id);
275-
final int bIndex = aParents.indexOf(b.id);
276-
277-
if (aIndex == -1 && bIndex == -1) {
278-
return bParents.indexOf(kRootNode) - aParents.indexOf(kRootNode);
279-
} else if (bIndex == -1) {
280-
return -1;
281-
} else if (aIndex == -1) {
279+
if (bParentOfA) {
282280
return 1;
281+
} else if (aParentOfB) {
282+
return -1;
283283
} else {
284-
return bIndex - aIndex;
284+
return aParents.length - bParents.length;
285285
}
286286
});
287287
}
@@ -427,6 +427,7 @@ class NodeProcessor {
427427
recursivelyCalculateChildren:
428428
!performLayoutRan && recursivelyCalculateChildrenGlobalBoxes,
429429
updatingSortedNodeList: updatingSortedNodeList,
430+
didPerformLayout: performLayoutRan,
430431
);
431432

432433
_computeInnerBoxLocal(node);
@@ -436,14 +437,19 @@ class NodeProcessor {
436437
BaseNode node, {
437438
bool recursivelyCalculateChildren = true,
438439
bool updatingSortedNodeList = false,
440+
bool didPerformLayout = false,
439441
}) {
440442
// Order matters.
441443
// Middle and inner boxes depend on outer.
442444
// Rotated boxes depend on all of them.
443-
_computeOuterBoxGlobal(
444-
node,
445-
updatingSortedNodeList: updatingSortedNodeList,
446-
);
445+
// If a perform layout was run, then the layout system produced new
446+
// global outer boxes. No need to recompute them.
447+
if (!didPerformLayout) {
448+
_computeOuterBoxGlobal(
449+
node,
450+
updatingSortedNodeList: updatingSortedNodeList,
451+
);
452+
}
447453
_computeMiddleBoxGlobal(node);
448454
_computeInnerBoxGlobal(node);
449455

@@ -458,6 +464,7 @@ class NodeProcessor {
458464
_computeGlobalAndRotatedBoxes(
459465
childNode,
460466
updatingSortedNodeList: updatingSortedNodeList,
467+
didPerformLayout: didPerformLayout,
461468
);
462469
}
463470
}
@@ -571,7 +578,7 @@ class NodeProcessor {
571578
/// top-left corner of the bounding box.
572579
///
573580
/// if [updatingSortedNodeList] is true, then you are currently updating a
574-
/// list of nodes one-by-one order from parent to child order. If this is
581+
/// list of nodes one-by-one from parent to child order. If this is
575582
/// the case, then we can make important assumptions that can help optimize
576583
/// and avoid recursive computation.
577584
static Vec calculateGlobalRotatedBoxTopLeft(
@@ -698,7 +705,7 @@ class NodeProcessor {
698705
/// top-left corner of the bounding box.
699706
///
700707
/// This function is used when you are updating a list of nodes one-by-one
701-
/// order from parent to child order. If this is the case, then we can make
708+
/// from parent to child order. If this is the case, then we can make
702709
/// important assumptions that can help optimize and avoid recursive
703710
/// computation.
704711
static Vec calculateSortedGlobalRotatedBoxTopLeft(
@@ -724,8 +731,6 @@ class NodeProcessor {
724731
: boundaryType.getGlobalBoxForNode(parent);
725732
final NodeBox nodeBox = boundaryType.getLocalBoxForNode(node);
726733

727-
currentVec = Vec.zero;
728-
729734
if (node.globalRotationDegrees == 0) {
730735
currentVec = parentBox is OuterNodeBox
731736
? parentBox.innerTopLeft

lib/src/api/nodes/base_node.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import 'dart:math';
44

55
import 'package:codelessly_json_annotation/codelessly_json_annotation.dart';
6+
import 'package:collection/collection.dart';
67
import 'package:equatable/equatable.dart';
78

89
import '../constants.dart';

0 commit comments

Comments
 (0)