1

I have a base class that contains an unassigned pointer _vertices, and I want to populate it from the derived class constructor:

class GameRenderable {  
    bool _indexed;
    int _vertSize;
    int _idxSize;

    unsigned int VAO, VBO, EBO;
    unsigned int _texture0;

protected:
    float *_vertices;
    unsigned int *_indices;

public:
    GameRenderable(GameRenderableMode grm);
    ~GameRenderable();

    void Render();

protected:
    void SetupBuffer(int, int);
};

void GameRenderable::SetupBuffer(int vs, int is) {
    _indexed = false;

    glGenVertexArrays(1, &VAO);
    glGenBuffers(1, &VBO);

    glBindVertexArray(VAO);
    glBindBuffer(GL_ARRAY_BUFFER, VBO);
    glBufferData(GL_ARRAY_BUFFER, vs, _vertices, GL_STATIC_DRAW);

    // position attribute
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 5 * sizeof(float), (void*)0);
    glEnableVertexAttribArray(0);
    // texture coord attribute
    glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 5 * sizeof(float), (void*)(3 * sizeof(float)));
    glEnableVertexAttribArray(1);

    _idxSize = is;
    _vertSize = vs;
}

void GameRenderable::Render() {
    glActiveTexture(GL_TEXTURE0);
    glBindTexture(GL_TEXTURE_2D, _texture0);

    glBindVertexArray(VAO);

    glm::mat4 model = glm::mat4(1.0f);
    GetShader()->setMat4("model", model);

    glDrawArrays(GL_TRIANGLES, 0, _vertSize);
}

The derived classes:

class Cube : public GameRenderable {
public:
    Cube();
    ~Cube();
};

Cube::Cube():GameRenderable(GameRenderableMode::_3D) {
    /* Cube ASCII
          E--------F    A   -0.5f,  0.5f, -0.5f
         /|       /|    B    0.5f,  0.5f, -0.5f
        A--------B |    C   -0.5f, -0.5f, -0.5f
        | |      | |    D    0.5f, -0.5f, -0.5f
        | G------|-H    E   -0.5f,  0.5f,  0.5f
        |/       |/     F    0.5f,  0.5f,  0.5f
        C--------D      G   -0.5f, -0.5f,  0.5f
                        H    0.5f, -0.5f,  0.5f
    */
    float vertices[] = {
        -0.5f, -0.5f, -0.5f,  0.0f, 0.0f,
         0.5f, -0.5f, -0.5f,  1.0f, 0.0f,
         0.5f,  0.5f, -0.5f,  1.0f, 1.0f,
         0.5f,  0.5f, -0.5f,  1.0f, 1.0f,
        -0.5f,  0.5f, -0.5f,  0.0f, 1.0f,
        -0.5f, -0.5f, -0.5f,  0.0f, 0.0f,

        -0.5f, -0.5f,  0.5f,  0.0f, 0.0f,
         0.5f, -0.5f,  0.5f,  1.0f, 0.0f,
         0.5f,  0.5f,  0.5f,  1.0f, 1.0f,
         0.5f,  0.5f,  0.5f,  1.0f, 1.0f,
        -0.5f,  0.5f,  0.5f,  0.0f, 1.0f,
        -0.5f, -0.5f,  0.5f,  0.0f, 0.0f,

        -0.5f,  0.5f,  0.5f,  1.0f, 0.0f,
        -0.5f,  0.5f, -0.5f,  1.0f, 1.0f,
        -0.5f, -0.5f, -0.5f,  0.0f, 1.0f,
        -0.5f, -0.5f, -0.5f,  0.0f, 1.0f,
        -0.5f, -0.5f,  0.5f,  0.0f, 0.0f,
        -0.5f,  0.5f,  0.5f,  1.0f, 0.0f,

         0.5f,  0.5f,  0.5f,  1.0f, 0.0f,
         0.5f,  0.5f, -0.5f,  1.0f, 1.0f,
         0.5f, -0.5f, -0.5f,  0.0f, 1.0f,
         0.5f, -0.5f, -0.5f,  0.0f, 1.0f,
         0.5f, -0.5f,  0.5f,  0.0f, 0.0f,
         0.5f,  0.5f,  0.5f,  1.0f, 0.0f,

        -0.5f, -0.5f, -0.5f,  0.0f, 1.0f,
         0.5f, -0.5f, -0.5f,  1.0f, 1.0f,
         0.5f, -0.5f,  0.5f,  1.0f, 0.0f,
         0.5f, -0.5f,  0.5f,  1.0f, 0.0f,
        -0.5f, -0.5f,  0.5f,  0.0f, 0.0f,
        -0.5f, -0.5f, -0.5f,  0.0f, 1.0f,

        -0.5f,  0.5f, -0.5f,  0.0f, 1.0f,
         0.5f,  0.5f, -0.5f,  1.0f, 1.0f,
         0.5f,  0.5f,  0.5f,  1.0f, 0.0f,
         0.5f,  0.5f,  0.5f,  1.0f, 0.0f,
        -0.5f,  0.5f,  0.5f,  0.0f, 0.0f,
        -0.5f,  0.5f, -0.5f,  0.0f, 1.0f
    };

    _vertices = new float[sizeof(vertices)];
    memcpy_s(_vertices, sizeof(vertices), vertices, sizeof(float));

    SetupBuffer(36, 0);
    SetupTexture("container.jpg");
}

Cube::~Cube() {
}

When I call SetupBuffer() the array passed to openGL is always only 1 element, and nothing is drawn. If I put the code of SetupBuffer() directly inside the constructor, it works.

SsJVasto
  • 486
  • 2
  • 13
  • 2
    You probably are looking for `std::vector` instead, and do you mean to assign only the first element in `Deriv1`'s constructor? – Tas Sep 02 '18 at 21:58
  • 3
    For "inheritance", try `class Deriv1 : public Base {...}`. More importantly, you need to decide if you even *WANT* inheritance in this particular scenario - I don't see any obvious "use case" in the example you've provided, because I don't see anyplace where "Derive1" or "Derive2" actually do anything different from "Base". There's also the separate question "why use an array instead of a vector" :( – paulsm4 Sep 02 '18 at 22:00
  • 1
    This is not your actual code. – Timbo Sep 02 '18 at 22:00
  • No, I tried that, but wouldn't it be a waste of resources to convert the vector to an array when I need to access it? (The function I need to pass the array to only consumes arrays, not vectors) – SsJVasto Sep 02 '18 at 22:00
  • Ok, I'll post my actual code – SsJVasto Sep 02 '18 at 22:01
  • 1
    The code in the base class is mostly okay. It should initialize `_arr` to `nullptr`, to ensure that the delete in the destructor is okay. The problem you’re seeing is somewhere else. – Pete Becker Sep 02 '18 at 22:05
  • @paulsm4 I edited to include the actual code, it whould make more sense now, but is a longer question... – SsJVasto Sep 02 '18 at 22:13
  • 3
    If you want your data to have a size, use a vector, not an array. `sizeof(array)` is the size of the pointer, not the array itself. – Retired Ninja Sep 02 '18 at 22:14
  • This is not the exact same situation, but it is the exact same issue. https://stackoverflow.com/questions/1328223/when-a-function-has-a-specific-size-array-parameter-why-is-it-replaced-with-a-p – Retired Ninja Sep 02 '18 at 22:15
  • @RetiredNinja If I use a `std::vector`, I can't pass it as a parameter to `glBufferData()`, wouldn't it be costly to build a `std::vector` only to convert it to an array to send to the GPU? – SsJVasto Sep 02 '18 at 22:16
  • @RetiredNinja I see it's similar, but I'm not passing it as a reference, the pointer is a member of the base class, and my derived class populates it... – SsJVasto Sep 02 '18 at 22:19
  • 4
    Note that you can very easily convert a `std::vector` to an array using `v.data()` so you won't have an issue there. It costs nothing. – Tas Sep 02 '18 at 22:25
  • 1
    "wouldn't it be costly to build a std::vector only to convert it to an array" - Nope. From cppreference.com, "The elements are stored contiguously, which means that elements can be accessed not only through iterators, but also using offsets to regular pointers to elements. This means that a pointer to an element of a vector _may be passed to any function_ that expects a pointer to an element of an array." The vector contains the data and which can be accessed like any array. No converting required. – 2785528 Sep 02 '18 at 22:39
  • 1
    The 2nd parameter of [`glBufferData`](https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glBufferData.xhtml) has to be the size of the buffer in **bytes** and not the number of vertex attributes -> `SetupBuffer(36 * 5 * sizeof(float), 0);` or `SetupBuffer(sizeof(vertices), 0);` or `SetupBuffer(_vertices.size() * sizeof(*_vertices.data()), 0);` – Rabbid76 Sep 03 '18 at 06:35

1 Answers1

1

GL Buffer Issues

The sizeof( type ) operator returns the size in bytes, so for example the code:

std::cout << sizeof(float) << std::endl;

will print the output: 4

so on the following line:

_vertices = new float[sizeof(vertices)];

you are allocating 4 times the amount of data that you need to store the array vertices. It should be something like:

_vertices = new float[sizeof(vertices) / sizeof(float)];
//this is more ugly but could also be:
_vertices = (float*)(new char[sizeof(vertices)]);
//that works since an ascii 'char' is 1 byte

the function memcpy_s from MSDN:

Parameters

dest New buffer.

destSize Size of the destination buffer, in bytes for memcpy_s and wide characters (wchar_t) for wmemcpy_s.

src Buffer to copy from.

count Number of characters to copy.

so the call should be something like:

memcpy_s(_vertices, sizeof(vertices), vertices, sizeof(vertices));
//although, I would just use the regular 'memcpy':
memcpy(_vertices, vertices, sizeof(vertices));

Your SetupBuffer function takes the parameter vs which is input directly into glBufferData, however glBufferData also takes the size in bytes:

size

Specifies the size in bytes of the buffer object's new data store.

so entering the value 36 will not suffice. You want to find a way to pass the value of sizeof(vertices), although since you know the stride of each vertex (5 floats), you could change the call to:

glBufferData(GL_ARRAY_BUFFER, vs * 5 * sizeof(float), _vertices, GL_STATIC_DRAW);

As many of the comments mentioned, you could also use the std::vector class to simplify some of the code, however I think it is much more important to address the misunderstandings in the code you have already written.

OOP Issues

I am fairly sure the above will fix your problem. However, just to make sure the constructor:

Cube::Cube() : GameRenderable(GameRenderableMode::_3D) {

doesn't overwrite the GameRenderable constructor, the GameRenderable constructor will be called first (with the parameter _3D) so ensure that the code within the GameRenderable constructor doesn't break anything (as I can't see it).

Community
  • 1
  • 1
nedb
  • 586
  • 1
  • 4
  • 12