Only detect exact collinear intersections in traces#6980
Only detect exact collinear intersections in traces#6980grzesiek2010 merged 5 commits intogetodk:masterfrom
Conversation
|
@dbemke is this looking better? |
Issue #6979 and the one described in #6967 don't show that they are invalid polygon.
The geometry is also in https://docs.google.com/document/d/1daqyvOECyh6pYMeealpVbPWrmONF4DgUHVvrcbRgWrE/edit?tab=t.0 |
Good to discuss further, but for this PR let's go with it as-is. I think we're going to need some feedback from users: making endpoint intersections "fuzzy" could be disastrous for some cases. |
| val intersectionPoint = intersectionSegment.interpolate(intersectPosition) | ||
| val lineSegment = LineSegment(points.last(), intersectionPoint) | ||
| val intersectingSegment = | ||
| LineSegment(lineSegment.start, lineSegment.interpolate(1.1)) |
There was a problem hiding this comment.
Adding a line that intersects through another (rather than just touching) avoids needing to use an epsilon to account for the inaccurate interpolate and is far more realistic.
| * Returns `true` if any segment of the trace intersects with any other and `false` otherwise. | ||
| */ | ||
| fun Trace.intersects(): Boolean { | ||
| fun Trace.intersects(epsilon: Double = 0.0): Boolean { |
There was a problem hiding this comment.
As mentioned in the PR description, I'm assuming we're going to end up tuning an epsilon for the intersects XPath function or allowing it to be tweaked so I've left this in.
shared/src/test/java/org/odk/collect/shared/geometry/GeometryTest.kt
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| fun Trace.addRandomIntersectingSegment(): Trace { | ||
| val intersectionSegment = segments().dropLast(1).random() |
There was a problem hiding this comment.
Why do we need to ignore the last segment? As I understand it, it avoids the scenario when the additional segment lies on the last segment but why? This still would be treated as an intersection.
There was a problem hiding this comment.
Good question! I'd meant to add a comment when moving this to a method and then forgotten:
/**
* Choose random segment, a random (interpolated) point on that segment and then create a new
* trace with an additional point just beyond that to create an intersecting trace.
*
* Never chooses the last segment as a target for intersecting as that can only create a
* collinear intersection which is unlikely to be accurate due to inaccuracies in [interpolate].
*/
This is basically the "two consecutive segments intersect" special case, and we test this it specifically (Trace#intersects returns true when two segments and they intersect).
|
Tested with success Verified on Android 16, 11 Verified cases:
|
|
Tested with success Verified on Android 10 |
Closes #6979
Why is this the best possible solution? Were any other approaches considered?
I've removed the default epsilon from our orientation which is used as part of intersection detection. Instead, now an epsilon can be passed to both
LineSegment#intersectsandTrace#intersects. This allows us to only detect exact collinear intersections (one endpoint touching another line) in the app, while also allowing tolerance for testing with linear interpolation (which is hard to get 100% exact).We may want to use an epsilon in the app as well, but until we get more feedback, avoiding false positives is safer - I think leaving the ability to add one in right now is a good idea.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
This should make it very hard to create an intersection where the endpoint of one segment is touching (or close to touching) another line.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest