Add integer equivalents for `Rect`

[LiamGallagher737]

Objective

Add integer equivalents for the Rect type.

Closes #7967

Solution

• Add IRect and URect
• Use a macro to keep things tidy

Changelog

Added IRect and URect types.

[LiamGallagher737]

I have put this as a draft as I am unsure of how to implement the doc tests for the associated methods. The tests require used of the arguments passed into the macro, however I cannot find an easy way to do this. I can use the #[doc = ...] attribute to do this but it results in very unreadable code, if someone knows a better way to achieve this, please let me know.

Example of #[doc = ...] use. https://github.com/bevyengine/bevy/blob/9439952a9403a97e32c6670597334a079c857fbd/crates/bevy_math/src/rect.rs#L139-L140

[LiamGallagher737]

macro_rules! just wasn't the right tool for the job so I've resorted to just manually implementing both types.

[LiamGallagher737]

Removed the assert for checking if the size is positive in URect's from size and half size methods as UVec2 is always positive. https://github.com/bevyengine/bevy/blob/a19968e085387a795afce6ea8162b7d92b82aaec/crates/bevy_math/src/rects/urect.rs#L77

[nicoburns]

https://github.com/bevyengine/bevy/issues/8540 suggests creating a generic Rect<T> instead. These types could then become aliases. Might that be a better path here to avoid code duplication? We have such types in Taffy https://github.com/DioxusLabs/taffy/blob/main/src/geometry.rs and they work pretty well.

[nicopap]

Note that glam "solves" the issue of code duplication (think of Vec2, IVec2 and UVec2) by using the tera templating engine to generate code.

For bevy, I'd be in favor of what nicoburns describes. Having a Rect<T> and then re-exporting a pub type IRect = Rect<i32> allows having one implementation of Rect, one doc string per method, one set of methods. So when changing something Rect-related, only one place needs to be changed and checked by reviewers later.

The disadvantage is that I always found type aliases a bit confusing in rustdoc (which is what makes glam better than nalgebra imo) but I think a Rect<T> would be the better solution long term

[james7132]

I think the problem with Rect<T> is the need to return glam types (i.e. Vec2, IVec2, UVec2), as well as the associated type level assertions needed for each one potentially being different. This is potentially resolvable by using a trait with associated types, but even then it's a bit rough around the edges in terms of UX when working with the types.

[LiamGallagher737]

For URect I've created another method outset as the inset method takes a positive only integer, however to me it makes more sense for inset to create a smaller rect but the orginal f32 rect implementation has it creating a larger rect and only when a negative number was passed did it create a smaller rect.

For URect I have it so inset creates a smaller rect and outset creates a larger one but this makes it out of sync with IRect and Rect. I'm happy to change IRect to create a smaller rect with a positive inset but as Rect is already in use anyone using the inset method would now have to invert what they are passing in, I think changing this is still the correct way forward but would appreciate some input on this.

[LiamGallagher737]

Did a quick poll on Discord and from this I think the best solution is to have URect's inset method take an i32

[LiamGallagher737]

Someone else also pointed out than inset could return an option instead of an empty rect when a negative insets absolute value is larger than the half size. Personally I think this should be done in a separate PR as it would be a breaking change for Rect and so far, this PR isn't breaking.

Infact all panicking methods should really return an option or result, I can do this in a follow up PR

[djeedai]

it makes more sense for inset to create a smaller rect but the orginal f32 rect implementation has it creating a larger rect and only when a negative number was passed did it create a smaller rect.

The API is inspired by Druid/Kurbo. It takes the current rectangle and adds (positive) an inset, creating a new larger rectangle. https://docs.rs/druid/latest/druid/struct.Rect.html#method.inset

I'm happy to change IRect to create a smaller rect with a positive inset but as Rect is already in use anyone using the inset method would now have to invert what they are passing in, I think changing this is still the correct way forward but would appreciate some input on this.

I'm not. I think the current API is correct. And as you pointed it will break everyone in possibly subtle ways.

Someone else also pointed out than inset could return an option instead of an empty rect when a negative insets absolute value is larger than the half size.

This is redundant. If the user wants to know if the resulting rectangle is empty, they can call is_empty() on the result. There's no need to make things more annoying with an Option in the common case. In general for UI you want to apply all styles and options, then at the very end possibly test if empty as an optimization. Having to check the result of Option at each step makes the API really "heavy" to deal with.