fdehau / tui-rs

Build terminal user interfaces and dashboards using Rust
MIT License
10.83k stars 483 forks source link

Table widget panics when table doesnt fit screen #470

Closed deepu105 closed 3 years ago

deepu105 commented 3 years ago

Describe the bug The table widget is panicking when it doesn't for the screen

Here is the stack trace thrown at https://github.com/kdash-rs/kdash/blob/main/src/ui/overview.rs#L262

thread '<unnamed>' panicked at 'Trying to access position outside the buffer: x=189, y=16, area=Rect { x: 0, y: 0, width: 189, height: 40 }', /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/buffer.rs:217:9
   0: kdash::panic_hook
             at src/main.rs:57:46
   1: kdash::main::{{closure}}::{{closure}}
             at src/main.rs:76:5
   2: std::panicking::rust_panic_with_hook
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:581:17
   3: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:484:9
   4: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/sys_common/backtrace.rs:153:18
   5: rust_begin_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:483:5
   6: std::panicking::begin_panic_fmt
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:437:5
   7: tui::buffer::Buffer::index_of
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/buffer.rs:217:9
   8: tui::buffer::Buffer::get_mut
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/buffer.rs:184:17
   9: tui::buffer::Buffer::set_style
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/buffer.rs:358:17
  10: tui::widgets::table::render_cell
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/widgets/table.rs:513:5
  11: <tui::widgets::table::Table as tui::widgets::StatefulWidget>::render
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/widgets/table.rs:440:17
  12: tui::terminal::Frame<B>::render_stateful_widget
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/terminal.rs:137:9
  13: kdash::ui::overview::draw_nodes
             at src/ui/overview.rs:304:5
  14: kdash::ui::overview::draw_active_context_tabs
             at src/ui/overview.rs:116:10
  15: kdash::ui::overview::draw_overview
             at src/ui/overview.rs:22:5
  16: kdash::ui::draw
             at src/ui/mod.rs:49:7
  17: kdash::start_ui::{{closure}}::{{closure}}
             at src/main.rs:179:23
  18: tui::terminal::Terminal<B>::draw
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tui-0.14.0/src/terminal.rs:259:9
  19: kdash::start_ui::{{closure}}
             at src/main.rs:179:5
  20: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/mod.rs:80:19
  21: kdash::main::{{closure}}
             at src/main.rs:122:3
  22: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/mod.rs:80:19
  23: tokio::park::thread::CachedParkThread::block_on::{{closure}}
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/park/thread.rs:263:54
  24: tokio::coop::with_budget::{{closure}}
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/coop.rs:106:9
  25: std::thread::local::LocalKey<T>::try_with
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:272:16
  26: std::thread::local::LocalKey<T>::with
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:248:9
  27: tokio::coop::with_budget
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/coop.rs:99:5
      tokio::coop::budget
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/coop.rs:76:5
      tokio::park::thread::CachedParkThread::block_on
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/park/thread.rs:263:31
  28: tokio::runtime::enter::Enter::block_on
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/runtime/enter.rs:151:13
  29: tokio::runtime::thread_pool::ThreadPool::block_on
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/runtime/thread_pool/mod.rs:71:9
  30: tokio::runtime::Runtime::block_on
             at /home/deepu/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/runtime/mod.rs:452:43
  31: kdash::main
             at src/main.rs:73:1
  32: core::ops::function::FnOnce::call_once
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  33: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:137:18
  34: std::rt::lang_start::{{closure}}
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
  35: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/core/src/ops/function.rs:259:13
      std::panicking::try::do_call
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:381:40
      std::panicking::try
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panicking.rs:345:19
      std::panic::catch_unwind
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/panic.rs:382:14
      std::rt::lang_start_internal
             at /rustc/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/library/std/src/rt.rs:51:25
  36: std::rt::lang_start
             at /home/deepu/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5
  37: main
  38: __libc_start_main
  39: _start
────────────────

To Reproduce

I couldnt reproduce this on a standalone setup as it seems to work fine with same data as in error case. Below are the table rows when error happened

vec![
                    "gke-hello-hipster-default-pool-cddd8f85-0t5r",
                    "Ready",
                    "<none>",
                    "v1.18.16-gke.2100",
                    "7",
                    "950m",
                    "2828",
                    "15%",
                    "20%",
                    "6d23h",
                ],
                vec![
                    "gke-hello-hipster-default-pool-cddd8f85-9gzv",
                    "Ready",
                    "<none>",
                    "v1.18.16-gke.2100",
                    "7",
                    "900m",
                    "2820",
                    "20%",
                    "20%",
                    "6d23h",
                ],

So i'm unable to pinpoint whats going wrong here

Expected behavior Should not panick regardless of screen size

Desktop (please complete the following information):

Dhghomon commented 3 years ago

I had something similar happen to me that seems to have been due to moving the Layout out of the loop. I have this:

        let v = Layout::default() // v for vertical
        .direction(Direction::Vertical)
        .margin(1)
        .constraints(
            [
                Constraint::Min(3),
                Constraint::Percentage(30),
                Constraint::Percentage(70),
            ]
            .as_ref(),
        )
        .split(terminal.size().unwrap());        let v = Layout::default() // v for vertical
        .direction(Direction::Vertical)
        .margin(1)
        .constraints(
            [
                Constraint::Min(3),
                Constraint::Percentage(30),
                Constraint::Percentage(70),
            ]
            .as_ref(),
        )
        .split(terminal.size().unwrap());

Then there's a loop and the terminal displays with all of the widgets. Later on I ended up moving this out of the loop (thinking that it made sense because I use this layout each time) and that's when it started panicking by saying that the index is (some number+1) while the length is (some number).

Not sure if that helps you but seems similar enough to mention. Here's what the stack trace looked like:

 thread 'main' panicked at 'index out of bounds: the len is 918 but the index is 919', C:\Users\mithr\.cargo\registry\src\github.com-1ecc6299db9ec823\tui-0.14.0\src\buffer.rs:185:14
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\std\src\panicking.rs:493
   1: core::panicking::panic_fmt
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\core\src\panicking.rs:92
   2: core::panicking::panic_bounds_check
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\core\src\panicking.rs:69
   3: tui::buffer::Buffer::set_style
   4: <tui::widgets::block::Block as tui::widgets::Widget>::render
   5: tui::terminal::Terminal<B>::draw
   6: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   7: ZN4core3ptr622drop_in_place$LT$core..iter..adapters..map..map_fold$LT$$RF$tui..layout..Constraint$C$tui..layout..Rect$C$$LP$$RP$$C$tui..layout..split..$u7b$$u7b$closure$u7d$$u7d$$C$core..iter..traits..iterator..Iterator..for_each..call$LT$tui..layout..Rec
   8: std::rt::lang_start::{{closure}}
   9: core::ops::function::impls::{{impl}}::call_once
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\library\core\src\ops\function.rs:280
  10: std::panicking::try::do_call
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\std\src\panicking.rs:379
  11: std::panicking::try
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\std\src\panicking.rs:343
  12: std::panic::catch_unwind
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\std\src\panic.rs:431
  13: std::rt::lang_start_internal
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0\/library\std\src\rt.rs:51
  14: main
  15: invoke_main
             at D:\agent\_work\9\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  16: __scrt_common_main_seh
             at D:\agent\_work\9\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  17: BaseThreadInitThunk
  18: RtlUserThreadStart
deepu105 commented 3 years ago

Hi @Dhghomon thanks for the info, just a quick question, when you mean you moved the layout out of the loop, what does it mean? I also have common util functions that builds the layout but in the end they are still called inside the UI loop for rendering.

The issue keeps happening for me when the table is bigger than window and during resize. Aso your error seems to index out of bounds where as for me its Trying to access position outside the buffer so I think the reason could be different as well.

The weird thing is this does not happen on VSCode terminal often and in Yakuake it happens more often and in Tilix it happens all the time. So may be this is dependent on the emulator as well

The error always comes from this function in tui-rs (src/buffer.rs:217:9) so possibly the size update calculation hapens slower due to some race condition

    pub fn index_of(&self, x: u16, y: u16) -> usize {
        debug_assert!(
            x >= self.area.left()src/buffer.rs:217:9
                && x < self.area.right()
                && y >= self.area.top()
                && y < self.area.bottom(),
            "Trying to access position outside the buffer: x={}, y={}, area={:?}",
            x,
            y,
            self.area
        );
        ((y - self.area.y) * self.area.width + (x - self.area.x)) as usize
    }
Dhghomon commented 3 years ago

Ah, probably a different issue then. What I meant by moving out of the loop is that I have a loop where all the Widgets are created on the fly and used as instant commands (as tui says to do) and one day I got the idea that it might be more efficient to keep the Layout outside of the loop (since it doesn't change) and then only call render_widget on each of the parts of the layout inside of the loop, and that's when the problems began.

The code's a bit different now but you can see it here:

https://github.com/Dhghomon/dictionariums/blob/main/src/main.rs

The part that I moved to the top out of the loop is this one:

    let v = Layout::default() // v for vertical
        .direction(Direction::Vertical)
        .margin(1)
        .constraints(
            [
                Constraint::Min(3),
                Constraint::Percentage(30),
                Constraint::Percentage(70),
            ]
            .as_ref(),
        )
        .split(terminal.size().unwrap());
deepu105 commented 3 years ago

Ah ok. Hmmm, I think that indeed could be problematic as I assume that some layout related stuff needs to be calculated on the fly especially the area on loop to accomodate for resize etc. But its just a guess. My issue is quite different indeed.

deepu105 commented 3 years ago

I'm willing to try and fix this but I would need some guidance as i'm very new to TUI

fdehau commented 3 years ago

@deepu105 Hey 👋 . This looks like an off by one error in the computation of the widths of the table columns. More than the data passed to the widget, I would be interested by the values of all parameters passed to the Table builder methods such as column_spacing, widths, highlight_symbol and the area StatefulWidget::render is called with.

deepu105 commented 3 years ago

Hi this is one of the table https://github.com/kdash-rs/kdash/blob/main/src/ui/overview.rs#L265 let me know if this helps. Else I can try to create a standalone sample

deepu105 commented 3 years ago

Ok I was finally able to reproduce it with a standalone sample. If you resize the terminal fast enough, this breaks (didn't add a panic hook so doesnt show the trace)

#[allow(dead_code)]
mod util;

use crate::util::event::{Event, Events};
use std::{error::Error, io};
use termion::{event::Key, input::MouseTerminal, raw::IntoRawMode, screen::AlternateScreen};
use tui::{
    backend::TermionBackend,
    layout::{Constraint, Layout, Direction},
    style::{Color, Modifier, Style},
    widgets::{Block, Borders, Cell, Row, Table, TableState},
    Terminal,
};

pub struct StatefulTable<'a> {
    state: TableState,
    items: Vec<Vec<&'a str>>,
}

impl<'a> StatefulTable<'a> {
    fn new() -> StatefulTable<'a> {
        StatefulTable {
            state: TableState::default(),
            items: vec![
                vec![
                    "gke-hello-hipster-default-pool-cddd8f85-0t5r",
                    "Ready",
                    "<none>",
                    "v1.18.16-gke.2100",
                    "7",
                    "950m",
                    "2828",
                    "15%",
                    "20%",
                    "6d23h",
                ],
                vec![
                    "gke-hello-hipster-default-pool-cddd8f85-9gzv",
                    "Ready",
                    "<none>",
                    "v1.18.16-gke.2100",
                    "7",
                    "900m",
                    "2820",
                    "20%",
                    "20%",
                    "6d23h",
                ],
                vec![
                    "gke-hello-hipster-default-pool-cddd8f85-9gzv",
                    "Ready",
                    "<none>",
                    "v1.18.16-gke.2100",
                    "7",
                    "900m",
                    "2820",
                    "20%",
                    "20%",
                    "6d23h",
                ],
            ],
        }
    }
    pub fn next(&mut self) {
        let i = match self.state.selected() {
            Some(i) => {
                if i >= self.items.len() - 1 {
                    0
                } else {
                    i + 1
                }
            }
            None => 0,
        };
        self.state.select(Some(i));
    }

    pub fn previous(&mut self) {
        let i = match self.state.selected() {
            Some(i) => {
                if i == 0 {
                    self.items.len() - 1
                } else {
                    i - 1
                }
            }
            None => 0,
        };
        self.state.select(Some(i));
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    // Terminal initialization
    let stdout = io::stdout().into_raw_mode()?;
    let stdout = MouseTerminal::from(stdout);
    let stdout = AlternateScreen::from(stdout);
    let backend = TermionBackend::new(stdout);
    let mut terminal = Terminal::new(backend)?;

    let events = Events::new();

    let mut table = StatefulTable::new();

    // Input
    loop {
        terminal.draw(|f| {
            let chunks = Layout::default()
            .constraints(vec![Constraint::Length(2), Constraint::Min(0)])
            .direction(Direction::Vertical)
            .margin(1)
            .split(f.size());

            let block = Block::default().borders(Borders::TOP).title("Table Title test");
            let rows = table.items.iter().map(|item| {
                let style = Style::default().fg(Color::Green);

                let cells: Vec<Cell> = item.iter().map(|c| Cell::from(*c)).collect();
                Row::new(cells).style(style)
            });

            let header = Row::new(vec![
                "Name", "Status", "Roles", "Version", "Pods", "CPU", "Mem", "CPU %", "Mem %", "Age",
            ])
                .style(Style::default().fg(Color::Green))
                .bottom_margin(0);

            let t = Table::new(rows)
                .header(header)
                .block(block)
                .highlight_style(Style::default().add_modifier(Modifier::REVERSED))
                .highlight_symbol("=>")
                .widths(&[
                    Constraint::Percentage(30),
                    Constraint::Percentage(10),
                    Constraint::Percentage(15),
                    Constraint::Percentage(10),
                    Constraint::Percentage(5),
                    Constraint::Percentage(5),
                    Constraint::Percentage(5),
                    Constraint::Percentage(5),
                    Constraint::Percentage(5),
                    Constraint::Percentage(10),
                ]);

            f.render_stateful_widget(t, chunks[1], &mut table.state);
        })?;

        if let Event::Input(key) = events.next()? {
            match key {
                Key::Char('q') => {
                    break;
                }
                Key::Down => {
                    table.next();
                }
                Key::Up => {
                    table.previous();
                }
                _ => {}
            }
        };
    }

    Ok(())
}
andschneider commented 3 years ago

Hello all. I've been getting a similar crash:

thread 'main' panicked at 'Trying to access position outside the buffer: x=98, y=11, area=Rect { x: 0, y: 0, width: 98, height: 26 }

Here's the table:

  let selected_style = Style::default().bg(Color::Blue).fg(Color::Black);
  let t = Table::new(rows)
      .header(header)
      .block(
          Block::default()
              .borders(Borders::ALL)
              .border_type(BorderType::Rounded)
              .title("scoreboard"),
      )
      .highlight_style(selected_style)
      .highlight_symbol(">> ")
      .widths(&[
          Constraint::Percentage(15),
          Constraint::Percentage(15),
          Constraint::Percentage(25),
          Constraint::Percentage(45),
      ]);

Things I've tried:

Not sure why the Constraint seemed to do it. Happy to help further if needed!

deepu105 commented 3 years ago

Seems like the workaround doesnt help always. I did that for every table and it still crashes at times

deepu105 commented 3 years ago

@andschneider for your workaround does it matter if I subtract 1 from last or any column?

andschneider commented 3 years ago

@andschneider for your workaround does it matter if I subtract 1 from last or any column?

I only tried with the last column. I thought it would work with any of them though.

deepu105 commented 3 years ago

This is becoming quite an annoying issue as more and more people are opening crash reports in KDash due to this. I would be happy to try and fix this, but I have no idea where to start. @fdehau if you can point me in the right direction may be I can give it a shot

andschneider commented 3 years ago

Hey @deepu105, I started digging around last week and think I can fix it (at least with a small hack).

Can you post the tables that are causing a crash so I can test them? Mostly need the widths, highlight symbol, block, and column spacing.

Also, the width of the Rect at which the crash occurs.

deepu105 commented 3 years ago

I was able to reproduce it with this sample, but not always, only if I resize fast enough https://github.com/fdehau/tui-rs/issues/470#issuecomment-831171722

andschneider commented 3 years ago

@deepu105 I added that one as a test case, it was happening at 189 width as can be seen in the stack trace from your first comment. Part of the trouble with this is seems to happen at various widths...

Do you know if that's the same table your users are reporting a crash on as well? Would love to get more tests cases if possible.

In the meantime, I opened a PR with my fix. Feel free to try it and see if it helps!

deepu105 commented 3 years ago

Actually it happens on almost all tables when the data is as long as the column and resize happens

On Wed, 21 Jul 2021, 8:36 pm Andrew Schneider, @.***> wrote:

@deepu105 https://github.com/deepu105 I added that one as a test case, it was happening at 189 width as can be seen in the stack trace from your first comment. Part of the trouble with this is seems to happen at various widths...

Do you know if that's the same table your users are reporting a crash on as well? Would love to get more tests cases if possible.

In the meantime, I opened a PR with my fix. Feel free to try it and see if it helps!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fdehau/tui-rs/issues/470#issuecomment-884406190, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIOKF3AUZ2QCS66OREWVJDTY4HTXANCNFSM43B4KZ4A .