Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] TextView performance regression with LinearLayout #728

Open
night-crawler opened this issue Apr 17, 2023 · 1 comment
Open

[BUG] TextView performance regression with LinearLayout #728

night-crawler opened this issue Apr 17, 2023 · 1 comment
Labels

Comments

@night-crawler
Copy link

night-crawler commented Apr 17, 2023

Describe the bug
compute_rows() method is called multiple times. In views with thousands of rows it causes significant performance issues. This issue is a twin of #582 (EDIT: one more: 698)

To Reproduce
The easiest way to reproduce is here: https://github.com/night-crawler/cursive-lazy-text-view/blob/main/src/main.rs
(You can clone the full sample)

use cursive::direction::Orientation;
use cursive::theme::ColorStyle;
use cursive::traits::{Nameable, Resizable, Scrollable};
use cursive::utils::markup::StyledString;
use cursive::view::ScrollStrategy;
use cursive::views::{Checkbox, EditView, LinearLayout, Panel, ResizedView};

// use crate::ltv::TextView;
use cursive::views::TextView;

fn mk_tv() -> ResizedView<TextView> {
    let line = "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.";
    let mut tv = TextView::new("");

    for _ in 0..100 {
        for color in [ColorStyle::primary(), ColorStyle::secondary(), ColorStyle::highlight_inactive()].iter() {
            tv.append(StyledString::styled(line, *color));
        }
    }

    tv.full_screen()
}

fn main() {
    let mut siv = cursive::default();

    let mut main_layout = LinearLayout::new(Orientation::Vertical);
    let mut filter_layout = LinearLayout::new(Orientation::Horizontal);

    let filter_edit_view = EditView::new();
    let since_minutes_edit_view = EditView::new();
    let filter_tail_lines_edit_view = EditView::new();

    let cb_timestamps = Checkbox::new()
        .checked()
        .with_name("cb1");

    let cb_previous = Checkbox::new()
        .checked()
        .with_name("prev");

    let filter_edit_view_panel = Panel::new(filter_edit_view).title("Search").full_width();
    let since_minutes_panel = Panel::new(since_minutes_edit_view).title("Since minutes");
    let filter_tail_lines_panel = Panel::new(filter_tail_lines_edit_view).title("Tail lines");
    let cb_timestamps_panel = Panel::new(cb_timestamps).title("Timestamps");
    let cb_previous_panel = Panel::new(cb_previous).title("Previous");

    filter_layout.add_child(filter_edit_view_panel);
    filter_layout.add_child(cb_timestamps_panel);
    filter_layout.add_child(cb_previous_panel);
    filter_layout.add_child(since_minutes_panel);
    filter_layout.add_child(filter_tail_lines_panel);

    main_layout.add_child(filter_layout.full_width());

    let tv = mk_tv().scrollable()
        .scroll_x(true)
        .scroll_y(true)
        .scroll_strategy(ScrollStrategy::StickToBottom);
    main_layout.add_child(tv);

    let panel = Panel::new(main_layout)
        .title("Sample")
        .with_name("Sample");

    siv.add_layer(panel);
    siv.run();
}

Expected behavior
Should be redraw rows without a need / that often.

Screenshots
image

Environment

  • Operating system used: Linux 6.0.5-zen3-xanmod1-1
  • Backend used: termion
  • Current locale (run locale in a terminal): LANG=en_US.UTF-8
  • Cursive version: 0.20

Additional context

When cached, it obviously becomes much faster (https://github.com/night-crawler/cursive-lazy-text-view/blob/main/src/ltv.rs#L378). If such a caching is acceptable, I can open a PR, or implement a better idea and open a PR.

@gyscos
Copy link
Owner

gyscos commented Jul 7, 2023

Hi, and thanks for the report!

I know about the performance issues of LinearLayout, and am still not sure how to fix that.
Caching helps a bit, but as the nesting of LinearLayout gets deeper, you start to need exponentially more cache values :(

I'm not sure yet how to get a one-pass layout logic to avoid this. Might have to redesign a bit the entire layout system to surface more information. :^S

In the meantime, I'd be fine with some caching to improve the most common use-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants