fix(compose): key parent-page compositions by absolute page index

Parent-page text now uses the absolute page index as its cache key, so repeated page numbers and renumbering do not reuse another page's composition.

Validation: targeted canvas and PDF regressions, plus all six cargo xtask ci gates on main after #514 (1,179 native tests passed, 3 ignored; 15 crates passed WASM compilation checks).

Task: #523
This commit is contained in:
seanth authored and GitHub committed 2026-10-11 21:27:52 -06:00
1 parent 80c2904681
commit a21e8db14f
5 files changed
+111 -46

No files matched your search

+21 -11
View File
@@ -13,7 +13,8 @@ use designcraft_doc::{Document, Item, Story, StoryId, Styles, WrapMode};
use crate::vars::RunningIndex;
use crate::{ComposeOptions, ComposedStory};
type Key = (StoryId, Option<String>);
/// A story, composed for one absolute page (parent-page items) or for its own frames (`None`).
type Key = (StoryId, Option<usize>);
struct Entry {
sig: Vec<usize>,
@@ -55,11 +56,23 @@ impl Cache {
Self::default()
}
/// The composed story, from cache or freshly composed.
pub fn get(&self, doc: &Document, sid: StoryId, page_name: Option<&str>) -> Arc<ComposedStory> {
/// The composed story, from cache or freshly composed. `page` is the absolute page a
/// parent-page item is shown on (its page number markers and variables are that page's), or
/// `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<usize>) -> Arc<ComposedStory> {
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_name.map(str::to_string));
let key = (sid, page);
// 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));
if let Some(name) = &page_name {
use std::hash::{Hash, Hasher};
let mut h = std::collections::hash_map::DefaultHasher::new();
name.hash(&mut h);
sig.push(h.finish() as usize);
}
let with_vars = crate::vars::has_vars(story);
let running = with_vars && crate::vars::has_running(doc, story);
if with_vars {
@@ -125,23 +138,20 @@ impl Cache {
return e.out.clone();
}
}
// Parent-page items are composed per page; variables need that page's index.
let page = match page_name {
Some(n) if with_vars => (0..doc.page_count()).find(|&i| doc.page_name(i) == n),
_ => None,
};
// Parent-page items are composed per page: markers show its name, variables need its index.
let page = page.filter(|_| with_vars);
let running = running.then(|| self.running_index(doc));
let out = Arc::new(crate::compose_story(
doc,
sid,
&ComposeOptions { page_name: page_name.map(str::to_string), page, running, label: None, xrefs: with_xrefs.then(|| self.xref_index(doc)) },
&ComposeOptions { page_name, page, running, label: None, xrefs: with_xrefs.then(|| self.xref_index(doc)) },
));
let mut g = self.map.lock().unwrap_or_else(|e| e.into_inner());
let stamp = g.1;
if g.0.len() > 4096 {
g.0.retain(|_, e| stamp - e.stamp < 64);
}
let replaced = g.0.insert(key.clone(), Entry { sig, _keep: (story.clone(), doc.styles.clone(), keep_items), out: out.clone(), stamp });
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()
&& let Some(old) = replaced
+58 -1
View File
@@ -129,6 +129,63 @@ fn define(s: &mut Session, p: &Value) -> Result<Value> {
mod tests {
use super::*;
/// A parent-page page-number marker follows Numbering & Section Options: changing a section's
/// start number renames its pages without changing their index or the number of sections, and
/// the marker composed for a page shows the new name.
#[test]
fn parent_page_number_follows_section_renumbering() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 3, "facingPages": false})).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 sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story;
assert_eq!(s.doc().unwrap().doc.story(sid).unwrap().text, marker);
let shown = |s: &Session, page: usize| -> usize {
let d = s.doc().unwrap().doc.clone();
let cs = s.cache.get(&d, sid, Some(page));
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!(shown(&s, 2), 1, "page 3");
s.execute("layout.section", &json!({"page": 1, "startNumber": 100})).unwrap();
assert_eq!(s.doc().unwrap().doc.page_name(2), "102");
assert_eq!(shown(&s, 2), 3, "page 102 after renumbering");
}
/// Two sections that both number from 1 repeat page names ("1, 2, 1, 2"). A parent-page running
/// header is composed for the page it is shown on, not the first page with that name.
#[test]
fn running_header_on_parent_page_with_repeated_page_numbers() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 4, "facingPages": false})).unwrap();
s.execute("layout.section", &json!({"page": 3, "startNumber": 1})).unwrap();
let names: Vec<String> = (0..4).map(|i| s.doc().unwrap().doc.page_name(i)).collect();
assert_eq!(names, ["1", "2", "1", "2"]);
s.execute("style.paragraph.create", &json!({"name": "Heading"})).ok();
// The only heading is on the third page (the second section's page 1).
let sp = s.doc().unwrap().doc.page_loc(2).unwrap().0;
let r = s.execute("frame.create", &json!({"spread": sp, "rect": [72, 200, 400, 260], "content": "text", "text": "Epsilon"})).unwrap();
let sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story;
s.execute("text.select", &json!({"story": sid.0, "anchor": 0, "focus": 1})).unwrap();
s.execute("style.paragraph.apply", &json!({"name": "Heading"})).unwrap();
s.execute("variables.define", &json!({"name": "Chapter Head", "type": "runningHeader", "style": "Heading"})).unwrap();
let r = s
.execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [72, 30, 500, 60], "content": "text", "text": ""}))
.unwrap();
let sid = s.doc().unwrap().doc.item(designcraft_doc::ItemId(r["id"].as_u64().unwrap())).unwrap().text_frame().unwrap().story;
s.execute("text.select", &json!({"story": sid.0, "anchor": 0, "focus": 0})).unwrap();
s.execute("variables.insert", &json!({"name": "Chapter Head"})).unwrap();
let d = s.doc().unwrap().doc.clone();
let shown = |page: usize| -> usize {
let cs = s.cache.get(&d, sid, Some(page));
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!(shown(0), 0, "no heading on or before the first page");
assert_eq!(shown(2), "Epsilon".len(), "the third page (named 1) shows its own heading");
assert_eq!(shown(3), "Epsilon".len(), "the fourth page (named 2) carries it");
}
#[test]
fn running_header_on_parent_page_follows_headings() {
let mut s = Session::new();
@@ -158,7 +215,7 @@ mod tests {
assert!(matches!(d.find(fid).unwrap().spread, designcraft_doc::SpreadRef::Parent(0)));
// Visible glyphs of the header composed for each page: "Alpha · 3" vs "Epsilon · 3".
let shown = |page: usize| -> usize {
let cs = s.cache.get(&d, sid, Some(&d.page_name(page)));
let cs = s.cache.get(&d, sid, Some(page));
cs.frames.iter().flat_map(|f| f.lines.iter()).map(|l| l.glyphs.iter().filter(|g| g.visible && g.adv > 0.0).count()).sum()
};
let (p1, p2, p3) = (shown(0), shown(1), shown(2));
+15 -16
View File
@@ -630,7 +630,6 @@ impl Exporter<'_> {
let Some((ppi, ppage)) = doc.parent_page_for(first + pi) else { continue };
let parent = &doc.parents[ppi];
let dx = page.x - parent.pages[ppage].x;
let page_name = doc.page_name(first + pi);
for it in &parent.items {
if it.layer != layer.id || page.overridden.contains(&it.id) {
continue;
@@ -639,7 +638,7 @@ impl Exporter<'_> {
if parent.page_at_x(it.bounds().center().x) != Some(ppage) && parent.pages.len() > 1 {
continue;
}
self.top_item(s, it, Affine::translate((dx, 0.0)), Some(&page_name), true);
self.top_item(s, it, Affine::translate((dx, 0.0)), Some(first + pi), true);
}
}
for it in &sp.items {
@@ -679,9 +678,9 @@ impl Exporter<'_> {
}
/// A spread or parent item, inside its structure tag when tagging.
fn top_item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>, on_parent: bool) {
fn top_item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option<usize>, on_parent: bool) {
if !self.opts.tagged || it.hidden || it.nonprinting {
return self.item(s, it, parent, page_name);
return self.item(s, it, parent, page);
}
let story = it.text_frame().map(|t| t.story);
let figure = !it.alt_text.is_empty() || matches!(it.content, Content::Graphic(_)) || it.has_nested_items();
@@ -689,7 +688,7 @@ impl Exporter<'_> {
// Page furniture and decoration.
let kind = if on_parent { ArtifactType::Page } else { ArtifactType::Other };
s.start_tagged(ContentTag::Artifact(Artifact::new(kind, None)));
self.item(s, it, parent, page_name);
self.item(s, it, parent, page);
s.end_tagged();
return;
}
@@ -700,12 +699,12 @@ impl Exporter<'_> {
self.tags.push(TagEntry::Story(sid));
}
self.tag_story = Some(sid);
self.item(s, it, parent, page_name);
self.item(s, it, parent, page);
self.tag_story = None;
return;
}
let id = s.start_tagged(ContentTag::Other);
self.item(s, it, parent, page_name);
self.item(s, it, parent, page);
s.end_tagged();
let alt = (!it.alt_text.is_empty()).then(|| it.alt_text.clone());
let mut g = TagGroup::new(Tag::Figure(alt));
@@ -751,10 +750,10 @@ impl Exporter<'_> {
tree
}
fn item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>) {
fn item(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option<usize>) {
let gf = &it.effects.gradient_feather;
if !gf.on || it.hidden || it.nonprinting {
return self.item_inner(s, it, parent, page_name);
return self.item_inner(s, it, parent, page);
}
// Gradient feather: a luminosity mask (white = opaque) over the object.
let xf = parent * it.xf;
@@ -804,11 +803,11 @@ impl Exporter<'_> {
ms.finish();
let stream = sb.finish();
s.push_mask(krilla::mask::Mask::new(stream, krilla::mask::MaskType::Luminosity));
self.item_inner(s, it, parent, page_name);
self.item_inner(s, it, parent, page);
s.pop();
}
fn item_inner(&mut self, s: &mut Surface, it: &Item, parent: Affine, page_name: Option<&str>) {
fn item_inner(&mut self, s: &mut Surface, it: &Item, parent: Affine, page: Option<usize>) {
if it.hidden || it.nonprinting {
return;
}
@@ -843,7 +842,7 @@ impl Exporter<'_> {
s.push_clip_path(c, &FillRule::NonZero);
}
for c in it.children() {
self.item(s, c, xf, page_name);
self.item(s, c, xf, page);
}
if clip.is_some() {
s.pop();
@@ -863,7 +862,7 @@ impl Exporter<'_> {
if !it.children().is_empty() {
let pushes = Self::push_group(s, it.opacity, it.blend) + Self::push_isolation(s, it);
for c in it.shown_children() {
self.item(s, c, xf, page_name);
self.item(s, c, xf, page);
}
for _ in 0..pushes {
s.pop();
@@ -921,7 +920,7 @@ impl Exporter<'_> {
}
}
Content::Text(tfr) => {
let cs = self.cache.get(doc, tfr.story, page_name);
let cs = self.cache.get(doc, tfr.story, page);
if let (Some(ft), Some(pt)) = (cs.frame(it.id), &tfr.options.path) {
// Type on a path: the glyphs as outlines along the path.
for (si, gp) in designcraft_compose::path_glyphs(&cs, ft, &bp, pt) {
@@ -966,11 +965,11 @@ impl Exporter<'_> {
if let Content::Text(tfr) = &it.content
&& let Some(st) = doc.story(tfr.story).filter(|st| !st.objects.is_empty())
{
let cs = self.cache.get(doc, tfr.story, page_name);
let cs = self.cache.get(doc, tfr.story, page);
if let Some(ft) = cs.frame(it.id) {
for o in &ft.objects {
if let Some(obj) = st.objects.get(o.index) {
self.item(s, &obj.item, xf * Affine::translate(o.origin.to_vec2()), page_name);
self.item(s, &obj.item, xf * Affine::translate(o.origin.to_vec2()), page);
}
}
}
+4 -4
View File
@@ -66,7 +66,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_name: Option<&str>) {
pub(crate) fn draw_item_fx(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option<usize>) {
let doc = f.doc;
let e = &it.effects;
let sw = if it.stroke.is_none() { 0.0 } else { it.stroke.extent() };
@@ -95,7 +95,7 @@ impl Renderer {
let moved = Frame { view: fr.view * Affine::translate(off), ..*fr };
c.set_transform(moved.view);
c.push_layer(None, None, None, None, Some(filter));
me.draw_body(c, &moved, it, bp, xf, page_name);
me.draw_body(c, &moved, it, bp, xf, page);
if grow > 0.0 && solid {
c.set_transform(moved.view * xf);
c.set_paint(peniko::Color::BLACK);
@@ -119,7 +119,7 @@ impl Renderer {
self.with_filters(ctx, f, reach, w / 4.0, |me, c, fr| {
c.set_transform(fr.view * xf);
c.push_clip_layer(bp);
me.draw_body(c, fr, it, bp, xf, page_name);
me.draw_body(c, fr, it, bp, xf, page);
// Keep the body where the blurred inset silhouette is.
c.set_transform(Affine::IDENTITY);
c.push_layer(None, Some(BlendMode::new(Mix::Normal, Compose::DestIn)), None, None, None);
@@ -139,7 +139,7 @@ impl Renderer {
c.pop_layer();
});
} else {
self.draw_body(ctx, f, it, bp, xf, page_name);
self.draw_body(ctx, f, it, bp, xf, page);
}
// Directional feather: one fading ramp per side, multiplied in.
let df = &e.directional_feather;
+13 -14
View File
@@ -216,7 +216,7 @@ pub struct Renderer {
resources: Resources,
glyphs: text::GlyphCache,
/// Composed stories looked up during the current render.
stories: HashMap<(StoryId, Option<String>), Arc<ComposedStory>>,
stories: HashMap<(StoryId, Option<usize>), Arc<ComposedStory>>,
pub threads: u16,
pub stats: FrameStats,
}
@@ -408,7 +408,6 @@ impl Renderer {
let Some((ppi, ppage)) = doc.parent_page_for(first + pi) else { continue };
let parent = &doc.parents[ppi];
let dx = page.x - parent.pages[ppage].x;
let page_name = doc.page_name(first + pi);
let pr = parent.pages[ppage].bounds();
for it in &parent.items {
if it.layer != layer.id || page.overridden.contains(&it.id) {
@@ -419,7 +418,7 @@ impl Renderer {
continue;
}
let _ = pr;
self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), Some(&page_name));
self.draw_item(ctx, f, it, Affine::translate((dx, 0.0)), Some(first + pi));
}
}
}
@@ -431,7 +430,7 @@ impl Renderer {
}
}
fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page_name: Option<&str>) {
fn draw_item(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, parent: Affine, page: Option<usize>) {
if it.hidden || f.opts.hidden.contains(&it.id) || f.opts.printing_only && it.nonprinting {
return;
}
@@ -458,7 +457,7 @@ impl Renderer {
ctx.set_transform(f.view * xf);
ctx.push_clip_layer(&bp);
for c in it.children() {
self.draw_item(ctx, f, c, xf, page_name);
self.draw_item(ctx, f, c, xf, page);
}
ctx.pop_layer();
if !it.stroke.is_none() {
@@ -489,11 +488,11 @@ impl Renderer {
ctx.fill_path(&c.path.to_bezpath());
ctx.pop_layer();
}
self.draw_item(ctx, f, c, xf, page_name);
self.draw_item(ctx, f, c, xf, page);
}
} else {
for c in it.shown_children() {
self.draw_item(ctx, f, c, xf, page_name);
self.draw_item(ctx, f, c, xf, page);
}
}
if layered {
@@ -520,9 +519,9 @@ impl Renderer {
ctx.push_layer(None, Some(blend_mode(it.blend)), Some(it.opacity), None, None);
}
if effects {
self.draw_item_fx(ctx, f, it, &bp, xf, page_name);
self.draw_item_fx(ctx, f, it, &bp, xf, page);
} else {
self.draw_body(ctx, f, it, &bp, xf, page_name);
self.draw_body(ctx, f, it, &bp, xf, page);
}
if layered {
ctx.pop_layer();
@@ -531,18 +530,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_name: Option<&str>) -> Arc<ComposedStory> {
let key = (sid, page_name.map(str::to_string));
fn story(&mut self, f: &Frame, sid: StoryId, page: Option<usize>) -> Arc<ComposedStory> {
let key = (sid, page);
if let Some(cs) = self.stories.get(&key) {
return cs.clone();
}
let cs = f.cache.get(f.doc, sid, page_name);
let cs = f.cache.get(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_name: Option<&str>) {
pub(crate) fn draw_body(&mut self, ctx: &mut RenderContext, f: &Frame, it: &Item, bp: &BezPath, xf: Affine, page: Option<usize>) {
let doc = f.doc;
// Fill.
if !it.fill.is_none() {
@@ -575,7 +574,7 @@ impl Renderer {
ctx.pop_layer();
}
Content::Text(tf) => {
let cs = self.story(f, tf.story, page_name);
let cs = self.story(f, tf.story, page);
if let Some(ft) = cs.frame(it.id) {
match &tf.options.path {
Some(pt) => self.draw_path_text(ctx, f, &cs, ft, xf, bp, pt),