Ensure _localMatMdf is reset to false
#3040
Merged
Conversation
This file contains 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
With efeb109, support for transform effects has been added, which required introducing another matrix on `TransformElement` to represent the final transform matrix including the effects. This matrix is denoted as `localMat` If there are no transform effects, `localMat` is the same (referentially) as the existing transform matrix (`mat`). Otherwise, it's calculated by composing the transform effects and then composing that with the "old" transform matrix. Alongside `localMat`, a `_localMatMdf` member is introduced, which tracks whether the local matrix was modified since its last commit to the DOM, analogously to `_matMdf`. Unfortunately, a logic bug introduced by efeb109 meant that, in the absence of transform effects, `_localMatMdf` was latched to `true`, i.e. no code path would ever be able to reset it to `false` after it was once set to `true` (e.g. as a result of the matrix being animated). This causes a serious performance regression, since all matrices that once had `_matMdf` set to `true` will keep being updated in the DOM every frame, causing costly full style recalculations. Fix by forcing `_matMdf` and `_localMatMdf` to be equivalent in the case of no transform effects, just as `mat` and `localMat` are referentially equal. In the case of >0 transform effects, the custom logic in `renderLocalTransform()` will take over and compute `_localMatMdf` separately from and based on `_matMdf`.
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.
With efeb109, support for transform effects has been added, which required introducing another matrix on
TransformElementto represent the final transform matrix including the effects. This matrix is denoted aslocalMatIf there are no transform effects,
localMatis the same (referentially) as the existing transform matrix (mat). Otherwise, it's calculated by composing the transform effects and then composing that with the "old" transform matrix.Alongside
localMat, a_localMatMdfmember is introduced, which tracks whether the local matrix was modified since its last commit to the DOM, analogously to_matMdf.Unfortunately, a logic bug introduced by efeb109 meant that, in the absence of transform effects,
_localMatMdfwas latched totrue, i.e. no code path would ever be able to reset it tofalseafter it was once set totrue(e.g. as a result of the matrix being animated).This causes a serious performance regression, since all matrices that once had
_matMdfset totruewill keep being updated in the DOM every frame, causing costly full style recalculations.Fix by forcing
_matMdfand_localMatMdfto be equivalent in the case of no transform effects, just asmatandlocalMatare referentially equal. In the case of >0 transform effects, the custom logic inrenderLocalTransform()will take over and compute_localMatMdfseparately from and based on_matMdf.Affected versions:
lottie-web@5.12.0to latest