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

after firing map.pm.enableDraw(...) - on each "mousemove" event getting error "Cannot read property 'lat' of null" #257

Closed
DavidLore opened this issue Nov 28, 2017 · 4 comments

Comments

@DavidLore
Copy link

DavidLore commented Nov 28, 2017

Hey there,

First, I have to say that despite the error that promping on console, the behaviour of the functionality is great. The result is pure Rectangle and the working progress is great.

The operation :

I'm trying to draw few shapes using leaflet.pm, and I found some troubles while using the method:
map.pm.enableDraw("Rectangle", { drawing: true, // snapping snappable: true, snapDistance: 20, allowSelfIntersection: true, finishOn: 'dblclick', templineStyle: { color: 'blue' }, hintlineStyle: { color: 'blue', dashArray: [5, 5] }, cursorMarker: true, pathOptions: { color: 'red', fillColor: 'orange', fillOpacity: 0.7 }, markerStyle: { opacity: 0.5, draggable: true, } });

Then, when I'm moving with the cursor around the leaflet map I'm getting error.
I already debugged the leaflet module all the day and I found the source of the bug :
In the function :
`
// @method latLngToLayerPoint(latlng: LatLng): Point

// Given a geographical coordinate, returns the corresponding pixel coordinate

// relative to the [origin pixel](#map-getpixelorigin)

latLngToLayerPoint: function (latlng) {
var projectedPoint = this.project(toLatLng(latlng))._round();
return projectedPoint._subtract(this.getPixelOrigin());
}
`
The function getting the parameter latlng that expecting to be type of LatLng but instead he getting
an array of 17 elements of type LatLng.

arrayoflatlng

So - when executing the function toLatLng(latlng) it returns null, and then this null sended to 'this.project' method and then it throws an error to console.

So I tried to change and manipulate the code in purpose to avoid error's on console, and I have done it, but the performance of the drawing Rectangle process (while moving the cursor around the map) became awful (just getting too slow when mouse move during the draw), so I can't really found a good solution to the error's on console.

  • Needless to say that I already tried to wrap this code area by try & catch but I haven't find a solution that will work well.
  • I also tried to send each element of array with for-loop, and also tried to send only the last/first point in the array. All this workarounds has led me to heavy performance issue (But the error's on console disappeared).

I want to say again - the performance when I getting the error's on console were great, but I want to avoid the error's that coming from the leaflet module.

By the way, I'm using the latest version of leaflet : "1.2.0+HEAD.1ac320b"

Thank you friends.

@codeofsumit
Copy link
Contributor

hey @DavidLore. Do you mind using ``` to wrap the provided code so I actually know what options you passed? If I copy that, most of the code is commented out.

Also: if you can provide a JSfiddle showing the problem, it'd be much more helpful for me to get to the bottom of this.

Thanks for your help!

@codeofsumit
Copy link
Contributor

Haven't been able to reproduce this yet, sorry. I'll close this issue for now. Feel free to reopen the issue with a JSfiddle and error reproduction 👍

@smeijer
Copy link

smeijer commented May 2, 2018

Would be awesome if you could reopen this issue. As I too experienced this issue.

The bug lies in calculating the closest layer for snapping purpose. And to be specific, this only happens when there is a MultiPolygon in the _snapList.

I've created a fiddle to show this issue. https://jsfiddle.net/vz227cqc/

A failing polygon:

{
  "type": "Feature",
  "geometry": {
    "type": "MultiPolygon",
    "coordinates": [
      [
        [
          [4.671962, 52.409447],
          [4.674379, 52.406469],
          [4.676519, 52.409963],
          [4.671962, 52.409447],
        ]
      ]
    ]
  },
  "properties": {},
}

Note that this is a single ring MultiPolygon only for demo purpose. Simply extracting the first ring, and turning it into a working Polygon like I did in the fiddle, is not a viable solution.

The bug is triggered at: _handleSnapping -> _calcClosestLayer -> _calcLayerDistances

https://github.com/codeofsumit/leaflet.pm/blob/eba28964071fa16fa438567640221b0653e8d2d6/src/js/Mixins/Snapping.js#L214-L221

So checking the _calcLayerDistances method, I've found the issue, at line 254 coords = layer.getLatLngs()[0];:

https://github.com/codeofsumit/leaflet.pm/blob/eba28964071fa16fa438567640221b0653e8d2d6/src/js/Mixins/Snapping.js#L251-L255

For a Polygon this code is correct. But, in case the layer had been created from a MultiPolygon, layer.getLatLngs()[0]; returns a new 2D Array. Resulting in coord here, being a new coords array (ring), and A being an Array instead of an coord:

https://github.com/codeofsumit/leaflet.pm/blob/eba28964071fa16fa438567640221b0653e8d2d6/src/js/Mixins/Snapping.js#L275-L278

@smeijer
Copy link

smeijer commented May 3, 2018

I guess #249 is related and possibly the same issue.

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

Successfully merging a pull request may close this issue.

3 participants