Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET: Clear the OpenTK namespace #13160

Closed
rolfbjarne opened this issue Oct 28, 2021 · 2 comments · Fixed by #13767
Closed

.NET: Clear the OpenTK namespace #13160

rolfbjarne opened this issue Oct 28, 2021 · 2 comments · Fixed by #13767
Assignees
Labels
breaking-change If an issue or a pull request represents a breaking change dotnet-pri0 .NET 6: required for stable release enhancement The issue or pull request is an enhancement
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 28, 2021

Types in the OpenTK namespace

We want to remove types from the OpenTK namespace, and these types fall in three categories:

  1. Types we can replace with an existing .NET type
  2. Types that don't have an equivalent .NET type, so we have to come up with an alternative type.
  3. Types we don't need, so we can remove them.

One important question is which namespace to use for 2. Options:

  1. Keep using OpenTK. This is not an option, because that's exactly what we're trying to move away from.
  2. ObjCRuntime: not really very math-like, and this namespace shouldn't become a catch-all for everything that doesn't fit elsewhere either.
  3. CoreNumerics: A mix of Apple's Core* frameworks with System.Numerics.
  4. CoreMath: same as 3.
  5. CoreGraphics: existing Apple namespace. Con: might end up with a name clash in the future depending on what Apple does.
  6. Something else?

Ref: #2571
Ref: https://bugzilla.xamarin.com/show_bug.cgi?id=58599

Different type

The following types have an equivalent in .NET, so the proposal is to use that type instead:

OpenTK type .NET type
OpenTK.Matrix4 System.Numerics.Matrix4x4
OpenTK.Quaternion System.Numerics.Quaternion
OpenTK.Vector2 System.Numerics.Vector2
OpenTK.Vector2d System.Numerics.Vector<double>
OpenTK.Vector2i System.Numerics.Vector<int>
OpenTK.Vector3 System.Numerics.Vector3
OpenTK.Vector3d System.Numerics.Vector<double>
OpenTK.Vector3i System.Numerics.Vector<int>
OpenTK.Vector4 System.Numerics.Vector4
OpenTK.Vector4d System.Numerics.Vector<double>
OpenTK.Vector4i System.Numerics.Vector<int>

Different namespace

The following OpenTK types have no equivalent in .NET, so the proposal is to copy the OpenTK implementation, but use a different namespace.

  • OpenTK.Matrix2
  • OpenTK.Matrix3
  • OpenTK.Matrix4d
  • OpenTK.Quaterniond

The following are types that we created ourselves, but put in the OpenTK namespace.

  • OpenTK.NMatrix4d
  • OpenTK.NMatrix2
  • OpenTK.NMatrix3
  • OpenTK.NMatrix4x3
  • OpenTK.NMatrix4
  • OpenTK.NVector3d
  • OpenTK.NVector3

Types to remove

The following types don't seem to be used by any other API, so they can be removed

  • OpenTK.Box2
  • OpenTK.Functions
  • OpenTK.Half
  • OpenTK.Vector2h
  • OpenTK.Vector3h
  • OpenTK.Vector4h
  • OpenTK.BezierCurve
  • OpenTK.BezierCurveCubic
  • OpenTK.BezierCurveQuadric

Don't know yet

  • ❓OpenTK.MathHelper

Ref: #13087

@rolfbjarne rolfbjarne added enhancement The issue or pull request is an enhancement breaking-change If an issue or a pull request represents a breaking change dotnet-pri0 .NET 6: required for stable release labels Oct 28, 2021
@rolfbjarne rolfbjarne added this to the .NET 6 milestone Oct 28, 2021
@akoeplinger
Copy link
Member

For OpenTK.Matrix2, OpenTK.Matrix3 and OpenTK.Matrix4d I wonder if you could use ValueTuple<Vector3, Vector3, Vector3> etc. instead.

@Perksey
Copy link

Perksey commented Nov 6, 2021

For OpenTK.Matrix2, OpenTK.Matrix3 and OpenTK.Matrix4d I wonder if you could use ValueTuple<Vector3, Vector3, Vector3> etc. instead.

Sadly not, value tuples have auto layout rather than sequential: not suitable for interop...


System.Numerics.Vector

Note: Vector isn't quite akin to Vector2, namely it's variably sized and actually might hold more than 2 Ts in some cases. Because of this, the memory layout probably isn't suitable for your needs (interop, presumably?)

There's a proposal for generic "dimensional" vectors (dotnet/runtime#24168), but these have been postponed to .NET 7. The best solution is probably to whip up your own vectors in a Xamarin namespace. When .NET 7 generic vectors become available, either implicit casts can be added or throw those types out if you still feel in the mood to break things between major releases.


For your reference:

We've been down this road and made our own maths library which fills these holes for us (i.e. MathHelper, called Scalar in our library; and generic vector types like Vector2D). It's possibly a bit overkill for what Xamarin needs, but figured I'd drop it here for your reference (please change the namespace if you use anything): https://github.com/dotnet/Silk.NET/tree/main/src/Maths/Silk.NET.Maths


One more thing to add is please don't call anything VectorN or MatrixNxN regardless of namespace as these will conflict with System.Numerics if ever they get those types as well if users are using both namespaces - this is why the above link uses VectorND for its vectors and a capital X for its matrices.

StephaneDelcroix added a commit that referenced this issue Nov 10, 2021
Clean the OpenTK namespace, part 0: remove unused types (#13160)

Removing:
 - Box2,
 - Functions
 - Half
 - Vector[2|3|4]h
 - BezierCuve*
StephaneDelcroix added a commit that referenced this issue Jan 18, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 19, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 19, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 19, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 21, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 25, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 26, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
StephaneDelcroix added a commit that referenced this issue Jan 27, 2022
fixes #13160

- removes unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace
@rolfbjarne rolfbjarne mentioned this issue Feb 3, 2022
44 tasks
rolfbjarne added a commit that referenced this issue Feb 14, 2022
fixes #13160

- remove unused types
- use System.Numerics when possible
- move own created types from OpenTK namespace to CoreGraphics
- create missing types in CoreGraphics namespace

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
@ghost ghost locked as resolved and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change If an issue or a pull request represents a breaking change dotnet-pri0 .NET 6: required for stable release enhancement The issue or pull request is an enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants