mapbox / mapbox-gl-draw

Draw tools for mapbox-gl-js
https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-draw/
ISC License
946 stars 591 forks source link

Snap to point and line #947

Open stevage opened 4 years ago

stevage commented 4 years ago

I'm working on a snapping mode for line and polygon drawing that supports snapping moved or created vertices to edges in existing lines/polygons or existing points. (Haven't decided about snapping to vertices in existing lines/polygons yet).

I'm looking for feedback on my design to see if it would be accepted as a PR.

When instantiating the mapbox-gl-draw object, you can pass through snapLayers: ['mylayer'] to indicate that features in mylayer should be snapped to. The default is no snap layers. For that reason, my thinking is towards making this an improvement to the standard draw_line_string and draw_polygon modes (since it's completely backwardly compatible) rather than new custom modes, as suggested in #865.

On the implementation side:

My implementation isn't complete, but it seems to work pretty solidly so far.

Thoughts?

Screen Recording 2019-12-24 at 4 25 09 pm

stevage commented 4 years ago

My branch is at https://github.com/stevage/mapbox-gl-draw/blob/snapping/src/snapping.js

I've put most of the snapping code in this single module, snapping.js. I haven't fully grokked the paradigm of the whole library, so it will probably look a bit weird at this point.

The actual touchpoint in each drawing mode is very small, like this:

DrawLineString.onMouseMove = function(state, e) {
  const lngLat = this._ctx.snapping.snapCoord(e.lngLat);
Plantain commented 4 years ago

This is great. We're definitely going to use this.

Plantain commented 4 years ago

One point I've noticed trying this - this blows out the side of the standalone library from ~110KB to 600KB.

stevage commented 4 years ago

One point I've noticed trying this - this blows out the side of the standalone library from ~110KB to 600KB.

Oh yeah, because it was still just proof of concept I pulled in the whole Turf library, even though I only need a tiny bit of it.

daniel-fschr commented 4 years ago

This is a great idea, but I don't understand how to implement this mode When instantiating the mapbox-gl-draw object. Can you give me an example? Thank you!

stevage commented 4 years ago

Sure, good idea. I've done some more work which I haven't pushed yet, too. I realised that the library has to be able to snap to layers that didn't exist when it got instantiated.

daniel-fschr commented 4 years ago

Hi Stevage, I am quite inexperienced with using custom modes in Mapbox Draw. Please have a look at my code below.

var draw = new MapboxDraw({
        snapLayers: ['Lines_data'],
        });

draw.onMouseMove = function(state, e) {
      const lngLat = this._ctx.snapping.snapCoord(e.lngLat);
 }

How do I implement your mode here and why does draw.onMouseMove not working?

Thank you for your help.

amelielecoz commented 4 years ago

Hi this a great feature. I would like to use it on a round trip line. Do you think it is possible to choose to snap my point to the going or coming line ?

stevage commented 4 years ago

@daniel-fschr You shouldn't need the onMouseMove event. But in any case, this PR is not really complete enough for me to be providing support - you might just need to be patient for a bit.

stevage commented 4 years ago

@amelielecoz Sorry, not quite sure what you mean by "the going or coming line"? Can you spell it out a bit more?

amelielecoz commented 4 years ago

Hi @setvage, let's take this exemple where I have a trace going from west to east, then going back to the starting point. I am trying to figure out how to make sure my point will be attached exactly where I want. I mean that I want to attach point only to the going line or the coming back line. I know it is a bit tricky and my English is not perfect, I hope you will understand. I am working on this problem for a bit now... If you have any thought on this I'd be happy to discuss

stevage commented 4 years ago

Ok, I understand what you mean. I'm not sure I can think of an easy way of supporting that?

NourdineMazali commented 4 years ago

Sure, good idea. I've done some more work which I haven't pushed yet, too. I realised that the library has to be able to snap to layers that didn't exist when it got instantiated.

@stevage, snapping to layers that didn't exist yet, is this implemented or not yet ? Thank you.

LuizAsFight commented 4 years ago

@stevage it's looking good man I'm gonna try it here. Why isn't it at official repo? Did you try to pull request?

Thank you for sharing!

Plantain commented 4 years ago

This used to work really nicely, but the snapping radius seems to be wildly different between mapbox-gl-js 1.8.1 and 1.11.1 - it's way too aggressively snapping with 1.11.1. I haven't managed to determine what changed to cause that yet.

segheysens commented 9 months ago

@stevage not sure if you've had any requests for this recently, but I just came across this feature in leaflet-geoman, and that library might provide some inspiration

dh-jburke commented 2 days ago

Snapping functionality is something I'm looking for and would found extremely useful - are there any current methods for enabling this on listed sources / layers?