From fc1aeaeccc55892577f807464370ee5f677ec33f Mon Sep 17 00:00:00 2001 From: Weber Grandizoli Date: Fri, 21 Aug 2026 15:13:54 -0300 Subject: [PATCH 1/3] fix: position editor lines by accumulated per-line heights when unwrapped normal_compute_screen_lines placed every visual line at index * its own line height, which is only correct when all lines share one height (the in-code TODO acknowledged this). With WrapMethod::None every buffer line is one visual line, so real heights can be summed without forcing text layouts: screen line positions, point<->line hit testing and the total document height now accumulate style.line_height per line. Wrapped editors keep the uniform-grid behaviour unchanged. --- src/views/editor/mod.rs | 115 +++++++++++++++++++++++++++++++++------ src/views/editor/view.rs | 2 +- 2 files changed, 99 insertions(+), 18 deletions(-) diff --git a/src/views/editor/mod.rs b/src/views/editor/mod.rs index 9786750df..b3b9b1c8f 100644 --- a/src/views/editor/mod.rs +++ b/src/views/editor/mod.rs @@ -772,6 +772,51 @@ impl Editor { self.style().line_height(self.id(), line) } + /// Whether line positions must be computed from real per-line heights. + /// Without wrapping every buffer line is one visual line, so the heights + /// come straight from the styling without forcing any text layout. + /// (With wrapping the editor still assumes a uniform line height.) + pub fn per_line_heights_active(&self) -> bool { + self.es + .with_untracked(|es| matches!(es.wrap_method(), WrapMethod::None)) + } + + /// The y of the top of `line`: the sum of every real line height above it. + /// Only exact under `WrapMethod::None`. + pub fn line_y_unwrapped(&self, line: usize) -> f64 { + let style = self.style(); + let edid = self.id(); + (0..line) + .map(|l| f64::from(style.line_height(edid, l))) + .sum() + } + + /// The line whose vertical span contains `y`, walking real heights. + /// Only exact under `WrapMethod::None`. + pub fn line_at_y_unwrapped(&self, y: f64, last_line: usize) -> usize { + let style = self.style(); + let edid = self.id(); + let mut acc = 0.0; + for l in 0..=last_line { + acc += f64::from(style.line_height(edid, l)); + if acc > y { + return l; + } + } + last_line + } + + /// The height of the whole document: real per-line heights when they are + /// exact, the uniform grid otherwise. + pub fn total_height(&self) -> f64 { + let last = self.last_vline().get(); + if self.per_line_heights_active() { + self.line_y_unwrapped(last + 1) + } else { + f64::from(self.line_height(0)) * (last + 1) as f64 + } + } + // === Line Information === /// Iterate over the visual lines in the view, starting at the given line. @@ -1042,24 +1087,30 @@ impl Editor { /// Get the actual (line, col) of a particular point within the editor. pub fn line_col_of_point_with_phantom(&self, point: Point) -> (usize, usize) { - let line_height = f64::from(self.style().line_height(self.id(), 0)); let info = if point.y <= 0.0 { Some(self.first_rvline_info()) } else { self.screen_lines .with_untracked(|sl| { sl.iter_line_info().find(|info| { - info.vline_y <= point.y && info.vline_y + line_height >= point.y + let h = f64::from( + self.style() + .line_height(self.id(), info.vline_info.rvline.line), + ); + info.vline_y <= point.y && info.vline_y + h >= point.y }) }) .map(|info| info.vline_info) }; let info = info.unwrap_or_else(|| { - for (y_idx, info) in self.iter_rvlines(false, RVLine::default()).enumerate() { - let vline_y = y_idx as f64 * line_height; - if vline_y <= point.y && vline_y + line_height >= point.y { + // Walk real heights: with variable line heights y_idx * h is wrong. + let mut acc_y = 0.0; + for info in self.iter_rvlines(false, RVLine::default()) { + let h = f64::from(self.style().line_height(self.id(), info.rvline.line)); + if acc_y <= point.y && acc_y + h >= point.y { return info; } + acc_y += h; } self.last_rvline_info() @@ -1088,25 +1139,30 @@ impl Editor { mode: Mode, point: Point, ) -> ((usize, usize), bool, CursorAffinity) { - // TODO: this assumes that line height is constant! - let line_height = f64::from(self.style().line_height(self.id(), 0)); let info = if point.y <= 0.0 { Some(self.first_rvline_info()) } else { self.screen_lines .with_untracked(|sl| { sl.iter_line_info().find(|info| { - info.vline_y <= point.y && info.vline_y + line_height >= point.y + let h = f64::from( + self.style() + .line_height(self.id(), info.vline_info.rvline.line), + ); + info.vline_y <= point.y && info.vline_y + h >= point.y }) }) .map(|info| info.vline_info) }; let info = info.unwrap_or_else(|| { - for (y_idx, info) in self.iter_rvlines(false, RVLine::default()).enumerate() { - let vline_y = y_idx as f64 * line_height; - if vline_y <= point.y && vline_y + line_height >= point.y { + // Walk real heights: with variable line heights y_idx * h is wrong. + let mut acc_y = 0.0; + for info in self.iter_rvlines(false, RVLine::default()) { + let h = f64::from(self.style().line_height(self.id(), info.rvline.line)); + if acc_y <= point.y && acc_y + h >= point.y { return info; } + acc_y += h; } self.last_rvline_info() @@ -1628,13 +1684,23 @@ pub fn normal_compute_screen_lines( ) -> ScreenLines { let lines = &editor.lines; let style = editor.style.get(); - // TODO: don't assume universal line height! + let variable_heights = editor.per_line_heights_active(); let line_height = style.line_height(editor.id(), 0); let (y0, y1) = base.with_untracked(|base| (base.active_viewport.y0, base.active_viewport.y1)); // Get the start and end (visual) lines that are visible in the viewport - let min_vline = VLine((y0 / line_height as f64).floor() as usize); - let max_vline = VLine((y1 / line_height as f64).ceil() as usize); + let (min_vline, max_vline) = if variable_heights { + let last = editor.last_vline().get(); + ( + VLine(editor.line_at_y_unwrapped(y0, last)), + VLine(editor.line_at_y_unwrapped(y1, last) + 1), + ) + } else { + ( + VLine((y0 / line_height as f64).floor() as usize), + VLine((y1 / line_height as f64).ceil() as usize), + ) + }; let cache_rev = editor.doc.get().cache_rev().get(); editor.lines.check_cache_rev(cache_rev); @@ -1666,14 +1732,29 @@ pub fn normal_compute_screen_lines( ) .take(count); + let mut acc_y = if variable_heights { + editor.line_y_unwrapped(min_vline.get()) + } else { + 0.0 + }; for (i, vline_info) in iter.enumerate() { rvlines.push(vline_info.rvline); let line_height = f64::from(style.line_height(editor.id(), vline_info.rvline.line)); - let y_idx = min_vline.get() + i; - let vline_y = y_idx as f64 * line_height; - let line_y = vline_y - vline_info.rvline.line_index as f64 * line_height; + let (vline_y, line_y) = if variable_heights { + // Without wrapping rvline.line_index is 0: the line starts here. + let y = acc_y; + acc_y += line_height; + (y, y) + } else { + let y_idx = min_vline.get() + i; + let vline_y = y_idx as f64 * line_height; + ( + vline_y, + vline_y - vline_info.rvline.line_index as f64 * line_height, + ) + }; // Add the information to make it cheap to get in the future. // This y positions are shifted by the baseline y0 diff --git a/src/views/editor/view.rs b/src/views/editor/view.rs index 5743317b6..48f52febd 100644 --- a/src/views/editor/view.rs +++ b/src/views/editor/view.rs @@ -400,7 +400,7 @@ impl EditorView { max_line_width.max(parent_size.width()) }; - let last_line_height = line_height * (editor.last_vline().get() + 1) as f64; + let last_line_height = editor.total_height(); let height = last_line_height; let margin_bottom = From 6f4fcbb9c7e568d04b54a5d9fc8335fbfd3af6f7 Mon Sep 17 00:00:00 2001 From: Weber Grandizoli Date: Fri, 21 Aug 2026 17:17:27 -0300 Subject: [PATCH 2/3] fix: position editor lines by real per-line heights, wrapped or not --- src/views/editor/gutter.rs | 2 +- src/views/editor/mod.rs | 202 +++++++++++++++++++++----------- src/views/editor/text.rs | 11 ++ src/views/editor/view.rs | 23 ++-- src/views/editor/visual_line.rs | 13 ++ 5 files changed, 174 insertions(+), 77 deletions(-) diff --git a/src/views/editor/gutter.rs b/src/views/editor/gutter.rs index 1f676e8fc..42e2a760a 100644 --- a/src/views/editor/gutter.rs +++ b/src/views/editor/gutter.rs @@ -225,7 +225,7 @@ impl EditorGutterView { // Height is determined by editor content let line_height = f64::from(editor.line_height(0)); - let last_line_height = line_height * (editor.last_vline().get() + 1) as f64; + let last_line_height = editor.total_height(); let margin_bottom = if editor.es.with_untracked(|es| es.scroll_beyond_last_line()) { let parent_size = editor.parent_size.get_untracked(); parent_size.height().min(last_line_height) - line_height diff --git a/src/views/editor/mod.rs b/src/views/editor/mod.rs index b3b9b1c8f..6037e7cec 100644 --- a/src/views/editor/mod.rs +++ b/src/views/editor/mod.rs @@ -772,48 +772,50 @@ impl Editor { self.style().line_height(self.id(), line) } - /// Whether line positions must be computed from real per-line heights. - /// Without wrapping every buffer line is one visual line, so the heights - /// come straight from the styling without forcing any text layout. - /// (With wrapping the editor still assumes a uniform line height.) + /// Whether line positions must be computed from real per-line heights, + /// because [`Styling::line_height`] varies from line to line. pub fn per_line_heights_active(&self) -> bool { - self.es - .with_untracked(|es| matches!(es.wrap_method(), WrapMethod::None)) + !self.style().uniform_line_height(self.id()) + } + + /// The full height of buffer `line`, every wrapped row included. + pub fn line_height_total(&self, line: usize) -> f64 { + f64::from(self.line_height(line)) * self.lines.cached_line_count(line) as f64 } /// The y of the top of `line`: the sum of every real line height above it. - /// Only exact under `WrapMethod::None`. - pub fn line_y_unwrapped(&self, line: usize) -> f64 { - let style = self.style(); - let edid = self.id(); - (0..line) - .map(|l| f64::from(style.line_height(edid, l))) - .sum() + pub fn line_y(&self, line: usize) -> f64 { + (0..line).map(|l| self.line_height_total(l)).sum() } - /// The line whose vertical span contains `y`, walking real heights. - /// Only exact under `WrapMethod::None`. - pub fn line_at_y_unwrapped(&self, y: f64, last_line: usize) -> usize { - let style = self.style(); - let edid = self.id(); - let mut acc = 0.0; - for l in 0..=last_line { - acc += f64::from(style.line_height(edid, l)); - if acc > y { - return l; - } - } - last_line + /// The y of the top of a visual line, wrapped rows included. + pub fn rvline_y(&self, rvline: RVLine) -> f64 { + self.line_y(rvline.line) + + rvline.line_index as f64 * f64::from(self.line_height(rvline.line)) + } + + /// The visual line whose vertical span contains `y`, walking real heights. + pub fn rvline_at_y(&self, y: f64) -> RVLine { + let heights = (0..=self.last_line()).map(|line| { + ( + f64::from(self.line_height(line)), + self.lines.cached_line_count(line), + ) + }); + row_at_y(heights, y) + .map(|(line, index, _)| RVLine::new(line, index)) + .unwrap_or_else(|| self.last_rvline()) } - /// The height of the whole document: real per-line heights when they are - /// exact, the uniform grid otherwise. + /// The height of the whole document: real per-line heights when they vary, + /// the uniform grid otherwise. pub fn total_height(&self) -> f64 { - let last = self.last_vline().get(); if self.per_line_heights_active() { - self.line_y_unwrapped(last + 1) + (0..=self.last_line()) + .map(|line| self.line_height_total(line)) + .sum() } else { - f64::from(self.line_height(0)) * (last + 1) as f64 + f64::from(self.line_height(0)) * (self.last_vline().get() + 1) as f64 } } @@ -1678,6 +1680,26 @@ fn create_view_effects(cx: Scope, ed: &Editor) { }); } +/// Walks lines given as `(height, rows)` until `y` falls inside one, and +/// returns that line, the row of it `y` landed on, and the y of the row's top. +/// `None` once the walk runs past the end of the document. +/// +/// Every row of a line shares that line's height — wrapping repeats a line, it +/// does not resize it. +fn row_at_y(lines: impl Iterator, y: f64) -> Option<(usize, usize, f64)> { + let mut acc = 0.0; + for (line, (height, rows)) in lines.enumerate() { + let total = height * rows as f64; + if acc + total > y { + let index = ((y - acc) / height).floor().max(0.0) as usize; + let index = index.min(rows.saturating_sub(1)); + return Some((line, index, acc + index as f64 * height)); + } + acc += total; + } + None +} + pub fn normal_compute_screen_lines( editor: &Editor, base: RwSignal, @@ -1688,67 +1710,67 @@ pub fn normal_compute_screen_lines( let line_height = style.line_height(editor.id(), 0); let (y0, y1) = base.with_untracked(|base| (base.active_viewport.y0, base.active_viewport.y1)); - // Get the start and end (visual) lines that are visible in the viewport - let (min_vline, max_vline) = if variable_heights { - let last = editor.last_vline().get(); - ( - VLine(editor.line_at_y_unwrapped(y0, last)), - VLine(editor.line_at_y_unwrapped(y1, last) + 1), - ) - } else { - ( - VLine((y0 / line_height as f64).floor() as usize), - VLine((y1 / line_height as f64).ceil() as usize), - ) - }; let cache_rev = editor.doc.get().cache_rev().get(); editor.lines.check_cache_rev(cache_rev); - let min_info = editor.iter_vlines(false, min_vline).next(); - let mut rvlines = Vec::new(); let mut info = HashMap::new(); - let Some(min_info) = min_info else { - return ScreenLines { - lines: Rc::new(rvlines), - info: Rc::new(info), - diff_sections: None, - base, + // Where the visible run starts, the y of its top, and how far to walk. + // With real heights the run ends at the first line past the viewport, so + // the count is open and the loop breaks on y instead. + let (start_rvline, mut acc_y, min_vline, count) = if variable_heights { + let start = editor.rvline_at_y(y0); + (start, editor.rvline_y(start), 0, usize::MAX) + } else { + // Get the start and end (visual) lines that are visible in the viewport + let min_vline = VLine((y0 / line_height as f64).floor() as usize); + let max_vline = VLine((y1 / line_height as f64).ceil() as usize); + + let Some(min_info) = editor.iter_vlines(false, min_vline).next() else { + return ScreenLines { + lines: Rc::new(rvlines), + info: Rc::new(info), + diff_sections: None, + base, + }; }; + + // TODO: the original was min_line..max_line + 1, are we iterating too little now? + // the iterator is from min_vline..max_vline + ( + min_info.rvline, + 0.0, + min_vline.get(), + max_vline.get() - min_vline.get(), + ) }; - // TODO: the original was min_line..max_line + 1, are we iterating too little now? - // the iterator is from min_vline..max_vline - let count = max_vline.get() - min_vline.get(); let iter = lines .iter_rvlines_init( editor.text_prov(), cache_rev, editor.config_id(), - min_info.rvline, + start_rvline, false, ) .take(count); - let mut acc_y = if variable_heights { - editor.line_y_unwrapped(min_vline.get()) - } else { - 0.0 - }; for (i, vline_info) in iter.enumerate() { - rvlines.push(vline_info.rvline); - let line_height = f64::from(style.line_height(editor.id(), vline_info.rvline.line)); let (vline_y, line_y) = if variable_heights { - // Without wrapping rvline.line_index is 0: the line starts here. + if acc_y >= y1 { + break; + } + // Every wrapped row of a line shares that line's height, so the top + // of the line is however many rows we are into it. let y = acc_y; acc_y += line_height; - (y, y) + (y, y - vline_info.rvline.line_index as f64 * line_height) } else { - let y_idx = min_vline.get() + i; + let y_idx = min_vline + i; let vline_y = y_idx as f64 * line_height; ( vline_y, @@ -1756,6 +1778,8 @@ pub fn normal_compute_screen_lines( ) }; + rvlines.push(vline_info.rvline); + // Add the information to make it cheap to get in the future. // This y positions are shifted by the baseline y0 info.insert( @@ -1827,3 +1851,49 @@ impl CursorInfo { self.blink(); } } + +#[cfg(test)] +mod tests { + use super::row_at_y; + + /// `(height, rows)` per line, the shape [`row_at_y`] walks. + const UNIFORM: [(f64, usize); 3] = [(10.0, 1), (10.0, 1), (10.0, 1)]; + /// A heading over two body lines: the case a uniform grid gets wrong. + const VARIABLE: [(f64, usize); 3] = [(30.0, 1), (20.0, 1), (20.0, 1)]; + /// A line wrapped onto three rows, then a plain one. + const WRAPPED: [(f64, usize); 2] = [(20.0, 3), (20.0, 1)]; + + fn at(lines: &[(f64, usize)], y: f64) -> Option<(usize, usize, f64)> { + row_at_y(lines.iter().copied(), y) + } + + #[test] + fn a_uniform_document_lands_where_division_would() { + assert_eq!(at(&UNIFORM, 0.0), Some((0, 0, 0.0))); + assert_eq!(at(&UNIFORM, 9.9), Some((0, 0, 0.0))); + assert_eq!(at(&UNIFORM, 10.0), Some((1, 0, 10.0))); + assert_eq!(at(&UNIFORM, 25.0), Some((2, 0, 20.0))); + } + + #[test] + fn a_tall_line_pushes_the_ones_under_it_down() { + assert_eq!(at(&VARIABLE, 29.9), Some((0, 0, 0.0))); + assert_eq!(at(&VARIABLE, 30.0), Some((1, 0, 30.0))); + assert_eq!(at(&VARIABLE, 55.0), Some((2, 0, 50.0))); + } + + #[test] + fn a_wrapped_line_owns_a_row_per_wrap() { + assert_eq!(at(&WRAPPED, 0.0), Some((0, 0, 0.0))); + assert_eq!(at(&WRAPPED, 25.0), Some((0, 1, 20.0))); + assert_eq!(at(&WRAPPED, 45.0), Some((0, 2, 40.0))); + assert_eq!(at(&WRAPPED, 60.0), Some((1, 0, 60.0))); + } + + #[test] + fn past_the_last_line_there_is_no_row() { + assert_eq!(at(&UNIFORM, 30.0), None); + assert_eq!(at(&WRAPPED, 80.0), None); + assert_eq!(at(&[], 0.0), None); + } +} diff --git a/src/views/editor/text.rs b/src/views/editor/text.rs index ea2d772d5..ff0a2b3ac 100644 --- a/src/views/editor/text.rs +++ b/src/views/editor/text.rs @@ -309,6 +309,17 @@ pub trait Styling { (1.5 * font_size).round().max(font_size) } + /// Whether every line of the editor has the same height. + /// + /// When this is true, the default, lines sit on a uniform grid and their + /// positions are arithmetic. Return false when [`Styling::line_height`] + /// varies from line to line — a markdown editor sizing its headings, say: + /// positions are then accumulated from the real heights, which costs a walk + /// over the lines above the viewport. + fn uniform_line_height(&self, _edid: EditorId) -> bool { + true + } + fn font_family(&self, _edid: EditorId, _line: usize) -> Cow<'_, [FamilyOwned]> { Cow::Borrowed(&[FamilyOwned::SansSerif]) } diff --git a/src/views/editor/view.rs b/src/views/editor/view.rs index 48f52febd..906d7c599 100644 --- a/src/views/editor/view.rs +++ b/src/views/editor/view.rs @@ -996,11 +996,10 @@ impl View for EditorView { let inner_node = self.inner_node.unwrap(); - // TODO: don't assume there's a constant line height let line_height = f64::from(editor.line_height(0)); let width = editor.max_line_width().max(parent_size.width()); - let last_line_height = line_height * (editor.last_vline().get() + 1) as f64; + let last_line_height = editor.total_height(); let height = last_line_height.max(parent_size.height()); let margin_bottom = if editor.es.with_untracked(|es| es.scroll_beyond_last_line()) { @@ -1471,14 +1470,18 @@ fn editor_content( let LineRegion { x, width, rvline } = cursor_caret(&editor, offset, !cursor.is_insert(), cursor.affinity()); - // TODO: don't assume line-height is constant - let line_height = f64::from(editor.line_height(0)); - - // TODO: is there a good way to avoid the calculation of the vline here? - let vline = editor.vline_of_rvline(rvline); - let rect = - Rect::from_origin_size((x, vline.get() as f64 * line_height), (width, line_height)) - .inflate(10.0, 1.0); + let (caret_y, line_height) = if editor.per_line_heights_active() { + ( + editor.rvline_y(rvline), + f64::from(editor.line_height(rvline.line)), + ) + } else { + let line_height = f64::from(editor.line_height(0)); + // TODO: is there a good way to avoid the calculation of the vline here? + let vline = editor.vline_of_rvline(rvline); + (vline.get() as f64 * line_height, line_height) + }; + let rect = Rect::from_origin_size((x, caret_y), (width, line_height)).inflate(10.0, 1.0); let viewport = viewport.get_untracked(); let smallest_distance = (viewport.y0 - rect.y0) diff --git a/src/views/editor/visual_line.rs b/src/views/editor/visual_line.rs index 5632f77b8..766332fcf 100644 --- a/src/views/editor/visual_line.rs +++ b/src/views/editor/visual_line.rs @@ -370,6 +370,19 @@ impl Lines { self.font_sizes.borrow().font_size(line) } + /// How many visual lines `line` takes up, read off its cached text layout. + /// + /// A line that has not been laid out yet counts as one, the same assumption + /// [`Lines::last_vline`] makes. + pub fn cached_line_count(&self, line: usize) -> usize { + let font_size = self.font_size(line); + self.text_layouts + .borrow() + .get(font_size, line) + .map(|layout| layout.line_count()) + .unwrap_or(1) + } + /// Get the last visual line of the file. /// /// Cached. From 14b253d7e7386a4930a257045ba121cf3c9ecb7f Mon Sep 17 00:00:00 2001 From: Weber Grandizoli Date: Fri, 21 Aug 2026 17:37:55 -0300 Subject: [PATCH 3/3] docs: changelog entry for the variable line height fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ffeed414..37a57bae7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- Position editor lines by their real heights when a `Styling` reports `uniform_line_height` as false, wrapped lines included [#1084](https://github.com/lapce/floem/pull/1084) - Use by default `std::sync::mpsc::channel` and place crossbeam behind the `crossbeam` feature [#775](https://github.com/lapce/floem/pull/775) ## [0.2.0] - 2024-11-13