-
Notifications
You must be signed in to change notification settings - Fork 137
unified ManifoldParams and PolygonParams #1167
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why those tests are failing? I didn't notice anything functionally wrong in my read-through.
src/polygon.cpp
Outdated
@@ -528,7 +527,7 @@ class EarClip { | |||
|
|||
void PrintVert() const { | |||
#ifdef MANIFOLD_DEBUG | |||
if (!params.verbose) return; | |||
if (!ManifoldParams().verbose) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to separate the verbosity or have levels or something. The triangulator is orders of magnitude more verbose than the Boolean is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is now an int
Oh, I forgot to disable that test :P |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1167 +/- ##
==========================================
- Coverage 91.70% 91.60% -0.11%
==========================================
Files 32 32
Lines 5980 5989 +9
==========================================
+ Hits 5484 5486 +2
- Misses 496 503 +7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
The old way of specifying parameters for manifold and polygon separately is quite confusing. This merges them, and removed
PolygonParams
all together.This is a small breaking that hopefully no one is using, except our own testing code. That said, the next release should still bump the minor version (3.1.0) together with other changes in decimation etc.