zellij-org / zellij

A terminal workspace with batteries included
https://zellij.dev
MIT License
19.7k stars 618 forks source link

Epilepsy warning: Flickering due to segmented rendering with apps which refresh constantly such as top or pw-top #2828

Closed pallaswept closed 7 months ago

pallaswept commented 9 months ago

zellij --version: 0.38.2 stty size: 58 240 uname -av or ver(Windows): Linux 6.5.4-1-default #1 SMP PREEMPT_DYNAMIC Wed Sep 20 05:07:04 UTC 2023 (fdd7e9e) x86_64 x86_64 x86_64 GNU/Linux

List of programs you interact with as, PROGRAM --version: output cropped meaningful, for example:

kitty 0.30.0 created by Kovid Goyal foot version: 1.15.3 -pgo +ime +graphemes +fractional-scaling +cursor-shape -assertions konsole 23.08.1

pw-top Compiled with libpipewire 0.3.80 Linked with libpipewire 0.3.80

Further information Start terminal - reproducible in foot, kitty and konsole run zellij run pw-top observe flicker during updates

Log (not much to see here but this is what I got)

INFO   |zellij_client            | 2023-09-30 19:04:50.886 [main      ] [zellij-client/src/lib.rs:152]: Starting Zellij client! 
INFO   |zellij_server            | 2023-09-30 19:04:50.890 [main      ] [zellij-server/src/lib.rs:245]: Starting Zellij server! 
INFO   |zellij_server            | 2023-09-30 19:04:50.942 [main      ] [zellij-server/src/lib.rs:913]: Compiling plugins using Cranelift 
INFO   |zellij_server::plugins   | 2023-09-30 19:04:50.942 [wasm      ] [zellij-server/src/plugins/mod.rs:137]: Wasm main thread starts 
INFO   |zellij_server::plugins::p| 2023-09-30 19:04:50.952 [async-std/runti] [zellij-server/src/plugins/plugin_loader.rs:487]: Loaded plugin 'tab-bar' from cache folder at '/home/pallaswept/.cache/zellij' in 3.946799ms 
INFO   |zellij_server::plugins::p| 2023-09-30 19:04:50.953 [async-std/runti] [zellij-server/src/plugins/plugin_loader.rs:487]: Loaded plugin 'status-bar' from cache folder at '/home/pallaswept/.cache/zellij' in 5.03523ms 
INFO   |zellij_server::plugins::w| 2023-09-30 19:05:05.342 [wasm      ] [zellij-server/src/plugins/wasm_bridge.rs:198]: Bye from plugin 0 
ERROR  |zellij_server::os_input_o| 2023-09-30 19:05:05.342 [screen    ] [zellij-server/src/os_input_output.rs:867]: Failed to apply cached resizes: failed to send message to pty writer 
INFO   |zellij_client            | 2023-09-30 19:05:05.342 [main      ] [zellij-client/src/lib.rs:479]: Bye from Zellij! 
ERROR  |zellij_server::os_input_o| 2023-09-30 19:05:05.342 [screen    ] [zellij-server/src/os_input_output.rs:856]: Failed to cache resizes: failed to send message to pty writer 
ERROR  |zellij_server::os_input_o| 2023-09-30 19:05:05.342 [screen    ] [zellij-server/src/os_input_output.rs:867]: Failed to apply cached resizes: failed to send message to pty writer 
INFO   |zellij_server::plugins::w| 2023-09-30 19:05:05.342 [wasm      ] [zellij-server/src/plugins/wasm_bridge.rs:198]: Bye from plugin 1 
INFO   |zellij_server::plugins   | 2023-09-30 19:05:05.343 [wasm      ] [zellij-server/src/plugins/mod.rs:339]: wasm main thread exits 
imsnif commented 9 months ago

Thanks for the report. I changed the title to a more appropriate one.

pallaswept commented 9 months ago

Thanks for the report. I changed the title to a more appropriate one.

I want to make this really clear for your TL;DR/IDGAF (I don't know which it is, but it's obviously one of them) approach to epilepsy:

What you've got here is a bug that could actually kill a human being.

I understand that's not a comfortable thing to have to label your software with, and that sucks, but knowingly hiding it is not morally acceptable behaviour. Like 0.05% of people 5 in every 10,000, will even care when they see the warning, and the other 99.95% of people will not even be deterred... Come on, how many times have you played a computer game with one of these warnings (which are there because the likes of EA don't want to get sued for killing some kid) and gone "wow I shouldn't play this"? Practically nobody cares about a warning. Except the people who need it. Those 0.05% deserve fair warning. Me, I'll just go partially blind and have a splitting headache and throw up all day for a week. Someone sicker than me could have it much worse off.

This is not specific to pw-top, it happened in a handful of apps (like good old fashioned top, for example, anything that updated regularly would do it), pw-top was just a prime candidate. This change of title is not more appropriate, it's less appropriate, it's less accurate, and socially irresponsible and morally questionable.

Do the right thing, man. It won't push anyone away from your software in any kind of numbers that will hurt, but it might just save someone a serious impact to their health. It doesn't need to be all serious and "oooohh ahhh deaddly app is dedly" in giant text, just a quick word of warning on the front page that there are currently known bugs causing flickering that could be a problem for epileptics and the like. You don't need to be all doom and gloom. But you do need to do something.

To the two of you who gave this a thumbs down: I'm going to apply Hanlon's razor to your despicable behaviour so that I can feel sorry for you rather than being angry at you. Maybe you ought to go visit a children's hospital neurology ward and see what's at stake here so you have some perspective about why I'm taking this seriously and why you oughtn't be so foolish. You should take a good look at how the developer handled this and learn from it. More people acting like imsnif and less like you did, would make this world a better place. Be better.

imsnif commented 9 months ago

Hey @pallaswept - as I mentioned in the other issue: I am very sorry for your experience. Really, from the bottom of my heart. While I don't think this is Zellij (more on this below), I fully believe you that you experienced discomfort and am sorry for any part the code I wrote played in that.

Because I find this subject important, I went and read out about it on the accessibility level. According to the Epileptic Society, the rate of flickers that causes seizures in epileptic patients is 3-30 hertz (source). This is also reflected in the accessibility guidelines of the web consortium.

Not only could I not reproduce any such rate of flickers with pw-top, having Zellij flicker in this way for any reason that is not explicitly app created would be nearly impossible. Given the size of our pty buffer and the rate at which we send interrupt renders. The chances of this happening one time are astronomical, the chances of it happening consistently for you are imaginary.

I am once again sorry for your experience, and am looking forward to learning more about the subject by being pointed to credible sources. I will otherwise use this issue to address the specific flicker you pointed out and once again thank you for reporting it.

pallaswept commented 9 months ago

Thanks for putting in the serious effort man, that's very cool of you. Please don't feel sorry, it's not like you deliberately designed it to do this :) I've still got zellij installed and I'll stay subbed to this issue, so I look forward to giving it another shot when you have some wins. You have a winning attitude, I'm sure it won't be long :)

pallaswept commented 9 months ago

I generally try to stick to the facts and technicalities in GH issues, but I feel like I need to say this: Ever since that post, I have been sitting here reflecting on seeing that blue dot notification, and immediately feeling a sinking dread of ending up in some kind of stupid internet argument with some asshole sociopath dev who couldn't handle finding a bug in their app and just wanted to 'shoot the messenger' (with which, unfortunately, the software development world is well populated; I know, because it used to be my profession), and then when I saw it was this thread I was even more worried, (because we had already disagreed, not because of anything you'd done or anything like that, don't get me wrong!)..... and then reading your reply, and feeling an overwhelming sense of relief.

I really want to thank you for the way you've handled all this. It could have been really unpleasant but your behaviour is outstanding and I really appreciate it. You're strong enough to stick to your guns but noble enough to find an amicable outcome. Such honour is sadly uncommon. I wish there were more devs like you. I hope that you have a wonderful week and run into some other high quality human beings as you go. Really, thanks. Thank you so much.

pallaswept commented 9 months ago

I had an interesting experience with this today. I was using tmux (which I'm liking less and less by the second) on foot, and saw a bit of flicker from pw-top in there, too - not like what I saw to make this issue, but some. Foot has some parameters which control the way the screen is drawn, (see https://manpages.ubuntu.com/manpages/impish/man5/foot.ini.5.html#tweak and the parameters delayed-render-lower, delayed-render-upper) and pushing out the lower time to a considerably long time (thus forcing updates to be made less often) was enough to stop the flicker there. So I'm pretty sure this isn't specific to zellij, it's effecting tmux as well; nor to any specific terminal emulator (I tested kitty and saw the same behaviour as with foot), it seems like pw-top is really pushing an edge case on how it refreshes the screen. I do think that zellij and the terminal emulators should be able to handle apps that are misbehaving, but this definitely strikes me as an app that's misbehaving.

TL;DR I do think zellij (and tmux, foot, and kitty, and anything else that might be displaying a potentially naughty app) should be designed to handle naughty apps gracefully (as I say, it even flickered in zellij with good old fashioned top; naughty apps are a thing, for sure), but I think the real trigger of this issue and this experience is pw-top, refreshing the screen very excessively, or something like that.

I'm going to follow it up with the pipewire guys, too. I'm not sure they'll be too keen to bear the weight of this though, because outside of either zellij or tmux, in just foot or kitty or konsole, without any multiplexer, pw-top doesn't flicker at all. So they could rightly argue that it's the multiplexers' faults and not theirs. But while it's far far worse in zellij,it's not specific to zellij, and I thought that was important information to share.

dnkl commented 9 months ago

@imsnif have you considered using Application Synchronized Updates (private mode 2026)?

https://gitlab.freedesktop.org/terminal-wg/specifications/-/merge_requests/2

Like most things over at terminal-wg, the bike shedding prevented the PR from being merged, but it's more or less a de facto standard now, supported by quite a few terminals. (note that the PR describes two DCS sequences, but what everybody ended up using was private mode 2026 instead. Some terminals, for example foot, support both).

dnkl commented 9 months ago

Not sure if there's anything zellij can do to help with "bad" applications, but perhaps it can ensure that it trickles through to the terminal in a way that keeps the applications original intention. I.e. don't flicker for what the application perceives as an atomic transaction ("frame").

dnkl commented 9 months ago

I do think that zellij and the terminal emulators should be able to handle apps that are misbehaving, but this definitely strikes me as an app that's misbehaving.

@pallaswept "delayed rendering", in one way or another, is the way terminal emulators handle it. Nearly all terminal emulators do this (Alacritty being one of them that does not, last time I checked).

But a terminal cannot delay rendering indefinitely, since it doesn't know when the application is "done". It'll always be possible to write an application that flickers.

Application Synchronized Updates is the solution to the problem, but obviously requires changes in all applications. It's unfortunate, but given that the application itself is the only one that knows when a frame begins, and ends, there's no way around this.

Also, fyi, what well-behaving applications did before application synchronized updates, is to buffer everything, and write the entire buffer in one go when the frame is done. This can however still cause flicker, if the amount of data is too large.

pallaswept commented 9 months ago

@dnkl thanks so much for the insightful information you've shared here. It's obviously a topic with which you're well-acquainted, so it's kind of you to share your knowledge on the subject.

I mentioned earlier that foot was able to 'save the day' in this case;

pushing out the lower time to a considerably long time (thus forcing updates to be made less often) was enough to stop the flicker

To be more specific, you might find it interesting that, in order to prevent pw-top from flickering on tmux on foot, I had to set delayed-render-lower=8333333 delayed-render-upper=16666666 Which, obviously, is extreme. The lower time had to be set so high (at least half the refresh rate of the display) that I had to increase the upper time to make room for it (which is why I ended up pushing the upper time out to an entire frame's worth). For your interest, setting it to 0 to disable the delayed rendering entirely, turned pw-top into an absolute nightmare which I couldn't even look at for more than a fraction of a second.

It'll always be possible to write an application that flickers.

Of course, there's only so much that can be done here. An application that 'flickers' might even be the intention - for example I'm thinking of the tools used to measure frame times by flashing coloured frames or constantly changing coloured strips (perhaps not the realm of terminal apps, but you get my point - sometimes the app might want to flicker). So naturally I understand that the weight of solving this issue can't rest entirely upon the terminal or muxer or whatever app hosts the client application. As you say, "given that the application itself is the only one that knows when a frame begins, and ends, there's no way around this." and I completely agree.

While there may be no "official" standard to handle this issue, if there's a "pseudo" standard that's in wide use, I think that's almost as good and certainly better than nothing.

I have plenty of development experience but the bulk of my career was spent in embedded devices or management of development and support teams, so something as complex and specialised as terminal emulation isn't my wheelhouse, at all. I do wonder why it is that pw-top won't flicker in foot, or in kitty, but will flicker in foot or kitty running zellij or tmux; nor why it's worse in zellij than tmux.

If I'm to make suggestions to the pipewire devs about timing the output from their app, perhaps you can offer some insight here. I wouldn't blame the PW devs seeing an issues from me about this, if they started pw-top themselves in their terminal emulator, saw no flicker, and gave it a "works for me" and ignore/close the issue. If I tell them "well hey try it in zellij" and they do see the problem, then I also wouldn't blame them for saying "well, take it up with zellij, it works fine without zellij, so it's a zellij problem".

Obviously this is an issue that's shared between zellij and the applications (specifically pw-top and top, but I'm sure there will be others), and I'm keen to get the applications to play nicely with zellij, so I hope you can offer me some guidance as to how to bring this up with the application devs.

dnkl commented 9 months ago

I do wonder why it is that pw-top won't flicker in foot, or in kitty, but will flicker in foot or kitty running zellij or tmux; nor why it's worse in zellij than tmux.

Terminal multiplexers are terminal emulators themselves. That means what is potentially a nice, buffered, single-write from the application, could eventually end up being multiple writes (by the multiplexer), to the top-level terminal emulator (meaning potential flicker). It all depends on how buffering, and write coalescing is done by the multiplexer.

If I'm to make suggestions to the pipewire devs about timing the output from their app, perhaps you can offer some insight here.

I'd just kindly point them to private mode 2026. The fact that it (pw-top) flickers without foot's delayed rendering means they're not playing nice at all (no buffering at all of their writes). And while many terminal emulators are able to mask that, it breaks down in terminal multiplexers. If they don\t want to implement application synchronized updates, then ask them to buffer their frame updates into single writes. You might also suggest to them that they test pw-top in low-latency terminal emulators.

I don't think there's that much else that can be done, I'm afraid.

pallaswept commented 9 months ago

Thanks so much for your input here @dnkl you've been immensely helpful. I wonder if you would mind if I refer to this conversation, when I bring this up with the pipewire devs?

The fact that it (pw-top) flickers without foot's delayed rendering means they're not playing nice at all (no buffering at all of their writes)

I had a quick look at its source and it appears to simply attempt to redraw the entire screen every time any parameter it displays, changes....and since they're reading out numbers measured in microseconds, you can imagine how often it might update. And no, I didn't see any sign (although I may have missed something) of buffering or even delaying updates/updating on a timer - which explains why when disabling foot's delayed updates, turned it into a complete nightmare. If it is as it initially appears to me, it's definitely not playing nicely and is definitely written in a such a way that it would be the most extreme example of how to cause screen flicker in a terminal.

Terminal multiplexers are terminal emulators themselves. That means what is potentially a nice, buffered, single-write from the application, could eventually end up being multiple writes (by the multiplexer), to the top-level terminal emulator (meaning potential flicker). It all depends on how buffering, and write coalescing is done by the multiplexer.

Right, so that explains why I see it in zellij and tmux but not in foot or kitty. It all makes sense to me now. Thanks again for all your help.

@imsnif This issue is now a feature request for private mode 2026 support to be added to zellij 😆 Probably some other kind of handling similar to foot's delayed updates, would be a feature request too, just for those client apps which don't support private mode 2026.

Meanwhile I will try to get pw-top sorted out. There are probably 999999999 other apps out there that need some love here too, but I'll stick to pw-top for starters.

dnkl commented 9 months ago

I wonder if you would mind if I refer to this conversation, when I bring this up with the pipewire devs?

@pallaswept of course (this is a public forum after all)!

pallaswept commented 9 months ago

Issue logged over in Pipewire land https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/3567 Foot and kitty (my shortlist of terminal emulators) both support it already I didn't have much positivity from tmux devs so I won't bother them with it, hopefully they'll pick it up themselves as it spreads in popularity And now just hoping to see support in zellij

Progress is being made :) Thanks so much to you both for all of your help.

pallaswept commented 7 months ago

Oh fantastic! Thanks @imsnif !!! You too, @dnkl :)

l2dy commented 4 months ago

Issue logged over in Pipewire land https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/3567 Foot and kitty (my shortlist of terminal emulators) both support it already I didn't have much positivity from tmux devs so I won't bother them with it, hopefully they'll pick it up themselves as it spreads in popularity And now just hoping to see support in zellij

Just FYI, tmux 3.4 uses SM 2026 by default. https://github.com/tmux/tmux/commit/1a14d6d2e1c2a23544eefa6e5e65e05ee7e1aca7