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

Improve CoverageSimplifier #1060

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dr-jts
Copy link
Contributor

@dr-jts dr-jts commented Jun 19, 2024

Improves CoverageSimplier:

  • Adds small ring removal, with configurable tolerance factor
  • Adds smoothness weighting, to provide smoother results
  • Adds ability to specify separate tolerances for inner and outer edges
  • Adds support for a separate tolerance for each input coverage element

Copy link

@nstrahl nstrahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an error where only one tolerance was considered for each coverage edge


setCoordinates(edges, linesSimp);
int index1 = covEdge.getAdjacentIndex(0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int index1 = covEdge.getAdjacentIndex(1);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Need a unit test to verify that.

@nstrahl
Copy link

nstrahl commented Jun 22, 2024

I have a suggestion here.

Take the geometry from example testInnerHoleTouchingShell.

image

If the tolerances 0,500,500 are provided then the holes are not simplified. I think it should be possible to coarsen hole or island polygons since they do not have any significant overlap with the exterior ring of the geometry enclosing them. Therefore we should probably skip the tolerance comparisson in such a case. A flag could be passed (e.g. forceHoleTolerances) to make this optional.

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

Successfully merging this pull request may close these issues.

None yet

2 participants