From 9d720e5fe43860b2a3c4220a914ca96225aed430 Mon Sep 17 00:00:00 2001 From: Maic Siemering Date: Tue, 1 Jul 2025 15:27:29 +0200 Subject: [PATCH 1/3] remove debug output --- arcade/examples/gui/6_size_hints.py | 1 - 1 file changed, 1 deletion(-) diff --git a/arcade/examples/gui/6_size_hints.py b/arcade/examples/gui/6_size_hints.py index 8801eb8470..2d3959a0c3 100644 --- a/arcade/examples/gui/6_size_hints.py +++ b/arcade/examples/gui/6_size_hints.py @@ -135,7 +135,6 @@ def on_change(event: UIOnChangeEvent): content_anchor.add(UISpace(height=20)) self.ui.execute_layout() - self.ui.debug() def main(): From ec4abf4dcc8de4de4da19d67a4430ed6f1f03252 Mon Sep 17 00:00:00 2001 From: Maic Siemering Date: Tue, 1 Jul 2025 15:47:04 +0200 Subject: [PATCH 2/3] remove debug output in UIManager --- arcade/gui/ui_manager.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/arcade/gui/ui_manager.py b/arcade/gui/ui_manager.py index 2fcc29f1e3..3a5707d47d 100644 --- a/arcade/gui/ui_manager.py +++ b/arcade/gui/ui_manager.py @@ -529,12 +529,10 @@ def _set_active_widget(self, widget: UIWidget | None): return if self._active_widget: - print(f"Deactivating widget {self._active_widget.__class__.__name__}") self._active_widget._active = False self._active_widget = widget if self._active_widget: - print(f"Activating widget {self._active_widget.__class__.__name__}") self._active_widget._active = True def debug(self): From 9afc0ee44158aa825a3b354f056226054554ba6d Mon Sep 17 00:00:00 2001 From: Maic Siemering Date: Tue, 1 Jul 2025 15:47:43 +0200 Subject: [PATCH 3/3] Bind class methods within a widget to avoid memory leak --- arcade/gui/experimental/focus.py | 6 +- arcade/gui/experimental/scroll_area.py | 14 +-- arcade/gui/property.py | 89 ++++++++++++------- arcade/gui/widgets/__init__.py | 86 ++++++++++++++---- arcade/gui/widgets/buttons.py | 2 +- arcade/gui/widgets/image.py | 6 +- arcade/gui/widgets/layout.py | 54 +++++------ arcade/gui/widgets/slider.py | 10 +-- arcade/gui/widgets/text.py | 24 ++--- arcade/gui/widgets/toggle.py | 4 +- tests/unit/gui/test_layouting_anchorlayout.py | 7 +- tests/unit/gui/test_layouting_boxlayout.py | 50 +++++++---- tests/unit/gui/test_property.py | 38 ++++++++ tests/unit/gui/test_uilabel.py | 6 +- tests/unit/gui/test_widget_tree.py | 50 ++++++++++- 15 files changed, 313 insertions(+), 133 deletions(-) diff --git a/arcade/gui/experimental/focus.py b/arcade/gui/experimental/focus.py index e806150120..a41cd158d5 100644 --- a/arcade/gui/experimental/focus.py +++ b/arcade/gui/experimental/focus.py @@ -71,9 +71,9 @@ class UIFocusMixin(UIWidget): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - bind(self, "_debug", self.trigger_full_render) - bind(self, "_focused_widget", self.trigger_full_render) - bind(self, "_focusable_widgets", self.trigger_full_render) + bind(self, "_debug", UIFocusMixin.trigger_full_render) + bind(self, "_focused_widget", UIFocusMixin.trigger_full_render) + bind(self, "_focusable_widgets", UIFocusMixin.trigger_full_render) def on_event(self, event: UIEvent) -> bool | None: # pass events to children first, including controller events diff --git a/arcade/gui/experimental/scroll_area.py b/arcade/gui/experimental/scroll_area.py index e8818976c1..5b8ad10617 100644 --- a/arcade/gui/experimental/scroll_area.py +++ b/arcade/gui/experimental/scroll_area.py @@ -44,11 +44,11 @@ def __init__(self, scroll_area: UIScrollArea, vertical: bool = True): self.with_border(color=arcade.uicolor.GRAY_CONCRETE) self.vertical = vertical - bind(self, "_thumb_hover", self.trigger_render) - bind(self, "_dragging", self.trigger_render) - bind(scroll_area, "scroll_x", self.trigger_full_render) - bind(scroll_area, "scroll_y", self.trigger_full_render) - bind(scroll_area, "rect", self.trigger_full_render) + bind(self, "_thumb_hover", UIScrollBar.trigger_render) + bind(self, "_dragging", UIScrollBar.trigger_render) + bind(scroll_area, "scroll_x", UIScrollBar.trigger_full_render) + bind(scroll_area, "scroll_y", UIScrollBar.trigger_full_render) + bind(scroll_area, "rect", UIScrollBar.trigger_full_render) def on_event(self, event: UIEvent) -> bool | None: # check if we are scrollable @@ -234,8 +234,8 @@ def __init__( size=canvas_size, ) - bind(self, "scroll_x", self.trigger_full_render) - bind(self, "scroll_y", self.trigger_full_render) + bind(self, "scroll_x", UIScrollArea.trigger_full_render) + bind(self, "scroll_y", UIScrollArea.trigger_full_render) def add(self, child: W, **kwargs) -> W: """Add a child to the widget.""" diff --git a/arcade/gui/property.py b/arcade/gui/property.py index e8ad3e8914..400536f9b8 100644 --- a/arcade/gui/property.py +++ b/arcade/gui/property.py @@ -3,6 +3,7 @@ import traceback from collections.abc import Callable from contextlib import contextmanager, suppress +from enum import Enum from typing import Any, Generic, TypeVar, cast from weakref import WeakKeyDictionary, ref @@ -18,6 +19,41 @@ AnyListener = NoArgListener | InstanceListener | InstanceValueListener | InstanceNewOldListener +class _ListenerType(Enum): + """Enum to represent the type of listener""" + + NO_ARG = 0 + INSTANCE = 1 + INSTANCE_VALUE = 2 + INSTANCE_NEW_OLD = 3 + + @staticmethod + def detect_callback_type(callback: AnyListener) -> "_ListenerType": + """Normalizes the callback so every callback can be invoked with the same signature.""" + signature = inspect.signature(callback) + + # first detect the old *args default listener signatures + with suppress(TypeError): + signature.bind(..., ...) + return _ListenerType.INSTANCE_VALUE + + # check for the most common signature + with suppress(TypeError): + signature.bind() + return _ListenerType.NO_ARG + + # check for the other + with suppress(TypeError): + signature.bind(..., ..., ...) + return _ListenerType.INSTANCE_NEW_OLD + + with suppress(TypeError): + signature.bind(...) + return _ListenerType.INSTANCE + + raise TypeError("Callback is not callable") + + class _Obs(Generic[P]): """ Internal holder for Property value and change listeners @@ -29,14 +65,14 @@ def __init__(self, value: P): self.value = value # This will keep any added listener even if it is not referenced anymore # and would be garbage collected - self._listeners: dict[AnyListener, InstanceNewOldListener] = dict() + self._listeners: dict[AnyListener, _ListenerType] = dict() def add( self, callback: AnyListener, ): """Add a callback to the list of listeners""" - self._listeners[callback] = _Obs._normalize_callback(callback) + self._listeners[callback] = _ListenerType.detect_callback_type(callback) def remove(self, callback): """Remove a callback from the list of listeners""" @@ -44,31 +80,9 @@ def remove(self, callback): del self._listeners[callback] @property - def listeners(self) -> list[InstanceNewOldListener]: - return list(self._listeners.values()) - - @staticmethod - def _normalize_callback(callback) -> InstanceNewOldListener: - """Normalizes the callback so every callback can be invoked with the same signature.""" - signature = inspect.signature(callback) - - with suppress(TypeError): - signature.bind(1, 1) - return lambda instance, new, old: callback(instance, new) - - with suppress(TypeError): - signature.bind(1, 1, 1) - return lambda instance, new, old: callback(instance, new, old) - - with suppress(TypeError): - signature.bind(1) - return lambda instance, new, old: callback(instance) - - with suppress(TypeError): - signature.bind() - return lambda instance, new, old: callback() - - raise TypeError("Callback is not callable") + def listeners(self) -> list[tuple[AnyListener, _ListenerType]]: + """Returns a list of all listeners and type, both weak and strong.""" + return list(self._listeners.items()) class Property(Generic[P]): @@ -147,9 +161,16 @@ def dispatch(self, instance, value, old_value): """ obs = self._get_obs(instance) - for listener in obs.listeners: + for listener, _listener_type in obs.listeners: try: - listener(instance, value, old_value) + if _listener_type == _ListenerType.NO_ARG: + listener() # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE: + listener(instance) # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE_VALUE: + listener(instance, value) # type: ignore[call-arg] + elif _listener_type == _ListenerType.INSTANCE_NEW_OLD: + listener(instance, value, old_value) # type: ignore[call-arg] except Exception: print( f"Change listener for {instance}.{self.name} = {value} raised an exception!", @@ -157,7 +178,7 @@ def dispatch(self, instance, value, old_value): ) traceback.print_exc() - def bind(self, instance, callback): + def bind(self, instance: Any, callback: AnyListener): """Binds a function to the change event of the property. A reference to the function will be kept. @@ -200,7 +221,7 @@ def __set__(self, instance, value: P): self.set(instance, value) -def bind(instance, property: str, callback): +def bind(instance, property: str, callback: AnyListener): """Bind a function to the change event of the property. A reference to the function will be kept, so that it will be still @@ -220,6 +241,11 @@ class MyObject: my_obj.name = "Hans" # > Value of <__main__.MyObject ...> changed to Hans + Binding to a method of the Property owner itself can cause a memory leak, because the + owner is strongly referenced. Instead, bind the class method, which will be invoked with + the instance as first parameter. + + Args: instance: Instance owning the property property: Name of the property @@ -228,6 +254,7 @@ class MyObject: Returns: None """ + # TODO rename property to property_name for arcade 4.0 (just to be sure) t = type(instance) prop = getattr(t, property) if not isinstance(prop, Property): diff --git a/arcade/gui/widgets/__init__.py b/arcade/gui/widgets/__init__.py index 39dae93160..3d752dd39b 100644 --- a/arcade/gui/widgets/__init__.py +++ b/arcade/gui/widgets/__init__.py @@ -1,10 +1,12 @@ from __future__ import annotations +import weakref from abc import ABC from collections.abc import Iterable from enum import IntEnum from types import EllipsisType -from typing import TYPE_CHECKING, NamedTuple, TypeVar +from typing import Any, Generic, TYPE_CHECKING, NamedTuple, TypeVar, overload +from weakref import WeakKeyDictionary from pyglet.event import EVENT_HANDLED, EVENT_UNHANDLED, EventDispatcher from pyglet.math import Vec2 @@ -31,6 +33,7 @@ from arcade.gui.ui_manager import UIManager W = TypeVar("W", bound="UIWidget") +P = TypeVar("P") class FocusMode(IntEnum): @@ -51,6 +54,51 @@ class _ChildEntry(NamedTuple): data: dict +class WeakRef(Generic[P]): + """A weak reference to a UIWidget parent, which is used to prevent memory leaks.""" + + __slots__ = ("name", "obs") + name: str + """Attribute name of the property""" + obs: WeakKeyDictionary[Any, weakref.ref[P]] + """Weak dictionary to hold the values""" + + def __init__(self): + self.obs = WeakKeyDictionary() + + def get(self, instance: Any) -> P | None: + """Get value for owner instance""" + # If the value is not set, return None + value = self.obs.get(instance) + return value() if value else None + + def set(self, instance, value: P | None): + """Set value for owner instance""" + # Store a weak reference to the value + if value is None: + self.obs.pop(instance, None) + else: + self.obs[instance] = weakref.ref(value) + + def __set_name__(self, owner, name): + self.name = name + + @overload + def __get__(self, instance: None, instance_type) -> Self: ... + + @overload + def __get__(self, instance: Any, instance_type) -> P | None: ... + + def __get__(self, instance: Any | None, instance_type) -> Self | P | None: + """Get the value for the owner instance, or None if not set.""" + if instance is None: + return self + return self.get(instance) + + def __set__(self, instance, value: P | None): + self.set(instance, value) + + @copy_dunders_unimplemented class UIWidget(EventDispatcher, ABC): """The :class:`UIWidget` class is the base class required for creating widgets. @@ -71,6 +119,9 @@ class UIWidget(EventDispatcher, ABC): size_hint_max: max width and height in pixel """ + parent: WeakRef[UIManager | UIWidget | None] = WeakRef() + """A weak reference to the parent UIManager or UIWidget, + which does not prevent garbage collection of the parent.""" rect = Property(LBWH(0, 0, 1, 1)) visible = Property(True) focused = Property(False) @@ -113,7 +164,6 @@ def __init__( ): self._requires_render = True self.rect = LBWH(x, y, width, height) - self.parent: UIManager | UIWidget | None = None # Size hints are properties that can be used by layouts self.size_hint = size_hint @@ -126,21 +176,21 @@ def __init__( for child in children: self.add(child) - bind(self, "rect", self.trigger_full_render) - bind(self, "focused", self.trigger_full_render) + bind(self, "rect", UIWidget.trigger_full_render) + bind(self, "focused", UIWidget.trigger_full_render) bind( - self, "visible", self.trigger_full_render + self, "visible", UIWidget.trigger_full_render ) # TODO maybe trigger_parent_render would be enough - bind(self, "_children", self.trigger_render) - bind(self, "_border_width", self.trigger_render) - bind(self, "_border_color", self.trigger_render) - bind(self, "_bg_color", self.trigger_render) - bind(self, "_bg_tex", self.trigger_render) - bind(self, "_padding_top", self.trigger_render) - bind(self, "_padding_right", self.trigger_render) - bind(self, "_padding_bottom", self.trigger_render) - bind(self, "_padding_left", self.trigger_render) - bind(self, "_strong_background", self.trigger_render) + bind(self, "_children", UIWidget.trigger_render) + bind(self, "_border_width", UIWidget.trigger_render) + bind(self, "_border_color", UIWidget.trigger_render) + bind(self, "_bg_color", UIWidget.trigger_render) + bind(self, "_bg_tex", UIWidget.trigger_render) + bind(self, "_padding_top", UIWidget.trigger_render) + bind(self, "_padding_right", UIWidget.trigger_render) + bind(self, "_padding_bottom", UIWidget.trigger_render) + bind(self, "_padding_left", UIWidget.trigger_render) + bind(self, "_strong_background", UIWidget.trigger_render) def add(self, child: W, **kwargs) -> W: """Add a widget as a child. @@ -692,9 +742,9 @@ def __init__( self.interaction_buttons = interaction_buttons - bind(self, "pressed", self.trigger_render) - bind(self, "hovered", self.trigger_render) - bind(self, "disabled", self.trigger_render) + bind(self, "pressed", UIInteractiveWidget.trigger_render) + bind(self, "hovered", UIInteractiveWidget.trigger_render) + bind(self, "disabled", UIInteractiveWidget.trigger_render) def on_event(self, event: UIEvent) -> bool | None: """Handles mouse events and triggers on_click event if the widget is clicked. diff --git a/arcade/gui/widgets/buttons.py b/arcade/gui/widgets/buttons.py index adc0919e74..df216326f5 100644 --- a/arcade/gui/widgets/buttons.py +++ b/arcade/gui/widgets/buttons.py @@ -134,7 +134,7 @@ def __init__( if texture_disabled: self._textures["disabled"] = texture_disabled - bind(self, "_textures", self.trigger_render) + bind(self, "_textures", UITextureButton.trigger_render) # prepare label with default style _style = self.get_current_style() diff --git a/arcade/gui/widgets/image.py b/arcade/gui/widgets/image.py index f732753168..149a327244 100644 --- a/arcade/gui/widgets/image.py +++ b/arcade/gui/widgets/image.py @@ -55,9 +55,9 @@ def __init__( height=height if height else texture.height, **kwargs, ) - bind(self, "texture", self.trigger_render) - bind(self, "alpha", self.trigger_full_render) - bind(self, "angle", self.trigger_full_render) + bind(self, "texture", UIImage.trigger_render) + bind(self, "alpha", UIImage.trigger_full_render) + bind(self, "angle", UIImage.trigger_full_render) @override def do_render(self, surface: Surface): diff --git a/arcade/gui/widgets/layout.py b/arcade/gui/widgets/layout.py index 386532d707..2c17cf29a9 100644 --- a/arcade/gui/widgets/layout.py +++ b/arcade/gui/widgets/layout.py @@ -270,13 +270,13 @@ def __init__( self._size_hint_requires_update = True - bind(self, "_children", self._update_size_hints) - bind(self, "_border_width", self._update_size_hints) + bind(self, "_children", UIBoxLayout._trigger_size_hint_update) + bind(self, "_border_width", UIBoxLayout._trigger_size_hint_update) - bind(self, "_padding_left", self._update_size_hints) - bind(self, "_padding_right", self._update_size_hints) - bind(self, "_padding_top", self._update_size_hints) - bind(self, "_padding_bottom", self._update_size_hints) + bind(self, "_padding_left", UIBoxLayout._trigger_size_hint_update) + bind(self, "_padding_right", UIBoxLayout._trigger_size_hint_update) + bind(self, "_padding_top", UIBoxLayout._trigger_size_hint_update) + bind(self, "_padding_bottom", UIBoxLayout._trigger_size_hint_update) self._update_size_hints() @@ -288,11 +288,11 @@ def add(self, child: W, **kwargs) -> W: child: The widget to add to the layout. """ # subscribe to child's changes, which might affect the own size hint - bind(child, "_children", self._trigger_size_hint_update) - bind(child, "rect", self._trigger_size_hint_update) - bind(child, "size_hint", self._trigger_size_hint_update) - bind(child, "size_hint_min", self._trigger_size_hint_update) - bind(child, "size_hint_max", self._trigger_size_hint_update) + bind(child, "_children", UIBoxLayout._trigger_size_hint_update) + bind(child, "rect", UIBoxLayout._trigger_size_hint_update) + bind(child, "size_hint", UIBoxLayout._trigger_size_hint_update) + bind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update) + bind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update) return super().add(child, **kwargs) @@ -300,11 +300,11 @@ def add(self, child: W, **kwargs) -> W: def remove(self, child: UIWidget): """Remove a child from the layout.""" # unsubscribe from child's changes - unbind(child, "_children", self._trigger_size_hint_update) - unbind(child, "rect", self._trigger_size_hint_update) - unbind(child, "size_hint", self._trigger_size_hint_update) - unbind(child, "size_hint_min", self._trigger_size_hint_update) - unbind(child, "size_hint_max", self._trigger_size_hint_update) + unbind(child, "_children", UIBoxLayout._trigger_size_hint_update) + unbind(child, "rect", UIBoxLayout._trigger_size_hint_update) + unbind(child, "size_hint", UIBoxLayout._trigger_size_hint_update) + unbind(child, "size_hint_min", UIBoxLayout._trigger_size_hint_update) + unbind(child, "size_hint_max", UIBoxLayout._trigger_size_hint_update) return super().remove(child) @@ -514,13 +514,13 @@ def __init__( self.align_horizontal = align_horizontal self.align_vertical = align_vertical - bind(self, "_children", self._trigger_size_hint_update) - bind(self, "_border_width", self._trigger_size_hint_update) + bind(self, "_children", UIGridLayout._trigger_size_hint_update) + bind(self, "_border_width", UIGridLayout._trigger_size_hint_update) - bind(self, "_padding_left", self._trigger_size_hint_update) - bind(self, "_padding_right", self._trigger_size_hint_update) - bind(self, "_padding_top", self._trigger_size_hint_update) - bind(self, "_padding_bottom", self._trigger_size_hint_update) + bind(self, "_padding_left", UIGridLayout._trigger_size_hint_update) + bind(self, "_padding_right", UIGridLayout._trigger_size_hint_update) + bind(self, "_padding_top", UIGridLayout._trigger_size_hint_update) + bind(self, "_padding_bottom", UIGridLayout._trigger_size_hint_update) # initially update size hints # TODO is this required? @@ -547,11 +547,11 @@ def add( row_span: Number of rows the widget will stretch for. """ # subscribe to child's changes, which might affect the own size hint - bind(child, "_children", self._trigger_size_hint_update) - bind(child, "rect", self._trigger_size_hint_update) - bind(child, "size_hint", self._trigger_size_hint_update) - bind(child, "size_hint_min", self._trigger_size_hint_update) - bind(child, "size_hint_max", self._trigger_size_hint_update) + bind(child, "_children", UIGridLayout._trigger_size_hint_update) + bind(child, "rect", UIGridLayout._trigger_size_hint_update) + bind(child, "size_hint", UIGridLayout._trigger_size_hint_update) + bind(child, "size_hint_min", UIGridLayout._trigger_size_hint_update) + bind(child, "size_hint_max", UIGridLayout._trigger_size_hint_update) return super().add( child, diff --git a/arcade/gui/widgets/slider.py b/arcade/gui/widgets/slider.py index 12e632b648..9d2ccb545b 100644 --- a/arcade/gui/widgets/slider.py +++ b/arcade/gui/widgets/slider.py @@ -91,11 +91,11 @@ def __init__( self._cursor_width = self.height // 3 # trigger render on value changes - bind(self, "value", self.trigger_full_render) - bind(self, "value", self._ensure_step) - bind(self, "hovered", self.trigger_render) - bind(self, "pressed", self.trigger_render) - bind(self, "disabled", self.trigger_render) + bind(self, "value", UIBaseSlider.trigger_full_render) + bind(self, "value", UIBaseSlider._ensure_step) + bind(self, "hovered", UIBaseSlider.trigger_render) + bind(self, "pressed", UIBaseSlider.trigger_render) + bind(self, "disabled", UIBaseSlider.trigger_render) self.register_event_type("on_change") diff --git a/arcade/gui/widgets/text.py b/arcade/gui/widgets/text.py index 3123669d58..e4e8d6db8a 100644 --- a/arcade/gui/widgets/text.py +++ b/arcade/gui/widgets/text.py @@ -148,14 +148,14 @@ def __init__( if height: self._label.height = int(height) - bind(self, "rect", self._update_label) + bind(self, "rect", UILabel._update_label) # update size hint when border or padding changes - bind(self, "_border_width", self._update_size_hint_min) - bind(self, "_padding_left", self._update_size_hint_min) - bind(self, "_padding_right", self._update_size_hint_min) - bind(self, "_padding_top", self._update_size_hint_min) - bind(self, "_padding_bottom", self._update_size_hint_min) + bind(self, "_border_width", UILabel._update_size_hint_min) + bind(self, "_padding_left", UILabel._update_size_hint_min) + bind(self, "_padding_right", UILabel._update_size_hint_min) + bind(self, "_padding_top", UILabel._update_size_hint_min) + bind(self, "_padding_bottom", UILabel._update_size_hint_min) self._update_size_hint_min() @@ -570,11 +570,11 @@ def __init__( self.register_event_type("on_change") - bind(self, "hovered", self._apply_style) - bind(self, "pressed", self._apply_style) - bind(self, "invalid", self._apply_style) - bind(self, "disabled", self._apply_style) - bind(self, "_active", self._on_active_changed) + bind(self, "hovered", UIInputText._apply_style) + bind(self, "pressed", UIInputText._apply_style) + bind(self, "invalid", UIInputText._apply_style) + bind(self, "disabled", UIInputText._apply_style) + bind(self, "_active", UIInputText._on_active_changed) # initial style application self._apply_style() @@ -859,7 +859,7 @@ def __init__( multiline=multiline, ) - # bind(self, "rect", self._update_layout) + bind(self, "rect", self._update_layout) def fit_content(self): """Set the width and height of the text area to contain the whole text.""" diff --git a/arcade/gui/widgets/toggle.py b/arcade/gui/widgets/toggle.py index 5e1786efe2..bdc288d92a 100644 --- a/arcade/gui/widgets/toggle.py +++ b/arcade/gui/widgets/toggle.py @@ -82,8 +82,8 @@ def __init__( self.value = value self.register_event_type("on_change") - bind(self, "value", self.trigger_render) - bind(self, "value", self._dispatch_on_change_event) + bind(self, "value", UITextureToggle.trigger_render) + bind(self, "value", UITextureToggle._dispatch_on_change_event) super().__init__( x=x, diff --git a/tests/unit/gui/test_layouting_anchorlayout.py b/tests/unit/gui/test_layouting_anchorlayout.py index 4714f82123..0948dab833 100644 --- a/tests/unit/gui/test_layouting_anchorlayout.py +++ b/tests/unit/gui/test_layouting_anchorlayout.py @@ -51,8 +51,9 @@ def test_place_widget_relative_to_own_content_rect(window): assert dummy.top == 378 -def test_place_box_layout(window): - subject = UIAnchorLayout(width=500, height=500) +def test_place_box_layout(window, ui): + subject = UIAnchorLayout(width=500, height=500, size_hint=None) + ui.add(subject) box = UIBoxLayout() box.add(UIDummy(width=100, height=100)) @@ -60,7 +61,7 @@ def test_place_box_layout(window): subject.add(child=box, anchor_x="center_x", align_y=-20, anchor_y="top") - subject._do_layout() + ui.execute_layout() assert subject.rect == LBWH(0, 0, 500, 500) assert box.rect == LBWH(200, 280, 100, 200) diff --git a/tests/unit/gui/test_layouting_boxlayout.py b/tests/unit/gui/test_layouting_boxlayout.py index 377823fb31..20aadc3d24 100644 --- a/tests/unit/gui/test_layouting_boxlayout.py +++ b/tests/unit/gui/test_layouting_boxlayout.py @@ -26,8 +26,9 @@ def test_do_layout_vertical_with_initial_children(window): assert element_2.left == 100 -def test_do_layout_vertical_add_children(window): - group = UIBoxLayout(vertical=True) +def test_do_layout_vertical_add_children(window, ui): + group = UIBoxLayout(vertical=True, size_hint=None) + ui.add(group) element_1 = UIDummy() element_2 = UIDummy() @@ -36,7 +37,7 @@ def test_do_layout_vertical_add_children(window): group.add(element_2) group.rect = LBWH(100, 200, *group.size_hint_min) - group.do_layout() + ui.execute_layout() assert element_1.top == 400 assert element_1.bottom == 300 @@ -47,17 +48,18 @@ def test_do_layout_vertical_add_children(window): assert element_2.left == 100 -def test_do_layout_vertical_add_child_with_initial_children(window): +def test_do_layout_vertical_add_child_with_initial_children(window, ui): element_1 = UIDummy() element_2 = UIDummy() element_3 = UIDummy() - group = UIBoxLayout(vertical=True, children=[element_1, element_2]) + group = UIBoxLayout(vertical=True, children=[element_1, element_2], size_hint=None) + ui.add(group) group.add(element_3) group.rect = LBWH(100, 200, *group.size_hint_min) - group.do_layout() + ui.execute_layout() assert element_1.top == 500 assert element_1.bottom == 400 @@ -146,8 +148,9 @@ def test_do_layout_horizontal_with_initial_children(window): assert element_2.top == 300 -def test_do_layout_horizontal_add_children(window): - group = UIBoxLayout(vertical=False) +def test_do_layout_horizontal_add_children(window, ui): + group = UIBoxLayout(vertical=False, size_hint=None) + ui.add(group) element_1 = UIDummy() element_2 = UIDummy() @@ -156,7 +159,7 @@ def test_do_layout_horizontal_add_children(window): group.add(element_2) group.rect = LBWH(100, 200, *group.size_hint_min) - group.do_layout() + ui.execute_layout() assert element_1.left == 100 assert element_1.right == 200 @@ -191,16 +194,17 @@ def test_do_layout_horizontal_add_child_with_initial_children(window): assert element_3.top == 300 -def test_horizontal_group_keep_left_alignment_while_adding_children(window): +def test_horizontal_group_keep_left_alignment_while_adding_children(window, ui): element_1 = UIDummy() element_2 = UIDummy() element_3 = UIDummy() - group = UIBoxLayout(vertical=False, children=[element_1, element_2]) + group = UIBoxLayout(vertical=False, children=[element_1, element_2], size_hint=None) + ui.add(group) group.add(element_3) group.rect = LBWH(100, 200, *group.size_hint_min) - group.do_layout() + ui.execute_layout() assert group.left == 100 assert group.top == 300 @@ -259,31 +263,41 @@ def test_do_layout_horizontal_space_between(window): assert element_2.left == 210 -def test_size_hint_min_contains_children_vertically(window): - box = UIBoxLayout() +def test_size_hint_min_contains_children_vertically(window, ui): + box = UIBoxLayout(size_hint=None) + ui.add(box) box.add(UIDummy(width=100, height=100)) box.add(UIDummy(width=100, height=100)) + ui.execute_layout() + assert box.size_hint_min == (100, 200) -def test_size_hint_min_contains_children_horizontal(window): - box = UIBoxLayout(vertical=False) +def test_size_hint_min_contains_children_horizontal(window, ui): + box = UIBoxLayout(vertical=False, size_hint=None) + ui.add(box) + ui.add(box) box.add(UIDummy(width=100, height=100)) box.add(UIDummy(width=100, height=100)) + ui.execute_layout() + assert box.size_hint_min == (200, 100) -def test_size_hint_contains_border_and_padding(window): - box = UIBoxLayout() +def test_size_hint_contains_border_and_padding(window, ui): + box = UIBoxLayout(size_hint=None) + ui.add(box) box.with_border(width=3) box.with_padding(top=10, right=20, bottom=30, left=40) box.add(UIDummy(width=100, height=100)) box.add(UIDummy(width=100, height=100)) + ui.execute_layout() + assert box.size_hint_min == (100 + 2 * 3 + 20 + 40, 200 + 2 * 3 + 10 + 30) diff --git a/tests/unit/gui/test_property.py b/tests/unit/gui/test_property.py index b3d25b57c6..8b95e37a44 100644 --- a/tests/unit/gui/test_property.py +++ b/tests/unit/gui/test_property.py @@ -213,6 +213,44 @@ def test_gc_entries_are_collected(): assert len(MyObject.name.obs) == 0 +def test_obj_collected_when_using_class_method(): + class ObserverAndObject(Observer, MyObject): + pass + + obj = ObserverAndObject() + bind(obj, "name", ObserverAndObject.call) + + # Keeps referenced objects + gc.collect() + assert len(MyObject.name.obs) == 1 + + # delete ref and trigger gc + del obj + gc.collect() + + # No leftovers + assert len(MyObject.name.obs) == 0 + + +def test_gc_bound_methods_strongly_referenced(): + class ObserverAndObject(Observer, MyObject): + pass + + obj = ObserverAndObject() + bind(obj, "name", obj.call) + + # Keeps referenced objects + gc.collect() + assert len(ObserverAndObject.name.obs) == 1 + + # delete ref and trigger gc + del obj + gc.collect() + + # No leftovers + assert len(ObserverAndObject.name.obs) == 1 + + def test_gc_keeps_bound_methods(): observer = Observer() obj = MyObject() diff --git a/tests/unit/gui/test_uilabel.py b/tests/unit/gui/test_uilabel.py index a78828d166..7413deb1c9 100644 --- a/tests/unit/gui/test_uilabel.py +++ b/tests/unit/gui/test_uilabel.py @@ -79,9 +79,10 @@ def test_change_text_triggers_full_render_without_background(window): should fit the size to the text. This is not natively supported by either arcade.Text or pyglet.Label. Because text length variates between different os, we can only test boundaries, which indicate a proper implementation. """ + mock = Mock() label = UILabel(text="First Text") - label.parent = Mock() + label.parent = mock label.text = "Second Text" label.parent.trigger_render.assert_called_once() @@ -93,9 +94,10 @@ def test_change_text_triggers_render_with_background(window): should fit the size to the text. This is not natively supported by either arcade.Text or pyglet.Label. Because text length variates between different os, we can only test boundaries, which indicate a proper implementation. """ + mock = Mock() label = UILabel(text="First Text").with_background(color=Color(255, 255, 255, 255)) - label.parent = Mock() + label.parent = mock label.text = "Second Text" label.parent.trigger_render.assert_not_called() diff --git a/tests/unit/gui/test_widget_tree.py b/tests/unit/gui/test_widget_tree.py index 13b39fe78f..fee72f39c8 100644 --- a/tests/unit/gui/test_widget_tree.py +++ b/tests/unit/gui/test_widget_tree.py @@ -1,4 +1,6 @@ -from arcade.gui import UIEvent +import gc + +from arcade.gui import UIEvent, UIWidget from arcade.gui.widgets import UIDummy @@ -103,3 +105,49 @@ def test_iterate_widget_children(window): # THEN assert list(parent) == [child1, child2] + + +def test_chained_widgets_are_collected_by_gc(): + """ + Test that chained widgets are collected by garbage collector. + This is to ensure that there are no memory leaks when widgets are + added and removed in a chain. + """ + + def objs_in_memory(obj_type): + """Check if an object of a specific type is in memory.""" + return len([obj for obj in gc.get_objects() if isinstance(obj, obj_type)]) + + gc.collect() + start_count = objs_in_memory(UIWidget) + + root = UIWidget() + root.add(UIWidget()) + + # children are not collected until the parent is deleted + gc.collect() + assert objs_in_memory(UIWidget) == start_count + 2 + + del root + gc.collect() + + if objs_in_memory(UIWidget) > start_count: + print("Render object graph...") + import objgraph + + objgraph.show_chain( + objgraph.find_backref_chain( + [obj for obj in gc.get_objects() if isinstance(obj, UIWidget)][1], + objgraph.is_proper_module, + ), + # filename="chain.png", + ) + + # print("Render backrefs...") + # objgraph.show_backrefs( + # [[obj for obj in gc.get_objects() if isinstance(obj, UIWidget)][1]], + # max_depth=15, + # # filename="sample-graph.png", + # ) + + assert objs_in_memory(UIWidget) == start_count