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

fix(snapping): fix bug when the map contains a multi-geometry features (MultiPolygon / MultiLine) #293

Closed
wants to merge 4 commits into from

Conversation

smeijer
Copy link

@smeijer smeijer commented May 3, 2018

MultiPolygon layers trigger a bug in the snapping algorithm. This PR fixes this by breaking the MultiPolygon layers up before adding to the _snapList.

Fixes #257. More details explained in this comment: #257 (comment)

@smeijer
Copy link
Author

smeijer commented May 3, 2018

Related PR: #279

I haven't tested, but I don't expect that both are needed. The scope of #279 looks larger by adding MultiPolygon support, whereas this PR exists to "just fix the snapping bug #257".

My first idea of fixing the snapping bug was also to loop over the rings in the _calcLayerDistances method. But I decided to take the more simple approach of pre-fixing it in the _snapList for performance purpose. (create a couple of extra layers upon init vs an extra loop on every mouse-move).

That being said, maybe it's needed to add support for MultiPoint or MultiLine the same way. But again, haven't tested those. Adding support for them in the same way should be an easy job though.

@smeijer
Copy link
Author

smeijer commented May 3, 2018

Turned out, MultiPoint works as expected. Not causing any troubles.

MultiLine lacks the same support. I'll push an update in a few minutes.

@smeijer
Copy link
Author

smeijer commented May 19, 2018

Any chance this one can get merged? I'm currently using my own fork. Would be nice if I could use the main branch again.

@smeijer
Copy link
Author

smeijer commented Jun 15, 2018

Rebased on develop

@codeofsumit
Copy link
Contributor

@smeijer thank you for your work on this.
Just to clarify: if we had proper multi polygon/line support, this would still be needed (as it's part of the solution) - do you agree?

@codeofsumit
Copy link
Contributor

I've noticed some bugs in this PR.

Currently, you can't snap a hole to it's outer polygon ring, in this PR that is now possible but not to all sides.
Also: a vertex of a polygon can now be snapped to it's original position which should not be possible.

But this PR really helps me in getting my head back into this issue 👍 - I'm looking at multi polygons today and hope to make some progress

@smeijer smeijer changed the title fix(snapping): fix bug when the map contains a MultiPolygon layer fix(snapping): fix bug when the map contains a multi-geometry features (MultiPolygon / MultiLine) Jun 15, 2018
@codeofsumit codeofsumit mentioned this pull request Jun 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants