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

Improve conversion from After Effects shadow softness value to CALayer.shadowRadius value #2175

Merged
merged 4 commits into from
Sep 11, 2023

Conversation

calda
Copy link
Member

@calda calda commented Sep 8, 2023

This PR fixes an issue where the previous math converting from an After Effects shadow softness value to a CALayer.shadowRadius was producing results that were very incorrect.

When rendering a drop shadow, we can't directly apply the softness value from After Effects to the layer.shadowRadius field. If we do this, it produces shadows that are much softer than they appear in after effects.

I had previously tried to hand-tune a sample animation to perfectly match after effects, which resulted in a weird quadratic equation. This turns out to be over-turned and not general, because it results in junk results for some other values (e.g. in this case below was converting 120 to -11).

Instead, we should just use a simple "divide by X" conversion. This should be good enough without being overfit to any specific test case. I worked with Remington to identify that dividing by 5 seems to work best in the most cases (and most importantly, this actual non-sample animation below).

Before

2023-09-08 15 33 12

After

2023-09-08 15 58 03

@calda
Copy link
Member Author

calda commented Sep 11, 2023

ptal @thedrick

Copy link
Contributor

@thedrick thedrick left a comment

Choose a reason for hiding this comment

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

LGTM @calda

@calda calda merged commit 70d39d2 into master Sep 11, 2023
17 checks passed
@calda calda deleted the cal--shadow-fixes branch September 11, 2023 16:35
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
allenchen1154 added a commit to allenchen1154/lottie-android that referenced this pull request Jul 24, 2024
This applies a constant scale factor to the distance and softness (aka blur or radius) properties on drop shadow effects so that the end result more closely matches what is shown in After Effects and on other platforms.

See airbnb/lottie-ios#2175 for similar changes made to the iOS Lottie implementation.
gpeal added a commit to airbnb/lottie-android that referenced this pull request Aug 4, 2024
These changes make several improvements to how drop shadow effects are displayed:
- Drop shadows now take parent alpha into account. This means that if a layer or fill has an animated opacity, the drop shadow will multiply that opacity with its own.
- Adds drop shadow support to Image layers. There are some visual bugs with how the shadows are rendered, however.
- Applies a constant scale factor to the distance and softness values from the Lottie file before passing them to `Paint.setShadowLayer()` to more closely match how they are displayed in After Effects. See airbnb/lottie-ios#2175 for similar changes on iOS.
- Adds three snapshot test files for distance, softness, and alpha validation.

Distance test file:
<img src="https://github.com/user-attachments/assets/aa559e60-0d8f-403b-acd0-fa571d6dcff5" width=400>

Softness test file:
<img src="https://github.com/user-attachments/assets/faa9819f-6584-49f5-91b7-7462213044f5" width=400>

Example of Image layer shadow bug, right side is how it should look (capture from After Effects):
<img width="1824" alt="image" src="https://github.com/user-attachments/assets/a3fb85c3-a5d1-4a6b-bbda-ff4e5ebd2fb5">

Co-authored-by: Gabriel Peal <gabriel@gpeal.com>
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.

2 participants