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 bounding volume types #10946

Merged
merged 10 commits into from
Jan 10, 2024

Conversation

NiseVoid
Copy link
Contributor

Objective

Implement bounding volume trait and the 4 types from #10570. I will add intersection tests in a future PR.

Solution

Implement mostly everything as written in the issue, except:

  • Intersection is no longer a method on the bounding volumes, but a separate trait.
  • I implemented a visible_area since it's the most common usecase to care about the surface that could collide with cast rays.
    • Maybe we want both?

Changelog

  • Added bounding volume types to bevy_math

@Jondolf Jondolf self-requested a review December 12, 2023 09:55
@Jondolf Jondolf added C-Feature A new feature, making something new possible A-Math Fundamental domain-agnostic mathematical operations labels Dec 12, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I really like this API and architecture though, and I'm totally content to leave off implementations and extensions for follow-up PRs. Requests for now:

  1. I've left some feedback on test structure to keep things tidy.
  2. I'd like module level doc comments in bounding/mod.rs explaining the relationship between the 3 traits.
  3. I'd like better docs on the traits, explaining how they're intended to be used.

I'd also like to replace bevy_sprite's bespoke Aabb stuff with this, but again, follow-up.

crates/bevy_math/src/bounding/bounded2d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded2d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded2d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded2d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded2d.rs Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/mod.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

Just a couple more language nits

crates/bevy_math/src/bounding/mod.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

Just noticed that properties for 3D volumes are private. Otherwise, I don't think I see any outstanding issues, so I think this should be good. My comment on the bounding circle/sphere radius is non-blocking since you can also just get the radius from the underlying primitive.

crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
crates/bevy_math/src/bounding/bounded3d.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

I think everything looks good now, thanks! More helpers and functionality should probably be implemented in follow-up PRs.

These could be some next steps regarding bounding volumes:

  • Add more helpers (e.g. construct an Aabb2d from a BoundingCircle and vice versa)
  • Implement Bounded2d/Bounded3d for primitives (I have 2D implemented already, PR coming soon)
  • Implement IntersectVolume
    • Intersections between AABBs, bounding circles and bounding spheres
    • Ray intersections with bounding volumes (for raycasting)
    • Maybe shape intersections with bounding volumes (for shapecasting)
  • Replace CollisionBox in bevy_sprite
  • Use for mesh AABBs
  • Long-term: Use for BVHs, collision detection, raycasting, shapecasting, etc.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Great tests and docs, thoughtful API design. Nice work and exceptional patience ;)

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 10, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 10, 2024
Merged via the queue into bevyengine:main with commit c4e479a Jan 10, 2024
22 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jan 18, 2024
# Objective

Closes #10570.

#10946 added bounding volume types and traits, but didn't use them for
anything yet. This PR implements `Bounded2d` and `Bounded3d` for Bevy's
primitive shapes.

## Solution

Implement `Bounded2d` and `Bounded3d` for primitive shapes. This allows
computing AABBs and bounding circles/spheres for them.

For most shapes, there are several ways of implementing bounding
volumes. I took inspiration from [Parry's bounding
volumes](https://github.com/dimforge/parry/tree/master/src/bounding_volume),
[Inigo Quilez](http://iquilezles.org/articles/diskbbox/), and figured
out the rest myself using geometry. I tried to comment all slightly
non-trivial or unclear math to make it understandable.

Parry uses support mapping (finding the farthest point in some direction
for convex shapes) for some AABBs like cones, cylinders, and line
segments. This involves several quat operations and normalizations, so I
opted for the simpler and more efficient geometric approaches shown in
[Quilez's article](http://iquilezles.org/articles/diskbbox/).

Below you can see some of the bounding volumes working in 2D and 3D.
Note that I can't conveniently add these examples yet because they use
primitive shape meshing, which is still WIP.


https://github.com/bevyengine/bevy/assets/57632562/4465cbc6-285b-4c71-b62d-a2b3ee16f8b4


https://github.com/bevyengine/bevy/assets/57632562/94b4ac84-a092-46d7-b438-ce2e971496a4

---

## Changelog

- Implemented `Bounded2d`/`Bounded3d` for primitive shapes
- Added `from_point_cloud` method for bounding volumes (used by many
bounding implementations)
- Added `point_cloud_2d/3d_center` and `rotate_vec2` utility functions
- Added `RegularPolygon::vertices` method (used in regular polygon AABB
construction)
- Added `Triangle::circumcenter` method (used in triangle bounding
circle construction)
- Added bounding circle/sphere creation from AABBs and vice versa

## Extra

Do we want to implement `Bounded2d` for some "3D-ish" shapes too? For
example, capsules are sort of dimension-agnostic and useful for 2D, so I
think that would be good to implement. But a cylinder in 2D is just a
rectangle, and a cone is a triangle, so they wouldn't make as much sense
to me. A conical frustum would be an isosceles trapezoid, which could be
useful, but I'm not sure if computing the 2D AABB of a 3D frustum makes
semantic sense.
github-merge-queue bot pushed a commit that referenced this pull request Jan 22, 2024
# Objective

#10946 added bounding volume types and an `IntersectsVolume` trait, but
didn't actually implement intersections between bounding volumes.

This PR implements AABB-AABB, circle-circle / sphere-sphere, and
AABB-circle / AABB-sphere intersections.

## Solution

Implement `IntersectsVolume` for bounding volume pairs. I also added
`closest_point` methods to return the closest point on the surface /
inside of bounding volumes. This is used for AABB-circle / AABB-sphere
intersections.

---------

Co-authored-by: IQuick 143 <[email protected]>
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-Feature A new feature, making something new possible 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.

5 participants