-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor to go async #41
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g _rc_canvas_group to None
Korijn
reviewed
Dec 6, 2024
I have to say you did something really special here! |
Korijn
approved these changes
Dec 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds support for async callback functions, and support for trio. Major refactoring was needed for this.
Introduction
I was adding support for async event handlers, but that turned into a complete overhaul, moving from a system based on timers and callbacks to async functions. I guess that's what async does. Let's also add a loop for trio, I thought casually 😅 . But this meant that backend canvases and loops must be more loosely coupled, which opened up a rabbit-hole way deeper than I imagined.
To be honest this process was quite a ride, and sometimes I wondered what the hell I was doing. But the end result is pretty darn nice, in my humble opinion. The API has barely changed, but opens up possibilities that can have big implications.
How this affects the API
async
beforedef your_event_handler(event):
.RenderCanvas.select_loop(loop)
at the top of your code to run on an alternative loop.rendercanvas.trio.loop
object..run_async()
method so the loop can be started in a trio-nesque or asyncio-ish way.loop.call_later()
, which works with all loop-backends.What async stuff can people use?
Users writing backend-agnostic code can use
sleep
andEvent
. These can be imported fromrendercanvas.utils.asyncs
. Or they can usesniffio
and then import it from eithertrio
,asyncio
orrendercanvas._async_adapter
.Users writing code explicitly for asyncio can simpy use all the asyncio stuff. Same for Trio. If a user is using qt, they're probably not interested in async functions.
How this affects the internal code
Changes
sniffio
(a tiny pure-python lib).BaseRenderCanvas.select_loop()
class method.BaseTimer
and backend timer classes.BaseCanvasGroup
and subclasses for each backend.EventEmitter
is closed when done, clearing the handlers, for improved gc._loop.py
is completely refactored, scheduler moved out into_scheduler.py
call_later
mechanic.Considered alternative approaches
A bit detailed, mostly for the record, you can probably skip this part.
Only process events asynchronously
I started with the idea, that when the scheduler process events, it will will do so in a co-routine that is then scheduled to run in an event-loop. At the end of the co-routine it would continue the scheduling-loop using a callback.
asyncio.get_current_loop().create_task(..)
.asyncio.new_event_loop().run_until_complete(..)
.If we decide to forget about trio, this can be made to work. But the code becomes an odd mix of async code and callbacks, which does not feel very elegant. Plus the fact that this cannot work with trio felt to me like a sign that this approach may be flawed.
Allow some sort of async initialization step
If we provide an initialization event that supports async callbacks, these can be used to get the wgpu device object asynchronously. For the rest of the lifetime, one should simply not using any async wgpu methods. This approach suffers from many of the same problems as the above. And if we have one event that supports async callbacks, why not all?
An alternative would be to use timets/ callbacks to wait for the future/promise of
request_adapter()
to resolve. Yuk.A global loop proxy
To be able to use the
GlfwRenderCanvas
with both the asyncio and the trio loop, the loop needed to be less tight to the canvas class. One approach I tried is to have a global object. All canvases "use" that proxy, and the proxy defers to whatever is the current loop.This was problematic for a few reasons. For one, the global object made testing a bit harder. It also raised questions like whether the active loop can change while there were canvases running. The biggest problem is related to interactive settings. Imagine being in Jupyter, or in an IDE that has asyncio running. I don't want users to need to call
loop.run()
there. But then we cannot know (for certain) what loop the user wants to use.The
CanvasGroup
that I eventually implemented solves the problem, by setting a default canvas (for each canvas backend), and allowing the user to select a different loop, but only when there are no live canvases in the group.