From 648dd42810e4858aa313950c63abc5bf26252d44 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:12:11 +1100 Subject: [PATCH] fix(edit): a column is measured by its whole rows; an isolated row keeps its width Two holes in alignment_frame: - A column's right edge came from the runs that start at its left edge, so a row of several styles ended at its first run, and the column could fail to hold a title wider than that run. It now takes the right edge of each complete row (row_rect). - A row with no column to align in was framed by its own box, which the rewrite also used as the width its text wraps at: changing an isolated row's text together with an alignment wrapped it at its old width, word by word. When the text changes and no column holds the row, alignment_frame now returns None, so the row keeps the width it has without the alignment. With the text unchanged the own box is still the frame, so aligning twice is still a no-op. --- crates/edit/src/tests.rs | 35 ++++++++++++++++++++++++ crates/edit/src/text/rewrite.rs | 2 +- crates/edit/src/text_regions/grouping.rs | 29 ++++++++++++++------ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 48df3e4395..7691039f24 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -7255,6 +7255,41 @@ fn a_justified_paragraph_from_the_editor_fills_its_rows() { assert!(row("end")[2] < right - 20.0, "the last row isn't"); } +/// A column whose rows are several runs (a second style on the same baseline) reaches the right +/// edge of its whole rows, not of the run at the left edge: a title wider than that first run is +/// still held by the column and aligns to it (CodeRabbit on b9940d7d). +#[test] +fn a_column_is_measured_by_its_whole_rows_not_its_first_runs() { + let page = "BT /F1 18 Tf 1 0 0 1 100 672 Tm (A longer title) Tj ET BT /F1 10 Tf 12 TL 50 610 Td (The first column runs to here.) Tj ET BT /F1 11 Tf 1 0 0 rg 1 0 0 1 185 610 Tm (and keeps going on.) Tj ET BT /F1 10 Tf 12 TL 50 598 Td (A second row of it.) Tj ET"; + let blocks = text::text_blocks(&text_page(page), 0).unwrap(); + let body = blocks.iter().find(|b| b.text.starts_with("The first")).expect("the body is a block"); + assert!(body.rows.first().is_some_and(|row| row.len() >= 2), "the body's first row is several runs: {:?}", body.rows); + let lines = text::text_lines(&text_page(page), 0).unwrap(); + let column_right = + body.rows.iter().filter_map(|row| row.iter().filter_map(|i| lines.get(*i)).map(|l| l.rect[2]).reduce(f64::max)).fold(f64::MIN, f64::max); + let mut doc = text_page(page); + let index = blocks.iter().position(|b| b.text == "A longer title").expect("the title is a block of its own"); + let style = text::BlockStyle { aligns: vec![Some(added::Align::Right)], ..Default::default() }; + text::rewrite_block(&mut doc, 0, index, Some("A longer title"), &style).unwrap(); + let title = text::text_lines(&doc, 0).unwrap().into_iter().find(|l| l.text == "A longer title").unwrap().rect; + assert!((title[2] - column_right).abs() < 1.0, "right: on the whole row's right edge, {title:?} vs {column_right}"); +} + +/// A row with no column of its own is aligned in its own box, but that box is not the width its new +/// text wraps at: changing the text together with an alignment keeps a single row on one row, as +/// it does without the alignment (CodeRabbit on b9940d7d). +#[test] +fn an_isolated_row_edited_with_an_alignment_does_not_wrap_at_its_old_width() { + for align in [added::Align::Center, added::Align::Right] { + let mut doc = text_page("BT /F1 18 Tf 1 0 0 1 100 700 Tm (Title) Tj ET"); + let longer = "A much longer title for the page"; + let style = text::BlockStyle { aligns: vec![Some(align)], ..Default::default() }; + text::rewrite_block(&mut doc, 0, 0, Some(longer), &style).unwrap(); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.iter().map(|l| l.text.as_str()).collect::>(), [longer], "{align:?}: the title stays on one row"); + } +} + /// A heading across the whole page sits closer to the title than the title's own column does: the /// title is still aligned in its own column, not across the heading's page-wide frame (FredApps's /// review of 43f848d5). Centre and right both go by the column. diff --git a/crates/edit/src/text/rewrite.rs b/crates/edit/src/text/rewrite.rs index 4d4e16d669..3f436d7b24 100644 --- a/crates/edit/src/text/rewrite.rs +++ b/crates/edit/src/text/rewrite.rs @@ -766,7 +766,7 @@ pub(crate) fn rewrite_block_from( // An alignment for a one-row paragraph is laid out across the column it sits in, as a // paragraph of that width moved to its left edge. let framed; - let style = match alignment_frame(&lines, &blocks, &b, style) { + let style = match alignment_frame(&lines, &blocks, &b, style, text.is_some_and(|t| t != b.editor_text)) { Some([left, right]) => { framed = BlockStyle { width: Some(right - left), offset: Some([left - b.rect[0], 0.0]), ..style.clone() }; &framed diff --git a/crates/edit/src/text_regions/grouping.rs b/crates/edit/src/text_regions/grouping.rs index 2e44290117..3a16e4f7c0 100644 --- a/crates/edit/src/text_regions/grouping.rs +++ b/crates/edit/src/text_regions/grouping.rs @@ -49,9 +49,15 @@ fn neighbour_left(lines: &[TextLine], block: &TextBlock) -> Option { /// is wider than the column the title sits in, and gives it no frame while a narrower one does. A lone row's own box is no frame /// to align in: left-aligning a title centred on the page left it where it was, and since a single /// row grows to the page's margin, centring it again moved it right each time. `None` when no -/// alignment is asked for, or when the paragraph is several rows, turned, or given a width or a -/// move of its own. -pub(crate) fn alignment_frame(lines: &[TextLine], blocks: &[TextBlock], block: &TextBlock, style: &BlockStyle) -> Option<[f64; 2]> { +/// alignment is asked for, when the paragraph is several rows, turned, or given a width or a move of +/// its own, or when its text changes and no column holds it (`text_changed`). +pub(crate) fn alignment_frame( + lines: &[TextLine], + blocks: &[TextBlock], + block: &TextBlock, + style: &BlockStyle, + text_changed: bool, +) -> Option<[f64; 2]> { let asked = style.align.is_some() || style.aligns.iter().any(Option::is_some); if !asked || style.width.is_some() || style.offset.is_some() || block.rows.len() != 1 || block.rotation.abs() > 0.01 { return None; @@ -62,12 +68,13 @@ pub(crate) fn alignment_frame(lines: &[TextLine], blocks: &[TextBlock], block: & // The column an overlapping paragraph makes, when it holds the row. let column = |other: &TextBlock| -> Option<[f64; 2]> { let left = other.rect[0]; + // Whole rows, not their runs: a row of several styles reaches its last run's right edge. let right = other - .lines + .rows .iter() - .filter_map(|i| lines.get(*i)) - .filter(|line| (line.rect[0] - left).abs() < tolerance) - .map(|line| line.rect[2]) + .filter_map(|row| row_rect(row, lines)) + .filter(|row| (row[0] - left).abs() < tolerance) + .map(|row| row[2]) .fold(f64::NEG_INFINITY, f64::max); (right.is_finite() && right - left > own[1] - own[0] && own[0] >= left - tolerance && own[1] <= right + tolerance).then_some([left, right]) }; @@ -77,7 +84,13 @@ pub(crate) fn alignment_frame(lines: &[TextLine], blocks: &[TextBlock], block: & .filter_map(|other| column(other).map(|frame| (frame, gap(other)))) .min_by(|(a, near_a), (b, near_b)| (a[1] - a[0]).total_cmp(&(b[1] - b[0])).then(near_a.total_cmp(near_b))) .map(|(frame, _)| frame); - Some(frame.unwrap_or(own)) + // The row's own box, with no column to align in, is no width for new text to wrap at: edited, + // the row keeps the width it would have had without the alignment. + match frame { + Some(frame) => Some(frame), + None if text_changed => None, + None => Some(own), + } } /// Whether a block reads as centred, and so must keep its centre when it grows or wraps.