[BUG] Rendering specific text crashes Companion

[semenovnick]

Is this a bug in companion itself or a module?

• I believe this to be a bug in companion

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

When i insert large text (like responce of HTTP Get Request from generic http module) or just in custom variable editor. If i use this variable in in text for knob. Or some variable (for example vmix title text) have large text in it. Or I just insert large text in button text field, companion crashes with stack trace:

PID 44796 received ACCESS_VIOLATION for address: 0x7ffead3ecf50
[node-napi-v9.node]: segfault::init
[node-napi-v9.node]: segfault::showCallstack
[node-napi-v9.node]: segfault::handleSignal
[KERNELBASE.dll]: UnhandledExceptionFilter
[ntdll.dll]: memset
[ntdll.dll]: _C_specific_handler
[ntdll.dll]: _chkstk
[ntdll.dll]: RtlRaiseException
[ntdll.dll]: KiUserExceptionDispatcher
[skia.win32-x64-msvc.node]: napi_register_module_v1
[skia.win32-x64-msvc.node]: napi_register_module_v1
[skia.win32-x64-msvc.node]: napi_register_module_v1
[skia.win32-x64-msvc.node]: napi_register_module_v1
[skia.win32-x64-msvc.node]
[node.exe]: v8::base::CPU::has_popcnt

Seems like error in canvas submodule...

Steps To Reproduce

paste large text in Button text field

Expected Behavior

Companion not crushes

Environment (please complete the following information)

- OS: Win 10
- Browser: Any
- Companion Version: from 4.3.3 up to 3.5.2

Additional context

No response

[thedist]

I wonder if this may be the cause of the issue in #3282

[dnmeid]

I have just tried to reproduce this problem but I could not generate a crash. I tried rendering text from a custom variable that I doubled repeatedly. I stopped at 100 something million characters, because it started getting slow but no crash.
I tried fixed font size of 7pt and Auto.
Only difference is, I have Windows 11.

What do you mean with "large"? GB?

[semenovnick]

I checked today. You right, It happens not every time. But if you add generic http get module. Add JSON Response Data Variable to custom variable. Add this variable as text for any button and as test send request to:
https://www.lipsum.com/feed/html?amount=5&&what=words which responds Lorem Ipsum html. Crash happens in 10/10 attempts.
Also if variable is not on the button text, module saves responce silently, crash happens when after adding this variabe to button text (it shows that it problem in companion not in http module).
Maybe it is because of hidden symbols or somethig\r\n. Because if after storing in variable after http get I make cut and paste back in that variable and then add this variable to button everything OK
Video showing that:

2025-02-14.15-20-11.mp4

[dnmeid]

OK, so far I can say that there are no fancy characters in the response that coud cause the crash.
I can reproduce it with sizes over 1050 characters but it seems that it needs something to be special within that text to crash.

[semenovnick]

Just tried on fresh companion install on win 11... Same crash when storing html in a variable.

[Julusian]

I'm also noticing how slow it is to draw buttons with this much text on them, cpu looks to be maxing out a core for multiple seconds, which looks to be our algorithm for splitting the text into lines. I'm going to see what I can do on this

One potential solution I am wondering about is adding some arbitrary limits to how much text we try to draw.
something like limiting it to 1000 characters overall, and 100 per line. That will allow us to take various shortcuts and make drawing quicker.

That doesnt necessarily address the underlying issue though, just making it less likely to occur

[dnmeid]

One potential solution I am wondering about is adding some arbitrary limits to how much text we try to draw.

I thought, I'd already done that when speeding up the linebreaking algorithm for the Auto size. Maybe it got lost.

[Julusian]

I thought, I'd already done that when speeding up the linebreaking algorithm for the Auto size. Maybe it got lost.

It was not drawing any lines that wouldn't be visible, but it is still chunking up the text that will never be visible.
I have a fix for this ready.

I'm also seeing each call to findLastChar take 2ms+, presumably as it tries to draw 10000 chars on one line, and then tries again with various shorter lengths.
I was going to do the limiting to 100 chars, but found some cases where we aren't handling multi-codepoint unicode currently..

[Julusian]

@dnmeid feel free to look over my changes 7ed5448

I am wondering about refactoring this logic into its own class and writing some unit tests for it. probably not checking the visual output, just the chunking and numbers

[dnmeid]

@Julusian with your fixes I could not reproduce the error any more, even not when I take out the slice and feed the whole text.
So in terms of this issue, I consider it a fix.

But while looking at the code I saw that it has quite evolved from what I wrote back in the days and there are now some new errors introduced.

1. Line break does seem to not work as it should. I've seen breaks within words where a break at a word boundary would have been possible.
2. Calculation of the overall height or line height seems to be off. I can generate lines that are partially cut off.
3. A gui realated problem: the size dropdown works different in main branch. It does'n show the options, only the currently selected value. The options are only shown when the value is completely cleared.

As you say, the speed is also not not good any more. Last time I revisited that part of the code I could speed it up to something like 14ms for a worst case scenario with large text and Auto size. Didn't measure now but it feels like a second with fixed size and two seconds with Auto. The original code was at many points not looking nice but it was tweaked a little bit to speed, e.g. working often with bytes and not with characters.

I wonder if this part should not only have its own class but done from scratch (the 3rd time then ;-) ).
One of my ideas is to not do the rendering in node anymore. I'm thinking of using the electron offscreen rendering. That would give us super fast html renderings with all the benefits of html and css styling. The only thing holding me back is the headless versions, we would need to use some puppeteer based tool for the headless build then and I honestly have no clue how a raspberry pi would handle that, aside from the increased size that I could live with.
In the long term I think we need to offer even better graphical possibilities and I wonder how long the canvas will be an adequate solution.

[Julusian]

have any example of 1 and 2 I can look at?

yeah 3 is a side effect of making it possible to edit custom dropdown values. might need to opt out of that behaviour on this dropdown

re performance, I admit I only really looked at this for long strings, so my focus was on breaking out of the loops early, and trying to avoid computations that would never yield a sensible result (measuring the width of 10000 chars on one line). But those got down to being in the scale of 10-50ms (I think), so I don't know why shorter strings would have suddenly become much longer.

Some of the other changes I did while in here was, using a predictable string for measuring line height, otherwise the line height grew if you added an emoji to line 1; and using Intl.Segmenter to split the string into characters, the prior way mostly worked, but failed with some characters, I tried to minimise the cost of this by ensuring it would split just once at the beginning (but i suppose in auto it will still be doing it once per size)

I wonder if this part should not only have its own class but done from scratch (the 3rd time then ;-) ).

I don't particularly fancy doing that tbh, even this change which felt safe appears to have side effects...

One of my ideas is to not do the rendering in node anymore. I'm thinking of using the electron offscreen rendering.

Yeah, I think that is going to be a challenge for headless versions, typically chrome/electron/CEF don't run without at least a partial desktop environment, which makes the headless version be not particularly server friendly. It is doable, but people (including me) wont be happy about it.
Something I have wondered about is whether we would be better off to pick some golang/rust/c/c++ library that looks good and fast and write this renderer in that instead. Of course, I have no idea what library to use other than skia, which likely won't change functionality significantly to now. Or go back to not using a library, but making a canvas lib in some native language for performance (it looks like buttons has taken that route).

Another option to consider is whether we just drop some of the auto sizing behaviour when doing the drawing overhaul. I dont know what could be dropped and what would be helpful to drop, but could be worth considering

[dnmeid]

have any example of 1 and 2 I can look at?

<title>Lorem shoud be completely on the next line and also <meta
The lines are cut at top and bottom, there should be one line less instead. The l of lang is cropped at the left side, so it seems also the line lenght is not calculated correct any more or is the whole text placed wrong.

3 is a side effect of making it possible to edit custom dropdown values

It wasn't before. The preset options have always been there and you could add a custom value. Actually that should be the behavior of all dropdowns with custom functionality, isn't it any more? There are predefined options and you can add your own value, but the predefined options are always shown.

using a predictable string for measuring line height

That may be the problem with the height. When switching to the canvas I decided to now use the real height of the rendered line. This can vary dramatically with non latin characters and even with something like  or Ǜ. The problem with the color emojis was just that the emoji font has a completely different line spacing than the text font.

Yeah, I think that is going to be a challenge for headless versions, typically chrome/electron/CEF don't run without at least a partial desktop environment

I was thinking the electron chromium only for the electron builds and puppeteer only for the headless builds. I've not tried it so far but from the first look there is a headless mode for chromium and it needs only the packages libxss1 libappindicator1 libindicator7 as prerequisites on linux. On our side it would be some more OS dependent special treatments.
I'm not sure yet whether I want to start invest time evaluating this.

some golang/rust/c/c++library

skia-canvas is c++ and our node wrapper is rust. I don't think we'll find a faster solution with the same functionality. I did also try the skia-canvas built-in line break functionality, but that was additional functionality of our old library and is not part of the google skia-canvas and it was not much faster. I think the key is the higher integration of a html renderer.

[Julusian]

I'm not seeing that line break issue locally (on linux, but that shouldnt matter)

The top and bottom I am, forgot to trim the last line off..

3 is a side effect of making it possible to edit custom dropdown values
It wasn't before.

I mean it was a recent change which doesnt play well here #2910. I've fixed this case so that it behaves like before.

That may be the problem with the height. When switching to the canvas I decided to now use the real height of the rendered line. This can vary dramatically with non latin characters and even with something like  or Ǜ. The problem with the color emojis was just that the emoji font has a completely different line spacing than the text font.

The problem I was looking to solve here is the latest comment of #2731. It was only sampling the height of the first line, so the spacing may be correct, but also may not be depending on what the line was. I did try using 'correct' spacing for each line, but then emoji/unicode characters had a weirdly long space after them, and this felt like the best compromise, and closer to how it behaved before but more consistently. I am not set on my solution here though, I don't mind it being refined or reverted.

I was thinking the electron chromium only for the electron builds and puppeteer only for the headless builds. I've not tried it so far but from the first look there is a headless mode for chromium and it needs only the packages libxss1 libappindicator1 libindicator7 as prerequisites on linux. On our side it would be some more OS dependent special treatments.

One challenge here will be that the current headless builds are the user downloading the electron build and discarding some of the files. https://github.com/bitfocus/companion-pi/blob/c976f482ded0e9552143b02d371d19ce8bc7c427/update.sh#L148-L152
No reason that couldn't change though

skia-canvas is c++ and our node wrapper is rust. I don't think we'll find a faster solution with the same functionality.

I am not really after faster as such, mainly more features. But I am pretty sure that a pure native version would be faster, the interop between js and c++ does add some overhead.
I am also wondering if doing so would reduce the risk of crashes like this. With this being the second segfault discovered within a couple of weeks, it leaves me concerned.

[dnmeid]

I'm not seeing that line break issue locally (on linux, but that shouldnt matter)

Interesting, my screenshot is from linux too. Running the dev environment on Ubuntu 22.04
Using 7pt fixed size. When I use Auto I get a different result but also with line breaks within words.
E.g.

There should be a line break at the space.
It should decrease the font size to have better line breaks even when more lines would be possible. It seems the font size is only decreased to 24pt when there are more lines possible and then only decreases more when we need to fit more lines.

[Julusian]

I get the same line break there in 3.5.2:

so that looks to be the algorithm choosing it. Something else to improve with the overhaul, adding an option to control when it breaks would be nice