Multiplication operator not returning value

Question

I have a Matrix3 class which has an overloaded * operator so i should be able to go:

Matrix3 m1, m2, m3
m2.setRotateX(3.98f); //values are not important just setting a value.
m3.setRotateZ(9.62f);

m1 = m2 * m3

Expected result: m1 is assigned the result of m2 * m3 calculation. Instead it appears to be still assigned whatever was in memory when it was created.

I have looked at it in the debugger and the result code at the end of the operator * method is assigned to the correct values but it never is assigned for some reason.

I am running Visual Studio 2017 (I tried updating to latest version) and currently running with the Debug x86 profile.

I should also point out I am not using a pre-made maths library because this is part of an assignment.

Here is the full code for the class: matrix3.h

#pragma once
#include "vector3.h"

class Matrix3
{
public:
    union
    {
        struct
        {
            Vector3 xAxis;
            Vector3 yAxis;
            union {

                Vector3 zAxis;
                Vector3 translation;
            };
        };
        Vector3 axis[3];
        float data[3][3];
    };
    static const Matrix3 identity;

    Matrix3(Vector3 x, Vector3 y, Vector3 z) : xAxis{ x }, yAxis{ y }, zAxis{ z } { }
    Matrix3(float f1, float f2, float f3, float f4, float f5, float f6, float f7, float f8, float f9) : data{ f1, f2, f3, f4, f5, f6, f7, f8, f9 } {}
    Matrix3() : Matrix3{ identity } {}

    Matrix3(const Matrix3 &other)
    {
        for (int i = 0; i > 3; i++)
            axis[i] = other.axis[i];
    }




    Matrix3& operator= (const Matrix3& other);

    Vector3& operator[] (int index);
    const Vector3& operator[] (int index) const;
    Matrix3 operator * (const Matrix3& other) const;
    Vector3 operator * (const Vector3& v) const;

    Matrix3 operator + (const Matrix3& other) const;
    Matrix3 operator - (const Matrix3& other) const;
    operator float*() { return reinterpret_cast<float*>(data); }

    Matrix3 transpose() const;

    void setScaled(float x, float y, float z);
    void setScaled(const Vector3& v);
    void scale(float x, float y, float z);
    void scale(const Vector3 v);

    void setRotateX(float radians);
    void setRotateY(float radians);
    void setRotateZ(float radians);

    void rotateX(float radians);
    void rotateY(float radians);
    void rotateZ(float radians);

    void setEuler(float pitch, float yaw, float roll);


    operator std::string() const;
    friend std::ostream& operator<< (std::ostream& os, const Matrix3& matrix);
};

matrix3.cpp

#include "Matrix3.h"

const Matrix3 Matrix3::identity = Matrix3({ 1, 0, 0 }, { 0, 1, 0 }, { 0, 0, 1 });
static const int MATRIX_DIMS = 3;


Vector3& Matrix3::operator[] (int index)
{
    return axis[index];
}

const Vector3& Matrix3::operator[] (int index) const
{
    return axis[index];
}


Matrix3& Matrix3::operator=(const Matrix3& other)
{
    xAxis = other.xAxis;
    yAxis = other.yAxis;
    zAxis = other.zAxis;

    return *this;
}

Matrix3 Matrix3::operator * (const Matrix3& other) const
{
    Matrix3 result;

    for (int r = 0; r < 3; r++)
        for (int c = 0; c < 3; c++)
            result.data[c][r] = data[0][r] * other.data[c][0] +
            data[1][r] * other.data[c][1] +
            data[2][r] * other.data[c][2];

    return result;
}

Vector3 Matrix3::operator * (const Vector3& v) const
{
    Vector3 result;

    for (int r = 0; r < 3; ++r)
    {
        result[r] = 0;
        for (int i = 0; i < MATRIX_DIMS; ++i)
            result[r] += data[i][r] * v[i];
    }

    return result;
}

Matrix3 Matrix3::operator+(const Matrix3& other) const
{
    Matrix3 result;

    for (int x = 0; x < MATRIX_DIMS; x++)
        for (int y = 0; y < MATRIX_DIMS; y++)
            result[x][y] = data[x][y] + other[x][y];

    return result;
}

Matrix3 Matrix3::operator-(const Matrix3& other) const
{
    Matrix3 result;

    for (int x = 0; x < MATRIX_DIMS; x++)
        for (int y = 0; y < MATRIX_DIMS; y++)
            result[x][y] = data[x][y] - other[x][y];

    return result;
}

Matrix3 Matrix3::transpose() const
{
    Matrix3 result;

    for (int r = 0; r < MATRIX_DIMS; ++r)
        for (int c = 0; c < MATRIX_DIMS; ++c)
            result.data[r][c] = data[c][r];

    return result;
}

void Matrix3::setScaled(const Vector3& v)
{
    // set scale of each axis
    xAxis = { v.x, 0, 0 };
    yAxis = { 0, v.y, 0 };
    zAxis = { 0, 0, v.z };
}

void Matrix3::setScaled(float x, float y, float z)
{
    // set scale of each axis
    xAxis = { x, 0, 0 };
    yAxis = { 0, y, 0 };
    zAxis = { 0, 0, z };
}

void Matrix3::scale(const Vector3 v)
{
    Matrix3 m;
    m.setScaled(v.x, v.y, v.z);

    *this = *this * m;
}

void Matrix3::scale(float x, float y, float z)
{
    Matrix3 m;
    m.setScaled(x, y, z);

    *this = *this * m;
}

void Matrix3::rotateX(float radians)
{
    Matrix3 m;
    m.setRotateX(radians);

    *this = *this * m;
}

void Matrix3::rotateY(float radians)
{
    Matrix3 m;
    m.setRotateY(radians);

    *this = *this * m;
}

void Matrix3::rotateZ(float radians)
{
    Matrix3 m;
    m.setRotateZ(radians);

    *this = *this * m;
}

void Matrix3::setRotateX(float radians)
{
    xAxis = { 1, 0, 0 };
    yAxis = { 0, cosf(radians), sinf(radians) };
    zAxis = { 0, -sinf(radians), cosf(radians) };
}

void Matrix3::setRotateY(float radians)
{
    xAxis = { cosf(radians), 0, -sinf(radians) };
    yAxis = { 0, 1, 0 };
    zAxis = { sinf(radians), 0, cosf(radians) };
}

void Matrix3::setRotateZ(float radians)
{
    xAxis = { cosf(radians), sinf(radians), 0 };
    yAxis = { -sinf(radians), cosf(radians), 0 };
    zAxis = { 0, 0, 1 };
}

void Matrix3::setEuler(float pitch, float yaw, float roll)
{
    Matrix3 x, y, z;
    x.setRotateX(pitch);
    y.setRotateY(yaw);
    z.setRotateZ(roll);

    *this = z * y * x;
}


Matrix3::operator std::string() const
{
    std::string result = "";

    for (int r = 0; r < MATRIX_DIMS; ++r)
    {
        result += "{";
        for (int c = 0; c < MATRIX_DIMS; ++c)
        {
            result += std::to_string(data[r][c]);

            if (r != MATRIX_DIMS - 1 || c != MATRIX_DIMS - 1)
                result += ",";
        }
        result += "}";
    }

    return result;

}

std::ostream& operator<< (std::ostream& os, const Matrix3& matrix)
{
    os << static_cast<std::string>(matrix);
    return os;
}

You can't use `union` to map data like that. Using any `union` member that was not the last to be assigned to is undefined behavior. — François Andrieux, Apr 03 '18 at 14:37
Not sure I understand what you mean. Is it because I am using both floats and Vector3 objects or is it the use of a union inside a union? — Wil, Apr 03 '18 at 14:43
That solved the problem thank you so much, been bashing my head against a wall for the last few hours on that one :). You should post that as an answer so I can give you credit for it. — Wil, Apr 03 '18 at 14:51
This is a _lot_ of code. Please be more minimal (without sacrificing correctness or reproducibility). — Lightness Races in Orbit, Apr 03 '18 at 15:26

score 2 · Accepted Answer · answered Apr 03 '18 at 14:56

2

The supplied code exhibits undefined behavior. In c++ only up to one member of a union is active at any time. Only a union's active member can be read from. Assigning to a union member makes it active and makes all other members inactive. For example, if you assign a value to identity.data it's undefined behavior to try to read from identity.axis until you assign a value to identity.axis, after which you can no longer read from identity.data. This is different from how it works in c.

By calling void Matrix3::setRotateX(float radians) you assign values to the union's xAxis, yAxis and zAxis members, making the struct component of the union active. Then, when you multiply m2 with m3 you call Matrix3 Matrix3::operator * (const Matrix3& other) const which reads from the union's data member which is not active, which leads to undefined behavior.

The easiest solution would likely to be to dispose of the union all together and use only one of the representations.

answered Apr 03 '18 at 14:56

François Andrieux

28,148
6
56
87

_"This is different from how it works in c."_ I am not certain, but is this actually true? I thought it was UB in C too, just peeps got away with it due to mainstream compiler behaviours (and/or just everybody _thought_ it was safe due to myths and legends). – Lightness Races in Orbit Apr 03 '18 at 15:27
[Looks like](https://stackoverflow.com/questions/2310483/purpose-of-unions-in-c-and-c#comment26826326_2313676) we can call it valid in all standard versions of C as long as we accept that a DR from 2004 applies retroactively (which is officially true but something I've never really acknowledged in practice to be quite honest) – Lightness Races in Orbit Apr 03 '18 at 15:30
@LightnessRacesinOrbit At least in C11. There is [this footnote](https://port70.net/~nsz/c/c11/n1570.html#note95) in this C11 standard draft. – François Andrieux Apr 03 '18 at 15:31

Multiplication operator not returning value

1 Answers1