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 MBL in non-noded situations #4529

Merged

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Dec 5, 2023

Identify the Bug or Feature request

Fixes #4528

Description of the Change

In the test case attached in the bug report, AWT returns the left and right triangles as separate paths, but the top and bottom triangles are treated as part of the surrounding rectangle by using a self-intersecting path. JTS' Polygonizer expects well-behaved input, which includes not allowing these self-intersection paths.

To solve this, we now make sure to node our geometry prior to polygonizing. This is done using a SnapRoundingNoder to make sure it is robust while also freeing us from having to enforce precision ourselves. And to make sure this sort of problem doesn't fly under the radar in the future, we log any errors that the polygonizer reports.

I've tested on some complex geometry and have not noticed any significant slowdown related to this change.

Possible Drawbacks

Performance is reduced for expose last path when the paths are long.

Documentation Notes

N/A

Release Notes

  • Fixed MBL when a corner of one region is on the edge of another.

This change is Reviewable

We now node our geometry before passing it to the `Polygonizer`. To make sure this is robust, we no longer enforce
precision ourselves but instead rely on `SnapRoundingNoder` to do so as it nodes the geometries.
@kwvanderlinde
Copy link
Collaborator Author

After further testing, I did find a performance regression for expose last path. It's probably not noticable for most typical situations, but for long paths (~400 steps in this case) it does become noticable. This is due to how GeometryUtil.toJts() is applied to the vision at each step during expose last path, which causes the noding costs to really add up. Other situations are able to cache the results of the JTS conversion until topology changes, so they are not susceptible to seeing this cost.

The need for GeometryUtil.toJts() in this case is one of the many things that will end up being removed by #4506, so performance in this area should improve again once that is done.

@cwisniew cwisniew added the bug label Dec 7, 2023
@cwisniew cwisniew added this pull request to the merge queue Dec 7, 2023
Merged via the queue into RPTools:develop with commit dd01594 Dec 7, 2023
4 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/4528-mbl-coner-on-edge branch December 21, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[Bug]: MBL polygons with corners touching edges can break movement
2 participants