dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.24k stars 4.73k forks source link

Adding "to array" methods to Vectors and Matrices #37403

Open Per-Malmberg opened 4 years ago

Per-Malmberg commented 4 years ago

System.Numerics Matrices do not seem to have any way to convert their values to flat arrays.

This would be helpful when you want to pass data to for example OpenGL via OpenTK or in my case when I have written my own C# bindings to Opengl and Vulkan.

I'd be happy to implement this if it's deemed suitable.

ghost commented 4 years ago

Tagging subscribers to this area: @tannergooding Notify danmosemsft if you want to be subscribed.

BreyerW commented 4 years ago

Span property getter would be best if possible. No allocation no copying useful indexer (returns ref) and if you really need copied data .toarray is available on span itself

Per-Malmberg commented 4 years ago

Span property getter would be best if possible. No allocation no copying useful indexer (returns ref) and if you really need copied data .toarray is available on span itself

I agree, but I believe matrices (System.Numerics only contains 3x2 and 4x4) are currently implemented using fields for each row/column. Not sure what impact using an array in the "background" would have.

BreyerW commented 4 years ago

@shakazed no no i dont think you need backing array with MemoryMarshall.CreateSpan although i dont remember if this work with classes only but documentation doesnt mention any constraints so i assume it works with structs too

Eg for matrix4x4 it would be MemoryMarshall.CreateSpan(ref M11,16); //16 because 16 float fields

If it doesnt work the way im thinking then you can use pointer constructor of span itself and unsafe block inside property preventing the "leak" of unsafe to outside

Per-Malmberg commented 4 years ago

Interesting. It looks like it should work for any type. I'll try it out and ser how it does :)

tannergooding commented 4 years ago

The vector types (but not the matrix types, unfortunately), have .CopyTo methods which allow going from a vector to an array or span. Likewise they have constructors which allow going from an array/span to a vector.

I think it would be reasonable to propose equivalents for the matrix and other types.

tannergooding commented 4 years ago

If we want to expose equivalent APIs on the Matrix4x4 and related types, then the API review process is detailed here: https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md

Essentially the original post would need to be updated with an API proposal similar to the good example listed under step one of the API Review Process. The only real "required" sections are Rationale and Proposed API, but the other sections can help clarify things and help with the general review process.

BreyerW commented 4 years ago

@shakazed If CreateSpan doesnt work inside property within struct (it has been awhile since i worked with spans and refs limits so my memory is fuzzy what works and what doesnt esp inside struct) and you really need toarray capabilities right now try using createspan outside struct itself like var matrix=new Matrix4x4(); var arr=MemoryMarshal.CreateSpan(ref matrix.M11,16).ToArray();

Of course thats a bit wordy/boilerplate-y since ur going to copy it everywhere u need array out of vectors/matrixes but should work right now and ideally in future what @tannergooding said will happen

tannergooding commented 4 years ago

It's worth noting that while MemoryMarshal.CreateSpan will work, it is unsafe and fully bypasses the normal lifetime rules of C#, so unless you are confident that you are still respecting those, I would caution against using it as it is very easy to start pointing to "dead" memory.

BreyerW commented 4 years ago

@tannergooding if one use .ToArray() immediately after span creation and used that array it should be ok right? (like in my most recent example) Of course this means allocation and memory copy but no safety concerns.

If allocation is a no then rule of thumb would be to never return nor store created span and no use-after-free/dispose while passing down span for consuming is ok as long as consumer doesnt store span nor dispose/free backing data correct?

Per-Malmberg commented 4 years ago

@tannergooding I had missed that there are CopyTo on Vectors. I agree that Matrices should have similar methods. I will rewrite the issue as a API proposal.