Rect2::has_point() breaks with negative size fixed

[ThakeeNathees]

Fix: #37617

Bugsquad edit: Mutually exclusive with at least a part of #37626

[groud]

All this code could simply be replaced by a call to Rect2.abs() on a copy of the rect. And then the remaining code could be left intact if ran on the copy instead.

[ThakeeNathees]

@groud I've simplified the code with abs() and not modifying the negative size ( unless there is no way to tell if the rect is flipped )

[groud]

My only question left on this PR is likely the cost of creating a "duplicate" abs() rect every time. Maybe we should add a check on the size signs first to allow the creation of a new object only when required ?

Anyway, this kind of fix is, for me, preferred over #37626. The documentation might be updated though, to explain what a negative size means on such Rect2.

[akien-mga]

My only question left on this PR is likely the cost of creating a "duplicate" abs() rect every time. Maybe we should add a check on the size signs first to allow the creation of a new object only when required ?

I guess we could cache the abs rect as an alternative representation of the negative sized rect, and use it in the methods that wouldn't work with negative size. But that depends on what the final consensus on #37626 would be indeed.

[aaronfranke]

I think it would be better to require the user to write .abs().has_point() if there is a negative size. Doing this automatically can be bad for performance, and it does things behind the user's back (I know @reduz hates doing things behind the user's back).

Therefore, this should either be a debug warning (as I propose in #37626), or stated in the documentation (or both).

[groud]

Doing this automatically can be bad for performance, and it does things behind the user's back (I know @reduz hates doing things behind the user's back).

I disagree on that, it's not doing something in the user back. For me, a Rect2d with a negative size is a perfectly valid use case for which all API functions should work correctly. Whether or not we use abs() internally to implement the function or not is not important, as it does not change anything to the end user.

[reduz]

This is an interesting discussion. I am more in favor of thinking that if you use negative size values, has_point should always return false, so I think the current behavior is the expected one.

[mhilbrunner]

Closing, we discussed in a PR meeting and decided to go ahead with #37626 instead.
Thanks!