From 4ac4219fd24361fc240382a479a7171007b45a70 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 28 Oct 2026 10:23:57 +0000 Subject: [PATCH 1/3] compose: settle a story's own anchored wraps by object count, not 4 passes A story whose custom or above-line anchored objects wrap its own later lines is composed first without those wraps, then again with each object where the previous pass placed it. That loop stopped after a fixed MAX_PASSES = 4 extra compositions. Each object's wrap pushes down the anchors of the objects after it, so in a chain of such objects the k-th object only lands where it belongs once the k - 1 wraps before it are in place. From roughly the fifth chained object on, the loop ran out before the story settled: the final composition placed the object in one spot but wrapped the text around where an earlier pass had put it, and lines printed straight over the object. Bound the loop by the story's own wrapping objects instead: n + 1 passes for n such objects (inline objects never wrap their own story and are left out). Composition 0 has no own wraps, so object 1 is already where it belongs; by induction pass k places object k + 1 correctly, pass n places the last one, and pass n + 1 wraps the text around it, after which the positions cannot move. The existing settled check still stops a story early as soon as its objects stop moving, so stories that settle sooner cost nothing extra. MAX_PASSES stays as an absolute safety net, raised to 64, for layouts that keep moving their objects (keep options, vertical justification) and for stories with very many anchored objects. Test: chained_anchored_objects_all_wrap_their_own_storys_lines anchors eight 100 x 60 pt bounding-box objects six lines apart in one story, each hanging from its anchor line, and checks that every object sits on its anchor line and every line it hangs over moves clear of it. Without this change it fails at the seventh object (a line 790 pt down sits at x 0 inside the object's band at 718 pt); with it, it passes. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- crates/compose/src/tests.rs | 38 +++++++++++++++++++++++++++++++++++++ crates/compose/src/wrap.rs | 15 ++++++++++----- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/crates/compose/src/tests.rs b/crates/compose/src/tests.rs index 3083261fb..aafef67e3 100644 --- a/crates/compose/src/tests.rs +++ b/crates/compose/src/tests.rs @@ -7155,6 +7155,44 @@ fn anchored_object_wraps_its_own_storys_later_lines() { assert!((o.origin.y - top).abs() < 1e-6); } +/// Each object's wrap pushes the anchors after it down, so where each object lands depends on the +/// wraps of all those before it; every object still ends up wrapping the lines it hangs over. +#[test] +fn chained_anchored_objects_all_wrap_their_own_storys_lines() { + const OBJECTS: usize = 8; + let text = [LOREM; 12].join(" "); + let (mut d, sid, _) = doc_with(&text, Rect::new(0.0, 0.0, 300.0, 3000.0), ParaAttrs::default()); + let plain = compose_story(&d, sid, &ComposeOptions::default()); + let lines = all_lines(&plain); + assert!(lines.len() > 2 + 6 * OBJECTS); + let left = lines[0].x0; + // One object every six lines, each hanging from its anchor's baseline over the lines below; + // inserted last first so the earlier offsets still hold. + let ats: Vec = (0..OBJECTS).map(|k| lines[2 + 6 * k].range.start + 3).collect(); + for &at in ats.iter().rev() { + anchor_box(&mut d, sid, at, 0.0, 0.0, true, WrapMode::BoundingBox); + } + let cs = compose_story(&d, sid, &ComposeOptions::default()); + assert!(!cs.is_overset()); + let lines = all_lines(&cs); + let anchors: Vec = d.story(sid).unwrap().text.match_indices(designcraft_doc::OBJECT_MARK).map(|(i, _)| i).collect(); + let objects = &cs.frames[0].objects; + assert_eq!(objects.len(), OBJECTS); + for o in objects { + let anchor_line = lines.iter().find(|l| l.range.contains(&anchors[o.index])).unwrap(); + assert!((o.origin.y - anchor_line.baseline).abs() < 1e-6, "object {} at {}, anchor line at {}", o.index, o.origin.y, anchor_line.baseline); + let under = lines_in_band(&cs, o.origin.y, o.origin.y + 60.0); + assert!(under.len() >= 2); + assert!( + under.iter().all(|l| l.x0 >= left + 100.0 - 1e-6), + "object {} at {}: {:?}", + o.index, + o.origin.y, + under.iter().map(|l| (l.baseline, l.x0)).collect::>() + ); + } +} + #[test] fn anchored_object_without_wrap_leaves_layout_unchanged() { let long = [LOREM; 3].join(" "); diff --git a/crates/compose/src/wrap.rs b/crates/compose/src/wrap.rs index e1cdc5fa7..7d38f0ef8 100644 --- a/crates/compose/src/wrap.rs +++ b/crates/compose/src/wrap.rs @@ -12,16 +12,20 @@ //! that come after the line holding its anchor; that line and the lines before it are never //! wrapped by it (an inline object sits in its line and wraps none of its own story). The story //! is composed again with the objects where the previous pass placed them until they stop -//! moving, at most [`MAX_PASSES`] more times. +//! moving. An object's wrap can push the anchors of the objects after it, so a chain of n such +//! objects needs n + 1 more compositions: the k-th pass places the (k + 1)-th object where the +//! wraps before it leave its anchor, and the last pass wraps around the last object. The story +//! is composed at most that many more times, and never more than [`MAX_PASSES`]. use designcraft_doc::{AnchorPosition, AnchoredObject, Document, Item, OBJECT_MARK, PageSide, SpreadRef, Story, StoryId, WrapMode}; use designcraft_geom::Affine; use crate::{ComposeOptions, ComposedStory, Exclusion, FrameSpec}; -/// Extra compositions of a story with its own anchored objects' wrap; a layout that keeps moving -/// its objects (keep options, vertical justification) stops here. -pub(crate) const MAX_PASSES: usize = 4; +/// Most extra compositions of a story with its own anchored objects' wrap, however many objects +/// it has; a layout that keeps moving its objects (keep options, vertical justification) stops +/// sooner, after one per object and one more. +pub(crate) const MAX_PASSES: usize = 64; /// How far an obstacle may move between passes and still count as settled (points). const SETTLED: f64 = 0.01; @@ -199,8 +203,9 @@ pub(crate) fn compose_among(doc: &Document, sid: StoryId, opts: &ComposeOptions, if !has_wrapping_objects(st) { return out; } + let own = st.objects.iter().filter(|o| wraps(o) && !matches!(o.position, AnchorPosition::Inline { .. })).count(); let mut prev = vec![Vec::new(); specs.len()]; - for _ in 0..MAX_PASSES { + for _ in 0..(own + 1).min(MAX_PASSES) { let added = exclusions(doc, &specs, &obstacles(doc, st, &out, true)); if settled(&added, &prev) { break; From a9256308e32ee8cadd2051226986003ff80fbcb9 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 29 Oct 2026 21:39:35 -0400 Subject: [PATCH 2/3] Keep at least four wrap passes for a story with few anchored objects A single wrapping object moved by keep options or vertical justification had four passes to settle before; one-per-object would have given it two. Co-Authored-By: Claude Opus 5.5 --- crates/compose/src/wrap.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/compose/src/wrap.rs b/crates/compose/src/wrap.rs index 7d38f0ef8..724310b00 100644 --- a/crates/compose/src/wrap.rs +++ b/crates/compose/src/wrap.rs @@ -15,7 +15,8 @@ //! moving. An object's wrap can push the anchors of the objects after it, so a chain of n such //! objects needs n + 1 more compositions: the k-th pass places the (k + 1)-th object where the //! wraps before it leave its anchor, and the last pass wraps around the last object. The story -//! is composed at most that many more times, and never more than [`MAX_PASSES`]. +//! is composed at most that many more times (at least [`MIN_PASSES`]), and never more than +//! [`MAX_PASSES`]. use designcraft_doc::{AnchorPosition, AnchoredObject, Document, Item, OBJECT_MARK, PageSide, SpreadRef, Story, StoryId, WrapMode}; use designcraft_geom::Affine; @@ -24,9 +25,13 @@ use crate::{ComposeOptions, ComposedStory, Exclusion, FrameSpec}; /// Most extra compositions of a story with its own anchored objects' wrap, however many objects /// it has; a layout that keeps moving its objects (keep options, vertical justification) stops -/// sooner, after one per object and one more. +/// sooner, after one per object and one more (but never fewer than [`MIN_PASSES`]). pub(crate) const MAX_PASSES: usize = 64; +/// Fewest extra compositions allowed before giving up on a layout that keeps moving: a single +/// object that keep options or vertical justification move still gets the room it had before. +const MIN_PASSES: usize = 4; + /// How far an obstacle may move between passes and still count as settled (points). const SETTLED: f64 = 0.01; @@ -205,7 +210,7 @@ pub(crate) fn compose_among(doc: &Document, sid: StoryId, opts: &ComposeOptions, } let own = st.objects.iter().filter(|o| wraps(o) && !matches!(o.position, AnchorPosition::Inline { .. })).count(); let mut prev = vec![Vec::new(); specs.len()]; - for _ in 0..(own + 1).min(MAX_PASSES) { + for _ in 0..(own + 1).max(MIN_PASSES).min(MAX_PASSES) { let added = exclusions(doc, &specs, &obstacles(doc, st, &out, true)); if settled(&added, &prev) { break; From cc47975960edb8ff3993b35f160dcbf9664a06c5 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 29 Oct 2026 21:43:16 -0400 Subject: [PATCH 3/3] Use clamp for the wrap pass bounds Co-Authored-By: Claude Opus 5.5 --- crates/compose/src/wrap.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/compose/src/wrap.rs b/crates/compose/src/wrap.rs index 724310b00..f28bba424 100644 --- a/crates/compose/src/wrap.rs +++ b/crates/compose/src/wrap.rs @@ -31,6 +31,8 @@ pub(crate) const MAX_PASSES: usize = 64; /// Fewest extra compositions allowed before giving up on a layout that keeps moving: a single /// object that keep options or vertical justification move still gets the room it had before. const MIN_PASSES: usize = 4; +// `clamp` needs min <= max. +const _: () = assert!(MIN_PASSES <= MAX_PASSES); /// How far an obstacle may move between passes and still count as settled (points). const SETTLED: f64 = 0.01; @@ -210,7 +212,7 @@ pub(crate) fn compose_among(doc: &Document, sid: StoryId, opts: &ComposeOptions, } let own = st.objects.iter().filter(|o| wraps(o) && !matches!(o.position, AnchorPosition::Inline { .. })).count(); let mut prev = vec![Vec::new(); specs.len()]; - for _ in 0..(own + 1).max(MIN_PASSES).min(MAX_PASSES) { + for _ in 0..(own + 1).clamp(MIN_PASSES, MAX_PASSES) { let added = exclusions(doc, &specs, &obstacles(doc, st, &out, true)); if settled(&added, &prev) { break;