Possible bug in CatmullRomSpline.js

[andr3nun3s]

While working on issue #89 I found a possible bug on CatmullRomSpline.js, there are two ifs checking firstTangent. I believe the second one should check if lastTagent is undefined. Am I correct?

var CatmullRomSpline = function(controlPoints, firstTangent, lastTangent) {
        if (!controlPoints || !(controlPoints instanceof Array) || controlPoints.length < 3) {
            throw new DeveloperError('controlPoints is required and must be an array of objects with point and time properties, with a length of at least 3.');
        }

        this._points = controlPoints;
        this._lastTimeIndex = 0;

        if (typeof firstTangent !== 'undefined') {
            this._ti = Cartesian3.clone(firstTangent);
        } else {
            var controlPoint0 = Cartesian3.clone(controlPoints[0].point);
            var controlPoint1 = Cartesian3.clone(controlPoints[1].point);
            var controlPoint2 = Cartesian3.clone(controlPoints[2].point);

            this._ti = controlPoint1
                           .multiplyByScalar(2.0)
                           .subtract(controlPoint2)
                           .subtract(controlPoint0)
                           .multiplyByScalar(0.5);
        }

        if (typeof firstTangent !== 'undefined') {
            this._to = Cartesian3.clone(lastTangent);
        } else {
            var n = controlPoints.length - 1;

            var controlPointn0 = Cartesian3.clone(controlPoints[n].point);
            var controlPointn1 = Cartesian3.clone(controlPoints[n - 1].point);
            var controlPointn2 = Cartesian3.clone(controlPoints[n - 2].point);

            this._to = controlPointn0
                           .subtract(controlPointn1.multiplyByScalar(2.0))
                           .add(controlPointn2)
                           .multiplyByScalar(0.5);
        }
    };

[pjcozzi]

I think you are right. @bagnell?

[bagnell]

@Andre-Nunes Yes, you are correct.

[pjcozzi]

@Andre-Nunes want to submit a fix - and a new test since our tests didn't catch this?

[andr3nun3s]

Yes I'll fix it. Is there any page about writing tests?

[mramato]

Turns out, there's not a whole lot of information on writing tests on our wiki, we should probably fix that.

1. All of the tests are in the Specs directly.
2. Each file in source is usually mapped to a file of the same name (plus Specs) in the Specs directory. For example, Source/Core/CatmullRomSpline.js has tests in Specs/Core/CatmullRomSplineSpecs.js.

So you can just go into the CamullRomSplineSpecs.js and add a new it function that causes the above issue.

[bagnell]

Fixed in above pull. Closing.