HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

OpenGL VertexArrayObject class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classvertexarrayobjectopengl

Problem

It seems to play up sometimes and I'm not sure if this is due to the class itself or the way I'm using it. Can someone confirm that this is a 'good' implementation of a VertexArrayObject class?

```
//The following code is public domain

public class VertexArray : GraphicsResource
{
protected readonly int ID;
DrawMode _drawMode;
VertexLayout _vertexType;

protected BufferUsageHint bufferUsage = BufferUsageHint.StaticDraw;
int vertexCount;
int vertexBufferID;

public DrawMode DrawMode
{
get { return _drawMode; }
set { _drawMode = value; }
}
public int VertexCount
{
get { return vertexCount; }
}
public BufferUsageHint BufferUsage
{
get { return bufferUsage; }
set { bufferUsage = value; }
}

public VertexArray()
: base()
{
ID = createVAO();
disposed = false;
}
public VertexArray(DrawMode drawMode)
: base()
{
_drawMode = drawMode;
ID = createVAO();
disposed = false;
}
int createVAO()
{
int id;
GL.GenBuffers(1, out vertexBufferID);
GL.GenVertexArrays(1, out id);
GL.BindVertexArray(id);
GL.BindBuffer(BufferTarget.ArrayBuffer, vertexBufferID);
VenGFX.currentVAO = this;
return id;
}
public void Bind()
{
if (vertexCount == 0)
throw new Exception("Vertex buffer has no data, and therefore can't be used");
if (disposed)
throw new ObjectDisposedException(ToString());
GL.BindVertexArray(ID);
VenGFX.currentVAO = this;
}
public void SetData(T[] Array) where T : struct, IVertex
{
GL.BindVertexArray(ID);
if (Array.Length > 0 && Array[0].Layout != _vertexType)
setVertexType(Array[0].Layout);
GL.BindBuffer(BufferTarget.ArrayBuffer, vertexBufferID);
GL.BufferData(BufferTarget.ArrayBuffer, (IntPtr)(Array.Length * _vertexType

Solution

I know nothing of OpenGL and what a correct implementation of a VertexArrayObject should look like, but I do have a couple of observations:

  • I like that the ID private field is readonly, ...but its naming breaks the naming convention for private fields, which should be either id or _id - looking at the other private fields _id would be more consistent.



  • I don't see why the other fields vertexCount and vertexBufferID (should be vertexBufferId) aren't prefixed with an underscore as well.



  • Vertical whitespace between methods/members isn't consistent either. Skip a line between } and the next member, it'll breathe better.



public VertexArray()
    : base()


The : base() part is redundant. Base constructor always gets called.

In the Release() override, you're doing this:

int id = ID;
    GL.DeleteBuffers(1, ref vertexBufferID);
    GL.DeleteVertexArrays(1, ref id);


Whatever DeleteVertexArrays is doing with id (passed by reference), you're not doing anything with the [possibly] modified value, I'm guessing because ID is readonly. But this is clearly a case where the "why" would be nicely explained with a short comment.

Lastly, properties like DrawMode that use a non-readonly backing field...

DrawMode _drawMode;
public DrawMode DrawMode
{
    get { return _drawMode; }
    set { _drawMode = value; }
}


...would probably be better off rewritten as auto-properties:

public DrawMode DrawMode { get; set; }
public BufferUsageHint BufferUsag { get; set; }
public int VertexCount { get; private set; }

Code Snippets

public VertexArray()
    : base()
int id = ID;
    GL.DeleteBuffers(1, ref vertexBufferID);
    GL.DeleteVertexArrays(1, ref id);
DrawMode _drawMode;
public DrawMode DrawMode
{
    get { return _drawMode; }
    set { _drawMode = value; }
}
public DrawMode DrawMode { get; set; }
public BufferUsageHint BufferUsag { get; set; }
public int VertexCount { get; private set; }

Context

StackExchange Code Review Q#1880, answer score: 6

Revisions (0)

No revisions yet.