search

dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

Make 2-dimensional array accessible in Magnification structs #339

Closed AArnott closed 7 years ago

AArnott commented 7 years ago

I expose these 2-dimensional arrays as an indexer property instead of a 2-dimensional array off a named property. This is a divergence from our typical pattern of keeping to the names used in the header files. The reason is the multi-dimensional array has a fixed length and must be allocated within the struct. As such, it cannot be exposed as an array (much less a multi-dimensional one) to the outside (at least as far as I have been able to determine). So instead, I could offer accessor methods to index into it, or I could expose the one-dimensional fixed array and leave it to callers to figure out how to index into it. I'm siding with aiding with the indexing. And a C# indexer seems more friendly and natural than methods.

I'm open to other ideas if I missed any.

AArnott commented 7 years ago

Good points. My concern is that if we expose the field, it won't match the native definition for that field. In the native definition, the field is a two-dimensional array but ours is only one dimensional. But I'll assume you already considered that with your feedback and I'll go ahead and expose the field.

vbfox commented 7 years ago

I don't think that it's a problem at the pinvoke layer to find 2D arrays in their vector form (When dealing with tools as matlab that store arrays in the invert order from C it's even very common).

I considered suggesting casts between the 2d array form & the structs but I wonder if it's not going over the scope of the library

AArnott commented 7 years ago

I considered suggesting casts between the 2d array form & the structs but I wonder if it's not going over the scope of the library

Can you elaborate? Can you cast between n-dimensions for arrays?

vbfox commented 7 years ago

I meant as an explicit cast that we would define, allowing casting a float[,] of the correct size to the struct type.

AArnott commented 7 years ago

I'm making the change now to expose the field, per your feedback. I'm taking advantage of the opportunity on these newly publicized fields to add an xml doc comment suggesting folks use the struct indexer for convenient indexing into the array.