Repository navigation
Make animation blit cache robust against 3d viewpoint changes. - #16070
Conversation
timhoffm
left a comment
timhoffm
left a comment
There was a problem hiding this comment.
This changes the view checking from notification to polling. We have to execute _get_view() every time we want to use the background. Have you evaluated the performance impact?
Can we introduce a generic view_changed callback instead.
|
_get_view returns the axes limits (plus the view angles in 3D), which is something that needs to be accessed anyways for drawing the artist. So yes it's a couple more attribute accesses but I doubt it matters (but I have not checked this). In theory custom projections need to implement _get_view anyways to interact properly with the home/previous/next buttons, so it seems annoying to request that they additionally need to implement view_changed (_get_view and view_changed going out of sync would be yet another possible source of bugs, whereas if there's only one source of truth, it can't go out of sync with itself). |
| pass | ||
| else: | ||
| if ax._get_view() == view: | ||
| ax.figure.canvas.restore_region(bg) |
There was a problem hiding this comment.
Might as well evict invalid backgrounds here as well?
|
This seems to be simpler over all. While this is in the hot-loop of the animation loop so we should be concerned, but on my few year laptop, |
tacaswell
left a comment
tacaswell
left a comment
There was a problem hiding this comment.
modulo pre-emptive cache eviction.
Instead of connecting to the xlim_changed/ylim_changed events to clear the blit cache, use Axes._get_view(), which also works for 3d axes.
Instead of connecting to the xlim_changed/ylim_changed events to clear
the blit cache, use Axes._get_view(), which also works for 3d axes.
Closes #16069. No test because good luck with that, but one can repro with the example at that issue.
PR Summary
PR Checklist