New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vispy 0.12: per-point symbol and several bugfixes #5312
Conversation
Turns out there was a bug in vispy 0.12. Will push 0.12.1 soon with a fix. |
Codecov Report
@@ Coverage Diff @@
## main #5312 +/- ##
=======================================
Coverage 89.01% 89.02%
=======================================
Files 594 594
Lines 50237 50277 +40
=======================================
+ Hits 44719 44758 +39
- Misses 5518 5519 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
8bb422c
to
1279da8
Compare
Yes; check this ci run for an example failure. Unfortuantley, I don't know how to get more info than this :/ |
I don't a see a segfault there - instead an |
Ah, I guess tox is running
I have some issues getting tox to work in my py38 environment. But the tests skipped here work with direct calls to pytest in such an environment, so that's more good news :) Still not particularly happy about skipping tests without well understood reasons, but it's not enough to block this PR, which we should get in soon to start exercising vispy 0.12. |
this looks like a memory violatation error based on fast google. |
Right, my bad: unfortunately when you run tox with
Yeah I feel the same, but if the tests block us completely rather than finding real errors, they're counterproductive. |
Like I said, I don't think this PR is blocked, so you should feel free to merge. I may dig a little deeper and follow up with some findings later though, as the error is pretty clearly related to the new vispy version (though it's still unclear if it's only Windows CI where that can happen). |
# Description With vispy 0.12 (#5312) we introduced some changes to how point sizes are computed. Specifically, the `scale` is no longer used to determine the size. The initial reason was #2213. For a long explanation (with all the caveats and complications), see vispy/vispy#2453 (specifically, [this comment has a summary of the behaviour](vispy/vispy#2453 (comment))). While the current state not ideal (and we're trying to figure out over at vispy if we can solve it), this at least solves nasty problems like #2213. However, it means that a size of `10` is still `10` even if the scale of the layer is `5` (and not `50`). This may "break" some code that relies on this transformation; in napari, one of the consequences is that the `new points layer` button on the viewer gui will now create tiny (or massive) points if the scale is different. To test, try this: ```py import napari import numpy as np v = napari.Viewer() v.add_image(np.random.rand(100, 100), scale=[10, 10]) ``` then click the "new points" button and try to annotate some points manually. On `main`, they'll be so small that they're invisible. --- This also exposed another issue (which became the main point of this PR): while point sizes are currently anisotropic, this information is not properly used in many places (visualisation being the primary); not only it's mostly unused, but it's handwaved in a few places (we arbitrarily take the average size over each dimension to determine the visualised size, we only allow setting isotropic sizes from gui and the `current_size`, and so on). Ultimately, we decided that we can't reasonably support anisotropic sizes, so we should do away with them. Most changes in this PR have to do with that. ## Type of change <!-- Please delete options that are not relevant. --> - [x] Bug-fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update --------- Co-authored-by: Andy Sweet <andrew.d.sweet@gmail.com> Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com> Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
With vispy 0.12 (#5312) we introduced some changes to how point sizes are computed. Specifically, the `scale` is no longer used to determine the size. The initial reason was #2213. For a long explanation (with all the caveats and complications), see vispy/vispy#2453 (specifically, [this comment has a summary of the behaviour](vispy/vispy#2453 (comment))). While the current state not ideal (and we're trying to figure out over at vispy if we can solve it), this at least solves nasty problems like scale of the layer is `5` (and not `50`). This may "break" some code that relies on this transformation; in napari, one of the consequences is that the `new points layer` button on the viewer gui will now create tiny (or massive) points if the scale is different. To test, try this: ```py import napari import numpy as np v = napari.Viewer() v.add_image(np.random.rand(100, 100), scale=[10, 10]) ``` then click the "new points" button and try to annotate some points manually. On `main`, they'll be so small that they're invisible. --- This also exposed another issue (which became the main point of this PR): while point sizes are currently anisotropic, this information is not properly used in many places (visualisation being the primary); not only it's mostly unused, but it's handwaved in a few places (we arbitrarily take the average size over each dimension to determine the visualised size, we only allow setting isotropic sizes from gui and the `current_size`, and so on). Ultimately, we decided that we can't reasonably support anisotropic sizes, so we should do away with them. Most changes in this PR have to do with that. <!-- Please delete options that are not relevant. --> - [x] Bug-fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update --------- Co-authored-by: Andy Sweet <andrew.d.sweet@gmail.com> Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com> Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
# Description With vispy 0.12 (napari#5312) we introduced some changes to how point sizes are computed. Specifically, the `scale` is no longer used to determine the size. The initial reason was napari#2213. For a long explanation (with all the caveats and complications), see vispy/vispy#2453 (specifically, [this comment has a summary of the behaviour](vispy/vispy#2453 (comment))). While the current state not ideal (and we're trying to figure out over at vispy if we can solve it), this at least solves nasty problems like napari#2213. However, it means that a size of `10` is still `10` even if the scale of the layer is `5` (and not `50`). This may "break" some code that relies on this transformation; in napari, one of the consequences is that the `new points layer` button on the viewer gui will now create tiny (or massive) points if the scale is different. To test, try this: ```py import napari import numpy as np v = napari.Viewer() v.add_image(np.random.rand(100, 100), scale=[10, 10]) ``` then click the "new points" button and try to annotate some points manually. On `main`, they'll be so small that they're invisible. --- This also exposed another issue (which became the main point of this PR): while point sizes are currently anisotropic, this information is not properly used in many places (visualisation being the primary); not only it's mostly unused, but it's handwaved in a few places (we arbitrarily take the average size over each dimension to determine the visualised size, we only allow setting isotropic sizes from gui and the `current_size`, and so on). Ultimately, we decided that we can't reasonably support anisotropic sizes, so we should do away with them. Most changes in this PR have to do with that. ## Type of change <!-- Please delete options that are not relevant. --> - [x] Bug-fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update --------- Co-authored-by: Andy Sweet <andrew.d.sweet@gmail.com> Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com> Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
# Description Bump vispy to 0.13 to get some extra features. Relevant changes are: - vispy/vispy#2465, which can improve performance dramatically for `mip`-style projections in volumes by stopping the ray cast early if the max clim was already reached - a couple improvements to touchaped gestures (vispy/vispy#2456, vispy/vispy#2483) Reverted defaults of `Markers.scaling` also mean that we need to add a couple lines in napari to maintain the same behaviour ( see vispy/vispy#2359 and #5312 if you want to dive deeper). cc @aganders3 who did much of the vispy work :) --------- Co-authored-by: Matthias Bussonnier <bussonniermatthias@gmail.com> Co-authored-by: napari-bot <napari-bot@users.noreply.github.com>
Bump vispy to 0.13 to get some extra features. Relevant changes are: - vispy/vispy#2465, which can improve performance dramatically for `mip`-style projections in volumes by stopping the ray cast early if the max clim was already reached - a couple improvements to touchaped gestures (vispy/vispy#2456, vispy/vispy#2483) Reverted defaults of `Markers.scaling` also mean that we need to add a couple lines in napari to maintain the same behaviour ( see vispy/vispy#2359 and #5312 if you want to dive deeper). cc @aganders3 who did much of the vispy work :) --------- Co-authored-by: Matthias Bussonnier <bussonniermatthias@gmail.com> Co-authored-by: napari-bot <napari-bot@users.noreply.github.com>
Description
Bump vispy to 0.12. This fixes several bugs that we fixed upstream, and introduces some new functionality. In short:
symbol
is now per-point, just like size, color and and so on. I added a small example to showcase this:multi_symbol.mp4
fixes Inconsist size of Point when rotate in 3d #2213, a long-standing issue with anisotropic scaling and points. To fix this, we now set the size of points correctly in world space, which is different from what currently happens in napari. On main, points with size
10
and scaling2
will result in actual size of20
. While this may seem reasonable in some cases, it breaks down with anisotropic scales. With this fix, a size of10
will be10
no matter the scaling. This means that this might "break" some existing code which relied on the bug.fixes the issue with clipping planes slowing down points reported in Dimension slider is very slow with points layer #5123, which was patched with an ugly workaround in Fix slow points #5133. The workaround is no longer needed.
thanks to @aganders3, the
attenuated_mip
portion of Volume rendering updates for isosurface and attenuated MIP #5215 was upstreamed to vispy, so it's no longer needed herethanks to @psobolewskiPhD , more PySide6 complications were fixed :)
gpu image interpolation with custom kernels was fixed, which allows to move forward with expose custom image interpolation kernels #5130
bonus: instanced rendering is now a first-class citizen in vispy, so we can start experimenting how to use it in napari :P
Type of change