tlambert03 commented 3 years ago

~Not done here, but curious how you feel about this pattern.~

In looking at the animation slider, I noticed that the animationSliderWidget itself had a lot of control over the creation of interpolated frames. and that whenever anything about the full movie changes, the slider widget triggers interpolation of the full movie:

        for state in self.animation._state_generator():
            self.interpol_states.append(state)

This PR adds a new object (FrameSequence(Sequence[ViewerState])). It acts as an (immutable) sequence of ViewerStates. You can index into it (frame_sequence[n]) to get the nth interpolated ViewerState of the final movie... and it will only calculate the state that is being requested (and it will cache it until the underlying key_frame list changes) You can get the length with len(frame_sequence)... This means that the animation slider can be a plain QSlider that just sets the animation state to the corresponding index of the FrameSequence

~probably a couple lingering bugs to work out. but wanted to get your thoughts first:~

codecov[bot] commented 3 years ago

Codecov Report

Merging #92 (59b2ecb) into main (a04d1b4) will increase coverage by 15.44%. The diff coverage is 93.47%.

@@             Coverage Diff             @@
##             main      #92       +/-   ##
===========================================
+ Coverage   74.93%   90.37%   +15.44%     
===========================================
  Files          18       20        +2     
  Lines         718      810       +92     
===========================================
+ Hits          538      732      +194     
+ Misses        180       78      -102

Impacted Files	Coverage Δ
napari_animation/_qt/frame_widget.py	`90.90% <0.00%> (+59.65%)`	:arrow_up:
napari_animation/animation.py	`76.92% <50.00%> (-4.73%)`	:arrow_down:
napari_animation/key_frame.py	`92.98% <90.00%> (-1.89%)`	:arrow_down:
...pari_animation/_qt/_tests/test_animation_widget.py	`100.00% <100.00%> (ø)`
napari_animation/_qt/animation_widget.py	`88.73% <100.00%> (+60.73%)`	:arrow_up:
napari_animation/_tests/conftest.py	`100.00% <100.00%> (ø)`
napari_animation/_tests/test_animation.py	`100.00% <100.00%> (ø)`
napari_animation/_tests/test_frame_sequence.py	`100.00% <100.00%> (ø)`
napari_animation/frame_sequence.py	`100.00% <100.00%> (ø)`
napari_animation/interpolation.py	`97.95% <100.00%> (-2.05%)`	:arrow_down:
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a04d1b4...59b2ecb. Read the comment docs.

tlambert03 commented 3 years ago

this is all set for another look

tlambert03 commented 3 years ago

ok, all set again... added a few more tests to cover some of these errors

alisterburt commented 3 years ago

just waiting on tests then will merge!

napari / napari-animation

Add FrameSequence object with lazy viewer state interpolation #92

Codecov Report