go-gl / gl

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

Cross-version initialization #85

Closed kernle32dll closed 6 years ago

kernle32dll commented 6 years ago

I noticed something odd, as I was plucking some libraries together for a small rendering test.

Apparently, OpenGL is not initialized cross-version. What this effectively means is that e.g. a main program which initializes OpenGL 3.3 and a library that uses (or rather - assumes) OpenGL 3.2, will not work together. See the attached code for an fail-fast example:

package main

import (
    gl32 "github.com/go-gl/gl/v3.2-core/gl"
    _ "github.com/go-gl/gl/v3.3-core/gl"
    "github.com/go-gl/glfw/v3.2/glfw"
)

func init() {
    runtime.LockOSThread()
}

func main() {
    // Ensure valid context
    if err := glfw.Init(); err != nil {
        closer.Fatalln(err)
    }
    win, err := glfw.CreateWindow(800, 600, "Demo", nil, nil)
    if err != nil {
        closer.Fatalln(err)
    }
    win.MakeContextCurrent()

    if err := gl32.Init(); err != nil {
        closer.Fatalln("opengl: init failed:", err)
    }
}

This will fail to compile under Ubuntu 16.04 with the following message:

# command-line-arguments
/usr/local/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/tmp/go-link-284705261/000001.o: In function `GlowGetProcAddress':
/home/bgerda/go/src/github.com/go-gl/gl/v3.3-core/gl/procaddr.go:54: multiple definition of `GlowGetProcAddress'
/tmp/go-link-284705261/000000.o:/home/bgerda/go/src/github.com/go-gl/gl/v3.2-core/gl/procaddr.go:54: first defined here
collect2: error: ld returned 1 exit status

I think I remember that my code did compile under Windows 10, but failed at runtime (I might mistaken though - will check later today. Something about multiple definitions, too).

I was under the assumption that this should work, as right now it is impossible to combine libraries which use different versions of go-gl (nuklear e.g. uses 3.2 - which clashes with my code which is based upon 3.3).

errcw commented 6 years ago

Ah, this is indeed a bug insofar as each binding generated by Glow declares its own procaddr file with conflicting function definitions. Though indeed I suspect this problem is not limited to procaddr but is shared by any C definitions that are shared across bindings. The easy solution would be to include a random uniquifier in every C function name, though we'd have to update Glow and regenerate all the bindings. (Fair warning, while I'd be happy to review a contribution, I don't have the personal time to make the fix in the near future.)

kernle32dll commented 6 years ago

@errcw OK I suspected as much. I will see if I can cook up the suggested fix in glow. However, this will only be the first step. As detailed in the initial post - I did run into an issue on windows where the code did compile but crashed at runtime. This was due to the case mentioned in the initial post: Initializing e.g. GL 3.3, but then trying to use GL 3.2 in Nuklear. The probable source of the problem is that bindings for 3.2 are not initialized (Edit: It is. I tried to access OpenGL methods without initializing first - it is the same error).

It remains to be seen if it would be possible (while very nasty) to initialize both GL 3.2 and 3.3 after the mentioned fix in glow. On the bright side - this could allow changing the GL implementation at runtime (e.g. different rendering backends of sorts).

kernle32dll commented 6 years ago

@errcw Here are the PRs. It was trivial to fix with cues taken from debug.go / debug.tmpl

glow: https://github.com/go-gl/glow/pull/87 bindings: https://github.com/go-gl/gl/pull/86

I verified that this fixes both the compilation problem (code in initial post), and allows to initialize different versions of OpenGL at the same time (e.g. 3.2 and 3.3). I did not test if initialization across major versions (e.g. 3.3 and 4.4) works, but it probably does.

errcw commented 6 years ago

Thanks, I've merged your PRs. Thanks also for your patience, I apologize I did not have the opportunity to review your changes earlier this week. I'm glad that your use case is now unblocked!

In retrospect it occurs to me that an equivalent fix might have been to mark the C functions as static (which, I realize now, is why the GL definitions do not experience the same conflict) but your approach is just as good.

dmitshur commented 6 years ago

In retrospect it occurs to me that an equivalent fix might have been to mark the C functions as static (which, I realize now, is why the GL definitions do not experience the same conflict) but your approach is just as good.

It sounds like that would be a cleaner fix, wouldn't it? If so, want me to file a cleanup issue for that task? In case one of us, or a contributor wants to make the small improvement.

errcw commented 6 years ago

Yes, might as well file an issue to track the cleanup, thanks.