`Rect2 size is negative, this is not supported` error regularly toasting

[Zylann]

Godot version

4.0 ed395c6b99915809347a87b0d65220c256d6ec3f

System information

Windows 10 64 bits GLES3 GeForce GTX 1060 6GB/PCIe/SSE2

Issue description

When I open about any project, the following error always gets printed and brought up by the toast system:

ERROR: Rect2 size is negative, this is not supported. Use Rect2.abs() to get a Rect2 with a positive size.
   at: Rect2::grow_individual (D:\PROJETS\INFO\GODOT\Engine\godot4_fork\core/math/rect2.h:243)

It even appears to be caused by the toast system itself when errors show up, but I'm not certain yet.

It occurs again when other errors get toasted, but I'm not sure what reproduces these. But so far I got it everytime when opening projects.

Steps to reproduce

Open any project. If no toasts show up, make one happen. After it resolves, the error likely shows up

Minimal reproduction project

No response

[Chaosus]

Probably a regression from https://github.com/godotengine/godot/pull/37626

[Zylann]

This happens because a StyleBox is drawn starting from a very thin rectangle: Which is grown negatively and becomes a null-area rectangle: And keeps being grown negatively afterward in the same function, making negative sizes appear: Then the next call to grow_individual, which does start with a check for negative size, highlights the problem.

This can happen when the size of a control is animated: it has inner parts smaller than the outer rectangle. As the control shrinks, the inner rectangle will disappear sooner... and eventually get rendered badly as it gets to a negative size.

So ironically, the function that checks for this invalid state, allows such invalid state to occur.

To be complete in the intent it carries, this function should then probably clamp its size like this:

    inline Rect2 grow_individual(real_t p_left, real_t p_top, real_t p_right, real_t p_bottom) const {
#ifdef MATH_CHECKS
        if (unlikely(size.x < 0 || size.y < 0)) {
            ERR_PRINT("Rect2 size is negative, this is not supported. Use Rect2.abs() to get a Rect2 with a positive size.");
        }
#endif
        Rect2 g = *this;
        g.position.x -= p_left;
        g.position.y -= p_top;
        g.size.width = MAX(0.0, g.size.width + p_left + p_right);
        g.size.height = MAX(0.0, g.size.height + p_top + p_bottom);

We could check the resulting rectangle itself and error if it became negative, however it is probably not as useful because every code using this function would have to do the check anyways (or do we want that?), and it's easier to check afterward if the rectangle has a null size, or just know that it will not be negative.

One side effect of the above snippet is that position will still be offset, but maybe we don't care about it since the rectangle is degenerate?

Another more involved approach to also handle position is to pick the midpoints when the grown edges get past each other:

    static inline void pad_edges(real_t &left, real_t &right, real_t pad_left, real_t pad_right) {
        left += pad_left;
        right += pad_right;
        if (left < right) {
            real_t mid = (left + right) * 0.5;
            left = mid;
            right = mid;
        }
    }

    inline Rect2 grow_individual(real_t p_left, real_t p_top, real_t p_right, real_t p_bottom) const {
        // ...
        Rect2 g = *this;

        real_t left = g.position.x;
        real_t top = g.position.y;
        real_t right = g.position.x + g.size.x;
        real_t bottom = g.position.y + g.size.y;

        pad_edges(left, right, -p_left, p_right);
        pad_edges(top, bottom, -p_top, p_bottom);

        g.position.x = left;
        g.position.y = top;
        g.size.x = right - left;
        g.size.y = bottom - top;

And for consistency, grow_by also needs to do this.

On top of this, the code ending up with null-area rectangles like this could stop drawing them.

Yet another approach is to stop having that check in this function. grow_individual, in isolation, does not fail if the rectangle becomes negative. Only the functions that assume positive sizes would throw errors. However if this is decided, every place that uses a Rect2 must actively check for negative sizes to catch the problem earlier and self-document itself (paranoid programming), instead of producing bad results. This is likely not properly defined at the moment so that would mean a lot of code to review. Also some functions might not even "fail", but produce a bad result either way.

All this, knowing that size is a public variable and could be set to a negative value anytime.

[Zylann]

I just tried the first snippet, seemed to work fine.

I then tried the second snippet, and Godot turned flat: