companion icon indicating copy to clipboard operation
companion copied to clipboard

fix: change canvas lib

Open Julusian opened this issue 4 months ago • 3 comments

~~I feel like this should go to 3.3, but it might be needed to fix some bugs in 3.2~~ This is now planned to land in 3.4 immediately after the release of 3.3

The skia-canvas lib we are using is problematic. There was a bug where loading the emoji font on windows could consume all available memory and crash #2714 , and another bug where it (also on windows) never returns when rendering certain unicode characters.

Also while at one point the library looked promising, it now looks to be abandoned. While there might be some viable forks of that library, this proposed change is definitely maintained better. (Long term, we may want to consider canvas, but that currently wont work due to not being node-api based yet).

This needs some more testing, to verify it doesn't have the same flaws on windows. And some testing to verify the drawing in more scenarios. My quick testing shows it to be something like a pixel off sometimes (pixel rounding?), but is otherwise identical to 3.2.2

Julusian avatar Mar 04 '24 22:03 Julusian

When I started the canvas stuff, I actually tried first with napi-rs/canvas, but unfortunately I couldn't get it to work by then. Also skia-canvas has some nice features on top of skia which napi-rs/canvas is missing. Mainly I was interested in the built in word-wrap/multiline support. At the end I found that the word wrap didn't had the flexibility we need with some custom valid line breaking characters and I stayed with own solution. So if now skia-canvas is loosing maintanance and napi-rs/canvas does work, I'm fine with the change.

Regarding the pixel preciceness, with skia-canvas I had often to position lines at .5 positions to fit exactly one pixel. This is also the correct way like it is in skia.

I have now a Windows machine at hand and can give the PR a try in the next few days.

dnmeid avatar Mar 06 '24 00:03 dnmeid

So far I haven't done any refinement other than fixing one very noticable issue with line heights, which was a simple swapping of some property names. My main interest is whether it has the same 'fatal' bugs as the previous lib did, I will need to ask someone who had those issues previously to test and verify.

And I haven't confirmed that this works in the built files that we distribute yet, but I don't expect that to be impossible, just needs some config tweaking to get setup

Julusian avatar Mar 06 '24 08:03 Julusian

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 10 '24 20:04 CLAassistant