-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
[WIP]: Add dev mode for faster rendering #323
base: main
Are you sure you want to change the base?
[WIP]: Add dev mode for faster rendering #323
Conversation
Will have a look later this weekend. Probably Sunday. Do you mind giving some code to go into dev mode and a bit more explanation to what should happen? |
Yes sure @Wikunia.
This is the way one should use it. I am still on the benchmarking part, trying to see what is the outcome. I did not find a test so far where not re-drawing the frames had any significant speed up. I will try to find one and add it as a test in a subsequent commit as test. Thanks. |
b933f60
to
52ca40a
Compare
Sure @codejaeger - I could probably do a cursory review today to give some feedback. 😄 |
Thanks! What's the |
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
==========================================
- Coverage 96.31% 95.84% -0.48%
==========================================
Files 21 21
Lines 1113 1130 +17
==========================================
+ Hits 1072 1083 +11
- Misses 41 47 +6
Continue to review full report at Codecov.
|
I also mentioned a reason why specifying the frame range could be a difficult option in this comment. |
@codejaeger, Sorry for poking my nose in here, but I totally understand the |
Lol we had the same thought @Sov-trotter just in a different issue #166 |
Will try that @Sov-trotter, thanks for the suggestion 😃 |
ea3b1ec
to
d05d310
Compare
@Wikunia I tried the approach of storing the frames in memory. In a crude way, there is a performance speed up of 25% when I try with examples generating 300 and 500 frames. I will add tests for these. I wanted to ask does this speed up seem reasonable against the increased memory consumption? |
I think it makes sense to provide the option with bigger memory consumption but for faster rerendering. |
PR Checklist
If you are contributing to
Javis.jl
, please make sure you are able to check off each item on this list:CHANGELOG.md
with whatever changes/features I added with this PR?Project.toml
+ set an upper bound of the dependency (if applicable)?test
directory (if applicable)?Link to relevant issue(s)
Attempt to close #166
How did you address these issues with this PR? What methods did you use?
The idea is to automatically track the updated frames in case the
dev_mode
is specified while creating the videos. This definitely has its limitations and cannot always track every unchanged frame i.e. if the image of the frame is left unchanged but somehow that frame was referred to or used since last render it will still get created in the next render.Currently, tests need to be added to benchmark the improvements (if any). @Wikunia @TheCedarPrince could you give a initial review if possible?
Thanks!