`BorderRect` is ambiguous

[ickshonpe]

What problem does this solve or what need does it fill?

Reviewer asked a question about the PR #21903 which made me realise this type is confusing.

In UI and world coordinates, “top” and “bottom” mean opposite things, so these fields are ambiguous. Anyone using BorderRect must decide themselves how to apply the vertical insets.

What solution would you like?

Resolve the ambiguity by replacing the directional BorderRect fields (left, right, top, bottom) with two Vec2 fields:

pub struct BorderRect {
    pub min: Vec2,
    pub max: Vec2,
}

Additional context

#15461
#15440

[it-me-joda]

This seems controversial. I find this proposed shape more difficult to reason about and it breaks the consistency with other types inside Bevy and the CSS-like style they all follow for the most part.

[ickshonpe]

This seems controversial. I find this proposed shape more difficult to reason about and it breaks the consistency with other types inside Bevy and the CSS-like style they all follow for the most part.

That's the problem really, BorderRect isn't a UI type, it's stored in bevy_sprite. The reason we use it in the UI as well is because of the shared image slicing API mostly. It's not part of the user UI api, that uses UiRect with Vals to define the layout. BorderRect is only used post-layout during picking and rendering. It's mostly used in conjunction with the Rect type which has min and max Vec2 fields. To combine a BorderRect with a Rect to define a rectangular annulus, currently you do something like:

    rect.min.x += border_rect.left;
    rect.max.x -= border_rect.right;
    rect.min.y += border_rect.top;
    rect.max.y -= border_rect.bottom;

It's easy to introduce subtle errors by adding the wrong edge values together though, and, since borders are most commonly homogeneous, these errors can easily be missed and slip into production code etc. So I wanted to add a helper function to BorderRect to apply the insets:

    pub fn apply_to_rect(self, mut rect: Rect) -> Rect {
        rect.min.x += self.left;
        rect.max.x -= self.right;
        rect.min.y += self.top;
        rect.max.y -= self.bottom;
        rect
    }

And this is a problem. The y-axis increases downwards for UI and upwards for 2d, so the helper function can't work in both contexts.

With the changes in #21915 though, there is no ambiguity:

    pub fn apply_to_rect(self, mut rect: Rect) -> Rect {
        rect.min += self.border_rect.min_inset;
        rect.max -= self.border_rect.max_inset;
        rect
    }

Using the min_inset and max_inset values removes the need to interpret top or bottom relative to the coordinate system, so the same logic works consistently in both UI and 2D.