Browse Source
Network: Fixes sorting on Layout, refactoring. (#3411)
* Network: Fixes sorting on Layout sorting, refactoring. Fix for #3403 Chromium has divergent behaviour on sorting of `undefined` values, a check has been added for these. The fix itself is small, it adds a check on `undefined` in the sorting function(s). In addition: - Fixed off-by-one error on tree index - Refactored away checks on visibility by replacing `_isVisible()` with a Strategy pattern for direction The latter removes a long-standing eyesore in `LayoutManager`; it's probably also faster. There is some hope that these fixes will improve the hierarchical layout initialization, which has had problems for quite a while. * reformat code block * Add next attempt to fix Travis unit test bug * Fix unit test - timeout * Adjustments for comment reviews * Moved direction strategies to separate module; added fix for 'window is undefined'revert-3409-performance
wimrijnders
7 years ago
committed by
Yotam Berkowitz
Yotam Berkowitz
4 changed files with 309 additions and 160 deletions
Split View
Diff Options
-
+12 -25lib/network/modules/CanvasRenderer.js
-
+52 -134lib/network/modules/LayoutEngine.js
-
+244 -0lib/network/modules/components/DirectionStrategy.js
-
+1 -1test/Network.test.js
+ 12
- 25
lib/network/modules/CanvasRenderer.js
View File
+ 52
- 134
lib/network/modules/LayoutEngine.js
View File
+ 244
- 0
lib/network/modules/components/DirectionStrategy.js
View File
| @ -0,0 +1,244 @@ | |||
| /** | |||
| * Helper classes for LayoutEngine. | |||
| * | |||
| * Strategy pattern for usage of direction methods for hierarchical layouts. | |||
| */ | |||
| /** | |||
| * Interface definition for direction strategy classes. | |||
| * | |||
| * This class describes the interface for the Strategy | |||
| * pattern classes used to differentiate horizontal and vertical | |||
| * direction of hierarchical results. | |||
| * | |||
| * For a given direction, one coordinate will be 'fixed', meaning that it is | |||
| * determined by level. | |||
| * The other coordinate is 'unfixed', meaning that the nodes on a given level | |||
| * can still move along that coordinate. So: | |||
| * | |||
| * - `vertical` layout: `x` unfixed, `y` fixed per level | |||
| * - `horizontal` layout: `x` fixed per level, `y` unfixed | |||
| * | |||
| * The local methods are stubs and should be regarded as abstract. | |||
| * Derived classes **must** implement all the methods themselves. | |||
| * | |||
| * @private | |||
| */ | |||
| class DirectionInterface { | |||
| /** @ignore **/ | |||
| abstract() { | |||
| throw new Error("Can't instantiate abstract class!"); | |||
| } | |||
| /** | |||
| * This is a dummy call which is used to suppress the jsdoc errors of type: | |||
| * | |||
| * "'param' is assigned a value but never used" | |||
| * | |||
| * @ignore | |||
| **/ | |||
| fake_use() { | |||
| // Do nothing special | |||
| } | |||
| /** | |||
| * Type to use to translate dynamic curves to, in the case of hierarchical layout. | |||
| * Dynamic curves do not work for these. | |||
| * | |||
| * The value should be perpendicular to the actual direction of the layout. | |||
| * | |||
| * @return {string} Direction, either 'vertical' or 'horizontal' | |||
| */ | |||
| curveType() { return this.abstract(); } | |||
| /** | |||
| * Return the value of the coordinate that is not fixed for this direction. | |||
| * | |||
| * @param {Node} node The node to read | |||
| * @return {number} Value of the unfixed coordinate | |||
| */ | |||
| getPosition(node) { this.fake_use(node); return this.abstract(); } | |||
| /** | |||
| * Set the value of the coordinate that is not fixed for this direction. | |||
| * | |||
| * @param {Node} node The node to adjust | |||
| * @param {number} position | |||
| * @param {number} [level] if specified, the hierarchy level that this node should be fixed to | |||
| */ | |||
| setPosition(node, position, level = undefined) { this.fake_use(node, position, level); this.abstract(); } | |||
| /** | |||
| * Get the width of a tree. | |||
| * | |||
| * A `tree` here is a subset of nodes within the network which are not connected to other nodes, | |||
| * only among themselves. In essence, it is a sub-network. | |||
| * | |||
| * @param {number} index The index number of a tree | |||
| * @return {number} the width of a tree in the view coordinates | |||
| */ | |||
| getTreeSize(index) { this.fake_use(index); return this.abstract(); } | |||
| /** | |||
| * Sort array of nodes on the unfixed coordinates. | |||
| * | |||
| * @param {Array.<Node>} nodeArray array of nodes to sort | |||
| */ | |||
| sort(nodeArray) { this.fake_use(nodeArray); this.abstract(); } | |||
| /** | |||
| * Assign the fixed coordinate of the node to the given level | |||
| * | |||
| * @param {Node} node The node to adjust | |||
| * @param {number} level The level to fix to | |||
| */ | |||
| fix(node, level) { this.fake_use(node, level); this.abstract(); } | |||
| /** | |||
| * Add an offset to the unfixed coordinate of the given node. | |||
| * | |||
| * @param {NodeId} nodeId Id of the node to adjust | |||
| * @param {number} diff Offset to add to the unfixed coordinate | |||
| */ | |||
| shift(nodeId, diff) { this.fake_use(nodeId, diff); this.abstract(); } | |||
| } | |||
| /** | |||
| * Vertical Strategy | |||
| * | |||
| * Coordinate `y` is fixed on levels, coordinate `x` is unfixed. | |||
| * | |||
| * @extends DirectionInterface | |||
| * @private | |||
| */ | |||
| class VerticalStrategy extends DirectionInterface { | |||
| /** | |||
| * Constructor | |||
| * | |||
| * @param {Object} layout reference to the parent LayoutEngine instance. | |||
| */ | |||
| constructor(layout) { | |||
| super(); | |||
| this.layout = layout; | |||
| } | |||
| /** @inheritdoc */ | |||
| curveType() { | |||
| return 'horizontal'; | |||
| } | |||
| /** @inheritdoc */ | |||
| getPosition(node) { | |||
| return node.x; | |||
| } | |||
| /** @inheritdoc */ | |||
| setPosition(node, position, level = undefined) { | |||
| if (level !== undefined) { | |||
| this.layout.hierarchical.addToOrdering(node, level); | |||
| } | |||
| node.x = position; | |||
| } | |||
| /** @inheritdoc */ | |||
| getTreeSize(index) { | |||
| let res = this.layout.hierarchical.getTreeSize(this.layout.body.nodes, index); | |||
| return {min: res.min_x, max: res.max_x}; | |||
| } | |||
| /** @inheritdoc */ | |||
| sort(nodeArray) { | |||
| nodeArray.sort(function(a, b) { | |||
| // Test on 'undefined' takes care of divergent behaviour in chrome | |||
| if (a.x === undefined || b.x === undefined) return 0; // THIS HAPPENS | |||
| return a.x - b.x; | |||
| }); | |||
| } | |||
| /** @inheritdoc */ | |||
| fix(node, level) { | |||
| node.y = this.layout.options.hierarchical.levelSeparation * level; | |||
| node.options.fixed.y = true; | |||
| } | |||
| /** @inheritdoc */ | |||
| shift(nodeId, diff) { | |||
| this.layout.body.nodes[nodeId].x += diff; | |||
| } | |||
| } | |||
| /** | |||
| * Horizontal Strategy | |||
| * | |||
| * Coordinate `x` is fixed on levels, coordinate `y` is unfixed. | |||
| * | |||
| * @extends DirectionInterface | |||
| * @private | |||
| */ | |||
| class HorizontalStrategy extends DirectionInterface { | |||
| /** | |||
| * Constructor | |||
| * | |||
| * @param {Object} layout reference to the parent LayoutEngine instance. | |||
| */ | |||
| constructor(layout) { | |||
| super(); | |||
| this.layout = layout; | |||
| } | |||
| /** @inheritdoc */ | |||
| curveType() { | |||
| return 'vertical'; | |||
| } | |||
| /** @inheritdoc */ | |||
| getPosition(node) { | |||
| return node.y; | |||
| } | |||
| /** @inheritdoc */ | |||
| setPosition(node, position, level = undefined) { | |||
| if (level !== undefined) { | |||
| this.layout.hierarchical.addToOrdering(node, level); | |||
| } | |||
| node.y = position; | |||
| } | |||
| /** @inheritdoc */ | |||
| getTreeSize(index) { | |||
| let res = this.layout.hierarchical.getTreeSize(this.layout.body.nodes, index); | |||
| return {min: res.min_y, max: res.max_y}; | |||
| } | |||
| /** @inheritdoc */ | |||
| sort(nodeArray) { | |||
| nodeArray.sort(function(a, b) { | |||
| // Test on 'undefined' takes care of divergent behaviour in chrome | |||
| if (a.y === undefined || b.y === undefined) return 0; // THIS HAPPENS | |||
| return a.y - b.y; | |||
| }); | |||
| } | |||
| /** @inheritdoc */ | |||
| fix(node, level) { | |||
| node.x = this.layout.options.hierarchical.levelSeparation * level; | |||
| node.options.fixed.x = true; | |||
| } | |||
| /** @inheritdoc */ | |||
| shift(nodeId, diff) { | |||
| this.layout.body.nodes[nodeId].y += diff; | |||
| } | |||
| } | |||
| export {HorizontalStrategy, VerticalStrategy}; | |||