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.

Sign up for GitHub

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

Jump to bottom

N-API port #946

Merged
merged 110 commits into from
Aug 7, 2020
Merged

N-API port #946

merged 110 commits into from
Aug 7, 2020

Conversation

artemp
Copy link
Member

@artemp artemp commented Apr 24, 2020 •
edited
Loading

@artemp artemp marked this pull request as draft April 24, 2020 14:49
@artemp artemp mentioned this pull request Apr 27, 2020
Closed
@Algunenano
Copy link
Contributor

Algunenano commented Apr 29, 2020 •
edited
Loading

Hi @artemp, are you doing this by hand or are you using some kind of tool?

@artemp
Copy link
Member Author

artemp commented Apr 29, 2020

Hi @artemp, are you doing this by hand or are you using some kind of tool?

Both but mostly manually.
First I ran

./node_modules/node-addon-api/tools/conversion.js src/

but the tool is not very clever and here are lots of inconsistent usage (pre NAN etc) added over time so I'm manually reviewing and fixing things.
Your feedback is appreciated!

/cc @Algunenano

@Algunenano
Copy link
Contributor

Makes sense. For now I'm only done an overview of the changes, trying to familiarize myself with n-api more than reviewing the changes themselves.

One question that I have, but that is outside of the scope of the PR, is what changes are needed in the deployment of pre-compiled binaries since now I'd expect a single .node per platform. But that comes after making things work with n-api.

@artemp
Copy link
Member Author

artemp commented Apr 29, 2020

single .node per platform. But that comes after making things work with n-api.

That's the goal ^ 👍

@artemp
N-API Image.fromBufferSync (WIP) [skip ci]
a5a9e1a
@artemp
N-API Image.isSolid Image.isSolidSync (WIP) [skip ci]
32e4b1d
@artemp
N-API Remove redundant IsNull() && IsUndefined() checks (WIP) [skip ci]
2a3ca71
@artemp
N-API - return filled image in Image.fill callback (WIP) [skip ci]
d27f8cc
@artemp
N-API return env.Undefined() after throwing JS exception (WIP) [ski…
1c97d78 
…p ci]
@artemp
N-API Image.data() (TODO: consider zero-copy interface /cc @springmeyer…
d413b31 
…) (WIP) [skip ci]
@artemp
N-API Image.painted, Image.premultiplySync, Image.premultiply, Image.…
e40ac46 
…demultiplySync, Image.demultiply, Image.resize, Image.resizeSync, Image.copy, Image.copySync, Image.clear, Image.clearSync (WIP) [skip ci]
@artemp
N-API buffer() method which returned zero-copyed JS buffer, caller mu…
e99ca12 
…st ensure Image object stays alive while buffer is in use

/cc @springmeyer
@artemp
N-API Image.filterSync, Image.filter, Image.composite (WIP) [skip ci]
244f623
@artemp
Pin node-addon-api to ~3.0.0
a2d5fa8
@artemp artemp changed the title NAN => N-API initial port (WIP) [skip ci] N-API port Aug 4, 2020
@artemp artemp merged commit 07a48ec into master Aug 7, 2020
@springmeyer springmeyer deleted the n-api branch August 7, 2020 21:03
This was referenced Aug 8, 2020
Closed
Closed
@springmeyer springmeyer mentioned this pull request Oct 22, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@springmeyer springmeyer springmeyer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@artemp @Algunenano @springmeyer