Skip to content

Commit

Permalink
[bugfix] Adjust scale bar position based on font_size when at top (na…
Browse files Browse the repository at this point in the history
…pari#7018)

# References and relevant issues
Closes: napari#7009

# Description
This PR aims to fix the visual issues with the scale bar text when the
scale bar is positioned at the top, instead of default down.
If the size of the text is changed, the offset from the top of the
canvas is increased proportionally.

In the process, this PR does two main things:
- accounts for the canvas DPI when scaling the text size. The defaults
(font_size, box height, etc.) were all hard coded around 96 dpi. This PR
corrects for that by getting the actual canvas DPI from vispy. A side
effect of this is that users with >96 DPI will see the text get smaller
-- we may wish to bump up the default from 10 to 11 or 12.
This has another side effect: partially addressing
napari#7516 but just for default
font_size, not when font_size is increased.

- computes the logical pixel height of the text in order to account for
it in the y_offset from the top of the screen.

Default settings with this PR (just setting top_right):
<img width="852" alt="image"
src="https://github.com/user-attachments/assets/a1fd9aa7-1c53-4b97-8e9e-06f112e8d3ac"
/>
Current napari main:
<img width="836" alt="image"
src="https://github.com/napari/napari/assets/76622105/af6e66a6-93b0-4bba-9283-544a8476241e">

Change to font_size 30 with this PR:
<img width="869" alt="image"
src="https://github.com/user-attachments/assets/0a59a9e3-b333-4aa7-aacc-aca6f74d5deb"
/>
versus current napari main:
<img width="841" alt="image"
src="https://github.com/napari/napari/assets/76622105/b590855d-baf6-4f78-abb3-30f18b97d226">

Edit: I tested this with my personal mac DPI 149 and with an external
display 108. The rendering of the scale_bar was identical to my eyes,
side by side.

---------

Co-authored-by: Juan Nunez-Iglesias <[email protected]>
  • Loading branch information
psobolewskiPhD and jni authored Jan 17, 2025
1 parent dc6433f commit 69ac355
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
31 changes: 31 additions & 0 deletions napari/_vispy/_tests/test_vispy_scale_bar_visual.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
from unittest.mock import MagicMock

import pytest

from napari._vispy.overlays.scale_bar import VispyScaleBarOverlay
from napari.components.overlays import ScaleBarOverlay

Expand All @@ -9,3 +13,30 @@ def test_scale_bar_instantiation(make_napari_viewer):
assert vispy_scale_bar.overlay.length is None
model.length = 50
assert vispy_scale_bar.overlay.length == 50


def test_scale_bar_positioning(make_napari_viewer):
viewer = make_napari_viewer()
# set devicePixelRatio to 2 so testing works on CI and local
viewer.window._qt_window.devicePixelRatio = MagicMock(return_value=2)
model = ScaleBarOverlay()
scale_bar = VispyScaleBarOverlay(overlay=model, viewer=viewer)

assert model.position == 'bottom_right'
assert model.font_size == 10
assert scale_bar.y_offset == 20

model.position = 'top_right'
assert scale_bar.y_offset == pytest.approx(20.333, abs=0.1)

# increasing size while at top should increase y_offset to 7 + font_size*1.33
model.font_size = 30
assert scale_bar.y_offset == pytest.approx(47, abs=0.1)

# moving scale bar back to bottom should reset y_offset to 20
model.position = 'bottom_right'
assert scale_bar.y_offset == 20

# changing font_size at bottom should have no effect
model.font_size = 50
assert scale_bar.y_offset == 20
34 changes: 32 additions & 2 deletions napari/_vispy/overlays/scale_bar.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def _on_zoom_change(self, *, force: bool = False):
self.node.transform.scale = [scale, 1, 1, 1]
self.node.text.text = f'{new_dim:g~#P}'
self.x_size = scale # needed to offset properly
self._on_position_change()
super()._on_position_change()

def _on_data_change(self):
"""Change color and data of scale bar and box."""
Expand Down Expand Up @@ -173,7 +173,37 @@ def _on_box_change(self):

def _on_text_change(self):
"""Update text information"""
self.node.text.font_size = self.overlay.font_size
# update the dpi scale factor to account for screen dpi
# because vispy scales pixel height of text by screen dpi
if self.node.text.transforms.dpi:
# use 96 as the napari reference dpi for historical reasons
dpi_scale_factor = 96 / self.node.text.transforms.dpi
else:
dpi_scale_factor = 1

self.node.text.font_size = self.overlay.font_size * dpi_scale_factor
# ensure we recalculate the y_offset from the text size when at top of canvas
if 'top' in self.overlay.position:
self._on_position_change()

def _on_position_change(self, event=None):
# prevent the text from being cut off by shifting down
if 'top' in self.overlay.position:
# convert font_size to logical pixels as vispy does
# in vispy/visuals/text/text.py
# 72 is the vispy reference dpi
# 96 dpi is used as the napari reference dpi
font_logical_pix = self.overlay.font_size * 96 / 72
# 7 is base value for the default 10 font size
self.y_offset = 7 + font_logical_pix
else:
self.y_offset = 20
super()._on_position_change()

def _on_visible_change(self):
# ensure that dpi is updated when the scale bar is visible
self._on_text_change()
return super()._on_visible_change()

def reset(self):
super().reset()
Expand Down

0 comments on commit 69ac355

Please sign in to comment.