Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions godot-core/src/builtin/aabb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,18 @@ impl Aabb {
self.position + self.size
}

/// Returns the position of one of the 8 vertices that compose this bounding box. With an `idx` of 0 this is the same as position, and an `idx` of 7 is the same as end.
///
/// # Panics
/// If `idx` is greater than 7.
#[inline]
pub fn endpoint(&self, idx: usize) -> Vector3 {
Comment on lines +199 to +204
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the "header" limited to 1 line, just describing what the function does. Details can be part of the "body":

Suggested change
/// Returns the position of one of the 8 vertices that compose this bounding box. With an `idx` of 0 this is the same as position, and an `idx` of 7 is the same as end.
///
/// # Panics
/// If `idx` is greater than 7.
#[inline]
pub fn endpoint(&self, idx: usize) -> Vector3 {
/// Returns the position of one of the 8 vertices that compose this bounding box.
///
/// With an `idx` of 0 this is the same as [`position`][Self::position], and an `idx` of 7 is the same as [`end()`][Self::end].
///
/// # Panics
/// If `idx` is greater than 7.
///
/// _Godot equivalent: `get_endpoint`_
#[doc(alias = "get_endpoint")]
#[inline]
pub fn get_corner(self, idx: usize) -> Vector3 {

I also added links to position/end().

Function should probably be named get_corner:

  • "get" prefix as it takes a parameter -- so consistent with new get_support
  • "endpoint" is really a bad name, and confusing with "end"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*self
.points()
.get(idx)
.expect("Tried to retrieve vertex no. {idx} from Aabb which has only 8 vertices")
}

/// Set size based on desired end-point.
///
/// NOTE: This does not make the AABB absolute, and `Aabb.abs()` should be called if the size becomes negative.
Expand Down Expand Up @@ -249,20 +261,27 @@ impl Aabb {
self.size.x.min(self.size.y.min(self.size.z))
}

/// Returns the support point in a given direction. This is useful for collision detection algorithms.
#[inline]
#[doc(alias = "get_support")]
#[deprecated = "renamed to `get_support`"]
pub fn support(self, dir: Vector3) -> Vector3 {
let half_extents = self.size * 0.5;
let relative_center_point = self.position + half_extents;
self.get_support(dir)
}

let signs = Vector3 {
x: dir.x.signum(),
y: dir.y.signum(),
z: dir.z.signum(),
};
/// Returns the support point in a given direction. This is useful for collision detection algorithms.
#[inline]
pub fn get_support(self, dir: Vector3) -> Vector3 {
let mut support = self.position;
if dir.x > 0. {
support.x += self.size.x;
}
if dir.y > 0. {
support.y += self.size.y;
}
if dir.z > 0. {
support.z += self.size.z;
}

half_extents * signs + relative_center_point
support
}

/// Checks whether two AABBs have at least one point in common.
Expand Down Expand Up @@ -302,20 +321,26 @@ impl Aabb {
&& end.z > b.position.z
}

/// Returns `true` if the AABB is on both sides of a plane.
/// Returns the 8 vertices that compose this bounding box. The first one is the same as `position`, while the last is the same as an `end`.
#[inline]
pub fn intersects_plane(self, plane: Plane) -> bool {
// The set of the edges of the AABB.
let points = [
pub fn points(&self) -> [Vector3; 8] {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub fn points(&self) -> [Vector3; 8] {
pub fn corners(&self) -> [Vector3; 8] {

Would also be good if get_corner + corners were defined next to each other, and had mutual intra-doc links.

As this function doesn't exist in Godot, it doesn't need an "equivalent" doc line.

I wonder if the order of corners should be documented? Is this some canonical ordering?
Do we expect it to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ordering is the same as in Aabb::get_endpoint https://github.com/godotengine/godot/blob/master/core/math/aabb.h#L244 which has been unchanged since the very beginning (back when it was called Rect3).

Equivalent Gdscript code is something along the lines of:

let my_points = [];

for i in range(8):
    my_points.push(my_aabb.get_endpoint(i))

Weird that Godot doesn't expose that – it is pretty common usecase 🤔.

[
self.position,
self.position + Vector3::new(0.0, 0.0, self.size.z),
self.position + Vector3::new(0.0, self.size.y, 0.0),
self.position + Vector3::new(0.0, self.size.y, self.size.z),
self.position + Vector3::new(self.size.x, 0.0, 0.0),
self.position + Vector3::new(self.size.x, self.size.y, 0.0),
self.position + Vector3::new(self.size.x, 0.0, self.size.z),
self.position + Vector3::new(0.0, self.size.y, self.size.z),
self.position + Vector3::new(self.size.x, self.size.y, 0.0),
self.position + self.size,
];
]
}

/// Returns `true` if the AABB is on both sides of a plane.
#[inline]
pub fn intersects_plane(self, plane: Plane) -> bool {
// The set of the edges of the AABB.
let points = self.points();

let mut over = false;
let mut under = false;
Expand Down
129 changes: 129 additions & 0 deletions itest/rust/src/builtin_tests/geometry/aabb_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright (c) godot-rust; Bromeon and contributors.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/
use godot::builtin::inner::InnerAabb;
use godot::builtin::math::assert_eq_approx;
use godot::builtin::{Aabb, Plane, Vector3};

use crate::framework::itest;

const TEST_AABB: Aabb = Aabb::new(Vector3::new(-1.5, 0.0, 2.0), Vector3::new(3.0, 2.0, 4.0));

#[itest]
fn aabb_equiv() {
let inner = InnerAabb::from_outer(&TEST_AABB);
let outer = TEST_AABB;
let test_aabb = Aabb::new(
TEST_AABB.position - Vector3::new(1.0, 0.25, 0.55),
TEST_AABB.size,
);
Comment on lines +19 to +22
Copy link
Member

Choose a reason for hiding this comment

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

Confusing to name test_aabb like TEST_AABB in different case, when it is a different one. I don't know if one of the two can have more specific names? Or at least the constant could be BASE_AABB or so...


#[rustfmt::skip]
let mappings_aabb = [
(
"abs",
inner.abs(),
outer.abs()
),
(
"grow",
inner.grow(4.0),
outer.grow(4.0)
),
(
"intersection",
inner.intersection(test_aabb),
outer.intersect(test_aabb).expect("Must intersect"),
),
(
"interpolate_with",
inner.merge(test_aabb),
outer.merge(test_aabb),
),
];

for (name, inner, outer) in mappings_aabb {
assert_eq_approx!(inner, outer, "function: {name}\n");
}

// Check endpoints as well.
for i in 0..8 {
assert_eq_approx!(
inner.get_endpoint(i as i64),
outer.endpoint(i),
"index: {i}\n"
);
}

#[rustfmt::skip]
let mappings_vector3 = [
(
"center",
inner.get_center(),
outer.center()
),
(
"longest_axis",
inner.get_longest_axis(),
outer.longest_axis().expect("Must have the longest axis"),
),
(
"shortest_axis",
inner.get_shortest_axis(),
outer.shortest_axis().expect("Must have the shortest axis"),
),
(
"support",
inner.get_support(Vector3::UP),
outer.get_support(Vector3::UP),
),
];

for (name, inner, outer) in mappings_vector3 {
assert_eq_approx!(inner, outer, "function: {name}\n");
}

let enclosed = TEST_AABB.grow(-1.0);
let test_plane = Plane::new(Vector3::UP, 0.0);

#[rustfmt::skip]
let mappings_bool = [
(
"encloses",
inner.encloses(enclosed),
outer.encloses(enclosed),
),
(
"has_surface",
inner.has_surface(),
outer.has_surface()
),
(
"has_volume",
inner.has_volume(),
outer.has_volume()
),
(
"intersects",
inner.intersects(enclosed),
outer.intersects(enclosed),
),
(
"intersects_plane",
inner.intersects_plane(test_plane),
outer.intersects_plane(test_plane),
),
(
"is_finite",
inner.is_finite(),
outer.is_finite()
),
];

for (name, inner, outer) in mappings_bool {
assert_eq!(inner, outer, "function: {name}\n");
}
}
1 change: 1 addition & 0 deletions itest/rust/src/builtin_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

mod geometry {
mod aabb_test;
mod basis_test;
mod plane_test;
mod projection_test;
Expand Down
Loading