Fix bounding_box extent for case of 3D multiscale layer #7545
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7545 +/- ##
==========================================
- Coverage 92.88% 92.84% -0.04%
==========================================
Files 632 633 +1
Lines 59258 59278 +20
==========================================
- Hits 55042 55039 -3
- Misses 4216 4239 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # and we need the augmented bounding box | ||
| bounds = self.layer._display_bounding_box_at_level( | ||
| self.layer._slice_input.displayed, len(self.layer.data) - 1 < 10000 /td> | ||
| ) + np.array([[-0.5, 0.5]]) |
There was a problem hiding this comment.
Could you add a comment about the -0.5, 0.5 part?
There was a problem hiding this comment.
This is cased by putting (0,0) in the middle of the upper-left corner. So the upper-left corner is in (-0.5, -0.5).
There was a problem hiding this comment.
That's the "augmented" part, to match the existing code.
There was a problem hiding this comment.
Is there a reason why you're not using simply layer._display_bounding_box_augmented? IIRC it should do exactly what you want, since there's also layer._display_bounding_box_augmented_data_level`?
There was a problem hiding this comment.
_display_bounding_box_augmented_data_level should work and then we wouldnt need this PR -- it's what the code uses -- but it ends up using the data_level from the 2D view when the overlay is updated. I did some testing in the issue, at some point after display = 3 _display_bounding_box_augmented_data_level is updated and correct, but not when the overlay is updated.
_display_bounding_box_augmented doesn't allow you to specify the data_level.
So the easiest thing was to use the method that does allow one to specify the level and then augment it manually.
There was a problem hiding this comment.
That's what I do here? len(self.layer.data) - 1 is the lowest level.
Unless I'm missing something.
Or do you meant to add a callback to dims.ndisplay in ViewerModel to update the overlay?
There was a problem hiding this comment.
We shouldn't need data level, I think @brisvag is saying that layer._display_bounding_box_augmented is in world coordinates so should work regardless? We know the layer extents in world coordinates which would make data levels moot.
There was a problem hiding this comment.
using layer._display_bounding_box_augmented returns the data_level 0 bounding box regardless of 2d/3d, zoom, etc. so that option is always wrong 😞
There was a problem hiding this comment.
But that should be correct in 3d, no? Or at least, it should... I'm testing stuff manually a bit and none of this behaves like I expect 😢
There was a problem hiding this comment.
No, in 3D multiscale uses the lowest pyramid level -- 0 is the highest.
Currently, the overlay uses the level currently displayed, which actually does work some of the time -- like when you are zoomed out so that the lowest level is displayed.
References and relevant issues
Closes: #7543
Description
3D multiscale layers use the lowest resolution when displayed in 3D.
The bounding_box overlay uses the
data_levelof the layer prior tondisplaychange finishing, so if you're looking at a higher rez 2D slice, then the 3D bounding box will mismatch the displayed data.In this PR I add a test that fails when the actual vertexes of the bounding box don't match those that would be expected of the lowest resolution level.
Then, for the overlay I add a check to see if the layer is 3D and multiscale, and if so use the lowest resolution bounding box (augmented).