bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.19k stars 3.47k forks source link

Bevy has too many rectangle / box types #13455

Open alice-i-cecile opened 4 months ago

alice-i-cecile commented 4 months ago

Problem

In Bevy, there are currently:

  1. https://dev-docs.bevyengine.org/bevy/a11y/accesskit/struct.Rect.html: f64, re-exported as part of our a11y work
  2. https://dev-docs.bevyengine.org/bevy/math/struct.Rect.html: f32, used in UI
  3. https://dev-docs.bevyengine.org/bevy/math/struct.URect.html: u32, used in UI
  4. https://dev-docs.bevyengine.org/bevy/render/primitives/struct.Aabb.html: f32 3 dimensional axis-aligned bounding box, used in rendering
  5. https://dev-docs.bevyengine.org/bevy/math/bounding/struct.Aabb2d.html: f32, 2 dimensional axis-aligned bounding box
  6. https://dev-docs.bevyengine.org/bevy/math/bounding/struct.Aabb3d.html: f32, 3 dimensional axis-aligned bounding box
  7. https://dev-docs.bevyengine.org/bevy/math/prelude/struct.Rectangle.html: f32, a mathematical primitive 2d shape w
  8. https://dev-docs.bevyengine.org/bevy/math/prelude/struct.Cuboid.html: f32, a mathematical primitive 3d shape
  9. https://dev-docs.bevyengine.org/bevy/ui/struct.UiRect.html: not a rectangle! Instead, used for things like padding

Please let me know if I've missed any.

We don't need this many distinct types for a rectangle! This results in user confusion, inconsistencies, duplicated API surfaces and more.

Proposed changes

  1. Don't re-export a11y types.
  2. Eliminate bevy_render::Aabb in favor of the bevy_math types. Needs benchmarking.
  3. Remove the Rect used by bevy_ui in favor of Rectangle.
  4. Consider how we want to handle integer versions of geometric primitives and do so cohesively.
  5. Investigate if we need distinct Aabb types, and if we do clearly document on them why just using a Rectangle or Cuboid isn't good enough.
Jondolf commented 4 months ago

Rectangle and Cuboid are defined just by their half-extents, and don't have any notion of their position. This is useful for things like colliders (more efficient and ergonomic), but means that they cannot be used as AABBs or replace the rect types.

mockersf commented 4 months ago

I think having a single type for every usages is more confusing than having specific types whose documentation can be tailored to what they're used for. And names of those types should convey their usage.

There's also UiRect which is not a rectangle

rparrett commented 3 months ago

I have been wanting to get rid of NodeRect in the animation_graph example.

s-puig commented 3 months ago

Eliminate bevy_render::Aabb in favor of the bevy_math types. Needs benchmarking.

Render::Aabb is a component and has a trait implementation for a render primitive so you can't remove it. I wouldn't be against newtyping, but delegation in Rust is terrible so i don't think there is anything to win either.

viridia commented 3 months ago

Just as an aside, the equivalent types in three.js are Box2 and Box3, both of which are defined by min/max vectors. (Theoretically you could have BoxN for any N whose min/max values are VecN. In practice, N is always either 2 or 3.)

ua-kxie commented 3 months ago

I think having a single type for every usages is more confusing than having specific types whose documentation can be tailored to what they're used for. And names of those types should convey their usage.

A valid point, but it doesn't feel like this is being applied. prelude::* exports Rectangle, URect, and Rect which all reads as "rectangle". Rectangle contains no position information, but that's not apparent from the name. URect has essentially the same documentation as Rect and it's not suggested anywhere why I might want to use it over Rect.

Rectangle and Cuboid are defined just by their half-extents, and don't have any notion of their position. This is useful for things like colliders (more efficient and ergonomic), but means that they cannot be used as AABBs or replace the rect types.

I ran into a bit of a snag with AabbCast2d where I wasn't sure if I should be providing the positional data in the ray or the aabb. Is Rectangle more suitable here?