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

Clamp gradient color opacity #1636

Merged
merged 2 commits into from
Oct 2, 2020
Merged

Conversation

AlexKGwyn
Copy link
Contributor

Fixes an issue where color positions that came before any opacity positions in a gradient color array were interpolated to negative values, throwing off the conversion into a color.

An example of a gradient color array that presented this issue:
"k": [0, 0.969, 0.514, 0.745, 0.116, 0.984, 0.537, 0.373, 1, 1, 0.559, 0, 0.607, 0, 0.776, 0.5, 0.945, 1]

Colors
0, 0.969, 0.514, 0.745,
0.116, 0.984, 0.537, 0.373,
1, 1, 0.559, 0,
Opacity
0.607, 0,
0.776, 0.5,
0.945, 1

Since the first color came before the first opacity position, the progress was calculated as about -3.59 causing the following linear interpolation to give out a large negative value which broke things when it was bit shifted into the alpha position for the final color.

Here are some screenshots:

Original issue
Screenshot_20201001-011043

Resolved
Screenshot_20201001-010909

Sample Json (as a txt because github doesn't allow json)
testjson.txt

@@ -139,7 +139,7 @@ private int getOpacityAtPosition(double position, double[] positions, double[] o
double lastPosition = positions[i - 1];
double thisPosition = positions[i];
if (positions[i] >= position) {
double progress = (position - lastPosition) / (thisPosition - lastPosition);
double progress = MiscUtils.clamp((position - lastPosition) / (thisPosition - lastPosition), 0, 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume it would be fine to change the precision of the progress and use an existing clamp method. I didn't want to make that change without some more knowledge about the choices here though.

@LottieSnapshotBot
Copy link

Snapshot Tests
28: Report Diff

@gpeal
Copy link
Collaborator

gpeal commented Oct 1, 2020

@AlexKGwyn This looks great. I verified that the diffs in the link above are, in fact, more accurate that the originals. Could you also add your json file to this folder. That will automatically include it in the report above.

@LottieSnapshotBot
Copy link

Snapshot Tests
28: Report Diff

Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

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

Thanks!

@gpeal gpeal merged commit 06274f4 into airbnb:master Oct 2, 2020
gpeal pushed a commit that referenced this pull request Oct 4, 2020
Fixes an issue where color positions that came before any opacity positions in a gradient color array were interpolated to negative values, throwing off the conversion into a color.
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.

3 participants