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

Tetrahedron sampling #13430

Merged
merged 8 commits into from
May 21, 2024
Merged

Conversation

mweatherley
Copy link
Contributor

Objective

Add interior and boundary sampling for the Tetrahedron primitive. This is part of ongoing work to bring the primitives to parity with each other in terms of their capabilities.

Solution

Tetrahedron implements the ShapeSample trait. To support this, there is a new public method Tetrahedron::faces which gets the faces of a tetrahedron as Triangle3ds. There are more sophisticated ideas for getting the faces we might want to consider in the future (e.g. adjusting according to the orientation), but this method gives the most mathematically straightforward answer, giving the faces the orientation induced by the tetrahedron itself.

let [a, b, c, d] = self.vertices;
[
Triangle3d::new(b, c, d),
Triangle3d::new(a, c, d).reversed(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the compiler will probably crunch through this, esp. given the #[inline] but couldn't we just reorder the arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I guess so, but I thought this would make it clearer what's actually going on, since the relationship between vertex order and orientation is otherwise not that clear (e.g. the choice of which vertices to interchange when Triangle3d::reverse is called is essentially arbitrary).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is clearer to me.

crates/bevy_math/src/sampling/shape_sampling.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/sampling/shape_sampling.rs Outdated Show resolved Hide resolved
@alice-i-cecile alice-i-cecile added C-Enhancement A new feature S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it A-Math Fundamental domain-agnostic mathematical operations labels May 20, 2024
@alice-i-cecile
Copy link
Member

A neat little theorem in there about linear transformations preserving probability densities correctly. Do you have a citation at hand?

@mweatherley
Copy link
Contributor Author

A neat little theorem in there about linear transformations preserving probability densities correctly. Do you have a citation at hand?

I guess this is just the standard fact that the Lebesgue measure of a linearly transformed set is the original Lebesgue measure times the determinant of the linear transformation. I can't find a great standalone source, but I guess I could just link to something like the Wikipedia article.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 21, 2024
Merged via the queue into bevyengine:main with commit b7ec19b May 21, 2024
27 checks passed
@mweatherley mweatherley deleted the tetrahedron-sampling branch May 21, 2024 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Enhancement A new feature S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants