Update SilkX Proposals #1792
Update SilkX Proposals #1792
Conversation
- Updated Pointer Proposal to match current implementation - Add Generic Math Proposal based on Current work
|
Paging @HurricanKai now we have a formal maths proposal. |
| public Vector2I(TScalar value); | ||
| public Vector2I(ReadOnlySpan<TScalar> span); | ||
|
|
||
| public Span<TScalar> AsSpan(); |
There was a problem hiding this comment.
This needs [UnscopedRef] (maybe [return: UnscopedRef], I forget) to ensure that the created span is correctly tracked.
| /// <returns>The quaternion that contains the summed values of <paramref name="value1" /> and <paramref name="value2" />.</returns> | ||
| /// <remarks>The <see cref="op_Addition" /> method defines the operation of the addition operator for <see cref="Quaternion" /> objects.</remarks> | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static Quaternion<T> operator +(Quaternion<T> value1, Quaternion<T> value2); |
There was a problem hiding this comment.
nit: Prefer left and right over value1 and value2 for these, since they are clearly the left and right sides of the + operator.
| where TVector : IVector<TVector, TScalar> | ||
| where TScalar : INumberBase<TScalar> | ||
| { | ||
| public TVector ScalarsEqual(TVector other); |
There was a problem hiding this comment.
Is this returning AllBitsSet for match and Zero for no match?
If so, do the other types need to expose this concept explicitly? Does there then need to be some IndexOfFirstMatch, IndexOfLastMatch, etc?
Should this just be called Equals (like Vector<T> and Vector128<T>) or maybe CompareEqual?
There was a problem hiding this comment.
What also, about IVectorComparable for the other comparison operators?
| /// <param name="left">The value which multiplies <paramref name="right" />.</param> | ||
| /// <returns>The product of <paramref name="right" /> multiplied-by <paramref name="left" />.</returns> | ||
| /// <exception cref="OverflowException">The product of <paramref name="right" /> multiplied-by <paramref name="left" /> is not representable by <typeparamref name="TVector" />.</exception> | ||
| static virtual TVector operator checked *(TScalar left, TVector right) => right * left; |
There was a problem hiding this comment.
nit: this should be checked(right * left), the other checked operators need to likewise reuse checked so that overflow checking can happen as expected.
| #endregion | ||
| } | ||
|
|
||
| public interface IModulusVector<TVector, TScalar> : |
There was a problem hiding this comment.
Do you really "need" these rather than just having them on INumberVector?
| static virtual TVector UnitX => { get; } | ||
| static virtual TVector UnitY => { get; } | ||
|
|
||
| TScalar X { get; } |
There was a problem hiding this comment.
X could be on IVector, since it wouldn't make sense to define a 0'd dimension vector >.>
-Slimmed it down though
| - For 3D Vectors, a Cross function which takes another vector and returns the cross product with our original vector. | ||
| - A static implementation of this function **must** be available as well. | ||
| - `+`, `-`, `*`, `/`, and `%` operators defined between two vectors of the same type which returns a vector which has had each operation applied component-wise. | ||
| - `+`, `-`, `*`, `/`, and `%` operators defined between a vector and a scalar value that matches the generic type which returns a vector which has had each operation applied component-wise with the scalar value. Both vector first and scalar first should be implemented. |
There was a problem hiding this comment.
scalar-vector only makes sense for *,/,% and im not sure scalar first makes sense for the latter two
There was a problem hiding this comment.
It is component-wise so (4,5) + 5 = (9,10)
This isn't necessary, but more as an easy of use. If generally agreed we shouldn't have these I'll remove
| - RoundToInt(Vector x) | ||
| - Returns `VectorNI`, where N matches the dimensionality of the vector | ||
| - **INFORMATIVE** This may require multiple methods depending on implementation | ||
| - FloorToInt(Vector x) | ||
| - Returns `VectorNI`, where N matches the dimensionality of the vector | ||
| - **INFORMATIVE** This may require multiple methods depending on implementation | ||
| - CeilingToInt(Vector x) | ||
| - Returns `VectorNI`, where N matches the dimensionality of the vector | ||
| - **INFORMATIVE** This may require multiple methods depending on implementation |
There was a problem hiding this comment.
suggest specifying that these should take a type parameter for the particular int scalar type to use
There was a problem hiding this comment.
There is a reason to have one that does and one that does not, which matches Maxine's implementation currently (int can be done implicitly, the others need casts I believe). This is an mainly an implementation detail, that I want to leave up to whoever is implementing it.
-and fix CreateFromPointNormal nomenclature
Making this a set of master types with obfuscated implmentation. Go For Perf
|
I'll merge this for simplicity in the meeting but make sure someone writes notes on the things we need to discuss a la the outstanding comments. |
* Update Proposal - Generation of Library Sources and PInvoke Mechanisms.md * Update documentation/proposals/Proposal - Generation of Library Sources and PInvoke Mechanisms.md * Update documentation/proposals/Proposal - Generation of Library Sources and PInvoke Mechanisms.md * Update documentation/proposals/Proposal - Generation of Library Sources and PInvoke Mechanisms.md * Update SilkX Proposals (#1792) * Update SilkX Proposals - Updated Pointer Proposal to match current implementation - Add Generic Math Proposal based on Current work * Added missing function proposals and fixing some language * Implementing Review Feedback * Major rewrite of Generic Math Proposal -Slimmed it down though * Remove Vector 5 note * Addressing Review Comments * More Review Comments * Vector Transform Adjustments -and fix CreateFromPointNormal nomenclature * Remove the Endians that escaped the first time * Update for Rect/Box changes * Added LH/RH Matrix Functions * Box/Rect changes Making this a set of master types with obfuscated implmentation. Go For Perf * Update Proposal - Generation of Library Sources and PInvoke Mechanisms.md * Bring back contexts - this was noted in WG review * Add meeting notes to the generic math proposal * Update Proposal - Generic Math.md --------- Co-authored-by: Andrew Davis <[email protected]>
Summary of the PR
This PR updates the proposals to reflect work being done on SilkX
Related issues, Discord discussions, or proposals
Proposal - Generation Of Library Sources and PInvoke Mechanism
Proposal - Generic Math