craftworkgames / MonoGame.Extended

Extensions to make MonoGame more awesome
http://www.monogameextended.net/
Other
1.44k stars 324 forks source link

[Core] Consider replacing Matrix2 and BitVector32 with equivalents #510

Closed craftworkgames closed 5 months ago

craftworkgames commented 6 years ago

Now that we're on .NET Core there's a few classes in our library that might be able to be replaced by similar classes in .NET Standard.

Matrix2 (or Matrix2D) can be replaced with Matrix3x2

BitVector32 can be replaced with System.Collections.Specialized.BitVector32

It would be interesting to do performance tests to see if there's any difference between the implementations.

stefanrbk commented 6 years ago

I'll look at these

stefanrbk commented 6 years ago

I have looked at MonoGame.Extended.Collections.BitVector32 and System.Collections.Specialized.BitVector32. The only tests I did involved the constructor with a literal, constructors with Min and Max constants, and a sequence of 14 indexed get/set commands. I ran these 50 million times. The difference in performance is +/- 100ms in total. I am going to examine the features to make sure they are compatible, but we could switch to .NET's version.

stefanrbk commented 6 years ago

I'm looking into the Matrix3x2 now and it looks like .NET has a new Vector2 as well. I'm playing around with both of those right now as well!

stefanrbk commented 6 years ago

Matrix3x2 is a bit more involved to implement. We would need to switch everything from Microsoft.Xna.Framework.Vector2 over to System.Numerics.Vector2. I'm unsure what performance differences we would see, but it's a pretty large rework as we need to disambiguate on every source file we use Microsoft.Xna.Framework.

BitVector32 is a few changes and I could implement it pretty quick if we wanna drop some unneeded source files.

Something else I ran across is Point2 in our code, which acts just like a Vector2. They are interchangeable since they are able to implicitly convert from one to the other, but that kind of implicit casting can impact performance. Is there a need for the float-based PointX instead of just using VectorX?

craftworkgames commented 6 years ago

it looks like .NET has a new Vector2

I don't think a new Vector2 is going to be of much use to us since that's defined in MonoGame. We don't have any control over it.

We would need to switch everything from Microsoft.Xna.Framework.Vector2 over to System.Numerics.Vector2

Yeah, that's not going to happen. Might as well stop there and keep using our current solution.

BitVector32 is a few changes and I could implement it pretty quick if we wanna drop some unneeded source files.

What do you mean by "implement it"? The goal here is to simply delete our implementation and use the one in .NET Core instead.

Something else I ran across is Point2 in our code, which acts just like a Vector2. They are interchangeable since they are able to implicitly convert from one to the other, but that kind of implicit casting can impact performance. Is there a need for the float-based PointX instead of just using VectorX?

That's a good question. To be honest I've been uncertain of the value of those structs since we added them way back when. If the implicit conversion is indeed impacting performance that's probably a good enough reason to remove them.

Honestly, it's difficult to say if anyone actually uses them but if Point2 is identical to Vector2 in every way then it's a minimal impact breaking change.

stefanrbk commented 6 years ago

What do you mean by "implement it"? The goal here is to simply delete our implementation and use the one in .NET Core instead.

The .NET version uses int where ours uses uint. Haha But otherwise it seems to behave the same.

lithiumtoast commented 6 years ago

Something else I ran across is Point2 in our code, which acts just like a Vector2.

They are fundamentally different types in mathematics.

From the book Fundamental of Computer Graphics, 4th Edition, Chapter 2.2: Vectors, pg 21. (Emphasis mine.)

A vector describes a length and a direction. It can be usefully represented by an arrow. Two vectors are equal if they have the same length and direction even if we think of them as being located in different places. ... Vectors can be used to represent many different things. For example, they can be used to store an offset, also called a displacement. ... Vectors can also be used to store a location, another word for position or point. Locations can be represented as a displacement from another location. Usually there is some understood origin location from which all other locations are stored as offsets. Note that locations are not vectors. ... you can add two vectors. However, it usually does not make sense to add two locations unless it is an intermediate operation when computing weighted averages of a location. Adding two offsets does make sense, so that is one reason why offsets are vectors. But this emphasizes that a location is not an offset; it is an offset from a specific origin location. The offset by itself is not the location.

So the value of having Point2 and Vector2 is readability and comprehension of code. Point2 is a location. Vector2 is a position, or offset, from a location. Wether to drop points altogether in favour of using just vectors is a hot topic of a debate in game development. Some believe that in the context of making games since they both have the same memory signature abusing the notation is okay, especially since (0,0) can be considered the origin for any vector. Others stick to fact that a point and a vector are conceptually different in mathematics. Personally I'm in the latter camp; when it comes to doing mathematics I want to be make sure the code is clear and readable as possible.

that kind of implicit casting can impact performance

Did you profile? I personally have fallen into the trap of making assumptions about performance far too often now. There is a lot going on behind the scenes from the compiler's point of view these days both with JIT and AOT. Also note that as an open source library, people can easily modify the code and remove Point2 if they feel so strongly about it.

stefanrbk commented 6 years ago

I did not do any profiling, but that was the next thing I was going to do. From the theory standpoint, any cast has an overhead and a processing time. I know it isn't the same as practical tests though. Mathematically, it makes sense to differentiate the types. I'll have to look at the differences more closely, but there are parts of the code where the there are 3+ casts between Vector2 and Point2 on the same line. I'll do some checking and probably make a separate issue with discussion.

craftworkgames commented 6 years ago

I'll do some checking and probably make a separate issue with discussion.

Did you get around to doing anything here? Probably best to just keep the discussion in this issue.

stefanrbk commented 6 years ago

I haven't had a chance to test this yet. Kinda trying to figure out how to approach the profiling...

lithiumtoast commented 3 years ago

This is being done in v4 with forking FNA. System.Numerics is being pulled in as a dependency.

binarycow commented 2 years ago

On the topic of BitVector32...

With .NET 6 / C# 10, "static abstract" interface members is in preview (should be non-preview with .NET 7)

While you wouldn't be able to useSystem.Collections.Specialized.BitVector32, using static abstract interface members would allow you to do this:

var byteVector = new BitVector<byte>();
var ushortVector = new BitVector<ushort>();
var uintVector = new BitVector<uint>();
var ulongVector = new BitVector<ulong>();

Otherwise, it works (almost) exactly like BitVector32, but can work for any type that implements the necessary interfaces.

Demo Source Code

AristurtleDev commented 5 months ago

Replacing Matrix2 with the System.Numerics.Matrix3x2 would be ideal, however it's going to be a nightmare to maintain at the moment. There would be a lot of code base internally to bounce between types like System.Numerics.Vector2 and Microsoft.Xna.Framework.Vector2.

From a user/consumer perspective, they are using MonoGame and we should supply the types back that are expected within MonoGame. So for this reason, I think it's best to hold off on System.Numerics at the moment. It does appear form conversation in the MonoGame discord and from this issue https://github.com/MonoGame/MonoGame/issues/7204 that MonoGame may implement Numerics in 5.0, and can be revisited then.

I've implemented a Matrix3x2 as a replacement for Matrix2 in #870. This largely contains the same Api as Matrix2 did, however some performance increases were implemented, especially round the way the Translation, Rotation and Scale properties are used. The new implementation is basically a union type you would find in C++.

AristurtleDev commented 5 months ago

BitVector32 was removed in commit https://github.com/craftworkgames/MonoGame.Extended/commit/6e73a59055d4f664748d35daad4b74b25526d92d by dylan previously.

Closing this issue

@binarycow if you would like to revisit the BitVector<T> type, please open a new issue for a feature request