Vector.normalize() needs to avoid division by zero

[lukehutch]

Current normalization code is as follows:

    public Vector3d normalize(Vector3d dest) {
        double invLength = 1.0 / length();
        dest.x = x * invLength;
        dest.y = y * invLength;
        dest.z = z * invLength;
        return dest;
    }

Instead division by zero should always be avoided, and the code should be something like the following to prevent division-by-zero (arguably dealing with a zero vector in the result is better from a robustness point of view than dealing with Inf or NaN, since these values propagate virally):

    public Vector3d normalize(Vector3d dest) {
        double lengthSq = lengthSquared();
        if (lengthSq < 1.0e-30) {
            dest.x = 0.0;
            dest.y = 0.0;
            dest.z = 0.0;
        } else {
            double invLength = 1.0 / Math.sqrt(lengthSq);
            dest.x = x * invLength;
            dest.y = y * invLength;
            dest.z = z * invLength;
        }
        return dest;
    }

Then there should be a Vector.isZero() method for quickly testing if a vector is (exactly) equal to zero, so that return conditions from methods like the result of normalize() can be quickly checked:

    public boolean isZero() {
        return x == 0.0 && y == 0.0 && z == 0.0;
    }

There should probably also be a Vector.isCloseToZero() method that would replace the if (lengthSq < 1.0e-30) test above:

    public boolean isCloseToZero() {
        double lengthSq = lengthSquared();
        return lengthSq < 1.0e-30;
    }

The constant 1.0e-30 could be smaller for float vectors (maybe 1.0e-12f) -- it should be something above the precision floor, but below the probable error/noise floor for the majority of applications.

[httpdigest]

I want to keep methods with (let's say) arbitrary epsilons and error recovery very low in JOML. If you try to normalize a zero vector, then you will be getting undefined (NaN) as result, because dividing zero by zero is undefined. In my opinion, this is more desirable than recovering from it in a way that may not be suitable for the client/user-application. My proposal: Leave the method as is and use isFinite to check for infinity/NaN after the normalization.

[lukehutch]

Fair argument. You should at least document all methods that can return infinity/NaN however, and suggest in the Javadoc that the user call isFinite on the result if it might be an exceptional value.

Also some methods can set just some components of a result to infinity/NaN. For example, Quaternionf.rotationAxis can set x, y and z to infinity, but w will always be valid. This should be documented too, because if a user just checks isFinite(q.w), they would miss catching the problem until it has propagated deeper into the program.

[httpdigest]

Yes, I'll document it and will add a Quaternion.isFinite() too.

[lukehutch]

Vector.isFinite() would also be a good idea. And probably even Matrix.isFinite() for testing all elements of a matrix.

[httpdigest]

Vector.isFinite() would also be a good idea.

I did meant that with my comment https://github.com/JOML-CI/JOML/issues/211#issuecomment-590770861 (did you open the link?)

[lukehutch]

Oh, sorry, I didn't -- I assumed that was a link to Math.isFinite(double) without checking it.

[httpdigest]

Changes are pushed, but it seems the oss.sonatype.org endpoint to publish snapshots is currently down (504 gateway timeout in the Travis build when trying to upload the artifacts): https://travis-ci.org/JOML-CI/JOML/jobs/655543569 (also it seems the logs cannot be pulled fully, so Travis also seems to have an issue... I'll manually retrigger the job tomorrow and keep this issue open until the artifacts are published.

[lukehutch]

Oh man, I run into this all the time when attempting to publish to Sonatype. I keep filing bugs each time it happens, but they never seem to fix the underlying causes in a robust way. It's a reliably unreliable service :-) But there's no great alternative right now (I tried BinTray, and it has its own problems...)

Thanks for your work on this!

[httpdigest]

Alright, just manually retriggered the build and this one went through, so 1.9.23-SNAPSHOT is available now.

[lukehutch]

Just one thing missing... Any method that can result in NaN/infinity in any component needs to recommend in the Javadoc that the user call isFinite on the result, if there's any chance of this happening (eg. when normalizing a zero vector).