Player animation #24554
Player animation #24554
Conversation
|
looks like I need to rebase my two commits, but latest main branch didn't seem very stable. any recommendations? |
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
9cc4e30 to
642167d
Compare
If event_source was stopped, it should remain stopped after resize event
642167d to
6e5456a
Compare
|
I think that the animation module has seen some work recently, so that is probably the reason for the need to rebase. In general, the approach is just to keep up rebasing in case of conflicts, although, of course, review can happen in parallel. |
| if self.useblit: | ||
| self.label.set_animated(True) | ||
| self.ax.patch.set_animated(True) | ||
| self.background = self.canvas.copy_from_bbox(self.ax.bbox) |
There was a problem hiding this comment.
I do not think it is safe to assume that we have drawn the figure at least once here, we need to defer this to the first draw event.
|
We are 👍🏻 on force-pushing to feature branches (as you did). Unfortunately if you are changing lines that are being worked on in parallel you are going to have to resolve the merge conflicts. |
| ONE_FORWARD_SYMBOL = "$\u29D0$" | ||
|
|
||
| def __init__(self, func, init_func, min_value=0, max_value=100, | ||
| pos=(0.125, 0.92), valstep=1, **kwargs): |
There was a problem hiding this comment.
This is going to need a docstring including the required signatures of the functions passed in.
|
This is cool work and am supportive of it going in, however I have some concerns about the blitting implementation in the widgets (being more careful about when we are sure that the background has been drawn at least once and all of the sizes are correct etc) and that this is documented well enough that someone can understand what the signatures of the functions passed in need to be passed in. Rather than using |
| @@ -400,6 +429,8 @@ def __init__(self, ax, label, valmin, valmax, valinit=0.5, valfmt=None, | |||
| super().__init__(ax, orientation, closedmin, closedmax, | |||
| valmin, valmax, valfmt, dragging, valstep) | |||
|
|
|||
| self.useblit = useblit and self.canvas.supports_blit | |||
There was a problem hiding this comment.
Can this sort of thing live in SliderBase?
If blitting is going to be supported on slider then it ought to also be supported on RangeSlider
|
great, I will probably just wait until those fixes are merged then |
|
I am going to push this to 3.8 as I am not confident that we will both of those other PRs resolved and have time to get this re-implemented on top of them by the end of the month. |
|
Moving this to draft. @mikaeltulldahl Hopefully you come back to it after the PRs above are merged! |
PR Summary
-Fix bug in Animation where the animation resumes when resizing window
-Make Button and Slider blittable
-Add PlayerAnimation class with play/pause, time functionality
this is still work in progress, I just thought it was done enough to get some feedback regarding if this is the way to go
should solve #20564
but with a bit less features. but if we actually want all those buttons, it's easy to add
could also add a speed slider/button for slow-motion/fast-forward effects
Known Bugs:
-PlayerAnimation does not work so well when blitting is turned off
-The slider get some strange artifacts with blitting(I have no idea why)
-The sliders value text does not update with blitting(I have no idea why)
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst