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

Refactor wgpu.gui for scheduling #618

Draft
wants to merge 51 commits into
base: main
Choose a base branch
from
Draft

Refactor wgpu.gui for scheduling #618

wants to merge 51 commits into from

Conversation

almarklein
Copy link
Member

@almarklein almarklein commented Oct 14, 2024

Refs

Summary

Canvas

  • Remove WgpuAutoGui, since its no simply a part of WgpuCanvasBase
  • Implementation of WgpuCanvasBase is cleaner because it moves events and scheduling out.

Events

  • Event handling logic is moved into a separate class.
  • The event handling logic can merge events by itself (Canvas implementations don't have to do this).
  • Events are queued until they are explicitly flushed (by the scheduling mechanics).
  • EventType enum!

Scheduling

  • The WgpuCanvasBase implements a scheduling mechanic (Canvas implementations no longer have to do this).
  • Multiple update modes are implemented: fastest, continuous, ondemand, manual.
  • The max_fps applies (only) to continuous and ondemand mode.
  • New force_draw() method for a blocking render call.

Public loop API

  • run() and call_later() functions are replaced with a loop object that has both as a method.
  • The call_later method returns a timer object, so the callback can be cancelled.
  • Also added call_repeated() that returns a timer object that keeps ticking. We really needed an abstract timer object for the scheduling. Might as well provide it to our users.

Design decisions

  • We assume that users use the event-loop corresponding to the canvas, and don't use two GUI systems at the same time (e.g. Qt with glfw). We can accommodate for this, but it will complicate the code and feels like a bad idea in general.
  • Similarly, our glwf window could run on any event loop, but we restrict it to asyncio (at least for now).
  • Do we keep canvas.add_event_handler() or do we switch to canvas.events.add_handler() or canvas.events.connect()?
    • I think canvas.add_event_handler() feels a bit long, but it is explicit, and helps the canvas feel flat/simple.
  • How to allow users to control scheduling, like setting mode or max_fps?
    • Proposal: use canvas.scheduler.max_fps, canvas.scheduler.mode, etc.

API changes

  • New canvas.force_draw().
  • Replaced from wgpu.gui.xx import WgpuCanvas, run with from wgpu.gui.xx import WgpuCanvas, loop
  • The glfw canvas is more friendly on your laptop battery (thanks to a proper event loop).
  • The WgpuAutoGui mixin class is removed.

Tasks

  • Update base canvas.
  • Take threading into account.
  • Update glfw gui
  • Update qt gui
  • Update wx gui
  • Update jupyter gui
  • Update offscreen gui
  • Don't draw when minimized.
  • Update docs
  • Show/test force_draw in an example.
  • Can we make 'ondemand' with calling request_draw() in the draw function equal 'continuous'. -> we test that this is the case.

After this PR

  • Handle sigint.
  • Animate events.
  • Allow draw_func to be an asyn function.
  • Can we move present() out of CanvasContext()?
  • Register the draw function as a draw event handler instead of with request_draw()?
  • Allow requesting a draw by setting a field in the event object?
  • Tracking statistics (track time spent on different things). Plus visualizing them.

@almarklein
Copy link
Member Author

I've been working on this for more than a week. This is hard, but it's beginning to take shape now.

@almarklein
Copy link
Member Author

This implements (amongst other things) the ideas presented in http://gameprogrammingpatterns.com/game-loop.html

As robert says:

image

In addition to that, we also gave multiple different GUI systems and event loops.

Do you own the game loop, or does the platform?

In our case that actually depends 😅 The scheduling code assumes it runs on an externally provided eventloop. For glfw we do provide the loop, but the scheduling logic does not know this.

wgpu/gui/base.py Outdated
Comment on lines 246 to 247
self._events.submit({"event_type": "before_draw"})
self._events.flush()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that this flush also flushes other pending events, which is not what should happen, and very likely the cause for why I cannot get force_draw() working on Qt.

I threw my hands in the air when I realized, hit my cup of tea, which fell over, spilling tea all over my desk, barely missing my laptop ...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... all is well that ends well. Good find!

@almarklein
Copy link
Member Author

Proposal: use canvas.scheduler.max_fps, canvas.scheduler.mode, etc.

These args can be passed when the canvas is instantiated. For now, we can leave it at that (no changing these at runtime).

@almarklein
Copy link
Member Author

Ok, this is almost done. All backends are working and tested on different platforms. I'm going to do a round of self-review and cleanup.

I think we will never merge this here, but in the new repo: #627

Comment on lines +59 to +61
if time.perf_counter() - self._draw_request_time > 0.02:
self._process_events()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +522 to +523
_surface_id = ffi.NULL

Copy link
Member Author

@almarklein almarklein Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be applied to wgpu

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@almarklein
Copy link
Member Author

This is ready for review. It does not actually break compatibility so much. You can run pygfx examples with it.

When this pr is approved, I'll re-recreate the pr at rendercanvas so we can apply it there.

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.

Blocking render call Sync events with draws
2 participants