Renamed Inflate to Union and added new Inflated implementation

[Phyyl]

Purpose of this PR

• Box.Inflate now has same behavior as System.Drawing's Rectangle.Inflate (extends by a size on all sides)
• Old behavior renamed to Union (grow box to include a point)
• Add Union(Box) to create a box that encapsulates both boxes

Testing status

• Tests where updated and added accordingly, all passes
• Done some manual testing, mostly from my framework moving from RectangleF to Box2

Comments

Terminology between Box structs and System.Drawing.Rectangle structs are divergent. Box.Inflate currently enlarges a rectangle to include a point. However, Rectangle.Inflate actually extends the size of the rectangle uniformly in all directions. I believe this is a lot more consistent with the term Inflate.

In this PR, the old Inflate behavior was moved to Union and Inflate now has the same behavior as Rectangle.

I know this was discussed in #1037 but I truely believe we should keep it in line with System.Drawing. Union makes more sense when you think of two boxes, why not also for a box and a point? Adding to that, What would you call inflating a box otherwise?

The only thing that bothers me is the change in API and the breaking change that this brings. That said, I still went ahead and made the change because I consider it [...] correcting the behaviour to an expected result.

[NogginBops]

Thanks for the PR!

This is a breaking change that is quite bad for users of the math types. It will not give any indication of changes but it will cause a breakage in functionality.

I can agree that it would make sense to match the Inflate behavior of System.Drawing.

There have been previous work on making the box math types better, see: #1341 #1037 (moved with #1378) #1331
We decided to move all of the breaking changes to OpenTK 5. But even then this is a really hard change to make as the signature for the two functions are similar and means it's really hard to tell end users that the behavior has changed.

[Phyyl]

I absolutely understand. One of the possible ways to merge this would be in 2 steps.

1. Add Union(Vector) and deprecate Inflate(Vector) in 4.7.x
2. Re-add Inflate in 5.0.x with new behavior.

This would give time to people that follow the releases but not to those who update from prior version. This also depends on when the release of 5.0 occurs.

[NogginBops]

Sorry for late reply.

I'm thinking we are going to have to make a migration guide for OpenTK 5, so maybe having a function with the same name but different behavior might be fine.

Our unofficial official motto for release dates is "this decade" so OpenTK 5 might come soon, but it might also take a bit.

Are you more interested in getting a function for doing what System.Drawing Inflate does, or are you more interested in "correct" naming for these functions?
I'm ok with deprecating Inflate and introduce Union, I'm also ok with adding a function for doing what System.Drawing does.

[Phyyl]

For naming, it's really confusing to have Inflate act like Join or Union. My changes rename Inflate to Union and add a new method Inflate that grows a box by a size (behavior from System.Drawing.Rectangle). The deprecation thing was an idea for transitioning to the new behavior. That being said, with an upgrade guide this could be skipped and my PR could have its 2 commits merged in my opinion. Let me know if there's anything I can do!

[NogginBops]

The migration guide would be for OpenTK 5, which means that we don't want to do this breaking change in OpenTK 4. We can definitely have the "correct" naming in OpenTK 5, but changing the behavior of this function without anyone being able to tell is not something we are doing to do.

Basically my main problem is that someone who has code using the current behavior of Inflate should be able to detect that something has changed.
So the option here is to either deprecate the old Inflate and explain that they might want to use Union. (or Size += extraSize if that does what the correct Inflate does.)

So I think that just deprecating Inflate and having the deprecation message point to Union + some workaround for Inflate is the way to go.

[NogginBops]

@Phyyl I'm guessing you're no longer going to pursue this change/PR?

[Phyyl]

I'm extremely busy these days and I can't contribute much right now. If there's a more specific task to be done I'd be happy to make room in my schedule but for now I'm using my own types.

[NogginBops]

That's ok, hope you will get time to contribute in the future 🙂
I'm opening an issue to continue to track this issue.

[Phyyl]

Great thanks!