Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GSoC'21] Different morphing modes #374

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

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

@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

src/Shape.jl Outdated Show resolved Hide resolved
@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

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

src/Shape.jl Show resolved Hide resolved
Sov-trotter and others added 3 commits August 3, 2021 18:36
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

CHANGELOG.md Outdated
@@ -1,5 +1,8 @@
# 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