3

My program raises a "vector subscript out of range" exception (EDIT: assertion) on a return statement. Well, it seems like it since it raises it exactly on that breakpoint.

Here is the function that causes it :

Matrix4 Perspective(float fov, float aspect, float near, float far) const {
    double yScale = 1.0 / tan(TO_RADIANS * fov / 2);
    double xScale = yScale / aspect;
    double depth = near - far;

    Matrix4 perspective;

    perspective[0][0] = xScale;
    perspective[1][1] = yScale;
    perspective[2][2] = (far + near) / depth;
    perspective[2][3] = 2 * far * near / depth;
    perspective[3][2] = -1;
    perspective[3][3] = 0;

    return perspective; // Raises exception here?
}

My default Matrix4 constructor is fine, it basically does this:

_matrix.resize(4);

for (unsigned int i = 0; i < 4; ++i)
    _matrix[i].resize(4);

_matrix being a std::vector<std::vector<float>> attribute. So everything is set to 0.

Finally, the piece of code that uses the result of the return statement is this:

Matrix4 camera = Perspective(70, 1, 0.2, 10);

And I have a copy constructor which looks fine aswell:

Matrix4(const Matrix4& matrix) {

    _matrix.reserve(4);

    for (unsigned int i = 0; i < 4; ++i) {
        _matrix[i].reserve(4);

        for (unsigned int j = 0; j < 4; ++j)
            _matrix[i][j] = matrix[i][j];
    }
}

(I also have an overloaded operator[] but the problem really cannot be caused by it.)

The exception assertion seems to be raised on the return perspective;, but maybe it is raised by the line of code that called the method, and the copy constructor somehow failed to copy? But in that case, Visual Studio should bring me inside the constructor since I'm using detailed step-by-step...

I'm lost at this point...

Torayl
  • 57
  • 6
  • 2
    `And I have a copy constructor which looks fine aswell:` no it doesn't. reserve doesn't do what you think it does. – UKMonkey Jan 24 '18 at 15:00
  • 1
    You need to `push_back()` after `reserve()` or even better just `resize()` it which is probably what you wanted. – Justin Randall Jan 24 '18 at 15:01
  • 4
    `_matrix = matrix._matrix;` seems enough. – Jarod42 Jan 24 '18 at 15:01
  • Actually, I think I know. It allocates enough memory to contain n elements of type T (the one of the vector) but doesn't initialize those memory emplacements. But in my case, it doesn't matter since I'm initializing them right after. I just didn't want to "waste" operations to null-initialize everything when I'm properly initializing them anyway. Where did I get something wrong? – Torayl Jan 24 '18 at 15:03
  • 1
    `it doesn't matter since I'm initializing them right after` no you're not; you're assigning to them. There might not seem like much difference but how can operator = be called on an object that doesn't exist? – UKMonkey Jan 24 '18 at 15:05
  • Alright, I see now. Thank you so much for the clarification! – Torayl Jan 24 '18 at 15:06
  • 1
    Side note - your debugger, with a breakpoint in your copy constructor should've given you everything you needed :) – UKMonkey Jan 24 '18 at 15:06
  • 1
    *vector subscript out of range" exception* -- I think that is an *assertion*, not an exception. The Visual C++ debug runtime issues a call to `assert()` to see if your indices exceed `vector::size() - 1`. If you want a true exception, throw in a few `at()` calls instead of using `[ ]` to access the elements. Then a `std::out_of_range` exception would be thrown.. – PaulMcKenzie Jan 24 '18 at 15:09
  • Your coding style is evil. Identifiers should differ by more than case. For example, you have a class `Perspective` and a variable `perspective`. This can lead to injected defects when the code is misread or typos. Also search the internet for "c++ leading underscore" – Thomas Matthews Jan 24 '18 at 15:13
  • `Perspective()` is actually a method. I appreciate the concern, though, and I see how this can cause issues. I will take a look at leading underscores. – Torayl Jan 24 '18 at 15:17
  • @PaulMcKenzie It was more of a confusion from me between both terms, I actually meant assertion. I edited my original post for it to be consistent with my issue and for future people with similar issues to find it easier. – Torayl Jan 24 '18 at 15:27

3 Answers3

4

In your copy constructor, change this:

_matrix.reserve(4);

to this:

_matrix.resize(4);

since you want to actually allocate space with vector::resize, in order for _matrix[i][j] = matrix[i][j]; to work smoothly. vector::reserve sets the vector's capacity.

Read more in vector resize VS reverse.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
3

You wrongly use reserve instead of resize.

But you can simplify your copy constructor to

Matrix4(const Matrix4& rhs) : _matrix(rhs._matrix) {}

or even, if applicable:

Matrix4(const Matrix4&) = default;
Jarod42
  • 203,559
  • 14
  • 181
  • 302
3

You have a common mistake. std::vector::reserve:

[i]ncrease[s] the capacity of the vector

, not the size, Thus you either have to use push_back after reserve, or use resize, like in your normal constructor.

Arnav Borborah
  • 11,357
  • 8
  • 43
  • 88