2

I'm trying to make a class of a 4*4 matrix, it is built from an array of 16 floats, but I also want to represent it as an array of 4 vec4's (for each column). The problem is it doesn't compile, and gives an error wherever I call mat4's constructor.
My mat4.h:

struct mat4 {

    union
    {
        float elements[4 * 4]; // column major ordering, index = row + col * 4
        vec4 columns[4];
    };

    mat4();
    mat4(float diagonal);

    mat4& mul(const mat4& other); //TODO: maybe return mat4
   // vec4 getColumn(int colIndex);
    static mat4 identity(); // construct and return an identity matrix
    static mat4 orthographic(float left, float right, float bottom, float top, float near, float far); // boundaries (clipping planes)
    static mat4 perspective(float fov, float aspectRatio, float near, float far);
    static mat4 translation(const vec3& translation);
    static mat4 scale(const vec3 &scale);
    static mat4 rotation(float angle, const vec3 & axis);
    friend mat4 operator*(mat4 left, const mat4 & right);
    mat4& operator*=(const mat4 &other); //TODO: check that it fits with the vectors
    friend std::ostream &operator<<(std::ostream &stream, const mat4 &m);
};

My mat4.cpp:

#include "mat4.h"
        mat4::mat4() {
            for (int i = 0; i < 4 * 4; i++)
                elements[i] = 0;
        }
        mat4::mat4(float diagonal) {
            for (int i = 0; i < 4 * 4; ++i) {
                elements[i] = 0;
            }
            for(int i = 0; i < 4; i += 1)
                elements[i + i * 4] = diagonal;
        }
        mat4& mat4::mul(const mat4 &other) {
            for (int i = 0; i < 4; ++i)        // col
                for (int j = 0; j < 4; ++j) {  // row
                    float sum = 0;
                    for (int k = 0; k < 4; ++k)
                        sum += elements[j + k * 4] * other.elements[k + i * 4];
                    elements[j + i * 4] = sum;
                }
            return *this;
        }

        /*vec4 mat4::getColumn(int colIndex) {
            //colIndex *= 4; // TODO: profile and see if it's the same as (colIndex * 4) in each cell
            return vec4(elements[0 + colIndex * 4], elements[1 + colIndex * 4], elements[2 + colIndex * 4], elements[3 + colIndex * 4]);
        }*/
        mat4 mat4::identity() {
            return mat4(1.0f);
        }

        mat4 operator*(mat4 left, const mat4 &right) {
            return left.mul(right);
        }

        mat4 &mat4::operator*=(const mat4 &other) {
            return mul(other);
        }

        mat4 mat4::orthographic(float left, float right, float bottom, float top, float near, float far) {
            mat4 result(1);
            result.elements[0 + 0 * 4] =  2.0f / (right - left);
            result.elements[1 + 1 * 4] =  2.0f / (top - bottom);
            result.elements[2 + 2 * 4] = -2.0f / (far - near);
            result.elements[0 + 3 * 4] = (left + right) / (left - right);
            result.elements[1 + 3 * 4] = (bottom + top) / (bottom - top);
            result.elements[2 + 3 * 4] = (far + near)   / (far - near);
            //result.elements[3 + 3 * 4] = 1; this is achieved by mat result(1);
            return result;
        }

        mat4 mat4::perspective(float fov, float aspectRatio, float near, float far) {
            mat4 result;

            float q = 1.0f / tanf(toRadians(fov) / 2.0f);
            result.elements[0 + 0 * 4] = q / aspectRatio;
            result.elements[1 + 1 * 4] = q;
            result.elements[2 + 2 * 4] = (near + far) / (near - far);   // -(far + near)  / (far - near);
            result.elements[3 + 2 * 4] = -1;
            result.elements[2 + 3 * 4] = 2 * far * near / (near - far); // -2 * far * near / (far - near);

            return result;
        }

        mat4 mat4::translation(const vec3 &translation) {
            mat4 result(1.0f); // identity matrix
            result.elements[0 + 3 * 4] = translation.x;     // create a matrix as follows: 1 0 0 x
            result.elements[1 + 3 * 4] = translation.y;     //                             0 1 0 y
            result.elements[2 + 3 * 4] = translation.z;     //                             0 0 1 z
            return result;                                  //                             0 0 0 1
        }

        mat4 mat4::scale(const vec3 &scale) {
            mat4 result(1.0f);
            result.elements[0 + 0 * 4] = scale.x;     // create a matrix as follows: x 0 0 0
            result.elements[1 + 1 * 4] = scale.y;     //                             0 y 0 0
            result.elements[2 + 2 * 4] = scale.z;     //                             0 0 z 0
            return result;                            //                             0 0 0 1
        }

        mat4 mat4::rotation(float angle, const vec3 &axis) {
            mat4 result(1.0f);
            float r = toRadians(angle);
            float s = sinf(r);
            float c = cosf(r);
            result.elements[0 + 0 * 4] = axis.x * (1 - c) + c;
            result.elements[1 + 0 * 4] = axis.x * axis.y * (1 - c) + axis.z * s;
            result.elements[2 + 0 * 4] = axis.x * axis.z * (1 - c) - axis.y * s;

            result.elements[0 + 1 * 4] = axis.y * axis.x * (1 - c) - axis.z * s;
            result.elements[1 + 1 * 4] = axis.y * (1 - c) + c;
            result.elements[2 + 1 * 4] = axis.y * axis.z * (1 - c) + axis.x * s;

            result.elements[0 + 2 * 4] = axis.z * axis.x * (1 - c) + axis.y * s;
            result.elements[1 + 2 * 4] = axis.z * axis.y * (1 - c) - axis.x * s;
            result.elements[2 + 2 * 4] = axis.z * (1 - c) + c;

            return result;
        }

        std::ostream &operator<<(std::ostream &stream, const mat4 &m) {
            stream << "mat4: ( ";
            for (int i = 0; i < 4; ++i) {
                for (int j = 0; j < 4; ++j) {
                    stream << m.elements[i + j * 4] << " ";
                }
                if(i < 3) stream << std::endl << "        ";
                else stream << ")";
            }
            return stream;
        }

My vec4.h:

#include <iostream>

        struct vec4 {
            float w, x, y, z;

            vec4() = default; // declare a default constructor as a no-parameter constructor (given that I have another one)

            vec4(float w, float x, float y, float z);

            vec4(const vec4 &v);

            vec4 add(const vec4 &other); 
            vec4 add(float w, float x, float y, float z); 
            vec4 sub(const vec4 &other); 
            vec4 sub(float w, float x, float y, float z); 
            vec4 mul(const vec4 &other); 
            vec4 mul(float w, float x, float y, float z); 
            vec4 div(const vec4 &other); 
            vec4 div(float w, float x, float y, float z); 

            friend vec4 operator+(vec4 left, const vec4 &right);

            friend vec4 operator-(vec4 left, const vec4 &right);

            friend vec4 operator*(vec4 left, const vec4 &right);

            friend vec4 operator/(vec4 left, const vec4 &right);

            vec4 operator+=(const vec4 &other);

            vec4 operator-=(const vec4 &other);

            vec4 operator*=(const vec4 &other);

            vec4 operator/=(const vec4 &other);

            bool operator==(const vec4 &other);

            bool operator!=(const vec4 &other);

            friend std::ostream &operator<<(std::ostream &stream, const vec4 &vector);
        };

My vec4.cpp:

/*     vec4::vec4() {
            w = 0;
            x = 0;
            y = 0;
            z = 0;
        }
*/
        vec4::vec4(float w, float x, float y, float z) {
            this->w = w;
            this->x = x;
            this->y = y;
            this->z = z;
        }

        vec4::vec4(const vec4 &v) {
            this->w = v.w;
            this->x = v.x;
            this->y = v.y;
            this->z = v.z;
        }

        vec4 vec4::add(const vec4 &other) {
            this->w += other.w;
            this->x += other.x;
            this->y += other.y;
            this->z += other.z;
            return *this;
        }

        vec4 vec4::add(float w, float x, float y, float z) {
            this->w += w;
            this->x += x;
            this->y += y;
            this->z += z;
            return *this;
        }

        vec4 vec4::sub(const vec4 &other) {
            this->w -= other.w;
            this->x -= other.x;
            this->y -= other.y;
            this->z -= other.z;
            return *this;
        }

        vec4 vec4::sub(float w, float x, float y, float z) {
            this->w -= w;
            this->x -= x;
            this->y -= y;
            this->z -= z;
            return *this;
        }

        vec4 vec4::mul(const vec4 &other) {
            this->w *= other.w;
            this->x *= other.x;
            this->y *= other.y;
            this->z *= other.z;
            return *this;
        }

        vec4 vec4::mul(float w, float x, float y, float z) {
            this->w *= w;
            this->x *= x;
            this->y *= y;
            this->z *= z;
            return *this;
        }

        vec4 vec4::div(const vec4 &other) {
            this->w /= other.w;
            this->x /= other.x;
            this->y /= other.y;
            this->z /= other.z;
            return *this;
        }

        vec4 vec4::div(float w, float x, float y, float z) {
            this->w /= w;
            this->x /= x;
            this->y /= y;
            this->z /= z;
            return *this;
        }

        std::ostream &operator<<(std::ostream &stream, const vec4 &vector) {
            stream << "vec4: (" << vector.w << ", " << vector.x << ", " << vector.y << ", " << vector.z << ")";
            return stream;
        }

        vec4 operator+(vec4 left, const vec4 &right) {
            return left.add(right);
        }

        vec4 operator-(vec4 left, const vec4 &right) {
            return left.sub(right);
        }

        vec4 operator*(vec4 left, const vec4 &right) {
            return left.mul(right);
        }

        vec4 operator/(vec4 left, const vec4 &right) {
            return left.div(right);
        }

        vec4 vec4::operator+=(const vec4 &other) {
            return add(other);
        }

        vec4 vec4::operator-=(const vec4 &other) {
            return sub(other);

        }

        vec4 vec4::operator*=(const vec4 &other) {
            return mul(other);

        }

        vec4 vec4::operator/=(const vec4 &other) {
            return div(other);

        }

        bool vec4::operator==(const vec4 &other) {
            return (this->w == other.w && this->x == other.x && this->y == other.y && this->z == other.z);
        }

        bool vec4::operator!=(const vec4 &other) {
            return !(*this == other);
        }

And the error log:

.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::identity()’:
.../src/math/mat4.cpp:36:29: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return mat4(1.0f);
                             ^
In file included from .../src/math/mat4.cpp:5:0:
.../src/math/mat4.h:11:16: note: ‘engine::math::mat4::mat4(engine::math::mat4&&)’ is implicitly deleted because the default definition would be ill-formed:
         struct mat4 {
                ^
.../src/math/mat4.h:16:31: error: union member ‘engine::math::mat4::<anonymous union>::columns’ with non-trivial ‘engine::math::vec4::vec4(const engine::math::vec4&)’
                 vec4 columns[4];
                               ^
.../src/math/mat4.cpp: In function ‘engine::math::mat4 engine::math::operator*(engine::math::mat4, const engine::math::mat4&)’:
.../src/math/mat4.cpp:40:34: error: use of deleted function ‘engine::math::mat4::mat4(const engine::math::mat4&)’
             return left.mul(right);
                                  ^
In file included from .../src/math/mat4.cpp:5:0:
.../src/math/mat4.h:11:16: note: ‘engine::math::mat4::mat4(const engine::math::mat4&)’ is implicitly deleted because the default definition would be ill-formed:
         struct mat4 {
                ^
.../src/math/mat4.h:16:31: error: union member ‘engine::math::mat4::<anonymous union>::columns’ with non-trivial ‘engine::math::vec4::vec4(const engine::math::vec4&)’
                 vec4 columns[4];
                               ^
.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::orthographic(float, float, float, float, float, float)’:
.../src/math/mat4.cpp:56:20: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return result;
                    ^
.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::perspective(float, float, float, float)’:
.../src/math/mat4.cpp:69:20: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return result;
                    ^
.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::translation(const engine::math::vec3&)’:
.../src/math/mat4.cpp:77:20: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return result;                                  //                             0 0 0 1
                    ^
.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::scale(const engine::math::vec3&)’:
.../src/math/mat4.cpp:85:20: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return result;                            //                             0 0 0 1
                    ^
.../src/math/mat4.cpp: In static member function ‘static engine::math::mat4 engine::math::mat4::rotation(float, const engine::math::vec3&)’:
.../src/math/mat4.cpp:105:20: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
             return result;
                    ^
CMakeFiles/GameEngine.dir/build.make:169: recipe for target 'CMakeFiles/GameEngine.dir/src/math/mat4.cpp.o' failed
make[3]: *** [CMakeFiles/GameEngine.dir/src/math/mat4.cpp.o] Error 1
make[3]: *** Waiting for unfinished jobs....
.../main.cpp: In function ‘int main(int, char**)’:
.../main.cpp:19:50: error: use of deleted function ‘engine::math::mat4::mat4(engine::math::mat4&&)’
     mat4 position = mat4::translation(vec3(2,3,4));
                                                  ^
In file included from .../src/math/math.h:8:0,
                 from .../main.cpp:4:
.../src/math/mat4.h:11:16: note: ‘engine::math::mat4::mat4(engine::math::mat4&&)’ is implicitly deleted because the default definition would be ill-formed:
         struct mat4 {
                ^
.../src/math/mat4.h:16:31: error: union member ‘engine::math::mat4::<anonymous union>::columns’ with non-trivial ‘engine::math::vec4::vec4(const engine::math::vec4&)’
                 vec4 columns[4];
                               ^

I assume most of the code is irrelevant so don't bother reading all of it. Just for comparison's sake, if I remove the link vec4 columns[4]; from the union at mat4.h, then everything is fantastic.

I've been breaking my head at this for the past hour so I would really like some help.

Thanks.

EDIT:

After trying what @0x499602D2 suggested and also adding mat4(mat4&&) = default; to mat4.h, I only have one error remaining:

.../src/math/mat4.cpp: In function ‘engine::math::mat4 engine::math::operator*(engine::math::mat4, const engine::math::mat4&)’:
.../src/math/mat4.cpp:39:34: error: use of deleted function ‘engine::math::mat4::mat4(const engine::math::mat4&)’
             return left.mul(right);
                                  ^
In file included from .../src/math/mat4.cpp:5:0:
.../src/math/mat4.h:11:16: note: ‘engine::math::mat4::mat4(const engine::math::mat4&)’ is implicitly declared as deleted because ‘engine::math::mat4’ declares a move constructor or move assignment operator
         struct mat4 {
                ^

What can I do to fix this?

shoham
  • 792
  • 2
  • 12
  • 30

2 Answers2

3

vec4 declares a copy-constuctor, so there is no implicit move-constructor generated for its class. Since you have an array of vec4 inside your union, its move-constructor is deleted since vec4's cannot be moved-from. Moreover, since vec4 has a user-provided copy-constructor, it is considered non-trivial and thus the union's copy-constructor is deleted too. Since the union is a member of mat4 (and the union has a deleted copy and move-constructor) mat4's copy and move-constructor is deleted too.

Deleted functions still play a role in overload resolution so the deleted move-constructor is chosen since you are initializing your return value from a temporary. To fix this, declare a default move-constructor inside vec4:

struct vec4 {
    // ...
    vec4(vec4&&) = default;
    // ...
};

and a copy-constructor in mat4:

mat4(mat4 const&) {}

If you have any members that need to be copied you have to do it manually AFAIK.

David G
  • 94,763
  • 41
  • 167
  • 253
  • Sorry for the newbie question, what is the difference between a copy-constructor and a move-constructor? I'm coming from a Java background and have no idea what a move constructor is... – shoham May 12 '15 at 18:31
  • @shoham The solution to your problem is to declare a move-constructor in `vec4`, see my edit. – David G May 12 '15 at 18:34
  • I did that now, it doesn't seem to help, but if I also do the same for mat4: `mat4(mat4&&) = default;` I only have one error remaining, I'll edit my answer so it'll be more comfortable to see. – shoham May 12 '15 at 18:38
  • @shoham Sorry for the changes. Your copy-constructor needs to be `mat4(mat4 const&) {}`. If you have any members that need to be copied you have to do it manually. – David G May 12 '15 at 18:55
  • Alright, now this works, but could you elaborate on all this defaulting? Why do I need to declare a new *empty* copy-constructor? What if for some reason I wanted to make a move-constructor? What is the logic behind your advice? Is that because I have a copy constructor in vec4? If so, why an empty one in mat4, or can I put stuff in it? – shoham May 12 '15 at 18:58
  • @shoham As a matter of fact, you don't need a move-constructor in `vec4`, get rid of it. All you need is the copy-constructor in `mat4`. The reason it is empty instead of `= default` is because the defaulted method would try to copy the union. The union can't be copied because it's copy-constructor is deleted because `vec4` declares its own copy-constructor (that's just the rules for unions). If you need to, you can use `mat4`'s copy-constructor to copy your members (but don't copy the union since it can't be copied!). To learn about move-constructors, see the link in my comment. – David G May 12 '15 at 19:11
  • That makes a lot more sense now, still pretty complex though. Thank you very much. – shoham May 12 '15 at 19:19
  • @shoham Don't worry about the details. Try using Serge's answer if it's easier to understand. :) – David G May 12 '15 at 19:23
0

0x499602D2's answer explains your error and answers the specific question you asked. But I think it is at least a strange design to store the matrix data in two possible ways. And your implementation is incoherent with it since in mat4& mat4::mul(const mat4 &other) (and other methods) you only use the elements part of the union.

IMHO you should considere to :

  • replace the union with a simple float elements[4 * 4];
  • add a constructor taking 4 vec4 parameters and storing values into the elements array
  • add a const method to extract the vectors from the matrix

Because it is much more common to define one single internal implementation and as many external representations as you want.

It does not directly answers your question (@0x499602D2 already did), but if you follow this advice, the problem will vanish, so this answer ...

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252