Fix bug filtering collinear points on vertical lines#81155
Conversation
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
| } | ||
| if (linearRing.getLon(i - 1) == linearRing.getLon(i + 1) | ||
| && linearRing.getLat(i - 1) > linearRing.getLat(i) != linearRing.getLat(i + 1) > linearRing.getLat(i)) { | ||
| // coplanar |
There was a problem hiding this comment.
I am a bit confused about terminology and the logic here. We have only 2 coordinates, so theoretically all our points are coplanar. Do you mean collinear here? Based on this logic we only remove points if we go down and then up within the same longitude. What's the reason for not removing colinear points in between? Basically, in the example below: POLYGON ((0 0, 0 10, 10 10, 10 5, 10 -10, 10 0, 0 0)) the current logic removes point B, but keep point A intact.
There was a problem hiding this comment.
doh! Yes, I meant collinear points
There was a problem hiding this comment.
I had a look and the current logic that what I expect that is to remove point A:
original: "POLYGON ((0 0, 0 10, 10 10, 10 5, 10 -10, 10 0, 0 0))"
processed: "POLYGON ((10 0, 10 -10, 10 10, 0 10, 0 0, 10 0))"
imotov
left a comment
There was a problem hiding this comment.
LGTM. Please add comments around this check so future me will not get confused again :)
In #59501 we added a process to remove collinear points that exist in vertical lines for polygons as they were making the polygon decomposition logic to fail. Unfortunately this logic introduced a bug while filtering the points that is fixed here.
fixes #81076