Refactor resizing logic and avoid mismatch between last resize event and draw (#120)

Author: almarklein

rendercanvas/_scheduler.py

@@ -140,13 +140,7 @@ async def __scheduler_task(self):
 
             last_tick_time = time.perf_counter()
 
-            # Process events, handlers may request a draw
-            if (canvas := self.get_canvas()) is None:
-                break
-            canvas._process_events()
-            del canvas
-
-            # Determine what to do next ...
+            # Determine whether to draw or not yet
 
             do_draw = False
 
@@ -166,28 +160,36 @@ async def __scheduler_task(self):
                     and time.perf_counter() - last_draw_time > 1 / self._min_fps
                 ):
                     do_draw = True
-
             elif self._mode == "manual":
                 pass
             else:
                 raise RuntimeError(f"Unexpected scheduling mode: '{self._mode}'")
 
-            # If we don't want to draw, we move to the next iter
-            if not do_draw:
-                continue
-
-            self._events.emit({"event_type": "before_draw"})
-
-            # Ask the canvas to draw
+            # Get canvas object or stop the loop
             if (canvas := self.get_canvas()) is None:
                 break
-            canvas._rc_request_draw()
-            del canvas
 
-            # Wait for the draw to happen
-            self._async_draw_event = Event()
-            await self._async_draw_event.wait()
-            last_draw_time = time.perf_counter()
+            # Process events now.
+            # Note that we don't want to emit events *during* the draw, because event
+            # callbacks do stuff, and that stuff may include changing the canvas size,
+            # or affect layout in a UI application, all which are not recommended during
+            # the main draw-event (a.k.a. animation frame), and may even lead to errors.
+            # The one exception is resize events, which we do emit during a draw, if the
+            # size has changed since the last time that events were processed.
+            canvas._process_events()
+
+            if not do_draw:
+                # If we don't want to draw, move to the next iter
+                del canvas
+                continue
+            else:
+                # Otherwise, request a draw ...
+                canvas._rc_request_draw()
+                del canvas
+                # ... and wait for the draw to happen
+                self._async_draw_event = Event()
+                await self._async_draw_event.wait()
+                last_draw_time = time.perf_counter()
 
         # Note that when the canvas is closed, we may detect it here and break from the loop.
         # But the task may also be waiting for a draw to happen, or something else. In that case

rendercanvas/base.py

@@ -122,7 +122,7 @@ def select_loop(cls, loop: BaseLoop) -> None:
     def __init__(
         self,
         *args,
-        size: Tuple[int, int] = (640, 480),
+        size: Tuple[float, float] = (640, 480),
         title: str = "$backend",
         update_mode: UpdateModeEnum = "ondemand",
         min_fps: float = 0.0,
@@ -151,6 +151,8 @@ def __init__(
             if (self._rc_canvas_group and self._rc_canvas_group.get_loop())
             else "no-loop",
         }
+        self._set_size_info((0, 0), 1.0)  # Init self.__size_info
+        self.__size_info["need_event"] = False
 
         # Events and scheduler
         self._events = EventEmitter()
@@ -177,8 +179,8 @@ def __init__(
     def _final_canvas_init(self):
         """Must be called by the subclasses at the end of their ``__init__``.
 
-        This sets the canvas size and title, which must happen *after* the widget itself
-        is initialized. Doing this automatically can be done with a metaclass, but let's keep it simple.
+        This sets the canvas logical size and title, which must happen *after* the widget itself
+        is initialized. (Doing this automatically can be done with a metaclass, but let's keep it simple.)
         """
         # Pop kwargs
         try:
@@ -211,7 +213,7 @@ def __del__(self):
 
     def get_physical_size(self) -> Tuple[int, int]:
         """Get the physical size of the canvas in integer pixels."""
-        return self._rc_get_physical_size()
+        return self.__size_info["physical_size"]
 
     def get_context(self, context_type: str) -> object:
         """Get a context object that can be used to render to this canvas.
@@ -288,6 +290,27 @@ def get_context(self, context_type: str) -> object:
 
     # %% Events
 
+    def _set_size_info(self, physical_size: Tuple[int, int], pixel_ratio: float):
+        """Must be called by subclasses when their size changes.
+
+        Backends must *not* submit a "resize" event; the base class takes care of that, because
+        it requires some more attention than the other events.
+
+        The subclass must call this when the actual viewport has changed. So not in ``_rc_set_logical_size()``,
+        but e.g. when the underlying GUI layer fires a resize event, and maybe on init.
+        """
+        w, h = physical_size
+
+        psize = int(w), int(h)
+        pixel_ratio = float(pixel_ratio)
+        lsize = psize[0] / pixel_ratio, psize[1] / pixel_ratio
+        self.__size_info = {
+            "physical_size": psize,
+            "logical_size": lsize,
+            "pixel_ratio": pixel_ratio,
+            "need_event": True,
+        }
+
     def add_event_handler(
         self, *args: EventTypeEnum | EventHandlerFunction, order: float = 0
     ) -> Callable:
@@ -308,6 +331,22 @@ def submit_event(self, event: dict) -> None:
 
     # %% Scheduling and drawing
 
+    def __maybe_emit_resize_event(self):
+        if self.__size_info["need_event"]:
+            self.__size_info["need_event"] = False
+            lsize = self.__size_info["logical_size"]
+            self._events.emit(
+                {
+                    "event_type": "resize",
+                    "width": lsize[0],
+                    "height": lsize[1],
+                    "pixel_ratio": self.__size_info["pixel_ratio"],
+                    # Would be nice to have more details. But as it is now, PyGfx errors if we add fields it does not know, so let's do later.
+                    # "logical_size": self.__size_info["logical_size"],
+                    # "physical_size": self.__size_info["physical_size"],
+                }
+            )
+
     def _process_events(self):
         """Process events and animations.
 
@@ -321,6 +360,9 @@ def _process_events(self):
         # Get events from the GUI into our event mechanism.
         self._rc_gui_poll()
 
+        # If the canvas changed size, send event
+        self.__maybe_emit_resize_event()
+
         # Flush our events, so downstream code can update stuff.
         # Maybe that downstream code request a new draw.
         self._events.flush()
@@ -426,9 +468,16 @@ def _draw_frame_and_present(self):
             if self._rc_get_closed():
                 return
 
-            # Process special events
-            # Note that we must not process normal events here, since these can do stuff
-            # with the canvas (resize/close/etc) and most GUI systems don't like that.
+            # Note: could check whether the known physical size is > 0.
+            # But we also consider it the responsiblity of the backend to not
+            # draw if the size is zero. GUI toolkits like Qt do this correctly.
+            # I might get back on this once we also draw outside of the draw-event ...
+
+            # Make sure that the user-code is up-to-date with the current size before it draws.
+            self.__maybe_emit_resize_event()
+
+            # Emit before-draw
+            self._events.emit({"event_type": "before_draw"})
 
             # Notify the scheduler
             if self.__scheduler is not None:
@@ -471,7 +520,7 @@ def get_logical_size(self) -> Tuple[float, float]:
         The logical size can be smaller than the physical size, e.g. on HiDPI
         monitors or when the user's system has the display-scale set to e.g. 125%.
         """
-        return self._rc_get_logical_size()
+        return self.__size_info["logical_size"]
 
     def get_pixel_ratio(self) -> float:
         """Get the float ratio between logical and physical pixels.
@@ -482,7 +531,7 @@ def get_pixel_ratio(self) -> float:
         pixel ratio >= 2.0. On MacOS (with a Retina screen) the pixel ratio is
         always 2.0.
         """
-        return self._rc_get_pixel_ratio()
+        return self.__size_info["pixel_ratio"]
 
     def close(self) -> None:
         """Close the canvas."""
@@ -611,18 +660,6 @@ def _rc_present_bitmap(self, *, data, format, **kwargs):
         """
         raise NotImplementedError()
 
-    def _rc_get_physical_size(self) -> Tuple[int, int]:
-        """Get the physical size (with, height) in integer pixels."""
-        raise NotImplementedError()
-
-    def _rc_get_logical_size(self) -> Tuple[float, float]:
-        """Get the logical size (with, height) in float pixels."""
-        raise NotImplementedError()
-
-    def _rc_get_pixel_ratio(self) -> float:
-        """Get ratio between physical and logical size."""
-        raise NotImplementedError()
-
     def _rc_set_logical_size(self, width: float, height: float):
         """Set the logical size. May be ignired when it makes no sense.
 

rendercanvas/glfw.py

@@ -246,6 +246,7 @@ def __init__(self, *args, present_method=None, **kwargs):
         self._screen_size_is_logical = False
 
         # Set size, title, etc.
+        self._determine_size()
         self._final_canvas_init()
 
         # Now show the window
@@ -270,17 +271,8 @@ def _determine_size(self):
         pixel_ratio = get_window_content_scale(self._window)[0]
         psize = get_physical_size(self._window)
 
-        self._pixel_ratio = pixel_ratio
-        self._physical_size = psize
-        self._logical_size = psize[0] / pixel_ratio, psize[1] / pixel_ratio
-
-        ev = {
-            "event_type": "resize",
-            "width": self._logical_size[0],
-            "height": self._logical_size[1],
-            "pixel_ratio": self._pixel_ratio,
-        }
-        self.submit_event(ev)
+        self._pixel_ratio = pixel_ratio  # store
+        self._set_size_info(psize, pixel_ratio)
 
     def _on_want_close(self, *args):
         # Called when the user attempts to close the window, for example by clicking the close widget in the title bar.
@@ -313,12 +305,7 @@ def _set_logical_size(self, new_logical_size):
             int(new_logical_size[0] * pixel_ratio * screen_ratio),
             int(new_logical_size[1] * pixel_ratio * screen_ratio),
         )
-
         self._screen_size_is_logical = screen_ratio != 1
-        # If this causes the widget size to change, then _on_size_change will
-        # be called, but we may want force redetermining the size.
-        if pixel_ratio != self._pixel_ratio:
-            self._determine_size()
 
     # %% Methods to implement RenderCanvas
 
@@ -348,15 +335,6 @@ def _rc_present_bitmap(self, **kwargs):
         # not really need one, since it's the most reliable backend to
         # render to the screen.
 
-    def _rc_get_physical_size(self):
-        return self._physical_size
-
-    def _rc_get_logical_size(self):
-        return self._logical_size
-
-    def _rc_get_pixel_ratio(self):
-        return self._pixel_ratio
-
     def _rc_set_logical_size(self, width, height):
         if width < 0 or height < 0:
             raise ValueError("Window width and height must not be negative")
@@ -406,7 +384,8 @@ def _on_pixelratio_change(self, *args):
             return
         self._changing_pixel_ratio = True  # prevent recursion (on Wayland)
         try:
-            self._set_logical_size(self._logical_size)
+            self._set_logical_size(self.get_logical_size())
+            self._determine_size()
         finally:
             self._changing_pixel_ratio = False
         self.request_draw()

rendercanvas/jupyter.py

@@ -29,8 +29,6 @@ def __init__(self, *args, **kwargs):
 
         # Internal variables
         self._last_image = None
-        self._pixel_ratio = 1
-        self._logical_size = 0, 0
         self._is_closed = False
         self._draw_request_time = 0
         self._rendercanvas_event_types = set(EventType)
@@ -82,19 +80,7 @@ def _rc_present_bitmap(self, *, data, format, **kwargs):
         assert format == "rgba-u8"
         self._last_image = np.frombuffer(data, np.uint8).reshape(data.shape)
 
-    def _rc_get_physical_size(self):
-        return int(self._logical_size[0] * self._pixel_ratio), int(
-            self._logical_size[1] * self._pixel_ratio
-        )
-
-    def _rc_get_logical_size(self):
-        return self._logical_size
-
-    def _rc_get_pixel_ratio(self):
-        return self._pixel_ratio
-
     def _rc_set_logical_size(self, width, height):
-        self._logical_size = width, height
         self.css_width = f"{width}px"
         self.css_height = f"{height}px"
 
@@ -117,10 +103,17 @@ def handle_event(self, event):
         if event_type == "close":
             self._is_closed = True
         elif event_type == "resize":
-            self._pixel_ratio = event["pixel_ratio"]
-            self._logical_size = event["width"], event["height"]
-
-        # Only submit events that rendercanvas known. Otherwise, if new events are added
+            logical_size = event["width"], event["height"]
+            pixel_ratio = event["pixel_ratio"]
+            physical_size = (
+                int(logical_size[0] * pixel_ratio),
+                int(logical_size[1] * pixel_ratio),
+            )
+            self._set_size_info(physical_size, pixel_ratio)
+            self.request_draw()
+            return
+
+        # Only submit events that rendercanvas knows. Otherwise, if new events are added
         # to jupyter_rfb that rendercanvas does not (yet) know, rendercanvas will complain.
         if event_type in self._rendercanvas_event_types:
             self.submit_event(event)

rendercanvas/offscreen.py

@@ -29,7 +29,7 @@ class OffscreenRenderCanvas(BaseRenderCanvas):
 
     def __init__(self, *args, pixel_ratio=1.0, format="rgba-u8", **kwargs):
         super().__init__(*args, **kwargs)
-        self._pixel_ratio = pixel_ratio
+        self._pixel_ratio = float(pixel_ratio)
         self._closed = False
         self._last_image = None
 
@@ -70,19 +70,13 @@ def _rc_force_draw(self):
     def _rc_present_bitmap(self, *, data, format, **kwargs):
         self._last_image = data
 
-    def _rc_get_physical_size(self):
-        return int(self._logical_size[0] * self._pixel_ratio), int(
-            self._logical_size[1] * self._pixel_ratio
-        )
-
-    def _rc_get_logical_size(self):
-        return self._logical_size
-
-    def _rc_get_pixel_ratio(self):
-        return self._pixel_ratio
-
     def _rc_set_logical_size(self, width, height):
-        self._logical_size = width, height
+        logical_size = float(width), float(height)
+        physical_size = (
+            int(logical_size[0] * self._pixel_ratio),
+            int(logical_size[1] * self._pixel_ratio),
+        )
+        self._set_size_info(physical_size, self._pixel_ratio)
 
     def _rc_close(self):
         self._closed = True