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

chore(lib/cli): one video per slide #242

Merged
merged 7 commits into from
Aug 20, 2023
Merged

Conversation

jeertmans
Copy link
Owner

As titled, this PR changes how Manim Slides used to work by only storing one video file per slide.

Previously, a slide would store all animations that occur during the given slide. Up to now, the only "advantage" of this was that it would allow the user to know which animation is played.
But, at the cost of a very complex logic in present, just especially for reversed slides.

On top of top, all converter actually need to concatenate the animations from each slide into one, so it is now performed at rendering time.

To migrate from previous Manim Slides versions, the best is the render the slides again, using manim render or manimgl render.

Currently, it is not possible to start at a given animation anymore. However, if wanted, I may re-implement this, but this would require to change the config file again.

As titled, this PR changes how Manim Slides used to work by only storing one video file per slide.

Previously, a slide would store all animations that occur during the given slide. Up to now, the only "advantage" of this was that it would allow the user to know which animation is played.
But, at the cost of a very complex logic in present, just especially for reversed slides.

On top of top, all converter actually need to concatenate the animations from each slide into one, so it is now performed at rendering time.

To migrate from previous Manim Slides versions, the best is the render the slides again, using `manim render` or `manimgl render`.

Currently, it is not possible to start at a given animation anymore. However, if wanted, I may re-implement this, but this would require to change the config file again.
@jeertmans jeertmans added cli Related to the command line interface lib Related to the library (a.k.a. module) present Related to the main "present" feature labels Aug 20, 2023
@jeertmans jeertmans temporarily deployed to github-pages August 20, 2023 16:10 — with GitHub Actions Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to github-pages August 20, 2023 17:10 Inactive
@jeertmans jeertmans temporarily deployed to github-pages August 20, 2023 17:19 — with GitHub Actions Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to github-pages August 20, 2023 17:32 Inactive
@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

Patch coverage: 78.51% and project coverage change: +4.46% 🎉

Comparison is base (7363281) 58.78% compared to head (92dcf90) 63.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #242      +/-   ##
==========================================
+ Coverage   58.78%   63.25%   +4.46%     
==========================================
  Files          14       15       +1     
  Lines        1752     1682      -70     
==========================================
+ Hits         1030     1064      +34     
+ Misses        722      618     -104     
Files Changed Coverage Δ
manim_slides/convert.py 74.19% <40.00%> (+0.98%) ⬆️
manim_slides/present.py 57.50% <46.66%> (+0.14%) ⬆️
manim_slides/slide.py 84.88% <77.77%> (+29.63%) ⬆️
manim_slides/utils.py 91.89% <91.89%> (ø)
manim_slides/config.py 84.49% <100.00%> (+9.91%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jeertmans jeertmans merged commit b321161 into main Aug 20, 2023
25 of 26 checks passed
@jeertmans jeertmans deleted the one-animation-per-slide branch August 20, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface lib Related to the library (a.k.a. module) present Related to the main "present" feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant