go-gl / gl

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

gl.DebugMessageCallback crash on OS X 10.11 #40

Closed gdm85 closed 8 years ago

gdm85 commented 8 years ago

As thoroughly documented in gdm85/wolfengo#1, the function gl.DebugMessageCallback cannot be used on Mac OS X 10.11 and causes always a crash with SIGSEGV.

I have no idea why this is happening, although it could be related to the call to GoStr(message) in there.

Alternatively, it could be possible that the memory access is denied because of a thread mismatch issue? e.g. the Go debug function is not locked on the same thread as the GL thread, but I doubt about this.

errcw commented 8 years ago

For the record I can reproduce this crash using the cube example. Here is the version of the Cube example I used: https://gist.github.com/errcw/e3311a0ed1a1c0113a92.

errcw commented 8 years ago

Oh, of course, sadly this is working as intended. If you check the supported extensions on OS X you'll notice that none of glDebugCallback[ARB,KHR] are supported. This is an unfortunate result of OS X continuing to only support through OpenGL 4.1 (with a few 4.2 extensions thrown in for El Cap).

If you try this same code on an up-to-date Windows or Linux box it works. Closing as WAI, sorry.

gdm85 commented 8 years ago

@errcw shouldn't this be documented in go-gl (is it already?) and/or not be built on darwin platforms? I can see this being done with a build tag for example.

errcw commented 8 years ago

As I understand it the prebuilt packages contain all extensions and because GL_KHR_debug defines glDebugMessageCallback the OpenGL 4.1 package (somewhat misleadingly) includes this function.

We could probably do a better job communicating what is extension vs. core in the package?

dmitshur commented 8 years ago

I can see this being done with a build tag for example.

A problem with using build tags to remove things not supported by other platforms is that it becomes impossible to write cross-platform code that may or may not use a feature dynamically at runtime depending on the platform. For example:

if runtime.GOOS != "darwin" {
    gl.DebugMessage(...)
}

Would fail to compile on OS X if that func isn't included for darwin build tag.

gdm85 commented 8 years ago

@errcw @shurcooL I get your points. In that case, is there a reliable way to query if a feature is available? Ideally this should come through a go-gl interface or some underlying OpenGL interface.

errcw commented 8 years ago

The admittedly awkward way to do this is call glGetString(GL_EXTENSIONS) and look for GL_KHR_debug.

gdm85 commented 8 years ago

Ok, that's fair enough.. :) I am going to do it that way. Would you be willing to accept a patch that adds this check directly in go-gl? It looks to me like a detail the library could take care of (and return an error accordingly)

errcw commented 8 years ago

What API did you have in mind exactly?

dmitshur commented 8 years ago

Be aware that go-gl/gl is a very low level API, it's just a wrapper around the C calls. I'm not sure we can afford to add verification in the main code path, as that will always be there at runtime.

gdm85 commented 8 years ago

@errcw @shurcooL I understand it is a low-level wrapper, however where "reflection" features (like looking up available OpenGL extensions) are available, one would think that it is responsibility of this middleware to prevent SIGSEGV, if not just because the information about which C functions are call-able is already material covered by this wrapper.

I think - questionable opinion of course - that generally a Go package that bridges Go code with C code, should do that in the safest way and disallowing what is invalid C calls as much as possible.

What I had in mind is something like this (please forgive the untested snippet):

diff --git a/all-core/gl/package.go b/all-core/gl/package.go
index cff3b2d..b24680e 100644
--- a/all-core/gl/package.go
+++ b/all-core/gl/package.go
@@ -9379,10 +9379,18 @@ func CullFace(mode uint32) {
        C.glowCullFace(gpCullFace, (C.GLenum)(mode))
 }

+func hasDebugExtension() {
+       return strings.Contains(GetString(EXTENSIONS), "GL_KHR_debug")
+}
+
 // specify a callback to receive debugging messages from the GL
-func DebugMessageCallback(callback DebugProc, userParam unsafe.Pointer) {
+func DebugMessageCallback(callback DebugProc, userParam unsafe.Pointer) error {
+       if !hasDebugExtension() {
+               return errors.New("debug callback extensions not available")
+       }
        userDebugCallback = callback
        C.glowDebugMessageCallback(gpDebugMessageCallback, (C.GLDEBUGPROC)(unsafe.Pointer(&callback)), userParam)
+       return nil
 }
 func DebugMessageCallbackARB(callback DebugProc, userParam unsafe.Pointer) {
        userDebugCallback = callback

If I a missing it big time and this is totally unfeasible/inconsistent, please tell me :)

Edit: I am well aware that in the (wild) implementations world of OpenGL, it could also happen that some of them has the actual feature but it's not reported as such in the extensions, and what I propose above would definitely make those inaccessible

errcw commented 8 years ago

I'd be very hesitant to introduce a potentially heavyweight runtime check for every function call. Moreover some functions are shared across extensions so it'd be potentially tricky to determine which set of extensions to query.

Another approach that on the surface seems great but falls over in practice is to check during Init whether the loaded function pointer is non-null. However on Linux glXGetProcAddress never returns null so the null checks are not particularly useful.

Given that go-gl is supposed to be nothing more than a thin wrapper over the OpenGL API my preference would be to simply leave the existing behavior, as awkward as it is.

gdm85 commented 8 years ago

I understand; my argument was that a feature to prevent crashes is too thin by itself to justify creating a middleware on top of go-gl, thus the burden will always be pushed on Go applications to defend from undesired panics. Go libraries shall - in my opinion - behave - as much as possible - as pure Go libraries also when they are encapsulating unsafe code.

Edit: is it worth mentioning anywhere in the documentation that some calls can cause panic? I don't want to bother you guys and I get that it's a complex problem (also with the 1.6 introduced extra checks) and that go-gl presents itself truly as a low-level wrapper already.

dmitshur commented 8 years ago

I'll just quickly mention, I think the ultimate solution is similar to what Vulkan does, as I understand it.

At a high level, it provides 2 implementations. One is a validation/debug implementation, that performs many runtime checks (bounds checking, etc.) to ensure all APIs are used correctly and pre-conditions are met. So it's not hard or expensive to add more checks.

Then, it offers the production implementation that strips all pre-condition checks. It's assumed that if your code didn't crash with the validation implementation, then running all those checks isn't needed during normal runs.

I'm not sure if go-gl/gl can get there in the short term, but I just wanted to describe the above. It solves the problem where it's desireable to both do more validation and also be more perfomant; you can have both, just in 2 implementations.

gdm85 commented 8 years ago

@shurcooL yes that would probably be the best of both worlds, and probably match what a professional debugging/tracing OpenGL implementation should allow.

As for the specific issue (OpenGL 2.1 on Mac OS X), at this point I wonder if the platform is at all certified with some testsuite to be 2.1 compatible?

dmitshur commented 8 years ago

As for the specific issue (OpenGL 2.1 on Mac OS X), at this point I wonder if the platform is at all certified with some testsuite to be 2.1 compatible?

TL;DR: Pretty much all modern Macs made within last 5~ years support OpenGL 2.1 easily.

OpenGL version support on OS X depends on multiple factors:

That said, OpenGL 2.1 is quite old and is pretty much supported by all modern Macs within the last few years.

OpenGL 3.2 support has been around since Mac OS X 10.7 Lion I believe (on capable hardware, which I think is almost all Macs that run that version), and more recently 4.1 with OS X 10.9 Mavericks (again, on capable hardware only).

For example, here's a screenshot of OpenGL Extensions Viewer on my Late 2011 15" MBP with Intel HD Graphics 3000 active, Core profile:

image

Here's with AMD Radeon HD 6770M active, Core profile:

image

This is running the latest OS X 10.11.3 El Capitan, so software caps out at 4.1. With AMD graphics, hardware provides 4.1, but the Intel 3000 can only provide 3.3 support.

gdm85 commented 8 years ago

@shurcooL many thanks for checking this out for me! I am not using any 4.x feature in WolfenGo, although there is still this issue: gdm85/wolfengo#4

At this point I guess it's a bug in the Go code, although the reason why it does happen only on Macs is a mystery...