4
\$\begingroup\$

I have created a vector math header library where the size and data type stored are template values, and I'd like it peer-reviewed please. I have a few main questions:

Will using my SizedVectorBase struct prevent template instantiation code bloat like I intend it to?

Are the functions I defined constexpr correct to do so, and did I miss any?

How can I define const basis vectors like Vector2 Zero(0,0) or Vector3 UnitZ(0,0,1)?

Am I missing any functionality a vector math library should have?

Any feedback is appreciated, thank you.

#pragma once
#include <algorithm>
#include <type_traits>
#include <stdexcept>
#include <cmath>

// Base vector without size templated to avoid code-bloated binaries
template<typename T>
struct SizedVectorBase
{
public:
    T& operator[](std::size_t index)
    {
        if (index >= size)
        {
            throw std::out_of_range("Operator [] access out of bounds on Vector struct");
        }
        else
        {
            return pData[index];
        }
    }

    const T& operator[](std::size_t index) const
    {
        if (index >= size)
        {
            throw std::out_of_range("Operator [] access out of bounds on Vector struct");
        }
        else
        {
            return pData[index];
        }
    }

    // Component-wise vector +=
    SizedVectorBase<T>& operator+=(const SizedVectorBase<T>& rhs)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] += rhs.pData[i];
        }
        return *this;
    }

    // Component-wise vector -=
    SizedVectorBase<T>& operator-=(const SizedVectorBase<T>& rhs)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] -= rhs.pData[i];
        }
        return *this;
    }

    // Component-wise vector *=
    SizedVectorBase<T>& operator*=(const SizedVectorBase<T>& rhs)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] *= rhs.pData[i];
        }
        return *this;
    }

    // Component-wise vector /=
    SizedVectorBase<T>& operator/=(const SizedVectorBase<T>& rhs)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] /= rhs.pData[i];
        }
        return *this;
    }

    // Scalar *=
    template<typename S>
    SizedVectorBase<T>& operator*=(const S& scalar)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] *= scalar;
        }
        return *this;
    }

    // Scalar /=
    template<typename S>
    SizedVectorBase<T>& operator/=(const S& scalar)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] /= scalar;
        }
        return *this;
    }

    // Length squared of vec
    T LengthSq() const
    {
        T sum = 0;
        for (std::size_t i = 0; i < size; ++i)
        {
            sum += pData[i] * pData[i];
        }
        return sum;
    }

    // Length of vec
    T Length() const
    {
        return sqrt(LengthSq());
    }

    // Normalize this vector in place
    void Normalize()
    {
        *this /= Length();
    }

    // Dot product
    T Dot(const SizedVectorBase<T>& other) const
    {
        T sum = 0;
        for (std::size_t i = 0; i < size; ++i)
        {
            sum += pData[i] * other.pData[i];
        }
        return sum;
    }

    // Component-wise clamp values between 0 and 1 this vector in place
    void Saturate()
    {
        Clamp(0, 1);
    }

    // Component-wise clamp values between min and max this vector in place
    void Clamp(const T& min, const T& max)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] = std::max(min, std::min(pData[i], max));
        }
    }

    // Component-wise absolute value this vector in place
    void Abs()
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] = abs(pData[i]);
        }
    }

protected:
    constexpr SizedVectorBase(std::size_t n, T* pMem)
        : size(n), pData(pMem)
    {}

    constexpr void LoopedCopyOtherRaw(const T* const otherRaw)
    {
        for (std::size_t i = 0; i < size; ++i)
        {
            pData[i] = otherRaw[i];
        }
    }

private:
    std::size_t size;
    T* pData;
};

// Generic vector
template<typename T, std::size_t n>
struct Vector : public SizedVectorBase<T>
{
    T data[n];

    // Default constructor value initializes each element of data
    constexpr Vector()
        : data(), SizedVectorBase<T>(n, data)
    {}

    constexpr explicit Vector(const T& fillVal)
        : SizedVectorBase<T>(n, data)
    {
        std::fill(data, data + n, fillVal);
    }

    // enable_if has int member type iff Args has n elements, allowing substitution to succeed
    // enable_if has to have a default argument to follow a parameter pack
    template<typename... Args, typename std::enable_if<sizeof...(Args) == n, int>::type = 0>
    constexpr Vector(const Args&... args)
        : data{ args... }, SizedVectorBase<T>(n, data)
    {}

    constexpr Vector(T* rawOtherVec)
        : SizedVectorBase<T>(n, data)
    {
        LoopedCopyOtherRaw(rawOtherVec);
    }

    constexpr Vector(const Vector<T, n>& other)
        : SizedVectorBase<T>(n, data)
    {
        LoopedCopyOtherRaw(other.data);
    }

    constexpr Vector<T, n>& operator=(const Vector<T, n>& other)
    {
        LoopedCopyOtherRaw(other.data);
        return *this;
    }
};

// Vector2 template specialization
template<typename T>
struct Vector<T, 2> : public SizedVectorBase<T>
{
    union
    {
        T data[2];
        struct { T x, y; };
    };

    // Default constructor value initializes each element of data
    constexpr Vector()
        : data(), SizedVectorBase<T>(2, data)
    {}

    constexpr explicit Vector(const T& fillVal)
        : x(fillVal), y(fillVal), SizedVectorBase<T>(2, data)
    {}

    constexpr Vector(const T& inX, const T& inY)
        : x(inX), y(inY), SizedVectorBase<T>(2, data)
    {}

    constexpr Vector(T* rawOtherVec)
        : SizedVectorBase<T>(2, data)
    {
        LoopedCopyOtherRaw(rawOtherVec);
    }

    constexpr Vector(const Vector<T, 2>& other)
        : SizedVectorBase<T>(2, data)
    {
        LoopedCopyOtherRaw(other.data);
    }

    constexpr Vector<T, 2>& operator=(const Vector<T, 2>& other)
    {
        LoopedCopyOtherRaw(other.data);
        return *this;
    }
};

// Vector3 template specialization
template<typename T>
struct Vector<T, 3> : public SizedVectorBase<T>
{
    union
    {
        T data[3];
        struct { T x, y, z; };
        struct { T r, g, b; };
        Vector<T, 2> xy;
    };

    // Default constructor value initializes each element of data
    constexpr Vector()
        : data(), SizedVectorBase<T>(3, data)
    {}

    constexpr explicit Vector(const T& fillVal)
        : x(fillVal), y(fillVal), z(fillVal), SizedVectorBase<T>(3, data)
    {}

    constexpr Vector(const T& inX, const T& inY, const T& inZ)
        : x(inX), y(inY), z(inZ), SizedVectorBase<T>(3, data)
    {}

    constexpr Vector(T* rawOtherVec)
        : SizedVectorBase<T>(3, data)
    {
        LoopedCopyOtherRaw(rawOtherVec);
    }

    constexpr Vector(const Vector<T, 3>& other)
        : SizedVectorBase<T>(3, data)
    {
        LoopedCopyOtherRaw(other.data);
    }

    constexpr Vector<T, 3>& operator=(const Vector<T, 3>& other)
    {
        LoopedCopyOtherRaw(other.data);
        return *this;
    }

    Vector<T, 3> Cross(const Vector<T, 3>& other) const
    {
        return Vector<T, 3>(y * other.z - z * other.y,
            z * other.x - x * other.z,
            x * other.y - y * other.x);
    }
};

// Vector4 template specialization
template<typename T>
struct Vector<T, 4> : public SizedVectorBase<T>
{
    union
    {
        T data[4];
        struct { T x, y, z, w; };
        struct { T r, g, b, a; };
        Vector<T, 2> xy;
        Vector<T, 3> xyz;
        Vector<T, 3> rgb;
    };

    // Default constructor value initializes each element of data
    constexpr Vector()
        : data(), SizedVectorBase<T>(4, data)
    {}

    constexpr explicit Vector(const T& fillVal)
        : x(fillVal), y(fillVal), z(fillVal), w(fillVal), SizedVectorBase<T>(4, data)
    {}

    constexpr Vector(const T& inX, const T& inY, const T& inZ, const T& inW)
        : x(inX), y(inY), z(inZ), w(inW), SizedVectorBase<T>(4, data)
    {}

    constexpr Vector(const Vector<T,3>& vec3, const T& scalar)
        : x(vec3.x), y(vec3.y), z(vec3.z), w(scalar), SizedVectorBase<T>(4, data)
    {}

    constexpr Vector(T* rawOtherVec)
        : SizedVectorBase<T>(4, data)
    {
        LoopedCopyOtherRaw(rawOtherVec);
    }

    constexpr Vector(const Vector<T, 4>& other)
        : SizedVectorBase<T>(4, data)
    {
        LoopedCopyOtherRaw(other.data);
    }

    constexpr Vector<T, 4>& operator=(const Vector<T, 4>& other)
    {
        LoopedCopyOtherRaw(other.data);
        return *this;
    }
};

// Component-wise vector addition
template<typename T, std::size_t n>
Vector<T, n> operator+(const Vector<T, n>& lhs, const Vector<T, n>& rhs)
{
    Vector<T, n> temp(lhs);
    temp += rhs;
    return temp;
}

// Component-wise vector subtraction
template<typename T, std::size_t n>
Vector<T, n> operator-(const Vector<T, n>& lhs, const Vector<T, n>& rhs)
{
    Vector<T, n> temp(lhs);
    temp -= rhs;
    return temp;
}

// Component-wise vector multiplication
template<typename T, std::size_t n>
Vector<T, n> operator*(const Vector<T, n>& lhs, const Vector<T, n>& rhs)
{
    Vector<T, n> temp(lhs);
    temp *= rhs;
    return temp;
}

// Component-wise vector division
template<typename T, std::size_t n>
Vector<T, n> operator/(const Vector<T, n>& lhs, const Vector<T, n>& rhs)
{
    Vector<T, n> temp(lhs);
    temp /= rhs;
    return temp;
}

// Scalar multiplication
template<typename T, std::size_t n, typename S>
Vector<T, n> operator* (const Vector<T, n>& vec, const S& scalar)
{
    Vector<T, n> temp(vec);
    temp *= scalar;
    return temp;
}

// Scalar multiplication
template<typename T, std::size_t n, typename S>
Vector<T, n> operator* (const S& scalar, const Vector<T, n>& vec)
{
    Vector<T, n> temp(vec);
    temp *= scalar;
    return temp;
}

// Scalar division
template<typename T, std::size_t n, typename S>
Vector<T, n> operator/(const Vector<T, n>& vec, const S& scalar)
{
    Vector<T, n> temp(vec);
    vec /= scalar;
    return temp;
}

// Negate unary -
template<typename T, std::size_t n>
Vector<T, n> operator-(const Vector<T, n>& vec)
{
    Vector<T, n> temp(vec);
    temp *= -1;
    return temp;
}

// Length squared of vec
template<typename T, std::size_t n>
T LengthSq(const Vector<T, n>& vec)
{
    return vec.LengthSq();
}

// Length of vec
template<typename T, std::size_t n>
T Length(const Vector<T, n>& vec)
{
    return vec.Length();
}

// Normalize a copy of vec
template<typename T, std::size_t n>
Vector<T, n> Normalize(const Vector<T, n>& vec)
{
    Vector<T, n> temp(vec);
    temp.Normalize();
    return temp;
}

// Dot product
template<typename T, std::size_t n>
T Dot(const Vector<T, n>& lhs, const Vector<T, n>& rhs)
{
    return lhs.Dot(rhs);
}

// Cross product
template<typename T>
Vector<T, 3> Cross(const Vector<T, 3>& lhs, const Vector<T, 3>& rhs)
{
    return lhs.Cross(rhs);
}

// Lerp from a to b by f
template<typename T, std::size_t n>
Vector<T, n> Lerp(const Vector<T, n>& a, const Vector<T, n>& b, float f)
{
    return Vector<T, n>(a + f * (b - a));
}

// Component-wise clamp values between 0 and 1 a copy of vec
template<typename T, std::size_t n>
Vector<T, n> Saturate(const Vector<T, n>& vec)
{
    Vector<T, n> temp(vec);
    temp.Saturate();
    return temp;
}

// Component-wise clamp values between min and max a copy of vec
template<typename T, std::size_t n>
Vector<T, n> Clamp(const Vector<T, n>& vec, const T& min, const T& max)
{
    Vector<T, n> temp(vec);
    temp.Clamp(min, max);
    return temp;
}

// Component-wise absolute value a copy of vec
template<typename T, std::size_t n>
Vector<T, n> Abs(const Vector<T, n>& vec)
{
    Vector<T, n> temp(vec);
    temp.Abs();
    return temp;
}

// Common aliases
using float2 = Vector<float, 2>;
using float3 = Vector<float, 3>;
using float4 = Vector<float, 4>;
using int2 = Vector<int, 2>;
using int3 = Vector<int, 3>;
using int4 = Vector<int, 4>;
using double2 = Vector<double, 2>;
using double3 = Vector<double, 3>;
using double4 = Vector<double, 4>;
\$\endgroup\$
  • \$\begingroup\$ template instantiation code bloat. Myth. Please don't perpetuate this, especially since it as not been true for nearly 20 years. The compiler may generate multiple versions (one per object file of the same type), but all modern linkers will remove all but one of these versions. \$\endgroup\$ – Martin York Mar 28 '18 at 18:25
  • \$\begingroup\$ @MartinYork My bad if that's a myth. To be clear, I'm trying to optimize functions like operator += where the main behavior is a sized loop. My goal is to have one template function generated where the loop is controlled by a variable rather than have a barely different function generated for every Vector of a different size. Can the linker actually optimize that? \$\endgroup\$ – Will Mar 28 '18 at 20:45
  • \$\begingroup\$ Though work is being done on that not yet (as far as I know). But that's because for every type of T you have that loop is not the same. Accesses to the array requires a different calculation based on the size of T also you need to understand what += mens for type T so this means you need a separate implementation for each T that you use. But this is not template bloat. This is a function for every type you use and writing the assembly by hand you will would still need to write a different function for each T. \$\endgroup\$ – Martin York Mar 28 '18 at 21:41
3
\$\begingroup\$

Correctness

  • I can't prove it off the top of my head, but I'm pretty sure your union/struct trick is undefined behavior, at least if T isn't a plain type. I'll let the language experts go to the bottom of the matter.

  • the order of initialization of your data and n variables is different from the order in which they appear in the constructor argument list. I don't see how it could harm you here, but compilers warn about it and fixing the code is the best way to silence warnings.

Interface and readability

At some point, when you reach a certain number of lines of code, you might want to consider separating interface and implementation. By that I mean:

class Vector {
    Vector();
    Vector& operator+=(const Vector& other);
}

Vector& Vector::operator+=(const Vector& other) { // implementation }

It makes for an easier reading and evaluation.

Interface uncoupling

At a glance, you can't see which function is missing. I bet that the day will come you'll need some other functionality, and you'll realize that, as huge as your interface is, it is impossible to extend it from the outside. For instance, there is no convenient way to iterate over the vector: for (auto i : myvec) ... is impossible because you didn't provide iterators. You can't use <algorithm> for the same reason.

Look at the standard containers: they offer memory and access management, nothing more (std::string is an exception); the combination of containers and algorithms that don't know anything about each other is what makes the standard library so powerful.

That's why you should, in my opinion, uncouple the container part and the operation part. Once you begin to think like that, you'll realize that your compile-time-sized container already exists: std::array.

std::array

It is interesting to try and reduce code bloat. But I don't see it realistically happening. I can't imagine a program where you would use 20+ different types of vectors. 2d/3d graphics, colorimetry, etc. account for much of the usage. Anyway, building on std::array you'll already benefit from extremely optimized implementations.

On that point -optimization-, I believe that your code is quite good but could be improved a bit though. You could have constexpred many more functions. There is also a somewhat arcane technique to speed up things which is known as loop unrolling, that you can simulate on compile-time-sized containers. I'm not sure I'd recommend it, because your compiler is likely to do it better than you, but it might be interesting to try it out.

The idea is to generate a list of indexes, and then to use parameter pack expansions:

template <std::size_t... Ns>
void print(const std::array<int, 5>& array, std::index_sequence<Ns...>) {
    auto unused = { (std::cout << std::get<Ns>(array), 0)... };
}

unused is an initializer_list containing as many zeroes as there are Nss, which isn't interesting at all, but std::get<Ns>(array) has been expanded also, and printed.

Loop unrolling

You can generalize the technique. You could even write a small library around compile-time loop unrolling. For instance, every scalar operation can be factorized thanks to this compile_time_map helper function:

#include <array>
#include <iostream>

template <typename T, std::size_t N, typename Fn, std::size_t... Ns>
auto compile_time_map_impl(Fn&& fn, std::array<T, N>& array, std::index_sequence<Ns...>) {
    [[maybe_unused]] // suppress the warning
    auto _ = { (fn(std::get<Ns>(array)), 0)... };
    return array;
}

template <typename T, std::size_t N, typename Fn>
auto compile_time_map(Fn&& fn, std::array<T, N>& array) {
    return compile_time_map_impl(std::forward<Fn>(fn), array, std::make_index_sequence<N>());
}

template <typename T, std::size_t N, typename F

int main() {
    auto test = std::array<int, 5>{1,2,3,4,5};
    test = compile_time_map([](auto& n) { ++n; }, test);
    for (auto i : test) std::cout << i << ' ';
    // output: 2 3 4 5 6
 }
\$\endgroup\$
  • 1
    \$\begingroup\$ Yes the union trick is UB. Assigning to one member of a union but reading from another is UB. See n4727 Section 12.3 Unions a non-static data member is active if its name refers to an object whose lifetime has begun and has not ended. At most one of the non-static data members of an object of union type can be active at any time. This means that only one member in the union has an active lifetime. \$\endgroup\$ – Martin York Mar 28 '18 at 18:19
  • 1
    \$\begingroup\$ From Section 6.6.3 Object lifetime Paragraph 7: after the lifetime of an object has ended .... object may be used but only in limited ways. .... The program has undefined behavior if: the glvalue is used to access the object \$\endgroup\$ – Martin York Mar 28 '18 at 18:19
  • 1
    \$\begingroup\$ BUT I would add this is a very common trick in C. \$\endgroup\$ – Martin York Mar 28 '18 at 18:20
  • \$\begingroup\$ @MartinYork I was relying on this cppreference page that says "many compilers implement, as a non-standard language extension, the ability to read inactive members of a union." Is there a better way to get component access with subscript notation without sometimes having UB? \$\endgroup\$ – Will Mar 28 '18 at 20:13
  • 1
    \$\begingroup\$ @will gist.github.com/Thors-Anvil/f1e9d34be3c3cefb45c292b23f10a286 \$\endgroup\$ – Martin York Mar 28 '18 at 21:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.