-
Notifications
You must be signed in to change notification settings - Fork 1.7k
epaint: Break up memozation of text layouts into paragraphs #4000
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
base: main
Are you sure you want to change the base?
Conversation
My two cents: The current Paragraph struct is misnamed, as it refers to a block of text that will be split into lines, not a typographic paragraph "/n/n". TextBlock would be a more appropriate name. When generating the TextBlocks, only the X positioning of glyphs is calculated. The Y positions are ignored at this stage. Line breaks happen later when converting TextBlocks to Rows, based on the wrap width. A TextBlock can span multiple Rows if it exceeds the wrap width. To properly position the wrapped Rows vertically, each TextBlock needs a paragraph index that is incremented for each one. When generating the Rows in line_break(), the paragraph (TextBlock ) index can be used to increment the Y position, so Rows from the same TextBlock are offset vertically. This prevents the issue of Rows overlapping each other at the same Y position when breaking a TextBlock into multiple Rows. So in summary, the TextBlocks are intermediate storage of positioned glyphs before line wrapping. The line b 8000 reaks and Y positioning happen in a later stage when converting TextBlocks to Rows. |
Huh, it says your comment was posted on the 10th, but I've been regularly checking this PR and it didn't appear for me until today. Weird. Anyway, it looks like all use of the My main question is: what is the frame of reference for the rendering positions inside a |
Maybe instead of using egui/crates/epaint/src/text/text_layout.rs Lines 68 to 122 in 3672b15
|
I dont get the solution, may be my problem is different than solving here. I want to show BIG 2 megabytes text, so why just dont virtualize lines, and make user pass big text as And make selection api as Range<(row, col)> |
## What (written by @emilk) When editing long text (thousands of line), egui would previously re-layout the entire text on each edit. This could be slow. With this PR, we instead split the text into paragraphs (split on `\n`) and then cache each such paragraph. When editing text then, only the changed paragraph needs to be laid out again. Still, there is overhead from splitting the text, hashing each paragraph, and then joining the results, so the runtime complexity is still O(N). In our benchmark, editing a 2000 line string goes from ~8ms to ~300 ms, a speedup of ~25x. In the future, we could also consider laying out each paragraph in parallel, to speed up the initial layout of the text. ## Details This is an ~~almost complete~~ implementation of the approach described by emilk [in this comment](<#3086 (comment)>), excluding CoW semantics for `LayoutJob` (but including them for `Row`). It supersedes the previous unsuccessful attempt here: #4000. Draft because: - [X] ~~Currently individual rows will have `ends_with_newline` always set to false. This breaks selection with Ctrl+A (and probably many other things)~~ - [X] ~~The whole block for doing the splitting and merging should probably become a function (I'll do that later).~~ - [X] ~~I haven't run the check script, the tests, and haven't made sure all of the examples build (although I assume they probably don't rely on Galley internals).~~ - [x] ~~Layout is sometimes incorrect (missing empty lines, wrapping sometimes makes text overlap).~~ - A lot of text-related code had to be changed so this needs to be properly tested to ensure no layout issues were introduced, especially relating to the now row-relative coordinate system of `Row`s. Also this requires that we're fine making these very breaking changes. It does significantly improve the performance of rendering large blocks of text (if they have many newlines), this is the test program I used to test it (adapted from <#3086>): <details> <summary>code</summary> ```rust use eframe::egui::{self, CentralPanel, TextEdit}; use std::fmt::Write; fn main() -> Result<(), eframe::Error> { let options = eframe::NativeOptions { ..Default::default() }; eframe::run_native( "editor big file test", options, Box::new(|_cc| Ok(Box::<MyApp>::new(MyApp::new()))), ) } struct MyApp { text: String, } impl MyApp { fn new() -> Self { let mut string = String::new(); for line_bytes in (0..50000).map(|_| (0u8..50)) { for byte in line_bytes { write!(string, " {byte:02x}").unwrap(); } write!(string, "\n").unwrap(); } println!("total bytes: {}", string.len()); MyApp { text: string } } } impl eframe::App for MyApp { fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { CentralPanel::default().show(ctx, |ui| { let start = std::time::Instant::now(); egui::ScrollArea::vertical().show(ui, |ui| { let code_editor = TextEdit::multiline(&mut self.text) .code_editor() .desired_width(f32::INFINITY) .desired_rows(40); let response = code_editor.show(ui).response; if response.changed() { println!("total bytes now: {}", self.text.len()); } }); let end = std::time::Instant::now(); let time_to_update = end - start; if time_to_update.as_secs_f32() > 0.5 { println!("Long update took {:.3}s", time_to_update.as_secs_f32()) } }); } } ``` </details> I think the way to proceed would be to make a new type, something like `PositionedRow`, that would wrap an `Arc<Row>` but have a separate `pos` ~~and `ends_with_newline`~~ (that would mean `Row` only holds a `size` instead of a `rect`). This type would of course have getters that would allow you to easily get a `Rect` from it and probably a `Deref` to the underlying `Row`. ~~I haven't done this yet because I wanted to get some opinions whether this would be an acceptable API first.~~ This is now implemented, but of course I'm still open to discussion about this approach and whether it's what we want to do. Breaking changes (currently): - The `Galley::rows` field has a different type. - There is now a `PlacedRow` wrapper for `Row`. - `Row` now uses a coordinate system relative to itself instead of the `Galley`. * Closes <#3086> * [X] I have followed the instructions in the PR template --------- Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Draft for now, I just want to confirm that I'm on the right track for the general logic, and make sure I know about the edge cases when splitting
LayoutJob
s and mergingGalley
s. It looks like I will need to:LayoutSection
s go into eachLayoutJob
, and change their offsetsLayoutJob
ends with a newlineGalley
s, merge therect
s andmesh_bounds
of eachGalley
together, and possibly offset them?elided
field makes it onto the fullGalley
if set on any paragraph, and stop adding the followingGalley
s if that field is setnum_vertices
andnum_indices
The code compiles right now, but does not pass
cargo cranky
and panics immediately when that codepath is run (byte index out of bounds, probably due to not filtering and offsettingLayoutSection
s. Also thetodo!()
.)TextEdit
#3086