From 9a1fe094f74be83108eae3a8b67408b9f614c92d Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Thu, 17 Oct 2024 13:44:43 -0400 Subject: [PATCH] imviz matched zoom: use center & radius instead of corners (#3215) * imviz matched zoom: use center & radius instead of corners * apply modification to PixelMatchedZoomMixin * delay callback across all zoom state attrs * update tests --- CHANGES.rst | 2 ++ .../cubeviz/plugins/tests/test_tools.py | 4 +++- jdaviz/configs/cubeviz/plugins/tools.py | 8 ++++++-- jdaviz/configs/imviz/plugins/tools.py | 6 +++++- jdaviz/configs/imviz/tests/test_tools.py | 16 ++++++++++----- jdaviz/core/tools.py | 20 +++++++++++++++++-- 6 files changed, 45 insertions(+), 11 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index ff8674968a..732515e20b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -64,6 +64,8 @@ Other Changes and Additions Bug Fixes --------- +- Improved performance and removed jittering for the matched box zoom tool. [#3215] + Cubeviz ^^^^^^^ diff --git a/jdaviz/configs/cubeviz/plugins/tests/test_tools.py b/jdaviz/configs/cubeviz/plugins/tests/test_tools.py index f5db618cd8..021c5e6c46 100644 --- a/jdaviz/configs/cubeviz/plugins/tests/test_tools.py +++ b/jdaviz/configs/cubeviz/plugins/tests/test_tools.py @@ -150,7 +150,9 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube, flux_viewer.state.x_max = 40 flux_viewer.state.y_max = 35 v = uncert_viewer - assert (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max) == (20, 40, 15, 35) + assert v.state.zoom_center_x == flux_viewer.state.zoom_center_x + assert v.state.zoom_center_y == flux_viewer.state.zoom_center_y + assert v.state.zoom_radius == flux_viewer.state.zoom_radius t_linkedpan.deactivate() diff --git a/jdaviz/configs/cubeviz/plugins/tools.py b/jdaviz/configs/cubeviz/plugins/tools.py index 5914151dcc..33ca136330 100644 --- a/jdaviz/configs/cubeviz/plugins/tools.py +++ b/jdaviz/configs/cubeviz/plugins/tools.py @@ -17,8 +17,12 @@ class _PixelMatchedZoomMixin(_MatchedZoomMixin): - match_keys = ('x_min', 'x_max', 'y_min', 'y_max') - disable_matched_zoom_in_other_viewer = False + match_axes = [] + disable_matched_zoom_in_other_viewer = True + + @property + def match_keys(self): + return ['zoom_center_x', 'zoom_center_y', 'zoom_radius'] def _is_matched_viewer(self, viewer): return isinstance(viewer, BqplotImageView) diff --git a/jdaviz/configs/imviz/plugins/tools.py b/jdaviz/configs/imviz/plugins/tools.py index 3ae81d4890..3c127c3d55 100644 --- a/jdaviz/configs/imviz/plugins/tools.py +++ b/jdaviz/configs/imviz/plugins/tools.py @@ -16,9 +16,13 @@ class _ImvizMatchedZoomMixin(_MatchedZoomMixin): - match_keys = ('x_min', 'x_max', 'y_min', 'y_max') + match_axes = [] disable_matched_zoom_in_other_viewer = True + @property + def match_keys(self): + return ['zoom_center_x', 'zoom_center_y', 'zoom_radius'] + def _is_matched_viewer(self, viewer): return isinstance(viewer, BqplotImageView) diff --git a/jdaviz/configs/imviz/tests/test_tools.py b/jdaviz/configs/imviz/tests/test_tools.py index 1d9845718d..8a62c7a65f 100644 --- a/jdaviz/configs/imviz/tests/test_tools.py +++ b/jdaviz/configs/imviz/tests/test_tools.py @@ -15,19 +15,25 @@ def test_panzoom_tools(self): t = v.toolbar.tools['jdaviz:boxzoommatch'] # original limits (x_min, x_max, y_min, y_max): -0.5 9.5 -0.5 9.5 + # original limits (zoom_center_x, zoom_center_y, zoom_radius): 4.5 4.5 5.0 original_limits = (v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max) assert_allclose(original_limits, (-0.5, 9.5, -0.5, 9.5)) + original_center_rad = (v.state.zoom_center_x, v.state.zoom_center_y, v.state.zoom_radius) + assert_allclose(original_center_rad, (4.5, 4.5, 5.0)) assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), original_limits) # noqa + assert_allclose((v2.state.zoom_center_x, v2.state.zoom_center_y, v2.state.zoom_radius), original_center_rad) # noqa t.activate() t.save_prev_zoom() v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max = (1, 8, 1, 8) - # second viewer should match these changes - assert (v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max) == (1, 8, 1, 8) + # second viewer should match these changes, wrt zoom center and radius + assert v2.state.zoom_center_x == v.state.zoom_center_x + assert v2.state.zoom_center_y == v.state.zoom_center_y + assert v2.state.zoom_radius == v.state.zoom_radius v.toolbar.tools['jdaviz:prevzoom'].activate() - # both should revert since they're still linked - assert_allclose((v.state.x_min, v.state.x_max, v.state.y_min, v.state.y_max), original_limits) # noqa - assert_allclose((v2.state.x_min, v2.state.x_max, v2.state.y_min, v2.state.y_max), original_limits) # noqa + # both should revert since they're still linked (boxzoommatch will re-activate) + assert_allclose((v.state.zoom_center_x, v.state.zoom_center_y, v.state.zoom_radius), original_center_rad) # noqa + assert_allclose((v2.state.zoom_center_x, v2.state.zoom_center_y, v2.state.zoom_radius), original_center_rad) # noqa v.toolbar.tools['jdaviz:prevzoom'].activate() # both should revert since they're still linked diff --git a/jdaviz/core/tools.py b/jdaviz/core/tools.py index df7e63494f..3571c1db9a 100644 --- a/jdaviz/core/tools.py +++ b/jdaviz/core/tools.py @@ -3,6 +3,7 @@ import numpy as np from echo import delay_callback +from functools import cached_property from glue.config import viewer_tool from glue.core import HubListener from glue.viewers.common.tool import Tool @@ -67,6 +68,14 @@ def match_keys(self): keys += [f'{ax}_min', f'{ax}_max'] return keys + @cached_property + def delay_callback_keys(self): + all_keys = ['x_min', 'x_max', 'y_min', 'y_max', + 'zoom_center_x', 'zoom_center_y', 'zoom_radius'] + return [k for k in all_keys + if np.all([hasattr(v.state, k) + for v in self._iter_matched_viewers(include_self=True)])] + def activate(self): if self.disable_matched_zoom_in_other_viewer: # mapping limits are not guaranteed to roundtrip, so we need to disable @@ -115,8 +124,8 @@ def on_limits_change(self, *args): viewer.zoom_level = old_level * float(to_fov_sky / orig_fov_sky) viewer.center_on(sky_cen) - else: - with delay_callback(viewer.state, *self.match_keys): + elif len(self.match_axes): + with delay_callback(viewer.state, *self.delay_callback_keys): for ax in self.match_axes: if None in orig_lims.values(): orig_range = np.inf @@ -134,6 +143,13 @@ def on_limits_change(self, *args): if not np.isnan(value) and (orig_value is None or abs(value-orig_lims.get(k, np.inf)) > tol): setattr(viewer.state, k, value) + else: + # match keys, but not match axes (e.g., zoom_center and zoom_radius) + with delay_callback(viewer.state, *self.delay_callback_keys): + for k in self.match_keys: + value = to_lims.get(k) + if not np.isnan(value): + setattr(viewer.state, k, value) def is_visible(self): return len(self.viewer.jdaviz_app._viewer_store) > 1