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

Changing the precision for geoContains for LineStrings? #153

Open
ondras opened this issue Jan 14, 2019 · 2 comments
Open

Changing the precision for geoContains for LineStrings? #153

ondras opened this issue Jan 14, 2019 · 2 comments
Labels

Comments

@ondras
Copy link
Contributor

ondras commented Jan 14, 2019

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.

@mbostock
Copy link
Member

mbostock commented Jan 14, 2019

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:

untitled 9

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!

@ondras
Copy link
Contributor Author

ondras commented Jan 15, 2019

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

AP+BP > AB + eps

to

(AP+BP)/AB > eps + 1

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 d3-geo, but rather to some other D3 lib (the one working with planar geometries) instead?

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

No branches or pull requests

3 participants