Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

add return values to global callback setters #27

Merged
merged 2 commits into from
Jun 17, 2020
Merged

add return values to global callback setters #27

merged 2 commits into from
Jun 17, 2020

Conversation

nlbuescher
Copy link
Collaborator

Glfw.setErrorCallback, Glfw.setJoystickCallback, and Glfw.setMonitorCallback (the last one is needed for imgui-docking) were not returning the previous callbacks. I missed those with the last fix, and this time I double checked that all functions named set…Callback have a return type.

@Dominaezzz
Copy link
Owner

Immutability (freezing) and threading on the native target might need a bit more thought.

@nlbuescher
Copy link
Collaborator Author

Probably. @ThreadLocal is probably not the best solution (that was already there). I usually put all the mutable properties top-level and have references to them in the object if they need to be public.

@nlbuescher
Copy link
Collaborator Author

A thought: (1) is it possible to detect if running on the main thread in native, and (2) should an exception be thrown if it's possible and a GLFW call is not running on main?

@Dominaezzz
Copy link
Owner

About (1), assuming main thread means, the thread in which GLFW is initialised, yeah there are a couple ways. You can store Worker.current and compare against future Worker.current calls or you can have a global @Threadlocal variable and do the same thing with it's hashCode() (as supposed to Worker.current).

About (2), I'm a bit sceptical about it but it's easier than dealing with frozen callbacks.

@nlbuescher
Copy link
Collaborator Author

Personally, I would circumvent freezing the callbacks by storing them top-level (above the Glfw object). I have a suspicion that that may cause memory leaks, but honestly, in this specific instance, that doesn't matter since the Glfw object is intended to be in scope globally until the program exits, and functions more as a namespace than anything else. I'd add cleanup code to Glfw.terminate (eg, set callbacks to null to release the references for GC), and call it a day

Another point to consider is how multithreading with GLFW is handled in a C/C++ context.

Overall though the native handling of multithreading is a necessary discussion, but I'm not sure it applies to this pull-request, since that wasn't one of the things I changed.

@Dominaezzz
Copy link
Owner

Oh, I just noticed that the callbacks are already being stored. I was reading the JVM version when I was thinking about it.

@Dominaezzz Dominaezzz merged commit 2094fba into Dominaezzz:master Jun 17, 2020
@Dominaezzz
Copy link
Owner

Thanks for the PR!

@Dominaezzz
Copy link
Owner

Published in 0.1.9-dev-10

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants