Skip to content

Feature/glam cross constants #21561

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+158 −0
Open

Feature/glam cross constants #21561

Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cf4b1dd
add CARDINALS constant to Dir2 and Dir3
Oct 16, 2025
9b2e9e1
added the DIAGONALS const
Oct 16, 2025
c10d08f
added ALL_NEIGHBORS constant
Oct 16, 2025
35ab172
applied formatter to new changes
Oct 16, 2025
a913e2b
fix clippy warning about precision
Oct 16, 2025
003b605
updated the 1/sqrt(3) const to be in line with the unstable rust defi…
Oct 16, 2025
99986cc
added reason for allow of excessive precision
Oct 16, 2025
4be20e8
replaced allow with expect as per CI hint
Oct 16, 2025
ddf0d5a
formatting and typo fixes
Oct 16, 2025
91452e2
added ALL_EDGES group and fixed the ALL_NEIGHBORS to have all the nei…
pin3free Oct 17, 2025
f6c1789
fix documentation terminology for neighbors
pin3free Oct 17, 2025
621263b
fix typos
pin3free Oct 17, 2025
a8f891d
Update crates/bevy_math/src/direction.rs
pine-free Oct 17, 2025
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
133 changes: 132 additions & 1 deletion crates/bevy_math/src/direction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use thiserror::Error;
/// An error indicating that a direction is invalid.
#[derive(Debug, PartialEq, Error)]
pub enum InvalidDirectionError {
/// The length of the direction vector is zero or very close to zero.
/// The length of the direction vector is zero or very close to zero.XES
#[error("The length of the direction vector is zero or very close to zero")]
Zero,
/// The length of the direction vector is `std::f32::INFINITY`.
Expand Down Expand Up @@ -107,6 +107,8 @@ impl Dir2 {
pub const NEG_Y: Self = Self(Vec2::NEG_Y);
/// The directional axes.
pub const AXES: [Self; 2] = [Self::X, Self::Y];
/// The cardinal directions.
pub const CARDINALS: [Self; 4] = [Self::X, Self::NEG_X, Self::Y, Self::NEG_Y];

/// The "north" direction, equivalent to [`Dir2::Y`].
pub const NORTH: Self = Self(Vec2::Y);
Expand All @@ -125,6 +127,25 @@ impl Dir2 {
/// The "south-west" direction, between [`Dir2::SOUTH`] and [`Dir2::WEST`].
pub const SOUTH_WEST: Self = Self(Vec2::new(-FRAC_1_SQRT_2, -FRAC_1_SQRT_2));

/// The diagonals between the cardinal directions.
pub const DIAGONALS: [Self; 4] = [
Self::NORTH_EAST,
Self::NORTH_WEST,
Self::SOUTH_EAST,
Self::SOUTH_WEST,
];
/// All neighboring directions on a grid. A combination of [`Self::CARDINALS`] and [`Self::DIAGONALS`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Neighborhoods aren't a standardised concept, so I think it would be good to specify it's in a 3x3 neighboorhood. Similarly for the 3d case specify a 3x3x3 neighboorhood.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

annotated the ALL_NEIGHBORDS collections with a "on a cube/square grid in a 3x3(x3) neighborhood"

pub const ALL_NEIGHBORS: [Self; 8] = [
Self::X,
Self::NEG_X,
Self::Y,
Self::NEG_Y,
Self::NORTH_EAST,
Self::NORTH_WEST,
Self::SOUTH_EAST,
Self::SOUTH_WEST,
];

/// Create a direction from a finite, nonzero [`Vec2`], normalizing it.
///
/// Returns [`Err(InvalidDirectionError)`](InvalidDirectionError) if the length
Expand Down Expand Up @@ -395,6 +416,116 @@ impl Dir3 {
pub const NEG_Z: Self = Self(Vec3::NEG_Z);
/// The directional axes.
pub const AXES: [Self; 3] = [Self::X, Self::Y, Self::Z];
/// The cardinal directions.
pub const CARDINALS: [Self; 6] = [
Self::X,
Self::NEG_X,
Self::Y,
Self::NEG_Y,
Self::Z,
Self::NEG_Z,
];

// Adding this allow here to make sure that the precision in FRAC_1_SQRT_2
// and here is the same
#[expect(
clippy::excessive_precision,
reason = "compatibility with the unstable rust definition"
)]
/// Approximation of 1/sqrt(3) needed for the diagonals in 3D space
const FRAC_1_SQRT_3: f32 = 0.577350269189625764509148780501957456_f32;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit weird, since an f32 can't hold more than about 7 digits of precision, but I don't know if there's maybe another reason to want an extra-precise constant definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these excessively long definitions are copied from the rust std::f32::consts, but I'm unsure why they would be there in the first place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i propose to keep this definition for now for the sake of consistency with the rust stdlib, so that when the FRAC_1_SQRT_3 gets stabilized we can just switch over to that and be certain that there's no weird behavior from unexpected changes

/// The diagonals between the cardinal directions.
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation here is a little confusing, from the comment I understood that this is a diagonal between two cardinal directions, like a diagonal between X and Y, but it's diagonals between triplets of cardinal directions. So it's directions pointing towards the vertices of a cube centered at the origin / the main diagonals of such a cube.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

changed the documentation to reflect that these are directions to vertices

the directions between 2 lines are represented in the ALL_EDGES group

pub const DIAGONALS: [Self; 8] = [
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
];
/// All neighboring directions on a grid. A combination of [`Self::CARDINALS`] and [`Self::DIAGONALS`]
pub const ALL_NEIGHBORS: [Self; 14] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't all neighbors in a 3x3x3 range, it's the directly adjacent ones coresponding to 6 faces of a cube + the ones corresponding to the 8 vertices of a cube, but then there's missing 12 neighbors which correspond to the edges of a cube (which are closer than the corner ones, so it imho doesn't make sense to not include them). Overall there should be 26 of them (Since 3x3x3 = 27 and we remove the center).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooh, thank you! missed that. will add later

I'm guessing we would want a definition for them as well, at least for consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

added the ALL_EDGES collection

Self::X,
Self::NEG_X,
Self::Y,
Self::NEG_Y,
Self::Z,
Self::NEG_Z,
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
Self(Vec3::new(
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
-Self::FRAC_1_SQRT_3,
)),
];

/// Create a direction from a finite, nonzero [`Vec3`], normalizing it.
///
Expand Down