Player animation by mikaeltulldahl · Pull Request #24554 · matplotlib/matplotlib · GitHub

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

Has pytest style unit tests (and pytest passes)
Documentation is sphinx and numpydoc compliant (the docs should build without error).
New plotting related features are documented with examples.

Release Notes

New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

looks like I need to rebase my two commits, but latest main branch didn't seem very stable. any recommendations?

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.

If event_source was stopped, it should remain stopped after resize event

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.

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.

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 make_adjustable, I think it would be better to make use of the new SubFigure functionality + subplot mosaic to layout the UI (and maybe as a stretch make it possible to pass in the axes to be used for the UI).

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

Button blitting is implemented in #23457.
Slider blitting is implemented in #23439, but the trouble there, as you have found, is that there is no good way to blit the text generically.

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!

github-actions Bot reviewed Nov 27, 2022

View reviewed changes

mikaeltulldahl force-pushed the player_animation branch from 9cc4e30 to 642167d Compare November 27, 2022 21:20

github-actions Bot added the status: needs rebase label Nov 27, 2022

mikaeltulldahl added 3 commits November 27, 2022 22:24

FIX: Animation shouldn't start by itself after resizing window

d0d5bd1

If event_source was stopped, it should remain stopped after resize event

Make Button and Slider support blitting

3df1db3

Added PlayerAnimation with example

6e5456a

mikaeltulldahl force-pushed the player_animation branch from 642167d to 6e5456a Compare November 27, 2022 21:26

github-actions Bot removed the status: needs rebase label Nov 27, 2022

mikaeltulldahl marked this pull request as ready for review November 27, 2022 21:27

oscargus added the topic: animation label Nov 28, 2022

tacaswell reviewed Nov 28, 2022

View reviewed changes

tacaswell added this to the v3.7.0 milestone Nov 28, 2022

tacaswell reviewed Nov 28, 2022

View reviewed changes

ianhi reviewed Nov 28, 2022

View reviewed changes

tacaswell modified the milestones: v3.7.0, v3.8.0 Dec 15, 2022

github-actions Bot added the status: needs rebase label Dec 29, 2022

jklymak added the status: waiting for other PR label Jan 30, 2023

jklymak marked this pull request as draft January 30, 2023 23:27

ksunden modified the milestones: v3.8.0, v3.9.0 Aug 9, 2023

QuLogic removed this from the v3.9.0 milestone Mar 13, 2024

QuLogic added this to the v3.10.0 milestone Mar 13, 2024

ksunden modified the milestones: v3.10.0, future releases Oct 9, 2024

+                      if self.useblit:
+                          self.label.set_animated(True)
+                          self.ax.patch.set_animated(True)
+                          self.background = self.canvas.copy_from_bbox(self.ax.bbox)

Player animation by mikaeltulldahl · Pull Request #24554 · matplotlib/matplotlib · GitHub

Repository navigation

Player animation - #24554

Player animation#24554
mikaeltulldahl wants to merge 3 commits into
matplotlib:mainfrom
mikaeltulldahl:player_animation

mikaeltulldahl commented Nov 27, 2022

Uh oh!

mikaeltulldahl commented Nov 27, 2022

Uh oh!

github-actions Bot left a comment

Uh oh!

oscargus commented Nov 28, 2022

Uh oh!

tacaswell Nov 28, 2022

Uh oh!

tacaswell commented Nov 28, 2022

Uh oh!

tacaswell Nov 28, 2022

Uh oh!

tacaswell commented Nov 28, 2022

Uh oh!

ianhi Nov 28, 2022

Uh oh!

QuLogic commented Nov 29, 2022

Uh oh!

mikaeltulldahl commented Dec 6, 2022

Uh oh!

tacaswell commented Dec 15, 2022

Uh oh!

jklymak commented Jan 30, 2023

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

+                  ONE_FORWARD_SYMBOL = "$\u29D0$"
+                  def __init__(self, func, init_func, min_value=0, max_value=100,
+                               pos=(0.125, 0.92), valstep=1, **kwargs):

                       super().__init__(ax, orientation, closedmin, closedmax,
                                        valmin, valmax, valfmt, dragging, valstep)
+                      self.useblit = useblit and self.canvas.supports_blit

Player animation by mikaeltulldahl · Pull Request #24554 · matplotlib/matplotlib · GitHub

Repository navigation

Player animation - #24554

Player animation#24554mikaeltulldahl wants to merge 3 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom mikaeltulldahl:player_animationmikaeltulldahl/matplotlib:player_animationCopy head branch name to clipboard

Conversation

mikaeltulldahl commented Nov 27, 2022

PR Summary

PR Checklist

Uh oh!

mikaeltulldahl commented Nov 27, 2022

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

oscargus commented Nov 28, 2022

Uh oh!

tacaswell Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Nov 28, 2022

Uh oh!

tacaswell Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Nov 28, 2022

Uh oh!

ianhi Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Nov 29, 2022

Uh oh!

mikaeltulldahl commented Dec 6, 2022

Uh oh!

tacaswell commented Dec 15, 2022

Uh oh!

jklymak commented Jan 30, 2023

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Player animation#24554
mikaeltulldahl wants to merge 3 commits into
matplotlib:mainfrom
mikaeltulldahl:player_animation