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

Polygonize handle isValid and duplicate points/edges #1714

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

Conversation

Traksewt
Copy link

@Traksewt Traksewt commented Jun 17, 2019

Polygonize previously had a TODO for isValid (for EdgeRing) and was hardcoded to return true. The original implementation that this was ported from expected the EdgeRings to have the same start and end point, meaning that the minimum number of vertices for an edge ring would be 4 (as point 1 == point 4). However, this implementation does not add the joining last vertex on the edge ring - it adds it as needed when converting to a polygon. This means that the minimum number of points for a edgering is 3 (forming a triangular polygon).

Polygonize was getting corrupted if the same vertex was getting reused within an edgering. As soon as you encounter a same vertex within an edgering, that edgering can be considered formed and the looping over edges should be aborted. This will allow other joining edges the chance to form other valid edgerings.

Polygonize was not handling when there were multiple connections within the graph between the same nodes. This is fixed by ensuring each edge in the graph is unique.

Polygonize was too trigger happy culling existing vertices by only considering the inner edges when looking for dangles. This did not consider the case for a single circle of edges, where in this case each vertex only has 1 inner edge - making it a candidate to be removed using the existing algorithm. The original algorithm pre-port was looking for nodes with degree 1. A node (as a part of a linear line) with 1 inner and 1 outer is not degree 1 yet it was getting culled by the existing algorithm. By checking the outer edges as well and only culling if inner edges <= 1 and outer edges == 0, it will avoid the case where this node is pointing to another node.

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Have read How To Contribute.
  • Run npm test at the sub modules where changes have occurred.
  • Run npm run lint to ensure code style at the turf module level.

Submitting a new TurfJS Module.

  • Overview description of proposed module.
  • Include JSDocs with a basic example.
  • Execute ./scripts/generate-readmes to create README.md.
  • Add yourself to contributors in package.json using "Full Name <@github Username>".

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

Successfully merging this pull request may close these issues.

None yet

1 participant