Fixed ComposeMatteLayers method to work for invisible layers #479
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
eliezerpMS
reviewed
Dec 15, 2021
source/LottieToWinComp/Masks.cs
Outdated
| { | ||
| // Matte layer is null, which means that it is never visible and LayerMatteType is equal to Invert. | ||
| // In this case we should just return mattedVisual because matte layer is effectively a no-op. | ||
| yield return new LayerTranslator.FromVisual(mattedVisual); |
There was a problem hiding this comment.
Is this what happens in After Effects? (As in if you make the matte inverted and not visible then it just shows the layer to be matted?)
Also should this produce an issue or something? Is there a case where a creator would have done this intentionally (have the matte never show up)?
There was a problem hiding this comment.
Yes this is how it works in AE.
Actually this is a good idea I think, because that is how we discovered this bug, Lottie-Windows produced wrong result but there were no issues to see what exactly is wrong.
There was a problem hiding this comment.
Cool. Thanks for adding the new issue!
I am thinking though that we probably want the issue to pop up even for the invert no-op case too right?
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
translatedLayer.GetVisualRootmay returnnullif it detects that layer will not be visible (it has opacity zero, or visible time range is out of parent's time range). If this layer should be matted or should be used as a matte layer it may cause some artifacts to appear.New implementation should be more robust and should handle most corner cases.
mattedLayerandmatteare not null -> compose them into one layermattedLayerisnull-> skip both layersmattelayers isnull-> we can skip them too, (!) except for the case whenMatteTypeisInvert, in this case we are inverting invisible layer which mean that it is always visible, so we should keepmattedLayer