Remove normalization from Utils.angle as it's effectively a noop about bezierjs HOT 6 CLOSED

No it doesn't: normalisation changes a vector's length, but preserves its angle, and so as long as you make sure to use the correct origin, atan2 will do the right thing. Is there an actual bug you found with input values that do the wrong thing? If so, can you add those to the issue?

Because this should work perfectly fine, and the following test code works exactly as it should, finding the correct angle a at every iteration:

cos = Math.cos; sin = Math.sin; tau = Math.PI *2;
for (var a = 0; a<Math.PI*2; a+=0.1) {
  v1 = {x: cos(a), y: sin(a) };
  v2 = {x: cos(2*a), y: sin(2*a) };
  r = utils.angle({x:0,y:0}, v1, v2);
  // difference between v1 and v2 should be a
  console.log(Math.abs(a - (r+tau)%tau) < 0.00001);
}

This gives me true for every iteration. 63 times on a+=0.1, 629 times on a+=0.01.

(That's not to say the normalisation should be kept: if the code does the same without normalisation then there is no point in wasting time on the computation, but that's a different issue)

from bezierjs.

Your code does not show the problem because you are using unit vectors, so it does not really matter if you normalise or not.

Try the same code using lengths > 1. (the angle is the same no matter vectors length).

cos = Math.cos; sin = Math.sin; tau = Math.PI *2;
var l1 = 2, l2 = 1;
for (var a = 0; a<Math.PI*2; a+=0.1) {
  v1 = {x: l1*cos(a), y: l1*sin(a) };
  v2 = {x: l2*cos(2*a), y: l2*sin(2*a) };
  r = utils.angle({x:0,y:0}, v1, v2);
  // difference between v1 and v2 should be a
  console.log(Math.abs(a - (r+tau)%tau) < 0.00001);
}

I suggest to remove the normalisation because there is no need for using unit vectors.

from bezierjs.

updated the issue title, will be removing them later today and spinning a new release.

from bezierjs.

Perfect.

However, I want to remark this is a bug, not a noop.

If you use lengths > 1 your code will throw only 1 TRUE and FALSE for the rest. This is because cross product is equal to l1 l2 sin(angle) but dot product is equal to cos(angle). Of course if l1 and l2 are equal to 1 (as in your code) the result will be ok, but not for any others l1 and l2 values.

Function should be modified as follows:

angle: function(o,v1,v2) {
      var dx1 = v1.x - o.x,
          dy1 = v1.y - o.y,
          dx2 = v2.x - o.x,
          dy2 = v2.y - o.y,
          //cross = dx1*dy2 - dy1*dx2,
          cross, // do not compute yet. This is the bug I said
          m1 = sqrt(dx1*dx1+dy1*dy1),
          m2 = sqrt(dx2*dx2+dy2*dy2),
          dot;

          // Now you normalize (or not).
          //Now it does not change the result, so now is noop, but only because we move cross product calculation below this line
          dx1/=m1; dy1/=m1; dx2/=m2; dy2/=m2;
      
         // Now compute dot and cross
         dot = dx1*dx2 + dy1*dy2;
         cross = dx1*dy2 - dy1*dx2, // this is bug fix if you want to keep the normalization
         return atan2(cross, dot);
    },

If for any reason you prefer using unit vectors as atan2 input you should normalize them before calculating cross and dot products, not in between.

from bezierjs.

noted, thanks for the explanation.

from bezierjs.

normalization removed and v2.2.2 pushed/published

from bezierjs.