-
Notifications
You must be signed in to change notification settings - Fork 246
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
Add callback functionality for input events #293
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! Can you explain a bit of your use-case/thought process behind this change please? I'm interested to see how this differs from using the existing pixel.Window input functions. |
I was thinking this feature would be nice but came to the conclusion most things i would do with callbacks i could just do with checking an input loop. However, the best idea I had was for UI elements (checking every mouse click if you're over a button on the screen, etc.) |
The intention was to provide a way to make use of the callback methods already existing in glfw. Instead of exposing them raw, I wrapped them so that glfw elements won't leak into the pixel library. One of the main benefits with this change, as I see it, is that you can avoid using an input loop for something that is event-driven "under-the-hood". |
}) | ||
|
||
w.window.SetCharCallback(func(_ *glfw.Window, r rune) { | ||
w.tempInp.typed += string(r) | ||
for _, cbfun := range w.callbacks.charCallbacks { | ||
go cbfun(r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason you are doing this asynchronously with go
instead of calling them sequentially? I feel like this could be an issue if callbacks overlap with each other. Not sure if that's a valid fear or not however. Would like to hearinput from you and @dusk125 on this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Say you register multiple callbacks for an event. Calling the callbacks asynchronously was done as to not keep one callback waiting for the previous one to finish. However, I see the reason for calling them sequentially, so that might be something that we could change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps maybe a waitgroup then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preference would be to call them sequentially, and leave it to the handler to do what it needs to async. Especially handling typing, things could get out of order which would be annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe have async as a bool option somehow?
So the thing I'm struggling with here is that I don't know if this actually buys you anything. GLFW might be able to send events async, but OpenGL is still single threaded and basically all calls in Pixel that go touch OpenGL are forcibly done in the main thread. The thing I'm not sure about (given my inexperience with GLFW) is if the callbacks are actually done async or if they're also called single threaded-ly; maybe you can speak to that? In Pixel, we call glfw.PollEvents (on the main thread) and then update our internal structure. |
@trondhumbor what's your use case for this feature? i'm curious |
Related to https://github.com/dusk125/pixelui/issues ? If so, would be nice to get this merged so the others can also be. |
Thanks for reminding me to go through the PixelUI issues :D All of those issues were waiting on PRs that have now been accepted into Pixel (they've now been closed); that is to say, this isn't related to the issues I was having in PixelUI. |
PixelUI is fantastic. Great addition to fantastic Pixel. Started to use PixelUI myself and noticed it needs to upgrade the underlying imgui-go (made an issue for it) Anyway, this is about Pixel so I hope this PR will be merged soon too. |
Is this PR still relevant? |
Allows adding custom callback functions, which will be called when glfw fires an input event.