Skip to content

Commit

Permalink
fix canvas cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
almarklein committed Oct 15, 2024
1 parent 9ee8cf1 commit baf4785
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 57 deletions.
6 changes: 2 additions & 4 deletions tests/renderutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def render_to_screen(
):
"""Render to a window on screen, for debugging purposes."""
import glfw
from wgpu.gui.glfw import WgpuCanvas, update_glfw_canvasses
from wgpu.gui.glfw import WgpuCanvas, loop

vbos = vbos or []
vbo_views = vbo_views or []
Expand Down Expand Up @@ -327,6 +327,4 @@ def draw_frame():
canvas.request_draw(draw_frame)

# Enter main loop
while update_glfw_canvasses():
glfw.poll_events()
glfw.terminate()
loop.run()
45 changes: 18 additions & 27 deletions tests/test_gui_glfw.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,34 +60,28 @@ def test_glfw_canvas_basics():

# Close
assert not canvas.is_closed()
if sys.platform.startswith("win"): # On Linux we cant do this multiple times
canvas.close()
glfw.poll_events()
assert canvas.is_closed()
canvas.close()
glfw.poll_events()
assert canvas.is_closed()


def test_glfw_canvas_del():
from wgpu.gui.glfw import WgpuCanvas, update_glfw_canvasses
import glfw
from wgpu.gui.glfw import WgpuCanvas, loop

loop = asyncio.get_event_loop()

async def miniloop():
for i in range(10):
glfw.poll_events()
update_glfw_canvasses()
await asyncio.sleep(0.01)
def run_briefly():
asyncio_loop = loop._loop
asyncio_loop.run_until_complete(asyncio.sleep(0.5))
# poll_glfw_briefly()

canvas = WgpuCanvas()
ref = weakref.ref(canvas)

assert ref() is not None
loop.run_until_complete(miniloop())
run_briefly()
assert ref() is not None
del canvas
if is_pypy:
gc.collect() # force garbage collection for pypy
loop.run_until_complete(miniloop())
assert ref() is None


Expand All @@ -110,9 +104,12 @@ def test_glfw_canvas_render():
"""Render an orange square ... in a glfw window."""

import glfw
from wgpu.gui.glfw import update_glfw_canvasses, WgpuCanvas
from wgpu.gui.glfw import WgpuCanvas, loop

loop = asyncio.get_event_loop()
def run_briefly():
asyncio_loop = loop._loop
asyncio_loop.run_until_complete(asyncio.sleep(0.5))
# poll_glfw_briefly()

canvas = WgpuCanvas(max_fps=9999)

Expand All @@ -128,30 +125,24 @@ def draw_frame2():

canvas.request_draw(draw_frame2)

# Give it a few rounds to start up
async def miniloop():
for i in range(10):
glfw.poll_events()
update_glfw_canvasses()
await asyncio.sleep(0.01)

loop.run_until_complete(miniloop())
run_briefly()
# There should have been exactly one draw now
# This assumes ondemand scheduling mode
assert frame_counter == 1

# Ask for a lot of draws
for i in range(5):
canvas.request_draw()
# Process evens for a while
loop.run_until_complete(miniloop())
run_briefly()
# We should have had just one draw
assert frame_counter == 2

# Change the canvas size
canvas.set_logical_size(300, 200)
canvas.set_logical_size(400, 300)
# We should have had just one draw
loop.run_until_complete(miniloop())
run_briefly()
assert frame_counter == 3

# canvas.close()
Expand Down
3 changes: 2 additions & 1 deletion tests_mem/test_gui_glfw.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
if not can_use_glfw:
pytest.skip("Need glfw for this test", allow_module_level=True)

loop = asyncio.get_event_loop_policy().get_event_loop()
loop = asyncio.get_event_loop()
if loop.is_running():
pytest.skip("Asyncio loop is running", allow_module_level=True)

Expand Down Expand Up @@ -54,6 +54,7 @@ def test_release_canvas_context(n):
yield c.get_context()

# Need some shakes to get all canvas refs gone.
# Note that the draw function closure holds a ref to the canvas.
del c
loop.run_until_complete(stub_event_loop())
gc.collect()
Expand Down
51 changes: 30 additions & 21 deletions wgpu/gui/_loop.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import time
import weakref

from ._gui_utils import log_exception

Expand Down Expand Up @@ -93,19 +94,21 @@ class Scheduler:
# schedule pseuso_tick pseuso_tick
# + event_tick

def __init__(self, canvas, *, min_fps=1, max_fps=30):
# Objects related to the canvas
self._canvas = canvas
def __init__(self, canvas, *, mode="ondemand", min_fps=1, max_fps=30):
# Objects related to the canvas.
# We don't keep a ref to the canvas to help gc. This scheduler object can be
# referenced via a callback in an event loop, but it won't prevent the canvas
# from being deleted!
self._canvas_ref = weakref.ref(canvas)
self._events = canvas._events

# The draw function
self._draw_frame = lambda: None
self._loop = canvas._get_loop()
# ... = canvas.get_context() -> No, context creation should be lazy!

# Lock the scheduling while its waiting
self._waiting_lock = False

# Scheduling variables
self._mode = "continuous"
self._mode = mode
self._min_fps = float(min_fps)
self._max_fps = float(max_fps)
self._draw_requested = True
Expand All @@ -120,11 +123,12 @@ def __init__(self, canvas, *, min_fps=1, max_fps=30):

# Start by doing the first scheduling.
# Note that the gui may do a first draw earlier, starting the loop, and that's fine.
canvas._get_loop().call_later(0.1, self._schedule_next_tick)
self._loop.call_later(0.1, self._schedule_next_tick)

def set_draw_func(self, draw_frame):
"""Set the callable that must be called to do a draw."""
self._draw_frame = draw_frame
def _get_canvas(self):
canvas = self._canvas_ref()
if not (canvas is None or canvas.is_closed()):
return canvas

def request_draw(self):
"""Request a new draw to be done. Only affects the 'ondemand' mode."""
Expand Down Expand Up @@ -183,13 +187,17 @@ def pseudo_tick():
# Enable scheduling again
self._waiting_lock = False

# Get canvas or stop
if (canvas := self._get_canvas()) is None:
return

if self._mode == "fastest":
# fastest: draw continuously as fast as possible, ignoring fps settings.
self._canvas._request_draw()
canvas._request_draw()

elif self._mode == "continuous":
# continuous: draw continuously, aiming for a steady max framerate.
self._canvas._request_draw()
canvas._request_draw()

elif self._mode == "ondemand":
# ondemand: draw when needed (detected by calls to request_draw).
Expand All @@ -199,7 +207,7 @@ def pseudo_tick():
time.perf_counter() - self._last_draw_time > 1 / self._min_fps
)
if self._draw_requested or its_draw_time:
self._canvas._request_draw()
canvas._request_draw()
else:
self._schedule_next_tick()

Expand All @@ -211,13 +219,13 @@ def pseudo_tick():
else:
raise RuntimeError(f"Unexpected scheduling mode: '{self._mode}'")

self._canvas._get_loop().call_later(delay, pseudo_tick)
self._loop.call_later(delay, pseudo_tick)

def event_tick(self):
"""A lightweight tick that processes evets and animations."""

# Get events from the GUI into our event mechanism.
self._canvas._get_loop().poll()
self._loop.poll()

# Flush our events, so downstream code can update stuff.
# Maybe that downstream code request a new draw.
Expand Down Expand Up @@ -250,7 +258,7 @@ def draw_tick(self):

# It could be that the canvas is closed now. When that happens,
# we stop here and do not schedule a new iter.
if self._canvas.is_closed():
if (canvas := self._get_canvas()) is None:
return

# Keep ticking
Expand All @@ -274,14 +282,15 @@ def draw_tick(self):
# Stats (uncomment to see fps)
count, last_time = self._draw_stats
fps = count / (time.perf_counter() - last_time)
self._canvas.set_title(f"wgpu {fps:0.1f} fps")
canvas.set_title(f"wgpu {fps:0.1f} fps")

# Perform the user-defined drawing code. When this errors,
# we should report the error and then continue, otherwise we crash.
with log_exception("Draw error"):
self._draw_frame()
canvas._draw_frame()
with log_exception("Present error"):
context = self._canvas._canvas_context
# Note: we use canvas._canvas_context, so that if the draw_frame is a stub we also dont trigger creating a context.
# Note: if vsync is used, this call may wait a little (happens down at the level of the driver or OS)
context = canvas._canvas_context
if context:
context.present()
# Note, if vsync is used, this call may wait a little (happens down at the level of the driver or OS)
11 changes: 8 additions & 3 deletions wgpu/gui/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ class WgpuCanvasBase(WgpuCanvasInterface):
def __init__(
self,
*args,
min_fps=1,
max_fps=30,
vsync=True,
present_method=None,
Expand All @@ -114,11 +113,13 @@ def __init__(
self._vsync = bool(vsync)
present_method # noqa - We just catch the arg here in case a backend does implement support it

self._draw_frame = lambda: None

self._events = EventEmitter()

self._scheduler = None
if use_scheduler:
self._scheduler = Scheduler(self, min_fps=min_fps, max_fps=max_fps)
self._scheduler = Scheduler(self, max_fps=max_fps)

def __del__(self):
# On delete, we call the custom close method.
Expand Down Expand Up @@ -167,11 +168,15 @@ def request_draw(self, draw_function=None):
if self._scheduler is None:
return
if draw_function is not None:
self._scheduler.set_draw_func(draw_function)
self._draw_frame = draw_function
self._scheduler.request_draw()

# todo: maybe requesting a new draw can be done by setting a field in an event?
# todo: can just make the draw_function a handler for the draw event?
# -> Note that the draw func is likely to hold a ref to the canvas. By storing it
# here, the circular ref can be broken. This fails if we'd store _draw_frame on the
# scheduler! So with a draw event, we should provide the context and more info so
# that a draw funcion does not need the canvas object.

def force_draw(self):
self._force_draw()
Expand Down
3 changes: 2 additions & 1 deletion wgpu/gui/qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,8 @@ def _app(self):
return QtWidgets.QApplication.instance() or QtWidgets.QApplication([])

def poll(self):
self._app.process_events()
# todo: make this a private method with a wgpu prefix.
pass # we assume the Qt event loop is running. Calling processEvents() will cause recursive repaints.

def call_later(self, delay, callback, *args):
func = callback
Expand Down

0 comments on commit baf4785

Please sign in to comment.