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

Can't avoid snap near line #2

Open
stevage opened this issue Aug 10, 2021 · 6 comments
Open

Can't avoid snap near line #2

stevage opened this issue Aug 10, 2021 · 6 comments

Comments

@stevage
Copy link

stevage commented Aug 10, 2021

First - thanks, this library is amazing. I have worked snapping in the past and it's a real pain!

I ran into a bit of an issue attempting to draw polygons. Here you can see I am attempting to trace the raster.

Screen.Recording.2021-08-10.at.4.21.47.PM.mov

The point I am trying to create is not near a vertex, so I don't expect it to snap. But it seems it detects that my point is near the line, and snaps to the nearest vertex.

I'm not sure if this is a bug or just surprising behaviour that should maybe be avoidable with an option.

(I have just discovered that holding Option temporarily prevents snapping, which is a fine workaround for me. That should probably be documented, too...)

@mhsattarian
Copy link
Owner

Hi,
I'm glad you're finding this useful.

you can actually control the snapping behavior using Boolean option draw.options.snap, changing this would take effect in the snapping modes:

draw.options.snap = false;

there's an example in the docs directory showing this. but you're right I should've mentioned this control option and the Alt (Option) key in the docs, my bad. will add them soon.

on a side note, there's also an option snapPx to reduce snapping distance. In your example specially, this should be reduced because lines are not that long and 15px is probably a lot:

const draw = new MapboxDraw({
  ...
  snapOptions: {
    snapPx: 15,
    snapToMidPoints: true,
  },
});

@stevage
Copy link
Author

stevage commented Aug 10, 2021

draw.options.snap = false;

Well, yes - but that just disables snapping entirely, which isn't what I want.

on a side note, there's also an option snapPx to reduce snapping distance.

Yes, I noticed that - but that doesn't really solve the issue either.

The more I think about it, the more I think this is incorrect behaviour - hovering the mouse near the midpoint of a line shouldn't cause any special snapping to happen if snapToMidpoints is false (as it is in my case).

@mhsattarian
Copy link
Owner

Well, yes - but that just disables snapping entirely, which isn't what I want.

Ah, I thought you can disable it and enable it later.

The more I think about it, the more I think this is incorrect behaviour - hovering the mouse near the midpoint of a line shouldn't cause any special snapping to happen if snapToMidpoints is false (as it is in my case).

setting snapToMidpoints to false would only disable snapping to mid-points (an imaginary point at the middle of a line or segments). I'm guessing when lines are not that long which happens in your case where we are zoomed in near the buildings, snapping wouldn't work properly; and that's because snapping give priority to vertices, and the max distance triggering this is set here manually:

const priorityDistance = 1.25;

would you think reducing this (and make it dynamic) would solve this problem?

@mhsattarian
Copy link
Owner

@stevage I tested changing this priorityDistance and it worked quite well:

MI5bivXq6G.mp4

here I'm setting the value to 0.0025

so, I updated the package on npm, making it possible to pass a custom value for priorityDistance in snapOptions as below:

snapOptions: {
    snapVertexPriorityDistance: 0.0025, // defaults to 1.25
}

@stevage
Copy link
Author

stevage commented Aug 13, 2021

I'm to be honest I'm really confused with the relationship between snapToMidpoints and snapVertexPriorityDistance

  • snapToMidpoints true, no snapVertexPriorityDistance: mouse snaps to the vertices only, even when you're near the midpoint of a segment (like in my original issue). Seems never useful?
  • snapToMidpoints false, no snapVertexPriorityDistance: seems exactly the same
  • snapToMidpoints false, snapVertexPriorityDistance: 0.00001 - snaps either to the vertex or somewhere along the segment, whichever is closer.
  • snapToMidpoints false, snapVertexPriorityDistance: 0.0025 - snaps either to the vertex or somewhere along the segment, but with a stronger bias for a vertex if there is one. This seems like a good default setting.

Since the behaviour of snapVertexPriorityDistance is subtle, hard to explain, and uses obscure units (what are they?), I'd suggest just picking a good default setting and leaving it at that.

I don't know if there is a bug or I'm not understanding what snapToMidpoints is meant to do, but it doesn't seem to change anything. It seems like it would be useful to be able to avoid snapping to line segments (and only snapping to vertices), but this setting doesn't seem to change that.

@mhsattarian
Copy link
Owner

Sorry for the late response,

you're assuming snapToMidPoints controls whether to snap only to vertices or also snap to the nearest point in the line/segments as well. but that's not what it does.
snapToMidPoints is controlling whether to snap to the middle point of each line/segment or not. just that. I guess it's not that useful and -as you mentioned- we instead need a more convenient option to control snapping behavior, and select to snap only to vertices, or snap to segments as well. I'm thinking it should not be that hard to add.

on the other hand, the snapVertexPriorityDistance option controls the min distance (in km) from each vertex where snapping to vertex takes priority over snapping to line/segments.
this is why when nothing is set for it, and the default value (which is 1.25km and is too much when the line/segment itself is less than a few kilometers long) is used it only snaps to the vertices and not the line/segment.

Since the behaviour of snapVertexPriorityDistance is subtle, hard to explain, and uses obscure units (what are they?), I'd suggest just picking a good default setting and leaving it at that.

I think its behavior is not that subtle, it can be dynamically (using a range element for example) or programmatically (based on zoom level I think) controlled so that the snaping is always acting as desired.
and I should've mentioned it's the minimum distance in kilometers, sorry.

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

No branches or pull requests

2 participants