Layout: autoflow, inserted pages and new parents follow dragged columns, mirrored across sides (#1096) (#1173)

* 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: autoflow, inserted pages and new parents follow dragged columns (#1096)

* IDML: a page without its own margins checks the default's column positions against its width (#1096)

* Layout: dragged columns mirror when a page changes side (#1096)

* Layout: pages that follow a one-page parent take its dragged columns mirrored (#1096)

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

---------

Co-authored-by: Hamza <mail.hamzanaveed@gmail.com>

crates/doc/src/build.rs

@@ -343,6 +343,8 @@ impl Document {
         let mut pages: Vec<(Page, Vec<Arc<Item>>)> = Vec::new();
         let mut parity: Vec<bool> = Vec::new();
         let mut kept: Vec<(usize, Arc<Spread>)> = Vec::new();
+        // Whether each shuffled page was a left page, to mirror its dragged columns if it moves.
+        let mut was_left: BTreeMap<PageId, bool> = BTreeMap::new();
         let mut abs = 0;
         for sp in self.spreads.drain(..) {
             if !sp.allow_shuffle && sp.pages.len() > 1 {
@@ -350,8 +352,9 @@ impl Document {
                 kept.push((pages.len(), sp));
                 continue;
             }
-            for _ in &sp.pages {
+            for p in &sp.pages {
                 parity.push(odd.get(abs).copied().unwrap_or(abs % 2 == 0));
+                was_left.insert(p.id, p.side == PageSide::Left);
                 abs += 1;
             }
             let mut sp = Arc::unwrap_or_clone(sp);
@@ -445,6 +448,19 @@ impl Document {
             out.insert(at, sp);
         }
         self.spreads = out;
+        // A page that crosses the spine keeps its inside and outside margins, so its margin box
+        // mirrors; its dragged column guides mirror with it, as in InDesign.
+        let crossed = |p: &Page| was_left.get(&p.id).is_some_and(|&l| l != (p.side == PageSide::Left)) && p.columns.positions.is_some();
+        for sp in &mut self.spreads {
+            if !sp.pages.iter().any(crossed) {
+                continue;
+            }
+            for p in &mut Arc::make_mut(sp).pages {
+                if crossed(&*p) {
+                    p.columns = p.columns.mirrored(p.margin_rect().width());
+                }
+            }
+        }
         // Pages can change sides, and so parent pages.
         self.sync_parent_margins();
     }
@@ -466,6 +482,10 @@ impl Document {
                 if let Some(t) = &template {
                     p.width = t.width;
                     p.height = t.height;
+                    // Dragged column guides too: the same width and margins keep them valid.
+                    // They are taken as for this single page; repaginating mirrors them if the
+                    // page lands on a left page.
+                    p.columns.positions = p.columns_from(t).positions;
                 }
                 ids.push(p.id);
                 p
@@ -675,7 +695,7 @@ impl Document {
                 if page.local_margins {
                     continue;
                 }
-                let Some(src) = self.parent_page(page).filter(|q| !q.same_margins(page)).map(|q| (q.margins, q.columns.clone())) else {
+                let Some(src) = self.parent_page(page).filter(|q| !q.same_margins(page)).map(|q| (q.margins, page.columns_from(q))) else {
                     continue;
                 };
                 if let Some(page) = Arc::make_mut(&mut self.spreads[si]).pages.get_mut(pi) {
@@ -879,6 +899,65 @@ mod tests {
         assert!(d.delete_pages(&[0, 1]).is_err());
     }
 
+    /// Pages inserted after a page with dragged column guides keep them, as they keep its column
+    /// count and gutter; pages that follow a parent take the parent's.
+    #[test]
+    fn inserted_pages_keep_dragged_columns() {
+        let dragged = Columns { count: 2, gutter: 12.0, positions: Some(vec![0.0, 100.0, 130.0, 540.0]) };
+        let widths = |p: &Page| p.column_rects().iter().map(|r| r.width()).collect::<Vec<_>>();
+        let mut d = Document::new(&NewDocument { pages: 1, facing_pages: false, ..Default::default() });
+        let pg = &mut Arc::make_mut(&mut d.spreads[0]).pages[0];
+        (pg.columns, pg.local_margins) = (dragged.clone(), true);
+        d.insert_pages(Some(0), 2, None).unwrap();
+        for i in 1..3 {
+            let p = d.page(i).unwrap();
+            assert_eq!(p.columns, dragged, "page {i}");
+            assert_eq!(widths(p), vec![100.0, 410.0], "page {i}");
+        }
+
+        let mut d = Document::new(&NewDocument { pages: 1, facing_pages: false, ..Default::default() });
+        Arc::make_mut(&mut d.parents[0]).pages[0].columns = dragged.clone();
+        d.sync_parent_margins();
+        d.insert_pages(Some(0), 1, d.parents.first().map(|p| p.id)).unwrap();
+        assert_eq!(d.page(1).map(widths), Some(vec![100.0, 410.0]));
+    }
+
+    /// A right page whose gutter was dragged toward the spine keeps it there when inserting a
+    /// page before it makes it a left page: its dragged columns mirror across the margin box.
+    #[test]
+    fn dragged_columns_mirror_when_a_page_changes_side() {
+        let mut d = Document::new(&NewDocument { pages: 1, facing_pages: true, ..Default::default() });
+        let pg = &mut Arc::make_mut(&mut d.spreads[0]).pages[0];
+        assert_eq!(pg.side, PageSide::Right);
+        let w = pg.margin_rect().width();
+        (pg.columns, pg.local_margins) = (Columns { count: 2, gutter: 12.0, positions: Some(vec![0.0, 100.0, 112.0, w]) }, true);
+        let id = pg.id;
+        // How far the gutter's spine-side edge is from the margin on the spine side.
+        let from_spine = |p: &Page| {
+            let (m, c) = (p.margin_rect(), p.column_rects());
+            match p.side {
+                PageSide::Left => m.x1 - c[1].x0,
+                _ => c[0].x1 - m.x0,
+            }
+        };
+        assert!((from_spine(d.page(0).unwrap()) - 100.0).abs() < 1e-9);
+
+        d.insert_pages(None, 1, None).unwrap();
+        let moved = d.page(1).unwrap();
+        assert_eq!((moved.id, moved.side), (id, PageSide::Left));
+        assert!((from_spine(moved) - 100.0).abs() < 1e-9, "{:?}", moved.column_rects());
+        let widths: Vec<f64> = moved.column_rects().iter().map(|r| r.width()).collect();
+        assert!((widths[0] - (w - 112.0)).abs() < 1e-9 && (widths[1] - 100.0).abs() < 1e-9, "{widths:?}");
+        // The inserted page is a right page like the one it copies: no mirroring.
+        assert_eq!(d.page(0).unwrap().columns.positions, Some(vec![0.0, 100.0, 112.0, w]));
+
+        // Moving it back to the right undoes the mirror.
+        d.delete_pages(&[0]).unwrap();
+        let back = d.page(0).unwrap();
+        assert_eq!(back.side, PageSide::Right);
+        assert!((from_spine(back) - 100.0).abs() < 1e-9, "{:?}", back.column_rects());
+    }
+
     #[test]
     fn items_travel_with_pages() {
         let mut d = Document::new(&NewDocument { pages: 3, ..Default::default() });

crates/doc/src/page.rs

@@ -106,6 +106,15 @@ impl Columns {
         *r = (at + width).min(hi);
         valid_column_positions(&pos, self.count, margin_width).then_some(pos)
     }
+
+    /// These columns reflected across a margin box `margin_width` wide, as on the facing page
+    /// across the spine: each custom position `p` becomes `margin_width - p`, in reverse order so
+    /// they stay ascending. Count and gutter stay; positions that are not valid (see
+    /// [`Columns::valid_positions`]) become `None`, the even split, which mirrors onto itself.
+    pub fn mirrored(&self, margin_width: f64) -> Columns {
+        let positions = self.valid_positions(margin_width).map(|p| p.iter().rev().map(|&v| inside(margin_width - v, margin_width)).collect());
+        Columns { count: self.count, gutter: self.gutter, positions }
+    }
 }
 
 /// `v` limited to `0..=w` without `clamp`, which panics when `w` is NaN.
@@ -304,9 +313,10 @@ impl Page {
     pub fn bounds(&self) -> Rect {
         Rect::new(self.x, 0.0, self.x + self.width, self.height)
     }
-    /// Same margins and columns as `other`.
+    /// Same margins and columns as `other`, with `other`'s columns as it would take them from this
+    /// page: mirrored when one is a left page and the other isn't.
     pub fn same_margins(&self, other: &Page) -> bool {
-        self.margins == other.margins && self.columns == other.columns
+        self.margins == other.margins && other.columns_from(self) == other.columns
     }
     /// Left and right margins resolved for the page side (inside is at the spine).
     pub fn left_right_margins(&self) -> (f64, f64) {
@@ -315,6 +325,16 @@ impl Page {
             _ => (self.margins.inside, self.margins.outside),
         }
     }
+    /// `src`'s columns as they fall on this page: the same when both pages put their inside
+    /// margin on the same physical side, mirrored across `src`'s margin box when one is a left
+    /// page and the other is not, as InDesign mirrors dragged column guides across the spine.
+    pub fn columns_from(&self, src: &Page) -> Columns {
+        if (self.side == PageSide::Left) == (src.side == PageSide::Left) {
+            src.columns.clone()
+        } else {
+            src.columns.mirrored(src.margin_rect().width())
+        }
+    }
     /// The live area inside the margins.
     pub fn margin_rect(&self) -> Rect {
         let (l, r) = self.left_right_margins();
@@ -602,6 +622,43 @@ mod tests {
         assert_eq!(c.with_gutter_moved(f64::NAN, 0, 100.0), None);
     }
 
+    /// Mirroring reflects dragged columns across the margin box and keeps them ascending; an
+    /// invalid set becomes the even split.
+    #[test]
+    fn mirrored_columns_reflect_across_the_margin_box() {
+        let two = Columns { count: 2, gutter: 12.0, positions: Some(vec![0.0, 100.0, 130.0, 540.0]) };
+        assert_eq!(two.mirrored(540.0), Columns { positions: Some(vec![0.0, 410.0, 440.0, 540.0]), ..two.clone() });
+        assert_eq!(two.mirrored(540.0).mirrored(540.0), two);
+
+        let three = Columns { count: 3, gutter: 10.0, positions: Some(vec![0.0, 50.0, 60.0, 200.0, 210.0, 300.0]) };
+        let m = three.mirrored(300.0);
+        assert_eq!(m.positions, Some(vec![0.0, 90.0, 100.0, 240.0, 250.0, 300.0]));
+        assert!(m.valid_positions(300.0).is_some());
+
+        // Positions that do not fit the margin box, or none at all, mirror to the even split.
+        assert_eq!(two.mirrored(600.0).positions, None);
+        assert_eq!(two.mirrored(f64::NAN).positions, None);
+        let unsorted = Columns { positions: Some(vec![0.0, 130.0, 100.0, 540.0]), ..two.clone() };
+        assert_eq!(unsorted.mirrored(540.0).positions, None);
+        let even = Columns { positions: None, ..two.clone() };
+        assert_eq!(even.mirrored(540.0), even);
+
+        // A left page takes a right page's columns mirrored, a page on the same side as they are.
+        let mut r = page(PageSide::Right, 612.0);
+        r.columns = Columns { count: 2, gutter: 12.0, positions: Some(vec![0.0, 100.0, 130.0, 522.0]) };
+        let l = page(PageSide::Left, 0.0);
+        assert_eq!(l.columns_from(&r).positions, Some(vec![0.0, 392.0, 422.0, 522.0]));
+        assert_eq!(page(PageSide::Right, 0.0).columns_from(&r), r.columns);
+
+        // A left page holding them mirrored has the same margins as that right page (a one-page
+        // parent in a facing document); holding them unmirrored, it doesn't.
+        let mut l = page(PageSide::Left, 0.0);
+        l.columns = l.columns_from(&r);
+        assert!(r.same_margins(&l));
+        l.columns = r.columns.clone();
+        assert!(!r.same_margins(&l));
+    }
+
     #[test]
     fn spread_spine_and_hit() {
         let s = Spread {

crates/engine/src/cmd/datamerge/mod.rs

@@ -436,7 +436,7 @@ fn assign_page(dst: &mut Document, abs: usize, from: &Page) {
     p.guides = from.guides.clone();
     p.parent = from.parent;
     p.margins = from.margins;
-    p.columns = from.columns.clone();
+    p.columns = p.columns_from(from);
     p.local_margins = from.local_margins;
     p.width = from.width;
     p.height = from.height;

crates/engine/src/cmd/inspect.rs

@@ -85,7 +85,9 @@ fn inspect(s: &mut Session, _p: &Value) -> Result<Value> {
                     let abs = d.first_page_of_spread(si) + pi;
                     json!({"index": abs, "name": d.page_name(abs), "bounds": [p.x, 0.0, p.x + p.width, p.height], "side": p.side,
                         "parent": p.parent.and_then(|pid| d.parents.iter().find(|x| x.id == pid)).and_then(|x| x.parent.as_ref()).map(|i| i.label()),
-                        "margins": p.margins, "columns": p.columns.count})
+                        "margins": p.margins, "columns": p.columns.count, "gutter": p.columns.gutter,
+                        "customColumns": p.columns.valid_positions(p.margin_rect().width()).is_some(),
+                        "columnRects": p.column_rects().iter().map(|r| [r.x0, r.y0, r.x1, r.y1]).collect::<Vec<_>>()})
                 }).collect::<Vec<_>>(),
                 "items": sp.items.iter().map(|i| item_json(i)).collect::<Vec<_>>(),
             })

crates/engine/src/cmd/layout.rs

@@ -246,8 +246,9 @@ pub fn specs() -> Vec<CommandSpec> {
                         let nsp = std::sync::Arc::make_mut(&mut d.spreads[nsi]);
                         let pg = &mut nsp.pages[npi];
                         let dx = pg.x - src.x;
-                        (pg.width, pg.height, pg.margins, pg.columns, pg.liquid) =
-                            (src.width, src.height, src.margins, src.columns.clone(), src.liquid);
+                        // A copy on the other side of the spine has its dragged columns mirrored.
+                        let columns = pg.columns_from(src);
+                        (pg.width, pg.height, pg.margins, pg.columns, pg.liquid) = (src.width, src.height, src.margins, columns, src.liquid);
                         pg.guides = src.guides.clone();
                         for g in pg.guides.iter_mut().filter(|g| g.orientation == designcraft_doc::Orientation::Vertical) {
                             g.position += dx;
@@ -301,8 +302,20 @@ pub fn specs() -> Vec<CommandSpec> {
             let name = str_param(p, "name").unwrap_or("Parent").to_string();
             s.edit(|d, _| {
                 let prefix = str_param(p, "prefix").map(str::to_string).unwrap_or_else(|| d.next_parent_prefix());
-                let m = d.page(0).map(|p| (p.columns.count, p.columns.gutter, p.margins)).unwrap_or((1, 12.0, Margins::uniform(36.0)));
-                let id = d.add_parent(&prefix, &name, m.0, m.1, m.2);
+                let src = d.page(0).cloned();
+                let (columns, margins) =
+                    src.as_ref().map_or_else(|| (designcraft_doc::Columns::default(), Margins::uniform(36.0)), |p| (p.columns.clone(), p.margins));
+                let id = d.add_parent(&prefix, &name, columns.count, columns.gutter, margins);
+                // Dragged column guides carry over to the parent pages they still fit, mirrored
+                // on the parent page across the spine from the first page.
+                if let (Some(src), Some(sp)) = (&src, d.parents.iter_mut().find(|x| x.id == id)) {
+                    for pg in &mut std::sync::Arc::make_mut(sp).pages {
+                        let columns = pg.columns_from(src);
+                        if columns.valid_positions(pg.margin_rect().width()).is_some() {
+                            pg.columns.positions = columns.positions;
+                        }
+                    }
+                }
                 Ok(json!({"id": id.0, "prefix": prefix}))
             })
         }),
@@ -2182,6 +2195,38 @@ mod parent_tests {
         assert_eq!(r, json!({"on": false, "parent": null}));
         assert!(!s.doc().unwrap().editing_parents);
     }
+
+    /// A new parent takes the first page's columns, dragged column guides included: the right
+    /// parent page as the first (right) page has them, the left one mirrored across the spine.
+    /// `document.inspect` reports where those columns are.
+    #[test]
+    fn a_new_parent_keeps_dragged_columns() {
+        let mut s = crate::Session::new();
+        s.execute("file.new", &json!({"pages": 1})).unwrap();
+        let dragged = designcraft_doc::Columns { count: 2, gutter: 12.0, positions: Some(vec![0.0, 100.0, 130.0, 540.0]) };
+        // There is no command for dragging a column guide yet, so set what an import would leave.
+        s.edit(|d, _| {
+            let pg = &mut std::sync::Arc::make_mut(&mut d.spreads[0]).pages[0];
+            (pg.columns, pg.local_margins) = (dragged.clone(), true);
+            Ok(())
+        })
+        .unwrap();
+        let id = s.execute("layout.parents.new", &json!({"prefix": "B"})).unwrap()["id"].as_u64().unwrap();
+        let d = &s.doc().unwrap().doc;
+        let parent = d.parents.iter().find(|p| p.id.0 == id).unwrap();
+        assert_eq!(parent.pages.len(), 2);
+        let side = |s: designcraft_doc::PageSide| parent.pages.iter().find(|p| p.side == s).unwrap();
+        let (left, right) = (side(designcraft_doc::PageSide::Left), side(designcraft_doc::PageSide::Right));
+        assert_eq!(right.columns, dragged);
+        assert_eq!(right.column_rects().iter().map(|r| r.width()).collect::<Vec<_>>(), vec![100.0, 410.0]);
+        assert_eq!(left.columns.positions, Some(vec![0.0, 410.0, 440.0, 540.0]));
+        assert_eq!(left.column_rects().iter().map(|r| r.width()).collect::<Vec<_>>(), vec![410.0, 100.0]);
+
+        let inspect = s.execute("document.inspect", &json!({})).unwrap();
+        let page = &inspect["spreads"][0]["pages"][0];
+        assert_eq!((page["columns"].clone(), page["customColumns"].clone()), (json!(2), json!(true)), "{page}");
+        assert_eq!(page["columnRects"][0], json!([36.0, 36.0, 136.0, 756.0]), "{page}");
+    }
 }
 
 #[cfg(test)]

crates/engine/src/cmd/place_text.rs

@@ -277,7 +277,7 @@ pub(crate) fn autoflow(d: &mut Document, sid: StoryId, max_pages: usize, cmd: &s
         if added >= max_pages {
             return Err(bad(cmd, format!("autoflow stopped after {max_pages} pages")));
         }
-        let per_page = if vertical { 1 } else { d.page(page).map_or(1, |p| p.columns.count.max(1) as usize) };
+        let per_page = if vertical { 1 } else { d.page(page).map_or(1, |p| p.column_rects().len().max(1)) };
         let pages = want.div_ceil(per_page).clamp(1, max_pages - added);
         let parent = d.page_loc(page).and_then(|(si, pi)| d.spreads[si].pages[pi].parent);
         new_pages.extend(d.insert_pages(Some(page), pages, parent)?);
@@ -511,6 +511,43 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    /// Autoflow follows dragged column guides: one frame per dragged column, on the first page and
+    /// on every page it adds.
+    #[test]
+    fn autoflow_sets_frames_on_dragged_columns() {
+        let dir = std::env::temp_dir().join(format!("dc-place-dragged-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let path = dir.join("long.txt");
+        std::fs::write(&path, vec!["Lorem ipsum dolor sit amet, consectetur adipiscing elit. ".repeat(12); 40].join("\n")).unwrap();
+        let mut s = Session::new();
+        s.execute("file.new", &json!({"preset": "Letter", "pages": 1, "columns": 2, "gutter": 18, "margins": 54})).unwrap();
+        // The margin box is 504 pt wide; the gutter is dragged left. There is no command for
+        // dragging a column guide yet, so set what an import would leave, on the parent.
+        let dragged = designcraft_doc::Columns { count: 2, gutter: 18.0, positions: Some(vec![0.0, 150.0, 170.0, 504.0]) };
+        s.edit(|d, _| {
+            for pg in &mut std::sync::Arc::make_mut(&mut d.parents[0]).pages {
+                pg.columns = dragged.clone();
+            }
+            d.sync_parent_margins();
+            Ok(())
+        })
+        .unwrap();
+        let r = s.execute("file.place", &json!({"path": path.to_string_lossy(), "page": 1, "autoflow": true})).unwrap();
+        assert_eq!(r["overset"], false, "{r}");
+        let d = &s.doc().unwrap().doc;
+        assert!(d.page_count() >= 3, "{r}");
+        let frames = &d.story(StoryId(r["story"].as_u64().unwrap())).unwrap().frames;
+        assert!(frames.len() > 2 * (d.page_count() - 1), "{} frames", frames.len());
+        for (i, f) in frames.iter().enumerate() {
+            let page = d.page_of_item(*f).unwrap();
+            assert_eq!(page, i / 2, "frame {i}");
+            let m = d.page(page).unwrap().margin_rect();
+            let (x0, x1) = [(0.0, 150.0), (170.0, 504.0)][i % 2];
+            assert_eq!(d.item(*f).unwrap().bounds(), Rect::new(m.x0 + x0, m.y0, m.x0 + x1, m.y1), "frame {i}");
+        }
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn autoflow_keeps_the_frame_the_user_drew() {
         let dir = std::env::temp_dir().join(format!("dc-place-drawn-{}", std::process::id()));