From 6b6cdae7ce95681d6a1194be70b375494166a8c6 Mon Sep 17 00:00:00 2001
From: Martin Haug <mhaug@live.de>
Date: Sun, 27 Jun 2021 12:28:40 +0200
Subject: Testing for incremental

Also, constraint bugfixes.
---
 src/layout/grid.rs        |  10 ++-
 src/layout/incremental.rs | 155 +++++++++++++++++++++++++++++++++++++++++++---
 src/layout/mod.rs         |  14 +----
 src/layout/pad.rs         |   7 ++-
 src/layout/par.rs         |  21 ++++++-
 src/layout/stack.rs       |  13 ++--
 6 files changed, 188 insertions(+), 32 deletions(-)

(limited to 'src/layout')

diff --git a/src/layout/grid.rs b/src/layout/grid.rs
index d2976502..996ce7c2 100644
--- a/src/layout/grid.rs
+++ b/src/layout/grid.rs
@@ -64,6 +64,8 @@ struct GridLayouter<'a> {
     children: &'a [AnyNode],
     /// The region to layout into.
     regions: Regions,
+    /// The original expand state of the target region.
+    original_expand: Spec<bool>,
     /// Resolved column sizes.
     rcols: Vec<Length>,
     /// The full main size of the current region.
@@ -135,7 +137,8 @@ impl<'a> GridLayouter<'a> {
         let full = regions.current.get(main);
         let rcols = vec![Length::zero(); cols.len()];
 
-        // We use the regions only for auto row measurement.
+        // We use the regions only for auto row measurement and constraints.
+        let original_expand = regions.expand;
         regions.expand = Gen::new(true, false).to_spec(main);
 
         Self {
@@ -144,8 +147,9 @@ impl<'a> GridLayouter<'a> {
             cols,
             rows,
             children: &grid.children,
-            constraints: Constraints::new(regions.expand),
+            constraints: Constraints::new(original_expand),
             regions,
+            original_expand,
             rcols,
             lrows: vec![],
             full,
@@ -506,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.regions.expand);
+        self.constraints = Constraints::new(self.original_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 a5c3cea3..e1d1ffba 100644
--- a/src/layout/incremental.rs
+++ b/src/layout/incremental.rs
@@ -7,13 +7,15 @@ use super::*;
 pub struct LayoutCache {
     /// Maps from node hashes to the resulting frames and regions in which the
     /// frames are valid.
-    pub frames: HashMap<u64, FramesEntry>,
+    pub frames: HashMap<u64, Vec<FramesEntry>>,
+    /// In how many compilations this cache has been used.
+    age: usize,
 }
 
 impl LayoutCache {
     /// Create a new, empty layout cache.
     pub fn new() -> Self {
-        Self { frames: HashMap::new() }
+        Self { frames: HashMap::new(), age: 0 }
     }
 
     /// Clear the cache.
@@ -26,12 +28,68 @@ impl LayoutCache {
     where
         F: FnMut(usize) -> bool,
     {
-        self.frames.retain(|_, entry| f(entry.level));
+        for (_, hash_ident) in self.frames.iter_mut() {
+            hash_ident.retain(|entry| f(entry.level));
+        }
     }
 
     /// Amount of items in the cache.
     pub fn len(&self) -> usize {
-        self.frames.len()
+        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;
+        for entry in self.frames.iter_mut().flat_map(|(_, x)| x.iter_mut()) {
+            for i in 0 .. (entry.temperature.len() - 1) {
+                entry.temperature[i] = entry.temperature[i + 1];
+            }
+            *entry.temperature.last_mut().unwrap() = Some(0);
+        }
+    }
+
+    /// What is the deepest level in the cache?
+    pub fn levels(&self) -> usize {
+        self.frames
+            .iter()
+            .flat_map(|(_, x)| x)
+            .map(|entry| entry.level + 1)
+            .max()
+            .unwrap_or(0)
+    }
+
+    /// Fetches the appropriate entry from the cache if there is any.
+    pub fn get(
+        &mut self,
+        hash: u64,
+        regions: Regions,
+    ) -> Option<Vec<Constrained<Rc<Frame>>>> {
+        self.frames.get_mut(&hash).and_then(|frames| {
+            for frame in frames {
+                let res = frame.check(regions.clone());
+                if res.is_some() {
+                    return res;
+                }
+            }
+
+            None
+        })
+    }
+
+    /// Inserts a new frame set into the cache.
+    pub fn insert(
+        &mut self,
+        hash: u64,
+        frames: Vec<Constrained<Rc<Frame>>>,
+        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]);
+        }
     }
 }
 
@@ -42,19 +100,72 @@ pub struct FramesEntry {
     pub frames: Vec<Constrained<Rc<Frame>>>,
     /// How nested the frame was in the context is was originally appearing in.
     pub level: usize,
+    /// How much the element was accessed during the last five compilations, the
+    /// most current one being the last element. `None` variants indicate that
+    /// the element is younger than five compilations.
+    temperature: [Option<usize>; 5],
 }
 
 impl FramesEntry {
+    /// Construct a new instance.
+    pub fn new(frames: Vec<Constrained<Rc<Frame>>>, level: usize) -> Self {
+        let mut temperature = [None; 5];
+        temperature[4] = Some(0);
+        Self { frames, level, temperature }
+    }
+
     /// Checks if the cached [`Frame`] is valid for the given regions.
-    pub fn check(&self, mut regions: Regions) -> Option<Vec<Constrained<Rc<Frame>>>> {
+    pub fn check(&mut self, mut regions: Regions) -> Option<Vec<Constrained<Rc<Frame>>>> {
         for (i, frame) in self.frames.iter().enumerate() {
             if (i != 0 && !regions.next()) || !frame.constraints.check(&regions) {
                 return None;
             }
         }
 
+        let tmp = self.temperature.get_mut(4).unwrap();
+        *tmp = Some(tmp.map_or(1, |x| x + 1));
+
         Some(self.frames.clone())
     }
+
+    /// 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
+    }
+
+    /// Get the amount of consecutive cycles in which this item has not
+    /// been used.
+    pub fn cooldown(&self) -> usize {
+        let mut age = 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;
+                }
+            }
+            age += 1
+        }
+
+        age
+    }
+
+    /// Whether this element was used in the last compilation cycle.
+    pub fn hit(&self) -> bool {
+        self.temperature.last().unwrap().unwrap_or(0) != 0
+    }
 }
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -107,15 +218,22 @@ impl Constraints {
         let base = regions.base.to_spec();
         let current = regions.current.to_spec();
 
-        current.eq_by(&self.min, |x, y| y.map_or(true, |y| x >= &y))
+        let res = 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 == &y))
-            && base.eq_by(&self.base, |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
     }
 
     /// Changes all constraints by adding the argument to them if they are set.
-    pub fn mutate(&mut self, size: Size) {
-        for x in &mut [self.min, self.max, self.exact, self.base] {
+    pub fn mutate(&mut self, size: Size, regions: &Regions) {
+        for x 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() {
                 *horizontal += size.width;
             }
@@ -123,6 +241,23 @@ impl Constraints {
                 *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);
+    }
+}
+
+fn zip<F>(
+    one: Spec<Option<Length>>,
+    other: Spec<Length>,
+    mut f: F,
+) -> Spec<Option<Length>>
+where
+    F: FnMut(Length, Length) -> Length,
+{
+    Spec {
+        vertical: one.vertical.map(|r| f(r, other.vertical)),
+        horizontal: one.horizontal.map(|r| f(r, other.horizontal)),
     }
 }
 
diff --git a/src/layout/mod.rs b/src/layout/mod.rs
index 10c30f41..dc819a16 100644
--- a/src/layout/mod.rs
+++ b/src/layout/mod.rs
@@ -102,18 +102,10 @@ impl Layout for AnyNode {
         regions: &Regions,
     ) -> Vec<Constrained<Rc<Frame>>> {
         ctx.level += 1;
-        let frames = ctx
-            .cache
-            .layout
-            .frames
-            .get(&self.hash)
-            .and_then(|x| x.check(regions.clone()))
-            .unwrap_or_else(|| {
+        let frames =
+            ctx.cache.layout.get(self.hash, regions.clone()).unwrap_or_else(|| {
                 let frames = self.node.layout(ctx, regions);
-                ctx.cache.layout.frames.insert(self.hash, FramesEntry {
-                    frames: frames.clone(),
-                    level: ctx.level - 1,
-                });
+                ctx.cache.layout.insert(self.hash, frames.clone(), ctx.level - 1);
                 frames
             });
         ctx.level -= 1;
diff --git a/src/layout/pad.rs b/src/layout/pad.rs
index 9d432e79..cfde0719 100644
--- a/src/layout/pad.rs
+++ b/src/layout/pad.rs
@@ -15,6 +15,7 @@ impl Layout for PadNode {
         ctx: &mut LayoutContext,
         regions: &Regions,
     ) -> Vec<Constrained<Rc<Frame>>> {
+        let original = regions.clone();
         let mut regions = regions.map(|size| size - self.padding.resolve(size).size());
 
         let mut frames = self.child.layout(ctx, &regions);
@@ -28,13 +29,13 @@ impl Layout for PadNode {
             let prev = std::mem::take(&mut frame.item);
             new.push_frame(origin, prev);
 
-            frame.constraints.mutate(padding.size() * -1.0);
+            frame.constraints.mutate(padding.size(), &original);
 
             if self.padding.left.is_relative() || self.padding.right.is_relative() {
-                frame.constraints.base.horizontal = Some(regions.base.width);
+                frame.constraints.base.horizontal = Some(original.base.width);
             }
             if self.padding.top.is_relative() || self.padding.bottom.is_relative() {
-                frame.constraints.base.vertical = Some(regions.base.height);
+                frame.constraints.base.vertical = Some(original.base.height);
             }
 
             regions.next();
diff --git a/src/layout/par.rs b/src/layout/par.rs
index 2208f9ee..0cf1cd23 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -213,10 +213,18 @@ impl<'a> ParLayouter<'a> {
             while !stack.regions.current.height.fits(line.size.height)
                 && !stack.regions.in_full_last()
             {
-                stack.constraints.max.vertical.set_min(line.size.height);
+                stack.constraints.max.vertical.set_min(
+                    stack.full.height - stack.regions.current.height + line.size.height,
+                );
                 stack.finish_region(ctx);
             }
 
+            if !stack.regions.current.height.fits(line.size.height)
+                && stack.regions.in_full_last()
+            {
+                stack.overflowing = true;
+            }
+
             // If the line does not fit horizontally or we have a mandatory
             // line break (i.e. due to "\n"), we push the line into the
             // stack.
@@ -303,11 +311,13 @@ impl ParItem<'_> {
 /// Stacks lines on top of each other.
 struct LineStack<'a> {
     line_spacing: Length,
+    full: Size,
     regions: Regions,
     size: Size,
     lines: Vec<LineLayout<'a>>,
     finished: Vec<Constrained<Rc<Frame>>>,
     constraints: Constraints,
+    overflowing: bool,
 }
 
 impl<'a> LineStack<'a> {
@@ -316,10 +326,12 @@ impl<'a> LineStack<'a> {
         Self {
             line_spacing,
             constraints: Constraints::new(regions.expand),
+            full: regions.current,
             regions,
             size: Size::zero(),
             lines: vec![],
             finished: vec![],
+            overflowing: false,
         }
     }
 
@@ -343,6 +355,12 @@ impl<'a> LineStack<'a> {
             self.constraints.exact.horizontal = Some(self.regions.current.width);
         }
 
+        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(self.size, self.size.height);
         let mut offset = Length::zero();
         let mut first = true;
@@ -362,6 +380,7 @@ impl<'a> LineStack<'a> {
 
         self.finished.push(output.constrain(self.constraints));
         self.regions.next();
+        self.full = self.regions.current;
         self.constraints = Constraints::new(self.regions.expand);
         self.size = Size::zero();
     }
diff --git a/src/layout/stack.rs b/src/layout/stack.rs
index 2a0b2806..92a1337f 100644
--- a/src/layout/stack.rs
+++ b/src/layout/stack.rs
@@ -86,8 +86,8 @@ impl<'a> StackLayouter<'a> {
         Self {
             stack,
             main,
+            constraints: Constraints::new(expand),
             expand,
-            constraints: Constraints::new(regions.expand),
             regions,
             full,
             used: Gen::zero(),
@@ -145,7 +145,10 @@ impl<'a> StackLayouter<'a> {
         while !self.regions.current.get(self.main).fits(size.main)
             && !self.regions.in_full_last()
         {
-            self.constraints.max.get_mut(self.main).set_min(size.main);
+            self.constraints
+                .max
+                .get_mut(self.main)
+                .set_min(size.main + self.used.main);
             self.finish_region();
         }
 
@@ -188,7 +191,9 @@ impl<'a> StackLayouter<'a> {
 
         // Make sure the stack's size satisfies the aspect ratio.
         if let Some(aspect) = self.stack.aspect {
-            self.constraints.exact = self.regions.current.to_spec().map(Some);
+            self.constraints.exact = self.full.to_spec().map(Some);
+            self.constraints.min = Spec::splat(None);
+            self.constraints.max = Spec::splat(None);
             let width = size
                 .width
                 .max(aspect.into_inner() * size.height)
@@ -244,6 +249,6 @@ impl<'a> StackLayouter<'a> {
         self.used = Gen::zero();
         self.ruler = Align::Start;
         self.finished.push(output.constrain(self.constraints));
-        self.constraints = Constraints::new(self.regions.expand);
+        self.constraints = Constraints::new(expand);
     }
 }
-- 
cgit v1.2.3


From 57bd3e23c79878d106ab8be17c71caca6c4f5a7c Mon Sep 17 00:00:00 2001
From: Martin <mhaug@live.de>
Date: Sun, 27 Jun 2021 17:08:40 +0200
Subject: Apply suggestions from code review

Co-authored-by: Laurenz <laurmaedje@gmail.com>
---
 src/layout/incremental.rs |  2 +-
 src/layout/par.rs         | 16 +++++++++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

(limited to 'src/layout')

diff --git a/src/layout/incremental.rs b/src/layout/incremental.rs
index e1d1ffba..953b76be 100644
--- a/src/layout/incremental.rs
+++ b/src/layout/incremental.rs
@@ -101,7 +101,7 @@ pub struct FramesEntry {
     /// How nested the frame was in the context is was originally appearing in.
     pub level: usize,
     /// How much the element was accessed during the last five compilations, the
-    /// most current one being the last element. `None` variants indicate that
+    /// most recent one being the last element. `None` variants indicate that
     /// the element is younger than five compilations.
     temperature: [Option<usize>; 5],
 }
diff --git a/src/layout/par.rs b/src/layout/par.rs
index 0cf1cd23..40cb84f2 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -219,12 +219,18 @@ impl<'a> ParLayouter<'a> {
                 stack.finish_region(ctx);
             }
 
-            if !stack.regions.current.height.fits(line.size.height)
-                && stack.regions.in_full_last()
-            {
-                stack.overflowing = true;
+            // If the line does not fit vertically, we start a new region.
+            while !stack.regions.current.height.fits(line.size.height) {
+                if stack.regions.in_full_last() {
+                    stack.overflowing = true;
+                    break;
+                }
+                
+                stack.constraints.max.vertical.set_min(
+                    stack.full.height - stack.regions.current.height + line.size.height,
+                );
+                stack.finish_region(ctx);
             }
-
             // If the line does not fit horizontally or we have a mandatory
             // line break (i.e. due to "\n"), we push the line into the
             // stack.
-- 
cgit v1.2.3


From 9bd8b7ddac046f581dc750e148147901d08cb0f4 Mon Sep 17 00:00:00 2001
From: Martin Haug <mhaug@live.de>
Date: Sun, 27 Jun 2021 18:06:39 +0200
Subject: Code review, new stack test

---
 src/layout/grid.rs        |  12 ++---
 src/layout/incremental.rs | 117 ++++++++++++++++++++++------------------------
 src/layout/pad.rs         |   3 +-
 src/layout/par.rs         |   6 ++-
 src/layout/stack.rs       |  18 +++++--
 5 files changed, 85 insertions(+), 71 deletions(-)

(limited to 'src/layout')

diff --git a/src/layout/grid.rs b/src/layout/grid.rs
index 996ce7c2..33fce064 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<bool>,
     /// The column tracks including gutter tracks.
     cols: Vec<TrackSizing>,
     /// 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<bool>,
     /// Resolved column sizes.
     rcols: Vec<Length>,
     /// 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 953b76be..a3051d0f 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<u64, Vec<FramesEntry>>,
     /// 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<Constrained<Rc<Frame>>>,
@@ -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<usize>; 5],
+    temperature: [usize; 5],
+    /// For how long the element already exists.
+    age: usize,
 }
 
 impl FramesEntry {
     /// Construct a new instance.
     pub fn new(frames: Vec<Constrained<Rc<Frame>>>, 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<F>(
+fn override_if_some(
     one: Spec<Option<Length>>,
     other: Spec<Length>,
-    mut f: F,
-) -> Spec<Option<Length>>
-where
-    F: FnMut(Length, Length) -> Length,
-{
+) -> Spec<Option<Length>> {
     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<T> {
     pub item: T,
@@ -275,8 +266,14 @@ impl<T> Deref for Constrained<T> {
     }
 }
 
+/// 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 cfde0719..f886bec8 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<Constrained<Rc<Frame>>> {
-        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, &regions);
@@ -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 40cb84f2..504981e6 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 92a1337f..045a06e1 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<Align>, Rc<Frame>)>,
@@ -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;
 
-- 
cgit v1.2.3


From 6f518c172805b6ac067a692eb8971c7ec1e50608 Mon Sep 17 00:00:00 2001
From: Martin <mhaug@live.de>
Date: Sun, 27 Jun 2021 18:30:25 +0200
Subject: Adjust comments

Co-authored-by: Laurenz <laurmaedje@gmail.com>

Update tests/typ/layout/stack.typ

Co-authored-by: Laurenz <laurmaedje@gmail.com>
---
 src/layout/incremental.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'src/layout')

diff --git a/src/layout/incremental.rs b/src/layout/incremental.rs
index a3051d0f..b6a059c5 100644
--- a/src/layout/incremental.rs
+++ b/src/layout/incremental.rs
@@ -162,7 +162,7 @@ impl FramesEntry {
     }
 }
 
-/// These constraints describe regions that match them.
+/// Describe regions that match them.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct Constraints {
     /// The minimum available length in the region.
-- 
cgit v1.2.3


From e9960b89424ab67e633076ccc9f8c420316b076a Mon Sep 17 00:00:00 2001
From: Martin Haug <mhaug@live.de>
Date: Sun, 27 Jun 2021 18:57:08 +0200
Subject: Code review: Reverse temperature

---
 src/layout/incremental.rs | 67 +++++++++++++++++++++++++----------------------
 src/layout/pad.rs         | 17 ++++++------
 src/layout/par.rs         | 16 ++++++-----
 src/layout/stack.rs       |  4 +--
 4 files changed, 57 insertions(+), 47 deletions(-)

(limited to 'src/layout')

diff --git a/src/layout/incremental.rs b/src/layout/incremental.rs
index b6a059c5..9eda0402 100644
--- a/src/layout/incremental.rs
+++ b/src/layout/incremental.rs
@@ -26,6 +26,11 @@ impl LayoutCache {
         self.frames.clear();
     }
 
+    /// Amount of items in the cache.
+    pub fn len(&self) -> usize {
+        self.frames.iter().map(|(_, e)| e.len()).sum()
+    }
+
     /// Retains all elements for which the closure on the level returns `true`.
     pub fn retain<F>(&mut self, mut f: F)
     where
@@ -41,9 +46,10 @@ impl LayoutCache {
         self.age += 1;
         for entry in self.frames.iter_mut().flat_map(|(_, x)| x.iter_mut()) {
             for i in 0 .. (entry.temperature.len() - 1) {
-                entry.temperature[i] = entry.temperature[i + 1];
+                entry.temperature[i + 1] = entry.temperature[i];
             }
-            *entry.temperature.last_mut().unwrap() = 0;
+            entry.temperature[0] = 0;
+            entry.age += 1;
         }
     }
 
@@ -99,12 +105,11 @@ pub struct FramesEntry {
     pub frames: Vec<Constrained<Rc<Frame>>>,
     /// How nested the frame was in the context is was originally appearing in.
     pub level: usize,
-    /// 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: [usize; 5],
     /// For how long the element already exists.
     age: usize,
+    /// How much the element was accessed during the last five compilations, the
+    /// most recent one being the first element.
+    temperature: [usize; 5],
 }
 
 impl FramesEntry {
@@ -113,8 +118,8 @@ impl FramesEntry {
         Self {
             frames,
             level,
-            temperature: [0; 5],
             age: 1,
+            temperature: [0; 5],
         }
     }
 
@@ -126,7 +131,7 @@ impl FramesEntry {
             }
         }
 
-        self.temperature[4] = self.temperature[4] + 1;
+        self.temperature[0] += 1;
 
         Some(self.frames.clone())
     }
@@ -141,16 +146,11 @@ impl FramesEntry {
     /// been used.
     pub fn cooldown(&self) -> usize {
         let mut cycle = 0;
-        for (i, &temp) in self.temperature.iter().enumerate().rev() {
-            if self.age > i {
-                if temp > 0 {
-                    return self.temperature.len() - 1 - i;
-                }
-            } else {
+        for &temp in &self.temperature[.. self.age] {
+            if temp > 0 {
                 return cycle;
             }
-
-            cycle += 1
+            cycle += 1;
         }
 
         cycle
@@ -158,7 +158,7 @@ impl FramesEntry {
 
     /// Whether this element was used in the last compilation cycle.
     pub fn hit(&self) -> bool {
-        self.temperature.last().unwrap() != &0
+        self.temperature[0] != 0
     }
 }
 
@@ -213,10 +213,10 @@ impl Constraints {
         let base = regions.base.to_spec();
         let current = regions.current.to_spec();
 
-        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)))
+            && 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)))
     }
 
     /// Changes all constraints by adding the `size` to them if they are `Some`.
@@ -235,18 +235,13 @@ impl Constraints {
             }
         }
 
-        self.exact = override_if_some(self.exact, regions.current.to_spec());
-        self.base = override_if_some(self.base, regions.base.to_spec());
-    }
-}
+        let current = regions.current.to_spec();
+        let base = regions.base.to_spec();
 
-fn override_if_some(
-    one: Spec<Option<Length>>,
-    other: Spec<Length>,
-) -> Spec<Option<Length>> {
-    Spec {
-        vertical: one.vertical.map(|_| other.vertical),
-        horizontal: one.horizontal.map(|_| other.horizontal),
+        self.exact.horizontal.set_if_some(current.horizontal);
+        self.exact.vertical.set_if_some(current.vertical);
+        self.base.horizontal.set_if_some(base.horizontal);
+        self.base.vertical.set_if_some(base.vertical);
     }
 }
 
@@ -272,9 +267,13 @@ 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);
+
+    /// Sets `other` as the value if the Option is `Some`.
+    fn set_if_some(&mut self, other: Length);
 }
 
 impl OptionExt for Option<Length> {
@@ -291,4 +290,10 @@ impl OptionExt for Option<Length> {
             None => *self = Some(other),
         }
     }
+
+    fn set_if_some(&mut self, other: Length) {
+        if self.is_some() {
+            *self = Some(other);
+        }
+    }
 }
diff --git a/src/layout/pad.rs b/src/layout/pad.rs
index f886bec8..9461f3ff 100644
--- a/src/layout/pad.rs
+++ b/src/layout/pad.rs
@@ -15,10 +15,11 @@ impl Layout for PadNode {
         ctx: &mut LayoutContext,
         regions: &Regions,
     ) -> Vec<Constrained<Rc<Frame>>> {
-        let mut original = regions.clone();
-        let mut regions = regions.map(|size| size - self.padding.resolve(size).size());
-
-        let mut frames = self.child.layout(ctx, &regions);
+        let mut regions = regions.clone();
+        let mut frames = self.child.layout(
+            ctx,
+            &regions.map(|size| size - self.padding.resolve(size).size()),
+        );
 
         for frame in &mut frames {
             let padded = solve(self.padding, frame.size);
@@ -29,19 +30,19 @@ impl Layout for PadNode {
             let prev = std::mem::take(&mut frame.item);
             new.push_frame(origin, prev);
 
-            frame.constraints.mutate(padding.size(), &original);
+            frame.constraints.mutate(padding.size(), &regions);
 
             if self.padding.left.is_relative() || self.padding.right.is_relative() {
-                frame.constraints.base.horizontal = Some(original.base.width);
+                frame.constraints.base.horizontal = Some(regions.base.width);
             }
             if self.padding.top.is_relative() || self.padding.bottom.is_relative() {
-                frame.constraints.base.vertical = Some(original.base.height);
+                frame.constraints.base.vertical = Some(regions.base.height);
             }
 
             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 504981e6..45eefe29 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -217,9 +217,11 @@ impl<'a> ParLayouter<'a> {
                 // 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,
-                );
+                stack
+                    .constraints
+                    .max
+                    .vertical
+                    .set_min(stack.size.height + line.size.height);
                 stack.finish_region(ctx);
             }
 
@@ -230,9 +232,11 @@ impl<'a> ParLayouter<'a> {
                     break;
                 }
 
-                stack.constraints.max.vertical.set_min(
-                    stack.full.height - stack.regions.current.height + line.size.height,
-                );
+                stack
+                    .constraints
+                    .max
+                    .vertical
+                    .set_min(stack.size.height + line.size.height);
                 stack.finish_region(ctx);
             }
             // If the line does not fit horizontally or we have a mandatory
diff --git a/src/layout/stack.rs b/src/layout/stack.rs
index 045a06e1..fa5fd584 100644
--- a/src/layout/stack.rs
+++ b/src/layout/stack.rs
@@ -88,12 +88,12 @@ impl<'a> StackLayouter<'a> {
         Self {
             stack,
             main,
-            constraints: Constraints::new(expand),
             expand,
             regions,
             full,
             used: Gen::zero(),
             ruler: Align::Start,
+            constraints: Constraints::new(expand),
             overflowing: false,
             frames: vec![],
             finished: vec![],
@@ -154,7 +154,7 @@ impl<'a> StackLayouter<'a> {
             self.constraints
                 .max
                 .get_mut(self.main)
-                .set_min(size.main + self.used.main);
+                .set_min(self.used.main + size.main);
             self.finish_region();
         }
 
-- 
cgit v1.2.3