Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document how to pass a callback contexts to the callbacks #815

Open
gnzlbg opened this issue Jul 28, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@gnzlbg
Copy link

commented Jul 28, 2016

Some callbacks require state. It is extremely common practice that C libraries taking callbacks allow the user to forward a callback context to the callbacks in order to avoid relying on global state.

For whatever reason GLFW doesn't directly allow/encourage this. The documentation, and in particular the quickstart, leaves one wondering whether it is possible, and the FAQ asserts that C++ objects methods cannot be used as callbacks (since they require state), which is "incorrect" for most callbacks (they can be used, but the objects have to be available within the callback). Recommending users to "use global state" is a smell to many. I think the docs should rather hint that "that's the easy way, but that passing a context object is possible to most callbacks". And then point users to the techniques to allow this in C and C++.

I don't know what is the best way to pass contexts to callbacks in GLFW. The only way I know exploits the fact that most callbacks take a GLFWWindow object, so one just needs to:

  • create a new type that contains a GLFWWindow such that the address of the type always matches the address of the GLFWWindow it contains (e.g. via inheritance in C++, or by having GLFWWindow as the first data-member of the type in C),
  • create an object of that type and initialize the GLFWWindow as usual (e.g. with glfwCreateWindow),
  • pass this object to GLFW as a GLFWWindow (e.g. by upcasting, taking the address of the window data member, ...)
  • from the callbacks that take a GLFWWindow (that is, most of them), down cast it back to your type (using dynamic_cast, reinterpret_cast, a C cast...) to access the content of your context (don't screw up the casting or you'll get UB).

To use a C++ object method as callback one just needs to store how to call the method in the callback context (e.g. by storing the object itself there, storing a std::function, ...), and then call it through the callback context from the callbacks.

From the documentation it seem as if GLFW wasn't designed at all with callback contexts in mind (in particular since some callbacks don't get passed a GLFWWindow), but in practice it supports it, and since it is what most users would expect one might just document how to do it properly.

@tombsar

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2016

Edit: Just to be clear; I'm not suggesting that the documentation could not be improved!

IMO the FAQ is perfectly correct: you cannot use a C++ member function as a callback. As in, you cannot pass the address of an object's member function to any of glfw's setcallback functions and expect it to work, it has to be a regular/static function.

The best way to achieve the mapping between glfw and C++ objects, in my opinion, is the way suggested by the FAQ: through glfwSetWindowUserPointer and glfwGetWindowUserPointer. A simple static wrapper function can then be used to call the relevant member function on the pointer with no additional global state required. http://www.glfw.org/docs/latest/window_guide.html#window_userptr.

I don't see where in the documentation it recommends to use global state for this. Could you link to the part you believe is misleading/incorrect?

I am a bit confused by your proposed method. In particular, how do you "pass this object to GLFW as a GLFWWindow"? glfw creates the struct internally, and only returns to you a pointer from glfwCreateWindow.

Any callbacks that do not have a window pointer are either not directly related to a window, or an oversight that should be corrected in a future release (if there is not an issue to track these, one should should definitely be added).

@gnzlbg

This comment has been minimized.

Copy link
Author

commented Jul 28, 2016

The best way to achieve the mapping between glfw and C++ objects, in my opinion, is the way suggested by the FAQ: through glfwSetWindowUserPointer and glfwGetWindowUserPointer. A simple static wrapper function can then be used to call the relevant member function on the pointer with no additional global state required. http://www.glfw.org/docs/latest/window_guide.html#window_userptr.

I wouldn't recommend adding a static member function (which might not be possible) and passing a pointer to the object directly (which doesn't work directly if you need two objects). I would just recommend passing a real context object, and accessing everything through there.

To improve the docs I would point this out in the quickstart guide when the first callback is encountered. That is, point out that if you want to access a callback context you can use this and that function to set it and get it from a window.

In the FAQ instead of recommending using a static member function, I'd either recommend casting the address of the object to its type (which allows you calling the member function directly without needing a static function), or just using a context object if one needs to call multiple member functions. It might be worth adding to the FAQ a section about how to pass a callback context that points to the Window User Pointer section.

In the Window User Pointer section I would add: "This can be used for any purpose you need (like passing a callback context)", and show an example.

@tombsar

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2016

instead of recommending using a static member function, I'd either recommend casting the address of the object to its type (which allows you calling the member function directly without needing a static function), or just using a context object if one needs to call multiple member functions

Do you have any code examples of your approach you could share? Without a regular C or static function for glfw to call in the first place, I am struggling to understand how it would work.

@gnzlbg

This comment has been minimized.

Copy link
Author

commented Jul 28, 2016

This has only been compiled in my head but it shows how it is done:

#include <iostream>
#include <GLFW/glfw3.h>

// Given two types
struct Foo { void foo() { std::cout << "foo" << std::endl; } };
struct Bar { void bar() { std::cout << "bar" << std::endl; } };

// A callback context:
struct callback_context {
  Foo foo;
  Bar bar;
};

// And a function to get the context from a window:
callback_context* get_context(GLFwindow* w) { 
  return static_cast<callback_context*>(glfwGetWindowUserPointer(w)); 
}

int main() {
  // Construct a callback context, get window, and set user pointer:
  callback_context cbc;
  auto* window = glfwCreateWindow(640, 480, "My Title", NULL, NULL);
  glfwSetWindowUserPointer(window, &cbc);

  // Define some callback, get the context, and call methods directly: 
  auto wcc = [](GLFWwindow* w) {
    callback_context* cbc_ptr = get_context(w);
    cbc_ptr->foo.foo(); 
    cbc_ptr->bar.bar();
  };

  // Set the callback and enjoy: 
  glfwSetWindowCloseCallback(wcc);
  while (!glfwWindowShouldClose(w)) {
    glfwPollEvents();  
    // on window close "foo\nbar\n" is printed to stdout.
  }
  // destroy windows and close
  return 0;
}

EDIT: this basically allows you to do anything and I think is a better idiom overall than suggesting to add static member functions to types.

@tombsar

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2016

Am I going crazy, or is that a completely different approach from the one in your original post?

Anyway, that example is pretty much identical to the method I was proposing, except using a lambda instead of a regular function (note I also don't recommend adding a static member to any types). It should be easy enough to get this written into the documentation.

@elmindreda

This comment has been minimized.

Copy link
Member

commented Aug 2, 2016

I'll update the documentation with an example and more mentions of the window user pointer.

@elmindreda elmindreda added this to the 3.3 milestone Aug 2, 2016

@elmindreda elmindreda self-assigned this Aug 2, 2016

@elmindreda elmindreda removed their assignment May 2, 2017

@babaliaris

This comment has been minimized.

Copy link

commented Jul 12, 2018

Callbacks are really bad in glfw and it's to easy to fix them!! Just update callbacks to work like this:

glfwSetFramebufferSizeCallback(window, (void *)MyWindowObject, OnWindowResize)

void OnWindowResize(GLFWwindow *window, void *object, int width, int height)
{
    WindowClass *w = (WindowClass *)object;

    w->SetWindowWidth(width);
    w->SetWindowHeight(height);
}

Please we really need callbacks that work like that!!! Now we have to make all the variables Global in order to update them inside callback functions!!!!!
@gnzlbg

This comment has been minimized.

Copy link
Author

commented Jul 12, 2018

Please we really need callbacks that work like that!!! Now we have to make all the variables Global in order to update them inside callback functions!!!!!

This is wrong, you don't have to do that. See my comment: #815 (comment)

@elmindreda elmindreda modified the milestones: 3.3, 3.4 Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.