Grid mode using vispy ViewBox and linked cameras #7870
Conversation
Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com>
for more information, see https://pre-commit.ci
443e246 to
da63dc6
Compare
|
Todo from pairing with @melonora:
|
|
@brisvag indeed, you also fixed a severe issue with multiscale layers, which are now properly rendered. 🎉 |
Unfortunately does not solve #8033. What test were you trying to solve? I don't see any in the recent commits that are screenshot related, just multiscale. The screenshot test in |
|
Ok, I've figured out the multiscale test fails. The I'm approving :) I tried really hard to break this, and couldn't. |
Uhm, I thought I solved it in 1372ed9, which is why I thought it might also solve your problem, by updating the canvas before screenshotting. But I might have misunderstood the test, I thought the divide by zero was failing due to the image being denegerate. Maybe we should revert that commit then? Yeah those multiscale are the same as mentioned in these comments, I should mark xfail! EDIT: actually, these might be more fixable than the above. Unfortunately, I'm not gonna be able to come back to this for a couple of days, sorry for the bad timing 😅 I'll let you decide what to do! |
|
The plot thickens. Apparently the screenshot Edit: and it thickens more. it still fails with
Maybe it's flaky? I'll try rerunning. Locally it passes for me. Edit2: Yes it's flaky, so it's not this PR, per-se. It is in the sense we change the draw mechanism, but not a blocker to the PR. I'll still put in a resolution or x-fail here. |
|
Ok, so here's my final decision. This test is actually still an issue on main (localy), and is the same issue as #8033. Without fully processing (like in the tests), we get just the canvas (hence the divide by zero, because the data doesn't ever show up). Hopefully this just means we are slightly slower in displaying the image, but it must be ever so slight, since this is the only test issue. And I mean the change in event processing must be so slight since itss flaky, so I'm not worried about performance. But I do think trying to fix #8033 is high because now it also probably effects others OSes. For that reason, I'm going to add the processing into this test only. I'm guessing this is probably the root of so much of our screenshot test flakiness.... Once the whole loop is processed is loaded, the figures/screenshots look good (as expected) |
| center = tuple(np.round(np.divide(screenshot.shape[:2], 2)).astype(int)) | ||
| np.testing.assert_almost_equal(screenshot[center], [0, 255, 255, 255]) |
There was a problem hiding this comment.
maybe here and above, comment out the code and add a note, as you did with other tests above, so that we know to revive the test as soon as we can?
There was a problem hiding this comment.
Will do, good catch! Didn't realize Lorenzo commented these out too ... another lead on how this issue is happening. 😬
There was a problem hiding this comment.
or, why does it need commented out? the test passes?
There was a problem hiding this comment.
ok actually I'm just confused on what I'm looking at. Is this the most recent commit? It's not what I see locally and I think it's the most recent commit. But this does look like a diff that has this code removed.
|
I am merging this PR because it has had many eyes from the team:
We will open follow-up PRs to address a few things, and others revealed during the pre-release including
|
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/test-out-the-upcoming-napari-0-6-2-release/114029/1 |
# References and and Description Follow up to #7870. After that PR, on main, stride results in layers stacking up in the wrong order, where lower indexed layers are on top of the stack. See #8053 and #8044 for original discussion ```python import napari from skimage import data viewer, layer = napari.imshow( data.lily(), name='lily', channel_axis=2, colormap=['red', 'green', 'blue', 'gray'], blending='translucent', opacity=1, ) viewer.grid.enabled = True viewer.grid.stride = 2 napari.run() ``` main:  this PR:  --------- Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
References and relevant issues
Feature: Highlight currently selected layer in grid mode #2954EDIT: postponed to a followup PRDescription
This PR replaces the current transform-based grid mode with a viewbox-based one using linked cameras.
Drag events will remain relative to the original viewbox, making annotation at the edges a breeze. Interacting with one viewbox, such as during annotation, will update the corresponding active layer, potentially in a different grid view.
Additionally, viewer canvas overlays (such as
scale_bar) now have agriddedproperty which, when set toTrue, will make it so the overlay appears in every grid box instead that on the general canvas.Grid mode spacing now works proportionally to the layer extensions (i.e. [0,1), as before this PR) or as a pixel value (1,1500]. If spacing amount exceeds a limit that will result in degenerate viewboxes, the spacing will automatically adjust and present a warning to the user. Stride continues to work as before. For discussion on changing the grid layout see #8044.