search

go-gl / gl

Go bindings for OpenGL (generated via glow)
MIT License
1.06k stars 72 forks source link

race detector checkptr fails on PtrOffset #124

Closed rcoreilly closed 3 years ago

rcoreilly commented 4 years ago

as of version 1.14, the race detector enables checkptr https://golang.org/doc/go1.14#compiler which fails on PtrOffset, at least for example in VertexAttribPointer:

gl.VertexAttribPointer(uint32(v.handle), int32(v.typ.Vec), gpu.TheGPU.Type(v.typ.Type), false, int32(str*4), gl.PtrOffset(off*4))

Not sure there is much to be done about this given the logic of checkptr but just in case someone else hits this obstacle, here's how to overcome it:

go build -race -gcflags=all=-d=checkptr=0

dertseha commented 4 years ago

Yup, this new checker cascades quite a lot. The core issue here is the signature of VertexAttribPointer(), which has unsafe.Pointer as the last parameter. The fix would be to change the type to be uintptr_t in the C code - which, for this method, is more fitting as it takes arbitrary offset numbers.

See also: https://golang.org/doc/go1.14#compiler

edit: I believe #80 and the discussion there will become relevant as well now.

neclepsio commented 4 years ago

In the meantime it gets fixed, you can use my fork.

rcoreilly commented 4 years ago

@neclepsio could you submit a PR to fix this issue in the main package?

neclepsio commented 4 years ago

@rcoreilly I did nothing but running what already done by @dertseha for issue #80. There is ongoing discussion in that issue on what and how exactly to fix. I doubt a pull request with only the result their own generator would be useful: they just need the time to think what to do. Since I needed a fix as soon as possible, and one was already available but not so easy to use, I made a fork so that everyone in my position can use it in the meantime.

dertseha commented 3 years ago

For clarification why #135 closes this issue: Instead of calling gl.VertexAttribPointer() the code needs to call the newly available gl.VertexAttribPointerWithOffset() - like this:

gl.VertexAttribPointerWithOffset(uint32(v.handle), int32(v.typ.Vec), gpu.TheGPU.Type(v.typ.Type), false, int32(str*4), uintptr(off*4))

Note the change in type of the last argument.

There are a few more overloads with WithOffset in their name. There might be more functions that need this treatment - see READMEs of go-gl/glow and go-gl/gl for more details on checkptr and overloads.