2

I'm attempting to use a union for a couple of structs, specfically aligning a 2D (4 by 4) float array with 4 Vector4's.

From my understanding, union looks at the sequential bytes from the first. If I have a 4x4 array and want to get a whole column, I would need to skip 16 bytes to get to the next row.

Example:

Matrix = {{a, b, c, d},
          {e, f, g, h},
          {h, i, j, k},
          {l, m, n, o};

if the data is given such that a 1D array is {a,b,c,d,e,..., o}, how would I get the last column (d,h,k,o)?

So how would I get the bytes: 12->16, 28->32, 44->48, 60->64?

the code I have that returns the last row (not what I want), is:

//matrix.h
struct Matrix4 {
    union{
        float values[4][4];
        Vector4 column[4];
    };

    //matrix methods etc
}

//main.cpp
Matrix4 position = Matrix3::translation(Vector3(1,2,3));
Vector4 column = position.column[2];
std::cout << (column) << std::endl;

The output matrix would be (don't remember how to use LaTex formatting sorry):

| 1 0 0 1 |
| 0 1 0 2 |
| 0 0 1 3 |
| 0 0 0 1 |

So the column I want is:

|1|
|2|
|3|
|1|

and the union gives:

|0|
|0|
|0|
|1|

(the last row, not column).

Would it just be easier to refactor my code to use a 1D array to get the sequential bytes or should I change how my matricies are constructed? or can I handle this with some other thing in union?

EDIT: My question is different to this other question because I already have created a matrix class and allocated memory. My question looks at the functionality of "union" and its limits, as well as using my definied struct 'Vector4' as a means of representing some data in a matrix. I'm looking into the possibility of skipping bytes to receive data that doesn't directly follow each other, not how to allocate said bytes.

timrau
  • 22,578
  • 4
  • 51
  • 64
Callum
  • 85
  • 10
  • 2
    unions only hold one of the values at any time. if you want to have 1d and 2d access to the same memory then use a `std::vector` and write `at(index)` and `at(row,col)` accessors – 463035818_is_not_an_ai Aug 22 '18 at 08:27
  • 9
    Don't use union to alias different types in C++, it's undefined behavior. – xskxzr Aug 22 '18 at 08:28
  • Possible duplicate of [C++ Matrix Class](https://stackoverflow.com/questions/2076624/c-matrix-class) – xskxzr Aug 22 '18 at 08:28
  • Can you rearrange the elements of the matrix so it becomes consistent with its reinterpretation as an array of column vectors? – harold Aug 22 '18 at 08:28
  • @user463035818 I'm trying to learn the language and I feel like using predefined vector structs/classes wouldn't help me understand what's going on, although they will more than likely be more efficient. I don't want to necessarily have 1D and 2D access in memory, I just want to get a column in the form of my Vector4, without the use of a method or function, to use later on in my project. For example, a position vector (x, y, z, 1). – Callum Aug 22 '18 at 08:37
  • @harold I think I could, but surely that would violate some laws of matricies or some conventions and would result in errors and unwanted behaviour? – Callum Aug 22 '18 at 08:39
  • @Callum not really, as long as the other code that uses it is consistent with that arrangement. Storing matrices as an array of columns isn't that weird, OpenGL does it too – harold Aug 22 '18 at 08:54
  • @xskxzr can you explain more by what you mean by 'undefined'?. The behaviour I am intending should be clear from the whole post, if not thats my bad. I intend for the union to give 4 values in the form of my Vector4 from my matrix, though it returns the final row, rather than column. I've noticed it is because a 2D array stores bytes in sequence via a row-major pattern. I want to know if it is possible to skip x amounts of bytes to get the correct values. – Callum Aug 22 '18 at 08:56
  • 4
    undefined behaviour : https://en.cppreference.com/w/cpp/language/ub union is wrong tool for this, union is used to store one of it's data members, not both at the same time and not to access data from one member using another. – KamilCuk Aug 22 '18 at 08:58
  • @harold that's what i'm going to be using it for! So I should just restructure my matrix definitions and methods then? – Callum Aug 22 '18 at 08:58
  • @Callum yes exactly, you can swap the row/col indexing in the methods, that "virtually transposes" the matrix in a way – harold Aug 22 '18 at 09:07
  • Can you give more simple and reproducible example? What does `Matrix3::translation(Vector3(1,2,3));` return? I don't get it, why don't you just `Matrix[column][3]` ? What is the definition of `Vector4`? If you have `int matrix[16];` you can use `int (*matrix2d)[4] = matrix` to access them as 2d. – KamilCuk Aug 22 '18 at 09:07
  • 2
    Using custom [iterators](https://en.wikipedia.org/wiki/Iterator) to iterate over the column is a good solution for a lot of cases. – Passer By Aug 22 '18 at 09:09
  • @KamilCuk My Matrix header has`static Matrix4 translation(const Vector3& translation);` all the method does is assign values to certain elements of the matrix and returns a Matrix4. I left out a lot of code (around 500+ lines) to try focus on the question. Vector4 is `Vector4(const float& x, const float& y, const float& z, const float& w);` – Callum Aug 22 '18 at 09:38
  • Which union member do you assign? – KamilCuk Aug 22 '18 at 09:39
  • I guess I assign the Matrix4. Not going to lie, I don't quite understand what you mean. I'm somewhat new to C++ as a whole – Callum Aug 22 '18 at 09:49
  • @Callum: The proper use of union: You have to write a program that has very tight requirements on memory efficiency. You have something, like the member of a class, that can be of different type, maybe indicated by a bool. You use a union in order not to have to either use pointers (which waste a tiny bit of memory) or two different members. What not to do with unions: Store data of one type in it and then acces it as if the other type was stored in. Aka what you ask for. Do *not* assume that data is stored in some given way in the union. That is using UB. – Aziuth Aug 22 '18 at 10:36
  • this is a [xy problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). You cannot use unions that way. What you actually want to do is possible, though its not clear exactly from the question. I guess you can find a duplicate or maybe create a new question.... – 463035818_is_not_an_ai Aug 22 '18 at 10:42

5 Answers5

3

This is a fairly common pattern, but an invalid one. It's popular because it's easy, and because mainstream compilers have traditionally permitted and supported it.

But you simply cannot, in a well-defined way, make use of unions in this manner. Unions are not a way to reinterpret data of one type as data of another type. Heck, reinterpret_cast doesn't even get you there: to do it safely you'd have to keep std::copying your bytes back and forth. The only time it's safe is with a char[] (or unsigned char[], or std::byte[]) which is allowed to alias arbitrary blocks of memory: as a consequence, unions can be useful for quick byte inspection. Beyond that, they are useful only if you don't need to use more than one of the union's members at a time.

What you should really be doing is picking one layout/format, then adding your own operator[] (or other function) variants on top to provide different interfaces and ways of interpreting the data at your call site.

Do it right and there'll not be any overhead.

Do it cleverly and you'll be able to apply your transformation "on the fly" in just the way you need. Provide a proxy type, a kind of iterator, that doesn't just iterate in linear sequence but instead iterates in the modified sequence that you need.

This logic can all be encapsulated in your Matrix4 type.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • The types `B` with the special rules for allowing the lifetime of an object of another type to overlap the lifetime of an object of type array of `B`, or allowing an lvalue-to-rvalue conversion for an expression of type `B` even if the glvalue result is not an object whose dynamic type is related to `B`, are `char`, `unsigned char`, and `std::byte`. – aschepler Aug 22 '18 at 11:55
2

You cannot access two different members of a union at the same time. The example from cppreference:

#include <string>
#include <vector>

union S
{
    std::string str;
    std::vector<int> vec;
    ~S() {} // needs to know which member is active, only possible in union-like class 
};          // the whole union occupies max(sizeof(string), sizeof(vector<int>))

int main()
{
    S s = {"Hello, world"};
    // at this point, reading from s.vec is undefined behavior
[...]

ie after assigning the str member you are not allowed to access the vec member.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
1

So hereby I decide to add a brand new answer involving none undefined behaviour I believe. Starring force type conversion:

#include <cstdlib>
#include <iostream>
#include <initializer_list>

using namespace std;

// Matrix4f dimension
#define DIM_MAT4        (4)

// Data structures
typedef struct _Vector4f
{
    float entries[DIM_MAT4];

    void
    println() const;

} Vector4f;

typedef struct _Matrix4f
{
    _Matrix4f(initializer_list<float> fl) : _trans_cache(NULL), _renew_cache(true)
    {
        copy(fl.begin(), fl.end(), entries);
    }

    _Matrix4f() : entries{.0f, .0f, .0f, .0f,
                    .0f, .0f, .0f, .0f,
                    .0f, .0f, .0f, .0f,
                    .0f, .0f, .0f, .0f}, _trans_cache(NULL), _renew_cache(true) {}

    ~_Matrix4f() { delete _trans_cache; }

    float &
    operator() (int, int);

    const float &
    operator() (int, int) const;

    const Vector4f *
    getRow(int) const;

    const Vector4f *
    getCol(int) const;

    _Matrix4f *
    transpose() const;

    void
    println() const;

private:

    float entries[DIM_MAT4 * DIM_MAT4];

    // cache the pointer to the transposed matrix
    mutable _Matrix4f *
    _trans_cache;
    mutable bool
    _renew_cache;

} Matrix4f;

Matrix4f *
Matrix4f::transpose() const
{
    if (not _renew_cache)
        return this->_trans_cache;

    Matrix4f * result = new Matrix4f;

    for (int k = 0; k < DIM_MAT4 * DIM_MAT4; k++)
    {
        int j = k % DIM_MAT4;
        int i = k / DIM_MAT4;

        result->entries[k] = this->entries[i + DIM_MAT4 * j];
    }

    _renew_cache = false;
    return this->_trans_cache = result;
}

float &
Matrix4f::operator() (int rid, int cid)
{
    _renew_cache = true;
    return this->entries[rid * DIM_MAT4 + cid];
}

const float &
Matrix4f::operator() (int rid, int cid) const
{
    return this->entries[rid * DIM_MAT4 + cid];
}

const Vector4f *
Matrix4f::getRow(int rid) const
{
    return reinterpret_cast<Vector4f *>(&(this->entries[rid * DIM_MAT4]));
}

const Vector4f *
Matrix4f::getCol(int cid) const
{
    return this->transpose()->getRow(cid);
}

void
Matrix4f::println() const
{
    this->getRow(0)->println();
    this->getRow(1)->println();
    this->getRow(2)->println();
    this->getRow(3)->println();

    cout << "Transposed: " << this->_trans_cache << endl;
}

void
Vector4f::println() const
{
    cout << '(' << (this->entries[0]) << ','
            << (this->entries[1]) << ','
            << (this->entries[2]) << ','
            << (this->entries[3]) << ')' << endl;
}

// Unit test
int main()
{
    Matrix4f mat = {1.0f, 0.0f, 0.0f, 1.0f,
                    0.0f, 1.0f, 0.0f, 2.0f,
                    0.0f, 0.0f, 1.0f, 3.0f,
                    0.0f, 0.0f, 0.0f, 1.0f};

    mat.println();

    // The column of the current matrix
    // is the row of the transposed matrix
    Vector4f * col = mat.getCol(3);

    cout << "Vector:" << endl;

    col->println();

    cout << "Matrix(2,3) = " << mat(2, 3) << endl;

    return 0;
}
Aziuth
  • 3,652
  • 3
  • 18
  • 36
KaiserKatze
  • 1,521
  • 2
  • 20
  • 30
  • Pointers are not guaranteed to be initialized with `null`, so write a constructor (For that reason, it *is* UB right now). Also, please try to write an accessor for your structure. If you do, nothing will set `_trans_cache` back to `null`, resulting in `transpose` returning the transposed matrix of the state when it was transposed for the first time. You'd need a setter method that always resets it. Maybe you realized this, but then why give out code that does not account for it? – Aziuth Aug 22 '18 at 10:49
  • @Aziuth Okay I'll come back to these matters after I have my dinner. :P – KaiserKatze Aug 22 '18 at 11:00
  • @Aziuth I'm back. Btw I didn't cleanup allocated memory in a deconstructor in the previous version as well which would lead to memory leakage. :P – KaiserKatze Aug 22 '18 at 11:18
  • @Aziuth I didn't figure it out what you mean by "_please try to write an accessor for your structure_". If you have any good idea, could you please revise my answer directly, thank you. – KaiserKatze Aug 22 '18 at 12:10
  • Like `float& Matrix4f::operator()(int row, int col)`. That is an accessor. Something one would want to have for convenience, which would not work with your design. – Aziuth Aug 22 '18 at 12:16
  • Now that you added the accessor, the problem I described earlier happens: Let's say you have a Matrix4f called matrix. Now you call transpose() on it, which creates _trans_cache of the current state. Then you call, for example, matrix(2,2) = 1;, and then transpose() again. The result of that call will be the transpose of the matrix *before* the entry at (2,2) was changed, because if(this->_trans_cache) is now true and thus no new transposed matrix is computed. Simplest solution would be to store a bool that signalizes that the transposed might not be consistent and should be recreated. – Aziuth Aug 24 '18 at 11:37
  • (If you want, I can edit your code accordingly, but since that is a major change, I won't do it without knowing that you want me to.) – Aziuth Aug 24 '18 at 11:41
  • If thread-safe is an issue, it's better to leave it to users to decide whether it's suitable to cache the transposed matrix or not I guess. – KaiserKatze Aug 24 '18 at 12:27
  • BTW. You are welcome to edit my post, and I am really eager to know how you are going to change it. :) – KaiserKatze Aug 24 '18 at 12:29
  • Not talking about making it thread safe, my example creates a bug when done sequentially. – Aziuth Aug 25 '18 at 11:56
  • 1
    Made an edit about what I talked about, and inserted const keywords where they fit. – Aziuth Aug 25 '18 at 12:05
1

Unions are only used to reserve a region of memory to different objects. But at each instant there can be only one alive object. So as a general rule if you have a union with two members a and b, if you have initialized a and a is the active member of the union, you should never try to read the value of b.

But there is an exception to this rule in the case where a and b share a common initial sequence: in this case the standard ensures the memory representation of a and b are sufficiently similar so that you can access the value of a through b.

Below an example of code that show defined behavior of access to union value through an inactive member:

    struct Vector4 {
       float values[4];
       };
    //I propose you two kinds of matrix, as is usualy done in linear algebra
    //left_matrix are efficient for matrix multiplication if they are on the
    //left side of the * symbol.
    struct left_matrix{
       Vector4 rows[4];
       //for this simple example we index using call operator:
       float& operator()(int i,int j){
           return rows[i].values[j];
           }
       };

    struct right_matrix{
       Vector4 columns[4];
       float& operator()(int i, int j){
          return columns[j].values[i];//the actual memory transposition is here.
          }
        };

    right_matrix 
    transpose_left_matrix(const left_matrix& m){
        union{
          left_matrix lm;
          right_matrix rm;
          };
        lm = m; //lm is the active member
        return rm; //but we can access rm
        }
Oliv
  • 17,610
  • 1
  • 29
  • 72
-1

The following answer is intended for G++ compiler and it is more efficient if you want to access columns of the same matrix repeatly.

#include <iostream>

using namespace std;

typedef struct _Vector4f
{
    float entries[4];
} Vector4f;

typedef struct _Matrix4f
{
    union {
        float entries[16];
        Vector4f rows[4];
    };

    // cache the pointer to the transposed matrix
    struct _Matrix4f *
    _trans_cache;

    struct _Matrix4f *
    transpose();

} Matrix4f;

Matrix4f *
Matrix4f::transpose()
{
    if (this->_trans_cache)
        return this->_trans_cache;

    Matrix4f * result = new Matrix4f;

    for (int k = 0; k < 16; k++)
    {
        int j = k % 4;
        int i = k / 4;

        result->entries[k] = this->entries[i + 4 * j];
    }

    return this->_trans_cache = result;
}

int main()
{
    Matrix4f mat = {1.0f, 0.0f, 0.0f, 1.0f,
                    0.0f, 1.0f, 0.0f, 2.0f,
                    0.0f, 0.0f, 1.0f, 3.0f,
                    0.0f, 0.0f, 0.0f, 1.0f};
    Vector4f col = mat.transpose()->rows[3];

    cout << (col.entries[0]) << ','
            << (col.entries[1]) << ','
            << (col.entries[2]) << ','
            << (col.entries[3]) << end;

    return 0;
}

At last, I want OP know that there are many libraries out there which implements many complex matrix operations, such as GNU Scientific Library. So there is hardly any reason to invent a wheel again. Hehe. :P

KaiserKatze
  • 1,521
  • 2
  • 20
  • 30
  • This code is undefined behavior in C++ / but it may work. – Oliv Aug 22 '18 at 09:30
  • Undefined behaviour is not monster as long as we know whether the code runs on certain platforms. It works with G++. – KaiserKatze Aug 22 '18 at 09:34
  • Yes I know, but until which g++ version it will work is hard to answer. – Oliv Aug 22 '18 at 09:37
  • 1
    But how is this better than just storing columns in the first place? – harold Aug 22 '18 at 10:08
  • Storing columns in the first place equals transposing the matrix when initializing it. I see no difference at all indeed. Maybe some users want to access rows of a matrix from time to time? – KaiserKatze Aug 22 '18 at 10:24
  • The key of my answer is my transposed matrix **cache** which makes accessing both rows and columns extremely fast. – KaiserKatze Aug 22 '18 at 10:26
  • 4
    _"Undefined behaviour is not monster as long as we know whether the code runs on certain platforms."_ No, no, no! This is a very dangerous misconception. _"It works with G++."_ It works with G++ **because G++ defines it**, so it isn't undefined with that compiler. Don't confuse that with "it's undefined but it still works with G++". Undefined behaviour is **always bad**. – Jonathan Wakely Aug 22 '18 at 10:33
  • 2
    @Oliv it's not hard to answer, because it's a [documented](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning) and supported feature of G++ (i.e. it's not undefined when using G++). – Jonathan Wakely Aug 22 '18 at 10:36
  • 2
    fine if G++ defines it, but still this is **not** stardard C++. If you are ok with throwing portability out of the window just to be fancy then go ahead, imho the price is too high – 463035818_is_not_an_ai Aug 22 '18 at 10:39
  • @JonathanWakely My question was about how long. Recently a patch was proposed to remove this behavior and L.Torvald has given its opinion about that: http://www.yodaiken.com/2018/06/07/torvalds-on-aliasing/. The question is how long gcc will keep this behavior?? – Oliv Aug 22 '18 at 10:40
  • Can only repeat what other people said before: Do not intentionally write code that has UB. If you tested it on a hundred computers with all different kinds of compilers, that does not matter, do not use it. You cannot guarantee that it works. And especially when there is totally not any need to use it. – Aziuth Aug 22 '18 at 10:41
  • @Oliv, that looks like a patch to stop using the feature in the kernel, not a patch to remove the feature from GCC. There have been no suggestions to remove it from GCC as far as I am aware. – Jonathan Wakely Aug 22 '18 at 10:46
  • @JonathanWakely You ain't dealing with Java. If you don't want all-platform compatiblity, it's totally okay to design architectures aimming at specific platforms. After all Windows APIs are drastically different from that of Unix/Linux. Should we call all the difference between Windows and Unix/Linux undefined behaviour? – KaiserKatze Aug 22 '18 at 10:47
  • No, because that's not what undefined behaviour means, and that's a stupid strawman. I didn't say you must write code portable to all platforms. I said that it's OK to rely on something that is **defined to be valid** by one compiler, because if it's defined then it's not undefined behaviour. A difference between Windows and Linux that is documented, supported, and defined as valid on Windows is not undefined. – Jonathan Wakely Aug 22 '18 at 10:48
  • @Oliv Standard C++ gives only a standard to design a compiler, and it doesn't force every compiler to behave as the same. So why do you force me to take care of an undefined behaviour in standard C++ if I choose to give a G++ answer. I love G++. – KaiserKatze Aug 22 '18 at 10:50
  • @JonathanWakely No to mention standard makers make stupid standards too. Let's take a look at the Holy Trinity of JavaScript. [![Holy Trinity of JavaScript](http://imgur.com/5pFXFbR)](https://www.reddit.com/r/ProgrammerHumor/comments/6tifn2/the_holy_trinity_and_javascript/) – KaiserKatze Aug 22 '18 at 10:53
  • @KaiserKatze I also only compile with gcc, the other option for me could be clang, but as I heavily use template meta programming I have fallen on so many clang bugs that I don't use it anymore. You can do what you want but just tell in the answer that this solution just work for gcc. And I did not downvoted. – Oliv Aug 22 '18 at 11:04
  • @JonathanWakely: It's not implementation-specified behaviour, it's undefined behaviour, so although you're right that we can rationalise based on documented facts of your specific compiler, and although you're right that this may be sufficient for practical purposes, it doesn't change the fact that C++ itself does not permit us to do this. I concede that this is ultimately academic unless you care about portability. – Lightness Races in Orbit Aug 22 '18 at 11:36
  • 1
    @JonathanWakely: ..... which, on reflection, appears to be what you already said ^_^ – Lightness Races in Orbit Aug 22 '18 at 11:37