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

Change veryLargeRect size to fix issue with masks (#1880) #1884

Merged
merged 1 commit into from
Dec 30, 2022

Conversation

nevil
Copy link
Contributor

@nevil nevil commented Dec 29, 2022

When using scaleToFill / scaleAspectFit / scaleAspectFill mode with an animation that has a subtract mask, the animation could sometimes fail to render. veryLargeRect is used as the size of the "bounds" rect from which the path is subtracted.

Alternative considered
I digged through the old code and found that the old Objective-C implementation actually passed in the bounds size.
I guess that could be an alternative solution if needed.

- (void)updateForFrame:(NSNumber *)frame withViewBounds:(CGRect)viewBounds {
   if ([self hasUpdateForFrame:frame]) {
     LOTBezierPath *path = [_pathInterpolator pathForFrame:frame cacheLengths:NO];

     if (self.maskNode.maskMode == LOTMaskModeSubtract) {
       CGMutablePathRef pathRef = CGPathCreateMutable();
       CGPathAddRect(pathRef, NULL, viewBounds);
       CGPathAddPath(pathRef, NULL, path.CGPath);
       self.path = pathRef;
       self.fillRule = @"even-odd";
     } else {
       self.path = path.CGPath;
     }

     self.opacity = [_opacityInterpolator floatValueForFrame:frame];
   }
 }

502b18ff#diff-233d9c9fcd51dcb99d9884451e850073e2712d14d3c05e0e91bae1c551abe91eR42

Video capture before/after the change

iPhone14 Pro Simulator

Before After
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-12-29.at.15.41.19.mp4
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-12-29.at.15.41.47.mp4

When using scaleToFill / scaleAspectFit / scaleAspectFill mode with an animation
that has a subtract mask, the animation could sometimes fail to render.
`veryLargeRect` is used as the size of the "bounds" rect from which the path is subtracted.
y: -100_000_000,
width: 200_000_000,
height: 200_000_000)
x: -10_000_000,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How large is a large rect? 🤷‍♂️

Copy link
Member

@calda calda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Nice find.

@nevil
Copy link
Contributor Author

nevil commented Dec 30, 2022

Seems like the same test is failing each time, but it shouldn't really be affected by the change to veryLargeRect as it isn't used in that test. 🤔

✖ testParsing_simpleAnimation, XCTAssertEqualWithAccuracy failed: ("1.3376318674468077") is not equal to ("2.0") +/- ("0.65")

@calda
Copy link
Member

calda commented Dec 30, 2022

The performance tests are flaky sometimes -- we just need to increase the threshold

@calda calda enabled auto-merge (squash) December 30, 2022 14:21
@calda calda merged commit f24ad55 into airbnb:master Dec 30, 2022
@nevil nevil deleted the mask-scale-fix branch December 31, 2022 06:42
iago849 pushed a commit to atteamapps/lottie-ios that referenced this pull request Feb 8, 2024
MoroziOS pushed a commit to MoroziOS/tmg-lottie-ios that referenced this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants