Layout: Margins and Columns keeps dragged columns, which scale with the margin box (#1096) (#1172)

* IDML: keep column guides that were dragged off centre (#1096)

InDesign records dragged column guides in `MarginPreference ColumnsPositions`,
as pairs of offsets from the margin box's left edge. The importer discarded them
and divided the columns evenly, so a page whose gutter the designer had moved
came back with it in the middle: on a 450 pt margin box, `0 150 170 450` was
read as columns at 215 and 235.

`Columns::positions` already existed, documented as "Custom column positions
(after dragging column guides)", but nothing set it and nothing read it. Now
`Page::column_rects` returns the custom rects when they are set — that one
method is what the canvas guides, snapping, threading and the first frame of a
placed story all go through, so they follow the dragged gutter — the importer
fills it, and the exporter writes the page's real positions instead of
recomputing an even split, so the round trip keeps them.

`layout.marginsAndColumns` clears them when a count or gutter is set, which is
what InDesign's Margins and Columns does. Without that the stale positions would
outrank the new value and the dialog would look broken.

Verified against InDesign-authored IDML rather than assumed, which settled two
things. The attribute is written whether or not anything was dragged — a real
page's values are exactly its even split — so an even set is dropped rather than
marking every imported page custom and freezing its columns; the tolerance for
that comparison only absorbs float noise, about 1e-14 pt on that page. And
`<Guide>` is a `<Page>` child, not a `<Spread>` one, which is where this
importer already looks: the reporter's missing ruler guide was 099fbd2, landed
the day the ticket was filed, not a second bug.

That commit left `lib.rs` still listing guides as ignored on import; corrected.

* IDML: custom column positions are validated and round-trip (#1096)

Columns::positions are used only when they are 2 x count finite edges,
ascending from 0 to the margin box width (within 0.01 pt); anything else
is the even split, in Page::column_rects, on IDML import (bounded parse,
checked against the page's margin box) and on IDML export.
Columns::with_gutter_moved moves one gutter's pair of guides between its
neighbours for the coming column-guide command.

ROADMAP: drop four progress-log lines the #1121 merge duplicated and move
its line to the top.

* Layout: Margins and Columns keeps dragged columns unless the count or gutter changes (#1096)

* Layout: a new margin or page width resets dragged columns (#1096)

* Layout: #1121's margin case follows the new-width rule (#1096)

* Layout: dragged columns scale with a new margin or page width (#1096)

* Layout: the page-width test drags a page whose columns are its own (#1096)

* ROADMAP: the progress line lands in one commit after the guide PRs

---------

Co-authored-by: Hamza <mail.hamzanaveed@gmail.com>
This commit is contained in:
aiongg
2026-10-22 02:23:32 -04:00
committed by GitHub
co-authored by Hamza
parent d7b9f716f1
commit 1ba06027ac
+200 -10
View File
@@ -88,6 +88,7 @@ fn dist(r: designcraft_geom::Rect, x: f64) -> f64 {
/// Apply the `margins`, `columns` and `gutter` params of `layout.marginsAndColumns` to one page.
fn set_margins_columns(pg: &mut designcraft_doc::Page, p: &Value) {
let width_before = pg.margin_rect().width();
match p.get("margins") {
Some(Value::Number(n)) => pg.margins = Margins::uniform(n.as_f64().unwrap_or(36.0)),
Some(v @ Value::Object(_)) => {
@@ -97,18 +98,62 @@ fn set_margins_columns(pg: &mut designcraft_doc::Page, p: &Value) {
}
_ => {}
}
// Setting a count or a gutter re-divides the columns evenly, as InDesign's Margins and
// Columns does: dragged column positions are what the new value replaces.
scale_columns(pg, width_before);
// A new count or gutter re-divides the columns evenly: dragged column positions are what
// the new value replaces. The dialog always sends both, so an unchanged value keeps them.
if let Some(c) = p.get("columns").and_then(Value::as_u64) {
pg.columns.count = (c as u32).clamp(1, 216);
pg.columns.positions = None;
let c = u32::try_from(c).unwrap_or(u32::MAX).clamp(1, 216);
if c != pg.columns.count {
pg.columns.count = c;
pg.columns.positions = None;
}
}
if let Some(g) = p.get("gutter").and_then(Value::as_f64) {
pg.columns.gutter = g.max(0.0);
if let Some(g) = p.get("gutter").and_then(Value::as_f64).filter(|g| g.is_finite()) {
let g = g.max(0.0);
if (g - pg.columns.gutter).abs() > GUTTER_TOLERANCE {
pg.columns.gutter = g;
pg.columns.positions = None;
}
}
}
/// Dragged column positions are measured across the margin box. When the box was
/// `width_before` wide and now has another width, they scale with it, as InDesign's do; a set
/// that doesn't scale into a valid one goes back to the even split rather than stay hidden.
fn scale_columns(pg: &mut designcraft_doc::Page, width_before: f64) {
let width = pg.margin_rect().width();
if (width - width_before).abs() <= GUTTER_TOLERANCE {
return;
}
let Some(positions) = pg.columns.positions.as_mut() else { return };
let factor = width / width_before;
if factor.is_finite() && factor > 0.0 {
positions.iter_mut().for_each(|p| *p *= factor);
// The ends sit exactly on the margins whatever the rounding.
if let Some(first) = positions.first_mut() {
*first = 0.0;
}
if let Some(last) = positions.last_mut() {
*last = width;
}
}
if pg.columns.valid_positions(width).is_none() {
pg.columns.positions = None;
}
}
/// Gives `pg` the page width `width`, scaling its dragged columns with the margin box.
fn set_page_width(pg: &mut designcraft_doc::Page, width: f64) {
let before = pg.margin_rect().width();
pg.width = width;
scale_columns(pg, before);
}
/// How far a gutter may differ from the page's and still count as unchanged, in points. The
/// dialog shows the gutter in the ruler unit, rounded (inches to four decimals: up to 0.0036 pt
/// off), so an OK that leaves the field alone sends back a slightly different number.
const GUTTER_TOLERANCE: f64 = 0.01;
pub fn specs() -> Vec<CommandSpec> {
vec![
cmd!(
@@ -481,7 +526,7 @@ pub fn specs() -> Vec<CommandSpec> {
for sp in d.spreads.iter_mut().chain(d.parents.iter_mut()) {
let sp = std::sync::Arc::make_mut(sp);
for pg in &mut sp.pages {
pg.width = d.settings.page_width;
set_page_width(pg, d.settings.page_width);
pg.height = d.settings.page_height;
}
sp.relayout();
@@ -961,7 +1006,7 @@ pub(crate) fn resize_pages_from(d: &mut designcraft_doc::Document, pages: &[usiz
let dw = w.map_or(0.0, |w| w.max(1.0) - pg.width);
let dh = h.map_or(0.0, |h| h.max(1.0) - pg.height);
if let Some(w) = w {
pg.width = w.max(1.0);
set_page_width(pg, w.max(1.0));
}
if let Some(h) = h {
pg.height = h.max(1.0);
@@ -1689,10 +1734,155 @@ mod adjust_tests {
assert_eq!(pg.columns.positions, None);
assert!((pg.column_rects()[1].x0 - pg.column_rects()[0].x1 - 24.0).abs() < 1e-6);
// Margins alone leave them alone: the offsets are relative to the margin box.
// Margins that keep the margin box's width leave them alone; a new width scales them.
drag(&mut s);
s.execute("layout.marginsAndColumns", &json!({"margins": 72})).unwrap();
s.execute("layout.marginsAndColumns", &json!({"margins": {"top": 72, "bottom": 72, "inside": 36, "outside": 36}})).unwrap();
assert_eq!(s.doc().unwrap().doc.page(0).unwrap().columns.positions, Some(vec![0.0, 100.0, 130.0, 540.0]));
s.execute("layout.marginsAndColumns", &json!({"margins": 72})).unwrap();
assert_scaled(s.doc().unwrap().doc.page(0).unwrap().columns.positions.clone(), 468.0);
}
/// The params the Margins and Columns dialog sends on OK: every margin, the count and the
/// gutter, whether or not the user changed them.
fn dialog_params(margin: f64, columns: u32, gutter: f64) -> serde_json::Value {
json!({"margins": {"top": margin, "bottom": margin, "inside": margin, "outside": margin},
"columns": columns, "gutter": gutter, "adjustLayout": false})
}
const DRAGGED: [f64; 4] = [0.0, 100.0, 130.0, 540.0];
fn dragged() -> designcraft_doc::Columns {
designcraft_doc::Columns { count: 2, gutter: 12.0, positions: Some(DRAGGED.to_vec()) }
}
/// `positions` are [`DRAGGED`] scaled from its 540 pt margin box to one `width` wide.
#[track_caller]
fn assert_scaled(positions: Option<Vec<f64>>, width: f64) {
let want: Vec<f64> = DRAGGED.iter().map(|p| p * width / 540.0).collect();
let got = positions.unwrap_or_default();
assert!(got.len() == want.len() && got.iter().zip(&want).all(|(g, w)| (g - w).abs() < 1e-6), "{got:?} ≠ {want:?}");
}
/// An OK in the dialog keeps dragged columns unless the count or gutter it sends differs.
#[test]
fn the_dialog_keeps_dragged_columns_unless_the_count_or_gutter_changes() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap();
let drag = |s: &mut Session| {
s.edit(|d, _| {
std::sync::Arc::make_mut(&mut d.spreads[0]).pages[0].columns = dragged();
Ok(())
})
.unwrap();
};
let positions = |s: &Session| s.doc().unwrap().doc.page(0).unwrap().columns.positions.clone();
// The same count and gutter (the gutter as an inch field rounds it) keep them in use.
drag(&mut s);
s.execute("layout.marginsAndColumns", &dialog_params(36.0, 2, 12.0)).unwrap();
assert_eq!(positions(&s), Some(DRAGGED.to_vec()));
s.execute("layout.marginsAndColumns", &dialog_params(36.0, 2, 0.1667 * 72.0)).unwrap();
assert_eq!(positions(&s), Some(DRAGGED.to_vec()));
let rects = s.doc().unwrap().doc.page(0).unwrap().column_rects();
assert!((rects[0].x1 - rects[0].x0 - 100.0).abs() < 1e-6, "the dragged columns are in use: {rects:?}");
// Top and bottom margins leave the margin box's width alone, and the columns with it.
let mut taller = dialog_params(36.0, 2, 12.0);
taller["margins"]["top"] = json!(72.0);
taller["margins"]["bottom"] = json!(72.0);
s.execute("layout.marginsAndColumns", &taller).unwrap();
assert_eq!(positions(&s), Some(DRAGGED.to_vec()));
// A new width scales them with the margin box, and the old width scales them back.
s.execute("layout.marginsAndColumns", &dialog_params(72.0, 2, 12.0)).unwrap();
assert_scaled(positions(&s), 468.0);
let rects = s.doc().unwrap().doc.page(0).unwrap().column_rects();
assert!((rects[0].x1 - rects[0].x0 - 100.0 * 468.0 / 540.0).abs() < 1e-6, "the scaled columns are in use: {rects:?}");
s.execute("layout.marginsAndColumns", &dialog_params(36.0, 2, 12.0)).unwrap();
assert_scaled(positions(&s), 540.0);
// A new count resets them.
drag(&mut s);
s.execute("layout.marginsAndColumns", &dialog_params(36.0, 3, 12.0)).unwrap();
let pg = s.doc().unwrap().doc.page(0).unwrap().clone();
assert_eq!((pg.columns.count, pg.columns.positions), (3, None));
// So does a new gutter.
drag(&mut s);
s.execute("layout.marginsAndColumns", &dialog_params(36.0, 2, 18.0)).unwrap();
let pg = s.doc().unwrap().doc.page(0).unwrap().clone();
assert_eq!((pg.columns.gutter, pg.columns.positions), (18.0, None));
}
/// A page of another width (Page Size, Document Setup) scales its dragged columns.
#[test]
fn a_new_page_width_scales_dragged_columns() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap();
let drag = |s: &mut Session| {
s.edit(|d, _| {
// A dragged page's columns are its own (`local_margins`), as the drag will make them.
let pg = &mut std::sync::Arc::make_mut(&mut d.spreads[0]).pages[0];
pg.columns = dragged();
pg.local_margins = true;
Ok(())
})
.unwrap();
};
let positions = |s: &Session| s.doc().unwrap().doc.page(0).unwrap().columns.positions.clone();
drag(&mut s);
s.execute("layout.pageSize", &json!({"pages": [1], "height": 900})).unwrap();
assert_eq!(positions(&s), Some(DRAGGED.to_vec()), "a new height keeps them");
s.execute("layout.pageSize", &json!({"pages": [1], "width": 700})).unwrap();
assert_scaled(positions(&s), 628.0);
s.execute("layout.pageSize", &json!({"pages": [1], "width": 612})).unwrap();
assert_scaled(positions(&s), 540.0);
drag(&mut s);
s.execute("layout.documentSetup", &json!({"width": 700})).unwrap();
assert_scaled(positions(&s), 628.0);
}
/// Parent pages follow the same rule, and the document pages that follow them see it.
#[test]
fn the_dialog_keeps_a_parents_dragged_columns_unless_the_count_or_gutter_changes() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap();
let drag = |s: &mut Session| {
s.edit(|d, _| {
std::sync::Arc::make_mut(&mut d.parents[0]).pages[0].columns = dragged();
d.sync_parent_margins();
Ok(())
})
.unwrap();
};
let positions = |s: &Session| {
let d = &s.doc().unwrap().doc;
(d.parents[0].pages[0].columns.positions.clone(), d.page(0).unwrap().columns.positions.clone())
};
let on_parent = |m: f64, c: u32, g: f64| {
let mut p = dialog_params(m, c, g);
p["parent"] = json!(0);
p
};
drag(&mut s);
assert_eq!(positions(&s), (Some(DRAGGED.to_vec()), Some(DRAGGED.to_vec())));
s.execute("layout.marginsAndColumns", &on_parent(36.0, 2, 12.0)).unwrap();
assert_eq!(positions(&s), (Some(DRAGGED.to_vec()), Some(DRAGGED.to_vec())));
s.execute("layout.marginsAndColumns", &on_parent(48.0, 2, 12.0)).unwrap();
let (parent, follower) = positions(&s);
assert_scaled(parent, 516.0);
assert_scaled(follower, 516.0);
drag(&mut s);
s.execute("layout.marginsAndColumns", &on_parent(36.0, 3, 12.0)).unwrap();
assert_eq!(positions(&s), (None, None));
drag(&mut s);
s.execute("layout.marginsAndColumns", &on_parent(36.0, 2, 24.0)).unwrap();
assert_eq!(positions(&s), (None, None));
}
}