Refactor canvas and loop closing logic

.github/workflows/ci.yml

@@ -152,27 +152,40 @@ jobs:
       fail-fast: false
       matrix:
         include:
+          # Loops
+          - backendname: 'raw'
+            deps: ''
+            os: windows-latest
+          - backendname: 'asyncio'
+            deps: ''
+            os: windows-latest
+          - backendname: 'trio'
+            deps: 'trio'
+            os: windows-latest
+          # GUI
           - backendname: 'glfw'
             deps: 'glfw'
             os: windows-latest
           - backendname: 'pyside6'
             deps: 'PySide6'
             testname: 'qt'
-            loopname: 'PySide6Loop'
             os: windows-latest
           - backendname: 'pyqt6'
             deps: 'PyQt6'
             testname: 'qt'
-            loopname: 'PyQt6Loop'
             os: windows-latest
           - backendname: 'pyqt5'
             deps: 'PyQt5'
             testname: 'qt'
-            loopname: 'PyQt5Loop'
             os: windows-latest
           - backendname: 'wx'
             deps: 'wxPython'
-            # loopname: 'WxLoop'  wx's event loop has limitations
+            os: windows-latest
+          # Other
+          - backendname: 'terminal'
+            deps: 'blessed'
+            os: windows-latest
+          - backendname: 'offscreen'
             os: windows-latest
     steps:
     - uses: actions/checkout@v7
@@ -189,16 +202,11 @@ jobs:
     - name: Install package and dev dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install .[tests]
-        pip install ${{ matrix.deps }}
+        pip install .[tests] ${{ matrix.deps }}
         rm -r rendercanvas
     - name: Test import
       run: |
         python -c 'import rendercanvas.${{ matrix.backendname }}'
-    - name: Test loop
-      if: matrix.loopname
-      run: |
-        pytest -v tests/test_loop.py -k ${{ matrix.loopname }}
     - name: Test backend canvas
       run: |
         pytest -v tests/test_backend_${{ matrix.testname || matrix.backendname }}.py

README.md

@@ -148,6 +148,7 @@ See the [contribution guide](CONTRIBUTING.md).
 * Use `ruff check` to check for linting errors.
 * Use `pytest tests` to run the tests.
 * Use `pytest examples` to run a subset of the examples.
+* Use `pytest tests/test_backend_xx.py` to run backend-specific tests, e.g. for `glfw`, `qt`, `wx`.
 
 ### Code of Conduct
 

rendercanvas/asyncio.py

@@ -62,6 +62,10 @@ async def _rc_run_async(self):
         while self.__pending_tasks:
             self._rc_add_task(*self.__pending_tasks.pop(0))
 
+        # Only do one cycle of processing tasks if there are no canvases.
+        if not self.get_canvases():
+            self._run_loop.call_soon(self.stop)
+
         # Wait for loop to finish
         if self._stop_event is None:
             self._stop_event = asyncio.Event()

rendercanvas/base.py

@@ -59,6 +59,10 @@ def _register_canvas(self, canvas, task):
             loop._register_canvas_group(self)
             loop.add_task(task, name="scheduler-task")
 
+    def _unregister_canvas(self, canvas):
+        """Used by the canvas to unregister itself when closed."""
+        self._canvases.discard(canvas)
+
     def select_loop(self, loop: BaseLoop) -> None:
         """Select the loop to use for this group of canvases."""
         if not (loop is None or isinstance(loop, BaseLoop)):
@@ -76,17 +80,8 @@ def get_loop(self) -> BaseLoop | None:
         """Get the currently associated loop (can be None for canvases that don't run a scheduler)."""
         return self._loop
 
-    def get_canvases(self, *, close_closed=False) -> list[BaseRenderCanvas]:
-        if close_closed:
-            closed_canvases = [
-                canvas for canvas in self._canvases if canvas.get_closed()
-            ]
-            for canvas in closed_canvases:
-                canvas.close()
-                self._canvases.discard(canvas)
-            return self._canvases
-        else:
-            return [canvas for canvas in self._canvases if not canvas.get_closed()]
+    def get_canvases(self) -> list[BaseRenderCanvas]:
+        return [canvas for canvas in self._canvases]
 
 
 class BaseRenderCanvas:
@@ -681,18 +676,34 @@ def get_pixel_ratio(self) -> float:
 
     def close(self) -> None:
         """Close the canvas."""
+        errors = []
+        # Close the canvas natively, the canvas may only be marked as closed once this is done
+        try:
+            self._rc_close()
+        except Exception as err:
+            errors.append(err)
+        # Unregister
+        self._rc_canvas_group._unregister_canvas(self)
         # Clear the draw-function, to avoid it holding onto e.g. wgpu objects.
         self._draw_frame = None  # type: ignore
         # Clear the canvas context too.
         try:
-            self._canvas_context._rc_close()  # type: ignore
-        except Exception:
-            pass
+            if self._canvas_context is not None:
+                self._canvas_context._rc_close()  # type: ignore
+        except Exception as err:
+            errors.append(err)
         self._canvas_context = None
-        # Clean events. Should already have happened in loop, but the loop may not be running.
+        # Close the event at least after get_closed() would return True
         self._events.close()
-        # Let the subclass clean up.
-        self._rc_close()
+        # Stop the loop if this was the last canvas. It's important to do now,
+        # because the native loop may stop right after, giving us no flow to properly close.
+        loop = self._rc_canvas_group.get_loop()
+        if loop is not None:
+            if not loop.get_canvases():
+                loop.stop()
+        # Errors
+        if errors:
+            raise errors[0]
 
     def get_closed(self) -> bool:
         """Get whether the window is closed."""
@@ -853,21 +864,21 @@ def _rc_set_logical_size(self, width: float, height: float):
     def _rc_close(self):
         """Close the canvas.
 
-        Note that ``BaseRenderCanvas`` implements the ``close()`` method, which is a
-        rather common name; it may be necessary to re-implement that too.
+        This is the place to delete the native widget, and to maybe set a flag
+        that ``_rc_get_closed()`` uses. It is not necessary to emit a close event,
+        because the base class handles that.
 
-        Backends should probably not mark the canvas as closed yet, but wait until the
-        underlying system really closes the canvas. Otherwise the loop may end before a
-        canvas gets properly cleaned up.
+        In a backend, all flows that lead to a close must call ``.close()``, so
+        that ``BaseRenderCanvas`` has a clear place to handle closing. It calls
+        ``_rc_close()``  from there.
 
-        Backends can emit a closed event, either in this method, or when the real close
-        happens, but this is optional, since the loop detects canvases getting closed
-        and sends the close event if this has not happened yet.
+        Note that backends may also have a ``close()`` method, which is
+        overridden by the base class.
         """
         pass
 
     def _rc_get_closed(self) -> bool:
-        """Get whether the canvas is closed."""
+        """Get whether the canvas is closed. A typical implementation uses a flag that is set in ``_rc_close()``."""
         return False
 
     def _rc_set_title(self, title: str):

rendercanvas/core/loop.py

@@ -36,7 +36,7 @@ class BaseLoop:
 
     Canvas backends can implement their own loop subclass (like qt and wx do), but a
     canvas backend can also rely on one of multiple loop implementations (like glfw
-    running on asyncio or trio).
+    running on ``raw``, ``asyncio`` or ``trio``).
 
     In the majority of use-cases, users don't need to know much about the loop. It will typically
     run once. In more complex scenario's the section below explains the working of the loop in more detail.
@@ -54,7 +54,7 @@ class BaseLoop:
         * Entered when the first canvas is created that is associated with this loop, or when a task is added.
         * It is assumed that the loop will become active soon.
         * This is when ``_rc_init()`` is called to get the backend ready for running.
-        * A special 'loop-task' is created (a coroutine, which is not yet running).
+        * A co-routine is created that will detect when the loop starts running, so some things can be initialized at the right moment.
     * running:
         * Entered when ``loop.run()`` is called.
         * The loop is now running.
@@ -65,26 +65,21 @@ class BaseLoop:
         * This means there is a persistent native loop already running, which rendercanvas makes use of.
     * active:
         * Entered when the backend-loop starts running, but not via the loop's ``run()`` method.
-        * This is detected via the loop-task.
+        * This is detected via the aforementioned co-routine.
         * Signal handlers and asyncgen hooks are installed if applicable.
-        * Detecting loop stopping occurs by the loop-task being cancelled.
 
     Notes related to starting and stopping:
 
     * The loop goes back to the "off" state once all canvases are closed.
-    * Stopping the loop (via ``.stop()``) closes the canvases, which will then stop the loop.
+    * Stopping the loop (via ``.stop()``) closes all canvases.
     * From there it can go back to the ready state (which would call ``_rc_init()`` again).
     * In backends like Qt, the native loop can be started without us knowing: state "active".
     * In interactive settings like an IDE that runs an asyncio or Qt loop, the
       loop becomes "interactive" as soon as the first canvas is created.
     * The rendercanvas loop can be in the 'off' state while the native loop is running (especially for the 'interactive' case).
-    * On Qt, the app's 'aboutToQuit' signal is used to stop this loop.
-    * On wx, the loop is stopped when all windows are closed.
 
     """
 
-    _stop_when_no_canvases = True
-
     def __init__(self):
         self.__tasks = set()  # only used by the async adapter
         self.__canvas_groups = set()
@@ -141,11 +136,11 @@ def _unregister_canvas_group(self, canvas_group):
         # A CanvasGroup will call this when it selects a different loop.
         self.__canvas_groups.discard(canvas_group)
 
-    def get_canvases(self, *, close_closed=False) -> list[BaseRenderCanvas]:
+    def get_canvases(self) -> list[BaseRenderCanvas]:
         """Get a list of currently active (not-closed) canvases."""
         canvases = []
         for canvas_group in self.__canvas_groups:
-            canvases += canvas_group.get_canvases(close_closed=close_closed)
+            canvases += canvas_group.get_canvases()
         return canvases
 
     def _ensure_initialized(self):
@@ -156,69 +151,16 @@ def _ensure_initialized(self):
         if self.__state == LoopState.off:
             self.__state = LoopState.ready
 
-        async def wrapper():
-            try:
-                with log_exception("Error in loop-task:"):
-                    await self._loop_task()
-            finally:
-                # We get here when the task is finished or cancelled.
-                self.__is_initialized = False
-
         self.__is_initialized = True
         self._rc_init()
-        self._rc_add_task(wrapper, "loop-task")
-        self.__using_adapter = len(self.__tasks) > 0
-
-    async def _loop_task(self):
-        # This task has multiple purposes:
-        #
-        # * Detect when the the loop starts running. When this code runs, it
-        #   means something is running the task.
-        # * Detect closed windows while the loop is running. This is nice,
-        #   because it means backends only have to mark the canvas as closed,
-        #   and the base canvas takes care that .close() is called and the close
-        #   event is emitted.
-        # * Stop the loop when there are no more canvases. Note that the loop
-        #   may also be stopped from the outside, in which case *this* task is
-        #   cancelled along with the other tasks.
-        # * Detect when the loop stops running, in case the native loop stops in
-        #   a friendly way, cancelling tasks, including *this* task.
-        # * Keep the GUI going even when the canvas loop is on pause e.g.
-        #   because its minimized (applies to backends that implement
-        #   _rc_gui_poll).
-
-        # In some cases the task may run after the loop was closed
-        if self.__state == LoopState.off:
-            return
-
-        # The loop has started!
-        self.__start()
-
-        try:
-            while True:
-                await sleep(0.1)
 
-                # Note that this triggers .close() on closed canvases, for proper cleanup and sending close event.
-                canvases = self.get_canvases(close_closed=True)
-
-                # Keep canvases alive
-                for canvas in canvases:
-                    canvas._rc_gui_poll()
-                    del canvas
-
-                # Break?
-                canvas_count = len(canvases)
-                del canvases
-                if not canvas_count and self._stop_when_no_canvases:
-                    break
+        # self._rc_add_task(wrapper, "loop-task")
+        self._rc_add_task(self._loop_start_detection_task, "loop-start-detection-task")
+        self.__using_adapter = len(self.__tasks) > 0
 
-        finally:
-            # We get here when we break the while-loop, but also when the task
-            # is cancelled (e.g. because the asyncio loop stops). In both cases
-            # we call stop from the *end* of the task, which is important since
-            # __stop() cancels all tasks, but cannot cancel the task that it is
-            # currently in.
-            self.stop(force=True)
+    async def _loop_start_detection_task(self):
+        if self.__state != LoopState.off:
+            self.__start()
 
     def add_task(
         self,
@@ -363,7 +305,7 @@ async def run_async(self) -> None:
         try:
             await self._rc_run_async()
         finally:
-            self.__state = LoopState.off
+            self.stop(force=True)
 
     def stop(self, *, force=False) -> None:
         """Close all windows and stop the currently running event-loop.
@@ -384,7 +326,7 @@ def stop(self, *, force=False) -> None:
         self.__should_stop += 2 if force else 1
 
         # Close all canvases
-        canvases = self.get_canvases(close_closed=True)
+        canvases = self.get_canvases()
         for canvas in canvases:
             try:
                 closed_by_loop = canvas._rc_closed_by_loop  # type: ignore
@@ -396,7 +338,7 @@ def stop(self, *, force=False) -> None:
             del canvas
 
         # Do a real stop?
-        if len(canvases) == 0 or self.__should_stop >= 2:
+        if len(self.get_canvases()) == 0 or self.__should_stop >= 2:
             self.__stop()
 
     def __setup_hooks(self):
@@ -426,7 +368,6 @@ def __restore_hooks(self):
 
         prev_asyncgen_hooks, prev_interrupt_hooks = self.__hook_data
         self.__hook_data = None
-
         if prev_asyncgen_hooks is not None:
             sys.set_asyncgen_hooks(*prev_asyncgen_hooks)
 
@@ -444,9 +385,10 @@ def __start(self):
     def __stop(self):
         """Move to the off-state."""
 
+        self.__is_initialized = False
+
         # Note that in here, we must fully bring our loop to a stop.
         # We cannot rely on future loop cycles.
-
         # Set flags to off state
         self.__state = LoopState.off
         self.__should_stop = 0