Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select Mode of Points Layer broken in 4 dimensions #6680

Open
jules-vanaret opened this issue Feb 21, 2024 · 3 comments 路 May be fixed by #6819
Open

Select Mode of Points Layer broken in 4 dimensions #6680

jules-vanaret opened this issue Feb 21, 2024 · 3 comments 路 May be fixed by #6819
Labels
bug Something isn't working

Comments

@jules-vanaret
Copy link
Contributor

馃悰 Bug Report

When displaying points 4D with the 3D view, selecting the "Select" mode of the Points layer then dragging the mouse across the canvas (not moving point, just dragging the cursor as if to create the usual selection bounding box) does nothing and raises an Assertion error. If a point is clicked instead, it can be moved normally without any error.
Here's the error I get

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
File ~/anaconda3/envs/napari-dev/lib/python3.9/site-packages/superqt/utils/_throttler.py:249, in ThrottledCallable._set_future_result(self=<superqt.utils._throttler.ThrottledCallable object>)
    248 def _set_future_result(self):
--> 249     result = self._func(*self._args[: self._max_args], **self._kwargs)
        self._func = <bound method VispyCanvas._on_mouse_move of <napari._vispy.canvas.VispyCanvas object at 0x7ff6c544fdf0>>
        self = <superqt.utils._throttler.ThrottledCallable object at 0x7ff6ac3160d0>
        self._args[: self._max_args] = (<MouseEvent blocked=False button=1 buttons=[1, 1] delta=[0. 0.] handled=False is_dragging=True last_event=MouseEvent modifiers=() native=<PyQt5.QtGui.QMouseEvent object at 0x7ff6c545c310> pos=[527 398] press_event=MouseEvent source=<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070> sources=[<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070>] time=1708521467.3701262 type=mouse_move>,)
        self._kwargs = {}
        self._args = (<MouseEvent blocked=False button=1 buttons=[1, 1] delta=[0. 0.] handled=False is_dragging=True last_event=MouseEvent modifiers=() native=<PyQt5.QtGui.QMouseEvent object at 0x7ff6c545c310> pos=[527 398] press_event=MouseEvent source=<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070> sources=[<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070>] time=1708521467.3701262 type=mouse_move>,)
        self._max_args = 1
    250     self._future.set_result(result)

File /napari/napari/_vispy/canvas.py:448, in VispyCanvas._on_mouse_move(self=<napari._vispy.canvas.VispyCanvas object>, event=<MouseEvent blocked=False button=1 buttons=[1, 1...459070>] time=1708521467.3701262 type=mouse_move>)
    436 def _on_mouse_move(self, event: MouseEvent) -> None:
    437     """Called whenever mouse moves over canvas.
    438 
    439     Parameters
   (...)
    446     None
    447     """
--> 448     self._process_mouse_event(mouse_move_callbacks, event)
        event = <MouseEvent blocked=False button=1 buttons=[1, 1] delta=[0. 0.] handled=False is_dragging=True last_event=MouseEvent modifiers=() native=<PyQt5.QtGui.QMouseEvent object at 0x7ff6c545c310> pos=[527 398] press_event=MouseEvent source=<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070> sources=[<NapariSceneCanvas (PyQt5) at 0x7ff6c5459070>] time=1708521467.3701262 type=mouse_move>
        self = <napari._vispy.canvas.VispyCanvas object at 0x7ff6c544fdf0>

File /napari/napari/_vispy/canvas.py:409, in VispyCanvas._process_mouse_event(self=<napari._vispy.canvas.VispyCanvas object>, mouse_callbacks=<function mouse_move_callbacks>, event=<ReadOnlyWrapper at 0x7ff69675b680 for MouseEvent>)
    407 layer = self.viewer.layers.selection.active
    408 if layer is not None:
--> 409     mouse_callbacks(layer, event)
        event = <ReadOnlyWrapper at 0x7ff69675b680 for MouseEvent at 0x7ff6954096d0>
        layer = <Points layer 'points' at 0x7ff6eb790c40>
        mouse_callbacks = <function mouse_move_callbacks at 0x7ff6debac940>

File /napari/napari/utils/interactions.py:178, in mouse_move_callbacks(obj=<Points layer 'points'>, event=<ReadOnlyWrapper at 0x7ff69675b680 for MouseEvent>)
    175 obj._persisted_mouse_event[gen].__wrapped__ = event
    176 try:
    177     # try to advance the generator
--> 178     next(gen)
        gen = <generator object select at 0x7ff6978be890>
    179 except StopIteration:
    180     # If done deleted the generator and stored event
    181     del obj._mouse_drag_gen[func]

File /napari/napari/layers/points/_points_mouse_bindings.py:92, in select(layer=<Points layer 'points'>, event=<ReadOnlyWrapper at 0x7ff696756100 for ReadOnlyWrapper>)
     89     layer._drag_box = np.array([layer._drag_start, coord])
     91     # update the drag up and normal vectors on the layer
---> 92     _update_drag_vectors_from_event(layer=layer, event=event)
        layer = <Points layer 'points' at 0x7ff6eb790c40>
        event = <ReadOnlyWrapper at 0x7ff696756100 for ReadOnlyWrapper at 0x7ff69675b680>
     94     layer._set_highlight()
     95 yield

File /napari/napari/layers/points/_points_mouse_bindings.py:195, in _update_drag_vectors_from_event(layer=<Points layer 'points'>, event=<ReadOnlyWrapper at 0x7ff696756100 for ReadOnlyWrapper>)
    191 if n_display == 3:
    192     # if in 3D, set the drag normal and up directions
    193     # get the indices of the displayed dimensions
    194     ndim_world = len(event.position)
--> 195     layer_dims_displayed = layer._world_to_layer_dims(
        layer = <Points layer 'points' at 0x7ff6eb790c40>
        ndim_world = 4
        event = <ReadOnlyWrapper at 0x7ff696756100 for ReadOnlyWrapper at 0x7ff69675b680>
    196         world_dims=event.dims_displayed, ndim_world=ndim_world
    197     )
    199     # get the view direction in displayed data coordinates
    200     layer._drag_normal = layer._world_to_displayed_data_ray(
    201         event.view_direction, layer_dims_displayed
    202     )

File /napari/napari/layers/base/base.py:1640, in Layer._world_to_layer_dims(self=<Points layer 'points'>, world_dims=[1, 2, 3], ndim_world=4)
   1580 def _world_to_layer_dims(
   1581     self, *, world_dims: npt.NDArray, ndim_world: int
   1582 ) -> np.ndarray:
   1583     """Map world dimensions to layer dimensions while maintaining order.
   1584 
   1585     This is used to map dimensions from the full world space defined by ``Dims``
   (...)
   1638         The corresponding layer dimensions with the same ordering as the given world dimensions.
   1639     """
-> 1640     return self._world_to_layer_dims_impl(
        self = <Points layer 'points' at 0x7ff6eb790c40>
        world_dims = [1, 2, 3]
        ndim_world = 4
   1641         world_dims, ndim_world, self.ndim
   1642     )

File /napari/napari/layers/base/base.py:1652, in Layer._world_to_layer_dims_impl(world_dims=<class 'numpy.ndarray'> (3,) int64, ndim_world=4, ndim=4)
   1648 """
   1649 Static for ease of testing
   1650 """
   1651 world_dims = np.asarray(world_dims)
-> 1652 assert world_dims.min() == 0
        world_dims = <class 'numpy.ndarray'> (3,) int64
   1653 assert world_dims.max() == len(world_dims) - 1
   1654 assert world_dims.ndim == 1

AssertionError:

When the cursor is not dragged but clicked, an Axis error raises (axisa: axis -1 is out of bounds for array of dimension 0). I won't show the error stack as it is quite long.

馃挕 Steps to Reproduce

  1. Display points in 4D. You can use the following snippet:
import napari
import numpy as np

np.random.seed(0)

data = np.random.random((2, 100,100,100))
points = np.random.random((3, 4)) * 100
points[:,0] = 0

viewer = napari.Viewer(ndisplay=3)
viewer.add_image(data)
viewer.add_points(points, size=10)
napari.run()
  1. Go in 3D view mode
  2. Make sure the "Select" mode of the Points Layer is selected. Click and drag your cursor across the canvas.

馃挕 Expected Behavior

Normally, the action described should draw a thin blue rectangular bounding box that allows selecting the points.

馃寧 Environment

napari: 0.1.dev3244+g0747da5
Platform: Linux-5.15.0-91-generic-x86_64-with-glibc2.31
System: Ubuntu 20.04.6 LTS
Python: 3.9.0 (default, Nov 15 2020, 14:28:56) [GCC 7.3.0]
Qt: 5.15.2
PyQt5: 5.15.9
NumPy: 1.24.2
SciPy: 1.9.1
Dask: 2023.4.0
VisPy: 0.14.1
magicgui: 0.7.2
superqt: 0.6.1
in-n-out: 0.1.7
app-model: 0.2.4
npe2: 0.7.4

OpenGL:

  • GL version: 4.6 (Compatibility Profile) Mesa 21.2.6
  • MAX_TEXTURE_SIZE: 16384
  • GL_MAX_3D_TEXTURE_SIZE: 2048

Settings path:

  • /home/jvanaret/.config/napari/napari-dev_ebc9c46b35c7ad556420cc56947aae90c9e81949/settings.yaml
    Plugins:
  • napari: 0.1.dev3244+g0747da5 (81 contributions)
  • napari-console: 0.0.9 (0 contributions)
  • napari-svg: 0.1.10 (2 contributions)
  • napari-threedee: 0.0.12 (28 contributions)

馃挕 Additional Context

No response

@jules-vanaret jules-vanaret added the bug Something isn't working label Feb 21, 2024
@dalthviz
Copy link
Member

dalthviz commented Feb 21, 2024

Hi there, gave this a check and seems like changing the logic at _world_to_layer_dims_impl

@staticmethod
def _world_to_layer_dims_impl(
world_dims: npt.NDArray, ndim_world: int, ndim: int
) -> npt.NDArray:
"""
Static for ease of testing
"""
world_dims = np.asarray(world_dims)
assert world_dims.min() == 0
assert world_dims.max() == len(world_dims) - 1
assert world_dims.ndim == 1
offset = ndim_world - ndim
order = world_dims - offset
order = order[order >= 0]
return order - order.min()
for something like:

    @staticmethod
    def _world_to_layer_dims_impl(
        world_dims: npt.NDArray, ndim_world: int, ndim: int
    ) -> npt.NDArray:
        """
        Static for ease of testing
        """
        offset = ndim_world - ndim
        order = np.array(world_dims)
        if offset == 0:
            return order
        if offset < 0:
            return np.concatenate((np.arange(-offset), order - offset))

        return order[order >= offset] - offset

helps.

I think the relevant PR where the logic changed is #6210 where an attempt to simplify the logic for the _world_to_layer_dims logic was done (while checking a fix for #6205). Maybe that logic simplification didn't took into account the points layer selection case handling on that method? 馃

@psobolewskiPhD
Copy link
Member

Sorry this fell through the cracks.
Thanks for looking into this @dalthviz
@brisvag do you have any thoughts on this one?

@brisvag
Copy link
Contributor

brisvag commented Apr 8, 2024

Not right now and it seems @dalthviz has a starting point. It would be good to see a PR with an example to see if this all makes sense!

@dalthviz dalthviz linked a pull request Apr 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants