-
Notifications
You must be signed in to change notification settings - Fork 567
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
Adds an idle loop to the x11 backend, and uses it for repainting. #1072
Conversation
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.
I left some comments. Feel free to ignore whichever parts you want to ignore. I am not quite confused about your thread-safety requirements / use of threads.
Thanks for the comments! Regarding the threading: the intention (although it probably isn't correct right now) is to do the interaction with X11 only the main thread, but to support threads for other things. For example, Regarding the interaction with |
The issue is that libxcb and XCBConnection have an internal queue of pending events that were read from the socket, but not yet passed to the caller. Thus "an event is (potentially) pending" is not the same as "the socket is readable". Let's say that the main thread is "almost done" with a main loop iteration and is about to call Hopefully this makes the issue clearer.
Well, you could do what Qt5 is doing: You spawn a thread that does The issue is that " Does that make sense? Do you have any ideas how x11rb could work differently? |
Thanks, that makes a lot of sense. The only thing I would suggest to change in x11rb would be to have an explanation like the one you gave in the API docs. I've been kind of spoiled by rust over the last couple years, to the extent that if something As far as druid is concerned, it sounds to me like everything will be fine as long as we restrict the xcb connection to one thread. As far as I can tell, access to the connection is restricted to the |
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.
Sorry to let this sit around for so long.
Looks pretty good so already, got a few more questions and suggestions.
Co-authored-by: Leopold Luley <[email protected]>
Co-authored-by: Leopold Luley <[email protected]>
Co-authored-by: Leopold Luley <[email protected]>
Co-authored-by: Leopold Luley <[email protected]>
Ok, I think the review comments are addressed now. |
Fixes #932 and #935.