egui
egui copied to clipboard
emilk
Problem with `Table` from egui_extras
Describe the bug
I recently changed to latest version of egui 0.20.1 from 0.19 in one of my projects.
the problem that I noticed so far is the Table wont work as it use to work in version 0.19, when trying to resize or make the windows full screen the table layout will fill up the entire Panel and push out anything that is beside it.
it would be better to show the problem with a video, I will attach some video in screenshot section.
To Reproduce
here is the an example source code, just compile and try to resize it...
use eframe::{NativeOptions, epaint::{Vec2, text::{LayoutJob, LayoutSection, TextWrapping}}, run_native, App, egui::{CentralPanel, Direction, Layout, TextFormat, SidePanel}, emath::Align};
use egui_extras::{TableBuilder, Column};
struct Test {
test_data: Vec<[String; 3]>,
}
impl Default for Test {
fn default() -> Self {
let mut test_data = Vec::with_capacity(1000);
for i in 0..1000 {
test_data.push([
"First one".to_owned(),
"Second one".to_owned(),
format!("{i}: The last one that should be looooooooooooooooooooooooooooooooooooong"),
]);
}
Self { test_data }
}
}
impl App for Test {
fn update(&mut self, ctx: &eframe::egui::Context, _frame: &mut eframe::Frame) {
SidePanel::right("side_panel")
.default_width(225.0)
.max_width(225.0)
.resizable(false)
.show(ctx, |ui| {
ui.label("just a simple side panel");
});
CentralPanel::default().show(ctx, |ui| {
TableBuilder::new(ui)
.striped(true)
.column(Column::initial(50.0).at_least(50.0).at_most(50.0))
.column(Column::initial(150.0).at_least(50.0).at_most(160.))
.column(Column::remainder().at_least(60.0))
.cell_layout(Layout::centered_and_justified(Direction::LeftToRight))
.resizable(true)
.header(20.0, |mut header| {
for text in ["One", "Two", "Long Text"] {
header.col(|ui| {
ui.heading(text);
});
}
})
.body(|body| {
body.rows(25.0, 1000, |row_idx, mut row| {
row.col(|ui| {
ui.label(&self.test_data[row_idx][0]);
});
row.col(|ui| {
ui.label(&self.test_data[row_idx][1]);
});
row.col(|ui| {
let job = LayoutJob {
text: self.test_data[row_idx][2].clone(),
sections: vec![LayoutSection {
leading_space: 0.0,
byte_range: 0..self.test_data[row_idx][2].len(),
format: TextFormat::default(),
}],
wrap: TextWrapping {
max_rows: 1,
break_anywhere: true,
overflow_character: Some('…'),
..Default::default()
},
break_on_newline: true,
halign: Align::Center,
..Default::default()
};
ui.centered_and_justified(|ui| {
ui.label(job);
});
});
})
})
});
}
}
fn main() {
let options = NativeOptions {
initial_window_size: Some(Vec2::new(680., 535.)),
min_window_size: Some(Vec2::new(680., 535.)),
..Default::default()
};
run_native(
"Test",
options,
Box::new(|_| Box::new(Test::default())));
}
(for version 0.19.0 just changing Column to Size is enough.
Expected behavior
I believe it should work as it been working in version 0.19.0
Screenshots and Videos
version 0.19
https://user-images.githubusercontent.com/18105381/206941941-046ba4a0-74ca-4155-a1f3-6da2da3f982c.mp4
here is the same program in version 0.20.1
https://user-images.githubusercontent.com/18105381/206942039-2bbcd901-a598-4586-b2a3-cc358f0693aa.mp4
Desktop (please complete the following information):
- OS: Windows
- Version: 10 (21H2)
Good find! Could you please make a minimal reproducing example?
Thank you for the fast respond.
the code that I sent in the first post To Reproduce section is fully working, just create a new cargo project with eframe and egui-extras as dependencies and then replace the source... that should be it.
@emilk I believe that is probably the same layouting bug I discovered during our investigation here https://github.com/emilk/egui/issues/2425#issuecomment-1347371420
So I will not open another issue. For me the table will not exapnd past a certain maximum vertical size when in the CentralPanel, but seeing this there is probably just a general glitch in the layouting code.
@Detector-I in your example, in the new version, can you also test what happens when you use the 'wrap' property on the labels inside the cells.
In 0.19.0 this is very fast for me, in 0.20.0 it slow to a crawl and is very stuttery.
I played the old git bisect game again and both the column resize performance issue as well as the layout problem when resizing the window boils down to this commit, which unfortunately is quite large^^
git bisect bad
2dc2a5540d7968ad8b1926468431c51e52e28c5e is the first bad commit
commit 2dc2a5540d7968ad8b1926468431c51e52e28c5e
Author: Emil Ernerfeldt <[email protected]>
Date: Wed Nov 30 19:56:06 2022 +0100
`egui_extras::Table` improvements (#2369)
* Use simple `ui.interact` for the resize line
* Introduce TableReizeState
* Simplify some code
* Add striped options to table demo
* Auto-size table columns by double-clicking the resize line
* Table: add option to auto-size the columns
* Table: don't let column width gets too small, unless clipping is on
* egui_extras: always use serde
Otherwise using `get_persisted` etc is impossible,
and working around that tedious.
* Avoid clipping last column in a resizable table
* Some better naming
* Table: Use new `Column` for setting column sizes and properties
Also make `clip` a per-column property
* All Table:s store state for auto-sizing purposes
* Customize each column wether or not it is resizable
* fix some auto-sizing bugs
* Fix shrinkage of adaptive column content
* Rename `scroll` to `vscroll` for clarity
* Add Table::scroll_to_row
* scroll_to_row takes alignment
* Fix bug in table sizing
* Strip: turn clipping OFF by default, because it is dangerous and sucks
* Add TableBody::mac_rect helper
* Table: add options to control the scroll area height.
* Docstring fixes
* Cleanup
crates/egui/src/containers/scroll_area.rs | 18 +-
crates/egui_demo_app/Cargo.toml | 4 +-
crates/egui_demo_lib/src/demo/table_demo.rs | 125 +++--
crates/egui_extras/Cargo.toml | 8 +-
crates/egui_extras/src/datepicker/button.rs | 3 +-
crates/egui_extras/src/datepicker/popup.rs | 15 +-
crates/egui_extras/src/layout.rs | 66 +--
crates/egui_extras/src/strip.rs | 27 +-
crates/egui_extras/src/table.rs | 791 +++++++++++++++++++++-------
9 files changed, 737 insertions(+), 320 deletions(-)
Full bisect log:
git bisect log
git bisect start
# status: waiting for both good and bad commits
# bad: [7d91e904810215952e7d9fcdb892c1e1e227f2ff] Release 0.20.0 - AccessKit, prettier text, overlapping widgets
git bisect bad 7d91e904810215952e7d9fcdb892c1e1e227f2ff
# status: waiting for good commit(s), bad commit known
# good: [97ce10320980ea4e8f415b000b322881eb5ec51f] Release 0.19.0 - wgpu backend, repaint_after, continue-after-close
git bisect good 97ce10320980ea4e8f415b000b322881eb5ec51f
# good: [8e79a5a8ae32daac964b6e4d073513bb596d5f96] Add egui_speedy2d link to README.md (#2211)
git bisect good 8e79a5a8ae32daac964b6e4d073513bb596d5f96
# good: [7d8154971bae6df8af00e0e19c62ef852d223091] Update ndk-sys v0.4.0 -> v0.4.1+23.1.7779620 (#2340)
git bisect good 7d8154971bae6df8af00e0e19c62ef852d223091
# bad: [8eb687cf04dd2b68dfe467d142db6b8759425e16] Glutin Upgrade (#2187)
git bisect bad 8eb687cf04dd2b68dfe467d142db6b8759425e16
# bad: [7133818c59269fee8cff8d182e776bda8858a727] Make sure scroll bars are always visible (#2371)
git bisect bad 7133818c59269fee8cff8d182e776bda8858a727
# good: [a3f1e5961f66d2c0acdf5575eb058eb352d5f6d4] Fix bug in keyboard shortcut formatting
git bisect good a3f1e5961f66d2c0acdf5575eb058eb352d5f6d4
# good: [0336816faf9f361e59ca77f99c19fcfa2bf7c993] Fix keyboard support in `DragValue` (#2342)
git bisect good 0336816faf9f361e59ca77f99c19fcfa2bf7c993
# bad: [85f8eeb9d5ca4be14c99740ac31052a9d246c773] Fix key pressed event (#2334)
git bisect bad 85f8eeb9d5ca4be14c99740ac31052a9d246c773
# bad: [2dc2a5540d7968ad8b1926468431c51e52e28c5e] `egui_extras::Table` improvements (#2369)
git bisect bad 2dc2a5540d7968ad8b1926468431c51e52e28c5e
# first bad commit: [2dc2a5540d7968ad8b1926468431c51e52e28c5e] `egui_extras::Table` improvements (#2369)
I think the code from @Detector-I above is actually enough to show the issue.
It might sound stupid, but how do I git bisect squashed commits of a PR? never done that. Is is bisectable in a sense that each commit is good enough so it actually works?
I think the code from @Detector-I above is actually enough to show the issue.
it is good enough to show the issue, but the point of a minimal reproduce it it cuts to the core of the problem, revealing the very essence of it, and cuts out a lot of overhead and noise and time when stepping it through with a debugger.
So I went through and was able to make it a little simpler. You can find the demonstrator for your own testing here.
https://github.com/markusdd/egui-table-issue
The issues I see are these:
- table does net extend beyond a certain length
- stuff right of the table gets pushed out on resize
- makeing an existing column smaller with wrapping text is extremly sluggish
@emilk
it is good enough to show the issue, but the point of a minimal reproduce it it cuts to the core of the problem, revealing the very essence of it, and cuts out a lot of overhead and noise and time when stepping it through with a debugger.
Oh I understand now... here is the smallest minimal reproducing example code that I can create that have the same problems as first post.
use eframe::{App, run_native, NativeOptions, egui::{CentralPanel, SidePanel, Direction, Layout}};
use egui_extras::{TableBuilder, Column};
struct Example;
impl App for Example {
fn update(&mut self, ctx: &eframe::egui::Context, _frame: &mut eframe::Frame) {
SidePanel::right("id")
.show(ctx, |ui| {
ui.label("text");
});
CentralPanel::default()
.show(ctx, |ui| {
TableBuilder::new(ui)
.cell_layout(Layout::centered_and_justified(Direction::LeftToRight))
.column(Column::remainder())
.body(|mut body| {
body.row(25.0, |mut row| {
row.col(|ui| {
ui.label("add_cell_contents");
});
});
})
});
}
}
fn main() {
run_native(
"minimal reproducing example",
NativeOptions::default(),
Box::new(|_| Box::new(Example))
)
}
https://user-images.githubusercontent.com/17937017/207747912-6d7e494f-c531-4248-adb1-a7a9f631f6d9.mp4
Some important notes:
- I choose to remove
resizable(true),striped(true), andheaderfrom theTableBuilderbecause after checking it without them I find out its still having the same problem; also I find out just a single column that haveColumn::remainderis enough for getting the problem, and at the end I tried to remove thecell_layoutbut by removing it I couldn't see the problem anymore. - I still used
SidePanelfor having another widget besideTablebut as you can see I removed some of the methods that I used in last code, because after checking them I find out using them didn't have anything todo with the problem. - I did get the problem that @markusdd mentioned with a little change in code (just by setting
resizabletotrueand having more then one column), but for sake of simplicity I leave it out for now.
Hope this one helps
I don't have time to dig into this right now, but feel free to investigate yourself!
I did some investigation and found the cause for this bug.
The problem occurs when the last column is a remainder and it allocates the full space available. This can happen when the layout is centered_and_justified or you call ui.allocate_rect(ui.available_rect_before_wrap(), Sense::click()); in the last column.
You can check this problem in the Table Demo if you use .cell_layout(Layout::centered_and_justified(Direction::LeftToRight)).
- When you expand the table the, last column will allocate the full width available, and this will be saved in the
TableState - If we shrink the table, the saved size from before will be used and the column will allocate the same size as before
- We store that size again even if the available space is not enough (we will scroll or clip)
The code that makes this happens is this, it stores the remaining available space or the largest column size for the last column:
if is_last_column && column.initial_width == InitialColumnSize::Remainder {
// If the last column is 'remainder', then let it fill the remainder!
let eps = 0.1; // just to avoid some rounding errors.
*column_width = available_width - eps;
*column_width = column_width.at_least(max_used_widths[i]);
*column_width = column_width.clamp(min_width, max_width);
break;
}
When the last column does not allocate the full available space this problem does not occur because
column_width = available_width - eps resets the size and the max_used_widths[i] will be the actual size used by the ui.
I tried fixing this but couldn't find a solution that didn't cause other bugs :/ Not sure if you have an idea on how this could be fixed @emilk
I am using egui/egui_extras 0.22, the "table does not extend beyond a certain length" issue is due to the default value of TableScrollOptions:
impl Default for TableScrollOptions {
fn default() -> Self {
Self {
vscroll: true,
stick_to_bottom: false,
scroll_to_row: None,
scroll_offset_y: None,
min_scrolled_height: 200.0,
max_scroll_height: 800.0,
auto_shrink: [true; 2],
}
}
}
If you don't want a max_scroll_height (for example because you're already putting the table in a bounded container) you can fix this by setting max_scroll_height to infinity when building the Table, for example:
TableBuilder::new(ui)
.column(Column::auto())
.column(Column::auto())
.max_scroll_height(f32::INFINITY)
.header(16.0, |mut header| {
header.col(|ui| { ui.strong("Count"); });
header.col(|ui| { ui.strong("Test"); });
})
.body(|mut body| { ... });
I think the 'proper' fix for this would be for the max_scroll_height (and min_scroll_height?) values in in the TableScrollOptions to be Optional and default to None unless the max_scroll_height builder function is called. It is somewhat surprising that there is a maximum height by default.
@emilk I can open a PR to make this change if you don't have any immediate concerns with it. I have tested this locally and it seems to work as expected.
