napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python
https://napari.org
BSD 3-Clause "New" or "Revised" License
2.07k stars 410 forks source link

Fix selection size when canvas limits are set for Points #6853

Closed brisvag closed 1 week ago

brisvag commented 1 month ago

References and relevant issues

Closes https://github.com/napari/napari/issues/6682

Description

The selection markers were not updated accordingly.

This is still not perfect (zooming in and out beyond the limits will change the thickness of the highlight), but it's a limitation of how hilight is implemented (by just using extra markers). Ideally we want to have a single marker take care of hilight as well, but this requires rewriting the shaders and it's not trivial.

psobolewskiPhD commented 4 weeks ago

Hmm, I don't see the donut effect anymore, but I also don't see the highlight at all. I changed thickness to 3 as well.

Do the selection markers need to be canvas_size + highlight thickness?

brisvag commented 4 weeks ago

Good point... what about now?

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 92.39%. Comparing base (caf6caf) to head (e395597). Report is 15 commits behind head on main.

Files Patch % Lines
napari/_vispy/visuals/points.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6853 +/- ## ========================================== - Coverage 92.44% 92.39% -0.05% ========================================== Files 614 614 Lines 54843 54859 +16 ========================================== - Hits 50697 50686 -11 - Misses 4146 4173 +27 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

psobolewskiPhD commented 4 weeks ago

The behavior seems good now. I did notice that the highlight scales with zoom differently than point size, but the same behavior happens on main.