Operations with some user-defined operators in my Matrix4 class are returning high values very high values

Question

i am writing a Matrix4 class for an OpenGL game:

static class Matrix4
{
private:
    float matrix[16];
    Matrix4 Identity()
    {
        matrix[1] = matrix[2] =
            matrix[3] = matrix[4] = matrix[6] =
            matrix[7] = matrix[8] = matrix[9] =
            matrix[11] = matrix[12] = matrix[13] = matrix[14] = 0.f;
        matrix[0] = matrix[5] = matrix[10] = matrix[15] = 1.f;
        return *this;
    }

public:
    Matrix4() { Identity(); };
    Matrix4(float vals[16])
    {
        matrix[0] = vals[0]; matrix[1] = vals[1]; matrix[2] = vals[2]; matrix[3] = vals[3];
        matrix[4] = vals[4]; matrix[5] = vals[5]; matrix[6] = vals[6]; matrix[7] = vals[7];
        matrix[8] = vals[8]; matrix[9] = vals[9]; matrix[10] = vals[10]; matrix[11] = vals[11];
        matrix[12] = vals[12]; matrix[13] = vals[13]; matrix[14] = vals[14]; matrix[15] = vals[15];
    }
    Matrix4(Vector4 vals[4])
    {
        matrix[0] = vals[0].GetX(); matrix[1] = vals[0].GetY(); matrix[2] = vals[0].GetZ(); matrix[3] = vals[0].GetW();
        matrix[4] = vals[1].GetX(); matrix[5] = vals[1].GetY(); matrix[6] = vals[1].GetZ(); matrix[7] = vals[1].GetW();
        matrix[8] = vals[2].GetX(); matrix[9] = vals[2].GetY(); matrix[10] = vals[2].GetZ(); matrix[11] = vals[2].GetW();
        matrix[12] = vals[3].GetX(); matrix[13] = vals[3].GetY(); matrix[14] = vals[3].GetZ(); matrix[15] = vals[3].GetW();
    }

    //OPERATOR DEFINITIONS
    //STUDY THE operator<< SHIT

    static friend std::ostream& operator<< (std::ostream& os, const Matrix4& other)
    {
        std::string final = "_____________________________________________\n| " + std::to_string(other.matrix[0]) + " " + std::to_string(other.matrix[1]) + " " + std::to_string(other.matrix[2]) + " " + std::to_string(other.matrix[3]) + " " + " |" + "\n" +
            "| " + std::to_string(other.matrix[4]) + " " + std::to_string(other.matrix[5]) + " " + std::to_string(other.matrix[6]) + " " + std::to_string(other.matrix[7]) + " " + " |" + "\n" +
            "| " + std::to_string(other.matrix[8]) + " " + std::to_string(other.matrix[9]) + " " + std::to_string(other.matrix[10]) + " " + std::to_string(other.matrix[11]) + " " + " |" + "\n" +
            "| " + std::to_string(other.matrix[12]) + " " + std::to_string(other.matrix[13]) + " " + std::to_string(other.matrix[14]) + " " + std::to_string(other.matrix[15]) + " " + " |";
        os.write(final.c_str(), final.size());
        return os;
    }

    Matrix4& operator+=(const Matrix4& other)
    {
        this->matrix[0] += other.matrix[0]; this->matrix[1] += other.matrix[1]; this->matrix[2] += other.matrix[2]; this->matrix[3] += other.matrix[3];
        this->matrix[4] += other.matrix[4]; this->matrix[5] += other.matrix[5]; this->matrix[6] += other.matrix[6]; this->matrix[7] += other.matrix[7];
        this->matrix[8] += other.matrix[8]; this->matrix[9] += other.matrix[9]; this->matrix[10] += other.matrix[10]; this->matrix[11] += other.matrix[11];
        this->matrix[12] += other.matrix[12]; this->matrix[13] += other.matrix[13]; this->matrix[14] += other.matrix[14]; this->matrix[15] += other.matrix[15];
    }

    Matrix4& operator-=(const Matrix4& other)
    {
        this->matrix[0] -= other.matrix[0]; this->matrix[1] -= other.matrix[1]; this->matrix[2] -= other.matrix[2]; this->matrix[3] -= other.matrix[3];
        this->matrix[4] -= other.matrix[4]; this->matrix[5] -= other.matrix[5]; this->matrix[6] -= other.matrix[6]; this->matrix[7] -= other.matrix[7];
        this->matrix[8] -= other.matrix[8]; this->matrix[9] -= other.matrix[9]; this->matrix[10] -= other.matrix[10]; this->matrix[11] -= other.matrix[11];
        this->matrix[12] -= other.matrix[12]; this->matrix[13] -= other.matrix[13]; this->matrix[14] -= other.matrix[14]; this->matrix[15] -= other.matrix[15];
    }

    Matrix4& operator*=(float scale)
    {
        this->matrix[0] *= scale; this->matrix[1] *= scale; this->matrix[2] *= scale; this->matrix[3] *= scale;
        this->matrix[4] *= scale; this->matrix[5] *= scale; this->matrix[6] *= scale; this->matrix[7] *= scale;
        this->matrix[8] *= scale; this->matrix[9] *= scale; this->matrix[10] *= scale; this->matrix[11] *= scale;
        this->matrix[12] *= scale; this->matrix[13] *= scale; this->matrix[14] *= scale; this->matrix[15] *= scale;
    }

    Matrix4& operator*=(Vector3& other)
    {
        this->matrix[0] *= other.GetX(); this->matrix[1] *= other.GetX(); this->matrix[2] *= other.GetX(); this->matrix[3] *= 1;
        this->matrix[4] *= other.GetY(); this->matrix[5] *= other.GetY(); this->matrix[6] *= other.GetY(); this->matrix[7] *= 1;
        this->matrix[8] *= other.GetZ(); this->matrix[9] *= other.GetZ(); this->matrix[10] *= other.GetZ(); this->matrix[11] *= 1;
        return *this;
    }

    Matrix4& operator*= (Vector4& other)
    {
        this->matrix[0] *= other.GetX(); this->matrix[1] *= other.GetX(); this->matrix[2] *= other.GetX(); this->matrix[3] *= other.GetX();
        this->matrix[4] *= other.GetY(); this->matrix[5] *= other.GetY(); this->matrix[6] *= other.GetY(); this->matrix[7] *= other.GetY();
        this->matrix[8] *= other.GetZ(); this->matrix[9] *= other.GetZ(); this->matrix[10] *= other.GetZ(); this->matrix[11] *= other.GetZ();
        this->matrix[12] *= other.GetW(); this->matrix[13] *= other.GetW(); this->matrix[14] *= other.GetW(); this->matrix[15] *= other.GetW();
        return *this;
    }

    Matrix4& operator= (Matrix4& other)
    {
        this->matrix[0] = other.matrix[0]; this->matrix[1] = other.matrix[1]; this->matrix[2] = other.matrix[2]; this->matrix[3] = other.matrix[3];
        this->matrix[4] = other.matrix[4]; this->matrix[5] = other.matrix[5]; this->matrix[6] = other.matrix[6]; this->matrix[7] = other.matrix[7];
        this->matrix[8] = other.matrix[8]; this->matrix[9] = other.matrix[9]; this->matrix[10] = other.matrix[10]; this->matrix[11] = other.matrix[11];
        this->matrix[12] = other.matrix[12]; this->matrix[13] = other.matrix[13]; this->matrix[14] = other.matrix[14]; this->matrix[15] = other.matrix[15];
        return *this;
    }


    //BUG: You need to put the matrix in a variable, otherwise you wil gett strange values
    Matrix4& operator+(const Matrix4& other)
    {
        Matrix4 tmp = *this;
        for (int i = 0; i < 16; i++)
        {
            tmp.matrix[i] += other.matrix[i];
        }
        return tmp;
    }

    Matrix4& operator-(const Matrix4& other)
    {
        Matrix4 tmp = *this;
        for (int i = 0; i < 16; i++)
        {
            tmp.matrix[i] -= other.matrix[i];
        }
        return tmp;
    }

    Matrix4& operator*(Vector3& other)
    {
        Matrix4 tmp = *this;
        tmp.matrix[0] *= other.GetX(); tmp.matrix[1] *= other.GetX(); tmp.matrix[2] *= other.GetX(); tmp.matrix[3] *= 1;
        tmp.matrix[4] *= other.GetY(); tmp.matrix[5] *= other.GetY(); tmp.matrix[6] *= other.GetY(); tmp.matrix[7] *= 1;
        tmp.matrix[8] *= other.GetZ(); tmp.matrix[9] *= other.GetZ(); tmp.matrix[10] *= other.GetZ(); tmp.matrix[11] *= 1;
        return tmp;
    }

    Matrix4& operator*(Vector4& other)
    {
        Matrix4 tmp = *this;
        tmp.matrix[0] *= other.GetX(); tmp.matrix[1] *= other.GetX(); tmp.matrix[2] *= other.GetX(); tmp.matrix[3] *= other.GetX();
        tmp.matrix[4] *= other.GetY(); tmp.matrix[5] *= other.GetY(); tmp.matrix[6] *= other.GetY(); tmp.matrix[7] *= other.GetY();
        tmp.matrix[8] *= other.GetZ(); tmp.matrix[9] *= other.GetZ(); tmp.matrix[10] *= other.GetZ(); tmp.matrix[11] *= other.GetZ();
        tmp.matrix[12] *= other.GetW(); tmp.matrix[13] *= other.GetW(); tmp.matrix[14] *= other.GetW(); tmp.matrix[15] *= other.GetW();
        return tmp;
    }

    //WIP: CREATE BINARY OPERATORS

    //Index goes from 0 to 3
    void SetCol(unsigned int index, float f1, float f2, float f3, float f4)
    {
        matrix[index * 4] = f1;
        matrix[index * 4 + 1] = f2;
        matrix[index * 4 + 2] = f3;
        matrix[index * 4 + 3] = f4;
    }

    //Index goes from 0 to 3
    void SetCol(unsigned int index, const float vals[])
    {
        matrix[index * 4] = vals[0];
        matrix[index * 4 + 1] = vals[1];
        matrix[index * 4 + 2] = vals[2];
        matrix[index * 4 + 3] = vals[3];
    }

    //Index goes from 0 to 3
    void SetCol(unsigned int index, Vector4 val)
    {
        matrix[index * 4] = val.GetX();
        matrix[index * 4 + 1] = val.GetY();
        matrix[index * 4 + 2] = val.GetZ();
        matrix[index * 4 + 3] = val.GetW();
    }

    Matrix4 MultiplyByVec3Floats (float x, float y, float z)
    {
        this->matrix[0] *= x; this->matrix[1] *= x; this->matrix[2] *= x; this->matrix[3] *= 1;
        this->matrix[4] *= y; this->matrix[5] *= y; this->matrix[6] *= y; this->matrix[7] *= 1;
        this->matrix[8] *= z; this->matrix[9] *= z; this->matrix[10] *= z; this->matrix[11] *= 1;
    }

    Matrix4 MultiplyByVec4Floats(float x, float y, float z, float w)
    {
        this->matrix[0] *= x; this->matrix[1] *= x; this->matrix[2] *= x; this->matrix[3] *= w;
        this->matrix[4] *= y; this->matrix[5] *= y; this->matrix[6] *= y; this->matrix[7] *= w;
        this->matrix[8] *= z; this->matrix[9] *= z; this->matrix[10] *= z; this->matrix[11] *= w;
        this->matrix[12] *= x; this->matrix[13] *= y; this->matrix[14] *= z; this->matrix[15] *= w;
    }
};`

With the following class, i then create some variables and thest them in my main file:

//#define TEST_VECTORS
#define TEST_MATRIX

#include <iostream>
#include <sdl/SDL.h>
#include<gl\glew.h>
#include <ostream>
#include <string>

#include "Game.h"
#include "gmath.h"



using namespace std;

int main(int argc, char* argv[])
{
#ifdef TEST_VECTORS
gmath::Vector3 vTestVector(5, 5, 5);
gmath::Vector3 vAnotherVector(1, 1, 1);
gmath::Vector3 vZeroVector = gmath::Vector3::Zero();
cout << vTestVector.Length() << endl;

cout << vTestVector + vAnotherVector << endl;

cout << vTestVector - vAnotherVector << endl;

cout << gmath::Vector3::Distance(vTestVector, vAnotherVector) << endl;

cout << vTestVector.Normalized() << endl;

cout << (vTestVector == vTestVector) << endl;

cout << gmath::Vector3::Dot(vTestVector, vAnotherVector) << endl;

cout << gmath::Vector3::Cross(vTestVector, vAnotherVector) << endl;
#endif

#ifdef TEST_MATRIX
gmath::Matrix4 mat;
float vals[] = { 1.f, 2.f, 3.f, 4.f };
mat.SetCol(1, vals);

gmath::Vector4 testVec(1.f, 4.f, 2.f, 1.f);
mat.SetCol(3, testVec);
mat.SetCol(0, 1.f, 5.f, 35.f, 6.f);

gmath::Matrix4 another;
another = mat;
std::cout << mat << std::endl;

std::cout << another << std::endl;

gmath::Matrix4 sum = another + mat;

cout << sum << std::endl;
gmath::Matrix4 prod = sum*testVec;
cout << prod << endl;
mat = gmath::Matrix4();
cout << mat << endl;

#endif
Game* g = new Game("MyGLTest", 800, 600, SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED);
while (g->isGameRunning())
{
    g->Update();
    g->HandleInput();
}

return 0;
}

The problem is that, if i try the following operation:

cout << another + mat << std::endl;

i get the -107374176.000000 number as output, repeated 16 times, while storing the value in a variable:

sum = another + mat;

works fine. What could be the problem?

When you see this kind of output, most likely the problem lies in out of bounds access or using dangling pointers/references. Make sure (with a debugger) that you are not accessing out of bound arrays. — vsoftco, May 09 '15 at 22:48
As i said, when storing the value into a variable and the outputting it, everything works fine, so i don't think that it could be a out of bounds access. — Giovanni Solimeno, May 09 '15 at 22:50

score 1 · Accepted Answer · edited May 23 '17 at 12:30

1

Your overloaded operators are declared as returning references, however some of them are missing the return *this; statement (operator+=, operator-=, operator*=). Also, in the operators that return *this, you are doing it wrong. You make a copy to *this then return the copy. Bam, dangling reference. Example from your code:

Matrix4& operator-(const Matrix4& other)
{
    Matrix4 tmp = *this; // this is a local object, cease to exist at function exit
    for (int i = 0; i < 16; i++)
    {
        tmp.matrix[i] -= other.matrix[i];
    }
    return tmp; // here you're done, return a reference to a local object
}

So you should really return by value here. Moreover, binary operators are best implemented as friends. See Operator overloading for an excellent guide.

edited May 23 '17 at 12:30

Community

1
1

answered May 09 '15 at 22:52

vsoftco

55,410
12
139
252

Damn, i just noticed that and fixed it. Thanks. Anyway, that didn't solve the error. – Giovanni Solimeno May 09 '15 at 22:55
@GiovanniSolimeno the issue is with returning references to local objects, see the edit. – vsoftco May 09 '15 at 22:56
Haven't seen it before, sorry. – Giovanni Solimeno May 09 '15 at 22:58
@GiovanniSolimeno also, try to compile with all warnings (`-Wall -Wextra` for g++/clang++). You will get warnings for those kind of issues. – vsoftco May 09 '15 at 23:00
Okay, i tried doing this: `friend Matrix4& operator+(const Matrix4& a, const Matrix4& b) { Matrix4 tmp = a; for (int i = 0; i < 16; i++) { tmp.matrix[i] = a.matrix[i] + b.matrix[i]; } return tmp; }` I am not sure if i did it correctly, in fact it's not working. P.S: I am using Visual Studio 2013 Community – Giovanni Solimeno May 09 '15 at 23:53
just return `Matrix4`, not `Matrix4&`, i.e. declare `friend Matrix4 operator+(...)` – vsoftco May 10 '15 at 01:01

Operations with some user-defined operators in my Matrix4 class are returning high values very high values

1 Answers1