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

Replace gbulb with a direct GTK main loop integration. #2548

Closed
wants to merge 6 commits into from

Conversation

freakboy3742
Copy link
Member

GBulb has a number of bugs logged against it; new versions of Python are particularly affected, due to changes in the underlying CPython asyncio libraries.

Gbulb is based on the idea of making the GTK event loop the main loop, and then trying to re-implement asyncio functionality using Glib primitives. This that the bulk of GBulb is a re-implementation of basic socket handling that ascynio has out-of-the-box.

This PR takes the other approach; try to integrate GTK into the asyncio event loop. GTKApplication.run() has a fairly simple implementation, so this seems like it could be a lot more reliable (and avoid any future issues of asyncio updates).

The one notable downside to this approach is that there is a sleep() involved; this means there is an inherent "delay" in responding to events. Reducing this delay causes the event loop to thrash, so there's a balance between the length of the delay and the responsiveness of the app.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@rmartin16
Copy link
Member

Gave this a quick spin; it ran successfully across multiple distros without issue.

As for responsiveness, I fired up an ancient Core 2 Duo laptop (since everything feels snappy on my daily driver) and the apps feel fine and quite usable...however...

(These observations were all made with the Tutorial2 example app.)

The idle CPU usage is noticeably higher...albeit probably tolerable. Using main, idle usage is practically undetectable; using this fork, it would hover around 1-2% just with the unfocused app window (keeping in mind, this is an 18 year old laptop).

Somewhat surprisingly (but perhaps reasonable), using either version and just running the mouse in circles on top of the app would bring the CPU usage to 10-15%.

This one may be more concerning....I noticed that the button animation when pressed was more difficult to trigger using this fork. I had to hold down the mouse button for longer to get the animation to actually trigger. This is less noticeable on my main machine but is still definitely a thing,

@freakboy3742
Copy link
Member Author

Yeah... that's what I was afraid of. My Linux setup is in a VM, so I wasn't sure if what I was seeing was actual CPU load, or some artefact of virtualisation - but it makes sense that it was actual load, and load that is dependent on GUI activity. The button animation is particularly problematic, and likely caused by the sleep - if the button doesn't respond immediately, then it won't need to draw a transient animation.

I'll keep poking and see if I can improve things.

@freakboy3742
Copy link
Member Author

I've now updated the code to not use a timeout-based solution, instead installing a Glib selector directly onto the asyncio event loop. The code is based on https://github.com/jhenstridge/asyncio-glib, which hasn't been updated for a while, but does appear to be based on public asyncio interfaces, rather than Gbulb's "re-implement the whole event loop", which will always be chasing changes in asyncio.

When it works, it seems to work a lot better; but there's a couple of weird oddities around the window close button and the quit menu item not being enabled in some of the tutorials.

@freakboy3742 freakboy3742 reopened this May 4, 2024
@freakboy3742
Copy link
Member Author

Another couple of updates, incorporating patches submitted to (but not merged into) the asyncio-glib library. These fixes seem to fix the issues with the close/exit functionality.

@freakboy3742
Copy link
Member Author

I'm going to close this PR for now. The most recent change seems to cause 100% CPU thrashing in the idle loop, so it's not really viable.

It also appears that PyGObject might (maybe) be gaining native asyncio support (see !189 and !500). I'd much rather move to an officially supported pygobject version of asyncio integration than maintain another ad-hoc implementation; #2550 is an experimental branch exploring that option.

@freakboy3742 freakboy3742 deleted the bulb-removal branch May 5, 2024 04:33
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