grammar
Source Link
Rish
  • 1.7k
  • 1
  • 7
  • 18
  • Use lower case for data members. It's fine for VBO, VAO, EBO since they are abbreviations (and you might consider renaming them as well), but change DRAW_MODE and PRIMITIVE_TYPE to lower case.

  • Presumably, Vertex is a struct containing just 3 floats for the position. When setting the attributes, you have a single call to set attribute 0. What happens when you expand your Vertex struct to contain normals, color, uv coordinates, tangents, et cetera? Okay, then you can go back into your init function and add those attributes. But what if a certain mesh doesn't contain color data? Since you have hardcoded the attributes, you have no way of changing it without breaking some other meshes.

  • You can use vertices.data() instead of &vertices[0].

  • Don't use identifiers beginning with an underscore. See here.

  • Pass the vectors using const reference.

  • Consider useusing std::move if you're not going to be using the vector again.

  • this is usually omitted by C++ programmers unless one needs to explicitly refer to the current object.

  • In draw() you don't need to bind the index buffer again.

  • As you mentioned, updating each vertex is horrible tactic. If you have a mesh with 10000s of10000 vertices, you will have spent a good part of the time available updating each mesh. Also it might be possible that certain entities share the mesh data (they might be instanced). If you update the data in the mesh, they will all get updated.

Here's a way to change it. Each entity has a handle to a Mesh (or a pointer) and a model matrix. Any update to position or scale or rotation is reflected in a change to the model matrix. Then, when rendering the entity, you bind your mesh's vertex array and pass the model matrix to the shader, which then combines it with the view-projection matrix to render the object.

You can of course do other optimizations, such as instanced rendering.

  • You have two duplicate vertices and indices vectors, one inherited from Mesh and other you declare as private.

  • Consider using move semantics on objects that you're throwing away after use, such as the Vertex in create_mesh_plane().

  • Use lower case for data members. It's fine for VBO, VAO, EBO since they are abbreviations (and you might consider renaming them as well), but change DRAW_MODE and PRIMITIVE_TYPE to lower case.

  • Presumably, Vertex is a struct containing just 3 floats for the position. When setting the attributes, you have a single call to set attribute 0. What happens when you expand your Vertex struct to contain normals, color, uv coordinates, tangents, et cetera? Okay, then you can go back into your init function and add those attributes. But what if a certain mesh doesn't contain color data? Since you have hardcoded the attributes, you have no way of changing it without breaking some other meshes.

  • You can use vertices.data() instead of &vertices[0].

  • Don't use identifiers beginning with an underscore. See here.

  • Pass the vectors using const reference.

  • Consider use std::move if you're not going to be using the vector again.

  • this is usually omitted by C++ programmers unless one needs to explicitly refer to the current object.

  • In draw() you don't need to bind the index buffer again.

  • As you mentioned, updating each vertex is horrible tactic. If you a mesh with 10000s of vertices, you will have spent a good part of time available updating each mesh. Also it might be possible that certain entities share the mesh data (they might be instanced). If you update the data in the mesh, they will all get updated.

Here's a way to change it. Each entity has a handle to a Mesh (or a pointer) and a model matrix. Any update to position or scale or rotation is reflected in a change to the model matrix. Then, when rendering the entity, you bind your mesh's vertex array and pass the model matrix to the shader, which then combines it with the view-projection matrix to render the object.

You can of course do other optimizations, such as instanced rendering.

  • You have two duplicate vertices and indices vectors, one inherited Mesh and other you declare as private.

  • Consider using move semantics on objects that you're throwing away after use, such as the Vertex in create_mesh_plane().

  • Use lower case for data members. It's fine for VBO, VAO, EBO since they are abbreviations (and you might consider renaming them as well), but change DRAW_MODE and PRIMITIVE_TYPE to lower case.

  • Presumably, Vertex is a struct containing just 3 floats for the position. When setting the attributes, you have a single call to set attribute 0. What happens when you expand your Vertex struct to contain normals, color, uv coordinates, tangents, et cetera? Okay, then you can go back into your init function and add those attributes. But what if a certain mesh doesn't contain color data? Since you have hardcoded the attributes, you have no way of changing it without breaking some other meshes.

  • You can use vertices.data() instead of &vertices[0].

  • Don't use identifiers beginning with an underscore. See here.

  • Pass the vectors using const reference.

  • Consider using std::move if you're not going to be using the vector again.

  • this is usually omitted by C++ programmers unless one needs to explicitly refer to the current object.

  • In draw() you don't need to bind the index buffer again.

  • As you mentioned, updating each vertex is horrible tactic. If you have a mesh with 10000 vertices, you will have spent a good part of the time available updating each mesh. Also it might be possible that certain entities share the mesh data (they might be instanced). If you update the data in the mesh, they will all get updated.

Here's a way to change it. Each entity has a handle to a Mesh (or a pointer) and a model matrix. Any update to position or scale or rotation is reflected in a change to the model matrix. Then, when rendering the entity, you bind your mesh's vertex array and pass the model matrix to the shader, which then combines it with the view-projection matrix to render the object.

You can of course do other optimizations, such as instanced rendering.

  • You have two duplicate vertices and indices vectors, one inherited from Mesh and other you declare as private.

  • Consider using move semantics on objects that you're throwing away after use, such as the Vertex in create_mesh_plane().

Source Link
Rish
  • 1.7k
  • 1
  • 7
  • 18

  • Use lower case for data members. It's fine for VBO, VAO, EBO since they are abbreviations (and you might consider renaming them as well), but change DRAW_MODE and PRIMITIVE_TYPE to lower case.

  • Presumably, Vertex is a struct containing just 3 floats for the position. When setting the attributes, you have a single call to set attribute 0. What happens when you expand your Vertex struct to contain normals, color, uv coordinates, tangents, et cetera? Okay, then you can go back into your init function and add those attributes. But what if a certain mesh doesn't contain color data? Since you have hardcoded the attributes, you have no way of changing it without breaking some other meshes.

  • You can use vertices.data() instead of &vertices[0].

  • Don't use identifiers beginning with an underscore. See here.

  • Pass the vectors using const reference.

  • Consider use std::move if you're not going to be using the vector again.

  • this is usually omitted by C++ programmers unless one needs to explicitly refer to the current object.

  • In draw() you don't need to bind the index buffer again.

  • As you mentioned, updating each vertex is horrible tactic. If you a mesh with 10000s of vertices, you will have spent a good part of time available updating each mesh. Also it might be possible that certain entities share the mesh data (they might be instanced). If you update the data in the mesh, they will all get updated.

Here's a way to change it. Each entity has a handle to a Mesh (or a pointer) and a model matrix. Any update to position or scale or rotation is reflected in a change to the model matrix. Then, when rendering the entity, you bind your mesh's vertex array and pass the model matrix to the shader, which then combines it with the view-projection matrix to render the object.

You can of course do other optimizations, such as instanced rendering.

  • You have two duplicate vertices and indices vectors, one inherited Mesh and other you declare as private.

  • Consider using move semantics on objects that you're throwing away after use, such as the Vertex in create_mesh_plane().