Skip to content

Conversation

@Sov-trotter
Copy link
Member

@Sov-trotter Sov-trotter commented Jul 24, 2021

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

  • Did I update CHANGELOG.md with whatever changes/features I added with this PR?
  • Did I make sure to only change the part of the file where I introduced a new change/feature?
  • Did I cover all corner cases to be close to 100% test coverage (if applicable)?
  • Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
  • Did I properly add test dependencies to the test directory (if applicable)?
  • Did I check relevant tutorials that may be affected by changes in this PR?
  • Did I clearly articulate why this PR was made the way it was and how it was made?

How did you address these issues with this PR? What methods did you use?
This PR introduces some manim like modes for morphing
https://azarzadavila-manim.readthedocs.io/en/latest/animation.html#transform

TEST SCRIPT

Longest distance mode of morphing
@Sov-trotter Sov-trotter changed the title Different morphing modes [GSoC'21] Different morphing modes Jul 24, 2021
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #374 (b79ff00) into master (d42cbf4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #374   +/-   ##
=======================================
  Coverage   96.25%   96.25%           
=======================================
  Files          35       35           
  Lines        1521     1523    +2     
=======================================
+ Hits         1464     1466    +2     
  Misses         57       57           
Impacted Files Coverage Δ
src/Shape.jl 97.05% <100.00%> (+0.03%) ⬆️
src/morphs.jl 96.46% <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 d42cbf4...b79ff00. Read the comment docs.

Copy link
Member

@Wikunia Wikunia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment here. Let me know when I should do a proper full review

@Sov-trotter
Copy link
Member Author

@Wikunia since the goal here is more visual appeal, should we provide some default easing function(apart from linear()) in this PR??

@Wikunia
Copy link
Member

Wikunia commented Jul 26, 2021

I think it makes sense to have the easing separated from the function to have it more composable. The default for all actions should be linear() I think. We might want to create predefined actions similar to predefined objects in the future where we can tackle this. What do you think?

@Sov-trotter
Copy link
Member Author

I think it makes sense to have the easing separated from the function to have it more composable.

Understood. I was just of the opinion, that in longer running videos it would rather be easy for the user if the small morphs had atleast some visual appeal by default.

We might want to create predefined actions similar to predefined objects in the future where we can tackle this.

Sure yeah. I had thought of that while tackling predefined objects. There are a few things to look into before that.
First is making the Javis Recipes(which are obviously actions) more "available". I mean to say that the users don't have to create overloads like:
(video, object, action, rel_frame) -> _translate(video, object, action, rel_frame)
and then go on to define their custom actions in _foo function.

Maybe a macro is handy here.
So this itself accounts for predefined actions(for recipes/internally)

Secondly we can have some predefined actions but at present I still don't know we to wrap around? Would like to hear what ideas you have.

@Wikunia
Copy link
Member

Wikunia commented Jul 26, 2021

Let's discuss that tmr in our meeting 😉

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Jul 30, 2021

Self TODO : Check if morphing modes work with #373

Sov-trotter and others added 10 commits July 30, 2021 13:48
Test morphing modes
Update changelog
Docstring for style
Format
Update reference images
Run all tests
rectify keyword argument
Copy link
Member

@Wikunia Wikunia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing

Sov-trotter and others added 3 commits August 3, 2021 18:36
Add docstring for prepare_to_interpoplate
I feel `floor(Int, ` is cleaner as circshift by a float doesn't make sense
@Wikunia
Copy link
Member

Wikunia commented Aug 3, 2021

Still not a fan of :long how about :shortest_dist and :visually_appealing ?
Maybe also add how it works in the docstring of morph_to.

@Wikunia
Copy link
Member

Wikunia commented Aug 6, 2021

We've decided to call those modes mode1, mode2, ... and have animations in the docs for the different morphing modes.
Additionally it would be good to have arguments to parameterize some of the modes like the one implemented in this PR

Sov-trotter and others added 4 commits August 8, 2021 20:37
conflicts
Fix changelog conflicts
saner morphing modes nomenclature
CHANGELOG.md Outdated
# Javis.jl - Changelog

## Unreleased
- Add a manim-like :long morhping mode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be updated here as well

@Wikunia Wikunia changed the base branch from master to main February 25, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants