Transform mode of Labels layer pivots wrong when non-empty Shapes/Points present

[psobolewskiPhD]

🐛 Bug Report

If there is a non-empty Shapes or Points layer present and you use Transform mode on a Labels layer (activate using 7), rotation pivots wrong: the bounding box desyncs the layer data.

Kapture.2024-05-06.at.21.08.31.mp4

💡 Steps to Reproduce

1. Open napari
2. Make a Shapes or Points that is not empty
3. Make a labels layer and paint something
4. Activate Transform mode (7) and try to rotate the labels layer

💡 Expected Behavior

The transform box should match the layer data and remain functional.

🌎 Environment

napari: 0.5.0a2.dev649+g2d2cac676
Platform: macOS-13.3.1-arm64-arm-64bit
System: MacOS 13.3.1
Python: 3.12.2 | packaged by conda-forge | (main, Feb 16 2024, 20:54:21) [Clang 16.0.6 ]
Qt: 6.6.1
PyQt6:
NumPy: 1.26.4
SciPy: 1.12.0
Dask: 2024.3.1
VisPy: 0.14.3.dev3
magicgui: 0.8.2
superqt: 0.6.2
in-n-out: 0.2.0
app-model: 0.2.5
npe2: 0.7.4

OpenGL:

• GL version: 2.1 Metal - 83.1
• MAX_TEXTURE_SIZE: 16384
• GL_MAX_3D_TEXTURE_SIZE: 2048

Screens:

• screen 1: resolution 1680x1050, scale 2.0

Optional:

• numba: 0.59.0
• triangle not installed

Settings path:

• /Users/piotrsobolewski/Library/Application Support/napari/napari-dev_a1eb8b76ba95fa16ad06e26097b46b8455dfbf0b/settings.yaml

💡 Additional Context

No response

[Czaki]

When you remove Shapes layer, you cannot longer even rotate this layer.

[dalthviz]

Gave this a check from the GUI and seems like what is failing is that the bounding box/transform box is being shown with some translation? I was able to keep transforming the layer but the place where mouse events trigger actual transformations doesn't correspond with the place the box handles are being displayed, instead they still correspond with the actual layer limits 🤔

Checking things around seems like when a shapes layer is created first that constrains the size of the labels layer? Maybe there are some missing things to do/setup so this size constraint is taken into account for the labels layer bounding box/transform box overlay?

Creating first a shapes layer	Creating first a labels layer

Also, if the labels layer is created first the bounding box/transform box is displayed correctly when doing transforms 🤔

[Czaki]

Ok. This is not a problem with the order of layer creation. It is a bug connected to translation. If you first create a shape or point layer, then labels layer will be created with negative translation.

Minimum script is:

import napari
import numpy as np

from napari.utils.transforms import Affine

viewer = napari.Viewer()
labels = viewer.add_labels(np.ones((10, 10), dtype=np.uint8), translate=(10, 10), affine=Affine(rotate=45))
labels.mode = "transform"

napari.run()

@brisvag it looks like we hit the same problem again...
@psobolewskiPhD could you update the description of issue to correctly point the problem?

[Czaki]

The problem is here:

napari/napari/_vispy/layers/base.py

Line 254 in 2102518

new_translate = (

In general, for simple layer it should evaluate to [0.5, 0.5], but does not. We need to describe it with equations and find a way to test it.

[dalthviz]

Maybe naive but seems like changing

napari/napari/_vispy/layers/base.py

Lines 246 to 249 in 2102518

	translate_child = (
	self.layer.translate[dims_displayed]
	+ self.layer.affine.translate[dims_displayed]
	)[::-1] - offset[::-1]

to be something like

translate_child = (
    simplified_transform.translate[dims_displayed]
)[::-1] - offset[::-1]

Makes things work (at least when checking things from the GUI with the minimum reproducer script/code snippet):

Is there a reason to not use the simplified transform for the translate value in the same way that it's used to get the rotate and scale values?

[Czaki]

Is there a reason to not use the simplified transform for the translate value in the same way that it's used to get the rotate and scale values?

I do not check it (Yes, I'm the author of this code too). Maybde it is enough, but I think that we should also find a way to test it and prepare excessive test coverage to validate all cases.

[brisvag]

This makes sense to me; agree that we need proper coverage here, but we don't even have proper agreements on the coordinate systems (#3783)...