search

napari / napari-animation

A napari plugin for making animations
https://napari.github.io/napari-animation/
Other
79 stars 27 forks source link

Slerp + dict of interpolation functions #62

Closed Fifourche closed 3 years ago

Fifourche commented 3 years ago

This PR is to propose a way for the user to give custom interpolation functions to compute frames. It also contains a Slerp default method. A related issue has been opened at #37 .

About custom interpolation functions

In details, I added a "methods" argument to the capture_keyframe function. "methods" is a dict of dicts containing functions, normally of same structure than "state".

For instance, the custom interpolation function for the state['camera']['angles'] would be looked for at methods['camera']['angles'].

A few things to notice and to be discussed I think :

About Slerp

I took advantage of this to add a Slerp method, relying on scipy. The default interpolation function for "angles" is set to Slerp.

An example code :

Here's an example code where "methods" is either set to a standard linear interpolation for "angles" and or defaulted, resulting in Slerp for "angles". Playing with the slider will help visualizing the difference.

import napari
from napari_animation import AnimationWidget
from skimage.data import cells3d

cells = cells3d()[:, 1]  # get some data

# Launch viewer to view data
viewer = napari.view_image(cells, colormap='magma')
viewer.dims.ndisplay = 3

# Instantiate the animation widget
animation_widget = AnimationWidget(viewer)
viewer.window.add_dock_widget(animation_widget, area='right')
animation = animation_widget.animation

# Angles interpolation is set to simple linear interpolation of a sequence
from napari_animation.utils import _interpolate_seq
methods_test = {'camera': {'angles': _interpolate_seq}}
# methods_test = {} # uncomment to default to Slerp instead

#%% Capture frames

animation.key_frames.clear()

viewer.camera.angles = (0, 0, 90)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (0, 0, 180)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (-180, -90, 0)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (180, 0, 0)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (-180, 90, 0)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (0, 0, -180)
animation.capture_keyframe(methods=methods_test)

viewer.camera.angles = (0.0, 0.0, 90.0)
animation.capture_keyframe(methods=methods_test)
sofroniewn commented 3 years ago

I'm afraid I havn't got too much time right now to think deeply about this, but I like the idea of getting rid of my pow 10 and log functions for zoom etc. that really did feel like a hack. I also tend to agree with a lot of @alisterburt points about API, keeping this hidden from users as much as possible if we know what should be done for an attribute, and using an API something like 

{
'camera.zoom' : Interpolation.LOG,
'camera.angles' : Interpolation.SLERP
}

which seems quite elegant.

I think this PR might also contain some changes from #61, so once we get that merged this should be easier to review. I should be able to get to it early next week, but in the mean time I say press on! 🚀

Fifourche commented 3 years ago

Thanks for your thoughts ! :) I agree with you, it should probably stay invisible to the user !

For this and for the cascading, what I had in mind at the time was the interpolation of the recently added layers' states. Some of the properties -not the base ones, but things like colormap or others- might be tricky to interpolate, and I imagined scenarios where the interpolation fucntion would need to change opacity as well as other parameters for instance.

It was and still isn't clear in my mind how to prepare for this, but in any case I agree with everything you said above, and will adopt the proposed form ! Thanks again 😃

alisterburt commented 3 years ago

awesome - look forward to seeing what you come up with @Fifourche !

You should join us on the napari zulip chat - there are also developer meetings each week, it would be great to have you there!

codecov[bot] commented 3 years ago

Codecov Report

Merging #62 (59a3cc8) into main (dee3dc3) will increase coverage by 4.48%. The diff coverage is 99.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   62.70%   67.18%   +4.48%     
==========================================
  Files          15       17       +2     
  Lines         732      835     +103     
==========================================
+ Hits          459      561     +102     
- Misses        273      274       +1
Impacted Files Coverage Δ
napari_animation/_tests/test_animation.py 100.00% <ø> (ø)
napari_animation/utils.py 97.43% <97.36%> (-2.57%) :arrow_down:
napari_animation/_tests/test_interpolation.py 100.00% <100.00%> (ø)
napari_animation/_tests/test_utils.py 100.00% <100.00%> (ø)
napari_animation/animation.py 79.54% <100.00%> (-0.46%) :arrow_down:
napari_animation/interpolation.py 100.00% <100.00%> (ø)

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 dee3dc3...59a3cc8. Read the comment docs.

Fifourche commented 3 years ago

I made some changed according to the discussion !

I created a dictionary :

interpolation_dict = {
    "camera.angles": Interpolation.SLERP,
    "camera.zoom": Interpolation.LOG,
}

The unspecified properties will be defaulted to Interpolation.LINEAR. One thing I don't know is where to place this dict ; I put it at the very end of interpolation.py but it may be a bit hidden there.

Also, I guess some tests should be changed accordingly ; I'll take care of that soon if you're ok with the commits so far !

sofroniewn commented 3 years ago

@Fifourche we've got some merge conflicts now since I merged #68, do you mind updating this PR. Then I think it is good to go!!