From b9940d7d8104763d5e7106543dc6b41acce942cd Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:38:05 +1100 Subject: [PATCH] fix(edit): a lone row aligns in its own column, not a closer page-wide heading alignment_frame measured a one-row paragraph's alignment in the frame of the nearest overlapping block. A full-width heading closer to the title than the title's own body column gave it a page-wide frame, so centring or right-aligning the title moved it out of its column. It now considers every overlapping block whose column holds the row and takes the narrowest, the nearest on a tie, so a page-wide heading gives the title no frame while a narrower column that holds it does. When no column holds the row it is still measured in its own box. The comment about page-wide headers named an exclusion the filter never made; the doc comment now says what is chosen and why. --- crates/edit/src/tests.rs | 27 +++++++++++++++++ crates/edit/src/text_regions/grouping.rs | 37 ++++++++++++++---------- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index be82ae144d..48df3e4395 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -7255,6 +7255,33 @@ fn a_justified_paragraph_from_the_editor_fills_its_rows() { assert!(row("end")[2] < right - 20.0, "the last row isn't"); } +/// 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. +#[test] +fn a_lone_row_aligns_in_its_own_column_not_a_closer_full_width_heading() { + let page = "BT /F1 12 Tf 1 0 0 1 50 700 Tm (A heading that runs across both columns of the page, well past the first column, to the far margin.) Tj ET BT /F1 18 Tf 1 0 0 1 120 672 Tm (Title) Tj ET BT /F1 10 Tf 12 TL 50 610 Td (The first column runs to here.) Tj T* (A second row of it.) Tj ET BT /F1 10 Tf 12 TL 320 610 Td (The second column has more rows.) Tj T* (One.) Tj T* (Two.) Tj T* (Three.) Tj ET"; + let blocks = text::text_blocks(&text_page(page), 0).unwrap(); + let column = blocks.iter().find(|b| b.text.starts_with("The first")).unwrap().rect; + let heading = blocks.iter().find(|b| b.text.starts_with("A heading")).unwrap().rect; + let title_block = blocks.iter().find(|b| b.text == "Title").expect("the title is a block of its own").rect; + let gap = |other: [f64; 4]| (other[1] - title_block[3]).max(title_block[1] - other[3]).max(0.0); + assert!(gap(heading) < gap(column), "the heading is the nearer block: {} vs {}", gap(heading), gap(column)); + assert!(heading[2] - heading[0] > 2.0 * (column[2] - column[0]), "the heading is page-wide: {heading:?} vs {column:?}"); + let title_after = |align: added::Align| { + let mut doc = text_page(page); + let blocks = text::text_blocks(&doc, 0).unwrap(); + let index = blocks.iter().position(|b| b.text == "Title").unwrap(); + let style = text::BlockStyle { aligns: vec![Some(align)], ..Default::default() }; + text::rewrite_block(&mut doc, 0, index, Some("Title"), &style).unwrap(); + text::text_lines(&doc, 0).unwrap().into_iter().find(|l| l.text == "Title").unwrap().rect + }; + let centred = title_after(added::Align::Center); + assert!(((centred[0] + centred[2]) / 2.0 - (column[0] + column[2]) / 2.0).abs() < 1.0, "centred in its own column: {centred:?} vs {column:?}"); + let right = title_after(added::Align::Right); + assert!((right[2] - column[2]).abs() < 1.0, "right: on its own column's right edge, {right:?} vs {column:?}"); +} + /// The same on a page of two columns, where the other column has more rows: a title over the /// first column is aligned across the first column, not its own box. #[test] diff --git a/crates/edit/src/text_regions/grouping.rs b/crates/edit/src/text_regions/grouping.rs index 357f1f7244..2e44290117 100644 --- a/crates/edit/src/text_regions/grouping.rs +++ b/crates/edit/src/text_regions/grouping.rs @@ -43,8 +43,10 @@ fn neighbour_left(lines: &[TextLine], block: &TextBlock) -> Option { } /// Where an alignment picked for a one-row paragraph is measured, as `[left, right]`: the column of -/// text it sits in (the nearest overlapping paragraph's left edge, to the furthest right -/// its rows reach) when that column holds it, otherwise its own box. A lone row's own box is no frame +/// text it sits in (an overlapping paragraph's left edge, to the furthest right its rows reach) +/// when that column holds it, otherwise its own box. Of the columns that hold it, the narrowest is +/// its own, the nearest on a tie: a heading across the whole page, even one closer than the body, +/// 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 @@ -57,22 +59,25 @@ pub(crate) fn alignment_frame(lines: &[TextLine], blocks: &[TextBlock], block: & let own = [block.rect[0], block.rect[2]]; let tolerance = block.size.max(1.0) * 0.5; let gap = |other: &TextBlock| (other.rect[1] - block.rect[3]).max(block.rect[1] - other.rect[3]).max(0.0); - // Page-wide headers and footers sharing this margin aren't part of the local column. - let neighbour = blocks + // The column an overlapping paragraph makes, when it holds the row. + let column = |other: &TextBlock| -> Option<[f64; 2]> { + let left = other.rect[0]; + let right = other + .lines + .iter() + .filter_map(|i| lines.get(*i)) + .filter(|line| (line.rect[0] - left).abs() < tolerance) + .map(|line| line.rect[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]) + }; + let frame = blocks .iter() .filter(|other| other.lines.iter().all(|i| !block.lines.contains(i)) && other.rect[2] > own[0] && other.rect[0] < own[1]) - .min_by(|a, b| gap(a).total_cmp(&gap(b))); - let Some(neighbour) = neighbour else { return Some(own) }; - let left = neighbour.rect[0]; - let right = neighbour - .lines - .iter() - .filter_map(|i| lines.get(*i)) - .filter(|line| (line.rect[0] - left).abs() < tolerance) - .map(|line| line.rect[2]) - .fold(f64::NEG_INFINITY, f64::max); - let holds = right.is_finite() && right - left > own[1] - own[0] && own[0] >= left - tolerance && own[1] <= right + tolerance; - Some(if holds { [left, right] } else { own }) + .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)) } /// Whether a block reads as centred, and so must keep its centre when it grows or wraps.