Tiling canvas overlays

[brisvag]

References and relevant issues

Part of the work in #7832.
Depends on #7835.
Diff relative to #7835: brisvag/napari@refactor/overlays...brisvag:napari:feature/tiling-overlays2

Description

Canvas overlays can be positioned on the canvas using a limited set of positions (top_left, bottom_right and so on).
Currently, if two overlays end up in the same place, they simply overlap.

This PR introduces tiling for canvas overlays by looping through all canvas overlays and assigning an (horizontal) offset equal to the space taken up by all previous overlays.

import napari
v = napari.Viewer()
v.add_points()

v.scale_bar.visible = True

v.text_overlay.visible = True
v.text_overlay.text = 'Some interesting text'
v.text_overlay.position = 'bottom_right'

napari.run()

On main:

With this PR:

[jni]

Wow so cool @brisvag! 😍

[codecov]

Codecov Report

❌ Patch coverage is 94.80519% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.04%. Comparing base (2554b31) to head (ca03e8b).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/napari/_vispy/utils/text.py	91.39%	8 Missing ⚠️
src/napari/_vispy/canvas.py	92.50%	6 Missing ⚠️
src/napari/_vispy/overlays/text.py	89.47%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7836      +/-   ##
==========================================
+ Coverage   92.98%   93.04%   +0.06%     
==========================================
  Files         703      704       +1     
  Lines       63365    63507     +142     
==========================================
+ Hits        58920    59091     +171     
+ Misses       4445     4416      -29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TimMonko]

Looks awesome!

How challenging would it be to add a vertical tiling option? This would generally be my preference for some things (like in the PR description). It's tough because it has to be generic enough for every canvas overlay, which might most often work nicely with horizontal (like colorbars).

But, just thinking it would be the adjustment between using the x_offsets vs y_offsets.

[brisvag]

I agree that it would be cool; the problem with that is not the actual implementation (which is indeed pretty easy with y_offset) but the complexity of the logic of combining horizontal and vertical tiling.

Imagine a scenario where you have 4 overlays at the same corner.

Let's say we have vertical and horizontal tiling be a parameter for each overlay, and we have 2 with "horizontal" and 2 with "vertical". What do you expect to happen? I don't think there's a unique answer to that :P

But I would definitely like to explore this further, I'm just saying it's way more complex than simply sticking to a single direction, and should be done in a followup PR!

[brisvag]

Alternatively, a simpler partial solution would be to have a per-position tiling option somewhere in the settings or the viewer itself, allowing to set something like overlays.top_left.orientation = 'vertical'. This would get better tiling for cases like the above, without having to deal with mixed tiling.

[TimMonko]

I agree that it would be cool; the problem with that is not the actual implementation (which is indeed pretty easy with y_offset) but the complexity of the logic of combining horizontal and vertical tiling.

Imagine a scenario where you have 4 overlays at the same corner.

Let's say we have vertical and horizontal tiling be a parameter for each overlay, and we have 2 with "horizontal" and 2 with "vertical". What do you expect to happen? I don't think there's a unique answer to that :P

But I would definitely like to explore this further, I'm just saying it's way more complex than simply sticking to a single direction, and should be done in a followup PR!

I was actually thinking of a hybrid of your two ideas. Sorry I didn't swing back to this much sooner!

Instead of the tiling logic being with the overlay itself, its tied to the canvas. Thus it becomes canvas.overlays.orientation= rather than canvas.overlays.scale_bar.orientation

Perhaps this waits until multi-canvas gets sorted and we go with this basic implementation for now. 😄

[brisvag]

Ok, summary of all the tests:

On my machine (no desktop environment, just window manager):

• default: 1, 1, 1
• Scaling using QT_SCALE_FACTOR=2: 2, 1, 2
• In all cases, everything looks fine

On @jni's mac:

• default is 1, 1, 2
• Scaling using OSX "more space" (:facepalm:): 1, 1, 2 (some hidden nonsense)
• Scaling using QT_SCALE_FACTOR=2: 1, 1, 4 (wtf)

On @Czaki's mac (uses an app called "better display" for rescaling:

• default is 1, 1, 1
• Scaling using the "better display" slider: 2, 2, 2
• in all cases using "better display", it results in the ugly overlap, depending on the font size

On @Czaki's windows:

• default is 1, 1, 1
• Every type of scaling results in 2, 2, 2 and similar, consistent changes across the board
• EVERTYTHING LOOKS FINE (????)

On @TimMonko's windows machines:

• crazy nonsense

Conclusions:

• 😭
• We probably need vispy top somehow detect some hidden thing that it's not currently detecting

[brisvag]

Y'all, I think we got it!

To summarize:

• most software actually assumes that "base dpi" is 96 (not 72..., though vispy says that, but it ultimately doesn't matter). This means that at a dpi of 96, a font size of 10 looks "as intended".
• scaling has ultimately nothing to do with any of this, it just made the issue more inconsistent and harder to figure out
• vispy rescales text based on dpi, but not the rest. So at double dpi, while font_size = 10 means 20, 10 pixels still means 10. This resulted in the text offset/padding being mismatched with the text size.
• I added a new Text visual that abstracts this away, so we can just do Text.font_size and Text.get_width_height and not worry about it.
• Tested this with @TimMonko and me and everything looks good. Would be good to get @Czaki confirmation as well, as he also had issues.

[TimMonko]

gridded is currently not being accounted for correctly with tiling. See how text_overlay overlaps scale_bar. This only happens when scale_bar.gridded= True is true for one and False for the other. If both are gridded it calculates correctly.
import napari

v = napari.Viewer()

v.scale_bar.visible = True
v.scale_bar.box = True
v.scale_bar.gridded = True

v.text_overlay.visible = True
v.text_overlay.text = 'Points'
v.text_overlay.font_size = 20
v.text_overlay.position = 'bottom_right'

v.grid.enabled = True
v.grid.stride = 2

ll = v.open_sample('napari', 'lily')

napari.run()

[TimMonko]

oh, and if its a struggled to figure out --- I want to be clear that my comment about the gridded text overlap with non-gridded is not a blocker. It's not a regression, because on main the same thing happens. I tried briefly to globally get the position of all overlays but only spent 15 minutes on it and got confused lol.
We can make follow-up PRs because this PR is already well behaved as long as nothing is gridded (which is new, and niche, anyways)

[brisvag]

That's not a bug, it's the intended behaviour (at least, I intended it xD). Overlays are tiled on a by-viewbox basiss, and the "main viewbox" (where non-gridded stuff goes) lives layered on top of the gridded viewboxes, so there is no interaction between overlays across views.

If we wanted to get what you expect, it could only work with the corners (e.g: not with CanvasPosition.BOTTOM_CENTER), and for each corner we would have to:

• get the viewbox at that corner in the grid
• get the non-gridded overlays at that corner
• treat them as if they were in the same viewbox for tiling purposes

Now that I write it out maybe it's not that hard... But yeah, let's do later xD

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/napari-v0-6-5-is-out/116715/1