turf icon indicating copy to clipboard operation
turf copied to clipboard

greatCircle returns array of NaN if start and end is the same

Open laurenceks opened this issue 2 years ago • 5 comments

@turf/great-circle 6.5.0

If the coordinates passed to greatCircle are the same it returns an array of NaNs.

This has happened during a line animation along a greatCricle. The animation draws a line up to the progress marker so that the line is divided in colour to show progress. Effectively the initial complete greatCircle line is being painted over by the same line (this is in mapbox-gl).

As the progress percentage in the first frame is 0% the coordinates passed to greatCricle are the same for start and end points. greatCricle returns a linestring with coordinates of [[NaN, NaN], [NaN, NaN]...], which cannot be used and cause an error.

Expected output would be an array of all coordinates the same (the start point).

Example

This does not work when coordinates.start.arrayLongLat and progressMarkerCoordinates are equal:

const progressRouteCoordinates = greatCircle(coordinates.start.arrayLongLat, progressMarkerCoordinates).geometry.coordinates;

Comparing the start and end and substituing a simple linestring if they are the same prevents this issue, but I feel shouldn't be necessary

const progressRouteCoordinates = progressMarkerCoordinates[0] === coordinates.start.long && progressMarkerCoordinates[1] === coordinates.start.lat ? [coordinates.start.arrayLongLat, progressMarkerCoordinates] :
            greatCircle(coordinates.start.arrayLongLat, progressMarkerCoordinates).geometry.coordinates;

Code context

const animateMap = (durationInMs = mapAnimationDurationInMs) => {
    //make sure the event listener only fires once
    scrolledIntoFrame.unobserve(document.querySelector("#map"));
    const easing = BezierEasing(0.16, 1, 0.3, 1); //easeOutExpo to match countUp
    let startTime = null;
    const progressAnimation = (timestamp) => {
        if (!startTime) {
            startTime = timestamp;
        }
        const runtime = timestamp - startTime;
        const progressPercent = easing(runtime / durationInMs);

        //calculate new coordinates based on current progress
        const progressMarkerCoordinates = along(linestring(route.data.geometry.coordinates),
            sheetsData["Total distance"] * progressPercent, {
                units: "kilometers"
            }).geometry.coordinates;

        //**ISSUE IS HERE** - without comparison, greatCircle returns array of NaN because the line hasn't progressed yet and so the start/end coordinates are the same
        const progressRouteCoordinates = greatCircle(coordinates.start.arrayLongLat, progressMarkerCoordinates).geometry.coordinates;


        const remainingRouteCoordinates = greatCircle([...progressRouteCoordinates].pop(),
            coordinates.end.arrayLongLat).geometry.coordinates;
        //update marker and lines
        progressMarkerMapBoxGl.setLngLat(progressMarkerCoordinates);
        map.getSource("progressRoute")
            .setData({
                ...progressRoute.data,
                geometry: {
                    ...progressRoute.data.geometry,
                    coordinates: progressRouteCoordinates,
                },
            });
        map.getSource("remainingRoute")
            .setData({
                ...remainingRoute.data,
                geometry: {
                    ...remainingRoute.data.geometry,
                    coordinates: remainingRouteCoordinates,
                },
            });

        //if not yet complete, request a new frame
        if (progressPercent < 1) {
            requestAnimationFrame(progressAnimation);
        }
    };
    requestAnimationFrame(progressAnimation);
};

Is this intended behaviour or a genuine issue?

(Similar old issues #1361 #1946 with lineOffset, ?related)

laurenceks avatar Oct 04 '22 15:10 laurenceks

Thanks for the bug report @laurenceks - it sounds like a more valid output would be a linestring with duplicated first and last coords.

rowanwins avatar Oct 06 '22 05:10 rowanwins

One question @laurenceks - do you think the resulting line should have two coords, or should it have npoints repeated as per the option? eg

const line = greatCircle(start, end)
=> line.geometry.coordinates.length = 2
or 
=> line.geometry.coordinates.length = 100

At this stage I'm leaning towards the first, and adding the following to the documentation

If the start and end positions are the same then a LineString with 2 points will be returned and npoints option will be ignored.

rowanwins avatar Oct 06 '22 05:10 rowanwins

My instinct is actually to match npoints as personally I would expect greatCircle to always give a consistent array length, even if it's an array of 100 duplicates. Whilst the shorter line string would work for my use case, there might be scenarios where someone's code is expecting an array of the same length (npoints) every time.

The issue at hand isn't the array length but the contents of the array. If greatCircle returned an array of 100 duplicates I think that would still plot fine on a map; it's the output of NaN causing the problem. I think anyone wanting to remove duplicates could use cleanCoords?

That being said I'm just an amateur and don't really have a strong opinion! My broad understanding is consistency is usually better with function returns but so long as it outputs a valid line string my issue would be fixed.

Ps thanks for picking this up quickly!

laurenceks avatar Oct 06 '22 16:10 laurenceks

Awesome - having slept on it I think I agree with you @laurenceks so I've updated the PR.

rowanwins avatar Oct 06 '22 21:10 rowanwins

Any update on this?

RobbertWolfs avatar Jan 22 '24 09:01 RobbertWolfs