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

Implement triangle validation and add unit tests #249

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

therealprof
Copy link
Contributor

Closes #240

Signed-off-by: Daniel Egger [email protected]

@therealprof
Copy link
Contributor Author

Please note that for some reason I can't seem to be able to create a 2D triangle because cross product somehow only works for 3D vertices?

Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you, @therealprof, looks good!

Please note that for some reason I can't seem to be able to create a 2D triangle because cross product somehow only works for 3D vertices?

First, I'm going to merge this besides this limitation. The math code isn't exposed in a public library (yet), and we don't seem to create 2D triangles, so it's fine for now.

Second, I think I get it:

  • Mathematically, at least according to my primitive understanding, the cross product is not defined for 2D vectors. The result is a vector that is perpendicular to the input vectors, which can't work in 2D.
  • So the question becomes, why does Vector::cross exist for 2D vectors? Answer: It was an accident, made possible because Matrix::cross exists for all matrices in nalgebra.
  • Digging into that, nalgebra::Matrix::cross is documented to panic for non-3D vectors, and will be removed for non-dynamic/non-3D vectors in the future.

Idea for fixing this:

  • Add a method that converts a Vector<D> into a Vector<3>. If it's a Vector<2>, the z component can be zero.
  • Call that method before computing the triangle area, so the cross product is always done on 3D vectors.

In addition, we should define Vector::cross only for Vector<3>.

I've made notes about all of these action items. I'll either do them myself later, or open issues to track them.

@hannobraun hannobraun merged commit 260d37a into hannobraun:main Feb 25, 2022
@therealprof therealprof deleted the triangle-validation branch February 25, 2022 11:57
@therealprof
Copy link
Contributor Author

therealprof commented Feb 25, 2022

You're right. The cross product is a 3D version of the generalised external product. In 2D the cross product degenerates to the determinant. I think the reason why nalgebra only allows for 3-dimensional vectors is because only then the result will be again a vector. A long time ago I might have known this... 😅

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.

Validate math::Triangle on construction
2 participants