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

Add interpolateColor worklet #960

Closed
wants to merge 1 commit into from

Conversation

likern
Copy link
Contributor

@likern likern commented Jul 8, 2020

Description

This PR adds interpolateColor worklet for Reanimated 2 which interpolates colors in RGBA space.
For now there is no way to interpolate colors, only numbers are allowed.

Changes

  • Added destructColor function which split number color into RGBA chanels
  • Added interpolateColor function which interpolates color

@likern likern changed the title ## Description ## Add interpolateColor worklet Jul 8, 2020
@likern likern changed the title ## Add interpolateColor worklet Add interpolateColor worklet Jul 8, 2020
@likern likern changed the title Add interpolateColor worklet DO NOT MERGE: Add interpolateColor worklet Jul 8, 2020
@likern likern changed the title DO NOT MERGE: Add interpolateColor worklet [DO NOT MERGE] Add interpolateColor worklet Jul 8, 2020
@likern
Copy link
Contributor Author

likern commented Jul 8, 2020

Unfortunately, I couldn't test these changes using modified react-native-reanimated package because I've got "Unsatisfied Link Error" for reanimated.so. I've tested using javascript playground.

If there are maintainers with complete build setup - please, verify this pull-request is working. I think it should.

photo5204307569185762798

@karol-bisztyga
Copy link
Contributor

karol-bisztyga commented Jul 9, 2020

Just a tiny note: you do not have to add [DO NOT MERGE], a draft pull request is a feature designed for such cases

@likern likern changed the title [DO NOT MERGE] Add interpolateColor worklet Add interpolateColor worklet Jul 9, 2020
@wcandillon
Copy link
Contributor

I have these functions working well I think at https://github.com/wcandillon/react-native-redash/pull/289/files#diff-1234f550d692aff6b8f2a1952daf79ec. Is this helping?

Copy link
Contributor

@karol-bisztyga karol-bisztyga left a comment

Choose a reason for hiding this comment

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

could you please provide a piece of code with which you tested those changes? Thanks in advance

type = type || Extrapolate.CLAMP;

const startColor = destructColor.apply({}, output[0]);
const endColor = destructColor.appy({}, output[1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, I guess expected word is apply

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 wasn't able to test this patch (I hoped react-native-reanimated have some kind of CI tests to check pull requests).
Also I wasn't able to build react-native-reanimated npm package to test my patch manually #1024 #950
But I was able test code in isolation manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like with appy misspell it doesn't work. If I would able to build react-native-reanimated npm package from master branch I'll fix misspell and add tests.

return undefined;
}

const r = interpolate.apply({}, x, input, [startColor[0], endColor[0]], type);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't those args be wrapped in an array like const r = interpolate.apply({}, [x, input, [startColor[0], endColor[0]], type]); ?

Copy link
Contributor

Choose a reason for hiding this comment

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

here and for the 3 following calls

Copy link
Member

Choose a reason for hiding this comment

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

@likern, any update on this?

@likern likern closed this Aug 21, 2020
@wcandillon
Copy link
Contributor

Interpolate color works well with redash worklets: https://github.com/wcandillon/react-native-redash/blob/master/src/Colors.ts#L201 feel free to copy this implementation if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants