fix: change canvas lib - Githubissues

bitfocus / companion

Bitfocus Companion enables the reasonably priced Elgato Streamdeck and other controllers to be a professional shotbox surface for an increasing amount of different presentation switchers, video playback software and broadcast equipment.

http://bitfocus.io/companion

Other

1.47k stars 489 forks source link

fix: change canvas lib #2785

Closed Julusian closed 1 month ago

Julusian commented 4 months ago

~~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

dnmeid commented 3 months ago

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.

Julusian commented 3 months ago

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

CLAassistant commented 2 months ago

All committers have signed the CLA.