Repository navigation
Fix time remapping with stretched precompositions - #427
Merged
Merged
Conversation
Signed-off-by: snowyukitty <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #427 +/- ##
=======================================
Coverage 83.95% 83.96%
=======================================
Files 163 163
Lines 6664 6672 +8
=======================================
+ Hits 5595 5602 +7
- Misses 1069 1070 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merge the two _timeRemapping checks into an if/else and record that the time stretch is intentionally not applied to remapped child time. Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the reproducible blank-frame case in the original attachment to #323.
The two later reports in that thread do not include equivalent assets, so this
PR does not claim to resolve them.
Root cause
When a precomposition has time remapping, lottie-flutter converts the remapped
seconds into child-frame progress and then divides that already-remapped result
by the layer's
sr. In the original animation, this second operation moves thechild outside its visible content and produces blank frames.
Time-remap output already identifies child time.
lottie-web
and both Lottie iOS rendering paths
(main-thread,
Core Animation)
use that output directly and apply start/stretch calculation only when
remapping is absent.
The Lottie specification
separately says stretch is applied first when both
srandtmare present.This PR does not change the input used to sample an animated
tm; it onlyremoves the output-side
srdivision.Lottie Android
currently retains that division, so this is an intentional compatibility
difference supported by the original asset and the web/iOS behavior.
Summary
ValueDelegate.timeRemapvalues as direct child time too,without applying the layer stretch to their output
the old extra division incorrectly selects frame 25
time stretch
Testing
pixel instead of red; the no-remap control passed
flutter test test/time_remapping_test.dart: 2 passedflutter analyzeflutter testinexample: 1 testflutter pub run tool/prepare_submit.dartflutter build webinexampleThe full root
flutter testrun on Windows completed with 990 passed and 30failed. Every failure was a golden pixel comparison. The tracked golden inputs
with time remapping all have an effective
srof 1, so those failures do notexercise the changed
tmplus non-unit-srpath.This repository documents golden generation and verification for pinned Apple
Silicon macOS runners. That canonical golden run was not available locally.