Load an OpenGL buffer from file

The function's job is to load a vertex attribute buffer or an element buffer from a specified path, and upload it to OpenGL. Also, optionally, return the number of vertices/indices loaded through an output parameter.

Signature:

GLuint loadBufferFromFile(std::string const& bufferPath, unsigned char vertexElementCount, GLenum vertexFormat, GLenum bufferType=GL_ARRAY_BUFFER, GLuint *vertexCount=NULL, GLenum usageHint=GL_STATIC_DRAW);

Body:

```
GLuint loadBufferFromFile(std::string const& bufferPath, unsigned char vertexElementCount, GLenum vertexFormat, GLenum bufferType, GLuint *vertexCount, GLenum usageHint)
{
const std::unordered_set VALID_VERTEX_FORMATS = {GL_FLOAT, GL_UNSIGNED_INT, GL_UNSIGNED_BYTE};
//bufferType and usageHint will be validated by OpenGL

if(vertexElementCount 4)
{
throw std::invalid_argument("Vertex element count must be in range ");
}

if(bufferType == GL_ELEMENT_ARRAY_BUFFER && vertexElementCount != 1)
{
throw std::invalid_argument("Buffer of type GL_ELEMENT_ARRAY_BUFFER must have element count of 1");
}

if(VALID_VERTEX_FORMATS.find(vertexFormat) == VALID_VERTEX_FORMATS.end())
{
throw std::invalid_argument("Invalid vertex format");
}

std::ifstream file;
file.open(bufferPath, std::ios::binary | std::ios::in);

if(!file.is_open())
{
throw std::runtime_error(std::string("Buffer file ") + bufferPath + " could not be opened");
}

file.seekg(0, file.end);
unsigned int length = file.tellg();
file.seekg(0, file.beg);

if(vertexCount != NULL)
{
std::size_t vertexSize;

switch(vertexFormat)
{
case GL_FLOAT:
vertexSize = sizeof(GLfloat);
break;
case GL_UNSIGNED_INT:
vertexSize = sizeof(GLuint);
break;
case GL_UNSIGNED_BYTE:
vertexSize = sizeof(GLubyte);
break;
}

As for your questions:

• Just use int, unless this is critical code where benchmarks have shown that integers are noticeably slower. Furthermore: your char will support 255 vertices which isn't a lot, I've loaded models with millions of vertices.

• This level of error checking is possibly an indication that your code smells. You have a function that accepts many parameters, perhaps you can split it into multiple functions or a class type of structure.

• Myself, I make everything const unless it cannot be const.

• If the size is known at compile time, how about std::array instead of std::vector? As for std::unordered_set it seems too fancy for the job (e.g., a self balancing tree with 3 items, whoop! whoop!), but I have no strong arguments.

• No opinion. Myself, I never use C++ exceptions - due to my lack of understanding / education. In other languages I never directly use the build-in Exception types, I always subclass my own - if anything it helps the library user understand if they are library errors or system errors.

• You can place your new'ed memory in a std::unique_ptr or use the resize features of std::vector or std::string

Couple more random things:

• file.close(); is optional, the std::ifstream will also close the resource (if it was open).

• You're not testing for OpenGL related errors.

• I don't like 'out' arguments, such as vertexCount. Just return a std::tuple with multiple arguments, or a struct.

• Does your file contain binary data? You're reading it from disk, and directly upload it to the GPU without parsing/validation? If anything, make sure you've actually read the amount of bytes that tellg claims are available.

• std::ifstream has a constructor, use it.

StackExchange Code Review Q#92787, answer score: 2