Return previous callbacks

[pwaller]

My motivation here is to make it possible to have reusable
pieces of code which you can plug in just by feeding it the
*glfw.Window. The reusable pieces would be called last and
they could record the previous callback and call it, forming
a callback chain.

The sort of thing you could do is implement a camera system
where you only need one function call to specify the window
and another to get the transformation matrix.

Strictly speaking, this is an API-change. However, I haven't
managed to find any examples that don't compile correctly after
this change yet, so I'm not sure if it counts as an
API-breaking-change.

@tapir @shurcooL

[dmitshur]

My motivation here is to make it possible to have reusable
pieces of code which you can plug in just by feeding it the
*glfw.Window. The reusable pieces would be called last and
they could record the previous callback and call it, forming
a callback chain.

Can you post such an example? That would be helpful.

[dmitshur]

What you wrote in the PR description makes me think of http://commandcenter.blogspot.nl/2014/01/self-referential-functions-and-design.html (some examples of similar designs can be seen in pipe, bluemonday packages).

However, the code changes seem to suggest it's something else.

[dmitshur]

Also, I'm looking at GLFW docs and I see, as I remembered, all of its Set*Callback funcs return the previous callback. Was there a reason glfw3 API did not mirror that?

[pwaller]

@shurcooL I had the same thought and started off trying to figure out how to get out of the glfw3 API the previous function. The problem is that it is impossible to have glfw call an arbitrary go function. That's why there is just one function which is passed in.

This implementation is a straightforward way of getting around that limitation whilst preserving the correct semantics.

This is a window wrapper which is able to "drop in" to existing code and provides F11 for toggling fullscreen - to do this you need to create a new window and copy over all of the callbacks, which isn't possible without this pull request.

Another example is this (not yet functioning) arcball implementation, which provides a nice interface for rotating objects using the mouse. Its usage looks something like this from the main function:

    arcball := NewArcball(window)

    for !window.ShouldClose() {

        gl.Clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT)

        gl.MatrixMode(gl.MODELVIEW)
        gl.LoadIdentity()

        i := [16]float64(ident.Mul(arcball.Rotation()).Normalize().Mat4())
        gl.MultMatrixd(&i)

        cube.Render(gl.LINES)

        window.SwapBuffers()
        glfw.PollEvents()
    }

The arcball implementation is automatically aware of window size changes without needing to be told explicitly, since it can replace the original resize callback with its own and call the original.

[dmitshur]

The arcball implementation is automatically aware of window size changes without needing to be told explicitly, since it can replace the original resize callback with its own and call the original.

That's really cool.

Okay, I don't think there's any doubt that this is a good API change. In fact, I would consider the previous lack of a return value from Set*Callback funcs an outright bug, since GLFW itself does return the callbacks in Set*Callback funcs:

Returns
The previously set callback, or NULL if no callback was set or an error occurred.

Now, I have a few comments on the current PR and how it achieves that goal.

[dmitshur]

I have a few minor code style comments, but before I get to that, I wanted to address a bigger point.

This PR is currently set to merge to master. I propose instead we make it a PR to merge into devel_glfw3.1 branch.

• The basic idea is that I think new feature development should be done against devel_glfw3.1, and the current master should be touched for critical bug fixes.
• devel_glfw3.1 has had a lot of changes in it by now, and rebasing it on top of this PR, if merged into master, will be some work. Also, there have been changes and new callbacks added in GLFW 3.1, so we'd need a 2nd PR for devel_glfw3.1 anyway.
• I understand you're probably using glfw3 on master yourself, and you want to use this feature right away. But that's easy to work around; I have a fork of glfw3 which simply mirrors the devel_glfw3.1 branch but makes it master, so it's go-gettable: github.com/shurcooL/glfw3. Feel absolutely free to use it yourself, but note you'll need GLFW compiled from master branch rather than 3.0.4. (Edit: I've just updated it to be in sync.)

What do you think of that?

[pwaller]

@shurcooL sounds OK to me. Wishing I had noticed the devel branch before. When are we planning to make it master?

[pwaller]

Ah, I see there is a PR for that.

[dmitshur]

The very moment GLFW 3.1 is released. Because it has GLFW 3.1-only feature support, and (very nice) breaking API improvements.

It also closes just about every open issue in this repo. You can't imagine how I'm looking forward to GLFW 3.1 to come out...

[pwaller]

How quickly is glfw 3.1 likely to find its way into distributions?

[dmitshur]

If it were up to me, on the very same day it's officially released. Otherwise, I don't know. I always compile GLFW from source myself.

[dmitshur]

I would suggest using named result parameters to make the code a little shorter, and to have better docs.

func SetErrorCallback(cbfun ErrorCallback) (previous ErrorCallback) {

See #51 and #52 for example.

Edit: Actually, the code won't be any shorter in this case, but I'd still do it for docs.

[pwaller]

Addressed in aace514

[pwaller]

I've rebased over on #76, I'll make fixes there.

[dmitshur]

It's a little inconsistent how there's a blank new line here and elsewhere, but not for SetScrollCallback and SetCursorPositionCallback.

[pwaller]

Addressed in 08dd40e