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

Error: Invalid LatLng object #65

Closed
Techn1x opened this issue Aug 20, 2017 · 10 comments
Closed

Error: Invalid LatLng object #65

Techn1x opened this issue Aug 20, 2017 · 10 comments
Labels

Comments

@Techn1x
Copy link
Contributor

Techn1x commented Aug 20, 2017

Since updating from to 1.3.2, I have noticed that some of my paths cause an error like the following;

Error: Invalid LatLng object: (NaN, 148.20798060000007)
    at LatLng (leaflet-src.js:1329)
    at Object.unproject (leaflet-src.js:1624)
    at Object.pointToLatLng (leaflet-src.js:1466)
    at NewClass.unproject (leaflet-src.js:3842)
    at leaflet.polylineDecorator.js:51
    at Array.map (<anonymous>)
    at projectPatternOnPath (leaflet.polylineDecorator.js:49)
    at NewClass._getDirectionPoints (leaflet.polylineDecorator.js:430)
    at leaflet.polylineDecorator.js:454
    at Array.map (<anonymous>)

I am definitely not passing in any points that are NaN, so I believe it's something to do with the algorithm. Some paths are completely OK, whereas others are not.

One of the paths with the error is 8 points long, and all of the lat/lngs are the same position. Another path is 320 points long, and many different points. What is common between them, is that both of them have (at least) their first 2 points in the path exactly the same, which I think is the cause.

The error occurs for the very first point in the path, and I think the problem stems from inside the function projectPatternOnPointPath(), in particular this line where segmentRatio is calculated
https://github.com/bbecquet/Leaflet.PolylineDecorator/blob/master/src/patternUtils.js#L94

At that line, positionOffset = some int, segment.distA and segment.distB are both equal to zero, so the line ends up dividing by zero, to get NaN, and then this NaN value is passed into interpolateBetweenPoints()

The algorithm is a little advanced for me so I don't think I can do a PR to fix it, but hopefully this is a good start

Here is one of my simpler paths that causes the error (they are all the same latlng point, so this might not be a good example to work with, let me know if you would like my 320 point path)

[
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806},
{lat: -21.9527438, lng: 148.2079806}
]
@Techn1x
Copy link
Contributor Author

Techn1x commented Aug 20, 2017

And here is the pattern I use;

{offset: 0, repeat: 300, symbol: L.Symbol.arrowHead({pixelSize: 12, headAngle: 35, pathOptions: {stroke: true, fillColor:'red', fillOpacity:1.0, weight:0.5,color:'black'}})}

@bbecquet
Copy link
Owner

Hi! I'm currently in holiday without a PC but I'll fix that when I'm back next week :)
On first sight it looks like the new algo has an edge-case problem with repeated coordinates. Special case but not uncommon in real life applications, so it shouldn't break. Shouldn't be hard to fix though.
And the next step will be to add a unit test suite with edge cades like that to limit regressions. Should have done it a long time ago.

@Techn1x
Copy link
Contributor Author

Techn1x commented Aug 22, 2017

No worries! Enjoy your holiday, you earned it :)

@bbecquet bbecquet added the bug label Aug 30, 2017
@bbecquet
Copy link
Owner

bbecquet commented Sep 6, 2017

Should be fixed in v1.4.0. Your failing polyline was really special, being just the same point repeated, so it was 0-length. But no reason for it to crash :)

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 6, 2017

Wonderful! 1.4.0 is working well for me now, thanks

Yes that polyline sure was special. Although I also had other polylines that simply had a few repeated points here and there.

@Techn1x Techn1x closed this as completed Sep 6, 2017
@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 6, 2017

Oops, sorry, spoke too soon :(

Here's a polyline track of 31 points that causes the same error. It looks like it's just the first two points in this array that are the same

[
{lat: -23.3911348, lng: 148.933824},
{lat: -23.3911348, lng: 148.933824},
{lat: -23.3910491, lng: 148.934028},
{lat: -23.3895226, lng: 148.936123},
{lat: -23.3886133, lng: 148.9373106},
{lat: -23.3882266, lng: 148.9376325},
{lat: -23.387741, lng: 148.937863},
{lat: -23.3871938, lng: 148.9379456},
{lat: -23.383047, lng: 148.9379633},
{lat: -23.382322, lng: 148.9382416},
{lat: -23.379297, lng: 148.9398948},
{lat: -23.37357, lng: 148.941373},
{lat: -23.3646515, lng: 148.9414395},
{lat: -23.360858, lng: 148.9411866},
{lat: -23.3584696, lng: 148.9404751},
{lat: -23.3562081, lng: 148.9393531},
{lat: -23.3459791, lng: 148.9335488},
{lat: -23.3358035, lng: 148.927778},
{lat: -23.327954, lng: 148.9233386},
{lat: -23.3190975, lng: 148.9200458},
{lat: -23.311487, lng: 148.9169328},
{lat: -23.3074813, lng: 148.9157366},
{lat: -23.3063348, lng: 148.9156843},
{lat: -23.3055973, lng: 148.915747},
{lat: -23.3054856, lng: 148.915261},
{lat: -23.3055186, lng: 148.9147111},
{lat: -23.3054763, lng: 148.9143403},
{lat: -23.3061438, lng: 148.9143375},
{lat: -23.3066985, lng: 148.9145951},
{lat: -23.3072503, lng: 148.9148051},
{lat: -23.309068, lng: 148.9153371}
]

@Techn1x Techn1x reopened this Sep 6, 2017
@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 6, 2017

Don't rush to fix this or anything, I'll just filter my polyline points so that I've got no doubles of the same point (I probably should be doing this anyway) :)

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 7, 2017

After 1.4.0 it seems that this is a very specific case. Polyline's that have double points in them anywhere else are fine, it's when the very first 2 points are the same that the error still occurs.

It seems I am definitely giving your algorithm a thorough testing! haha

Would this modification to projectPatternOnPointPath() be a good idea? It seems to fix it

function projectPatternOnPointPath(pts, pattern) {
    // 1. split the path as segment infos
    var segments = pointsToSegments(pts);

/*
  At this point here, if the first 2 points are the same, then the first segment in the array has distA == 0 
and distB == 0.
  I think also that if the first 3 points are the same, then the first 2 segments would also be distA == distB == 0, and so on.
*/
    // This should remove all starting segments that have distA === distB === 0
    while(segments.length > 0 && segments[0].distA === 0 && segments[0].distB === 0) {
        segments.removeAt(0);
    }

    var nbSegments = segments.length;
    if (nbSegments === 0) {
        return [];
    }
...

Or perhaps a more direct fix would be in pointsToSegments(pts) to not push segments that have both distA == distB == 0 in the first place?

@bbecquet
Copy link
Owner

bbecquet commented Sep 9, 2017

Hi!
I've implemented your last suggestion, i.e. modifying the pointsToSegments function to just push "real" segments, where a !== b.
The last version (1.4.1) should do it… until next time? :)
Thanks for bringing up these edge cases which help make the algo more robust!

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 10, 2017

Looks good now, hopefully won't need to re-open this time. Thanks

@Techn1x Techn1x closed this as completed Sep 10, 2017
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

2 participants