go-gl-legacy / gl

Go bindings for OpenGL
BSD 3-Clause "New" or "Revised" License
342 stars 52 forks source link

Non-conformant VertexAttribPointer implementation for OpenGL >= 3.0 #141

Closed jonvaldes closed 10 years ago

jonvaldes commented 10 years ago

According to the OpenGL API docs ( http://www.opengl.org/sdk/docs/man3/xhtml/glVertexAttribPointer.xml ), the "pointer" element in glVertexAttribPointer has been repurposed in OpenGL 3.x to indicate an offset into the currently bound buffer, effectively treating that GLvoid* parameter as a 32/64 bit uint.

In simple cases, a "nil" value is enough to indicate a zero offset, but more complex cases require specific offsets, which because of the go-gl API design are not easy to convey (how to make an interface{} have a 0x00000005 address?)

This is why I believe a modification to the current API is required for this way of using this function.

I propose adding another function with a uintptr parameter, so integer values can be passed directly, just like they are in the C API. The function signature would look something like this:

func (indx AttribLocation) AttribPointerOffset(size uint, typ GLenum, normalized bool, stride int, offset uintptr)

Here's a diff for a possible implementation of this API change:

diff --git a/attriblocation.go b/attriblocation.go
index a9301d9..a4ce8fd 100644
--- a/attriblocation.go
+++ b/attriblocation.go
@@ -6,6 +6,7 @@ package gl

 // #include "gl.h"
 import "C"
+import "unsafe"

 // AttribLocation

@@ -48,6 +49,11 @@ func (indx AttribLocation) AttribPointer(size uint, typ GLenum, normalized bool,
                glBool(normalized), C.GLsizei(stride), ptr(pointer))
 }

+func (indx AttribLocation) AttribPointerOffset(size uint, typ GLenum, normalized bool, stride int, offset uintptr) {
+       C.glVertexAttribPointer(C.GLuint(indx), C.GLint(size), C.GLenum(typ),
+               glBool(normalized), C.GLsizei(stride), unsafe.Pointer(offset))
+}
+
 func (indx AttribLocation) EnableArray() {
        C.glEnableVertexAttribArray(C.GLuint(indx))
 }
polyfloyd commented 10 years ago

The existing function (AttribLocation.AttribPointer) is already capable of accepting and converting a uintptr to an offset. Or do you want that extra function to bypass the runtime reflection?

jonvaldes commented 10 years ago

Well, I totally missed that feature. Thanks for telling me about it!

May I then suggest adding some documentation to that function, as the library source code and examples don't say anything about that capability?

pwaller commented 10 years ago

Pull requests welcome!

pwaller commented 10 years ago

Closing this one since the original problem is resolved.