Skip to content

Conversation

zoccoler
Copy link
Collaborator

@zoccoler zoccoler commented Jul 24, 2025

This pull request introduces new functionality to focus the viewer on highlighted objects in the napari_clusters_plotter widget, along with utility functions for affine transformations and camera adjustments. The most important changes include adding a signal handler for object highlighting, implementing the logic for focusing on selected objects, and creating reusable affine transformation and camera utilities.

New functionality for object highlighting and focusing:

  • Added a connection for the highlighted_changed_signal to the _on_highlighted_changed method in _setup_callbacks. This enables the widget to respond when an object is highlighted. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR186-R189)
  • Implemented _on_highlighted_changed, which focuses the viewer on a highlighted object in the layer. It handles different layer types such as Points, Labels, Surface, Shapes, and Tracks. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)

Utility functions for affine transformations:

Camera utilities for viewer adjustments:

To do:

  • evaluate if current behavior (centering and zooming in) feels like a good ux
  • implement unhighlighting (when pressing Escape) condition (reset_view)?
  • write tests

zoccoler added 5 commits July 1, 2025 12:27
…version for points layer)

Refactors the object focusing logic to correctly apply combined affine transformations (rotation, scale, shear, translation) when centering the viewer on a selected object. Adds a helper function to build the affine matrix and ensures only the selected layer's objects are considered for focusing.
Added support for focusing on selected objects in Labels layers by computing the center of the selected label and applying affine transformations. Introduced helper functions for affine transformation and default zoom calculation to improve camera centering and zooming behavior.
Improved the logic for focusing on selected objects in napari layers by refactoring affine transformation application and camera setting into dedicated functions. Enhanced code clarity, modularity, and error handling for single object selection, and streamlined the process for both Points and Labels layers.
Extended the _focus_object function to handle napari Surface, Shapes, and Tracks layers. Each layer type now computes the center and applies the affine transformation before setting the viewer camera. Also fixed center calculation for Points and Labels layers.
@zoccoler
Copy link
Collaborator Author

zoccoler commented Jul 24, 2025

This PR seems to be somewhat related to #441 . Here we highlight a point in the plotter and get a focusing effect in the corresponding object in the napari layer. There, if I understood it correctly, it is the other way around ?

zoccoler added 3 commits July 25, 2025 11:35
Replaces boolean indexing with integer indexing for shape selection in the _focus_object function to ensure compatibility with list of arrays in napari.layers.Shapes.
Replaces the custom _build_affine_matrix function with napari's Affine class for constructing affine transformation matrices. This simplifies the code and leverages napari's built-in utilities for affine transformations.
@jo-mueller
Copy link
Collaborator

This PR seems to be somewhat related to #441 . Here we highlight a point in the plotter and get a focusing effect in the corresponding object in the napari layer. There, if I understood it correctly, it is the other way around ?

Correct! Full bi-directionality :)

As for this PR, I think the new way of calculating the Affine transform is correct, but there seem to be some conflicts with the main branch? Also, side-note: I think I would not change the zoom upon jumping to a new object. The zoom could be something that a user set specifically for their dataset. In that case I could imagine it to be a bit annoying if the zoom jumped to something else upon highlighting a datapoint. It's easily something that we can build in later (or just comment out at this time).

Orrr we both try it and see how it feels ona few datasets. ^^"

zoccoler and others added 11 commits August 13, 2025 13:41
The _calculate_default_zoom function and its usage in _set_viewer_camera have been commented out. This prepares for potential future implementation of zooming in on highlighted objects, but currently leaves the functionality inactive.
Introduces two tests to verify that the viewer camera centers and dims step update correctly when a single or multiple points layers have highlighted selections in 3D mode. These tests ensure proper behavior when focusing on selected points, including handling of layer translations.
Simplified the logic for selecting the center of a track in the _focus_object function by using a ternary operator, as suggested by SIM108. Required by pre-commit.
@zoccoler
Copy link
Collaborator Author

zoccoler commented Aug 13, 2025

Alright, so I commented the zoom part for now, added tests for the points layer, solved conflicts, and fixed the pre-commit complaints.

An unhighlighting feature is not needed if we don't zoom in IMO, but we may bring it in at another PR.

Tests were passing locally, not sure why they are failing now. I can check here again next week as I will be out for a couple days. @jo-mueller feel free to jump in if you find the need to have this merged before that.

@zoccoler zoccoler marked this pull request as ready for review August 13, 2025 13:34
@zoccoler zoccoler requested a review from jo-mueller August 13, 2025 13:34
@jo-mueller
Copy link
Collaborator

@zoccoler I think I see what's happening - in some recent change, I changed the layer indexing to be done by uuid. But, as it turns out, querying a layer from the viewer like this viewer.layers[uuid] isn't possible, whereas viewer.layers[name] is - which means that we should definitely index by name. I'm reverting this elsewhere and send you a PR to this. Then it's good to go, where I am concerned 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants