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 almost junction behaviour for close endpoints #7309

Merged
merged 11 commits into from
Feb 27, 2020
Merged

Conversation

kymckay
Copy link
Collaborator

@kymckay kymckay commented Jan 31, 2020

Closes #7201

Basically, if an intersection is found, it now checks:

  1. Are any of the intersected way's endpoints within EXTEND_DIST - NODE_JOIN_DIST of the node being extended from
  2. If there are, which one has smallest change in angle under threshold atan(NODE_JOIN_DIST / EXTEND_DIST)

These thresholds were arrived at by considering the edge-most cases where we'd want endpoint joining to kick in:
(the distance threshold, we want to catch an effectively parallel way that would be joined just outside of the existing node joining distance)
Parallel

(the angle threshold, a perpendicular way would already be joined to the end node if the intersection occurs at the joining distance, so we want to catch cases where the change in edge angle is less than the angle of this triangle)
Perpendicular

Some before/after applying the quick fix for different test cases:

image image

(this one is both endpoints of the same way)
image image

(both close, connects to end which creates smallest change in angle)
image image

@kymckay kymckay added the validation An issue with the validation or Q/A code label Jan 31, 2020
@kymckay
Copy link
Collaborator Author

kymckay commented Feb 2, 2020

I think I could optimise this by running the nearby endpoint detection and angle checks in the quick fix code rather than in the validation itself. Will try to adapt as such 👍

@kymckay
Copy link
Collaborator Author

kymckay commented Feb 2, 2020

Ready for review now, due to all indentation changes in almost_junction.js I'll point out most significant changes on lines:

  • 21-27: declaring constants which control the validation and quick fix behaviour
  • 99-110: where I've changed the quick fix
  • 212-246: where I've added the functions used by above
  • 314: now includes id of the node before the end node - also saved into the issue entityIds data for use in the quick fix

Based on distances at which nodes are joined by the quickfix and the
validation extends ahead of an end node, if both ways have close end
nodes and joining them would result in a small change of angle for the
edited way then they will be joined.

Closes #7201
Just a bit more optimised since this is only relevant to check when the
quick fix is used
Realised these are used for highlighting and should remain as is.
@quincylvania quincylvania merged commit c39e035 into 2.x Feb 27, 2020
@quincylvania quincylvania added this to the Next Release milestone Feb 27, 2020
@quincylvania quincylvania deleted the endpoints-fix branch February 27, 2020 21:14
@quincylvania
Copy link
Collaborator

@SilentSpike Thanks so much!! I didn't trace every line of code but it appears to work as expected. Plus the code tests are a big reassurance. Just merged it as-is but we can make changes on the main branch if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validation An issue with the validation or Q/A code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strange connection of two paths
2 participants