Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getDistanceFromLine is returning NaN #227

Closed
jestebanzapata opened this issue Feb 24, 2020 · 6 comments
Closed

getDistanceFromLine is returning NaN #227

jestebanzapata opened this issue Feb 24, 2020 · 6 comments
Labels

Comments

@jestebanzapata
Copy link

jestebanzapata commented Feb 24, 2020

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

@cooperbarth
Copy link

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).

@paulmatvienko
Copy link

paulmatvienko commented May 19, 2020

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;
  }
}

@cooperbarth
Copy link

@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
}

@paulmatvienko
Copy link

@cooperbarth Yeah, you are right, my mistake

@manuelbieh
Copy link
Owner

🎉 This issue has been resolved in version 3.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@redcic75
Copy link

redcic75 commented Apr 9, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants