diff --git a/src/layout/grid.rs b/src/layout/grid.rs index 996ce7c26..33fce0642 100644 --- a/src/layout/grid.rs +++ b/src/layout/grid.rs @@ -56,6 +56,8 @@ struct GridLayouter<'a> { cross: SpecAxis, /// The axis of the main direction. main: SpecAxis, + /// The original expand state of the target region. + expand: Spec, /// The column tracks including gutter tracks. cols: Vec, /// The row tracks including gutter tracks. @@ -64,8 +66,6 @@ struct GridLayouter<'a> { children: &'a [AnyNode], /// The region to layout into. regions: Regions, - /// The original expand state of the target region. - original_expand: Spec, /// Resolved column sizes. rcols: Vec, /// The full main size of the current region. @@ -138,7 +138,7 @@ impl<'a> GridLayouter<'a> { let rcols = vec![Length::zero(); cols.len()]; // We use the regions only for auto row measurement and constraints. - let original_expand = regions.expand; + let expand = regions.expand; regions.expand = Gen::new(true, false).to_spec(main); Self { @@ -147,9 +147,9 @@ impl<'a> GridLayouter<'a> { cols, rows, children: &grid.children, - constraints: Constraints::new(original_expand), + constraints: Constraints::new(expand), regions, - original_expand, + expand, rcols, lrows: vec![], full, @@ -510,7 +510,7 @@ impl<'a> GridLayouter<'a> { self.used.main = Length::zero(); self.fr = Fractional::zero(); self.finished.push(output.constrain(self.constraints)); - self.constraints = Constraints::new(self.original_expand); + self.constraints = Constraints::new(self.expand); } /// Get the node in the cell in column `x` and row `y`. diff --git a/src/layout/incremental.rs b/src/layout/incremental.rs index 953b76bed..a3051d0ff 100644 --- a/src/layout/incremental.rs +++ b/src/layout/incremental.rs @@ -1,4 +1,5 @@ -use std::{collections::HashMap, ops::Deref}; +use std::collections::{hash_map::Entry, HashMap}; +use std::ops::Deref; use super::*; @@ -6,7 +7,9 @@ use super::*; #[derive(Default, Debug, Clone)] pub struct LayoutCache { /// Maps from node hashes to the resulting frames and regions in which the - /// frames are valid. + /// frames are valid. The right hand side of the hash map is a vector of + /// results because across one or more compilations, multiple different + /// layouts of the same node may have been requested. pub frames: HashMap>, /// In how many compilations this cache has been used. age: usize, @@ -28,16 +31,11 @@ impl LayoutCache { where F: FnMut(usize) -> bool, { - for (_, hash_ident) in self.frames.iter_mut() { - hash_ident.retain(|entry| f(entry.level)); + for (_, entries) in self.frames.iter_mut() { + entries.retain(|entry| f(entry.level)); } } - /// Amount of items in the cache. - pub fn len(&self) -> usize { - self.frames.iter().map(|(_, e)| e.len()).sum() - } - /// Prepare the cache for the next round of compilation pub fn turnaround(&mut self) { self.age += 1; @@ -45,11 +43,11 @@ impl LayoutCache { for i in 0 .. (entry.temperature.len() - 1) { entry.temperature[i] = entry.temperature[i + 1]; } - *entry.temperature.last_mut().unwrap() = Some(0); + *entry.temperature.last_mut().unwrap() = 0; } } - /// What is the deepest level in the cache? + /// The amount of levels stored in the cache. pub fn levels(&self) -> usize { self.frames .iter() @@ -85,16 +83,17 @@ impl LayoutCache { level: usize, ) { let entry = FramesEntry::new(frames, level); - if let Some(variants) = self.frames.get_mut(&hash) { - variants.push(entry); - } else { - self.frames.insert(hash, vec![entry]); + match self.frames.entry(hash) { + Entry::Occupied(o) => o.into_mut().push(entry), + Entry::Vacant(v) => { + v.insert(vec![entry]); + } } } } -#[derive(Debug, Clone)] /// Cached frames from past layouting. +#[derive(Debug, Clone)] pub struct FramesEntry { /// The cached frames for a node. pub frames: Vec>>, @@ -103,15 +102,20 @@ pub struct FramesEntry { /// How much the element was accessed during the last five compilations, the /// most recent one being the last element. `None` variants indicate that /// the element is younger than five compilations. - temperature: [Option; 5], + temperature: [usize; 5], + /// For how long the element already exists. + age: usize, } impl FramesEntry { /// Construct a new instance. pub fn new(frames: Vec>>, level: usize) -> Self { - let mut temperature = [None; 5]; - temperature[4] = Some(0); - Self { frames, level, temperature } + Self { + frames, + level, + temperature: [0; 5], + age: 1, + } } /// Checks if the cached [`Frame`] is valid for the given regions. @@ -122,8 +126,7 @@ impl FramesEntry { } } - let tmp = self.temperature.get_mut(4).unwrap(); - *tmp = Some(tmp.map_or(1, |x| x + 1)); + self.temperature[4] = self.temperature[4] + 1; Some(self.frames.clone()) } @@ -131,43 +134,35 @@ impl FramesEntry { /// Get the amount of compilation cycles this item has remained in the /// cache. pub fn age(&self) -> usize { - let mut age = 0; - for &temp in self.temperature.iter().rev() { - if temp.is_none() { - break; - } - age += 1; - } - age + self.age } /// Get the amount of consecutive cycles in which this item has not /// been used. pub fn cooldown(&self) -> usize { - let mut age = 0; + let mut cycle = 0; for (i, &temp) in self.temperature.iter().enumerate().rev() { - match temp { - Some(temp) => { - if temp > 0 { - return self.temperature.len() - 1 - i; - } - } - None => { - return age; + if self.age > i { + if temp > 0 { + return self.temperature.len() - 1 - i; } + } else { + return cycle; } - age += 1 + + cycle += 1 } - age + cycle } /// Whether this element was used in the last compilation cycle. pub fn hit(&self) -> bool { - self.temperature.last().unwrap().unwrap_or(0) != 0 + self.temperature.last().unwrap() != &0 } } +/// These constraints describe regions that match them. #[derive(Debug, Copy, Clone, PartialEq)] pub struct Constraints { /// The minimum available length in the region. @@ -218,49 +213,45 @@ impl Constraints { let base = regions.base.to_spec(); let current = regions.current.to_spec(); - let res = current.eq_by(&self.min, |&x, y| y.map_or(true, |y| x.fits(y))) + current.eq_by(&self.min, |&x, y| y.map_or(true, |y| x.fits(y))) && current.eq_by(&self.max, |x, y| y.map_or(true, |y| x < &y)) && current.eq_by(&self.exact, |&x, y| y.map_or(true, |y| x.approx_eq(y))) - && base.eq_by(&self.base, |&x, y| y.map_or(true, |y| x.approx_eq(y))); - - res + && base.eq_by(&self.base, |&x, y| y.map_or(true, |y| x.approx_eq(y))) } - /// Changes all constraints by adding the argument to them if they are set. + /// Changes all constraints by adding the `size` to them if they are `Some`. pub fn mutate(&mut self, size: Size, regions: &Regions) { - for x in std::array::IntoIter::new([ + for spec in std::array::IntoIter::new([ &mut self.min, &mut self.max, &mut self.exact, &mut self.base, ]) { - if let Some(horizontal) = x.horizontal.as_mut() { + if let Some(horizontal) = spec.horizontal.as_mut() { *horizontal += size.width; } - if let Some(vertical) = x.vertical.as_mut() { + if let Some(vertical) = spec.vertical.as_mut() { *vertical += size.height; } } - self.exact = zip(self.exact, regions.current.to_spec(), |_, o| o); - self.base = zip(self.base, regions.base.to_spec(), |_, o| o); + self.exact = override_if_some(self.exact, regions.current.to_spec()); + self.base = override_if_some(self.base, regions.base.to_spec()); } } -fn zip( +fn override_if_some( one: Spec>, other: Spec, - mut f: F, -) -> Spec> -where - F: FnMut(Length, Length) -> Length, -{ +) -> Spec> { Spec { - vertical: one.vertical.map(|r| f(r, other.vertical)), - horizontal: one.horizontal.map(|r| f(r, other.horizontal)), + vertical: one.vertical.map(|_| other.vertical), + horizontal: one.horizontal.map(|_| other.horizontal), } } +/// Carries an item that only applies to certain regions and the constraints +/// that describe these regions. #[derive(Debug, Copy, Clone, PartialEq)] pub struct Constrained { pub item: T, @@ -275,8 +266,14 @@ impl Deref for Constrained { } } +/// Extends length-related options by providing convenience methods for setting +/// minimum and maximum lengths on them, even if they are `None`. pub trait OptionExt { + // Sets `other` as the value if the Option is `None` or if it contains a + // value larger than `other`. fn set_min(&mut self, other: Length); + // Sets `other` as the value if the Option is `None` or if it contains a + // value smaller than `other`. fn set_max(&mut self, other: Length); } diff --git a/src/layout/pad.rs b/src/layout/pad.rs index cfde07192..f886bec86 100644 --- a/src/layout/pad.rs +++ b/src/layout/pad.rs @@ -15,7 +15,7 @@ impl Layout for PadNode { ctx: &mut LayoutContext, regions: &Regions, ) -> Vec>> { - let original = regions.clone(); + let mut original = regions.clone(); let mut regions = regions.map(|size| size - self.padding.resolve(size).size()); let mut frames = self.child.layout(ctx, ®ions); @@ -39,6 +39,7 @@ impl Layout for PadNode { } regions.next(); + original.next(); *Rc::make_mut(&mut frame.item) = new; } frames diff --git a/src/layout/par.rs b/src/layout/par.rs index 40cb84f2d..504981e66 100644 --- a/src/layout/par.rs +++ b/src/layout/par.rs @@ -190,6 +190,7 @@ impl<'a> ParLayouter<'a> { // line cannot be broken up further. if !stack.regions.current.fits(line.size) { if let Some((last_line, last_end)) = last.take() { + // The region must not fit this line for the result to be valid. if !stack.regions.current.width.fits(line.size.width) { stack.constraints.max.horizontal.set_min(line.size.width); } @@ -213,6 +214,9 @@ impl<'a> ParLayouter<'a> { while !stack.regions.current.height.fits(line.size.height) && !stack.regions.in_full_last() { + // Again, the line must not fit. It would if the space taken up + // plus the line height would fit, therefore the constraint + // below. stack.constraints.max.vertical.set_min( stack.full.height - stack.regions.current.height + line.size.height, ); @@ -225,7 +229,7 @@ impl<'a> ParLayouter<'a> { stack.overflowing = true; break; } - + stack.constraints.max.vertical.set_min( stack.full.height - stack.regions.current.height + line.size.height, ); diff --git a/src/layout/stack.rs b/src/layout/stack.rs index 92a1337fd..045a06e1b 100644 --- a/src/layout/stack.rs +++ b/src/layout/stack.rs @@ -62,6 +62,8 @@ struct StackLayouter<'a> { ruler: Align, /// The constraints for the current region. constraints: Constraints, + /// Whether the last region can fit all the remaining content. + overflowing: bool, /// Offset, alignment and frame for all children that fit into the current /// region. The exact positions are not known yet. frames: Vec<(Length, Gen, Rc)>, @@ -92,6 +94,7 @@ impl<'a> StackLayouter<'a> { full, used: Gen::zero(), ruler: Align::Start, + overflowing: false, frames: vec![], finished: vec![], } @@ -142,9 +145,12 @@ impl<'a> StackLayouter<'a> { } // Find a fitting region. - while !self.regions.current.get(self.main).fits(size.main) - && !self.regions.in_full_last() - { + while !self.regions.current.get(self.main).fits(size.main) { + if self.regions.in_full_last() { + self.overflowing = true; + break; + } + self.constraints .max .get_mut(self.main) @@ -203,6 +209,12 @@ impl<'a> StackLayouter<'a> { size = Size::new(width, width / aspect.into_inner()); } + if self.overflowing { + self.constraints.min.vertical = None; + self.constraints.max.vertical = None; + self.constraints.exact = self.full.to_spec().map(Some); + } + let mut output = Frame::new(size, size.height); let mut first = true; diff --git a/tests/ref/layout/stack.png b/tests/ref/layout/stack.png index 684905f73..2f190c4d3 100644 Binary files a/tests/ref/layout/stack.png and b/tests/ref/layout/stack.png differ diff --git a/tests/typ/layout/stack.typ b/tests/typ/layout/stack.typ index 006a1412e..50ed1eb19 100644 --- a/tests/typ/layout/stack.typ +++ b/tests/typ/layout/stack.typ @@ -7,3 +7,12 @@ rect(3cm, forest), rect(1cm, conifer), ) + +--- + +#let rect(width, color) = rect(width: 1cm, height: 0.4cm, fill: color) +// This stack overflows. +#box(height: 0.5cm, stack( + rect(3cm, forest), + rect(1cm, conifer), +)) diff --git a/tests/typeset.rs b/tests/typeset.rs index 9e0c95966..740d9a030 100644 --- a/tests/typeset.rs +++ b/tests/typeset.rs @@ -15,14 +15,14 @@ use walkdir::WalkDir; use typst::cache::Cache; use typst::color; -use typst::diag::{Diag, DiagSet, Level}; -use typst::eval::{EvalContext, FuncArgs, FuncValue, Scope, Value}; -use typst::exec::State; +use typst::diag::{Diag, DiagSet, Level, Pass}; +use typst::eval::{eval, EvalContext, FuncArgs, FuncValue, Scope, Value}; +use typst::exec::{exec, State}; use typst::geom::{self, Length, Point, Sides, Size}; use typst::image::ImageId; -use typst::layout::{Element, Fill, Frame, Shape, Text}; +use typst::layout::{layout, Element, Fill, Frame, Shape, Text}; use typst::loading::FsLoader; -use typst::parse::{LineMap, Scanner}; +use typst::parse::{parse, LineMap, Scanner}; use typst::syntax::{Location, Pos}; const TYP_DIR: &str = "./typ"; @@ -224,12 +224,22 @@ fn test_part( state.page.size = Size::new(Length::pt(120.0), Length::inf()); state.page.margins = Sides::splat(Some(Length::pt(10.0).into())); - let mut pass = - typst::typeset(loader, cache, Some(src_path), &src, &scope, state.clone()); + let parsed = parse(src); + let evaluated = eval( + loader, + cache, + Some(src_path), + Rc::new(parsed.output), + &scope, + ); + let executed = exec(&evaluated.output.template, state.clone()); + let layouted = layout(loader, cache, &executed.output); - if !compare_ref { - pass.output.clear(); - } + let mut diags = parsed.diags; + diags.extend(evaluated.diags); + diags.extend(executed.diags); + + let mut pass = Pass::new(layouted, diags); let mut ok = true; @@ -274,35 +284,25 @@ fn test_part( cache.layout.turnaround(); - let mut cached_result = - typst::typeset(loader, cache, Some(src_path), &src, &scope, state.clone()); + let cached_result = layout(loader, cache, &executed.output); - if !compare_ref { - cached_result.output.clear(); - } - - let misses: Vec<_> = cache + let misses = cache .layout .frames .iter() .flat_map(|(_, e)| e) .filter(|e| e.level == level && !e.hit() && e.age() == 2) - .collect(); + .count(); - if !misses.is_empty() { + if misses > 0 { ok = false; - let mut miss_count = 0; - for miss in misses { - dbg!(miss.frames.iter().map(|f| f.constraints).collect::>()); - miss_count += 1; - } println!( " Recompilation had {} cache misses on level {} (Subtest {}) ❌", - miss_count, level, i + misses, level, i ); } - if cached_result != pass { + if cached_result != pass.output { ok = false; println!( " Recompilation of subtest {} differs from clean pass ❌", @@ -314,6 +314,10 @@ fn test_part( cache.layout = reference_cache; cache.layout.turnaround(); + if !compare_ref { + pass.output.clear(); + } + (ok, compare_ref, pass.output) }