From 182db943c20ea7205af7578260ea424289b2cdc4 Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 00:51:41 +0000 Subject: [PATCH] fix(parents): bound every based-on walk; base items follow the parent showing them (#652) Cycle checks walk based_on with a seen-set over all parents, so a cycle longer than the drawing depth cap is still found. Snapping lists parent items through shown_parent_pages instead of recursing, and offers base items on a parent spread. A base parent's text on a parent spread is composed for that spread, so page-number markers show its prefix. Cmd+Shift-click hits what is drawn: it honours Show Parent Items, hidden and locked layers, the facing-page filter and layer order. IDML import warns when a parent spread's pages name different bases or their own spread. --- crates/compose/src/cache.rs | 41 +++++- crates/compose/src/lib.rs | 2 +- crates/doc/src/build.rs | 34 ++++- crates/engine/src/cmd/layout.rs | 28 +++++ crates/engine/src/cmd/overrides.rs | 34 +++++ crates/idml/src/import.rs | 17 ++- crates/pdf/src/tests.rs | 22 ++++ crates/render/src/fx.rs | 3 +- crates/render/src/lib.rs | 22 ++-- crates/render/src/text.rs | 4 +- crates/tools/src/lib.rs | 37 ++++-- crates/tools/src/snap.rs | 195 ++++++----------------------- crates/tools/src/tests.rs | 23 ++++ docs/gaps.md | 7 +- 14 files changed, 282 insertions(+), 187 deletions(-) diff --git a/crates/compose/src/cache.rs b/crates/compose/src/cache.rs index 8aabbf0f1..7388a9aed 100644 --- a/crates/compose/src/cache.rs +++ b/crates/compose/src/cache.rs @@ -13,8 +13,28 @@ use designcraft_doc::{Document, Item, Story, StoryId, Styles, WrapMode}; use crate::vars::RunningIndex; use crate::{ComposeOptions, ComposedStory}; -/// A story, composed for one absolute page (parent-page items) or for its own frames (`None`). -type Key = (StoryId, Option); +/// Where a story's frames are shown, which decides what its page-number markers and variables +/// read. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +pub enum ShownOn { + /// In its own frames. + #[default] + Own, + /// As a parent item on this absolute document page. + Page(usize), + /// As a base parent's item on this parent spread (index into `parents`): markers show that + /// parent's prefix. + Parent(usize), +} + +impl From> for ShownOn { + fn from(page: Option) -> Self { + page.map_or(ShownOn::Own, ShownOn::Page) + } +} + +/// A story, composed for where it is shown. +type Key = (StoryId, ShownOn); struct Entry { sig: Vec, @@ -61,12 +81,21 @@ impl Cache { /// `None` for a story's own frames. Pages are identified by index, not name: names repeat when /// sections restart their numbering. pub fn get(&self, doc: &Document, sid: StoryId, page: Option) -> Arc { + self.get_on(doc, sid, page.into()) + } + + /// [`Cache::get`] for any [`ShownOn`]. + pub fn get_on(&self, doc: &Document, sid: StoryId, on: ShownOn) -> Arc { let Some(story) = doc.stories.get(&sid) else { return Arc::new(ComposedStory { story: sid, ..Default::default() }) }; let (mut sig, keep_items) = signature(doc, story); - let key = (sid, page); + let key = (sid, on); // The key holds the page's index, which a renumbering doesn't change: its name (section // start number, style, prefix) is part of the signature. - let page_name = page.map(|p| doc.page_name(p)); + let (page_name, page) = match on { + ShownOn::Own => (None, None), + ShownOn::Page(p) => (Some(doc.page_name(p)), Some(p)), + ShownOn::Parent(pi) => (doc.parents.get(pi).and_then(|sp| sp.parent.as_ref()).map(|i| i.prefix.clone()), None), + }; if let Some(name) = &page_name { use std::hash::{Hash, Hasher}; let mut h = std::collections::hash_map::DefaultHasher::new(); @@ -153,7 +182,7 @@ impl Cache { } let replaced = g.0.insert(key, Entry { sig, _keep: (story.clone(), doc.styles.clone(), keep_items), out: out.clone(), stamp }); drop(g); - if key.1.is_none() + if key.1 == ShownOn::Own && let Some(old) = replaced { self.previous.lock().unwrap_or_else(|e| e.into_inner()).insert(sid, (old.sig, old.out)); @@ -170,7 +199,7 @@ impl Cache { let matches = |s: &[usize]| s.len() >= sig.len() && s[..sig.len()] == sig[..]; { let g = self.map.lock().unwrap_or_else(|e| e.into_inner()); - if let Some(e) = g.0.get(&(sid, None)) + if let Some(e) = g.0.get(&(sid, ShownOn::Own)) && matches(&e.sig) { return Some(e.out.clone()); diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index 3dd7c25b3..91ea88d47 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -36,7 +36,7 @@ use designcraft_fonts::{FontDb, ScopedFonts}; use designcraft_geom::{Point, Rect}; use crate::breaker::{Break, Spacing}; -pub use crate::cache::Cache; +pub use crate::cache::{Cache, ShownOn}; use crate::notes::Notes; pub use crate::notes::{find_note, hit_note, note_caret}; use crate::shape::{Glyph, StyleTable, SubstCtx}; diff --git a/crates/doc/src/build.rs b/crates/doc/src/build.rs index 4045819b5..ff510b41a 100644 --- a/crates/doc/src/build.rs +++ b/crates/doc/src/build.rs @@ -588,7 +588,20 @@ impl Document { /// Whether basing parent `pi` on `base` would make a `based_on` cycle (or names no parent). pub fn parent_base_cycles(&self, pi: usize, base: SpreadId) -> bool { - self.parent_index(base).is_none_or(|bi| bi == pi || self.parent_chain(bi).contains(&pi)) + let Some(mut cur) = self.parent_index(base) else { return true }; + // Not `parent_chain`: its depth cap would miss a long cycle. Every parent is visited at most once. + let mut seen = std::collections::HashSet::new(); + loop { + if cur == pi { + return true; + } + if !seen.insert(cur) || seen.len() > self.parents.len() { + return false; + } + let next = self.parents.get(cur).and_then(|ps| ps.parent.as_ref()).and_then(|info| info.based_on).and_then(|b| self.parent_index(b)); + let Some(n) = next else { return false }; + cur = n; + } } /// Clear `based_on` links that name a missing parent or close a cycle (hand-edited or damaged @@ -747,4 +760,23 @@ mod tests { assert_eq!(d.parents[1].parent.as_ref().unwrap().based_on, Some(a)); assert_eq!(d.parents[2].parent.as_ref().unwrap().based_on, None); } + + /// A cycle longer than the drawing depth cap is still found and broken. + #[test] + fn fix_parent_bases_breaks_a_long_cycle() { + let mut d = Document::new(&NewDocument { pages: 1, facing_pages: false, ..Default::default() }); + for i in 1..65 { + d.add_parent(&format!("P{i}"), "Parent", 1, 12.0, Margins::uniform(36.0)); + } + assert_eq!(d.parents.len(), 65); + let ids: Vec = d.parents.iter().map(|p| p.id).collect(); + for i in 0..65 { + base(&mut d, i, Some(ids[(i + 1) % 65])); + } + assert_eq!(d.parent_chain(0).len(), MAX_PARENT_DEPTH); + let w = d.fix_parent_bases(); + assert_eq!(w.len(), 1, "{w:?}"); + assert_eq!(d.parents[0].parent.as_ref().unwrap().based_on, None); + assert!((0..65).all(|i| d.parent_base_cycles(i, ids[(i + 1) % 65]) == (i == 0)), "only the cleared link would close it again"); + } } diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 7ff43c5d9..6a8519353 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -1005,6 +1005,13 @@ mod parent_base_tests { assert!(cyan(img.pixel(150, 150)), "B's item over A's: {:?}", img.pixel(150, 150)); assert!(paper(img.pixel(170, 20)), "the page's item over B's: {:?}", img.pixel(170, 20)); assert!(black(page(&s, 0).pixel(50, 100)) && !cyan(page(&s, 0).pixel(250, 100)), "page 1 uses A only"); + // Parent B itself shows A's items under its own. + let mut rr = designcraft_render::Renderer::new(); + rr.threads = 0; + let placed = [designcraft_render::Placed { spread: designcraft_doc::SpreadRef::Parent(1), xf: designcraft_geom::Affine::IDENTITY }]; + let opts = designcraft_render::RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() }; + let img = rr.render(&s.doc().unwrap().doc, &s.cache, &placed, 400, 400, designcraft_geom::Affine::IDENTITY, &opts); + assert!(black(img.pixel(50, 100)) && cyan(img.pixel(150, 150)), "{:?} {:?}", img.pixel(50, 100), img.pixel(150, 150)); let inspect = s.execute("document.inspect", &json!({})).unwrap(); assert_eq!(inspect["parents"][1]["basedOn"], "A-Parent"); assert_eq!(inspect["parents"][0]["basedOn"], Value::Null); @@ -1021,6 +1028,27 @@ mod parent_base_tests { assert_eq!(s.execute("document.inspect", &json!({})).unwrap()["parents"][1]["basedOn"], Value::Null); } + /// A's page-number marker shown on parent XYZ (based on A) reads XYZ, as markers on XYZ's own items do. + #[test] + fn base_parent_markers_read_the_showing_parent() { + let mut s = crate::Session::new(); + s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap(); + s.execute("layout.parents.new", &json!({"prefix": "XYZ"})).unwrap(); + s.execute("layout.parents.options", &json!({"parent": "XYZ", "basedOn": "A"})).unwrap(); + let marker = designcraft_doc::story::PAGE_NUMBER.to_string(); + let r = s + .execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [72, 30, 500, 60], "content": "text", "text": marker})) + .unwrap(); + let d = s.doc().unwrap().doc.clone(); + let sid = d.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story; + let glyphs = |on| -> usize { + let cs = s.cache.get_on(&d, sid, on); + cs.frames.iter().flat_map(|f| f.lines.iter()).map(|l| l.glyphs.iter().filter(|g| g.visible && g.adv > 0.0).count()).sum() + }; + assert_eq!(glyphs(designcraft_compose::ShownOn::Own), 1, "A"); + assert_eq!(glyphs(designcraft_compose::ShownOn::Parent(1)), 3, "XYZ"); + } + /// A file whose parents are based on each other opens, with the cycle broken and reported. #[test] fn opening_a_parent_cycle_breaks_it_with_a_warning() { diff --git a/crates/engine/src/cmd/overrides.rs b/crates/engine/src/cmd/overrides.rs index ac96b8e48..3fd8b2e80 100644 --- a/crates/engine/src/cmd/overrides.rs +++ b/crates/engine/src/cmd/overrides.rs @@ -216,4 +216,38 @@ mod tests { assert_eq!(rest.iter().map(|i| d.item(*i).unwrap().overrides).collect::>(), vec![Some(b_item)]); d.check().unwrap(); } + + /// Facing pages: 2-page B based on 2-page A. Each document page offers only the items on its + /// side of A, and the copy lands where the item showed: page 1 (a single right page at x 0) + /// shows A's right-page items 612 pt to the left. + #[test] + fn override_base_items_on_facing_pages_keeps_their_place() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 3, "facingPages": true})).unwrap(); + s.execute("layout.parents.new", &json!({"prefix": "B"})).unwrap(); + s.execute("layout.parents.options", &json!({"parent": "B", "basedOn": "A"})).unwrap(); + s.execute("layout.pages.applyParent", &json!({"pages": [0, 1, 2], "parent": "B"})).unwrap(); + let on_a = |s: &mut Session, rect: [f64; 4]| { + let r = s.execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": rect, "content": "unassigned"})).unwrap(); + ItemId(r["id"].as_u64().unwrap()) + }; + let left = on_a(&mut s, [72.0, 72.0, 200.0, 200.0]); + let right = on_a(&mut s, [684.0, 72.0, 812.0, 200.0]); + let copy_of = |s: &mut Session, page: usize, id: ItemId| -> Option { + let r = s.execute("layout.overrideParentItems", &json!({"page": page, "ids": [id.0]})).unwrap(); + r["ids"].as_array().unwrap().first().map(|v| ItemId(v.as_u64().unwrap())) + }; + assert_eq!(copy_of(&mut s, 0, left), None, "A's left-page item does not show on a right page"); + let c = copy_of(&mut s, 0, right).unwrap(); + let x0 = |s: &Session, id: ItemId| s.doc().unwrap().doc.item(id).unwrap().bounds().x0; + assert!((x0(&s, c) - 72.0).abs() < 1e-9, "{}", x0(&s, c)); + // Page 2 is the left page of the second spread, page 3 its right page at x 612. + let c = copy_of(&mut s, 1, left).unwrap(); + assert!((x0(&s, c) - 72.0).abs() < 1e-9, "{}", x0(&s, c)); + assert_eq!(s.doc().unwrap().doc.page_of_item(c), Some(1)); + let c = copy_of(&mut s, 2, right).unwrap(); + assert!((x0(&s, c) - 684.0).abs() < 1e-9, "{}", x0(&s, c)); + assert_eq!(s.doc().unwrap().doc.page_of_item(c), Some(2)); + s.doc().unwrap().doc.check().unwrap(); + } } diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 949abf3f4..5e42004f4 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -107,6 +107,7 @@ pub fn import_idml_report(bytes: &[u8], read_link: &dyn Fn(&str) -> Option IdmlError::Invalid(format!("{msg} ({})", warnings.join("; "))), @@ -190,6 +191,8 @@ struct Importer<'r> { stories: BTreeMap, story_ids: HashMap, parents: Vec, + /// Things read but not kept as they were, for the import report. + warnings: Vec, parent_ids: HashMap, spreads: Vec, page_index: HashMap, @@ -277,6 +280,7 @@ impl<'r> Importer<'r> { stories: BTreeMap::new(), story_ids: HashMap::new(), parents: Vec::new(), + warnings: Vec::new(), parent_ids: HashMap::new(), spreads: Vec::new(), page_index: HashMap::new(), @@ -1965,7 +1969,18 @@ impl<'r> Importer<'r> { let n = e.get("Name").unwrap_or("Parent"); n.split_once('-').map(|(_, b)| b.to_string()).unwrap_or_else(|| n.to_string()) }); - let based_on = pages.first().and_then(|(_, p)| p.get("AppliedMaster")).and_then(|m| self.parent_ids.get(m)).copied().filter(|b| *b != id); + // A parent spread has one base: its first page's. Pages that disagree lose theirs. + let applied: Vec> = + pages.iter().map(|(_, p)| p.get("AppliedMaster").and_then(|m| self.parent_ids.get(m)).copied()).collect(); + let label = format!("{prefix}-{name}"); + if applied.windows(2).any(|w| w[0] != w[1]) { + self.warnings.push(format!("parent {label}: its pages are based on different parents; only the first page's is kept")); + } + let based_on = applied.first().copied().flatten(); + if applied.contains(&Some(id)) { + self.warnings.push(format!("parent {label}: a page names its own parent as its base, which is ignored")); + } + let based_on = based_on.filter(|b| *b != id); Some(ParentInfo { prefix, name, based_on }) } else { None diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 9ac53d057..609a4b8d3 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -446,3 +446,25 @@ fn column_rules_are_drawn() { assert!((xs[0] - 214.0).abs() < 0.01 && (xs[1] - 398.0).abs() < 0.01, "{xs:?}"); assert!(rules(false).is_empty()); } + +/// A page whose parent B is based on A carries A's items, then B's. +#[test] +fn based_on_parent_items_export_base_first() { + let mut d = Document::new(&NewDocument { pages: 1, facing_pages: false, ..NewDocument::default() }); + d.swatches.push(Swatch::color("Base Cyan", Color::cmyk(1.0, 0.0, 0.0, 0.0))); + d.swatches.push(Swatch::color("Parent Magenta", Color::cmyk(0.0, 1.0, 0.0, 0.0))); + let a = d.parents[0].id; + let b = d.add_parent("B", "Opening", 1, 12.0, designcraft_doc::Margins::uniform(36.0)); + std::sync::Arc::make_mut(&mut d.parents[1]).parent.as_mut().unwrap().based_on = Some(a); + d.apply_parent(&[0], Some(b)).unwrap(); + let lid = d.default_layer(); + for (pi, swatch, r) in [(0, "Base Cyan", Rect::new(36.0, 36.0, 136.0, 136.0)), (1, "Parent Magenta", Rect::new(200.0, 36.0, 300.0, 136.0))] { + let mut it = Item::new(ItemId(d.alloc()), lid, designcraft_doc::Shape::Rectangle, designcraft_geom::shapes::rectangle(r)); + it.fill = Fill::swatch(swatch); + d.insert_item(SpreadRef::Parent(pi), it, None).unwrap(); + } + let s = uncompressed(&d, PdfOptions::default()); + let (cyan, magenta) = (s.find("1 0 0 0 k"), s.find("0 1 0 0 k")); + assert!(cyan.is_some() && magenta.is_some(), "both parents' items are on the page"); + assert!(cyan < magenta, "the base parent's item is drawn first"); +} diff --git a/crates/render/src/fx.rs b/crates/render/src/fx.rs index 6b29516f4..b52f7c28d 100644 --- a/crates/render/src/fx.rs +++ b/crates/render/src/fx.rs @@ -23,6 +23,7 @@ use vello_cpu::peniko::{self, BlendMode, Compose, Mix}; use vello_cpu::{RenderContext, kurbo}; use crate::{Frame, Renderer, color_of}; +use designcraft_compose::ShownOn; /// Shadow offset (spread units) for an InDesign angle (degrees, light direction) and distance. pub(crate) fn offset(angle: f64, distance: f64) -> Vec2 { @@ -66,7 +67,7 @@ fn shadow(sigma: f64, color: peniko::Color) -> Filter { impl Renderer { /// Draw an item with soft effects: below-effects, the (feathered) body, inner shadow. - pub(crate) fn draw_item_fx(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option) { + pub(crate) fn draw_item_fx(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: ShownOn) { let doc = f.doc; let e = &it.effects; let sw = if it.stroke.is_none() { 0.0 } else { it.stroke.extent() }; diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 7b42dcbb1..4c9c5e4b6 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -13,7 +13,7 @@ use std::collections::HashMap; use std::sync::Arc; use designcraft_color::BlendMode as DcBlend; -use designcraft_compose::{Cache, ComposedStory}; +use designcraft_compose::{Cache, ComposedStory, ShownOn}; use designcraft_doc::{Content, Document, Item, PageSide, SpreadRef, StoryId, StrokeAlign, StrokeType}; use designcraft_geom::corners; use designcraft_geom::{Affine, BezPath, Rect, Shape, Vec2}; @@ -216,7 +216,7 @@ pub struct Renderer { resources: Resources, glyphs: text::GlyphCache, /// Composed stories looked up during the current render. - stories: HashMap<(StoryId, Option), Arc>, + stories: HashMap<(StoryId, ShownOn), Arc>, pub threads: u16, pub stats: FrameStats, } @@ -404,6 +404,12 @@ impl Renderer { SpreadRef::Doc(si) => Some(doc.first_page_of_spread(si)), SpreadRef::Parent(_) => None, }; + // Base items compose for where they show: the document page, or this parent spread. + let shown_on = |pi: usize| match (sr, first) { + (SpreadRef::Parent(p), _) => ShownOn::Parent(p), + (_, Some(a)) => ShownOn::Page(a + pi), + (_, None) => ShownOn::Own, + }; for (pi, page) in sp.pages.iter().enumerate() { if !page.show_parent_items { continue; @@ -420,19 +426,19 @@ impl Renderer { if parent.page_at_x(it.bounds().center().x) != Some(ppage) && parent.pages.len() > 1 { continue; } - self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), first.map(|a| a + pi)); + self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), shown_on(pi)); } } } for it in &sp.items { if it.layer == layer.id { - self.draw_item(ctx, f, it, Affine::IDENTITY, None); + self.draw_item(ctx, f, it, Affine::IDENTITY, ShownOn::Own); } } } } - fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page: Option) { + fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page: ShownOn) { if it.hidden || f.opts.hidden.contains(&it.id) || f.opts.printing_only && it.nonprinting { return; } @@ -532,18 +538,18 @@ impl Renderer { /// The composed story for a frame (memoised for the current render: the cache lookup /// validates the whole thread each time). - fn story(&mut self, f: &Frame, sid: StoryId, page: Option) -> Arc { + fn story(&mut self, f: &Frame, sid: StoryId, page: ShownOn) -> Arc { let key = (sid, page); if let Some(cs) = self.stories.get(&key) { return cs.clone(); } - let cs = f.cache.get(f.doc, sid, page); + let cs = f.cache.get_on(f.doc, sid, page); self.stories.insert(key, cs.clone()); cs } /// Fill, content and stroke of a leaf item (no opacity, blend or effects). - pub(crate) fn draw_body(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option) { + pub(crate) fn draw_body(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: ShownOn) { let doc = f.doc; // Fill. if !it.fill.is_none() { diff --git a/crates/render/src/text.rs b/crates/render/src/text.rs index 734ec275e..e3d367889 100644 --- a/crates/render/src/text.rs +++ b/crates/render/src/text.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use std::sync::Arc; -use designcraft_compose::{ComposedStory, FrameText, Line}; +use designcraft_compose::{ComposedStory, FrameText, Line, ShownOn}; use designcraft_doc::ItemId; use designcraft_fonts::FontDb; use designcraft_geom::{Affine, BezPath, Rect, Shape}; @@ -410,7 +410,7 @@ impl Renderer { { for o in &ft.objects { if let Some(obj) = st.objects.get(o.index) { - self.draw_item(ctx, f, &obj.item, xf * Affine::translate(o.origin.to_vec2()), None); + self.draw_item(ctx, f, &obj.item, xf * Affine::translate(o.origin.to_vec2()), ShownOn::Own); } } } diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index e58be7508..0ec0b05a8 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -224,17 +224,32 @@ impl ToolContext<'_> { let pi = spread.page_at_x(sp.x)?; let page = self.doc.first_page_of_spread(si) + pi; let pg = spread.pages.get(pi)?; - // Front to back: the page's parent, then the parents it is based on. - self.doc.shown_parent_pages(SpreadRef::Doc(si), pi).into_iter().rev().find_map(|(ppi, ppg)| { - let parent = self.doc.parents.get(ppi)?; - let at = sp - designcraft_geom::Vec2::new(pg.x - parent.pages.get(ppg)?.x, 0.0); - parent - .items - .iter() - .rev() - .find(|it| !it.locked && !pg.overridden.contains(&it.id) && designcraft_doc::edit_hit(it, at, self.tol(4.0))) - .map(|it| (page, it.id)) - }) + if !pg.show_parent_items { + return None; + } + // What the renderer draws on this page, in its order: by layer, then the parent's bases + // before the parent. The last hit is the topmost. + let mut shown = Vec::new(); + for (ppi, ppg) in self.doc.shown_parent_pages(SpreadRef::Doc(si), pi) { + let Some(parent) = self.doc.parents.get(ppi) else { continue }; + let Some(parent_page) = parent.pages.get(ppg) else { continue }; + let off = designcraft_geom::Vec2::new(pg.x - parent_page.x, 0.0); + for it in &parent.items { + if pg.overridden.contains(&it.id) || parent.pages.len() > 1 && parent.page_at_x(it.bounds().center().x) != Some(ppg) { + continue; + } + shown.push((it, off)); + } + } + shown.sort_by_key(|(it, _)| self.doc.layer_rank(it.layer)); + shown + .into_iter() + .rev() + .find(|(it, off)| { + let layer = self.doc.layer(it.layer); + !it.locked && !it.hidden && !layer.is_some_and(|l| !l.visible || l.locked) && designcraft_doc::edit_hit(it, sp - *off, self.tol(4.0)) + }) + .map(|(it, _)| (page, it.id)) } fn hit_parent(&self, sr: SpreadRef, sp: Point) -> Option<(SpreadRef, ItemId)> { diff --git a/crates/tools/src/snap.rs b/crates/tools/src/snap.rs index 6d5c8f6a5..4471abcea 100644 --- a/crates/tools/src/snap.rs +++ b/crates/tools/src/snap.rs @@ -8,7 +8,7 @@ use std::collections::HashSet; -use designcraft_doc::{Item, ItemId, Orientation, PageSide, Selection, SpreadId, SpreadRef, StrokeAlign}; +use designcraft_doc::{Document, Item, ItemId, Orientation, Selection, SpreadRef, StrokeAlign}; use designcraft_geom::snap::snap_to_grid; use designcraft_geom::{Affine, Point, Rect, Vec2, format_measure}; @@ -204,15 +204,13 @@ fn match_rotation(cx: &ToolContext, req: &SnapRequest<'_>, zone_length: f64) -> push_rotation(&mut best, cx.selection, req, item, Affine::IDENTITY, false, 0, &mut seen, proposed, tol); } } - // Parent editing already lists those items on the spread. Do not add them again. - if !layout_edits_parents(cx) { + if shows_parent_items(cx, req) { push_parent_rotations(&mut best, cx, req, &mut seen, proposed, tol); } best.map(|(_, angle)| angle) } -/// Shown parent items, same page and `based_on` walk as parent lengths. -/// Parent items are only translated onto the page, so the angle is still `item.xf`. +/// Shown parent items. Parent items are only translated onto the page, so the angle is still `item.xf`. fn push_parent_rotations( best: &mut Option<(f64, f64)>, cx: &ToolContext, @@ -221,57 +219,11 @@ fn push_parent_rotations( proposed: f64, tol: f64, ) { - let SpreadRef::Doc(si) = req.spread else { return }; - let page_count = cx.doc.spreads.get(si).map(|sp| sp.pages.len()).unwrap_or(0); - let first = cx.doc.first_page_of_spread(si); - for pi in 0..page_count { - let Some(abs) = first.checked_add(pi) else { continue }; - let Some(page) = cx.doc.page(abs) else { continue }; - if !page.show_parent_items { - continue; - } - let doc_side = page.side; - let overridden = page.overridden.clone(); - let Some((ppi, _)) = cx.doc.parent_page_for(abs) else { continue }; - let mut seen_spreads = HashSet::new(); - push_parent_rotation_chain(best, cx, req, doc_side, &overridden, ppi, seen_items, &mut seen_spreads, proposed, tol); + for (item, _) in shown_parent_items(cx.doc, req.spread) { + push_rotation(best, cx.selection, req, item, Affine::IDENTITY, false, 0, seen_items, proposed, tol); } } -fn push_parent_rotation_chain( - best: &mut Option<(f64, f64)>, - cx: &ToolContext, - req: &SnapRequest<'_>, - doc_side: PageSide, - overridden: &[ItemId], - index: usize, - seen_items: &mut HashSet, - seen_spreads: &mut HashSet, - proposed: f64, - tol: f64, -) { - let Some(parent) = cx.doc.parents.get(index) else { return }; - if !seen_spreads.insert(parent.id) { - 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) { - for item in &parent.items { - if overridden.contains(&item.id) { - continue; - } - // A facing parent only shows items whose center sits on this document page's parent page. - if parent.pages.len() > 1 && parent.page_at_x(item.bounds().center().x) != Some(page_idx) { - continue; - } - push_rotation(best, cx.selection, req, item, Affine::IDENTITY, false, 0, seen_items, proposed, tol); - } - } - let Some(next_id) = next else { return }; - let Some(next_index) = cx.doc.parent_index(next_id) else { return }; - push_parent_rotation_chain(best, cx, req, doc_side, overridden, next_index, seen_items, seen_spreads, proposed, tol); -} - fn push_rotation( best: &mut Option<(f64, f64)>, selection: &Selection, @@ -938,7 +890,7 @@ fn dimension_lengths(cx: &ToolContext, req: &SnapRequest<'_>) -> Vec { push_own_lengths(&mut out, cx.selection, req, item, Affine::IDENTITY, false, 0, &mut seen); } } - if !layout_edits_parents(cx) { + if shows_parent_items(cx, req) { push_parent_lengths(&mut out, cx, req, &mut seen); } out @@ -1021,54 +973,11 @@ fn stroke_outset(item: &Item) -> f64 { } fn push_parent_lengths(out: &mut Vec, cx: &ToolContext, req: &SnapRequest<'_>, seen_items: &mut HashSet) { - let SpreadRef::Doc(si) = req.spread else { return }; - let page_count = cx.doc.spreads.get(si).map(|sp| sp.pages.len()).unwrap_or(0); - let first = cx.doc.first_page_of_spread(si); - for pi in 0..page_count { - let Some(abs) = first.checked_add(pi) else { continue }; - let Some(page) = cx.doc.page(abs) else { continue }; - if !page.show_parent_items { - continue; - } - let doc_side = page.side; - let overridden = page.overridden.clone(); - let Some((ppi, _)) = cx.doc.parent_page_for(abs) else { continue }; - let mut seen_spreads = HashSet::new(); - push_parent_length_chain(out, cx, req, doc_side, &overridden, ppi, seen_items, &mut seen_spreads); + for (item, _) in shown_parent_items(cx.doc, req.spread) { + push_own_lengths(out, cx.selection, req, item, Affine::IDENTITY, false, 0, seen_items); } } -fn push_parent_length_chain( - out: &mut Vec, - cx: &ToolContext, - req: &SnapRequest<'_>, - doc_side: PageSide, - overridden: &[ItemId], - index: usize, - seen_items: &mut HashSet, - seen_spreads: &mut HashSet, -) { - let Some(parent) = cx.doc.parents.get(index) else { return }; - if !seen_spreads.insert(parent.id) { - 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) { - for item in &parent.items { - if overridden.contains(&item.id) { - continue; - } - if parent.pages.len() > 1 && parent.page_at_x(item.bounds().center().x) != Some(page_idx) { - continue; - } - push_own_lengths(out, cx.selection, req, item, Affine::IDENTITY, false, 0, seen_items); - } - } - let Some(next_id) = next else { return }; - let Some(next_index) = cx.doc.parent_index(next_id) else { return }; - push_parent_length_chain(out, cx, req, doc_side, overridden, next_index, seen_items, seen_spreads); -} - /// Ruler guides, margins, and column sides. No overlay. /// /// A page guide attracts when the moving rect overlaps that page on the other axis. A spread @@ -1143,7 +1052,7 @@ fn pass_align(cx: &ToolContext, req: &SnapRequest<'_>, axis: Axis, tol: f64) -> offer_box(&mut best, req.rect, flags, axis, rect, tol, cx.snap.align_edges, cx.snap.align_centers); } } - if !layout_edits_parents(cx) { + if shows_parent_items(cx, req) { offer_parent_items(&mut best, cx, req, flags, axis, tol); } best.map(|b| b.hit) @@ -1159,6 +1068,12 @@ fn layout_edits_parents(cx: &ToolContext) -> bool { cx.layout.slots.iter().any(|slot| matches!(slot.spread, SpreadRef::Parent(_))) } +/// Whether to offer the parent items drawn on `req.spread`. A parent spread shows its bases' items, +/// which are not on it. +fn shows_parent_items(cx: &ToolContext, req: &SnapRequest<'_>) -> bool { + matches!(req.spread, SpreadRef::Parent(_)) || !layout_edits_parents(cx) +} + /// Visible box used for alignment. Groups with children contribute the union of the children. fn item_target_rect(selection: &Selection, req: &SnapRequest<'_>, item: &Item) -> Option { if item.hidden || align_skips_item(selection, req, item.id) { @@ -1228,72 +1143,44 @@ fn group_box(item: &Item, seen: &mut HashSet, depth: usize, filter: Opti acc.filter(|r| rect_finite(*r)) } -/// Parent items shown on the document spread, shifted into that spread's space. +/// Parent items shown on the spread, shifted into that spread's space. fn offer_parent_items(best: &mut Option, cx: &ToolContext, req: &SnapRequest<'_>, flags: [bool; 3], axis: Axis, tol: f64) { - let SpreadRef::Doc(si) = req.spread else { return }; - let page_count = cx.doc.spreads.get(si).map(|sp| sp.pages.len()).unwrap_or(0); - let first = cx.doc.first_page_of_spread(si); - for pi in 0..page_count { - let Some(abs) = first.checked_add(pi) else { continue }; - let Some(page) = cx.doc.page(abs) else { continue }; + for (item, dx) in shown_parent_items(cx.doc, req.spread) { + let Some(rect) = item_target_rect(cx.selection, req, item) else { continue }; + offer_box(best, req.rect, flags, axis, shift_x(rect, dx), tol, cx.snap.align_edges, cx.snap.align_centers); + } +} + +/// Parent items drawn on `spread`, back to front, each with the x shift from its parent spread: +/// a document page's parent and its bases, or a parent spread's bases. Same page and override +/// filters as the renderer. +fn shown_parent_items(doc: &Document, spread: SpreadRef) -> Vec<(&Item, f64)> { + let Some(sp) = doc.spread(spread) else { return Vec::new() }; + let mut out = Vec::new(); + for (pi, page) in sp.pages.iter().enumerate() { if !page.show_parent_items { continue; } - let doc_x = page.x; - let doc_side = page.side; - let overridden = page.overridden.clone(); - let Some((ppi, _)) = cx.doc.parent_page_for(abs) else { continue }; - let mut seen = HashSet::new(); - offer_parent_chain(best, cx, req, flags, axis, tol, doc_x, doc_side, &overridden, ppi, &mut seen); - } -} - -/// Walk `based_on`. A repeated spread id ends the walk. -fn offer_parent_chain( - best: &mut Option, - cx: &ToolContext, - req: &SnapRequest<'_>, - flags: [bool; 3], - axis: Axis, - tol: f64, - doc_x: f64, - doc_side: PageSide, - overridden: &[ItemId], - index: usize, - seen: &mut HashSet, -) { - let Some(parent) = cx.doc.parents.get(index) else { return }; - if !seen.insert(parent.id) { - 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) - && let Some(parent_page) = parent.pages.get(page_idx) - { - let dx = doc_x - parent_page.x; - if dx.is_finite() { + for (ppi, ppage) in doc.shown_parent_pages(spread, pi) { + let Some(parent) = doc.parents.get(ppi) else { continue }; + let Some(parent_page) = parent.pages.get(ppage) else { continue }; + let dx = page.x - parent_page.x; + if !dx.is_finite() { + continue; + } for item in &parent.items { - if overridden.contains(&item.id) { + if page.overridden.contains(&item.id) { continue; } - // Same other-page skip as the renderer: a facing parent only shows items - // whose center sits on the parent page this document page uses. - if parent.pages.len() > 1 && parent.page_at_x(item.bounds().center().x) != Some(page_idx) { + // A facing parent only shows items whose center sits on the parent page this page uses. + if parent.pages.len() > 1 && parent.page_at_x(item.bounds().center().x) != Some(ppage) { continue; } - let Some(rect) = item_target_rect(cx.selection, req, item) else { continue }; - offer_box(best, req.rect, flags, axis, shift_x(rect, dx), tol, cx.snap.align_edges, cx.snap.align_centers); + out.push((item.as_ref(), dx)); } } } - let Some(next_id) = next else { return }; - let Some(next_index) = cx.doc.parent_index(next_id) else { return }; - 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 { - if page_count >= 2 && side == PageSide::Left { Some(0) } else { page_count.checked_sub(1) } + out } fn shift_x(r: Rect, dx: f64) -> Rect { diff --git a/crates/tools/src/tests.rs b/crates/tools/src/tests.rs index 6c75b0da6..7f1abc1ca 100644 --- a/crates/tools/src/tests.rs +++ b/crates/tools/src/tests.rs @@ -83,6 +83,29 @@ fn cmd_shift_click_overrides_parent_item() { assert_eq!(a, vec![Action::Exec("layout.overrideParentItems".into(), serde_json::json!({"page": 0, "ids": [pid.0]}))]); } +/// On a page whose parent B is based on A, A's items are hit too, B's win where they overlap, and +/// nothing is hit when the page hides parent items. +#[test] +fn parent_hit_reaches_base_parent_items_topmost_first() { + let mut d = Document::new(&NewDocument { pages: 1, facing_pages: false, ..Default::default() }); + let lid = d.default_layer(); + let a = d.parents[0].id; + let b = d.add_parent("B", "Opening", 1, 12.0, designcraft_doc::Margins::uniform(36.0)); + Arc::make_mut(&mut d.parents[1]).parent.as_mut().unwrap().based_on = Some(a); + d.apply_parent(&[0], Some(b)).unwrap(); + let (a_item, _) = d.add_text_frame(SpreadRef::Parent(0), Rect::new(100.0, 100.0, 200.0, 200.0), lid, "a", ParaFormat::default()).unwrap(); + let (b_item, _) = d.add_text_frame(SpreadRef::Parent(1), Rect::new(150.0, 150.0, 250.0, 250.0), lid, "b", ParaFormat::default()).unwrap(); + let hit = |d: &Document, x: f64, y: f64| { + let (s, c, l) = (Selection::default(), Cache::new(), CanvasLayout::new(d, false)); + let off = l.xf(SpreadRef::Doc(0)).translation(); + ctx(d, &s, &c, &l).hit_parent_on_page(Point::new(x + off.x, y + off.y)) + }; + assert_eq!(hit(&d, 120.0, 120.0), Some((0, a_item))); + assert_eq!(hit(&d, 175.0, 175.0), Some((0, b_item)), "B's item is drawn over A's"); + Arc::make_mut(&mut d.spreads[0]).pages[0].show_parent_items = false; + assert_eq!(hit(&d, 120.0, 120.0), None); +} + #[test] fn gradient_tool_drag_sets_the_vector() { let mut d = Document::new(&NewDocument::default()); diff --git a/docs/gaps.md b/docs/gaps.md index ebd2d7da5..b00ddfb83 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Where DesignCraft falls short of InDesign -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (book chapter update safety) · **Target:** Adobe InDesign 2026 (21.6) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (Based on Parent leftovers) · **Target:** Adobe InDesign 2026 (21.6) Every known shortfall against InDesign 2026, one entry each, ranked by how much it stops an InDesign professional from doing real work. This is the work list: agents pick from the top. @@ -165,7 +165,9 @@ Kind: **F** feature · **U** UI/UX · **T** typography · **FF** file format · Profile / Edit Original, File ▸ File Info / Open PDF / Document and Print Presets / User Settings, Type ▸ Size presets, Table ▸ Table Options / Cell Options submenus, Paste Before/After, Convert Rows/Columns, Object ▸ Interactive ▸ Convert to (form fields), Flex Layout (new in 2026), - View ▸ Extras badges and Story Editor options. + View ▸ Extras badges and Story Editor options. Pages panel ▸ Parent Options (Based on Parent is + the `layout.parents.options` command only); overriding a base parent's items on a parent spread + itself (overrides work on document pages only). - **Estimate:** 40–70 h (most are thin wrappers over existing engine features). - **Doc:** [ui-parity.md](ui-parity.md) @@ -227,5 +229,6 @@ Cue. These are excluded from every percentage. | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Gap 16: Parent Options dialog and overrides of base items on a parent spread | | 2026-10-10 | minor | Gap 0 added and marked alpha blocker: manual text threading is missing (stage re-normalized to pre-alpha) | | 2026-10-10 | major | Created: 25 ranked gaps from the full re-measure against InDesign 2026 21.6 and issues #1–#352 |