ratatui-org / ratatui

Rust library that's all about cooking up terminal user interfaces (TUIs) šŸ‘Øā€šŸ³šŸ€
https://ratatui.rs
MIT License
8.8k stars 263 forks source link

Inline Viewport Flickering With High Message Throughput #584

Open danny-burrows opened 8 months ago

danny-burrows commented 8 months ago

Description

Hi there, I'm encountering a flickering issue when using the Viewport::Inline setting for the Terminal. On a high throughput of messages (i.e. I'm calling terminal.insert_before(...) with high frequency) there is some flickering behaviour likely caused by its scrolling operation.

To Reproduce

To reproduce you can change just two values in the inline.rs example.

  1. Set NUM_DOWNLOADS to 1000 or more.
  2. In downloads() change the distribution to be very small. (I.e. Change first line of the function to something like let distribution = Uniform::new(0, 3);)

This will have the effect of sending lots of very small downloads through and (for me) reproduces the behaviour. I could imagine the exact values could need tweaking but any number <10 for the upper bound on the distribution works for me to reproduce the flickering.

Expected behavior

Ideally this flickering would be eliminated completely.

Environment(s)

I've tested in all the environments I could get my hands on:

Linux Laptop

Home PC

Home PC WSL

danny-burrows commented 8 months ago

Just wanted to give a brief update for posterity; I've tested again on the new v0.24.0 and this problem persists.

joshka commented 8 months ago

I took a brief look at this the other day (having not looked at the inline code ever). The code has a terminal clear and then redraws the stuff you want to insert. Doing that many times a second is going to flicker. The most obvious thing to do would be the same thing you'd want to do for any high frequency screen update - batch it. I'm not sure that there's an obvious approach to doing that inside Ratatui for this particular method unfortunately. (I took a stab at using the BeginSynchronizedUpdates commands in Crossterm without any result.

More detailed the algorithm for this is:

  1. clear the viewport area
  2. add n lines to the terminal shifting the blank area up that number of lines
  3. draw the content starting from the top of that newly shifted location
  4. Some other stuff after this (the remaining lines / missing lines naming in the method are a little confusing and I'm not sure that I fully grok it yet).

I understand the idea (you want to render many lines before the viewport fast), but I'm not sure that If this is possible given the way ratatui and the terminal work together - perhaps you could talk it through and break down each step with reference to the the method? (That is unless the batch idea is the right one).

danny-burrows commented 8 months ago

Hi there, thanks for your response and for taking the time to look into this! I believe you do understand the crux of the issue.

My use-case is based around taking input from stdin, processing it, and outputting it in both its processed (in the Inline display) and unprocessed (through terminal.insert_before(...)) forms.

Interestingly, my old approach involved the more traditional 'alternate screen' approach, using a List for the unprocessed log. This method seemed to be able to withstand high throughput without flickering, as such I thought it should be achievable somehow in an inline context.

I also tried batch output using terminal.insert_before(x, ...) where x > 1 but there are still flickering issues with a tick rate of <30ms, and I believe from your explanation you probably explored a similar path with BeginSynchronizedUpdates. I appreciate the explanation and that also gives me another avenue to explore.

The fact that high throughput is handled by something such as a List object on an alternate screen leads me to believe the scrolling functionality of terminal.insert_before(...) has something to do with the flickering, but I haven't conducted any investigations to corroborate this.

joshka commented 8 months ago

Yeah - you're right the tick rate / batching won't help as it's the clear that causes the flickering.

I wonder if the algorithm should perhaps clear after the addition of lines rather than before, or perhaps manually clear the part of the buffer that is overwritten instead using the terminal clear function to do it? It would require a bunch of playing around to get this working right though.

The list approach you had previously goes through the buffer diffing process, so it just redraws the parts of the screen that are updated, and hence there isn't any flickering to see (assuming you're not clearing between each frame manually).

danny-burrows commented 8 months ago

Thanks for your explanation about the diffing process, that does make a lot of sense.

I've had a small play around with the insert_before(...) method. There's yet more strange behaviour I'm encountering and I'm unsure if it's intended:

The function signature for reference:

pub fn insert_before<F>(&mut self, height: u16, draw_fn: F) -> io::Result<()>

Firstly, if I try to insert with height > viewport_area.height there seems to be a random blank line inserted before the content produced by draw_fn is appended to the buffer. Setting the first occurrence of missing_lines in the function to a hard-coded value of 1 fixes this but I think it may be a bug in the calculation of the amount of lines to give to self.backend.append_lines(...)?;

Secondly, and this may be intended, the following line just after the guard:

...
self.clear()?;
let height = height.min(self.last_known_size.height); // <-- This line
self.backend.append_lines(height)?;
...

This line caps the height willing to be inserted. However, I find this to be odd seeing as the next line self.backend.append_lines(height)?; scrolls the buffer to accommodate for the lines added. I feel it would be more desirable to remove the cap and let the consumer deal with capping at the viewport height if they so wish. It was quite confusing as to why half of my output was being truncated without warning.

Apologies I don't have any simple minimal-case examples to give just yet, I'm still getting my head around the inner workings of Terminal, and this function in question. I hope some of this helps, I'll keep investigating in the mean-time, but I do really appreciate your timely responses and support thus far.

joshka commented 8 months ago

It's worth taking a look back at the original commit / PR that introduced this for context maybe on the tui-rs repo - IIRC there's not much info on this one though.

danny-burrows commented 8 months ago

Thanks for the suggestion, I took your advice and did some digging but was unfortunately unable to find much information on the reason behind some of the decision-making with this method. I've opened a PR #596 to address the two issues I was facing, perhaps the former was more pressing but I found a solution that remedies both.

joshka commented 7 months ago

Can you confirm that all the issues in here are fixed by #596 or are there remaining problems?

danny-burrows commented 7 months ago

Can you confirm that all the issues in here are fixed by #596 or are there remaining problems?

Hi there, apologies for the delay in replying to this.

596 did indeed fix most of the issues here apart from the flickering itself.

The flickering itself does still remain an issue. I can look into this in detail again when I get some more time but if possible it would be amazing if we could find a fix for it (as I believe there should be one).

kdheepak commented 5 months ago

I haven't looked at the different backends, but in Crossterm there's different kinds of clear:

pub enum ClearType {
    All,
    Purge,
    FromCursorDown,
    FromCursorUp,
    CurrentLine,
    UntilNewLine,
}

Maybe we need something like this?

g33kex commented 5 months ago

To avoid clearing the screen, the scrolling done by the terminal can be taken into account by updating the internal buffer. With Crossterm, I'm using this function to avoid clearing the terminal when scrolling:

fn scroll(
    terminal: &mut Terminal<CrosstermBackend<impl std::io::Write>>,
    height: u16,
) -> io::Result<()> {
    terminal.backend_mut().execute(ScrollUp(height))?;

    // Scroll the internal buffer to avoid unnecessary redraws
    let mut buffer = terminal.current_buffer_mut();
    let offset = (buffer.area().height.min(height) * buffer.area().width) as usize;
    buffer.content.drain(0..offset);
    buffer
        .content
        .extend(repeat_with(|| Cell::default()).take(offset));
    terminal.swap_buffers();
    Ok(())
}

Something like that could allow scrolling the viewport without having to redraw it.

Note that scrolling the behavior might change depending on the terminal/backend...

joshka commented 2 months ago

I wonder if there's anything to learn from Textual's most recent releases: