Permalink
Browse files

Merge branch 'review'

* review:
  Avoid divide by zero and massive forces at small distances
  Renamed f -> a. f is actually representing acceleration.
  A few updates based on code review by amcameron
  add review comments for springyui.js
  Add some code review comments.
  • Loading branch information...
2 parents 84bf82a + 686abaa commit 9a196f8673cadd4c6808c20e60a1bb8226c364bc @dhotson committed Nov 26, 2011
Showing with 18 additions and 6 deletions.
  1. +11 −6 springy.js
  2. +7 −0 springyui.js
View
@@ -310,7 +310,7 @@ Layout.ForceDirected.prototype.applyCoulombsLaw = function() {
if (point1 !== point2)
{
var d = point1.p.subtract(point2.p);
- var distance = d.magnitude() + 1.0;
+ var distance = d.magnitude() + 0.1; // avoid massive forces at small distances (and divide by zero)
var direction = d.normalise();
// apply force to each end point
@@ -343,22 +343,27 @@ Layout.ForceDirected.prototype.attractToCentre = function() {
Layout.ForceDirected.prototype.updateVelocity = function(timestep) {
this.eachNode(function(node, point) {
- point.v = point.v.add(point.f.multiply(timestep)).multiply(this.damping);
- point.f = new Vector(0,0);
+ // Is this, along with updatePosition below, the only places that your
+ // integration code exist?
+ point.v = point.v.add(point.a.multiply(timestep)).multiply(this.damping);
+ point.a = new Vector(0,0);
});
};
Layout.ForceDirected.prototype.updatePosition = function(timestep) {
this.eachNode(function(node, point) {
+ // Same question as above; along with updateVelocity, is this all of
+ // your integration code?
point.p = point.p.add(point.v.multiply(timestep));
});
};
+// Calculate the total kinetic energy of the system
Layout.ForceDirected.prototype.totalEnergy = function(timestep) {
var energy = 0.0;
this.eachNode(function(node, point) {
var speed = point.v.magnitude();
- energy += speed * speed;
+ energy += 0.5 * point.m * speed * speed;
});
return energy;
@@ -488,11 +493,11 @@ Layout.ForceDirected.Point = function(position, mass) {
this.p = position; // position
this.m = mass; // mass
this.v = new Vector(0, 0); // velocity
- this.f = new Vector(0, 0); // force
+ this.a = new Vector(0, 0); // acceleration
};
Layout.ForceDirected.Point.prototype.applyForce = function(force) {
- this.f = this.f.add(force.divide(this.m));
+ this.a = this.a.add(force.divide(this.m));
};
// Spring
View
@@ -34,6 +34,7 @@ jQuery.fn.springy = function(params) {
var canvas = this[0];
var ctx = canvas.getContext("2d");
+
var layout = this.layout = new Layout.ForceDirected(graph, stiffness, repulsion, damping);
// calculate bounding box of graph layout.. with ease-in
@@ -82,6 +83,8 @@ jQuery.fn.springy = function(params) {
selected = nearest = dragged = layout.nearest(p);
if (selected.node !== null) {
+ // Part of the same bug mentioned later. Store the previous mass
+ // before upscaling it for dragging.
dragged.point.m = 10000.0;
}
@@ -102,6 +105,9 @@ jQuery.fn.springy = function(params) {
});
jQuery(window).bind('mouseup',function(e) {
+ // Bug! Node's mass isn't reset on mouseup. Nodes which have been
+ // dragged don't repulse very well. Store the initial mass in mousedown
+ // and then restore it here.
dragged = null;
});
@@ -122,6 +128,7 @@ jQuery.fn.springy = function(params) {
};
Node.prototype.getHeight = function() {
+ // Magic number with no explanation.
return 20;
};

0 comments on commit 9a196f8

Please sign in to comment.