Merge rtl-master-sides: right-to-left binding puts parent page items on the right side

Conflict: idml tests.rs test modules (both).

crates/doc/src/build.rs

@@ -236,6 +236,9 @@ impl Document {
         if n == 2 {
             pages[0].side = PageSide::Left;
             pages[1].side = PageSide::Right;
+            if self.settings.right_to_left_binding {
+                pages.reverse();
+            }
         }
         let id = SpreadId(self.alloc());
         let mut sp = Spread {
@@ -250,6 +253,18 @@ impl Document {
         id
     }
 
+    /// List the pages of each facing parent spread in binding order: left then right, or right
+    /// then left when bound right to left. Positions stay, so the parent items keep their pages.
+    pub fn order_parent_pages(&mut self) {
+        let first = if self.settings.right_to_left_binding { PageSide::Right } else { PageSide::Left };
+        for sp in &mut self.parents {
+            let pair = matches!(sp.pages.as_slice(), [a, b] if a.side != b.side && a.side != PageSide::Single && b.side != PageSide::Single);
+            if pair && sp.pages.first().is_some_and(|p| p.side != first) {
+                Arc::make_mut(sp).pages.reverse();
+            }
+        }
+    }
+
     pub fn parent_index(&self, id: SpreadId) -> Option<usize> {
         self.parents.iter().position(|p| p.id == id)
     }
@@ -549,8 +564,7 @@ impl Document {
         let pi = self.parent_index(page.parent?)?;
         let ps = &self.parents[pi];
         // A parent spread can come without pages from a file.
-        let idx = if ps.pages.len() >= 2 && page.side == PageSide::Left { 0 } else { ps.pages.len().checked_sub(1)? };
-        Some((pi, idx))
+        Some((pi, ps.parent_page_for_side(page.side)?))
     }
 }
 

crates/doc/src/page.rs

@@ -296,6 +296,25 @@ impl Spread {
             None => self.bounds().center().x,
         }
     }
+    /// Indices of the leftmost and rightmost pages. A right-to-left bound spread lists its right
+    /// page first, so list order says nothing about position.
+    pub fn outer_page_indices(&self) -> Option<(usize, usize)> {
+        let mut it = self.pages.iter().enumerate();
+        let (first, _) = it.next()?;
+        Some(it.fold((first, first), |(l, r), (i, p)| (if p.x < self.pages[l].x { i } else { l }, if p.x > self.pages[r].x { i } else { r })))
+    }
+    /// The parent page (index into this parent spread) shown behind a document page on `side`:
+    /// the leftmost page for a left page and the rightmost for a right page, whichever binding
+    /// ordered the list; the last page for a single-sided page or a one-page parent.
+    pub fn parent_page_for_side(&self, side: PageSide) -> Option<usize> {
+        let (l, r) = self.outer_page_indices()?;
+        match side {
+            _ if self.pages.len() < 2 => Some(l),
+            PageSide::Left => Some(l),
+            PageSide::Right => Some(r),
+            PageSide::Single => self.pages.len().checked_sub(1),
+        }
+    }
     /// Index of the page under spread x (nearest page for the pasteboard).
     pub fn page_at_x(&self, x: f64) -> Option<usize> {
         if self.pages.is_empty() {
@@ -303,9 +322,7 @@ impl Spread {
         }
         self.pages.iter().position(|p| x >= p.x && x < p.x + p.width).or_else(|| {
             // The pasteboard: the page nearest that side.
-            let (l, r) = (0..self.pages.len()).fold((0, 0), |(l, r), i| {
-                (if self.pages[i].x < self.pages[l].x { i } else { l }, if self.pages[i].x > self.pages[r].x { i } else { r })
-            });
+            let (l, r) = self.outer_page_indices()?;
             Some(if x < self.pages[l].x { l } else { r })
         })
     }

crates/engine/src/cmd/layout.rs

@@ -273,6 +273,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     }
                     if let Some(b) = p.get("binding").and_then(Value::as_str) {
                         d.settings.right_to_left_binding = b == "rightToLeft";
+                        d.order_parent_pages();
                     }
                     if let Some(b) = bleed {
                         d.settings.bleed = b;
@@ -875,6 +876,38 @@ mod setup_tests {
         assert_eq!(d.spreads[1].pages.iter().map(|p| (p.side, p.x)).collect::<Vec<_>>(), vec![(Left, 0.0), (Right, w)]);
     }
 
+    /// Bound right to left, a parent spread lists its right page first; each document page still
+    /// shows the parent page on its own side, at its own position, also after an IDML round trip.
+    #[test]
+    fn right_to_left_binding_shows_the_parent_page_of_the_same_side() {
+        use designcraft_doc::PageSide::{Left, Right};
+        let mut s = crate::Session::new();
+        s.execute("file.new", &json!({"pages": 4})).unwrap();
+        s.execute("layout.documentSetup", &json!({"binding": "rightToLeft"})).unwrap();
+        let w = s.doc().unwrap().doc.settings.page_width;
+        let parent = json!({"kind": "parent", "index": 0});
+        for (rect, text) in [(json!([72, 36, 200, 60]), "left head"), (json!([w + 72.0, 36, w + 200.0, 60]), "right head")] {
+            s.execute("frame.create", &json!({"spread": parent, "rect": rect, "content": "text", "text": text})).unwrap();
+        }
+        let d = s.doc().unwrap().doc.clone();
+        assert_eq!(d.parents[0].pages.iter().map(|p| (p.side, p.x)).collect::<Vec<_>>(), vec![(Right, w), (Left, 0.0)]);
+        let docs = [(*d).clone(), designcraft_idml::import_idml(&designcraft_idml::export_idml(&d)).unwrap()];
+        for d in docs {
+            let mut s = crate::Session::new();
+            s.add_document(crate::DocState::new(d, None));
+            for (page, side, text) in [(0, Left, "left head"), (1, Right, "right head"), (2, Left, "left head"), (3, Right, "right head")] {
+                let r = s.execute("layout.overrideParentItems", &json!({"page": page})).unwrap();
+                let d = &s.doc().unwrap().doc;
+                let ids = r["ids"].as_array().unwrap();
+                assert_eq!(ids.len(), 1, "page {}", page + 1);
+                let it = d.item(designcraft_doc::ItemId(ids[0].as_u64().unwrap())).unwrap();
+                let story = d.story(it.text_frame().unwrap().story).unwrap();
+                assert_eq!((d.page(page).unwrap().side, story.text.trim_end()), (side, text), "page {}", page + 1);
+                assert_eq!(d.page_of_item(it.id), Some(page), "page {}", page + 1);
+            }
+        }
+    }
+
     /// Multi-column frames filled the left column first in a right-to-left document (#100):
     /// stories made in a right-to-left-bound document start their columns on the right.
     #[test]

crates/idml/src/tests.rs

@@ -8,6 +8,7 @@ use designcraft_geom::{Rect, shapes};
 use super::*;
 
 mod decorations;
+mod rtl_binding;
 mod variables;
 mod wrap;
 

crates/idml/src/tests/rtl_binding.rs

@@ -0,0 +1,91 @@
+use designcraft_doc::PageSide::{self, Left, Right};
+
+use super::*;
+
+const NS: &str = r#"xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging" DOMVersion="16.0""#;
+
+fn rect_item(id: &str, x0: f64, x1: f64) -> String {
+    let pts: String = [(x0, -280.0), (x0, -260.0), (x1, -260.0), (x1, -280.0)]
+        .iter()
+        .map(|(x, y)| format!(r#"<PathPointType Anchor="{x} {y}" LeftDirection="{x} {y}" RightDirection="{x} {y}"/>"#))
+        .collect();
+    format!(
+        r#"<Rectangle Self="{id}" ItemLayer="L1" ItemTransform="1 0 0 1 0 0"><Properties><PathGeometry><GeometryPathType PathOpen="false"><PathPointArray>{pts}</PathPointArray></GeometryPathType></PathGeometry></Properties></Rectangle>"#
+    )
+}
+
+/// Bound right to left, the first page of every spread (parent spreads too) lies right of the
+/// spine and the second left of it. The parent's right page holds a 100 pt wide item, its left
+/// page a 50 pt one. Five pages: 1 alone, then 2–3 and 4–5.
+fn right_to_left_fixture() -> Vec<u8> {
+    let page = |id: &str, x: f64, master: bool| {
+        let applied = if master { String::new() } else { r#" AppliedMaster="m1""#.to_string() };
+        format!(r#"<Page Self="{id}"{applied} GeometricBounds="0 0 600 400" ItemTransform="1 0 0 1 {x} -300"/>"#)
+    };
+    let master = format!(
+        r#"<idPkg:MasterSpread {NS}><MasterSpread Self="m1" Name="A-Parent" NamePrefix="A" BaseName="Parent" PageCount="2" ItemTransform="1 0 0 1 0 0">{}{}{}{}</MasterSpread></idPkg:MasterSpread>"#,
+        page("mp1", 0.0, true),
+        page("mp2", -400.0, true),
+        rect_item("right", 50.0, 150.0),
+        rect_item("left", -350.0, -300.0),
+    );
+    let spread = |id: &str, pages: &[(&str, f64)]| {
+        let ps: String = pages.iter().map(|(p, x)| page(p, *x, false)).collect();
+        format!(r#"<idPkg:Spread {NS}><Spread Self="{id}" PageCount="{}" ItemTransform="1 0 0 1 0 0">{ps}</Spread></idPkg:Spread>"#, pages.len())
+    };
+    let map = format!(
+        r#"<Document {NS} Self="d"><idPkg:Preferences src="Resources/Preferences.xml"/><Layer Self="L1" Name="Layer 1"/><idPkg:MasterSpread src="MasterSpreads/m1.xml"/><idPkg:Spread src="Spreads/sp0.xml"/><idPkg:Spread src="Spreads/sp1.xml"/><idPkg:Spread src="Spreads/sp2.xml"/></Document>"#
+    );
+    let prefs = format!(
+        r#"<idPkg:Preferences {NS}><DocumentPreference PageWidth="400" PageHeight="600" FacingPages="true" PageBinding="RightToLeft"/></idPkg:Preferences>"#
+    );
+    zip_files(&[
+        ("designmap.xml", &map),
+        ("Resources/Preferences.xml", &prefs),
+        ("MasterSpreads/m1.xml", &master),
+        ("Spreads/sp0.xml", &spread("sp0", &[("p1", 0.0)])),
+        ("Spreads/sp1.xml", &spread("sp1", &[("p2", 0.0), ("p3", -400.0)])),
+        ("Spreads/sp2.xml", &spread("sp2", &[("p4", 0.0), ("p5", -400.0)])),
+    ])
+}
+
+/// The parent items a document page shows, as (width, bounds on the document spread).
+fn shown_parent_items(d: &Document, abs: usize) -> Vec<(f64, Rect)> {
+    let (si, pi) = d.page_loc(abs).unwrap();
+    let page = &d.spreads[si].pages[pi];
+    let (ppi, ppage) = d.parent_page_for(abs).unwrap();
+    let parent = &d.parents[ppi];
+    let dx = page.x - parent.pages[ppage].x;
+    parent
+        .items
+        .iter()
+        .filter(|it| parent.page_at_x(it.bounds().center().x) == Some(ppage))
+        .map(|it| {
+            let b = it.bounds();
+            (b.width(), Rect::new(b.x0 + dx, b.y0, b.x1 + dx, b.y1))
+        })
+        .collect()
+}
+
+fn assert_parent_items_follow_page_sides(d: &Document) {
+    assert!(d.settings.right_to_left_binding);
+    let sides: Vec<PageSide> = d.parents[0].pages.iter().map(|p| p.side).collect();
+    assert_eq!(sides, vec![Right, Left], "the parent spread lists its right page first");
+    for (abs, side, width) in [(0, Right, 100.0), (1, Right, 100.0), (2, Left, 50.0), (3, Right, 100.0), (4, Left, 50.0)] {
+        let page = d.page(abs).unwrap();
+        assert_eq!(page.side, side, "page {}", abs + 1);
+        let shown = shown_parent_items(d, abs);
+        assert_eq!(shown.len(), 1, "page {}", abs + 1);
+        let (w, b) = shown[0];
+        assert!((w - width).abs() < 1e-6, "page {} shows the {width} pt item, not {w}", abs + 1);
+        assert!(b.x0 >= page.x - 1e-6 && b.x1 <= page.x + page.width + 1e-6, "page {}: {b:?} lies on the page", abs + 1);
+    }
+}
+
+#[test]
+fn right_to_left_parent_pages_follow_the_document_page_side() {
+    let d = import_idml(&right_to_left_fixture()).unwrap();
+    assert_parent_items_follow_page_sides(&d);
+    let again = import_idml(&export_idml(&d)).unwrap();
+    assert_parent_items_follow_page_sides(&again);
+}

crates/pdf/src/export.rs

@@ -73,8 +73,10 @@ fn sheets(doc: &Document, opts: &PdfOptions) -> Result<Vec<Sheet>> {
             }
             seen_spreads.push(si);
             let first = doc.first_page_of_spread(si);
-            let (l, _) = sp.pages.first().map(|p| bleed_lr(p.side, b)).unwrap_or((b[2], b[3]));
-            let (_, r) = sp.pages.last().map(|p| bleed_lr(p.side, b)).unwrap_or((b[2], b[3]));
+            // The outer edges by position: a right-to-left spread lists its right page first.
+            let outer = sp.outer_page_indices();
+            let (l, _) = outer.and_then(|(l, _)| sp.pages.get(l)).map(|p| bleed_lr(p.side, b)).unwrap_or((b[2], b[3]));
+            let (_, r) = outer.and_then(|(_, r)| sp.pages.get(r)).map(|p| bleed_lr(p.side, b)).unwrap_or((b[2], b[3]));
             let names: Vec<String> = (0..sp.pages.len()).map(|i| doc.page_name(first + i)).collect();
             (sp.bounds(), l, r, names.join("–"))
         } else {

crates/tools/src/snap.rs

@@ -257,7 +257,7 @@ fn push_parent_rotation_chain(
         return;
     }
     let next = parent.parent.as_ref().and_then(|info| info.based_on);
-    if let Some(page_idx) = shown_parent_page_index(parent.pages.len(), doc_side) {
+    if let Some(page_idx) = parent.parent_page_for_side(doc_side) {
         for item in &parent.items {
             if overridden.contains(&item.id) {
                 continue;
@@ -1071,7 +1071,7 @@ fn push_parent_length_chain(
         return;
     }
     let next = parent.parent.as_ref().and_then(|info| info.based_on);
-    if let Some(page_idx) = shown_parent_page_index(parent.pages.len(), doc_side) {
+    if let Some(page_idx) = parent.parent_page_for_side(doc_side) {
         for item in &parent.items {
             if overridden.contains(&item.id) {
                 continue;
@@ -1285,7 +1285,7 @@ fn offer_parent_chain(
         return;
     }
     let next = parent.parent.as_ref().and_then(|info| info.based_on);
-    if let Some(page_idx) = shown_parent_page_index(parent.pages.len(), doc_side)
+    if let Some(page_idx) = parent.parent_page_for_side(doc_side)
         && let Some(parent_page) = parent.pages.get(page_idx)
     {
         let dx = doc_x - parent_page.x;
@@ -1309,11 +1309,6 @@ fn offer_parent_chain(
     offer_parent_chain(best, cx, req, flags, axis, tol, doc_x, doc_side, overridden, next_index, seen);
 }
 
-/// Left page of a facing parent, otherwise the last page. Same choice as `Document::parent_page_for`.
-fn shown_parent_page_index(page_count: usize, side: PageSide) -> Option<usize> {
-    if page_count >= 2 && side == PageSide::Left { Some(0) } else { page_count.checked_sub(1) }
-}
-
 fn shift_x(r: Rect, dx: f64) -> Rect {
     Rect::new(r.x0 + dx, r.y0, r.x1 + dx, r.y1)
 }

crates/ui-egui/src/canvas.rs

@@ -634,9 +634,10 @@ fn draw_guides(app: &DesignApp, painter: &egui::Painter, xf: &Xf, doc: &Document
         let rect = |r: DRect| xf.rect(m.transform_rect_bbox(r));
         // Bleed: one rectangle around the spread (inside bleed only applies at the spread's outer edges).
         let b = s.bleed;
-        if app.ui.guides && b.iter().any(|v| *v > 0.0) && !sp.pages.is_empty() {
-            let first = &sp.pages[0];
-            let last = &sp.pages[sp.pages.len() - 1];
+        if app.ui.guides
+            && b.iter().any(|v| *v > 0.0)
+            && let Some((first, last)) = sp.outer_page_indices().and_then(|(l, r)| Some((sp.pages.get(l)?, sp.pages.get(r)?)))
+        {
             let l = if first.side == PageSide::Left { b[3] } else { b[2] };
             let r = if last.side == PageSide::Right || last.side == PageSide::Single { b[3] } else { b[2] };
             let sb = sp.bounds();