Skip to content

Commit

Permalink
Fix shapes interactivity with scale != 1 (selection, rotate/resize) (#…
Browse files Browse the repository at this point in the history
…5802)

# Description

Scale layer interactivity and visualisation are currently not accounting
for the layer scale.

Try the following:

```py
import numpy as np
import napari
v = napari.Viewer()
il = v.add_image(np.random.rand(100, 100))
sl = v.add_shapes([[0, 0], [1, 1]], shape_type='path', scale=[100, 100], edge_width=0.1)
```

You'll see a few issues:
- highlight line is not in screen space as advertised. (This is actually
a problem with points as well)
- the `rotate` handle is way too far because its position is also
calculated not accounting for scale
- interacting with the shape in any mode is basically impossible, cause
coordinates do not account for the scaling of the layer.

This PR basically adds a `/ layer.scale[-1]` in several places to get
back into correct "sceen space". Note that I used the last dimension as
we cannot do anisotropic sizes, just like we recently chose to do with
points in #5582.

This PR also fixes #4538 by changing the logic of how the "minimum drag"
is calculated.

PS: ideally we want to transition to using the `SelectionOverlay` in the
future, but this is a much bigger effort.
 
Also fixes #5752.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
  • Loading branch information
brisvag committed Jul 23, 2023
1 parent 9997588 commit 03b8d8e
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 43 deletions.
5 changes: 3 additions & 2 deletions napari/_vispy/layers/shapes.py
Expand Up @@ -81,10 +81,11 @@ def _on_highlight_change(self):
face_color,
edge_color,
pos,
width,
_,
) = self.layer._compute_vertices_and_box()

width = settings.appearance.highlight_thickness
# use last dimension of scale like (thickness cannot be anisotropic)
width = settings.appearance.highlight_thickness / self.layer.scale[-1]

if vertices is None or len(vertices) == 0:
vertices = np.zeros((1, self.layer._slice_input.ndisplay))
Expand Down
14 changes: 7 additions & 7 deletions napari/layers/shapes/_shapes_mouse_bindings.py
Expand Up @@ -163,7 +163,7 @@ def add_line(layer: Shapes, event: MouseEvent) -> None:
A proxy read only wrapper around a vispy mouse event.
"""
# full size is the initial offset of the second point compared to the first point of the line.
size = layer._vertex_size * layer.scale_factor / 4
size = layer._normalized_vertex_radius / 2
full_size = np.zeros(layer.ndim, dtype=float)
for i in layer._slice_input.displayed:
full_size[i] = size
Expand Down Expand Up @@ -192,7 +192,7 @@ def add_ellipse(layer: Shapes, event: MouseEvent):
event: MouseEvent
A proxy read only wrapper around a vispy mouse event.
"""
size = layer._vertex_size * layer.scale_factor / 4
size = layer._normalized_vertex_radius / 2
size_h = np.zeros(layer.ndim, dtype=float)
size_h[layer._slice_input.displayed[0]] = size
size_v = np.zeros(layer.ndim, dtype=float)
Expand All @@ -218,7 +218,7 @@ def add_rectangle(layer: Shapes, event: MouseEvent) -> None:
event: MouseEvent
A proxy read only wrapper around a vispy mouse event.
"""
size = layer._vertex_size * layer.scale_factor / 4
size = layer._normalized_vertex_radius / 2
size_h = np.zeros(layer.ndim, dtype=float)
size_h[layer._slice_input.displayed[0]] = size
size_v = np.zeros(layer.ndim, dtype=float)
Expand Down Expand Up @@ -736,10 +736,10 @@ def _move_active_element_under_cursor(

# prevent box from shrinking below a threshold size
size = (np.linalg.norm(box[Box.TOP_LEFT] - box_center),)
threshold = (
layer._vertex_size * layer.scale_factor / layer.scale[-1] / 2
)
if np.linalg.norm(size * drag_scale) < threshold:
if (
np.linalg.norm(size * drag_scale)
< layer._normalized_vertex_radius
):
drag_scale[:] = 1
# on vertical/horizontal drags we get scale of 0
# when we actually simply don't want to scale
Expand Down
36 changes: 14 additions & 22 deletions napari/layers/shapes/_tests/test_shapes_mouse_bindings.py
Expand Up @@ -52,6 +52,8 @@ def create_known_shapes_layer():
n_shapes = len(data)

layer = Shapes(data)
# very zoomed in, guaranteed no overlap between vertices
layer.scale_factor = 0.001
assert layer.ndim == 2
assert len(layer.data) == n_shapes
assert len(layer.selected_data) == 0
Expand Down Expand Up @@ -408,26 +410,14 @@ def test_vertex_remove(create_known_shapes_layer, Event):
)
mouse_press_callbacks(layer, event)

# Simulate drag end
event = ReadOnlyWrapper(
Event(
type='mouse_move',
is_dragging=True,
modifiers=[],
position=position,
pos=position,
)
)
mouse_move_callbacks(layer, event)
assert layer.events.data.call_args[1] == {
"value": layer.data,
"action": ActionType.CHANGE.value,
"data_indices": tuple(
select,
),
"vertex_indices": ((3,),),
"vertex_indices": ((0,),),
}
# Check new shape added at coordinates
assert len(layer.data) == n_shapes
assert len(layer.data[0]) == n_coord - 1

Expand Down Expand Up @@ -644,29 +634,31 @@ def test_drag_vertex(create_known_shapes_layer, Event):
layer.events.data = Mock()
layer.mode = 'direct'
layer.selected_data = {0}
position = tuple(layer.data[0][0])
old_position = tuple(layer.data[0][0])

# Simulate click
event = ReadOnlyWrapper(
Event(
type='mouse_press',
is_dragging=False,
modifiers=[],
position=position,
pos=position,
position=old_position,
pos=old_position,
)
)
mouse_press_callbacks(layer, event)

position = [0, 0]
new_position = [0, 0]
assert np.all(new_position != old_position)

# Simulate move, click, and release
event = ReadOnlyWrapper(
Event(
type='mouse_move',
is_dragging=True,
modifiers=[],
position=position,
pos=position,
position=new_position,
pos=new_position,
)
)
mouse_move_callbacks(layer, event)
Expand All @@ -677,8 +669,8 @@ def test_drag_vertex(create_known_shapes_layer, Event):
type='mouse_release',
is_dragging=True,
modifiers=[],
position=position,
pos=position,
position=new_position,
pos=new_position,
)
)
mouse_release_callbacks(layer, event)
Expand All @@ -693,7 +685,7 @@ def test_drag_vertex(create_known_shapes_layer, Event):
"data_indices": (0,),
"vertex_indices": vertex_indices,
}
np.testing.assert_allclose(layer.data[0][-1], [0, 0])
np.testing.assert_allclose(layer.data[0][0], [0, 0])


@pytest.mark.parametrize(
Expand Down
54 changes: 42 additions & 12 deletions napari/layers/shapes/shapes.py
Expand Up @@ -2269,6 +2269,20 @@ def refresh_text(self):
"""
self.text.refresh(self.features)

@property
def _normalized_scale_factor(self):
"""Scale factor accounting for layer scale.
This is often needed when calculating screen-space sizes and distances
of vertices for interactivity (rescaling, adding vertices, etc).
"""
return self.scale_factor / self.scale[-1]

@property
def _normalized_vertex_radius(self):
"""Vertex radius normalized to screen space."""
return self._vertex_size * self._normalized_scale_factor / 2

def _set_view_slice(self):
"""Set the view given the slicing indices."""
ndisplay = self._slice_input.ndisplay
Expand Down Expand Up @@ -2330,7 +2344,10 @@ def interaction_box(self, index):
box[Box.BOTTOM_LEFT] - box[Box.TOP_LEFT]
)
if length_box > 0:
r = self._rotation_handle_length * self.scale_factor
r = (
self._rotation_handle_length
* self._normalized_scale_factor
)
rot = (
rot
- r
Expand Down Expand Up @@ -2368,7 +2385,7 @@ def _outline_shapes(self):

centers, offsets, triangles = self._data_view.outline(index)
vertices = centers + (
self.scale_factor * self._highlight_width * offsets
self._normalized_scale_factor * self._highlight_width * offsets
)
vertices = vertices[:, ::-1]
else:
Expand Down Expand Up @@ -2635,7 +2652,7 @@ def _scale_box(self, scale, center=(0, 0)):
box = self._selected_box - center
box = np.array(box * scale)
if not np.all(box[Box.TOP_CENTER] == box[Box.HANDLE]):
r = self._rotation_handle_length * self.scale_factor
r = self._rotation_handle_length * self._normalized_scale_factor
handle_vec = box[Box.HANDLE] - box[Box.TOP_CENTER]
cur_len = np.linalg.norm(handle_vec)
box[Box.HANDLE] = box[Box.TOP_CENTER] + r * handle_vec / cur_len
Expand All @@ -2654,12 +2671,20 @@ def _transform_box(self, transform, center=(0, 0)):
box = self._selected_box - center
box = box @ transform.T
if not np.all(box[Box.TOP_CENTER] == box[Box.HANDLE]):
r = self._rotation_handle_length * self.scale_factor
r = self._rotation_handle_length * self._normalized_scale_factor
handle_vec = box[Box.HANDLE] - box[Box.TOP_CENTER]
cur_len = np.linalg.norm(handle_vec)
box[Box.HANDLE] = box[Box.TOP_CENTER] + r * handle_vec / cur_len
self._selected_box = box + center

def _update_draw(
self, scale_factor, corner_pixels_displayed, shape_threshold
):
super()._update_draw(
scale_factor, corner_pixels_displayed, shape_threshold
)
self._set_highlight(force=True)

def _get_value(self, position):
"""Value of the data at a position in data coordinates.
Expand Down Expand Up @@ -2688,17 +2713,23 @@ def _get_value(self, position):
# Check selected shapes
value = None
selected_index = list(self.selected_data)

if len(selected_index) > 0:
self.scale[self._slice_input.displayed]
# Get the vertex sizes. They need to be rescaled by a few parameters:
# - scale_factor, because vertex sizes are zoom-invariant
# - scale, because vertex sizes are not affected by scale (unlike in Points)
# - 2, because the radius is what we need

if self._mode == Mode.SELECT:
# Check if inside vertex of interaction box or rotation handle
box = self._selected_box[Box.WITH_HANDLE]
distances = abs(box - coord)

# Get the vertex sizes
sizes = self._vertex_size * self.scale_factor / 2

# Check if any matching vertices
matches = np.all(distances <= sizes, axis=1).nonzero()
matches = np.all(
distances <= self._normalized_vertex_radius, axis=1
).nonzero()
if len(matches[0]) > 0:
value = (selected_index[0], matches[0][-1])
elif self._mode in (
Expand All @@ -2709,11 +2740,10 @@ def _get_value(self, position):
vertices = self._data_view.displayed_vertices[inds]
distances = abs(vertices - coord)

# Get the vertex sizes
sizes = self._vertex_size * self.scale_factor / 2

# Check if any matching vertices
matches = np.all(distances <= sizes, axis=1).nonzero()[0]
matches = np.all(
distances <= self._normalized_vertex_radius, axis=1
).nonzero()[0]
if len(matches) > 0:
index = inds.nonzero()[0][matches[-1]]
shape = self._data_view.displayed_index[index]
Expand Down

0 comments on commit 03b8d8e

Please sign in to comment.