Closed antoinecla closed 6 years ago
Thanks!
Funny that I started writing Vertex tests some day ago. Anyway I'm still working on it, especially on Equality definition. Anyway the IVertex
interface is removed; at the current state, it is a YAGNI failure which I want to fix.
At the moment, the vector component comparison is relying on default operators:
public bool Equals(Vertex3f other)
{
return (x == other.x && y == other.y && z == other.z);
}
This implementation have simplified the T4 templates, and it seems more coherent, since it behaves just like anyone would intend. The previous implementation was intended to matrix/vector test scenarios, with a "large" absolute precision (high floating-point values should be avoided in real scenario, to preserve precision, isn't?).
The current implementation should support edge cases (i.e. NaN, Inf), but it still need to be tested. I'm thinking to introduce the following method for supporting the previous semantic:
public bool Equals(Vertex3f other, float precision)
To support relative precision, maybe a method like this could be cool:
public bool RelativelyEquals(Vertex3f other, float precision)
Excellent. Thanks for you detailed feedback.
I just found a similar issue in MatrixBase.IsIdentity()
and MatrixBase.Equals()
.
It could be useful to review all the Vertex and Matrix classes!
Vertex* tests requires some more testing, but the skeleton is done. Matrix classes are on the way, but surely they definitively need a review and a test set.
On a related subject, what do you think about adding static Translation, Rotation, etc? This would allow constructs such as:
shaderProgram.SetUniform(ctx, name, Matrix4x4.Rotation(q)*Matrix4x4.Translation(translation));
Instead of:
transform = new ModelMatrix();
transform.Translate(translation);
transform.Rotate(q);
shaderProgram.SetUniform(ctx, name, transform);
The following is the Equals method of Vertex3f:
I do not think that using an absolute Epsilon is the right approach in Equals(). This can lead to strange behaviors. Consider the following code:
I suggest that you get rid of epsilon entirely and stick to true equality (especially since the summary of the method indicates that it is a true equality).
You could add an "approximately equal" method but in that case use a relative error. Something like:
Math.Abs(X - other.X) >= Epsilon*Math.Max(Math.Abs(X), Math.Abs(other.X))
Your OpenGL binding is very good. Many thanks for your impressive work!