-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
Add some basic provenance tracking for triangles #374
base: master
Are you sure you want to change the base?
Conversation
adcdb95
to
420815d
Compare
@isovector This PR isn't ready for merge, but if you've got the time I would love to hear your thoughts on the overall concept! I believe you've looked at this same area of the code quite a bit. |
removeTriangleMeshAnnotations (AnnotatedTriangleMesh l) = TriangleMesh $ map fst l | ||
|
||
data TriangleProvenance | ||
= TriangleProvenance_SquareToTri Bool TriangleProvenance |
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.
What is this bool?
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.
There are just two call sites :P I'll put an enum type in before we consider merging.
= TriangleProvenance_SquareToTri Bool TriangleProvenance | ||
| TriangleProvenance_JoinXAligned TriangleProvenance TriangleProvenance | ||
| TriangleProvenance_JoinYAligned TriangleProvenance TriangleProvenance | ||
| TriangleProvenance_TesselateLoop Int |
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.
And this int?
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.
Yep, just an enum I haven't bothered writing yet.
I like the idea. One issue we ran into when trying to fix the whiskers (#353) is that the triangles were fine, but the points they contained were crazy. In a codebase as numerically heavy as this one, the solution was to manually binary search some asserts through the codepath, but it took a few days. I'm not sure what your intended use case is for this stuff, but it seems to me like you might want to track an analogous sort of providence (where in the code it came from, rather than just how it's built conceptually.) |
@isovector Awesome! Yes, I agree, code locations would also be useful - in fact, I originally started with CallStack rather than a custom datastructure. Since sometimes triangles get combined, I wanted to be able to track those back to the ones they were combined from, so CallStack isn't enough on its own, but it'd be nice to also have that in there. Regarding the whiskers: I think i've solved some of them in #375, though I definitely did not improve the solving; it looks like your patch does. It would be great to have points that are "pretty good" instead of just "not wildly bad" (about as much as my code does)! |
TODO:
|
for official output, i recommend adding a 'debugasciistl' file type, that is an ascii stl, with the final 'solid' being followed by this metadata, in structured form. Or, implement 3mf. :D |
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.
looks like a good start. :)
@@ -21,6 +21,8 @@ import Graphics.Implicit.ObjectUtil (getBox2, getBox3) | |||
import Data.Foldable(fold) | |||
import Linear ( V3(V3), V2(V2) ) | |||
|
|||
import GHC.Stack |
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.
what's this imported for?
import Graphics.Implicit.ObjectUtil (getBox3) | ||
import Graphics.Implicit.MathUtil(box3sWithin) | ||
|
||
import Control.Arrow(first, second) | ||
|
||
import Debug.Trace |
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.
Also this one.
No description provided.