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

feat: complete image filter api #2824

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

fdecampredon
Copy link
Contributor

This PR expose the complete set of ImageFilter available in Skia to the JS API

@fdecampredon fdecampredon force-pushed the feat-complete-image-filter branch from f78a188 to 00fe164 Compare December 19, 2024 13:57
@wcandillon wcandillon self-requested a review December 29, 2024 21:06
*/
MakeBlend: (
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to clean up the diff here?

* @param dy - Offset along the X axis
* @param input - if null, it will use the dynamic source image
*/
MakeOffset(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to clean the diff on the this file?

throw new NotImplementedOnRNWeb();
}

MakeDisplacementMap(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the diff is a bit dirty here.

return new JsiSkImageFilter(
this.CanvasKit,
this.CanvasKit.ImageFilter.MakeColorFilter(
JsiSkColorFilter.fromValue(cf),
input === null ? null : JsiSkImageFilter.fromValue(input)
input == null ? null : JsiSkImageFilter.fromValue(input)
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do?

Copy link
Contributor

@wcandillon wcandillon left a comment

Choose a reason for hiding this comment

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

Thank you for this, I'm very eager to merge t this. There is mainly a problem with the diff that prevents me to review this nicely but all and all it looks good.

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