feat(render): remove flickering

[LeperGnome]

Hey! This is my naive approach to fixing output flickering. Here are my thoughts behind the solution:

• Flickering is caused by ansi.EraseEntireLine escape chars, because it allows the time, when there's an empty output.
• So instead we just move cursor to the beginning of the drawing section on each new frame and write over latest frame, thus replacing old characters.
• Erase characters from the old frame on each line end with ansi.EraseLineRight
• Erase lines from old frame with ansi.EraseDisplayRight

I tested #1032 and it solves it. Also solved the issue for my project + a handfull of examples all work as intended.

I also looked into some PRs regarding render mechanism (e.g. #1027), and it seems like you're reworking it, but my change seems to provide good value for what it is, without messing up the API and so on...

I might just be missing some important concept here, so please let me know what you think, thanks!

[LeperGnome]

Hey guys! Any feedback? I'll gladly fix remaining tests if you confirm, that this is the right direction.

[meowgorithm]

Wow, thanks for this, @LeperGnome. I didn't expect this to work as well as it appears to.

We are indeed working on a new renderer, but this is potentially quite helpful in the meantime as you suggest. That said, it's still a very fundamental change, so we will need to test this very thoroughly in a variety of terminals, environments, and situations before we can merge it (for example, on Windows, behind SSH, and so on). Because of that, I'd like to merge it but can't offer an answer or ETA just yet.

[LeperGnome]

@meowgorithm that's great! Let me know if you need any help in testing / fixing golden solution.

[meowgorithm]

@LeperGnome yeah, if you could fix the tests that would be awesome!

[LeperGnome]

Could you please run the CI pipeline, so I can see if I fixed all the tests

[LeperGnome]

All right, checks green, let me know if you need any more help, looking forward to this thing being merged!

[meowgorithm]

Thanks for doing that! As I mentioned, this will take some time for us to vet, and I can neither offer and ETA nor promise it will be merged, but so far it seems pretty good.

[aymanbagabas]

Hi @LeperGnome, thank you for sending this PR. I think this makes sense to merge, just a few comments below. We also need to ensure that this is VT100 compliant and works with all terminals out there i.e. Linux Console, Urxvt, Xterm, etc.

[LeperGnome]

@aymanbagabas

ensure that this is VT100 compliant

I'm not a terminal emulator guru by any means, but it seems like control characters, that I've introduced are listed on vt100.net. Does it ensure the compliance?

I'm just educating myself, not proposing you to skip any testing

[aymanbagabas]

I'm not a terminal emulator guru by any means, but it seems like control characters, that I've introduced are listed on vt100.net. Does it ensure the compliance?

Yes, but I would still test this inside terminals, especially things like the Linux Console.

[LeperGnome]

@aymanbagabas Hi! What should I do about the skipLine? As I pointed out in review comment, it's not obvious for me, that this is actually an optimisation (correct me if I'm wrong). If decision was up to me, I would

• either remove it (as I did in PR)
• or conduct some testing with different terminal emulators, terminal sizes, rendered content

I'm not really sure, how would I measure results, since I would have to measure not only the speed of bubbletea program frame generation, but also emulator rendering speed with relation to frame content length.

[aymanbagabas]

@aymanbagabas Hi! What should I do about the skipLine? As I pointed out in review comment, it's not obvious for me, that this is actually an optimisation (correct me if I'm wrong). If decision was up to me, I would

It would be a huge optimization for remote sessions like SSH. You want to reduce the amount of data sent over the wire as much as possible in such cases. I think comparing strings is not expensive and saves a lot of data from being sent.

if newLines[i] == oldLines[i] {
  buf.WriteString(ansi.CursorDown1) // "\x1b[B"
} else {
  line = line + ansi.EraseLineRight
  // ...
}

ansi.CursorDown1 is only 3 bytes [\x1b [ B] compared to the whole line

[LeperGnome]

Oh, I never thought of this in context of ssh, I guess you're right!
I'll update my PR.

[LeperGnome]

@aymanbagabas Hey, I added skipping logic.
Also tried to make it work even with queued messages, spent some time debugging why it does work, hours later noticed this:

case printLineMessage:
    ... 
    r.repaint()

which removes our previous frame state.

So I figured that it's easier to just skip this optimization in this case... Also added a comment, so the next person won't struggle so much =)

P.S. I guess tests are failing on main, regardless of my changes

[mcoops]

I'll throw in, can confirm this works fantastic for our usage, especially in a pty, and it'd be a great stop gap until the rewrite for the renderer is ready.

The last commit especially has removed any random artifacting that we were experiencing - the TUI screen was stable except for some occasional, random refresh for a lack of a better word.

But it's really nice now!