Port to node-addon-api (and N-API) and remove NAN, libuv, and v8 #2235

chearon · 2023-04-23T19:02:29Z

I've finished the conversion to N-API after a month of working on this. This should help a lot with prebuilds (my main motivation tbh) enable compatibility with Bun and Electron, etc.

Because this uses APIs from N-API v6, I've bumped the minimum node version to v10.20.0.

Changes

Because node-addon-api uses C++ strings, I replaced some C strings with C++
CanvasRenderingContext2dInit and CanvasPatternInit (sends JS values to C++) are now simply bindings.setParseFont and bindings.setDOMMatrix

Drive-bys

I replaced maybe.Check() calls, which crash the whole process if it is empty, with UnwrapTo and UnwrapOr, so that in these rare cases node-canvas doesn't crash. In some instances, like when we're reading our own ImageData, I left the force-unwraps.

Performance

node-addon-api (C++ wrapper) and napi (ABI stable C library) incur two new layers of overhead when going between JS and C++. I've measured this to be an extra 0.2 µs per call function on my (2012) laptop. Users that call functions millions of times, for example, lots of lineTos, will see performance degradation after this PR. In the lineTo case, we may be able to make a faster Path2D API if we pack the paths into a byte array.
I included two perf commits to improve benchmarks a little bit

Future

node-addon-api has the ability to throw C++ exceptions when a JS exception happens. It is far easier to correctly handle errors with this enabled (for example, we wouldn't need the CHECK_RECEIVER code) and we should think about turning that on. For this PR, I considered that change out of scope, so the green code still uses Maybes.
We're closer to being able to close make module context-aware #1394. JS is all thread-safe now, but I don't know if all of the various C libraries we use are (and font_face_list is still global).

Fixes #923

gabrielschulhof · 2023-04-29T21:08:54Z

@chearon I was thinking about this a little. Might it make sense to create an ArrayBuffer on the JS side so that the parameters to all the moveto and lineto and other API calls do not result in any C++ calls at all, but get recorded in the ArrayBuffer instead? Then, when the ArrayBuffer gets full or the user issues a call to render, one call is made to C++ which iterates over the ArrayBuffer and executes all the moveto/lineto/etc. API calls stored therein.

The size of the ArrayBuffer can be tuned by the user as part of initialization. We can pick a sensible default if the user chooses not to provide a value.

Most importantly, this would not be visible to the users at all, because we'd start the execution of the batched API calls whenever the user issued a render.

It might affect debugging though, in which case we'd document that, for debugging, the ArrayBuffer size must be zero. or one, which would result in today's behaviour.

Julusian · 2023-04-29T23:23:50Z

src/JPEGStream.h


-  v8::Local<v8::Object> buf = Nan::NewBuffer((char *)dest->buffer, dest->bufsize).ToLocalChecked();
+  Napi::Object buf = Napi::Buffer<char>::New(env, (char *)dest->buffer, dest->bufsize);


Wrapping an existing pointer in a nodejs Buffer are not compatible with recent versions of electron.
See https://github.com/nodejs/node-addon-api/blob/main/doc/external_buffer.md
It looks like you might have some other occurrences of this too

Thanks for pointing that out. So I guess we can't claim Electron 21+ support yet. I'm going to consider that out of scope for this PR since we used external memory before these changes, but we should support Electron too without impacting Node performance. I wonder if Node will ever make the same change that Electron did too...

I'm so curious to see the use case for node-canvas in Electron. I thought you would just use <canvas>...

but we should support Electron too without impacting Node performance.

There is an alternate function Napi::Buffer<char>::NewOrCopy( designed for this case. Depending on whether you use dest again after this call, it may not require any more changes than that.

I'm so curious to see the use case for node-canvas in Electron. I thought you would just use ...

That would depend on whether you want to use it on the backend or frontend of the application, but I agree it does sound like an usual case.

I have a couple of applications which use electron for packaging and to get a tray icon, nothing more. They are also possible to run in plain nodejs. It might technically be possible to use a canvas element, but it wouldnt be nice.
I found a library for a usb device which uses canvas for generating the images to display. I think that one is quite likely to be in electron instead of nodejs

Actually there is a HUGE use case in electron. It is why myself and many have been praying for node-canvas adoption of N-API ;) as mentioned above a big use case is because in electron you split everything into renderer and main threads. In main, where you do all your node stuff, you cannot use <canvas> and hence the need for node-canvas. Passing stuff between main and renderer is a pain (via IPC) so it is very useful to have node-canvas in main thread.

Lastly, and most important for me, Mozilla pdf-js relies on node-canvas and I use it for many things in my electron app. Without node-canvas napi compatibility I am not able to use pdf-js in newer versions of electron.

Does that make sense? Any questions?

Yeah we are also relying on node-canvas to handle rendering the contents of the canvas on the renderer side as a PDF or SVG in the main process. Node-canvas support for rendering to PDF/SVG is a great plus in this case, and would not be possible to do in the rendered thread currently (I think).

Basically we use node-canvas to support exporting SVG and PDF versions of the canvas scene, which is a huge plus being able to export vector versions of whatever is drawn in the canvas!

Hoping that this would work with Electron versions 21+, but honestly if this already works with Electron version 20, that's a huge improvement already, so thank you for your work!

src/CanvasRenderingContext2d.cc

zbjornson · 2023-04-30T04:52:48Z

Might it make sense to create an ArrayBuffer on the JS side so that the parameters to all the moveto and lineto and other API calls do not result in any C++ calls at all, but get recorded in the ArrayBuffer instead? Then, when the ArrayBuffer gets full or the user issues a call to render, one call is made to C++ which iterates over the ArrayBuffer and executes all the moveto/lineto/etc. API calls stored therein.

I happened to have tried that a while ago (super sloppily, just for benchmarking) in f2e073c. That gave a 3.25x speedup in the NAN version and should give a larger speedup in the NAPI version because of the higher overhead. I'm not sure how it would actually work without explicit calls to render, since you can mix moveTo/lineTo, and as you said debugging would be weird.

However, even if it's not batched, copying the values to a TypedArray from JS helps: 2.3 Mops/sec -> 3.0 Mops/sec for lineTo().

crude diff

diff --git a/lib/context2d.js b/lib/context2d.js
index 103ec63..082df97 100644
--- a/lib/context2d.js
+++ b/lib/context2d.js
@@ -9,3 +9,12 @@
 const bindings = require('./bindings')
 
 module.exports = bindings.CanvasRenderingContext2d
+
+bindings.CanvasRenderingContext2d.prototype.lineTo = function(x, y) {
+	if (isFinite(x) && isFinite(y)) {
+		const ab = this._argsBuffer;
+		ab[0] = x;
+		ab[1] = y;
+		this._lineTo();
+	}
+}
diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc
index e1fb4b9..64c0c03 100644
--- a/src/CanvasRenderingContext2d.cc
+++ b/src/CanvasRenderingContext2d.cc
@@ -126,7 +126,7 @@ Context2d::Initialize(Napi::Env& env, Napi::Object& exports) {
     InstanceMethod<&Context2d::RoundRect>("roundRect"),
     InstanceMethod<&Context2d::MeasureText>("measureText"),
     InstanceMethod<&Context2d::MoveTo>("moveTo"),
-    InstanceMethod<&Context2d::LineTo>("lineTo"),
+    InstanceMethod<&Context2d::LineTo>("_lineTo"),
     InstanceMethod<&Context2d::BezierCurveTo>("bezierCurveTo"),
     InstanceMethod<&Context2d::QuadraticCurveTo>("quadraticCurveTo"),
     InstanceMethod<&Context2d::BeginPath>("beginPath"),
@@ -233,6 +233,10 @@ Context2d::Context2d(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Context2
   states.emplace();
   state = &states.top();
   pango_layout_set_font_description(_layout, state->fontDescription);
+
+  _argsArray = Napi::Float64Array::New(env, 9);
+  info.This().As<Napi::Object>().Set("_argsBuffer", _argsArray);
+  args = _argsArray.Data();
 }
 
 /*
@@ -2572,10 +2576,6 @@ Context2d::setTextPath(double x, double y) {
 
 void
 Context2d::LineTo(const Napi::CallbackInfo& info) {
-  double args[2];
-  if(!checkArgs(info, args, 2))
-    return;
-
   cairo_line_to(context(), args[0], args[1]);
 }
 
diff --git a/src/CanvasRenderingContext2d.h b/src/CanvasRenderingContext2d.h
index 745106e..385e06d 100644
--- a/src/CanvasRenderingContext2d.h
+++ b/src/CanvasRenderingContext2d.h
@@ -220,4 +220,6 @@ class Context2d : public Napi::ObjectWrap<Context2d> {
     cairo_t *_context = nullptr;
     cairo_path_t *_path;
     PangoLayout *_layout = nullptr;
+    Napi::Float64Array _argsArray;
+    double* args = nullptr;
 };

Tedious to do for everything, and we'd want to hide stuff better.

gabrielschulhof · 2023-04-30T22:54:37Z

@zbjornson awesome result! Sounds like it might be worth the effort, if it improves perf beyond just cancelling out the overhead incurred from switching to Node-API.

chearon · 2023-05-02T03:08:04Z

Buffering all of the path calls is super interesting but I do agree that the flushing of the path is the downside. It has to happen in at least clip(), fill(), stroke(), beginPath() and endPath(). Path2D would be much more elegant to implement since you make all the Cairo calls right in ctx.fill(path), but also looks harder, and still leaves ctx.lineTo slow.

However, even if it's not batched, copying the values to a TypedArray from JS helps: 2.3 Mops/sec -> 3.0 Mops/sec

I am kind of bummed about this being less pretty but that's a good speedup and it's easy.

I wish node-addon-api could be optimized more (that's got to be Napi::CallbackInfo and Napi::Number construction) but I don't see much opportunity. Unless we could modify InstanceMethod<&Context2d::LineTo> to take a LineTo function that's just a low-level napi_callback. That would be nice...

gabrielschulhof · 2023-05-14T03:09:05Z

I am kind of bummed about this being less pretty but that's a good speedup and it's easy.

I agree wholeheartedly. It may not be pretty, but node-canvas is definitely a performance-sensitive add-on.

I wish node-addon-api could be optimized more (that's got to be Napi::CallbackInfo and Napi::Number construction) but I don't see much opportunity. Unless we could modify InstanceMethod<&Context2d::LineTo> to take a LineTo function that's just a low-level napi_callback. That would be nice...

I'll take a look at node-addon-api to see if we can provide variants which accept a napi_callback, thus avoiding the extra construction and additional level of indirection. We cannot avoid the unwrap though, because we need to retrieve the pointer to the native instance. We did, after all, design node-addon-api to mix and match with plain Node-API.

zbjornson

Got through half of the files so far. Looks great, NAPI is so much nicer to work with.

src/Canvas.cc

zbjornson · 2023-05-14T20:35:49Z

src/Canvas.cc

-    , buf
-    , Nan::New<Number>(len) };
-  async.runInAsyncScope(Nan::GetCurrentContext()->Global(), streaminfo->fn, sizeof argv / sizeof *argv, argv);
+  Napi::Object buf = Napi::Buffer<uint8_t>::New(env, (uint8_t *)(data), len);


Here and everywhere there's Napi::Object xx = Napi::Buffer..., including the return type of Canvas::ToBuffer, should be

Suggested change

Napi::Object buf = Napi::Buffer<uint8_t>::New(env, (uint8_t *)(data), len);

auto buf = Napi::Buffer<uint8_t>::New(env, (uint8_t *)(data), len);

or Napi::Buffer<uint8_t> to avoid casting to the superclass.

(Avoids an MSVC warning https://learn.microsoft.com/en-us/cpp/code-quality/C26437.)

Napi::Value should work too if I understand correctly.

I don't know our stance on auto but I prefer seeing the types most of the time. Maybe I should set up type checking in my editor.

Napi::Value is still an ancestor class. Only auto or Napi::Buffer<uint8_t> avoid the implicit cast.

The link above is about slicing, which is slightly different than an implicit cast, but I see what you're saying. But wouldn't there be a slice happening every time you return a more derived type in a node-addon-api callback, which must have a return type of Napi::Value? It is designed to be used this way, so I'm not sure if it's really an issue. I don't see that MSVC warning on CI either.

Implicit cast to a super class, right. It's not actually an issue because the only members in the derived class are retrieved from V8 when needed. Fine to ignore.

src/Canvas.cc

src/CanvasPattern.cc

zbjornson · 2023-05-14T20:54:09Z

src/CanvasPattern.cc

-NAN_METHOD(Pattern::New) {
-  if (!info.IsConstructCall()) {
-    return Nan::ThrowTypeError("Class constructors cannot be invoked without 'new'");
+Pattern::Pattern(const Napi::CallbackInfo& info) : ObjectWrap<Pattern>(info), env(info.Env()) {


A crash happens with new canvas.CanvasPattern() or ctx.createPattern().

I'm not able to reproduce any crash. I get Uncaught TypeError: Image or Canvas expected for both of those, which is correct. I also tried passing a real image, and verified that is painting correctly.

> canvas = createCanvas(1,1) [Canvas 1x1] > canvas.getContext("2d").createPattern() # Exception thrown at 0x0000000068DF09A6 (libcairo-2.dll) in node.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF

Turns out a crash happens with the same code in master also, but with a different cause. Can deal with later.

src/CanvasPattern.cc

gabrielschulhof · 2023-05-20T02:04:33Z

@chearon

I'll take a look at node-addon-api to see if we can provide variants which accept a napi_callback, thus avoiding the extra construction and additional level of indirection. We cannot avoid the unwrap though, because we need to retrieve the pointer to the native instance. We did, after all, design node-addon-api to mix and match with plain Node-API.

I actually don't think we'd gain too much from being able to provide a napi_callback to an InstanceMethod, because napi_get_cb_info() needs to get called no matter what, because we need to retrieve the instance whose method is getting called. The call to napi_get_cb_info() is the heavy part. Now, FWIW, if any of the methods take more than 6 arguments, then there's likely need for optimization, because Napi::CallbackInfo has room on the stack for 6 arguments, but if the function takes more than that, it calls napi_get_cb_info() twice, after it allocates the required room for args:

https://github.com/nodejs/node-addon-api/blob/358b2d3b4f4cf0b3bdfc9897be888494ee36ae1d/napi-inl.h#L3745-L3766

If needed, we may want to expose the number of statically allocated arguments as a preprocessor directive:

https://github.com/nodejs/node-addon-api/blob/358b2d3b4f4cf0b3bdfc9897be888494ee36ae1d/napi.h#L1875

gabrielschulhof · 2023-05-21T00:08:11Z

@chearon actually, it looks like napi_unwrap takes the cake: http://gabrielschulhof.github.io/misc/templated.instance.method.perf.breakdown.svg. That is not something easily changed, unfortunately.

chearon · 2023-06-24T16:11:51Z

@gabrielschulhof thank you for looking into it. I was suggesting a direct call to an napi_function because it looked like the invocation of Napi::details::TemplatedInstanceVoidCallback and family was causing significant extra time. It doesn't look that way in your flame chart. ellipse takes more than 6 arguments and would be the only one I'm concerned about.

I think we should merge this and do Path2D after we're sure performance is a problem.

@zbjornson huge thanks for the detailed look. I think I got everything.

The CHECK_RECEIVER calls are all gone thanks to node-addon-api v7. That version claims to bump the minimum node version to 16, but I think that is wrong, at least for our purposes. There are no commits that look like they are adding new ABI calls that would break it. All of the tests pass with node 10.

zbjornson · 2023-06-27T15:42:12Z

Cool, I'll try to get through the remaining review this weekend.

Sakari369 · 2023-07-19T05:48:25Z

Hey, just wondering, any update on this getting merged ? :)
Or can I already use this branch, should work without the final changes ?

GitMurf · 2023-08-15T01:32:20Z

Any update on plans for when this will be "officially" released? Thanks much for the hard work!

chearon · 2023-08-15T15:39:34Z

I'm satisfied with it as-is, but would rather @zbjornson or @LinusU give an approval before merging. And anyone else that wants to review/test would be helping a lot!

@Sakari369 you can use this branch temporarily, it passes all of our tests.

Sakari369 · 2023-08-16T08:40:03Z

I'm satisfied with it as-is, but would rather @zbjornson or @LinusU give an approval before merging. And anyone else that wants to review/test would be helping a lot!

@Sakari369 you can use this branch temporarily, it passes all of our tests.

Thank you for the info, will test it out! 🙏

zbjornson

Great, just a few minor comments.

src/Image.cc

zbjornson · 2023-08-21T03:55:26Z

src/Canvas.cc

+    if (!env.IsExceptionPending()) {
+      Napi::Error::New(env, GENERIC_FACE_ERROR).ThrowAsJavaScriptException();


NAPI checks if an exception is pending and ignores subsequent calls to ThrowAsJavaScriptException(). You could delete all of these checks.

NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS is not enabled currently, so these are needed. I'll look into adding it though, I didn't know about that.

Good eyes. Fine by me if you want to keep the checks since the fatal exception is meant to reveal bugs.

src/Canvas.cc

zbjornson · 2023-08-21T21:03:48Z

Also - this fixes a few WPT tests (yarn test-wpt) but breaks several others. The ones broken in this branch are:

  WPT: path-objects
    42) 2d.path.arc.shape.3
    43) 2d.path.arc.selfintersect.1
    45) 2d.path.arc.zeroradius
    46) 2d.path.rect.selfintersect
    47) 2d.path.roundrect.zero.6
    48) 2d.path.roundrect.selfintersect

  WPT: shadows
    78) 2d.shadow.attributes.shadowBlur.invalid # this one looks like an int/uint issue

chearon · 2023-09-11T16:31:20Z

I do get a different number of failures (123 on master, 121 here), but the ones you listed are all failing for me on master too. Are you getting a difference in kinds of errors? I don't know much about those tests, but I'll fix anything they uncover.

zbjornson

Added a comment with the fix for the one new failing WPT test.

The others went away after I pulled and rebuilt. The value of Math.PI/2 passed from JS was somehow very slightly different in the NAN and N-API versions. I went back to the earlier commits in this PR and couldn't reproduce it. 😞

  42) WPT: path-objects
       2d.path.arc.shape.3:

      AssertionError [ERR_ASSERTION]: Red channel of the pixel at (1, 48)
      + expected - actual

      -242
      +0
      
      at _assertPixel (test\wpt\generated\path-objects.js:57:9)
      at Context.<anonymous> (test\wpt\generated\path-objects.js:1439:3)
      at processImmediate (node:internal/timers:466:21)

This branch before pull/rebuild:

master:

-1.5707963267948966  master
... something I didn't write down that ended in 7

With the fix below, 3 new WPT tests are passing in this branch. Nice work.

src/CanvasRenderingContext2d.cc

this makes the lineTo benchmark (lineTo executes a very small number of operations, so it mostly measures the js<->C++ barrier) run about 50% faster

abrenoch · 2023-10-12T13:34:09Z

Hey fantastic work on this PR, really glad to see this working with electron again.

In testing things out with pdfjs, I discovered the save property of CanvasRenderingContext2d is apparently read-only in this new build. pdfjs appears to have a function that temporarily reassigns several of the built-in methods. I have more info in the issue I opened on the pdf-to-img repo here.

I wanted to bring it up here for further consideration.

Thanks!

chearon · 2023-10-12T13:38:43Z

Oh, it should be assignable since it's assignable in the browser. If you/someone wants to make a PR I can review it, or I can get around to it sometime soon.

abrenoch · 2023-10-12T13:44:12Z

Gotcha @chearon, good to hear! I would love to help but I have just about no experience with with C++ - unfortunately the best I can do at the moment is complain 😄

Thanks again!

GitMurf · 2023-10-12T17:10:54Z

@abrenoch good catch! Given my main use of node-canvas is for usage with pdfjs, this is key for me as well :) thanks!

GitMurf · 2023-10-12T17:12:41Z

@chearon as an FYI, I do not know if this is expected or not, but the maintainer of pdf-to-img mentioned here k-yle/pdf-to-img#228 (comment) that they were not able to compile v3 pre-release and provided the error they received. Probably worth taking a look just to make sure it isn't something needed to be fixed before v3 release.

For quick viewing of the error here, I took a screenshot as well (link to comment above):

nrathi · 2023-10-19T17:39:04Z

Hi, thanks for all the hard work on this ticket! I'm very excited about v3, would anyone be able to provide an estimated timeline? Thanks in advance!

zbjornson · 2023-12-29T01:36:04Z

Update on prebuilds:

node-pre-gyp requires {napi_build_version} to be in the module_path, which means that canvas.node needs to live in something like build/Release/napi-7/canvas.node. That's annoying: gyp doesn't let you specify the output directory, so you need to use a post-install hook to copy the file there, and a bunch of lines will need updating when we change NAPI versions.

A DIY-approach is doable, but there are a handful of details to get right (e.g. proxy support). It would be nice to get rid of node-pre-gyp since we only use like 10% of its functionality (node-pre-gyp still needed? #1891).

rough script

const build_from_source = process.argv.includes("--build-from-source")
    && !process.argv.includes("--build-from-source=false");

function getBinStr() {
    const module_name = require("./package.json").name;
    const version = require("./package.json").version;
    const napi_build_version = "7";
    const platform = process.platform; // TODO support --target_platform
    const arch = process.arch; // TODO support --target_arch
    const libc = require("detect-libc").family;
    return `${module_name}-v${version}-napi-v${napi_build_version}-${platform}-${libc}-${arch}.tar.gz`;
}

if (build_from_source) {
    throw new Error("--build-from-source specified, skipping prebuild download.");
} else {
    // TODO Need to support proxies
    // TODO Need to support --canvas_binary_host_mirror
    const get = require("simple-get");
    const base_url = "https://github.com/Automattic/node-canvas/releases/download";
    const bin_str = getBinStr();
    console.log(`Downloading prebuild for ${bin_str}`);
    get.concat(`${base_url}/${bin_str}`, (err, res, data) => {
        if (err) {
            throw err;
        }
        if (res.statusCode !== 200) {
            console.error(`Download failed: ${res.statusCode}`);
            throw err;
        }
        // TODO untar and save data to build/Release/canvas.node
        return;
    });
}

Other modules seem to be moving away from node-pre-gyp in favor of prebuild and prebuild-install (e.g. sqlite3). It's a lot smaller than node-pre-gyp, if nothing else. prebuild-install has a fixed package name string format that looks like canvas-v{version}-napi-v{napi_version}-{platform}{libc}-{arch}.tar.gz, where {libc} is omitted if it's glibc.
The size of all three platforms' bundles is 73.5 MB, and will grow if/when we support ARM and musl. That's probably(?) too big to put in the npm tarball. librsvg alone is too big for some PaaSes like Netlify (Node-canvas dependency packages are too large to build project on Netlify #1954).

The tip commits on the master and prebuilds branches switch to prebuild-install, but the build failed for the rsvg reason above: https://github.com/Automattic/node-canvas/actions/runs/7352791664/job/20017985691 -- so it remains untested until that's fixed. If we don't want to switch to prebuild-install, feel free to force-push over my commits.

I might have time to keep working on this tomorrow. Input welcome on approaches.

chearon · 2023-12-29T17:16:56Z

Supporting librsvg alone is as much of a pain as building to every version of node was before N-API. It has so many Dockerfile dependencies. Would removing it from prebuilds allow us to publish the binaries in the NPM tarball? I'm sure there will be pitchforks, but people could still manually compile, or we could remove it in favor of a JS/WASM dependency, implement Path2D, etc.

zbjornson · 2023-12-29T21:55:20Z

Would removing it from prebuilds allow us to publish the binaries in the NPM tarball?

That shrinks it from 73.5 MB to 40-50 MB. The AWS Lambda ZIP upload size limit is 50 MB; Netlify I think is the same because it uploads to Lambda; Netlify Edge is 20 MB; Good Cloud Functions 2nd gen has no limit. So, maybe/it depends.

chearon · 2024-01-02T16:09:22Z

If you look at the "Make Bundle" section of the prebuild action, it looks like we are distributing .o files that total ~7MiB per platform as well. When I delete that and librsvg-2.2.dylib (not including any unreachable dependencies after that) the gzipped bundle on macOS is 4.9 MiB (down from 10.4 MiB).

If Linux and Windows get down to a number like that, I think it would be worth distributing in the NPM tarball and either remove SVG from prebuilds or start shipping it as a JS or WASM dependency.

(I would rather remove it from prebuilds than distribute it any way, which is kind of insane: that's an entire layout engine with a CSS engine, XML parser, etc. I think there are good technical reasons not to do so, but I don't expect users to care when they see SVG support "removed").

kotasudhakar · 2024-03-20T04:14:42Z

Please when can we expect this to be released?
This is a real problem as most of the libraries today uses the old Nan api(pdfjs, react-pdf, etc.,) and have node canvas as a dependency, which makes any electron app having canvas as dependency not work at all!

zbjornson · 2024-04-14T06:39:45Z

Had some time to work on this finally, but eesh are prebuilds miserable to maintain. Lots of new build breaks:

One of the macOS casks changed dependencies and created a conflict with another cask.
Maybe related, pip3 install --user macpack stopped working
A recent node-canvas PR added use of a C++20 feature (fixed)
Something changed with libseccomp-dev on Linux. pax-utils is failing to build and I don't know why. We're using the same git SHA of pax-utils that we've used before, and we're in a Docker container that I thought was supposed to provide a (more) fixed environment.

I only got one of those fixed, which at least made the Windows prebuild work. Any help on the other issues would be welcome.

kotasudhakar · 2024-04-14T12:10:31Z

@zbjornson Can we make a release until this, for now... And whenever we get to fix the issue for linux and Mac, you can make one more release

zbjornson · 2024-04-15T01:29:20Z

@kotasudhakar no, sorry. I haven't gotten back to the actual prebuild distribution part yet (the switch away from node-pre-gyp).

GitMurf · 2024-06-17T13:33:28Z

Curious where things stand today? We are looking at implementing Mozilla pdfjs but it relies on node-canvas and therefore we are not able to use with electron until the napi version is finished. Thanks for all the hard work!

chearon · 2024-06-17T16:11:03Z

I hope to pick up where @zbjornson left off on prebuilds, but it's the last thing I want to do in my free time. We could release under a beta tag without prebuilds for electron/other users in the meantime.

GitMurf · 2024-06-17T19:13:20Z

I hope to pick up where @zbjornson left off on prebuilds, but it's the last thing I want to do in my free time. We could release under a beta tag without prebuilds for electron/other users in the meantime.

@chearon I hear ya! I wish I could help but I just have no experience in this domain.

Doesn't electron require the prebuilds though? Or is that essentially what electron-rebuild does where it runs that instead of using a prebuild? Sorry for my lack of knowledge in this area.

zbjornson · 2024-06-19T21:09:04Z

Progress today:

Linux: Avoided the pax-utils issue by installing the built package instead of building it from source.
Linux: Update the librsvg version from 2.50.2 to 2.52.8. src: refactor and apply fixes #2229 switched to an API that was introduced in 2.52.

Linux and Windows now build, but we still have the installation part to do. I might work on that later today. Also want to look into making prebuilds from the 2.x NAN branch for newer Node.js versions.

I am completely new to homebrew, so hoping someone familiar with it can work on the Mac build. I've been fumbling around with the tip commits in https://github.com/Automattic/node-canvas/commits/prebuilds/.

zbjornson · 2024-06-20T00:31:15Z

Alright, v3.0.0-rc2 is available for testing, with prebuilds available on Linux x64 and Windows x64.

https://github.com/Automattic/node-canvas/releases/tag/v3.0.0-rc2

zbjornson · 2024-06-23T04:50:26Z

macOS x64 prebuilds are up and ready for testing. I didn't realize that macos-latest now means M1.

npm install canvas@next

chearon mentioned this pull request Apr 26, 2023

src: Avoid calling into C++ with a null this nodejs/node-addon-api#1313

Closed

Julusian reviewed Apr 29, 2023

View reviewed changes

zbjornson reviewed Apr 30, 2023

View reviewed changes

src/CanvasRenderingContext2d.cc Outdated Show resolved Hide resolved

zbjornson requested changes May 14, 2023

View reviewed changes

zbjornson added the semver-major label May 14, 2023

chearon force-pushed the napi branch from 8ffb6c4 to 83d8af0 Compare June 24, 2023 15:54

zbjornson requested changes Aug 21, 2023

View reviewed changes

7f8ddd mentioned this pull request Sep 11, 2023

Unable to find module canvas oven-sh/bun#4259

Closed

chearon force-pushed the napi branch from 83d8af0 to f070e01 Compare September 11, 2023 16:32

zbjornson reviewed Sep 12, 2023

View reviewed changes

src/CanvasRenderingContext2d.cc Outdated Show resolved Hide resolved

src/CanvasRenderingContext2d.cc Outdated Show resolved Hide resolved

chearon added 2 commits September 19, 2023 12:37

port to node-addon-api (remove NAN, v8, libuv)

c32c39b

optimize checkArgs

cc840a8

this makes the lineTo benchmark (lineTo executes a very small number of operations, so it mostly measures the js<->C++ barrier) run about 50% faster

abrenoch mentioned this pull request Oct 12, 2023

Notes on supporting node-canvas^3.0.0 k-yle/pdf-to-img#228

Open

joshkel mentioned this pull request Nov 10, 2023

Error: Module did not self-register #1536

Closed

Balearica mentioned this pull request Nov 26, 2023

make module context-aware #1394

Open

chearon mentioned this pull request Nov 27, 2023

🚢 3.0.0 planning issue #2232

Open

8 tasks

akx mentioned this pull request Dec 19, 2023

Dev deps: Resolve to as-of-yet-unreleased node-canvas version jupyterlab/jupyterlab#15541

Closed

zbjornson mentioned this pull request Feb 29, 2024

Add prebuilds for ARM64 Macs #2354

Open

nicolo-ribaudo mentioned this pull request Apr 26, 2024

npm intall not working on node v22 #2377

Closed

1 task

a2311kk mentioned this pull request Jul 15, 2024

React-PDF 導入後、Windows で npm start (electron-forge start) に失敗する ChubachiPT2024/manakan#152

Closed

Jarred-Sumner mentioned this pull request Jul 15, 2024

Support node-canvas oven-sh/bun#5835

Open

filfreire mentioned this pull request Aug 29, 2024

Chore/napi Kong/node-libcurl#50

Open

4 tasks


		v8::Local<v8::Object> buf = Nan::NewBuffer((char *)dest->buffer, dest->bufsize).ToLocalChecked();
		Napi::Object buf = Napi::Buffer<char>::New(env, (char *)dest->buffer, dest->bufsize);

	Napi::Object buf = Napi::Buffer<uint8_t>::New(env, (uint8_t *)(data), len);
	auto buf = Napi::Buffer<uint8_t>::New(env, (uint8_t *)(data), len);

		if (!env.IsExceptionPending()) {
		Napi::Error::New(env, GENERIC_FACE_ERROR).ThrowAsJavaScriptException();

Port to node-addon-api (and N-API) and remove NAN, libuv, and v8 #2235

Port to node-addon-api (and N-API) and remove NAN, libuv, and v8 #2235

Conversation

chearon commented Apr 23, 2023

Changes

Drive-bys

Performance

Future

gabrielschulhof commented Apr 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GitMurf May 6, 2023 • edited Loading

Choose a reason for hiding this comment

Sakari369 May 12, 2023 • edited Loading

Choose a reason for hiding this comment

zbjornson commented Apr 30, 2023

gabrielschulhof commented Apr 30, 2023 • edited Loading

chearon commented May 2, 2023

gabrielschulhof commented May 14, 2023 • edited Loading

zbjornson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zbjornson Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrielschulhof commented May 20, 2023 • edited Loading

gabrielschulhof commented May 21, 2023

chearon commented Jun 24, 2023

zbjornson commented Jun 27, 2023

Sakari369 commented Jul 19, 2023

GitMurf commented Aug 15, 2023

chearon commented Aug 15, 2023

Sakari369 commented Aug 16, 2023

zbjornson left a comment

Choose a reason for hiding this comment

zbjornson Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zbjornson commented Aug 21, 2023

chearon commented Sep 11, 2023

zbjornson left a comment

Choose a reason for hiding this comment

abrenoch commented Oct 12, 2023

chearon commented Oct 12, 2023

abrenoch commented Oct 12, 2023

GitMurf commented Oct 12, 2023

GitMurf commented Oct 12, 2023 • edited Loading

nrathi commented Oct 19, 2023

zbjornson commented Dec 29, 2023 • edited Loading

chearon commented Dec 29, 2023

zbjornson commented Dec 29, 2023

chearon commented Jan 2, 2024

kotasudhakar commented Mar 20, 2024 • edited Loading

zbjornson commented Apr 14, 2024

kotasudhakar commented Apr 14, 2024 • edited Loading

zbjornson commented Apr 15, 2024

GitMurf commented Jun 17, 2024

chearon commented Jun 17, 2024

GitMurf commented Jun 17, 2024

zbjornson commented Jun 19, 2024

zbjornson commented Jun 20, 2024

zbjornson commented Jun 23, 2024

GitMurf May 6, 2023 •

edited

Loading

Sakari369 May 12, 2023 •

edited

Loading

gabrielschulhof commented Apr 30, 2023 •

edited

Loading

gabrielschulhof commented May 14, 2023 •

edited

Loading

zbjornson Aug 21, 2023 •

edited

Loading

gabrielschulhof commented May 20, 2023 •

edited

Loading

zbjornson Aug 21, 2023 •

edited

Loading

GitMurf commented Oct 12, 2023 •

edited

Loading

zbjornson commented Dec 29, 2023 •

edited

Loading

kotasudhakar commented Mar 20, 2024 •

edited

Loading

kotasudhakar commented Apr 14, 2024 •

edited

Loading