Part of the Khronos Group
OpenGL.org

The Industry's Foundation for High Performance Graphics

from games to virtual reality, mobile phones to supercomputers

Results 1 to 9 of 9

Thread: Issues trying to add instancing to graphics system

  1. #1
    Junior Member Newbie
    Join Date
    Jul 2015
    Posts
    11

    Issues trying to add instancing to graphics system

    Hello. I am working on a 2D c++ game project. I am trying to improve the graphics system by reducing the number of draw calls. So I basically copied the Sprite class I had, and tweaked it into a new class: ISprite. And while I can create ISprites without any errors, they do not actually draw on the screen. Sprites and debug lines still display normally. So basically I'm trying to find out what is wrong with my instanced sprite system and would appreciate some help.

    Here is some relevant code:
    Vertex Shader
    Code :
    #version 330 core
     
    layout (location = 0) in vec3 position;
    layout (location = 1) in vec2 texCoords;
    layout (location = 2) in vec3 colorSource;
    layout (location = 3) in mat4 transform;
     
    out vec2 TexCoords;
    out vec4 Color;
     
    uniform mat4 uniformView;
    uniform mat4 uniformProjection;
     
    void main()
    {
      gl_Position = uniformProjection * uniformView * transform * vec4(position, 1.0f);
      TexCoords = texCoords;
      Color = vec4(colorSource, 1.0f);
    }

    Fragment Shader
    Code :
    #version 330 core
    in vec2 TexCoords;
    in vec4 Color;
     
    out vec4 color;
     
    uniform sampler2D Texture;
    uniform vec4 uniformColor;
     
    void main()
    {
        vec4 texColor = texture(Texture, TexCoords) * Color;
        if(texColor.a < 0.1)
            discard;
     
        color = texColor;
     
     }

    Load, does preparations required for drawing every sprite
    Code :
    void ISprite::Load(Shader spriteShader)
    {
      GLfloat vertices[] = {
        //X    Y     Z     
        0.5f, -0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f,
        -0.5f, -0.5f, 0.0f,
        0.5f, -0.5f, 0.0f,
        0.5f, 0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f
      };
     
      glGenVertexArrays(1, &vertexArray);
      glGenBuffers(1, &positionBuffer);
      glGenBuffers(1, &texCoordsBuffer);
      glGenBuffers(1, &colorBuffer);
      glGenBuffers(1, &matrixBuffer);
     
      glBindVertexArray(vertexArray);
     
      //For vertex Position
      glEnableVertexAttribArray(0);
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, (GLvoid*)0);
     
      //The vertex data will never change, so send that data now. 
      glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);
     
      //For texture coordinates
      glEnableVertexAttribArray(1);
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 0, (GLvoid*)0);
     
      //For Color
      glEnableVertexAttribArray(2);
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glVertexAttribPointer(2, 3, GL_FLOAT, GL_FALSE, 0, (GLvoid*)0);
     
      //For Transformation Matrix
      glEnableVertexAttribArray(3);
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glVertexAttribPointer(3, 1, GL_MATRIX4_ARB, GL_FALSE, 0, (GLvoid*)0);
     
      //Unbind
      glBindBuffer(GL_ARRAY_BUFFER, 0);
      glBindVertexArray(0);
     
      //Sets up support for instancing
      glVertexAttribDivisor(0, 0); 
      glVertexAttribDivisor(1, 1); 
      glVertexAttribDivisor(2, 1);
      glVertexAttribDivisor(3, 1);
     
      ISprite::shader = &spriteShader;
    }

    And the two drawing functions:
    Prepare Draw
    Code :
    void ISprite::prepareDraw(void)
    {
      //Adds their personal data to vectors shared by class 
      transformMatrices.push_back(calculateTransorm());
     
      for (int i = 0; i < 6; i += 2)
      {
        texture.updateAnimation();
        ISprite::textureCoordinatesAll.push_back(texture.textureCoordinates[i]);
        ISprite::textureCoordinatesAll.push_back(texture.textureCoordinates[i + i]);
      }
     
      ISprite::colorValues.push_back(color.x);
      ISprite::colorValues.push_back(color.y);
      ISprite::colorValues.push_back(color.z);
    }

    drawSprites
    Code :
    void ISprite::drawSprites(Texture testTexture)
    {
      shader->Use();
      glBindTexture(GL_TEXTURE_2D, testTexture.ID);
     
      for (std::vector<ISprite*>::iterator it = Isprites.begin(); it != Isprites.end(); ++it)
        (*it)->prepareDraw();
     
      glBindVertexArray(vertexArray);
      //Bind texture here if you want textures to work. if not, a single texture atlas will be bound
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glBufferData(GL_ARRAY_BUFFER, textureCoordinatesAll.size() * sizeof(GLfloat),
                    NULL, GL_STREAM_DRAW);  
      glBufferSubData(GL_ARRAY_BUFFER, 0, textureCoordinatesAll.size() * sizeof(GLfloat), 
                    textureCoordinatesAll.data());
     
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glBufferData(GL_ARRAY_BUFFER, colorValues.size() * sizeof(GLfloat),
                  NULL, GL_STREAM_DRAW);
      glBufferSubData(GL_ARRAY_BUFFER, 0, colorValues.size() * sizeof(GLfloat),
                   colorValues.data());
     
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glBufferData(GL_ARRAY_BUFFER, transformMatrices.size() * sizeof(glm::mat4),
                  NULL, GL_STREAM_DRAW);
      glBufferSubData(GL_ARRAY_BUFFER, 0, transformMatrices.size() * sizeof(glm::mat4),
                  transformMatrices.data());
     
      glDrawArraysInstanced(GL_TRIANGLES, 0, 6, Isprites.size());
     
      textureCoordinatesAll.clear();
      colorValues.clear();
      transformMatrices.clear();
      glBindBuffer(GL_ARRAY_BUFFER, 0);
      glBindVertexArray(0);
    }

  2. #2
    Senior Member OpenGL Guru
    Join Date
    Jun 2013
    Posts
    2,734
    Quote Originally Posted by redalwaysmafia View Post
    Code :
      glVertexAttribPointer(3, 1, GL_MATRIX4_ARB, GL_FALSE, 0, (GLvoid*)0);
    This is invalid.

    Vertex attributes which are matrices are treated as multiple consecutive attributes, one for each column, i.e.:
    Code :
      glVertexAttribPointer(3+0, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(0*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+1, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(1*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+2, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(2*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+3, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(3*4*sizeof(GLfloat)));

    The GL_MATRIXn_ARB enumerants are a historical relic. They are only used by the ARB_vertex_program and ARB_fragment_program extensions (which aren't the same thing as OpenGL 2+ shader programs), where they are valid as arguments to glMatrixMode.

  3. #3
    Junior Member Newbie
    Join Date
    Jul 2015
    Posts
    11
    Quote Originally Posted by GClements View Post
    This is invalid.

    Vertex attributes which are matrices are treated as multiple consecutive attributes, one for each column, i.e.:
    Code :
      glVertexAttribPointer(3+0, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(0*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+1, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(1*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+2, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(2*4*sizeof(GLfloat)));
      glVertexAttribPointer(3+3, 4, GL_FLOAT, GL_FALSE, 4*4*sizeof(GLfloat), (GLvoid*)(3*4*sizeof(GLfloat)));

    The GL_MATRIXn_ARB enumerants are a historical relic. They are only used by the ARB_vertex_program and ARB_fragment_program extensions (which aren't the same thing as OpenGL 2+ shader programs), where they are valid as arguments to glMatrixMode.
    Interesting. Does this change the way the shader needs to be written? Do i need to handle a layout 4, 5, and 6 and construct the matrix from it in the shader?
    Does the vertexattrib divisor call need to be different?
    Do buffer data calls and buffer sub data calls need to be different?

  4. #4
    Senior Member OpenGL Lord
    Join Date
    May 2009
    Posts
    5,976
    Does this change the way the shader needs to be written?
    No. Matrix attributes explicitly take up a number of locations equal to their column count. These locations are allocated sequentially following the assigned location.

    Your shader is fine; it's the code interacting with it that's wrong.

    Does the vertexattrib divisor call need to be different?
    Yes. Since you have 4 attribute locations, you need to assign a divisor to each location. Your code is feeding a series of vector attributes to the shader; the shader is simply automatically interpreting the group of vectors as a matrix.

    All that being said, really, you shouldn't bother with instancing for something like this. If you want to reduce the number of draw calls (to the extent that this would be impacting performance), it's far better to just generate the vertex/color/etc data for your sprites on the CPU, shove it into a buffer, and render it in one draw command.

    Instancing has a cost to it. It only tends to be worthwhile if the number of vertices per instance is non-trivially large, and if the instance count is suitably large (thousands).

  5. #5
    Junior Member Newbie
    Join Date
    Jul 2015
    Posts
    11
    So you're suggesting that I keep a similar structure where I put each object's data into a single container and then send that container? What changes to the system would this entail other than removing the attrib divisor calls and calling drawArrays instead of drawArraysInstanced ?

  6. #6
    Senior Member OpenGL Guru
    Join Date
    Jun 2013
    Posts
    2,734
    Quote Originally Posted by redalwaysmafia View Post
    So you're suggesting that I keep a similar structure where I put each object's data into a single container and then send that container? What changes to the system would this entail other than removing the attrib divisor calls and calling drawArrays instead of drawArraysInstanced ?
    Removing instancing would move most of work of the vertex shader onto the client side. Rather than sending 4 positions, 1 pairs of texture coordinates, 1 colour and 1 transformation, you'd send 4 (transformed) positions, 4 pairs of texture coordinates and 4 colours.

    Personally, I'd probably stick with instancing. Instancing will have a higher GPU load, a non-instanced approach a higher CPU load and higher memory consumption and bandwidth. For a 2D game, I doubt that you will be saturating any of those, so it doesn't really matter which you pick. Eliminating the instancing will improve portability (instancing requires at least OpenGL 3.1, and 3.3 to be useful), but the instanced approach leads to a cleaner design (the CPU is storing and sending the data in its "natural" format, leaving the decoding to the shaders).

  7. #7
    Junior Member Newbie
    Join Date
    Jul 2015
    Posts
    11
    Hello All. I've made some changes as per your suggestions. But Am still not getting things to successfully draw. I've made some changes to some of the functions, based on the advice I've received from here and other sources.

    No changes have been made to the shader, because people have been saying that the shader seems fine.

    Load
    Code :
    void ISprite::Load(Shader spriteShader)
    {
    	spriteShader.Use();
     
      GLfloat vertices[] = {
        //X    Y     Z     
        0.5f, -0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f,
        -0.5f, -0.5f, 0.0f,
        0.5f, -0.5f, 0.0f,
        0.5f, 0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f
      };
     
      glGenVertexArrays(1, &vertexArray);
      glGenBuffers(1, &positionBuffer);
      glGenBuffers(1, &texCoordsBuffer);
      glGenBuffers(1, &colorBuffer);
      glGenBuffers(1, &matrixBuffer);
     
      //The vertex data will never change, so send that data now. 
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);
     
      //For vertex Position
      glEnableVertexAttribArray(0);
      glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3*sizeof(GLfloat), (GLvoid*)0);
     
      //For texture coordinates
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glEnableVertexAttribArray(1);
      glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 2*sizeof(GLfloat), (GLvoid*)0);
     
      //For Color
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glEnableVertexAttribArray(2);
      glVertexAttribPointer(2, 4, GL_FLOAT, GL_FALSE, 4 * sizeof(GLfloat), (GLvoid*)0);
     
      //For Transformation Matrix
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      for (int i = 0; i < 4; ++i)
      {
    	  glEnableVertexAttribArray(3 + i);
    	  glVertexAttribPointer(3 + i, 4, GL_FLOAT, GL_FALSE, 
    		  4 * 4 * sizeof(GLfloat), (GLvoid*)(4 * i * sizeof(GLfloat)));
      }
     
      glBindVertexArray(vertexArray);
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glBindVertexArray(0);
     
      glVertexAttribDivisor(positionBuffer, 0);
      glVertexAttribDivisor(texCoordsBuffer, 1);
      glVertexAttribDivisor(colorBuffer, 1);
      glVertexAttribDivisor(matrixBuffer, 1);
      glVertexAttribDivisor(matrixBuffer + 1, 1);
      glVertexAttribDivisor(matrixBuffer + 2, 1);
      glVertexAttribDivisor(matrixBuffer + 3, 1);
     
      ISprite::shader = &spriteShader;
    }

    Prepare Draw
    Code :
    void ISprite::prepareDraw(void)
    {
      //Adds their personal data to vectors shared by class 
      glm::mat4 transform = calculateTransorm();
     
      for (int i = 0; i < 4; ++i)
      {
    	  for (int j = 0; j < 4; ++j)
    	    ISprite::transformMatrices.push_back(transform[i][j]);
      }
     
      texture.updateAnimation();
      for (int i = 0; i < 12; ++i)
        ISprite::textureCoordinatesAll.push_back(texture.textureCoordinates[i]);
     
      ISprite::colorValues.push_back(color.x);
      ISprite::colorValues.push_back(color.y);
      ISprite::colorValues.push_back(color.z);
      ISprite::colorValues.push_back(color.w);
    }

    Draw
    Code :
    void ISprite::drawSprites(Texture testTexture)
    {
      shader->Use();
     
      for (std::vector<ISprite*>::iterator it = Isprites.begin(); it != Isprites.end(); ++it)
        (*it)->prepareDraw();
     
      glBindVertexArray(vertexArray);
      glBindTexture(GL_TEXTURE_2D, testTexture.ID);
     
      //Bind texture here if you want textures to work. if not, a single texture atlas will be bound
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glBufferData(GL_ARRAY_BUFFER, textureCoordinatesAll.size() * sizeof(GLfloat),
    	  textureCoordinatesAll.data(), GL_STREAM_DRAW);
     
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glBufferData(GL_ARRAY_BUFFER, colorValues.size() * sizeof(GLfloat),
    	  colorValues.data(), GL_STREAM_DRAW);
     
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glBufferData(GL_ARRAY_BUFFER, transformMatrices.size() * sizeof(GLfloat),
    	  transformMatrices.data(), GL_STREAM_DRAW);
     
      glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 6, Isprites.size());
     
      textureCoordinatesAll.clear();
      colorValues.clear();
      transformMatrices.clear();
      glBindBuffer(GL_ARRAY_BUFFER, 0);
      glBindTexture(GL_TEXTURE_2D, 0);
      glBindVertexArray(0);
    }

  8. #8
    Senior Member OpenGL Guru
    Join Date
    Jun 2013
    Posts
    2,734
    Quote Originally Posted by redalwaysmafia View Post
    Load
    Code :
      glGenVertexArrays(1, &vertexArray);
      glGenBuffers(1, &positionBuffer);
      glGenBuffers(1, &texCoordsBuffer);
      glGenBuffers(1, &colorBuffer);
      glGenBuffers(1, &matrixBuffer);
     
      //The vertex data will never change, so send that data now. 
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);
     
      //For vertex Position
      glEnableVertexAttribArray(0);
      glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3*sizeof(GLfloat), (GLvoid*)0);
    At this point, the default VAO is still bound, so that's where the attribute settings are stored. You need to bind your VAO first.
    Quote Originally Posted by redalwaysmafia View Post
    Code :
      glBindVertexArray(vertexArray);
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glBindVertexArray(0);
    The above is pointless. The GL_ARRAY_BUFFER binding isn't stored in a VAO; it's global state. The buffer for each attribute (i.e. the buffer which is bound to GL_ARRAY_BUFFER at the time of a glVertexAttribPointer() call) is stored in a VAO, as is the GL_ELEMENT_ARRAY_BUFFER binding.
    Quote Originally Posted by redalwaysmafia View Post
    Code :
      glVertexAttribDivisor(positionBuffer, 0);
      glVertexAttribDivisor(texCoordsBuffer, 1);
      glVertexAttribDivisor(colorBuffer, 1);
      glVertexAttribDivisor(matrixBuffer, 1);
      glVertexAttribDivisor(matrixBuffer + 1, 1);
      glVertexAttribDivisor(matrixBuffer + 2, 1);
      glVertexAttribDivisor(matrixBuffer + 3, 1);
    Again, your VAO isn't bound at this point, so these settings are stored in the default VAO.

    Quote Originally Posted by redalwaysmafia View Post
    Draw
    Code :
      glBindVertexArray(vertexArray);
      glBindTexture(GL_TEXTURE_2D, testTexture.ID);
     
      //Bind texture here if you want textures to work. if not, a single texture atlas will be bound
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glBufferData(GL_ARRAY_BUFFER, textureCoordinatesAll.size() * sizeof(GLfloat),
    	  textureCoordinatesAll.data(), GL_STREAM_DRAW);
     
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glBufferData(GL_ARRAY_BUFFER, colorValues.size() * sizeof(GLfloat),
    	  colorValues.data(), GL_STREAM_DRAW);
     
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      glBufferData(GL_ARRAY_BUFFER, transformMatrices.size() * sizeof(GLfloat),
    	  transformMatrices.data(), GL_STREAM_DRAW);
     
      glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 6, Isprites.size());
    You're calling glDrawArraysInstanced() with your VAO bound, but the attribute settings are all in the default VAO; your VAO is never changed from its initial state.

  9. #9
    Junior Member Newbie
    Join Date
    Jul 2015
    Posts
    11
    Ok, so it souds like the main issue is improperly binding the VAO in the load function. Does this load function look more correct? Based on your advice, I changed my function to this. but this still doesn't draw.

    Load
    Code :
    void ISprite::Load(Shader spriteShader)
    {
      spriteShader.Use();
     
      GLfloat vertices[] = {
        //X    Y     Z     
        0.5f, -0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f,
        -0.5f, -0.5f, 0.0f,
        0.5f, -0.5f, 0.0f,
        0.5f, 0.5f, 0.0f,
        -0.5f, 0.5f, 0.0f
      };
     
      glGenVertexArrays(1, &vertexArray);
      glBindVertexArray(vertexArray);
     
      glGenBuffers(1, &positionBuffer);
      glGenBuffers(1, &texCoordsBuffer);
      glGenBuffers(1, &colorBuffer);
      glGenBuffers(1, &matrixBuffer);
     
      //For vertex Position
      glBindBuffer(GL_ARRAY_BUFFER, positionBuffer);
      glEnableVertexAttribArray(0);
      glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(GLfloat), (GLvoid*)0);
      //The vertex data will never change, so send that data now. 
      glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);
     
      //For texture coordinates
      glBindBuffer(GL_ARRAY_BUFFER, texCoordsBuffer);
      glEnableVertexAttribArray(1);
      glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 2*sizeof(GLfloat), (GLvoid*)0);
     
      //For Color
      glBindBuffer(GL_ARRAY_BUFFER, colorBuffer);
      glEnableVertexAttribArray(2);
      glVertexAttribPointer(2, 4, GL_FLOAT, GL_FALSE, 4 * sizeof(GLfloat), (GLvoid*)0);
     
      //For Transformation Matrix
      glBindBuffer(GL_ARRAY_BUFFER, matrixBuffer);
      for (int i = 0; i < 4; ++i)
      {
    	  glEnableVertexAttribArray(3 + i);
    	  glVertexAttribPointer(3 + i, 4, GL_FLOAT, GL_FALSE, 
    		  4 * 4 * sizeof(GLfloat), (GLvoid*)(4 * i * sizeof(GLfloat)));
      }
     
      glVertexAttribDivisor(0, 0);
      glVertexAttribDivisor(1, 1);
      glVertexAttribDivisor(2, 1);
      glVertexAttribDivisor(3, 1);
      glVertexAttribDivisor(3 + 1, 1);
      glVertexAttribDivisor(3 + 2, 1);
      glVertexAttribDivisor(3 + 3, 1);
     
      glBindVertexArray(0);
     
      ISprite::shader = &spriteShader;
    }

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •