Doc improve extrude#1521
Conversation
Choose some self-documenting variable names. Don't mutate. Add a few comments about the rest.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1521 +/- ##
=======================================
Coverage 92.41% 92.41%
=======================================
Files 35 35
Lines 6024 6029 +5
=======================================
+ Hits 5567 5572 +5
Misses 457 457 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Not surprising we get slightly smaller volumes.
|
I want to do some visual comparisons of some shapes before I believe the comment. |
elalish
left a comment
elalish
left a comment
There was a problem hiding this comment.
Good idea - out of curiosity, what does "Doc" refer to in your PR's name? I'll review in earnest when it's not marked draft.
| // OpenSCAD chooses the shorter diagonal because it "works well in | ||
| // most cases". | ||
| if (la::distance2(vertPos[thisVert], vertPos[lastVertLower]) >= | ||
| la::distance2(lastVertPos, vertPos[thisVertLower])) { |
There was a problem hiding this comment.
Good call, but can we skip this check for the common case of zero rotation? Also, I believe this check should always yield the same result for the same polygon index of each slice, so perhaps it can be calculated just once for the polygon?
|
Are you still working on this? |
|
Yes, sorry, I will get back to this but it'll be a while. I'm deep in trying to get extrusion to stay within a specified error tolerance. Some of that code might make parts of this redundant, well, not that there's a lot here. I still need to check visually whether this is the right thing. |
2 participants
Choose some self-documenting variable names. Don't mutate. Add a few
comments about the rest.
Includes my other PR because I was using it to test.