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

Add some basic provenance tracking for triangles #374

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ryantrinkle
Copy link
Contributor

No description provided.

@ryantrinkle
Copy link
Contributor Author

@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
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this bool?

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

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

And this int?

Copy link
Contributor Author

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.

@isovector
Copy link
Contributor

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.)

@ryantrinkle
Copy link
Contributor Author

@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)!

@ryantrinkle
Copy link
Contributor Author

ryantrinkle commented Feb 27, 2021

TODO:

  • i need to know the performance impact of keeping a tag around on every single triangle (it did not seem massive, but i'm sure it's nonzero)
  • it needs an official interface to get the data out, rather than Debug.Trace.trace
  • A couple of the provenance things are just Ints, which is silly
  • i don't think i completely followed the style rules

@julialongtin
Copy link
Member

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

Copy link
Member

@julialongtin julialongtin left a 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
Copy link
Member

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Also this one.

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.

4 participants