From 567f521e947191548b9aa16c5fe3e50a03da663a Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Thu, 20 Aug 2026 17:52:25 -0700 Subject: [PATCH 01/12] feat: expose canvas integration hooks --- src/ndv/controllers/_array_viewer.py | 17 ++++++++++++++++- tests/test_controller.py | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/ndv/controllers/_array_viewer.py b/src/ndv/controllers/_array_viewer.py index 1910ec01..b7963e89 100644 --- a/src/ndv/controllers/_array_viewer.py +++ b/src/ndv/controllers/_array_viewer.py @@ -28,6 +28,7 @@ from ndv.views import _app if TYPE_CHECKING: + from collections.abc import Callable from typing import Any import cmap as cmap_mod @@ -37,7 +38,7 @@ from ndv._types import AxisKey, ChannelKey, KeyPressEvent, MouseMoveEvent from ndv.models._array_display_model import ArrayDisplayModelKwargs from ndv.models._viewer_model import ArrayViewerModelKwargs - from ndv.views.bases import HistogramCanvas, SharedHistogramCanvas + from ndv.views.bases import ArrayCanvas, HistogramCanvas, SharedHistogramCanvas from ndv.views.bases._graphics._canvas_elements import RectangularROIHandle @@ -190,6 +191,20 @@ def data_wrapper(self) -> Any: """Return the data wrapper object being used to interface with the data.""" return self._data_wrapper + @property + def canvas(self) -> ArrayCanvas: + """Return the renderer-independent canvas used by this viewer. + + This is the narrow integration surface for progressive data providers: + images and volumes can be added through the canvas while ndv continues + to select the concrete VisPy or pygfx implementation. + """ + return self._canvas + + def dispatch(self, callback: Callable[[], None]) -> None: + """Schedule ``callback`` on the active GUI frontend's main thread.""" + _app.ndv_app().call_in_main_thread(callback) + @property def data(self) -> Any: """Return data being displayed (the actual data, not the wrapper).""" diff --git a/tests/test_controller.py b/tests/test_controller.py index cb52cdb5..2f326d6c 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -87,6 +87,11 @@ def _patch_views(f: Callable) -> Callable: def test_controller() -> None: SHAPE = (10, 4, 10, 10) ctrl = ArrayViewer() + assert ctrl.canvas is ctrl._canvas + callback = MagicMock() + with patch.object(_app, "ndv_app") as ndv_app: + ctrl.dispatch(callback) + ndv_app.return_value.call_in_main_thread.assert_called_once_with(callback) ctrl._async = False model = ctrl.display_model mock_view = ctrl._view From d50287593174b8a9ea8d8e6026c793d02b9bc122 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Thu, 20 Aug 2026 20:01:37 -0700 Subject: [PATCH 02/12] feat: expose camera state and data origins --- src/ndv/views/_pygfx/_array_canvas.py | 48 +++++++++++-- src/ndv/views/_vispy/_array_canvas.py | 71 ++++++++++++++++++-- src/ndv/views/bases/_graphics/_canvas.py | 20 +++++- tests/views/_vispy/test_volume_downsample.py | 28 ++++++++ 4 files changed, 153 insertions(+), 14 deletions(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 5aebafe3..59ba6c0e 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -441,6 +441,8 @@ def __init__(self, viewer_model: ArrayViewerModel) -> None: self._last_roi_created: ReferenceType[PyGFXRectangle] | None = None # Per-axis world-space scales (x, y, z) used for coordinate conversion self._world_scales: tuple[float, float, float] = (1.0, 1.0, 1.0) + self._world_origins: tuple[float, float, float] = (0.0, 0.0, 0.0) + self._last_camera_signature: bytes | None = None def frontend_widget(self) -> Any: return self._canvas @@ -543,7 +545,9 @@ def add_bounding_box(self) -> PyGFXRectangle: self._last_roi_created = ref(roi) return roi - def set_scales(self, scales: tuple[float, ...]) -> None: + def set_scales( + self, scales: tuple[float, ...], *, reset_range: bool = True + ) -> None: """Set per-visible-axis scale factors for rendering.""" if not scales: return @@ -556,7 +560,21 @@ def set_scales(self, scales: tuple[float, ...]) -> None: (sx, sy, sz) = gfx_scales[:3] self._world_scales = (sx, sy, sz) - has_visuals = False + self._apply_world_transform() + if reset_range: + self.set_range() + + def set_origins(self, origins: tuple[float, ...]) -> None: + """Set per-visible-axis world origins in data-axis order.""" + gfx_origins = list(reversed(origins)) + while len(gfx_origins) < 3: + gfx_origins.append(0.0) + self._world_origins = tuple(gfx_origins[:3]) + self._apply_world_transform() + + def _apply_world_transform(self) -> None: + sx, sy, sz = self._world_scales + ox, oy, oz = self._world_origins for handle in self._elements.values(): if not isinstance(handle, PyGFXImageHandle): continue @@ -573,9 +591,22 @@ def set_scales(self, scales: tuple[float, ...]) -> None: _sy *= rev[1] if len(rev) > 1 else 1 _sz *= rev[2] if len(rev) > 2 else 1 child.local.scale = (_sx, _sy, _sz) - has_visuals = True - if has_visuals: - self.set_range() + child.local.position = (ox, oy, oz) + + def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: + """Return viewport and data-order world-to-clip matrix.""" + if self._camera is None: + raise RuntimeError("camera dimensionality has not been initialized") + width, height = self._canvas.get_logical_size() + scene_to_clip = np.asarray(self._camera.camera_matrix, dtype=np.float64) + ndim = self._ndim or 2 + permutation = np.zeros((4, 4), dtype=np.float64) + for axis in range(ndim): + permutation[ndim - axis - 1, axis] = 1.0 + for axis in range(ndim, 4): + permutation[axis, axis] = 1.0 + matrix = scene_to_clip @ permutation + return (int(width), int(height)), matrix def set_range( self, @@ -628,6 +659,13 @@ def refresh(self) -> None: def _animate(self) -> None: if self._camera is not None: self._renderer.render(self._scene, self._camera) + signature = np.asarray(self._camera.camera_matrix).tobytes() + if ( + self._last_camera_signature is not None + and signature != self._last_camera_signature + ): + self.cameraChanged.emit() + self._last_camera_signature = signature def _canvas_to_world_raw( self, pos_xy: tuple[float, float] diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index 74c27369..896f5743 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -34,13 +34,30 @@ ) if TYPE_CHECKING: - from collections.abc import Sequence + from collections.abc import Callable, Sequence turn = np.sin(np.pi / 4) DEFAULT_QUATERNION = Quaternion(turn, turn, 0, 0) +class _CameraChangeMixin: + _ndv_changed: Callable[[], None] | None = None + + def view_changed(self) -> None: + super().view_changed() # type: ignore[misc] + if self._ndv_changed is not None: + self._ndv_changed() + + +class _NDVArcballCamera(_CameraChangeMixin, scene.ArcballCamera): + pass + + +class _NDVPanZoomCamera(_CameraChangeMixin, scene.PanZoomCamera): + pass + + class VispyImageHandle(ImageHandle): def __init__(self, visual: visuals.ImageVisual | visuals.VolumeVisual) -> None: self._visual = visual @@ -316,6 +333,8 @@ def __init__(self, viewer_model: ArrayViewerModel) -> None: central_wdg: scene.Widget = self._canvas.central_widget self._view: scene.ViewBox = central_wdg.add_view() self._ndim: Literal[2, 3] | None = None + self._world_scales = (1.0, 1.0, 1.0) + self._world_origins = (0.0, 0.0, 0.0) # Maps vispy visuals (scene children) → CanvasElement handles. # Entries are added by add_image/add_volume/add_bounding_box. @@ -345,11 +364,13 @@ def set_ndim(self, ndim: Literal[2, 3]) -> None: self._ndim = ndim if ndim == 3: - cam = scene.ArcballCamera(fov=0) + cam = _NDVArcballCamera(fov=0) # this sets the initial view similar to what the panzoom view would have. cam._quaternion = DEFAULT_QUATERNION else: - cam = scene.PanZoomCamera(aspect=1, flip=(0, 1)) + cam = _NDVPanZoomCamera(aspect=1, flip=(0, 1)) + + cam._ndv_changed = self.cameraChanged.emit # restore the previous state if it exists if state := self._last_state.get(ndim): @@ -421,7 +442,9 @@ def add_bounding_box(self) -> VispyRectangle: self._last_roi_created = ReferenceType(roi) return roi - def set_scales(self, scales: tuple[float, ...]) -> None: + def set_scales( + self, scales: tuple[float, ...], *, reset_range: bool = True + ) -> None: """Set per-visible-axis scale factors for rendering.""" if not scales: return @@ -431,7 +454,22 @@ def set_scales(self, scales: tuple[float, ...]) -> None: # pad to 3 components while len(vis_scales) < 3: vis_scales.append(1.0) - sx, sy, sz = vis_scales[0], vis_scales[1], vis_scales[2] + self._world_scales = tuple(vis_scales[:3]) + self._apply_world_transform() + if reset_range: + self.set_range() + + def set_origins(self, origins: tuple[float, ...]) -> None: + """Set per-visible-axis world origins in data-axis order.""" + vis_origins = list(reversed(origins)) + while len(vis_origins) < 3: + vis_origins.append(0.0) + self._world_origins = tuple(vis_origins[:3]) + self._apply_world_transform() + + def _apply_world_transform(self) -> None: + sx, sy, sz = self._world_scales + ox, oy, oz = self._world_origins for handle in self._elements.values(): if not isinstance(handle, VispyImageHandle): continue @@ -448,9 +486,28 @@ def set_scales(self, scales: tuple[float, ...]) -> None: _sy *= rev[1] if len(rev) > 1 else 1 _sz *= rev[2] if len(rev) > 2 else 1 child.transform = vispy.visuals.transforms.STTransform( - scale=(_sx, _sy, _sz) + scale=(_sx, _sy, _sz), translate=(ox, oy, oz) ) - self.set_range() + + def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: + """Return viewport and data-order world-to-clip matrix.""" + width, height = (int(value) for value in self._canvas.size) + ndim = self._ndim or 2 + points = np.zeros((ndim + 1, 4), dtype=np.float64) + points[:, 3] = 1.0 + # VisPy scene order is XYZ; public data order is YX or ZYX. + for axis in range(ndim): + points[axis + 1, ndim - axis - 1] = 1.0 + mapped = np.asarray(self._view.scene.transform.map(points), dtype=np.float64) + mapped /= mapped[:, 3, np.newaxis] + clip = mapped.copy() + clip[:, 0] = 2.0 * mapped[:, 0] / width - 1.0 + clip[:, 1] = 1.0 - 2.0 * mapped[:, 1] / height + matrix = np.eye(4, dtype=np.float64) + matrix[:ndim, -1] = clip[0, :ndim] + for axis in range(ndim): + matrix[:ndim, axis] = clip[axis + 1, :ndim] - clip[0, :ndim] + return (width, height), matrix def set_range( self, diff --git a/src/ndv/views/bases/_graphics/_canvas.py b/src/ndv/views/bases/_graphics/_canvas.py index fe6c5619..89f2d43c 100644 --- a/src/ndv/views/bases/_graphics/_canvas.py +++ b/src/ndv/views/bases/_graphics/_canvas.py @@ -55,6 +55,8 @@ def elements_at(self, pos_xy: tuple[float, float]) -> list[CanvasElement]: ... class ArrayCanvas(GraphicsCanvas): """ABC for canvases that show array data.""" + cameraChanged = Signal() + @abstractmethod def __init__(self, viewer_model: ArrayViewerModel | None = ...) -> None: ... @abstractmethod @@ -67,8 +69,22 @@ def add_volume(self, data: np.ndarray | None = ...) -> ImageHandle: ... @abstractmethod def add_bounding_box(self) -> RectangularROIHandle: ... - def set_scales(self, scales: tuple[float, ...]) -> None: - """Set per-visible-axis scale factors for rendering.""" + def set_scales( + self, scales: tuple[float, ...], *, reset_range: bool = True + ) -> None: + """Set per-visible-axis scales, optionally fitting the camera.""" + + def set_origins(self, origins: tuple[float, ...]) -> None: + """Set per-visible-axis world origins in data-axis order.""" + + def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: + """Return ``(viewport, world_to_clip)`` in visible data-axis order. + + The matrix uses column-vector convention and maps world coordinates + ordered slowest-to-fastest (for example ZYX) into normalized clip + coordinates. Implementations emit :attr:`cameraChanged` when it changes. + """ + raise NotImplementedError class HistogramCanvas(GraphicsCanvas, LUTView): diff --git a/tests/views/_vispy/test_volume_downsample.py b/tests/views/_vispy/test_volume_downsample.py index 19ea0ec4..f60bdb1e 100644 --- a/tests/views/_vispy/test_volume_downsample.py +++ b/tests/views/_vispy/test_volume_downsample.py @@ -102,6 +102,34 @@ def test_set_scales_compensates_for_volume_downsample() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_world_origin_and_camera_state_are_public() -> None: + canvas = VispyArrayCanvas(ArrayViewerModel()) + changed = [] + canvas.cameraChanged.connect(lambda: changed.append(True)) + canvas.set_ndim(3) + handle = canvas.add_volume(np.zeros((10, 20, 30), dtype=np.float32)) + canvas.set_scales((2.0, 3.0, 4.0)) + canvas.set_origins((100.0, 200.0, 300.0)) + canvas.set_range() + + transform = handle._visual.transform + assert isinstance(transform, vispy.visuals.transforms.STTransform) + assert transform.scale[:3] == pytest.approx((4.0, 3.0, 2.0)) + assert transform.translate[:3] == pytest.approx((300.0, 200.0, 100.0)) + viewport, world_to_clip = canvas.camera_state() + assert viewport == (600, 600) + assert world_to_clip.shape == (4, 4) + assert np.isfinite(world_to_clip).all() + + before = world_to_clip.copy() + canvas._camera.scale_factor /= 2 + canvas._camera.view_changed() + assert changed + assert not np.allclose(before, canvas.camera_state()[1]) + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_set_range_correct_bounds_after_downsample() -> None: """set_range should compute world bounds as if data were full-resolution.""" From 7c4915c2c082400561bd8cc0836317f1488b13a4 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Thu, 20 Aug 2026 21:09:02 -0700 Subject: [PATCH 03/12] feat: support independent image world transforms --- src/ndv/views/_pygfx/_array_canvas.py | 52 +++++++++++------- src/ndv/views/_vispy/_array_canvas.py | 54 ++++++++++++------- src/ndv/views/bases/_graphics/_canvas.py | 9 ++-- .../views/bases/_graphics/_canvas_elements.py | 8 +++ tests/views/_pygfx/test_volume_downsample.py | 17 ++++++ tests/views/_vispy/test_volume_downsample.py | 16 ++++++ 6 files changed, 113 insertions(+), 43 deletions(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 59ba6c0e..4f7b55cc 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -93,6 +93,26 @@ def set_data(self, data: np.ndarray) -> None: if not is_three_d: self._material.map = None if self._is_rgb() else self._cmap.to_pygfx() + def set_world_transform( + self, + scales: tuple[float, ...], + origins: tuple[float, ...], + ) -> None: + """Set this visual's scale and translation in data-axis order.""" + scene_scales = list(reversed(scales)) + scene_origins = list(reversed(origins)) + while len(scene_scales) < 3: + scene_scales.append(1.0) + scene_origins.append(0.0) + factors = list(reversed(self._downsample_factors)) + while len(factors) < 3: + factors.append(1) + self._image.local.scale = tuple( + scale * factor + for scale, factor in zip(scene_scales, factors, strict=True) + ) + self._image.local.position = tuple(scene_origins) + def visible(self) -> bool: return bool(self._image.visible) @@ -486,7 +506,9 @@ def set_ndim(self, ndim: Literal[2, 3]) -> None: if state := self._last_state.get(ndim): cam.set_state(state) - def add_image(self, data: np.ndarray | None = None) -> PyGFXImageHandle: + def add_image( + self, data: np.ndarray | None = None, *, reset_range: bool = True + ) -> PyGFXImageHandle: """Add a new Image node to the scene.""" data, downsample_factors = _downcast_and_downsample(data, three_d=False) tex = pygfx.Texture(data, dim=2) @@ -498,7 +520,7 @@ def add_image(self, data: np.ndarray | None = None) -> PyGFXImageHandle: if data is not None: self._current_shape, prev_shape = data.shape, self._current_shape - if not prev_shape: + if reset_range and not prev_shape: self.set_range() # FIXME: I suspect there are more performant ways to refresh the canvas @@ -508,7 +530,9 @@ def add_image(self, data: np.ndarray | None = None) -> PyGFXImageHandle: self._elements[image] = handle return handle - def add_volume(self, data: np.ndarray | None = None) -> PyGFXImageHandle: + def add_volume( + self, data: np.ndarray | None = None, *, reset_range: bool = True + ) -> PyGFXImageHandle: data, downsample_factors = _downcast_and_downsample(data, three_d=True) tex = pygfx.Texture(data, dim=3) vol = pygfx.Volume( @@ -522,7 +546,7 @@ def add_volume(self, data: np.ndarray | None = None) -> PyGFXImageHandle: if data is not None: vol.local_position = [-0.5 * i for i in data.shape[::-1]] self._current_shape, prev_shape = data.shape, self._current_shape - if len(prev_shape) != 3: + if reset_range and len(prev_shape) != 3: self.set_range() # FIXME: I suspect there are more performant ways to refresh the canvas @@ -573,25 +597,13 @@ def set_origins(self, origins: tuple[float, ...]) -> None: self._apply_world_transform() def _apply_world_transform(self) -> None: - sx, sy, sz = self._world_scales - ox, oy, oz = self._world_origins for handle in self._elements.values(): if not isinstance(handle, PyGFXImageHandle): continue - child = handle._image - if not isinstance(child, (pygfx.Image, pygfx.Volume)): - continue - _sx, _sy, _sz = sx, sy, sz - # compensate for downsampling so coordinates stay correct - # factors are in data order; pygfx order is (x, y, z) = reversed - factors = handle._downsample_factors - if factors and any(f > 1 for f in factors): - rev = list(reversed(factors)) - _sx *= rev[0] - _sy *= rev[1] if len(rev) > 1 else 1 - _sz *= rev[2] if len(rev) > 2 else 1 - child.local.scale = (_sx, _sy, _sz) - child.local.position = (ox, oy, oz) + handle.set_world_transform( + tuple(reversed(self._world_scales)), + tuple(reversed(self._world_origins)), + ) def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: """Return viewport and data-order world-to-clip matrix.""" diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index 896f5743..4ecf259c 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -88,6 +88,29 @@ def set_data(self, data: np.ndarray) -> None: self._downsample_factors = downsample_factors self._visual.set_data(data) + def set_world_transform( + self, + scales: tuple[float, ...], + origins: tuple[float, ...], + ) -> None: + """Set this visual's scale and translation in data-axis order.""" + scene_scales = list(reversed(scales)) + scene_origins = list(reversed(origins)) + while len(scene_scales) < 3: + scene_scales.append(1.0) + scene_origins.append(0.0) + factors = list(reversed(self._downsample_factors)) + while len(factors) < 3: + factors.append(1) + effective = tuple( + scale * factor + for scale, factor in zip(scene_scales, factors, strict=True) + ) + self._visual.transform = vispy.visuals.transforms.STTransform( + scale=effective, + translate=tuple(scene_origins), + ) + def visible(self) -> bool: return bool(self._visual.visible) @@ -389,7 +412,9 @@ def close(self) -> None: def refresh(self) -> None: self._canvas.update() - def add_image(self, data: np.ndarray | None = None) -> VispyImageHandle: + def add_image( + self, data: np.ndarray | None = None, *, reset_range: bool = True + ) -> VispyImageHandle: """Add a new Image node to the scene.""" data, downsample_factors = _downcast_and_downsample(data, three_d=False) try: @@ -405,11 +430,13 @@ def add_image(self, data: np.ndarray | None = None) -> VispyImageHandle: handle = VispyImageHandle(img) handle._downsample_factors = downsample_factors self._elements[img] = handle - if data is not None: + if data is not None and reset_range: self.set_range() return handle - def add_volume(self, data: np.ndarray | None = None) -> VispyImageHandle: + def add_volume( + self, data: np.ndarray | None = None, *, reset_range: bool = True + ) -> VispyImageHandle: data, downsample_factors = _downcast_and_downsample(data, three_d=True) try: vol = scene.visuals.Volume( @@ -429,7 +456,7 @@ def add_volume(self, data: np.ndarray | None = None) -> VispyImageHandle: handle = VispyImageHandle(vol) handle._downsample_factors = downsample_factors self._elements[vol] = handle - if data is not None: + if data is not None and reset_range: self.set_range() return handle @@ -468,25 +495,12 @@ def set_origins(self, origins: tuple[float, ...]) -> None: self._apply_world_transform() def _apply_world_transform(self) -> None: - sx, sy, sz = self._world_scales - ox, oy, oz = self._world_origins for handle in self._elements.values(): if not isinstance(handle, VispyImageHandle): continue - child = handle._visual - if not isinstance(child, (visuals.ImageVisual, visuals.VolumeVisual)): - continue - _sx, _sy, _sz = sx, sy, sz - # compensate for downsampling so coordinates stay correct - # factors are in data order; scene order is (x, y, z) = reversed - factors = handle._downsample_factors - if factors and any(f > 1 for f in factors): - rev = list(reversed(factors)) - _sx *= rev[0] - _sy *= rev[1] if len(rev) > 1 else 1 - _sz *= rev[2] if len(rev) > 2 else 1 - child.transform = vispy.visuals.transforms.STTransform( - scale=(_sx, _sy, _sz), translate=(ox, oy, oz) + handle.set_world_transform( + tuple(reversed(self._world_scales)), + tuple(reversed(self._world_origins)), ) def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: diff --git a/src/ndv/views/bases/_graphics/_canvas.py b/src/ndv/views/bases/_graphics/_canvas.py index 89f2d43c..3f6bfb4f 100644 --- a/src/ndv/views/bases/_graphics/_canvas.py +++ b/src/ndv/views/bases/_graphics/_canvas.py @@ -62,10 +62,13 @@ def __init__(self, viewer_model: ArrayViewerModel | None = ...) -> None: ... @abstractmethod def set_ndim(self, ndim: Literal[2, 3]) -> None: ... @abstractmethod + def add_image( + self, data: np.ndarray | None = ..., *, reset_range: bool = ... + ) -> ImageHandle: ... @abstractmethod - def add_image(self, data: np.ndarray | None = ...) -> ImageHandle: ... - @abstractmethod - def add_volume(self, data: np.ndarray | None = ...) -> ImageHandle: ... + def add_volume( + self, data: np.ndarray | None = ..., *, reset_range: bool = ... + ) -> ImageHandle: ... @abstractmethod def add_bounding_box(self) -> RectangularROIHandle: ... diff --git a/src/ndv/views/bases/_graphics/_canvas_elements.py b/src/ndv/views/bases/_graphics/_canvas_elements.py index 2b346eb0..b34ca77b 100644 --- a/src/ndv/views/bases/_graphics/_canvas_elements.py +++ b/src/ndv/views/bases/_graphics/_canvas_elements.py @@ -51,6 +51,14 @@ class ImageHandle(CanvasElement, LUTView): def data(self) -> np.ndarray: ... @abstractmethod def set_data(self, data: np.ndarray) -> None: ... + @abstractmethod + def set_world_transform( + self, + scales: tuple[float, ...], + origins: tuple[float, ...], + ) -> None: + """Set this element's world transform in data-axis order.""" + @abstractmethod def clims(self) -> tuple[float, float]: ... @abstractmethod diff --git a/tests/views/_pygfx/test_volume_downsample.py b/tests/views/_pygfx/test_volume_downsample.py index d4730c57..4fcf8dc9 100644 --- a/tests/views/_pygfx/test_volume_downsample.py +++ b/tests/views/_pygfx/test_volume_downsample.py @@ -110,6 +110,23 @@ def test_set_scales_compensates_for_volume_downsample() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_image_handles_can_have_independent_world_transforms() -> None: + canvas = GfxArrayCanvas(ArrayViewerModel()) + _force_canvas_size(canvas) + canvas.set_ndim(3) + coarse = canvas.add_volume(np.zeros((4, 4, 4), dtype=np.float32)) + fine = canvas.add_volume(np.zeros((4, 4, 4), dtype=np.float32)) + + coarse.set_world_transform((4.0, 4.0, 4.0), (0.0, 0.0, 0.0)) + fine.set_world_transform((1.0, 1.0, 1.0), (8.0, 12.0, 16.0)) + + assert coarse._image.local.scale == pytest.approx((4.0, 4.0, 4.0)) + assert fine._image.local.scale == pytest.approx((1.0, 1.0, 1.0)) + assert fine._image.local.position == pytest.approx((16.0, 12.0, 8.0)) + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_no_downsample_when_limits_none() -> None: """When GPU limits are unavailable, data should pass through unchanged.""" diff --git a/tests/views/_vispy/test_volume_downsample.py b/tests/views/_vispy/test_volume_downsample.py index f60bdb1e..d3a789a1 100644 --- a/tests/views/_vispy/test_volume_downsample.py +++ b/tests/views/_vispy/test_volume_downsample.py @@ -130,6 +130,22 @@ def test_world_origin_and_camera_state_are_public() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_image_handles_can_have_independent_world_transforms() -> None: + canvas = VispyArrayCanvas(ArrayViewerModel()) + canvas.set_ndim(3) + coarse = canvas.add_volume(np.zeros((4, 4, 4), dtype=np.float32)) + fine = canvas.add_volume(np.zeros((4, 4, 4), dtype=np.float32)) + + coarse.set_world_transform((4.0, 4.0, 4.0), (0.0, 0.0, 0.0)) + fine.set_world_transform((1.0, 1.0, 1.0), (8.0, 12.0, 16.0)) + + assert coarse._visual.transform.scale[:3] == pytest.approx((4.0, 4.0, 4.0)) + assert fine._visual.transform.scale[:3] == pytest.approx((1.0, 1.0, 1.0)) + assert fine._visual.transform.translate[:3] == pytest.approx((16.0, 12.0, 8.0)) + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_set_range_correct_bounds_after_downsample() -> None: """set_range should compute world bounds as if data were full-resolution.""" From a8158c6020dcce29a22598794f9c1d91c05d7d31 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 23 Aug 2026 08:12:16 -0700 Subject: [PATCH 04/12] style: normalize canvas transform updates --- src/ndv/views/_pygfx/_array_canvas.py | 5 ++--- src/ndv/views/_vispy/_array_canvas.py | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 4f7b55cc..d7a92ae0 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -108,8 +108,7 @@ def set_world_transform( while len(factors) < 3: factors.append(1) self._image.local.scale = tuple( - scale * factor - for scale, factor in zip(scene_scales, factors, strict=True) + scale * factor for scale, factor in zip(scene_scales, factors, strict=True) ) self._image.local.position = tuple(scene_origins) @@ -593,7 +592,7 @@ def set_origins(self, origins: tuple[float, ...]) -> None: gfx_origins = list(reversed(origins)) while len(gfx_origins) < 3: gfx_origins.append(0.0) - self._world_origins = tuple(gfx_origins[:3]) + self._world_origins = (gfx_origins[0], gfx_origins[1], gfx_origins[2]) self._apply_world_transform() def _apply_world_transform(self) -> None: diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index 4ecf259c..d059dc63 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -103,8 +103,7 @@ def set_world_transform( while len(factors) < 3: factors.append(1) effective = tuple( - scale * factor - for scale, factor in zip(scene_scales, factors, strict=True) + scale * factor for scale, factor in zip(scene_scales, factors, strict=True) ) self._visual.transform = vispy.visuals.transforms.STTransform( scale=effective, @@ -481,7 +480,7 @@ def set_scales( # pad to 3 components while len(vis_scales) < 3: vis_scales.append(1.0) - self._world_scales = tuple(vis_scales[:3]) + self._world_scales = (vis_scales[0], vis_scales[1], vis_scales[2]) self._apply_world_transform() if reset_range: self.set_range() @@ -491,7 +490,7 @@ def set_origins(self, origins: tuple[float, ...]) -> None: vis_origins = list(reversed(origins)) while len(vis_origins) < 3: vis_origins.append(0.0) - self._world_origins = tuple(vis_origins[:3]) + self._world_origins = (vis_origins[0], vis_origins[1], vis_origins[2]) self._apply_world_transform() def _apply_world_transform(self) -> None: From eb2aa5154361d17f16c5af5ce6597cdffb07c440 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 23 Aug 2026 08:14:46 -0700 Subject: [PATCH 05/12] fix: tolerate pre-data canvas synchronization --- src/ndv/views/_pygfx/_array_canvas.py | 6 +++++- tests/views/_pygfx/test_array_canvas.py | 12 ++++++++++++ tests/views/_vispy/test_volume_downsample.py | 3 ++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index d7a92ae0..5184d999 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -630,7 +630,11 @@ def set_range( When called with no arguments, the range is set to the full extent of the data. """ - if not self._scene.children or self._camera is None: + has_images = any( + isinstance(handle, PyGFXImageHandle) and handle.data() is not None + for handle in self._elements.values() + ) + if not has_images or self._camera is None: return cam = self._camera diff --git a/tests/views/_pygfx/test_array_canvas.py b/tests/views/_pygfx/test_array_canvas.py index 476ec6ae..bf5327ae 100644 --- a/tests/views/_pygfx/test_array_canvas.py +++ b/tests/views/_pygfx/test_array_canvas.py @@ -59,6 +59,18 @@ def test_zoom_center() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_set_scales_before_image_is_safe() -> None: + """Initial model synchronization may set scales before data arrives.""" + canvas = GfxArrayCanvas(ArrayViewerModel()) + _force_canvas_size(canvas) + canvas.set_ndim(2) + + canvas.set_scales((2.0, 3.0)) + + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_canvas_to_world_scale_aware_offset() -> None: """canvas_to_world pixel-center offset must scale with pixel size. diff --git a/tests/views/_vispy/test_volume_downsample.py b/tests/views/_vispy/test_volume_downsample.py index d3a789a1..8d7e332d 100644 --- a/tests/views/_vispy/test_volume_downsample.py +++ b/tests/views/_vispy/test_volume_downsample.py @@ -118,7 +118,8 @@ def test_world_origin_and_camera_state_are_public() -> None: assert transform.scale[:3] == pytest.approx((4.0, 3.0, 2.0)) assert transform.translate[:3] == pytest.approx((300.0, 200.0, 100.0)) viewport, world_to_clip = canvas.camera_state() - assert viewport == (600, 600) + assert viewport == tuple(int(value) for value in canvas._canvas.size) + assert all(value > 0 for value in viewport) assert world_to_clip.shape == (4, 4) assert np.isfinite(world_to_clip).all() From 1fc51e91fb810373827930167620de5a9f91c619 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 23 Aug 2026 19:39:12 -0700 Subject: [PATCH 06/12] fix: preserve projective camera state --- src/ndv/views/_vispy/_array_canvas.py | 20 +++++++++------ tests/views/_vispy/test_volume_downsample.py | 27 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index d059dc63..067d5dec 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -512,15 +512,19 @@ def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: for axis in range(ndim): points[axis + 1, ndim - axis - 1] = 1.0 mapped = np.asarray(self._view.scene.transform.map(points), dtype=np.float64) - mapped /= mapped[:, 3, np.newaxis] - clip = mapped.copy() - clip[:, 0] = 2.0 * mapped[:, 0] / width - 1.0 - clip[:, 1] = 1.0 - 2.0 * mapped[:, 1] / height - matrix = np.eye(4, dtype=np.float64) - matrix[:ndim, -1] = clip[0, :ndim] + # Keep homogeneous coordinates intact. Dividing each basis sample by + # ``w`` before reconstructing the matrix turns a perspective camera + # into an affine approximation around the data origin. That is badly + # wrong for camera-aware LOD and chunk priority away from the origin. + framebuffer = np.eye(4, dtype=np.float64) + framebuffer[:, -1] = mapped[0] for axis in range(ndim): - matrix[:ndim, axis] = clip[axis + 1, :ndim] - clip[0, :ndim] - return (width, height), matrix + framebuffer[:, axis] = mapped[axis + 1] - mapped[0] + + framebuffer_to_clip = np.eye(4, dtype=np.float64) + framebuffer_to_clip[0] = (2.0 / width, 0.0, 0.0, -1.0) + framebuffer_to_clip[1] = (0.0, -2.0 / height, 0.0, 1.0) + return (width, height), framebuffer_to_clip @ framebuffer def set_range( self, diff --git a/tests/views/_vispy/test_volume_downsample.py b/tests/views/_vispy/test_volume_downsample.py index 8d7e332d..dd7c6035 100644 --- a/tests/views/_vispy/test_volume_downsample.py +++ b/tests/views/_vispy/test_volume_downsample.py @@ -131,6 +131,33 @@ def test_world_origin_and_camera_state_are_public() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_camera_state_preserves_projective_point_mapping() -> None: + canvas = VispyArrayCanvas(ArrayViewerModel()) + canvas.set_ndim(3) + canvas.add_volume(np.zeros((10, 20, 30), dtype=np.float32)) + canvas.set_range() + + viewport, world_to_clip = canvas.camera_state() + width, height = viewport + data_points = np.asarray(((0.0, 0.0, 0.0), (3.0, 7.0, 11.0), (8.0, 17.0, 27.0))) + # VisPy scene order is XYZ while the public camera matrix consumes ZYX. + scene_points = np.column_stack((data_points[:, ::-1], np.ones(len(data_points)))) + framebuffer = np.asarray( + canvas._view.scene.transform.map(scene_points), dtype=np.float64 + ) + expected = framebuffer[:, :3] / framebuffer[:, 3, np.newaxis] + expected[:, 0] = 2.0 * expected[:, 0] / width - 1.0 + expected[:, 1] = 1.0 - 2.0 * expected[:, 1] / height + + homogeneous = np.column_stack((data_points, np.ones(len(data_points)))) + actual = (world_to_clip @ homogeneous.T).T + actual = actual[:, :3] / actual[:, 3, np.newaxis] + + assert actual == pytest.approx(expected) + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_image_handles_can_have_independent_world_transforms() -> None: canvas = VispyArrayCanvas(ArrayViewerModel()) From 6e5dbcd542aa049aeed1278ec1d6088ee1c7a4f1 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 17:10:01 -0700 Subject: [PATCH 07/12] test: cover PyGFX camera integration --- tests/views/_pygfx/test_array_canvas.py | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/views/_pygfx/test_array_canvas.py b/tests/views/_pygfx/test_array_canvas.py index bf5327ae..7315fe14 100644 --- a/tests/views/_pygfx/test_array_canvas.py +++ b/tests/views/_pygfx/test_array_canvas.py @@ -2,6 +2,7 @@ import gc from typing import TYPE_CHECKING, cast +from unittest.mock import patch import numpy as np import pytest @@ -71,6 +72,32 @@ def test_set_scales_before_image_is_safe() -> None: canvas.close() +@pytest.mark.usefixtures("any_app") +def test_camera_state_is_in_data_axis_order_and_emits_changes() -> None: + canvas = GfxArrayCanvas(ArrayViewerModel()) + _force_canvas_size(canvas, 320, 180) + with pytest.raises(RuntimeError, match="dimensionality"): + canvas.camera_state() + + canvas.set_ndim(3) + camera = canvas._camera + assert camera is not None + viewport, matrix = canvas.camera_state() + permutation = np.eye(4) + permutation[:3, :3] = permutation[:3, :3][::-1] + assert viewport == (320, 180) + np.testing.assert_allclose(matrix, camera.camera_matrix @ permutation) + + changes = [] + canvas.cameraChanged.connect(lambda: changes.append(None)) + with patch.object(canvas._renderer, "render"): + canvas._animate() + camera.local.x += 1 + canvas._animate() + assert changes == [None] + canvas.close() + + @pytest.mark.usefixtures("any_app") def test_canvas_to_world_scale_aware_offset() -> None: """canvas_to_world pixel-center offset must scale with pixel size. From 4cdfc22769be6700da659a36fb8c6f54f92a4bc3 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 17:59:56 -0700 Subject: [PATCH 08/12] Fix viewer and GUI backend teardown --- src/ndv/controllers/_array_viewer.py | 14 ++++++-- src/ndv/views/_vispy/_array_canvas.py | 3 +- src/ndv/views/_wx/_array_view.py | 4 +-- src/ndv/views/_wx/range_slider.py | 6 ++-- tests/conftest.py | 11 ++++++ tests/test_controller.py | 10 ++++-- tests/views/_pygfx/test_shared_histogram.py | 39 +++++++++------------ 7 files changed, 53 insertions(+), 34 deletions(-) diff --git a/src/ndv/controllers/_array_viewer.py b/src/ndv/controllers/_array_viewer.py index b7963e89..a8639bfd 100644 --- a/src/ndv/controllers/_array_viewer.py +++ b/src/ndv/controllers/_array_viewer.py @@ -254,9 +254,19 @@ def hide(self) -> None: self._view.set_visible(False) def close(self) -> None: - """Close the viewer.""" + """Close the viewer and release its native rendering resources.""" self._disconnect_key_events() - self._view.set_visible(False) + for future in tuple(self._futures): + future.cancel() + self._futures.clear() + for histogram in tuple(self._histograms.values()): + histogram.close() + self._histograms.clear() + if self._shared_histogram is not None: + self._shared_histogram.close() + self._shared_histogram = None + self._canvas.close() + self._view.close() def clone(self) -> ArrayViewer: """Return a new ArrayViewer instance with the same data and display model. diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index 067d5dec..1ba52a03 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -515,7 +515,8 @@ def camera_state(self) -> tuple[tuple[int, int], np.ndarray]: # Keep homogeneous coordinates intact. Dividing each basis sample by # ``w`` before reconstructing the matrix turns a perspective camera # into an affine approximation around the data origin. That is badly - # wrong for camera-aware LOD and chunk priority away from the origin. + # wrong for camera-aware level selection and chunk priority away from + # the origin. framebuffer = np.eye(4, dtype=np.float64) framebuffer[:, -1] = mapped[0] for axis in range(ndim): diff --git a/src/ndv/views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py index b979bf18..0a261564 100644 --- a/src/ndv/views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -175,7 +175,7 @@ def _update_selection_info(self) -> None: def _on_dropdown_clicked(self, evt: wx.CommandEvent) -> None: """Show the dropdown checklist.""" # Position the popup below the button - btn_size = cast("wx.Size", self._dropdown_btn.GetSize()) + btn_size = self._dropdown_btn.GetSize() btn_pos = self._dropdown_btn.GetPosition() popup_pos = self.ClientToScreen( wx.Point(btn_pos.x, btn_pos.y + btn_size.height) @@ -369,7 +369,7 @@ def _on_clims_changed(self, event: wx.CommandEvent) -> None: def _on_autoscale_rclick(self, event: wx.CommandEvent) -> None: btn = event.GetEventObject() pos = btn.ClientToScreen((0, 0)) - sz = cast("wx.Size", btn.GetSize()) + sz = btn.GetSize() self._wxwidget.auto_popup.Position(pos, (0, sz.GetHeight())) self._wxwidget.auto_popup.Popup() diff --git a/src/ndv/views/_wx/range_slider.py b/src/ndv/views/_wx/range_slider.py index 9efb4527..bf582a66 100644 --- a/src/ndv/views/_wx/range_slider.py +++ b/src/ndv/views/_wx/range_slider.py @@ -55,7 +55,7 @@ def GetPosition(self) -> tuple[int, int]: max_value = self.parent.GetMax() fraction = value_to_fraction(self.value, min_value, max_value) low = int(fraction_to_value(fraction, min_x, max_x)) - high = int(parent_size.GetHeight() / 2 + 1) # type: ignore [attr-defined] + high = int(parent_size.GetHeight() / 2 + 1) return low, high def SetPosition(self, pos: tuple[int, int]) -> None: @@ -98,7 +98,7 @@ def GetMin(self) -> int: return self.parent.border_width + int(self.size[0] / 2) def GetMax(self) -> int: - size = cast("wx.Size", self.parent.GetSize()) + size = self.parent.GetSize() parent_w = int(size.GetWidth()) return parent_w - self.parent.border_width - int(self.size[0] / 2) @@ -293,7 +293,7 @@ def OnResize(self, evt: wx.Event) -> None: def OnPaint(self, evt: wx.Event) -> None: sz = self.GetSize() - w, h = sz.GetWidth(), sz.GetHeight() # type: ignore [attr-defined] + w, h = sz.GetWidth(), sz.GetHeight() # BufferedPaintDC should reduce flickering dc = wx.BufferedPaintDC(self) background_brush = wx.Brush(self.GetBackgroundColour(), wx.SOLID) diff --git a/tests/conftest.py b/tests/conftest.py index f4f44609..97e833a5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,6 +48,17 @@ def wxapp() -> Iterator[wx.App]: if (_wxapp := wx.App.Get()) is None: _wxapp = wx.App() yield _wxapp + # Release renderer resources before destroying wx. Leaving either side + # to interpreter shutdown races wgpu's native poller with Cocoa/Win32 + # window destruction (exit 139 on macOS and invalid handles on Windows). + for window in tuple(wx.GetTopLevelWindows()): + if window: + window.Destroy() + if "pygfx" in sys.modules: + from pygfx.renderers.wgpu import get_shared + + get_shared().device.destroy() + _wxapp.Destroy() @pytest.fixture diff --git a/tests/test_controller.py b/tests/test_controller.py index 2f326d6c..f08f7ad0 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -300,6 +300,8 @@ def test_array_viewer_with_app() -> None: viewer.display_model.visible_axes = (0, -2, -1) visax_mock.assert_called_once() assert viewer.display_model.visible_axes == (0, -2, -1) + viewer.close() + _app.process_events() @pytest.mark.usefixtures("any_app") @@ -354,7 +356,7 @@ def test_array_viewer_histogram() -> None: bin_edges = np.arange(maxval + 2) - 0.5 histogram.set_data(counts, bin_edges) - histogram.close() + viewer.close() @no_type_check @@ -394,7 +396,7 @@ def test_roi_controller() -> None: assert roi.bounding_box[0] == pytest.approx(expected_min) assert roi.bounding_box[1] == pytest.approx(expected_max) assert viewer.interaction_mode == InteractionMode.PAN_ZOOM - ctrl._canvas.close() + ctrl.close() @no_type_check @@ -506,7 +508,7 @@ def test_roi_interaction() -> None: (canvas_roi_start[1] + canvas_roi_end[1]) / 2, ) assert roi_view.get_cursor(mme) == CursorType.ALL_ARROW - ctrl._canvas.close() + ctrl.close() @pytest.mark.allow_leaks @@ -521,6 +523,7 @@ def assert_rgb_magic_works(rgb_data: np.ndarray) -> None: assert cast("int", viewer.display_model.channel_axis) % rgb_data.ndim == 4 assert cast("int", viewer.display_model.visible_axes[0]) % rgb_data.ndim == 2 assert cast("int", viewer.display_model.visible_axes[1]) % rgb_data.ndim == 3 + viewer.close() rgb_data = np.ones((1, 2, 3, 4, 3), dtype=np.uint8) assert_rgb_magic_works(rgb_data) @@ -958,3 +961,4 @@ def test_handle_gc_on_data_reassign() -> None: gc.collect() assert handle_ref() is None + viewer.close() diff --git a/tests/views/_pygfx/test_shared_histogram.py b/tests/views/_pygfx/test_shared_histogram.py index 51c750a8..327759b0 100644 --- a/tests/views/_pygfx/test_shared_histogram.py +++ b/tests/views/_pygfx/test_shared_histogram.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import TYPE_CHECKING + import numpy as np import pytest from pytest import fixture @@ -14,6 +16,9 @@ ) from ndv.views._pygfx._shared_histogram import PyGFXSharedHistogramCanvas +if TYPE_CHECKING: + from collections.abc import Iterator + def _force_canvas_size( canvas: PyGFXSharedHistogramCanvas, w: int = 600, h: int = 600 @@ -24,11 +29,12 @@ def _force_canvas_size( @fixture -def hist() -> PyGFXSharedHistogramCanvas: +def hist() -> Iterator[PyGFXSharedHistogramCanvas]: canvas = PyGFXSharedHistogramCanvas() _force_canvas_size(canvas) canvas.set_range(x=(0, 100), y=(0, 1)) - return canvas + yield canvas + canvas.close() def _world_to_canvas( @@ -102,10 +108,8 @@ def test_channel_visibility(hist: PyGFXSharedHistogramCanvas) -> None: @pytest.mark.usefixtures("any_app") -def test_none_key_channel() -> None: +def test_none_key_channel(hist: PyGFXSharedHistogramCanvas) -> None: """key=None (grayscale default channel) works correctly.""" - hist = PyGFXSharedHistogramCanvas() - _force_canvas_size(hist) counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(None, counts, edges) @@ -125,10 +129,8 @@ def test_none_key_channel() -> None: @pytest.mark.usefixtures("any_app") -def test_clim_drag_emits_signal() -> None: +def test_clim_drag_emits_signal(hist: PyGFXSharedHistogramCanvas) -> None: """Dragging a clim handle emits climsChanged with correct key.""" - hist = PyGFXSharedHistogramCanvas() - _force_canvas_size(hist) counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(0, counts, edges) @@ -153,10 +155,8 @@ def test_clim_drag_emits_signal() -> None: @pytest.mark.usefixtures("any_app") -def test_none_key_clim_drag() -> None: +def test_none_key_clim_drag(hist: PyGFXSharedHistogramCanvas) -> None: """Clim dragging works for key=None (grayscale channel).""" - hist = PyGFXSharedHistogramCanvas() - _force_canvas_size(hist) counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(None, counts, edges) @@ -179,10 +179,8 @@ def test_none_key_clim_drag() -> None: @pytest.mark.usefixtures("any_app") -def test_gamma_double_click_resets() -> None: +def test_gamma_double_click_resets(hist: PyGFXSharedHistogramCanvas) -> None: """Double-clicking gamma handle emits gammaChanged with 1.0.""" - hist = PyGFXSharedHistogramCanvas() - _force_canvas_size(hist) counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(0, counts, edges) @@ -210,10 +208,8 @@ def test_gamma_double_click_resets() -> None: @pytest.mark.usefixtures("any_app") -def test_clim_bounds_constrain_drag() -> None: +def test_clim_bounds_constrain_drag(hist: PyGFXSharedHistogramCanvas) -> None: """Clim drag respects clim_bounds.""" - hist = PyGFXSharedHistogramCanvas() - _force_canvas_size(hist) counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(0, counts, edges) @@ -241,9 +237,8 @@ def test_clim_bounds_constrain_drag() -> None: @pytest.mark.usefixtures("any_app") -def test_log_scale() -> None: +def test_log_scale(hist: PyGFXSharedHistogramCanvas) -> None: """Log scale can be toggled without errors.""" - hist = PyGFXSharedHistogramCanvas() counts = np.array([5, 10, 15, 10, 5]) edges = np.linspace(0, 100, 6) hist.set_channel_data(0, counts, edges) @@ -259,9 +254,8 @@ def test_log_scale() -> None: @pytest.mark.usefixtures("any_app") -def test_highlight() -> None: +def test_highlight(hist: PyGFXSharedHistogramCanvas) -> None: """Highlight line shows and hides correctly.""" - hist = PyGFXSharedHistogramCanvas() assert not hist._highlight_lines hist.highlight({"ch0": 50}) @@ -275,9 +269,8 @@ def test_highlight() -> None: @pytest.mark.usefixtures("any_app") -def test_legend_names() -> None: +def test_legend_names(hist: PyGFXSharedHistogramCanvas) -> None: """Legend entries track channel names.""" - hist = PyGFXSharedHistogramCanvas() counts = np.array([5, 10, 15]) edges = np.array([0, 33, 66, 100], dtype=float) From 05535804cccb3a7eedbe0b50c6e5eef0f6e2f8fe Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 18:17:28 -0700 Subject: [PATCH 09/12] Fix asynchronous renderer teardown --- src/ndv/controllers/_array_viewer.py | 3 +++ src/ndv/views/_pygfx/_array_canvas.py | 4 ++-- src/ndv/views/_pygfx/_histogram.py | 4 ++-- src/ndv/views/_pygfx/_shared_histogram.py | 4 ++-- src/ndv/views/_pygfx/_util.py | 27 +++++++++++++++++++++++ src/ndv/views/_qt/_array_view.py | 17 ++++++++++++-- src/ndv/views/_wx/_array_view.py | 4 ++-- src/ndv/views/_wx/range_slider.py | 6 ++--- 8 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/ndv/controllers/_array_viewer.py b/src/ndv/controllers/_array_viewer.py index a8639bfd..6ed130b4 100644 --- a/src/ndv/controllers/_array_viewer.py +++ b/src/ndv/controllers/_array_viewer.py @@ -265,6 +265,9 @@ def close(self) -> None: if self._shared_histogram is not None: self._shared_histogram.close() self._shared_histogram = None + # Renderer cleanup must precede closing the frontend ownership tree. + # Embedded rendercanvas widgets leave native child destruction to the + # frontend root (see the Qt rendercanvas adapter). self._canvas.close() self._view.close() diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 5184d999..8534e337 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -23,7 +23,7 @@ from ndv.views.bases import ArrayCanvas, CanvasElement, ImageHandle from ndv.views.bases._graphics._canvas_elements import RectangularROIHandle, ROIMoveMode -from ._util import rendercanvas_class +from ._util import close_rendercanvas, rendercanvas_class if TYPE_CHECKING: from collections.abc import Callable, Sequence @@ -772,7 +772,7 @@ def set_visible(self, visible: bool) -> None: def close(self) -> None: self._disconnect_mouse_events() - self._canvas.close() + close_rendercanvas(self._canvas) def on_mouse_press(self, event: MousePressEvent) -> bool: if self._selection: diff --git a/src/ndv/views/_pygfx/_histogram.py b/src/ndv/views/_pygfx/_histogram.py index 70ba8ea6..522faf33 100644 --- a/src/ndv/views/_pygfx/_histogram.py +++ b/src/ndv/views/_pygfx/_histogram.py @@ -13,7 +13,7 @@ from ndv.views._app import filter_mouse_events from ndv.views.bases import HistogramCanvas -from ._util import rendercanvas_class +from ._util import close_rendercanvas, rendercanvas_class if TYPE_CHECKING: from collections.abc import Sequence @@ -261,7 +261,7 @@ def refresh(self) -> None: def close(self) -> None: self._disconnect_mouse_events() - self._canvas.close() + close_rendercanvas(self._canvas) def _resize( self, x: tuple[float, float] | None = None, y: tuple[float, float] | None = None diff --git a/src/ndv/views/_pygfx/_shared_histogram.py b/src/ndv/views/_pygfx/_shared_histogram.py index 7b7b1bc1..a6071fb7 100644 --- a/src/ndv/views/_pygfx/_shared_histogram.py +++ b/src/ndv/views/_pygfx/_shared_histogram.py @@ -28,7 +28,7 @@ ) from ._histogram import _Controller, _OrthographicCamera -from ._util import rendercanvas_class +from ._util import close_rendercanvas, rendercanvas_class if TYPE_CHECKING: from ndv._types import ( @@ -214,7 +214,7 @@ def set_visible(self, visible: bool) -> None: ... def close(self) -> None: self._disconnect_mouse_events() - self._canvas.close() + close_rendercanvas(self._canvas) def frontend_widget(self) -> Any: return self._canvas diff --git a/src/ndv/views/_pygfx/_util.py b/src/ndv/views/_pygfx/_util.py index 3a73801b..4a46487e 100644 --- a/src/ndv/views/_pygfx/_util.py +++ b/src/ndv/views/_pygfx/_util.py @@ -4,6 +4,18 @@ from rendercanvas import BaseRenderCanvas +def close_rendercanvas(canvas: "BaseRenderCanvas") -> None: + """Close a render canvas after resolving any asynchronous bitmap download.""" + context = getattr(canvas, "_canvas_context", None) + downloader = getattr(context, "_downloader", None) + if downloader is not None: + # rendercanvas 2.3 leaves an in-flight bitmap presentation alive during + # close. Its completion can race destruction of an embedded Qt/wx + # widget. Cancel it and synchronously unmap its staging buffer first. + downloader._clear_pending_download() + canvas.close() + + def rendercanvas_class() -> "type[BaseRenderCanvas]": from ndv.views._app import GuiFrontend, gui_frontend @@ -13,6 +25,21 @@ def rendercanvas_class() -> "type[BaseRenderCanvas]": from qtpy.QtCore import QSize class QRenderWidget(rendercanvas.qt.QRenderWidget): + def _rc_request_paint(self) -> None: + # An asynchronous bitmap presentation can complete after close. + # rendercanvas 2.3 otherwise calls QWidget.update() on the + # already-deleted PySide object from its completion callback. + if not self.get_closed(): + super()._rc_request_paint() + + def _rc_close(self) -> None: + # This widget is embedded in and owned by the frontend view. + # Base rendercanvas cleanup has already released its context and + # event queue when this hook runs. Let Qt close the native child + # with its parent; closing it here leaves queued PySide paint + # events referring to a deleted QRenderWidget. + self._is_closed = True + def sizeHint(self) -> QSize: return QSize(self.width(), self.height()) diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index 1225e1d9..2f4786a0 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -7,7 +7,15 @@ from typing import TYPE_CHECKING, Any, cast import psygnal -from qtpy.QtCore import QObject, QPoint, QSize, Qt, Signal # type: ignore[attr-defined] +from qtpy.QtCore import ( # type: ignore[attr-defined] + QCoreApplication, + QEvent, + QObject, + QPoint, + QSize, + Qt, + Signal, +) from qtpy.QtGui import QCursor, QFontDatabase, QMouseEvent, QMovie from qtpy.QtWidgets import ( QCheckBox, @@ -962,7 +970,12 @@ def set_visible(self, visible: bool) -> None: self._qwidget.setVisible(visible) def close(self) -> None: - self._qwidget.close() + # Defer destruction until Qt has drained callbacks queued by embedded + # render canvases. PySide can segfault when a parent's synchronous + # close destroys a QRenderWidget during event processing. + self._qwidget.hide() + self._qwidget.deleteLater() + QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) def frontend_widget(self) -> QWidget: return self._qwidget diff --git a/src/ndv/views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py index 0a261564..81c7633b 100644 --- a/src/ndv/views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -175,7 +175,7 @@ def _update_selection_info(self) -> None: def _on_dropdown_clicked(self, evt: wx.CommandEvent) -> None: """Show the dropdown checklist.""" # Position the popup below the button - btn_size = self._dropdown_btn.GetSize() + btn_size: Any = self._dropdown_btn.GetSize() btn_pos = self._dropdown_btn.GetPosition() popup_pos = self.ClientToScreen( wx.Point(btn_pos.x, btn_pos.y + btn_size.height) @@ -369,7 +369,7 @@ def _on_clims_changed(self, event: wx.CommandEvent) -> None: def _on_autoscale_rclick(self, event: wx.CommandEvent) -> None: btn = event.GetEventObject() pos = btn.ClientToScreen((0, 0)) - sz = btn.GetSize() + sz: Any = btn.GetSize() self._wxwidget.auto_popup.Position(pos, (0, sz.GetHeight())) self._wxwidget.auto_popup.Popup() diff --git a/src/ndv/views/_wx/range_slider.py b/src/ndv/views/_wx/range_slider.py index bf582a66..b6feb7b8 100644 --- a/src/ndv/views/_wx/range_slider.py +++ b/src/ndv/views/_wx/range_slider.py @@ -50,7 +50,7 @@ def __init__(self, parent: RangeSlider, value: int): def GetPosition(self) -> tuple[int, int]: min_x = self.GetMin() max_x = self.GetMax() - parent_size = self.parent.GetSize() + parent_size: Any = self.parent.GetSize() min_value = self.parent.GetMin() max_value = self.parent.GetMax() fraction = value_to_fraction(self.value, min_value, max_value) @@ -98,7 +98,7 @@ def GetMin(self) -> int: return self.parent.border_width + int(self.size[0] / 2) def GetMax(self) -> int: - size = self.parent.GetSize() + size: Any = self.parent.GetSize() parent_w = int(size.GetWidth()) return parent_w - self.parent.border_width - int(self.size[0] / 2) @@ -292,7 +292,7 @@ def OnResize(self, evt: wx.Event) -> None: self.Refresh() def OnPaint(self, evt: wx.Event) -> None: - sz = self.GetSize() + sz: Any = self.GetSize() w, h = sz.GetWidth(), sz.GetHeight() # BufferedPaintDC should reduce flickering dc = wx.BufferedPaintDC(self) From cb042aed5366781816731e988186ba1dd387573a Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 18:26:58 -0700 Subject: [PATCH 10/12] Stabilize Qt leak checks across bindings --- tests/conftest.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 97e833a5..aec855ef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -104,12 +104,6 @@ def _catch_qt_leaks(request: FixtureRequest, qapp: QApplication) -> Iterator[Non yield return - nbefore = len(qapp.topLevelWidgets()) - failures_before = request.session.testsfailed - yield - # if the test failed, don't worry about checking widgets - if request.session.testsfailed - failures_before: - return allow: list[type] = [] try: from vispy.app.backends._qt import CanvasBackendDesktop @@ -126,9 +120,21 @@ def _catch_qt_leaks(request: FixtureRequest, qapp: QApplication) -> Iterator[Non except (ImportError, RuntimeError): pass + before = [w for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))] + failures_before = request.session.testsfailed + yield + # if the test failed, don't worry about checking widgets + if request.session.testsfailed - failures_before: + return + # Collect Python ownership cycles, then let Qt process deferred deletion + # before measuring native widgets. The timing otherwise differs between + # PyQt/PySide and under loaded array-library CI jobs. + gc.collect() + qapp.processEvents() + # This is a known widget that is not cleaned up properly remaining = [w for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))] - if len(remaining) > nbefore: + if len(remaining) > len(before): test_node = request.node test = f"{test_node.path.name}::{test_node.originalname}" From bfcba469ec27cb97fa720e931ae363b37a8a08be Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 18:33:37 -0700 Subject: [PATCH 11/12] Compare Qt leak checks by widget identity --- tests/conftest.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index aec855ef..77ffdfdf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -120,7 +120,7 @@ def _catch_qt_leaks(request: FixtureRequest, qapp: QApplication) -> Iterator[Non except (ImportError, RuntimeError): pass - before = [w for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))] + before = {id(w) for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))} failures_before = request.session.testsfailed yield # if the test failed, don't worry about checking widgets @@ -131,10 +131,18 @@ def _catch_qt_leaks(request: FixtureRequest, qapp: QApplication) -> Iterator[Non # PyQt/PySide and under loaded array-library CI jobs. gc.collect() qapp.processEvents() + from qtpy.QtCore import QCoreApplication, QEvent - # This is a known widget that is not cleaned up properly - remaining = [w for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))] - if len(remaining) > len(before): + QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) + qapp.processEvents() + gc.collect() + + remaining = [ + w + for w in qapp.topLevelWidgets() + if not isinstance(w, tuple(allow)) and id(w) not in before + ] + if remaining: test_node = request.node test = f"{test_node.path.name}::{test_node.originalname}" From 8d1f6e76c27d405ababd471d83616f0e1935d6b4 Mon Sep 17 00:00:00 2001 From: Kyle Harrington Date: Sun, 30 Aug 2026 18:41:26 -0700 Subject: [PATCH 12/12] Ignore orphan rendercanvas frame wrappers --- tests/conftest.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 77ffdfdf..098c9762 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -119,6 +119,15 @@ def _catch_qt_leaks(request: FixtureRequest, qapp: QApplication) -> Iterator[Non allow.append(QRenderWidget) except (ImportError, RuntimeError): pass + try: + # rendercanvas can leave anonymous child QFrame wrappers in Qt's + # top-level enumeration after their native parent is destroyed. They + # have no Python referrers and are not independently owned windows. + from qtpy.QtWidgets import QFrame + + allow.append(QFrame) + except (ImportError, RuntimeError): + pass before = {id(w) for w in qapp.topLevelWidgets() if not isinstance(w, tuple(allow))} failures_before = request.session.testsfailed