gl.Init called without a current context on Windows backend, and fails.

[dmitshur]

I gave the Windows build a try, and unfortunately we get a fatal error on start up initializing gl:
gl.Init: glActiveTexture

I believe the problem is that gl.Init must be called after MakeCurrentContext as wglGetProcAddress requires a bound context: When no current rendering context exists or the function fails, the return value is NULL.

Originally reported in google/gxui#86 (comment).

[dmitshur]

I see two possible ways of fixing this.

1. Either make ContextSwitcher mandatory on all platforms, and have it call gl.Init() at the right time.
2. Or create a gl.Init() for users to call, and require them to call it after they do window.MakeContextCurrent().

So it's a question of gl.Init() being implicit or explicit. Any preferences on this? /cc @slimsag @ben-clayton @errcw @elmindreda

Note that the ContextSwitcher must continue to exist in both solutions, because it's the only way the context pointer can be passed from GLFW that creates the WebGL context to the gl bindings. I think on desktop that pointer is passed via thread local storage or something, therefore there's no direct connection in code (I am unsure about this, please correct if wrong).

So with second choice, user code to initialize a program would be:

err := glfw.Init(gl.ContextSwitcher)
if err != nil {
    log.Fatalln("glfw.Init:", err)
}
defer glfw.Terminate()

window, err := glfw.CreateWindow(640, 480, "Test", nil, nil)
if err != nil {
    log.Fatalln(err)
}

window.MakeContextCurrent()
if err := gl.Init(); err != nil {
    log.Fatalln("Failed to initialize gl bindings: %v", err)
}

...

With the first choice, user doesn't need to call gl.Init() as it would happen internally in window.MakeContextCurrent().

I also wonder, if a context is detached, window closed, then a new window is opened, new context is created, then gl.Init() would need to be called again, right? I think so. Definitely in the case of WebGL, and quite likely on Windows too, especially if a different video card ends up being used, etc.

[dmitshur]

I am leaning towards making gl.Init implicit (called inside MakeContextCurrent via the ContextSwitcher mechanism). Having gl.Init is just a limitation of the GL bindings, it's not a part of the OpenGL spec. If you're coming from another language, this is probably new and you may mess it up. So automating its invocation is probably better, right?

[ben-clayton]

The part I really don't like with the implicit approach (my suggested fix) is that there's no contract on the type handed to ContextSwitcher.MakeContextCurrent.
I understand that the JS context object needs to be handed between the packages, but it feels really kludgy.

Have you considered just merging the two goxjs packages?

[dmitshur]

On the other hand, "context switching" and "initializing gl bindings" do feel like two slightly separate concerns, maybe. I am not completely confident on when gl.Init needs to be called. Is it after every time you switch contexts? Or only after the first time you've created a window? Making it implicit would require me to get this right. Making it explicit puts that responsibility on the user. I'd rather do the latter unless I have 100% certainty I can get it right all the time, but I don't have that kind of confidence right now.

The part I really don't like with the implicit approach (my suggested fix) is that there's no contract on the type handed to ContextSwitcher.MakeContextCurrent.
I understand that the JS context object needs to be handed between the packages, but it feels really kludgy.

I'm actually working on making that contract more typed than just a single multi-purpose interface{}, so this can be improved. In fact, working with this is very helpful to me in refining and better understanding the ContextSwitcher approach, so it should improve as a result.

Have you considered just merging the two goxjs packages?

When I started out, glfw actually imported gl and performed the context creation inside. But the goal as suggested by @crawshaw in google/gxui#49 (comment):

your goglfw looks a lot like what I was thinking of for gxwt. One extra ideal I'd try to reach for: no external references to any gl packages. That way it could be used by other drivers (for example, html5 canvas, or one of the many GL replacement languages that are appearing).

That lead me to trying to decouple them, and this interface{} bridge was my first attempt.

I wanted the exact glfw and gl packages not to import each other, that way it's possible to swap them out, have different versions, and there could be more than one package that provides gl bindings, etc.

[ben-clayton]

I am also not completely confident on when gl.Init needs to be called. Is it after every time you switch contexts? Or only after the first time you've created a window?

That's a good question. Really, the go package should have a function table per GL context - see the wglGetProcAddress Remarks section, but I think it only has a single set of globals. In which case, I'd also side with explicit over implicit.

[dmitshur]

All right, let me try bringing back gl.Init then and see how well that works out. My attempt to make it implicit so far is messy and I don't like it.

[dmitshur]

Actually, it became quite a bit better when I decided to reduce the responsibility of ContextSwitcher (which would need to be renamed) from "switch context" and "initialize gl bindings" to just the latter, and left the task of context switching inside glfw.

That way it can be a bit cleaner, see https://gist.github.com/shurcooL/bb5eb9d0bf14ec71fdfc.

I'll still try the approach of having gl.Init explicit for users to call up next.

[dmitshur]

Ok, I think I'll stay with keeping gl.Init internal in the gl/glfw code. My reasoning is as follows. I think my current solution is simple and it works well when there's a single window, making its context current and running.

I'd like to start off with this simpler solution that does not require API additions on the user side (i.e., they don't need to start adding gl.Init where they previously didn't). I'd like to use the extra time to think about this whole situation and come up with better ideas.

If there are any issues with the current fix, then we can revisit this. I think by the time there's a usage case where there are multiple windows, multiple contexts being made current and running gl.Init every time becomes expensive (80-60 ms on my machine) and it makes sense to cache the results or so, then we can have a better idea how to handle that.

Since I have absolutely no code that uses multiple windows/contexts, I don't want to try to solve a problem which I cannot test or see how well it works.

So I'll start simple, and move forward from there.

Really, the go package should have a function table per GL context - see the wglGetProcAddress Remarks section, but I think it only has a single set of globals.

That's a great point, and I agree that'd be a more correct, efficient long term solution. But I don't want to rush into it right away, I'd rather make it an optional future enhancement, driven by a testable use case.

[slimsag]

I've been following along, perhaps a bit silently -- but @shurcooL I highly agree with your last comment and think it is the right way to approach this.

[dmitshur]

This issue is now resolved via goxjs/glfw@c2e5c56 and a52900d.