Refactor and comment ♻

4 files changed, 308 insertions, 257 deletions

diff --git a/src/exec/state.rs b/src/exec/state.rs
index c579bc4e..82f653e9 100644
--- a/src/exec/state.rs
+++ b/src/exec/state.rs
@@ -97,6 +97,7 @@ pub struct ParState {
     /// The spacing between lines (dependent on scaled font size).
     pub leading: Linear,
     /// The spacing between words (dependent on scaled font size).
+    // TODO: Don't ignore this.
     pub word_spacing: Linear,
 }
 
diff --git a/src/layout/par.rs b/src/layout/par.rs
index cad53cd5..f7d67981 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -6,7 +6,7 @@ use xi_unicode::LineBreakIterator;
 
 use super::*;
 use crate::exec::FontProps;
-use crate::util::RangeExt;
+use crate::util::{RangeExt, SliceExt};
 
 type Range = std::ops::Range<usize>;
 
@@ -35,14 +35,14 @@ pub enum ParChild {
 impl Layout for ParNode {
     fn layout(&self, ctx: &mut LayoutContext, areas: &Areas) -> Vec<Frame> {
         // Collect all text into one string used for BiDi analysis.
-        let (text, ranges) = self.collect_text();
+        let text = self.collect_text();
 
         // Find out the BiDi embedding levels.
         let bidi = BidiInfo::new(&text, Level::from_dir(self.dir));
 
         // Build a representation of the paragraph on which we can do
         // linebreaking without layouting each and every line from scratch.
-        let layout = ParLayout::new(ctx, areas, self, bidi, ranges);
+        let layout = ParLayout::new(ctx, areas, self, bidi);
 
         // Find suitable linebreaks.
         layout.build(ctx, areas.clone(), self)
@@ -54,21 +54,49 @@ impl ParNode {
     /// with a space character and other non-text nodes with the object
     /// replacement character. Returns the full text alongside the range each
     /// child spans in the text.
-    fn collect_text(&self) -> (String, Vec<Range>) {
+    fn collect_text(&self) -> String {
         let mut text = String::new();
-        let mut ranges = vec![];
+        for string in self.strings() {
+            text.push_str(string);
+        }
+        text
+    }
 
-        for child in &self.children {
-            let start = text.len();
-            match *child {
-                ParChild::Spacing(_) => text.push(' '),
-                ParChild::Text(ref piece, _, _) => text.push_str(piece),
-                ParChild::Any(_, _) => text.push('\u{FFFC}'),
+    /// The range of each item in the collected text.
+    fn ranges(&self) -> impl Iterator<Item = Range> + '_ {
+        let mut cursor = 0;
+        self.strings().map(move |string| {
+            let start = cursor;
+            cursor += string.len();
+            start .. cursor
+        })
+    }
+
+    /// The string representation of each child.
+    fn strings(&self) -> impl Iterator<Item = &str> {
+        self.children.iter().map(|child| match child {
+            ParChild::Spacing(_) => " ",
+            ParChild::Text(ref piece, _, _) => piece,
+            ParChild::Any(_, _) => "\u{FFFC}",
+        })
+    }
+}
+
+impl From<ParNode> for AnyNode {
+    fn from(par: ParNode) -> Self {
+        Self::new(par)
+    }
+}
+
+impl Debug for ParChild {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        match self {
+            Self::Spacing(amount) => write!(f, "Spacing({:?})", amount),
+            Self::Text(text, _, align) => write!(f, "Text({:?}, {:?})", text, align),
+            Self::Any(any, align) => {
+                f.debug_tuple("Any").field(any).field(align).finish()
             }
-            ranges.push(start .. text.len());
         }
-
-        (text, ranges)
     }
 }
 
@@ -85,16 +113,6 @@ struct ParLayout<'a> {
     ranges: Vec<Range>,
 }
 
-/// A prepared item in a paragraph layout.
-enum ParItem<'a> {
-    /// Spacing between other items.
-    Spacing(Length),
-    /// A shaped text run with consistent direction.
-    Text(ShapedText<'a>, Align),
-    /// A layouted child node.
-    Frame(Frame, Align),
-}
-
 impl<'a> ParLayout<'a> {
     /// Build a paragraph layout for the given node.
     fn new(
@@ -102,28 +120,26 @@ impl<'a> ParLayout<'a> {
         areas: &Areas,
         par: &'a ParNode,
         bidi: BidiInfo<'a>,
-        ranges: Vec<Range>,
     ) -> Self {
         // Prepare an iterator over each child an the range it spans.
-        let iter = ranges.into_iter().zip(&par.children);
-
         let mut items = vec![];
         let mut ranges = vec![];
 
         // Layout the children and collect them into items.
-        for (range, child) in iter {
+        for (range, child) in par.ranges().zip(&par.children) {
             match *child {
                 ParChild::Spacing(amount) => {
                     items.push(ParItem::Spacing(amount));
                     ranges.push(range);
                 }
                 ParChild::Text(_, ref props, align) => {
-                    split_runs(&bidi, range, |sub, dir| {
-                        let text = &bidi.text[sub.clone()];
-                        let shaped = shape(text, dir, &mut ctx.env.fonts, props);
+                    // TODO: Also split by language and script.
+                    for (subrange, dir) in split_runs(&bidi, range) {
+                        let text = &bidi.text[subrange.clone()];
+                        let shaped = shape(ctx, text, dir, props);
                         items.push(ParItem::Text(shaped, align));
-                        ranges.push(sub);
-                    });
+                        ranges.push(subrange);
+                    }
                 }
                 ParChild::Any(ref node, align) => {
                     let frames = node.layout(ctx, areas);
@@ -141,23 +157,33 @@ impl<'a> ParLayout<'a> {
 
     /// Find first-fit line breaks and build the paragraph.
     fn build(self, ctx: &mut LayoutContext, areas: Areas, par: &ParNode) -> Vec<Frame> {
-        let mut start = 0;
-        let mut last = None;
         let mut stack = LineStack::new(par.line_spacing, areas);
 
+        // The current line attempt.
+        // Invariant: Always fits into `stack.areas.current`.
+        let mut last = None;
+
+        // The start of the line in `last`.
+        let mut start = 0;
+
         // Find suitable line breaks.
         // TODO: Provide line break opportunities on alignment changes.
         for (end, mandatory) in LineBreakIterator::new(self.bidi.text) {
-            let mut line = LineLayout::new(&self, start .. end, ctx);
+            // Compute the line and its size.
+            let mut line = LineLayout::new(ctx, &self, start .. end);
 
+            // If the line doesn't fit anymore, we push the last fitting attempt
+            // into the stack and rebuild the line from its end. The resulting
+            // line cannot be broken up further.
             if !stack.areas.current.fits(line.size) {
                 if let Some((last_line, last_end)) = last.take() {
                     stack.push(last_line);
                     start = last_end;
-                    line = LineLayout::new(&self, start .. end, ctx);
+                    line = LineLayout::new(ctx, &self, start .. end);
                 }
             }
 
+            // If the line does not fit vertically, we start a new area.
             if !stack.areas.current.height.fits(line.size.height)
                 && !stack.areas.in_full_last()
             {
@@ -165,14 +191,21 @@ impl<'a> ParLayout<'a> {
             }
 
             if mandatory || !stack.areas.current.width.fits(line.size.width) {
+                // 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.
                 stack.push(line);
                 start = end;
                 last = None;
 
+                // If there is a trailing line break at the end of the
+                // paragraph, we want to force an empty line.
                 if mandatory && end == self.bidi.text.len() {
-                    stack.push(LineLayout::new(&self, end .. end, ctx));
+                    stack.push(LineLayout::new(ctx, &self, end .. end));
                 }
             } else {
+                // Otherwise, the line fits both horizontally and vertically
+                // and we remember it.
                 last = Some((line, end));
             }
         }
@@ -185,69 +218,156 @@ impl<'a> ParLayout<'a> {
     }
 
     /// Find the index of the item whose range contains the `text_offset`.
-    #[track_caller]
-    fn find(&self, text_offset: usize) -> usize {
-        find_range(&self.ranges, text_offset).unwrap()
+    fn find(&self, text_offset: usize) -> Option<usize> {
+        self.ranges.binary_search_by(|r| r.locate(text_offset)).ok()
     }
 }
 
+/// Split a range of text into runs of consistent direction.
+fn split_runs<'a>(
+    bidi: &'a BidiInfo,
+    range: Range,
+) -> impl Iterator<Item = (Range, Dir)> + 'a {
+    let mut cursor = range.start;
+    bidi.levels[range.clone()]
+        .group_by_key(|&level| level)
+        .map(move |(level, group)| {
+            let start = cursor;
+            cursor += group.len();
+            (start .. cursor, level.dir())
+        })
+}
+
+/// A prepared item in a paragraph layout.
+enum ParItem<'a> {
+    /// Spacing between other items.
+    Spacing(Length),
+    /// A shaped text run with consistent direction.
+    Text(ShapedText<'a>, Align),
+    /// A layouted child node.
+    Frame(Frame, Align),
+}
+
 impl ParItem<'_> {
-    /// The size and baseline of the item.
-    pub fn measure(&self) -> (Size, Length) {
+    /// The size of the item.
+    pub fn size(&self) -> Size {
+        match self {
+            Self::Spacing(amount) => Size::new(*amount, Length::ZERO),
+            Self::Text(shaped, _) => shaped.size,
+            Self::Frame(frame, _) => frame.size,
+        }
+    }
+
+    /// The baseline of the item.
+    pub fn baseline(&self) -> Length {
         match self {
-            Self::Spacing(amount) => (Size::new(*amount, Length::ZERO), Length::ZERO),
-            Self::Text(shaped, _) => (shaped.size, shaped.baseline),
-            Self::Frame(frame, _) => (frame.size, frame.baseline),
+            Self::Spacing(_) => Length::ZERO,
+            Self::Text(shaped, _) => shaped.baseline,
+            Self::Frame(frame, _) => frame.baseline,
         }
     }
 }
 
-/// Split a range of text into runs of consistent direction.
-fn split_runs(bidi: &BidiInfo, range: Range, mut f: impl FnMut(Range, Dir)) {
-    let levels = &bidi.levels[range.clone()];
-
-    let mut start = range.start;
-    let mut last = match levels.first() {
-        Some(&level) => level,
-        None => return,
-    };
-
-    // Split into runs with the same embedding level.
-    for (idx, &level) in levels.iter().enumerate() {
-        let end = range.start + idx;
-        if last != level {
-            f(start .. end, last.dir());
-            start = end;
+/// A simple layouter that stacks lines into areas.
+struct LineStack<'a> {
+    line_spacing: Length,
+    areas: Areas,
+    finished: Vec<Frame>,
+    lines: Vec<LineLayout<'a>>,
+    size: Size,
+}
+
+impl<'a> LineStack<'a> {
+    fn new(line_spacing: Length, areas: Areas) -> Self {
+        Self {
+            line_spacing,
+            areas,
+            finished: vec![],
+            lines: vec![],
+            size: Size::ZERO,
+        }
+    }
+
+    fn push(&mut self, line: LineLayout<'a>) {
+        self.areas.current.height -= line.size.height + self.line_spacing;
+
+        self.size.width = self.size.width.max(line.size.width);
+        self.size.height += line.size.height;
+        if !self.lines.is_empty() {
+            self.size.height += self.line_spacing;
         }
-        last = level;
+
+        self.lines.push(line);
     }
 
-    f(start .. range.end, last.dir());
+    fn finish_area(&mut self, ctx: &mut LayoutContext) {
+        let expand = self.areas.expand.horizontal;
+        self.size.width = expand.resolve(self.size.width, self.areas.full.width);
+
+        let mut output = Frame::new(self.size, self.size.height);
+        let mut first = true;
+        let mut offset = Length::ZERO;
+
+        for line in mem::take(&mut self.lines) {
+            let frame = line.build(ctx, self.size.width);
+            let Frame { size, baseline, .. } = frame;
+
+            let pos = Point::new(Length::ZERO, offset);
+            output.push_frame(pos, frame);
+
+            if first {
+                output.baseline = offset + baseline;
+                first = false;
+            }
+
+            offset += size.height + self.line_spacing;
+        }
+
+        self.finished.push(output);
+        self.areas.next();
+        self.size = Size::ZERO;
+    }
+
+    fn finish(mut self, ctx: &mut LayoutContext) -> Vec<Frame> {
+        self.finish_area(ctx);
+        self.finished
+    }
 }
 
 /// A lightweight representation of a line that spans a specific range in a
 /// paragraph's text. This type enables you to cheaply measure the size of a
 /// line in a range before comitting to building the line's frame.
 struct LineLayout<'a> {
+    /// The paragraph the line was created in.
     par: &'a ParLayout<'a>,
+    /// The range the line spans in the paragraph.
     line: Range,
+    /// A reshaped text item if the line sliced up a text item at the start.
     first: Option<ParItem<'a>>,
+    /// Middle items which don't need to be reprocessed.
     items: &'a [ParItem<'a>],
+    /// A reshaped text item if the line sliced up a text item at the end. If
+    /// there is only one text item, this takes precedence over `first`.
     last: Option<ParItem<'a>>,
+    /// The ranges, indexed as `[first, ..items, last]`. The ranges for `first`
+    /// and `last` aren't trimmed to the line, but it doesn't matter because
+    /// we're just checking which range an index falls into.
     ranges: &'a [Range],
+    /// The size of the line.
     size: Size,
+    /// The baseline of the line.
     baseline: Length,
 }
 
 impl<'a> LineLayout<'a> {
     /// Create a line which spans the given range.
-    fn new(par: &'a ParLayout<'a>, mut line: Range, ctx: &mut LayoutContext) -> Self {
+    fn new(ctx: &mut LayoutContext, par: &'a ParLayout<'a>, mut line: Range) -> Self {
         // Find the items which bound the text range.
-        let last_idx = par.find(line.end - 1);
+        let last_idx = par.find(line.end.saturating_sub(1)).unwrap();
         let first_idx = if line.is_empty() {
             last_idx
         } else {
-            par.find(line.start)
+            par.find(line.start).unwrap()
         };
 
         // Slice out the relevant items and ranges.
@@ -257,44 +377,57 @@ impl<'a> LineLayout<'a> {
         // Reshape the last item if it's split in half.
         let mut last = None;
         if let Some((ParItem::Text(shaped, align), rest)) = items.split_last() {
-            // Compute the string slice indices local to the shaped result.
-            let range = &par.ranges[last_idx];
-            let start = line.start.max(range.start) - range.start;
-            let end = line.end - range.start;
-
-            // Trim whitespace at the end of the line.
-            let end = start + shaped.text[start .. end].trim_end().len();
-            line.end = range.start + end;
-
-            if start != end || rest.is_empty() {
-                // Reshape that part (if the indices span the full range reshaping
-                // is fast and does nothing).
-                let reshaped = shaped.reshape(start .. end, &mut ctx.env.fonts);
-                last = Some(ParItem::Text(reshaped, *align));
-            }
+            // Compute the range we want to shape, trimming whitespace at the
+            // end of the line.
+            let base = par.ranges[last_idx].start;
+            let start = line.start.max(base);
+            let end = start + par.bidi.text[start .. line.end].trim_end().len();
+            let range = start - base .. end - base;
+
+            // Reshape if necessary.
+            if range.len() < shaped.text.len() {
+                // If start == end and the rest is empty, then we have an empty
+                // line. To make that line have the appropriate height, we shape the
+                // empty string.
+                if !range.is_empty() || rest.is_empty() {
+                    // Reshape that part.
+                    let reshaped = shaped.reshape(ctx, range);
+                    last = Some(ParItem::Text(reshaped, *align));
+                }
 
-            items = rest;
+                items = rest;
+                line.end = end;
+            }
         }
 
         // Reshape the start item if it's split in half.
         let mut first = None;
         if let Some((ParItem::Text(shaped, align), rest)) = items.split_first() {
-            let range = &par.ranges[first_idx];
-            let start = line.start - range.start;
-            let end = line.end.min(range.end) - range.start;
-            if start != end {
-                let reshaped = shaped.reshape(start .. end, &mut ctx.env.fonts);
-                first = Some(ParItem::Text(reshaped, *align));
+            // Compute the range we want to shape.
+            let Range { start: base, end: first_end } = par.ranges[first_idx];
+            let start = line.start;
+            let end = line.end.min(first_end);
+            let range = start - base .. end - base;
+
+            // Reshape if necessary.
+            if range.len() < shaped.text.len() {
+                if !range.is_empty() {
+                    let reshaped = shaped.reshape(ctx, range);
+                    first = Some(ParItem::Text(reshaped, *align));
+                }
+
+                items = rest;
             }
-            items = rest;
         }
 
         let mut width = Length::ZERO;
         let mut top = Length::ZERO;
         let mut bottom = Length::ZERO;
 
+        // Measure the size of the line.
         for item in first.iter().chain(items).chain(&last) {
-            let (size, baseline) = item.measure();
+            let size = item.size();
+            let baseline = item.baseline();
             width += size.width;
             top = top.max(baseline);
             bottom = bottom.max(size.height - baseline);
@@ -314,33 +447,38 @@ impl<'a> LineLayout<'a> {
 
     /// Build the line's frame.
     fn build(&self, ctx: &mut LayoutContext, width: Length) -> Frame {
-        let (size, baseline) = (self.size, self.baseline);
-        let full_size = Size::new(size.width.max(width), size.height);
+        let full_width = self.size.width.max(width);
+        let full_size = Size::new(full_width, self.size.height);
+        let free_width = full_width - self.size.width;
 
-        let mut output = Frame::new(full_size, baseline);
+        let mut output = Frame::new(full_size, self.baseline);
+        let mut ruler = Align::Start;
         let mut offset = Length::ZERO;
 
-        let mut ruler = Align::Start;
         self.reordered(|item| {
-            let (frame, align) = match *item {
+            let frame = match *item {
                 ParItem::Spacing(amount) => {
                     offset += amount;
                     return;
                 }
                 ParItem::Text(ref shaped, align) => {
-                    (shaped.build(&mut ctx.env.fonts), align)
+                    ruler = ruler.max(align);
+                    shaped.build(ctx)
+                }
+                ParItem::Frame(ref frame, align) => {
+                    ruler = ruler.max(align);
+                    frame.clone()
                 }
-                ParItem::Frame(ref frame, align) => (frame.clone(), align),
             };
 
-            ruler = ruler.max(align);
-
-            let range = offset .. full_size.width - size.width + offset;
-            let x = ruler.resolve(self.par.dir, range);
-            let y = baseline - frame.baseline;
+            let Frame { size, baseline, .. } = frame;
+            let pos = Point::new(
+                ruler.resolve(self.par.dir, offset .. free_width + offset),
+                self.baseline - baseline,
+            );
 
-            offset += frame.size.width;
-            output.push_frame(Point::new(x, y), frame);
+            output.push_frame(pos, frame);
+            offset += size.width;
         });
 
         output
@@ -348,11 +486,12 @@ impl<'a> LineLayout<'a> {
 
     /// Iterate through the line's items in visual order.
     fn reordered(&self, mut f: impl FnMut(&ParItem<'a>)) {
+        // The bidi crate doesn't like empty lines.
         if self.line.is_empty() {
             return;
         }
 
-        // Find the paragraph that contains the frame.
+        // Find the paragraph that contains the line.
         let para = self
             .par
             .bidi
@@ -366,108 +505,32 @@ impl<'a> LineLayout<'a> {
 
         // Find the items for each run.
         for run in runs {
-            let first_idx = self.find(run.start);
-            let last_idx = self.find(run.end - 1);
+            let first_idx = self.find(run.start).unwrap();
+            let last_idx = self.find(run.end - 1).unwrap();
             let range = first_idx ..= last_idx;
 
             // Provide the items forwards or backwards depending on the run's
             // direction.
             if levels[run.start].is_ltr() {
                 for item in range {
-                    f(self.get(item));
+                    f(self.get(item).unwrap());
                 }
             } else {
                 for item in range.rev() {
-                    f(self.get(item));
+                    f(self.get(item).unwrap());
                 }
             }
         }
     }
 
     /// Find the index of the item whose range contains the `text_offset`.
-    #[track_caller]
-    fn find(&self, text_offset: usize) -> usize {
-        find_range(self.ranges, text_offset).unwrap()
+    fn find(&self, text_offset: usize) -> Option<usize> {
+        self.ranges.binary_search_by(|r| r.locate(text_offset)).ok()
     }
 
     /// Get the item at the index.
-    #[track_caller]
-    fn get(&self, index: usize) -> &ParItem<'a> {
-        self.iter().nth(index).unwrap()
-    }
-
-    /// Iterate over the items of the line.
-    fn iter(&self) -> impl Iterator<Item = &ParItem<'a>> {
-        self.first.iter().chain(self.items).chain(&self.last)
-    }
-}
-
-/// Find the range that contains the position.
-fn find_range(ranges: &[Range], pos: usize) -> Option<usize> {
-    ranges.binary_search_by(|r| r.locate(pos)).ok()
-}
-
-/// Stacks lines into paragraph frames.
-struct LineStack<'a> {
-    line_spacing: Length,
-    areas: Areas,
-    finished: Vec<Frame>,
-    lines: Vec<LineLayout<'a>>,
-    size: Size,
-}
-
-impl<'a> LineStack<'a> {
-    fn new(line_spacing: Length, areas: Areas) -> Self {
-        Self {
-            line_spacing,
-            areas,
-            finished: vec![],
-            lines: vec![],
-            size: Size::ZERO,
-        }
-    }
-
-    fn push(&mut self, line: LineLayout<'a>) {
-        self.size.width = self.size.width.max(line.size.width);
-        self.size.height += line.size.height;
-        if !self.lines.is_empty() {
-            self.size.height += self.line_spacing;
-        }
-
-        self.areas.current.height -= line.size.height + self.line_spacing;
-        self.lines.push(line);
-    }
-
-    fn finish_area(&mut self, ctx: &mut LayoutContext) {
-        let expand = self.areas.expand.horizontal;
-        let full = self.areas.full.width;
-        self.size.width = expand.resolve(self.size.width, full);
-
-        let mut output = Frame::new(self.size, self.size.height);
-        let mut y = Length::ZERO;
-        let mut first = true;
-
-        for line in mem::take(&mut self.lines) {
-            let frame = line.build(ctx, self.size.width);
-            let height = frame.size.height;
-
-            if first {
-                output.baseline = y + frame.baseline;
-                first = false;
-            }
-
-            output.push_frame(Point::new(Length::ZERO, y), frame);
-            y += height + self.line_spacing;
-        }
-
-        self.finished.push(output);
-        self.areas.next();
-        self.size = Size::ZERO;
-    }
-
-    fn finish(mut self, ctx: &mut LayoutContext) -> Vec<Frame> {
-        self.finish_area(ctx);
-        self.finished
+    fn get(&self, index: usize) -> Option<&ParItem<'a>> {
+        self.first.iter().chain(self.items).chain(&self.last).nth(index)
     }
 }
 
@@ -490,21 +553,3 @@ impl LevelExt for Level {
         if self.is_ltr() { Dir::LTR } else { Dir::RTL }
     }
 }
-
-impl From<ParNode> for AnyNode {
-    fn from(par: ParNode) -> Self {
-        Self::new(par)
-    }
-}
-
-impl Debug for ParChild {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        match self {
-            Self::Spacing(amount) => write!(f, "Spacing({:?})", amount),
-            Self::Text(text, _, align) => write!(f, "Text({:?}, {:?})", text, align),
-            Self::Any(any, align) => {
-                f.debug_tuple("Any").field(any).field(align).finish()
-            }
-        }
-    }
-}
diff --git a/src/layout/shaping.rs b/src/layout/shaping.rs
index 5a3cd292..faa178d3 100644
--- a/src/layout/shaping.rs
+++ b/src/layout/shaping.rs
@@ -6,7 +6,7 @@ use fontdock::FaceId;
 use rustybuzz::UnicodeBuffer;
 use ttf_parser::GlyphId;
 
-use super::{Element, Frame, Glyph, Text};
+use super::{Element, Frame, Glyph, LayoutContext, Text};
 use crate::env::FontLoader;
 use crate::exec::FontProps;
 use crate::font::FaceBuf;
@@ -60,12 +60,12 @@ enum Side {
 
 impl<'a> ShapedText<'a> {
     /// Build the shaped text's frame.
-    pub fn build(&self, loader: &mut FontLoader) -> Frame {
+    pub fn build(&self, ctx: &mut LayoutContext) -> Frame {
         let mut frame = Frame::new(self.size, self.baseline);
-        let mut x = Length::ZERO;
+        let mut offset = Length::ZERO;
 
         for (face_id, group) in self.glyphs.as_ref().group_by_key(|g| g.face_id) {
-            let pos = Point::new(x, self.baseline);
+            let pos = Point::new(offset, self.baseline);
             let mut text = Text {
                 face_id,
                 size: self.props.size,
@@ -73,12 +73,12 @@ impl<'a> ShapedText<'a> {
                 glyphs: vec![],
             };
 
-            let face = loader.face(face_id);
+            let face = ctx.env.fonts.face(face_id);
             for glyph in group {
                 let x_advance = face.convert(glyph.x_advance).scale(self.props.size);
                 let x_offset = face.convert(glyph.x_offset).scale(self.props.size);
                 text.glyphs.push(Glyph { id: glyph.glyph_id, x_advance, x_offset });
-                x += x_advance;
+                offset += x_advance;
             }
 
             frame.push(pos, Element::Text(text));
@@ -91,11 +91,11 @@ impl<'a> ShapedText<'a> {
     /// shaping process if possible.
     pub fn reshape(
         &'a self,
+        ctx: &mut LayoutContext,
         text_range: Range<usize>,
-        loader: &mut FontLoader,
     ) -> ShapedText<'a> {
         if let Some(glyphs) = self.slice_safe_to_break(text_range.clone()) {
-            let (size, baseline) = measure(glyphs, loader, self.props);
+            let (size, baseline) = measure(&mut ctx.env.fonts, glyphs, self.props);
             Self {
                 text: &self.text[text_range],
                 dir: self.dir,
@@ -105,7 +105,7 @@ impl<'a> ShapedText<'a> {
                 glyphs: Cow::Borrowed(glyphs),
             }
         } else {
-            shape(&self.text[text_range], self.dir, loader, self.props)
+            shape(ctx, &self.text[text_range], self.dir, self.props)
         }
     }
 
@@ -176,18 +176,21 @@ impl Debug for ShapedText<'_> {
 
 /// Shape text into [`ShapedText`].
 pub fn shape<'a>(
+    ctx: &mut LayoutContext,
     text: &'a str,
     dir: Dir,
-    loader: &mut FontLoader,
     props: &'a FontProps,
 ) -> ShapedText<'a> {
+    let loader = &mut ctx.env.fonts;
+
     let mut glyphs = vec![];
     let families = props.families.iter();
     if !text.is_empty() {
-        shape_segment(&mut glyphs, 0, text, dir, loader, props, families, None);
+        shape_segment(loader, &mut glyphs, 0, text, dir, props, families, None);
     }
 
-    let (size, baseline) = measure(&glyphs, loader, props);
+    let (size, baseline) = measure(loader, &glyphs, props);
+
     ShapedText {
         text,
         dir,
@@ -200,14 +203,14 @@ pub fn shape<'a>(
 
 /// Shape text with font fallback using the `families` iterator.
 fn shape_segment<'a>(
+    loader: &mut FontLoader,
     glyphs: &mut Vec<ShapedGlyph>,
     base: usize,
     text: &str,
     dir: Dir,
-    loader: &mut FontLoader,
     props: &FontProps,
     mut families: impl Iterator<Item = &'a str> + Clone,
-    mut first: Option<FaceId>,
+    mut first_face: Option<FaceId>,
 ) {
     // Select the font family.
     let (face_id, fallback) = loop {
@@ -219,17 +222,16 @@ fn shape_segment<'a>(
             },
             // We're out of families, so we don't do any more fallback and just
             // shape the tofus with the first face we originally used.
-            None => match first {
+            None => match first_face {
                 Some(id) => break (id, false),
                 None => return,
             },
         }
     };
 
-    // Register that this is the first available font.
-    if first.is_none() {
-        first = Some(face_id);
-    }
+    // Remember the id if this the first available face since we use that one to
+    // shape tofus.
+    first_face.get_or_insert(face_id);
 
     // Fill the buffer with our text.
     let mut buffer = UnicodeBuffer::new();
@@ -245,7 +247,8 @@ fn shape_segment<'a>(
     let infos = buffer.glyph_infos();
     let pos = buffer.glyph_positions();
 
-    // Collect the shaped glyphs, reshaping with the next font if necessary.
+    // Collect the shaped glyphs, doing fallback and shaping parts again with
+    // the next font if necessary.
     let mut i = 0;
     while i < infos.len() {
         let info = &infos[i];
@@ -263,30 +266,32 @@ fn shape_segment<'a>(
                 safe_to_break: !info.unsafe_to_break(),
             });
         } else {
-            // Do font fallback if the glyph is a tofu.
-            //
-            // First, search for the end of the tofu sequence.
-            let k = i;
-            while infos.get(i + 1).map_or(false, |info| info.codepoint == 0) {
-                i += 1;
-            }
-
             // Determine the source text range for the tofu sequence.
             let range = {
-                // Examples
+                // First, search for the end of the tofu sequence.
+                let k = i;
+                while infos.get(i + 1).map_or(false, |info| info.codepoint == 0) {
+                    i += 1;
+                }
+
+                // Then, determine the start and end text index.
                 //
-                // Here, _ is a tofu.
-                // Note that the glyph cluster length is greater than 1 char!
+                // Examples:
+                // Everything is shown in visual order. Tofus are written as "_".
+                // We want to find out that the tofus span the text `2..6`.
+                // Note that the clusters are longer than 1 char.
                 //
-                // Left-to-right clusters:
-                // h a l i h a l l o
-                // A   _   _   C   E
-                // 0   2   4   6   8
+                // Left-to-right:
+                // Text:     h a l i h a l l o
+                // Glyphs:   A   _   _   C   E
+                // Clusters: 0   2   4   6   8
+                //              k=1 i=2
                 //
-                // Right-to-left clusters:
-                // O L L A H I L A H
-                // E   C   _   _   A
-                // 8   6   4   2   0
+                // Right-to-left:
+                // Text:     O L L A H I L A H
+                // Glyphs:   E   C   _   _   A
+                // Clusters: 8   6   4   2   0
+                //                  k=2 i=3
 
                 let ltr = dir.is_positive();
                 let first = if ltr { k } else { i };
@@ -295,22 +300,21 @@ fn shape_segment<'a>(
                 let last = if ltr { i.checked_add(1) } else { k.checked_sub(1) };
                 let end = last
                     .and_then(|last| infos.get(last))
-                    .map(|info| info.cluster as usize)
-                    .unwrap_or(text.len());
+                    .map_or(text.len(), |info| info.cluster as usize);
 
                 start .. end
             };
 
             // Recursively shape the tofu sequence with the next family.
             shape_segment(
+                loader,
                 glyphs,
                 base + range.start,
                 &text[range],
                 dir,
-                loader,
                 props,
                 families.clone(),
-                first,
+                first_face,
             );
         }
 
@@ -321,14 +325,14 @@ fn shape_segment<'a>(
 /// Measure the size and baseline of a run of shaped glyphs with the given
 /// properties.
 fn measure(
-    glyphs: &[ShapedGlyph],
     loader: &mut FontLoader,
+    glyphs: &[ShapedGlyph],
     props: &FontProps,
 ) -> (Size, Length) {
+    let mut width = Length::ZERO;
     let mut top = Length::ZERO;
     let mut bottom = Length::ZERO;
-    let mut width = Length::ZERO;
-    let mut vertical = |face: &FaceBuf| {
+    let mut expand_vertical = |face: &FaceBuf| {
         top = top.max(face.vertical_metric(props.top_edge).scale(props.size));
         bottom = bottom.max(-face.vertical_metric(props.bottom_edge).scale(props.size));
     };
@@ -338,14 +342,14 @@ fn measure(
         // first available font.
         for family in props.families.iter() {
             if let Some(face_id) = loader.query(family, props.variant) {
-                vertical(loader.face(face_id));
+                expand_vertical(loader.face(face_id));
                 break;
             }
         }
     } else {
         for (face_id, group) in glyphs.group_by_key(|g| g.face_id) {
             let face = loader.face(face_id);
-            vertical(face);
+            expand_vertical(face);
 
             for glyph in group {
                 width += face.convert(glyph.x_advance).scale(props.size);
diff --git a/src/util.rs b/src/util.rs
index 6fda2fb5..72db4518 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -1,4 +1,5 @@
-/// Utilities.
+//! Utilities.
+
 use std::cmp::Ordering;
 use std::ops::Range;