Skip to content

Conversation

gdey
Copy link
Member

@gdey gdey commented Aug 11, 2019

This code only does the triangulation using geom library and structures.

gdey added 4 commits August 10, 2019 15:18
* [geom] Triangle was not getting written because it system did not know how to store it. Made it act like a polygoner, by giving it a LinearRings method.
* [debugger] Added filename atrribute to recorder
* [debugger] Cleaned up log output by guarding it with debug
* Got the top level dir converted over to not use `quadedge/geometry` package.
Ported over subdivision triangulation.
@coveralls
Copy link

coveralls commented Aug 11, 2019

Pull Request Test Coverage Report for Build 236

  • 150 of 728 (20.6%) changed or added relevant lines in 11 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-3.4%) to 58.7%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/debugger/debugger.go 2 4 50.0%
triangle.go 0 3 0.0%
internal/debugger/recorder.go 0 9 0.0%
planar/triangulate/gdey/quadedge/quadedge/stack.go 0 13 0.0%
planar/triangulate/gdey/quadedge/triangulate.go 0 16 0.0%
planar/triangulate/gdey/quadedge/subdivision/qtype.go 21 39 53.85%
planar/triangulate/gdey/quadedge/subdivision/triangle.go 0 41 0.0%
planar/triangulate/gdey/quadedge/quadedge/edge.go 43 98 43.88%
planar/triangulate/gdey/quadedge/quadedge/topo.go 14 72 19.44%
planar/triangulate/gdey/quadedge/subdivision/subdivision.go 56 419 13.37%
Files with Coverage Reduction New Missed Lines %
planar/triangulate/constraineddelaunay/triangulator.go 3 58.52%
Totals Coverage Status
Change from base Build 219: -3.4%
Covered Lines: 5185
Relevant Lines: 8833

💛 - Coveralls

Debugger needs cgo to work. More the recorder is geopackage which requires cgo. So, only initialize the debugger when cgo is enabled.
Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

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

I don't have much understanding of the algorithm here, but no immediate code bugs popped out at me. LGTM

Changed ReplaceAll with Replace, as replace all was introduced in 1.12.
@gdey gdey merged commit dd4788e into master Aug 11, 2019
@gdey gdey deleted the port_gdey branch August 11, 2019 22:35
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.

3 participants