-
Notifications
You must be signed in to change notification settings - Fork 161
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
Changing the precision for geoContains for LineStrings? #153
Comments
With the current implementation, making the delta/epsilon configurable wouldn’t do what you expect, assuming that you expect the configurable delta to mean the distance between the point and the line. The current implementation (which is limited to a single line segment per issue #154, but you can imagine how to extrapolate to multiple line segments) does something like this, where AB is the line segment and P is the point to test: If the distance AP + the distance PB is less than or equal to the distance AB plus some epsilon, then P is considered on the line AB. This is a faster and simpler test than computing the closest point along AB to P, and then measuring the distance between that point and P. (See Closest Point on Line for the Euclidean case, but note that we’re dealing with spherical geometry here, which typically makes things harder.) So, I think making the current delta configurable won’t work, because it’s really an implementation detail of the current approach. Making it configurable with a true distance test would be a reasonable API, but I’m not signing up to implement it. You’re welcome to take a crack at it, though! |
Hi @mbostock, thanks for the prompt response! The current solution happens to give pretty usable results, even for higher epsilon values. I suppose that we might normalize the case by dividing the difference by AB length, effectively converting the current test
to
So that the value would not depend on segment length. What do you think? I agree that spherical geometry is a problem here. Moreover, the feature-picking I am looking for already assumes projected geometries, so perhaps the functionality shall not be added to |
Hi @mbostock ,
geoContains seems to work with LineStrings using the math/epsilon value, which is fixed at
1e-6
. It would be nice to have this value configurable, say for the cursor-based feature picking stuff.I am willing to submit a PR, if the mentioned approach sounds feasible.
The text was updated successfully, but these errors were encountered: