manuelbieh/geolib

getDistanceFromLine is returning NaN

jestebanzapata opened this issue ยท 6 comments

When I called the getDistanceFromLine method with the next variables, I'm getting a NaN result:

`const point = { longitude: -75.63287336843746, latitude: 6.278381350919607 };

const lineStart = {
longitude: -75.6220658304469,
latitude: 6.285304104233529 };

const lineEnd = {
longitude: -75.62216373107594,
latitude: 6.285232119894652 };

geolib.getDistanceFromLine(
point,
lineStart,
lineEnd
);
`

I don't know if it is related with the issue #129

I had a similar issue - in my case, I rewrote the method to account for inputs being the same point (division by 0 errors are not accounted for in the current implementation).

It happens when d1, d2 or d3 are equal to 0. So in when calculates alpha and beta (2 * d1 * d3) = 0 it returns NaN. Here is my fix:

const getDistanceFromLine = (
    point: GeolibInputCoordinates,
    lineStart: GeolibInputCoordinates,
    lineEnd: GeolibInputCoordinates
) => {
  const d1 = getDistance(from, point);
  const d2 = getDistance(point, to);
  const d3 = getDistance(from, to);
  
  if (d1 === 0 || d2 === 0 || d3 === 0) {
    return 0;
  } else {
    const alpha = Math.acos((d1 * d1 + d3 * d3 - d2 * d2) / (2 * d1 * d3));
    const beta = Math.acos((d2 * d2 + d3 * d3 - d1 * d1) / (2 * d2 * d3));
    
    if (alpha > Math.PI / 2) return d1;
    if (beta > Math.PI / 2) return d2;
    
    return Math.sin(alpha) * d1;
  }
}

@kalnitski The behavior you detailed is wrong. If d3 is 0, then lineStart and lineEnd have a distance of 0 from each other. In this case, the return value should not be 0, but the distance from point to either lineStart or lineEnd.

if (d1 === 0 || d2 === 0) {
    return 0;
} else if (d3 === 0) { // lineStart and lineEnd are the same - return point-to-point distance
    return d1;
} else {
    // math
}

@cooperbarth Yeah, you are right, my mistake

๐ŸŽ‰ This issue has been resolved in version 3.2.2 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€

Hello @manuelbieh ,

I've just noticed that the fix you've commited and pushed does not deal with all the problematic cases. When we call getDistanceFromLine we still get NaN when point === startLine and when startLine === endLine.

This fix in getDistanceFromLine.ts should correct this issue:

    if (d1 === 0 || d2 === 0) {
        // point located at the exact same place as lineStart or lineEnd
        return 0;
    }
    if (d3 === 0) {
        return d1; // lineStart and lineEnd are the same - return point-to-point distance
    }

Tests I've created that still fail:

   it('https://github.com/manuelbieh/geolib/issues/227 - Point === startLine', () => {
        expect(
            getDistanceFromLine(
                {
                    latitude: 53,
                    longitude: 5,
                },
                {
                    latitude: 53,
                    longitude: 5,
                },
                {
                    latitude: 53,
                    longitude: 6,
                }
            )
        ).toEqual(0);
    });

    // The test below does not fail but it's by accident.
    it('https://github.com/manuelbieh/geolib/issues/227 - Point === endLine', () => {
        expect(
            getDistanceFromLine(
                {
                    latitude: 53,
                    longitude: 5,
                },
                {
                    latitude: 53,
                    longitude: 6,
                },
                {
                    latitude: 53,
                    longitude: 5,
                }
            )
        ).toEqual(0);
    });

    it('https://github.com/manuelbieh/geolib/issues/227 - startLine === endLine', () => {
        expect(
            getDistanceFromLine(
                {
                    latitude: 53,
                    longitude: 6,
                },
                {
                    latitude: 53,
                    longitude: 5,
                },
                {
                    latitude: 53,
                    longitude: 5,
                }
            )
        ).not.toBeNaN();
    });

I don't have the access right to push this modification on a new branch on your repo.