phetsims / kite

A library for creating, manipulating and displaying 2D shapes in JavaScript.
http://scenerystack.org/
MIT License
15 stars 6 forks source link

Excessive Vector2 instance allocation at Cubic.js #53

Closed AshrafSharf closed 9 years ago

AshrafSharf commented 9 years ago

@jonathanolson I did some profiling on Gene Expression Basics and found three methods in Cubic.js is particularly generating too many Vector2 instances. I have noticed, you have already marked as TODO. Have referenced these methods below.


degreeReduced: function( epsilon ) {
      epsilon = epsilon || 0; // if not provided, use an exact version
      // TODO: allocation reduction
      // TODO: performance: don't divide both by 2 here, combine it later!!
      var controlA = this._control1.timesScalar( 3 ).minus( this._start ).dividedScalar( 2 );
      var controlB = this._control2.timesScalar( 3 ).minus( this._end ).dividedScalar( 2 );
      if ( controlA.minus( controlB ).magnitude() <= epsilon ) {
        return new kite.Quadratic(
          this._start,
          controlA.average( controlB ), // average the control points for stability. they should be almost identical
          this._end
        );
      }
      else {
        // the two options for control points are too far away, this curve isn't easily reducible.
        return null;
      }
    }

tangentAt: function( t ) {
      var mt = 1 - t;
      return this._start.times( -3 * mt * mt ).plus( this._control1.times( 3 * mt * mt - 6 * mt * t ) ).plus( this._control2.times( 6 * mt * t - 3 * t * t ) ).plus( this._end.times( 3 * t * t ) );
    },

 // t value for the cusp, and the related determinant and inflection points
    computeCuspInfo: function() {
      // from http://www.cis.usouthal.edu/~hain/general/Publications/Bezier/BezierFlattening.pdf
      // TODO: allocation reduction
      var a = this._start.times( -1 ).plus( this._control1.times( 3 ) ).plus( this._control2.times( -3 ) ).plus( this._end );
      var b = this._start.times( 3 ).plus( this._control1.times( -6 ) ).plus( this._control2.times( 3 ) );
      var c = this._start.times( -3 ).plus( this._control1.times( 3 ) );

      var aPerp = a.perpendicular();
      var bPerp = b.perpendicular();
      var aPerpDotB = aPerp.dot( b );

      this._tCusp = -0.5 * ( aPerp.dot( c ) / aPerpDotB );
      this._tDeterminant = this._tCusp * this._tCusp - ( 1 / 3 ) * ( bPerp.dot( c ) / aPerpDotB );
      if ( this._tDeterminant >= 0 ) {
        var sqrtDet = Math.sqrt( this._tDeterminant );
        this._tInflection1 = this._tCusp - sqrtDet;
        this._tInflection2 = this._tCusp + sqrtDet;
      }
      else {
        this._tInflection1 = NaN;
        this._tInflection2 = NaN;
      }
    },
jonathanolson commented 9 years ago

There now should only be 1 allocation for each function mentioned (the new value to return).