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

move clipboard updates to a focus callback, perfs enh + stability fix #2334

Conversation

bpinsard
Copy link
Contributor

We had a curious issue for years where eye or world window would freeze on our production setup only. However moving the mouse over the frozen window would usually unfreeze it. The main issue was data loss, because the software was not processing camera frames and the network buffers would overflow.

After inspecting the stack when that freezing occurs, it seems that calls to glfw.get_clipboard_string sometimes hangs.
This might be specific interactions with our software / hardware, or that in our config (FPS/ screen framerate) the clipboard was concurrently written/read by eye and world windows, causing the hangs.

In the meantime, profiling would show that a small but non-negligible portion of the CPU cycles would be spent querying the clipboard.

I here moved that call to the focus callback: the logic being, if the OS clipboard (not the pyglui one that seems separate IIUC) was modified, it is only happening if the focus was given to another window, so we can retrieve it only when the pupil window get focus again.
That way, the clipboard is not probed at the screen framerate and likelyhood of hangs is largely dimished. It also separates the processing loop from call to OS clipboard that seems likely to disrupt RT reqs of pupil.

Let me know if that makes any sense and if you want me to make changes.

@bpinsard bpinsard force-pushed the fix/glfw_clipboard_perf_stability branch from e113509 to d17b571 Compare September 20, 2023 14:38
@bpinsard
Copy link
Contributor Author

Just realized that it had been documented in #2312 recently.
So this PR should fix #2312

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

Successfully merging this pull request may close these issues.

2 participants