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

Vector math usage improvements #74

Closed
mosra opened this Issue Nov 9, 2014 · 3 comments

Comments

2 participants
@mosra
Owner

mosra commented Nov 9, 2014

Vector operations

After spending some time with GLM and GLSL, I'm considering to change the API for binary operations on vectors and other structures. Currently the functions such as dot(), cross() and angle() are implemented as static member functions, e.g.:

Vector3 a, b;
Float x = Vector3::dot(a, b);

Complex c, d;
Rad angle = Complex::angle(c, d);

My proposed alternative is to make dot(), cross(), angle() free functions in Math namespace (also to be more in line with other math functions, such as Math::abs(), Math::sin(), Math::lerp() etc.), so the above example would change to this:

Vector3 a, b;
Float x = Math::dot(a, b);

Complex c, d;
Rad angle = Math::angle(c, d);

The original approach has a few downsides:

  • the calls are more verbose than necessary (dot() could easily be free function without any ambiguity for the compiler)
  • there is redundant and/or confusing information (is it important that I'm calling Vector3::dot() and not e.g. Math::Vector<3, Float>::dot() or Color3::dot()? Is there any difference? Can I call e.g. Quaternion::dot() with the same arguments?)

But there are also upsides:

  • IDE autocompletion: writing Vector3::a| has clear intent and the autocompletion will suggest Vector3::angle() as first or second entry. From Math::a| (or even a|) the intent is not clear at all.
  • Compiler diagnostics: when the user mistakenly calls Vector3::dot() with Vector2 and Vector3, the compiler will helpfully suggest that the first type doesn't match. When dot() would be free function, the compiler has ~10 overloads to select from, overwhelming the user with error log.
  • Readability: e.g. "the operation does a dot product of two quaternions" vs. "the operation does a dot product of variables a and b, which are... quaternions?!"

Yes or no? (As always, if I make the change, the original API will stay there for some time, marked as deprecated, allowing the users to move to the new syntax when they want to)

Vector constructor helpers

Besides binary operations there are various helpers for constructing vectors, mainly to help with transformations, such as Vector*::xAxis(), Vector*::xScale() etc. Currently these are also represented as static member functions:

Quaternion transformation = Quaternion::rotation(Vector3::yAxis(), 56.0_degf);

SceneGraph::Object<SceneGraph::MatrixTransformation2D> object;
object.scale(Vector2::yScale(0.5f));

The enclosing class specifies dimensionality of the output (Vector3::yAxis() results in {0, 1, 0} while Vector2::yAxis() results in {0, 1}, so the extraction to free functions will not be trivial. My proposed solution is to make Math::xAxis() etc. return some "meta-type", which then would be recognized by Vector2/Vector3 constructor and extended to proper dimension count. The above example would then look like this:

Quaternion transformation = Quaternion::rotation(Math::yAxis(), 56.0_degf);

SceneGraph::Object<SceneGraph::MatrixTransformation2D> object;
object.scale(Math::yScale(0.5f));

This looks simpler, slightly easier to write and doesn't carry redundant or potentially confusing information. There are two issues, though:

  • The returned type will be some implementation-defined (and probably undocumented) meta-type, which might hurt in combination with auto:
auto x1 = Math::xAxis();
x1.length(); // Error: length() is not a member of Math::Implementation::AxisType
Vector2 x2 = Math::xAxis();
x2.length(); // ok
  • Calling Vector2::xAxis() has well-defined underlying type (Vector2 is typedef for Math::Vector2<Float>), but what is the underlying type of Math::xAxis()? Implicit conversions would be disallowed, like with other vector types, which might be annoying in some cases. On the other side, allowing implicit conversions might be even more annoying.
// this somehow has to work, no matter what
Vector2 a = Math::xAxis();
Vector3i x = Math::xAxis();

// error about conversion of int to float would be annoying here
Vector2 y = Math::yAxis(1);

// implicit conversion and precision loss would be annoying here
Vector3i z = Math::zScale(1.5f); 

Yes or no? If yes, keep then the original Vector2::xAxis() etc. functions (as they have well-defined no-surprise behavior), or also mark them for deprecation and removal?

@mosra mosra added the feature label Nov 9, 2014

@LB--

This comment has been minimized.

Show comment
Hide comment
@LB--

LB-- Nov 10, 2014

Contributor

Vector operations - You can also add function references that alias the real functions and thus have them in both namespaces at once.

Contributor

LB-- commented Nov 10, 2014

Vector operations - You can also add function references that alias the real functions and thus have them in both namespaces at once.

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Nov 10, 2014

Owner

@LB-- Thanks for the input. Yup, that's also possible, but I'm trying to avoid ambiguity/confusion by having exactly one way to do given operation. Currently, two functions with the same name in different namespace mean that they are doing something slightly different (for example, Math::max() vs. Vector3::max()).

Owner

mosra commented Nov 10, 2014

@LB-- Thanks for the input. Yup, that's also possible, but I'm trying to avoid ambiguity/confusion by having exactly one way to do given operation. Currently, two functions with the same name in different namespace mean that they are doing something slightly different (for example, Math::max() vs. Vector3::max()).

@mosra mosra modified the milestone: Last pre-Vulkan release Mar 10, 2015

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 25, 2015

Owner

Vector operations moved into free functions in 64bc239, the lines are generally shorter after the change, which is always a good thing.

Decided not to do anything with constructor helpers as that would lead to overly complicated code with no real benefit.

Owner

mosra commented Mar 25, 2015

Vector operations moved into free functions in 64bc239, the lines are generally shorter after the change, which is always a good thing.

Decided not to do anything with constructor helpers as that would lead to overly complicated code with no real benefit.

@mosra mosra closed this Mar 25, 2015

@mosra mosra added this to the 2015.05 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment