Add Vector2.rotatedAbout

[samreid]

CCK has this code in LightBulb.js

  // See http://stackoverflow.com/questions/12161277/how-to-rotate-a-vertex-around-a-certain-point
  // TODO: move to Vector2?
  var rotatedAbout = function( point, origin, angle ) {
    var dx = (point.x - origin.x);
    var dy = (point.y - origin.y);
    var cos = Math.cos( angle );
    var sin = Math.sin( angle );
    return new Vector2(
      origin.x + dx * cos - dy * sin,
      origin.y + dx * sin + dy * cos
    );
  };

@jonathanolson do you think it is appropriate to move this function to dot? If so, what should the exact signature be?

[jonathanolson]

Moving it to dot sounds good (Node's rotateAround can't be used here it seems).

Presumably:

• Vector2 method rotateAboutPoint/rotateAroundPoint, mutable: function( origin, angle )
• Vector2 method rotatedAboutPoint/rotatedAroundPoint, immutable: function( origin, angle )

For higher-dimension cases (Vector3/Vector4), we'd presumably handle rotating around an axis, with a different name and handling (don't need right now).

Implementation looks good (already optimized).

Let me know if I should add this, or if you'd like to.

[samreid]

I'm happy to proceed but wanted to briefly discuss the naming discrepancy between Node.rotateAround and the proposed method names for Vector2. Is it desirable for them to differ? Should one be changed so they match up?

[samreid]

Why did you say:

Vector2 method rotateAboutPoint/rotateAroundPoint?

Did you mean rotateAbout/rotateAboutPoint, where the first one would take x,y and the 2nd would take vector2?

[jonathanolson]

Did you mean rotateAbout/rotateAboutPoint, where the first one would take x,y and the 2nd would take vector2?

Just meant I wasn't sure whether 'about' or 'around' should be used, ideally it would match Node's naming also.

Did you mean rotateAbout/rotateAboutPoint, where the first one would take x,y and the 2nd would take vector2?

It may be desirable to have multiple independent functions for this?

inherit( Object, Vector2, {
  // ...
  rotateAboutXY: function( x, y, angle ) { ... mutable logic... },
  rotateAboutPoint: function( point, angle ) { return this.rotateAboutXY( point.x, point.y, angle ); },
  rotatedAboutXY: function( x, y, angle ) { ... immutable logic ... },
  rotatedAboutPoint: function( point, angle ) { return this.rotatedAboutXY( point.x, point.y, angle ); }
} );

[samreid]

Rough draft committed above, @jonathanolson would you mind spot checking it to see if I've forgotten anything?

[jonathanolson]

Some quick notes (I'd handle but dot is getting checked out to many other branches right now):

1. Needs visibility annotations
2. @param {Vector2} angle should be number on rotatedAboutPoint
3. rotatedAboutPoint and rotateAboutXY missing returns annotation
4. use @returns instead of @return
5. Instead of "Mutable method that rotates a vector about an x,y point.", note that it mutates THIS vector.
6. I was expecting the immutable version to use the mutable version (so mutable has better performance and no GCs), not the other way around.

[samreid]

Thanks for the review @jonathanolson, I think I addressed the issues in the above 2 commits. Can you please double check?

[jonathanolson]

Looks good to me, feel free to close if that's everything you needed.