Implement `parley_core::ShapedText` by tomcur · Pull Request #679 · linebender/parley

tomcur · 2026-07-09T14:53:01Z

This moves shaped run processing from parley to parley_core, and lets parley_core own the shaped results in ShapedText. parley_core now fully owns shaping.

parley stores ShapedText in its LayoutData, dropping its cluster/glyph/etc. vecs. It keeps some run data as a parallel vector to ShapedText::runs, storing some data that parley_core is not aware of.

A large part of the diff (process_clusters) was moved verbatim to core. ShapedContext::push_run is mostly blocks moved out of the old fn push_run that was in parley/src/layout/data.rs.

There's one escape hatch that we should try to get rid of: because parley was mutating shaped data in-place for justification and letter spacing and such, parley_core exposes #[doc(hidden)] mutable access to two ShapedText fields to keep parley happy. This in-place mutation isn't really great in itself, e.g., for justification, parley already has a hack to "unjustify" in-place... but with reshaping I suspect it's untenable.

Benches close to neutral on my machine

Default Style - arabic 20 characters               [   8.8 us ...   9.0 us ]      +2.10%*
Default Style - latin 20 characters                [   4.3 us ...   4.3 us ]      -0.18%
Default Style - japanese 20 characters             [   8.2 us ...   8.3 us ]      +0.88%
Default Style - arabic 1 paragraph                 [  47.7 us ...  47.9 us ]      +0.48%
Default Style - latin 1 paragraph                  [  16.6 us ...  16.6 us ]      +0.40%
Default Style - japanese 1 paragraph               [  70.1 us ...  70.7 us ]      +0.81%
Default Style - arabic 4 paragraph                 [ 200.7 us ... 200.4 us ]      -0.17%
Default Style - latin 4 paragraph                  [  62.2 us ...  62.9 us ]      +1.07%*
Default Style - japanese 4 paragraph               [  99.5 us ... 100.0 us ]      +0.52%
Styled - arabic 20 characters                      [   9.8 us ...  10.0 us ]      +1.85%*
Styled - latin 20 characters                       [   5.4 us ...   5.4 us ]      +0.19%
Styled - japanese 20 characters                    [   8.7 us ...   8.8 us ]      +0.92%
Styled - arabic 1 paragraph                        [  50.2 us ...  50.5 us ]      +0.63%
Styled - latin 1 paragraph                         [  20.9 us ...  21.2 us ]      +1.17%*
Styled - japanese 1 paragraph                      [  75.9 us ...  76.8 us ]      +1.27%*
Styled - arabic 4 paragraph                        [ 219.2 us ... 219.3 us ]      +0.06%
Styled - latin 4 paragraph                         [  81.6 us ...  82.4 us ]      +0.96%
Styled - japanese 4 paragraph                      [ 107.1 us ... 108.4 us ]      +1.23%*

nicoburns · 2026-07-09T15:23:01Z

There's one escape hatch that we should try to get rid of: because parley was mutating shaped data in-place for justification and letter spacing and such, parley_core exposes #[doc(hidden)] mutable access to two ShapedText fields to keep parley happy.

Why are the fields of ShapedText private at all? Wouldn't it be useful to consumers to have full access to the data?

tomcur · 2026-07-09T18:46:39Z

Why are the fields of ShapedText private at all? Wouldn't it be useful to consumers to have full access to the data?

The API gives full read access. I'm not sure about mutable access, as there are quite a few invariants that need to be upheld. There's a lot of cross-referencing; e.g., runs contain ranges tiling ShapedText's vecs, and e.g., run advances must be the sum of their glyphs' advances as well as their clusters' advances. Once we have reshaping, there'll be a bit more going on still.

Of course if a user has mutable access and breaks something, that's mostly fine as it's their own fault, but I'm not sure mutation is strongly needed. Parley currently only mutates advances in-place, but it's hacky. E.g. if a layout is justified, the advances are changed, and relayout requires a pass to unjustify first. As the original values are now lost, it's doing the same calculations in reverse; this kind of thing is going to drift because of compounding rounding errors after enough passes.

If ShapedText is a static source of truth for a paragraph, mutations are always applied from a clean slate. With what I have in mind currently, reshaping also only reads ShapedText and doesn't mutate it.

(I think having a bit of a closer look at what exactly the API exposes would be good, after this PR lands we've mostly arrived at the targeted seam between parley_core and parley.)

tomcur · 2026-07-12T22:22:27Z

+/// A normalized font coordinate.
+///
+/// This is a 16-bit fixed-point number with a 14-bit fractional part. For font coordinates, its
+/// useful values are in the range -1.0..=1.0.
+#[repr(transparent)]
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)]
+pub struct NormalizedCoord(i16);


This prevents exposing font_types::F2Dot14, but perhaps we should add bytemuck support so users can easily cast slices.

I think this type makes sense, but it ought to live in parlance (we have also previously just used plain i16 for this).

The prior art is https://docs.rs/vello_encoding/latest/vello_encoding/type.NormalizedCoord.html.

taj-p

LGTM with comments addressed! Nice 🚀 !!

taj-p · 2026-07-17T21:50:09Z

-        self.coords.clear();
        self.styles.clear();
        self.inline_boxes.clear();
-        self.runs.clear();


Since we still accumulate a self.runs, don't we need to continue clearing it here? Otherwise, rebuilding the same layout will see new runs back onto previous runs

Yes, absolutely. I initially fully removed RunData but later brought it back as parallel storage (with fields parley_core doesn't need to know about). Opened #701 so tests catch this!

taj-p · 2026-07-17T21:53:56Z

+    glyphs: Vec<Glyph>,
+    fonts: Vec<FontInstance>,
+    normalized_coords: Vec<NormalizedCoord>,
+    features: Vec<FontFeature>,


features is never written to AFAICT

True; removed. (It'll come back when we do reshaping.)

taj-p · 2026-07-17T21:55:48Z

+    }
+
+    run_advance
+}


oops - my bad. Rereading this code, I think we might have forgotten to add the last cluster's advance?

See this diff

diff --git a/parley_core/src/shape/shaped_text.rs b/parley_core/src/shape/shaped_text.rs index 559fdae..3043d51 100644 --- a/parley_core/src/shape/shaped_text.rs +++ b/parley_core/src/shape/shaped_text.rs @@ -420,10 +420,12 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>( for (glyph_info, glyph_pos) in glyph_infos.iter().zip(glyph_positions.iter()) { // Flush previous cluster if we've reached a new cluster if cluster_id != glyph_info.cluster { - run_advance += cluster_advance; let num_components = num_components(glyph_info.cluster, cluster_id, last_cluster_id); - cluster_advance /= num_components as f32; let is_newline = to_whitespace(cluster_start_char.1) == Whitespace::Newline; + if !is_newline { + run_advance += cluster_advance; + } if cluster_id != glyph_info.cluster { - run_advance += cluster_advance; let num_components = num_components(glyph_info.cluster, cluster_id, last_cluster_id); - cluster_advance /= num_components as f32; let is_newline = to_whitespace(cluster_start_char.1) == Whitespace::Newline; + if !is_newline { + run_advance += cluster_advance; + } + cluster_advance /= num_components as f32; let cluster_type = if num_components > 1 { debug_assert!(!is_newline); ClusterType::LigatureStart @@ -520,6 +522,10 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>( Direction::Rtl => 0, }; let num_components = num_components(next_cluster_id, cluster_id, last_cluster_id); + let is_newline = to_whitespace(cluster_start_char.1) == Whitespace::Newline; + if !is_newline { + run_advance += cluster_advance; + } if num_components > 1 { // This is a ligature - create ligature start + ligature components @@ -562,7 +568,6 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>( ); } } else { - let is_newline = to_whitespace(cluster_start_char.1) == Whitespace::Newline; let cluster_type = if is_newline { ClusterType::Newline } else { @@ -671,3 +676,50 @@ fn push_cluster( advance: final_advance, }); } + +#[cfg(test)] +mod tests { + use alloc::vec::Vec; + + use crate::{Analysis, AnalysisOptions, Analyzer}; + + use super::{Direction, process_clusters}; + + #[test] + fn run_advance_includes_final_cluster() { + let text = "a"; // <--- only one glyph in the output. I believe this is a valid bug that predates this change. + let mut analyzer = Analyzer::new(); + let mut analysis = Analysis::new(); + analyzer.analyze( + text, + &AnalysisOptions { + word_break: &[], + line_break_override: None, + }, + &mut analysis, + ); + + let mut glyph_info = harfrust::GlyphInfo::default(); + glyph_info.glyph_id = 1; + glyph_info.cluster = 0; + let mut glyph_position = harfrust::GlyphPosition::default(); + glyph_position.x_advance = 20; + + let mut clusters = Vec::new(); + let mut glyphs = Vec::new(); + let run_advance = process_clusters( + Direction::Ltr, + &mut clusters, + &mut glyphs, + 0.5, + &[glyph_info], + &[glyph_position], + analysis.char_info(), + &[0], + text.char_indices(), + ); + + let cluster_advance = clusters.iter().map(|cluster| cluster.advance).sum(); + assert_eq!(run_advance, cluster_advance); + } +}

Yeah, I think so! An LLM also flagged this when I was doing the migration, so I put this on a list of things to look at soon-ish (unless you want to take a look).

taj-p · 2026-07-17T22:03:07Z

+                let cluster = run.range.char_range.end;
+                let text = run.range.byte_range.end;


Is this a typo? Should it be

Suggested change

let cluster = run.range.char_range.end;

let text = run.range.byte_range.end;

let cluster = run.clusters_range.end;

let text = run.range.byte_range.end;

Yep! Whoops! These clusters are one-to-one with characters, and as parley just shapes every single character in sequence the ranges are coincidentally identical.

taj-p · 2026-07-17T22:04:02Z

+                    let normalized_coords =
+                        &Vec::from_iter(run.normalized_coords().iter().map(|c| c.to_bits()));


This allocation is unfortunate! Do we want to have a bytemuck feature or similar so that we can:

let normalized_coords: &[i16] = bytemuck::cast_slice(run.normalized_coords());

Yeah, I think so, though Nico also requested the type to live in parlance, which probably makes sense. See the comment thread at #679 (comment).

I'll merge this for now and follow up in a separate PR.

taj-p · 2026-07-17T22:05:25Z

+                        let clusters =
+                            &mut layout.shaped_text.clusters_mut()[line_item.cluster_range.clone()];


🙈 - don't look here! 😆

Agree with PR description - you're right we should just expose this as a mutable until we finish the migration and discussion above.

taj-p · 2026-07-17T22:07:34Z

+            !shaped_run.clusters_range.is_empty(),
+            "Shaped runs return by `parley_core` must be non-empty"
+        );
+        let style_index = self.shaped_text.clusters()[shaped_run.clusters_range.start].style_index;


In the future, I think it's going to be fun to figure out how to support divergent glyph styles within a single cluster

I think some of the discussion in #660 and #661 is related.

taj-p · 2026-07-17T22:11:52Z

                    })
            },
            analysis_data_sources,
-            #[inline(always)]


taj-p · 2026-07-17T22:14:09Z

 core_maths = { version = "0.1.1", optional = true }
 accesskit = { workspace = true, optional = true }
 hashbrown = { workspace = true }
-harfrust = { workspace = true }


taj-p · 2026-07-17T22:27:53Z

+            let font = &self.fonts[font_index];
+            let font_ref =
+                skrifa::FontRef::from_index(font.font.data.as_ref(), font.font.index).unwrap();
+            skrifa::metrics::Metrics::new(


This call is ~2.2% of our entire pipeline, which was interesting to find out. I thought we might want to cache it in an LRU in the future, but there's probably bigger fish to fry before then!

taj-p · 2026-07-17T22:32:50Z

+        let glyph_positions = glyph_buffer.glyph_positions();
+        let scale_factor = options.font_size / units_per_em;
+        let clusters_start = self.clusters.len();
+        let is_rtl = !item.bidi_level.is_multiple_of(2);


I'm wondering whether a low effort perf optimisation is to reserve capacity ahead of time:

self.clusters.reserve(range.char_range.len()); self.glyphs.reserve(glyph_infos.len());

Turns out the answer is "yes"! See: #702 (comment).

taj-p · 2026-07-17T22:38:08Z

    let grapheme_cluster_boundaries = analysis_data_sources
        .grapheme_segmenter()
        .segment_str(item_text);


Hmmmmm, definitely outside of scope of this PR (and not a priority at all), but I guess because in analysis we calculate grapheme boundaries, this call isn't strictly required here

This moves run processing from `parley` to `parley_core`, and lets `parley_core` own the shaped results. `parley_core` now fully owns shaping. `parley` replaces its cluster/glyph/etc. stores in `LayoutData` with `ShapedText`, and keeping its run data (though dropping some fields) to store some run data `parley_core` is not aware of. There's one escape hatch that we should try to get rid of soon: because `parley` was mutating shaped data in-place for justification and letter spacing and such, `parley_core` exposes `#[doc(hidden)]` mutable access to two `ShapedText` fields to keep `parley` happy. That won't work anymore once we get reshaping across breaks. (For justification, that also meant we have a hack that has to unjustify in-place...)

To catch issues like linebender#679 (comment).

Implementing linebender#679 (comment) seems to be a nice little speed up. This pre-allocates at the granularity of items, which means the glyph allocation is speculative. An alternative is to pre-allocate at the smaller granularity of runs, once you know how many glyphs you need to push, but that turns out to be slower as you need to reallocate more often. We *could* expose `ShapedText::reserve` publicly and let users handle pre-allocation (at, e.g., the paragraph level), which theoretically could be slightly faster still, but that adds API surface for something that mostly stops mattering once you're reusing the `ShapedText` allocation. ``` $ cargo bench --bench main -- compare ../target/benchmarks/main -t 8. Default Style - arabic 20 characters [ 9.0 us ... 8.8 us ] -2.09%* Default Style - latin 20 characters [ 4.3 us ... 4.1 us ] -3.14%* Default Style - japanese 20 characters [ 8.3 us ... 8.3 us ] -0.33% Default Style - arabic 1 paragraph [ 48.7 us ... 47.8 us ] -1.80%* Default Style - latin 1 paragraph [ 16.8 us ... 16.5 us ] -2.07%* Default Style - japanese 1 paragraph [ 70.6 us ... 70.1 us ] -0.76% Default Style - arabic 4 paragraph [ 208.1 us ... 201.5 us ] -3.13%* Default Style - latin 4 paragraph [ 64.0 us ... 62.8 us ] -1.84%* Default Style - japanese 4 paragraph [ 99.9 us ... 98.8 us ] -1.03%* Styled - arabic 20 characters [ 10.1 us ... 9.8 us ] -2.32%* Styled - latin 20 characters [ 5.4 us ... 5.3 us ] -1.94%* Styled - japanese 20 characters [ 8.9 us ... 9.0 us ] +1.59%* Styled - arabic 1 paragraph [ 51.2 us ... 50.2 us ] -1.86%* Styled - latin 1 paragraph [ 21.4 us ... 21.1 us ] -1.11%* Styled - japanese 1 paragraph [ 77.2 us ... 76.7 us ] -0.73% Styled - arabic 4 paragraph [ 227.8 us ... 221.3 us ] -2.88%* Styled - latin 4 paragraph [ 83.2 us ... 82.3 us ] -1.09%* Styled - japanese 4 paragraph [ 109.1 us ... 108.1 us ] -0.98% ```

tomcur force-pushed the parley-core-shapedtext branch 2 times, most recently from b413454 to 41e0700 Compare July 9, 2026 15:19

tomcur force-pushed the parley-core-shapedtext branch from 41e0700 to 8203d81 Compare July 9, 2026 15:40

tomcur force-pushed the parley-core-shapedtext branch from 13a1cf7 to 99881e5 Compare July 10, 2026 14:30

tomcur mentioned this pull request Jul 12, 2026

Implement shaping in parley_core #675

Merged

tomcur force-pushed the parley-core-shapedtext branch from 0c5b94e to f94e80e Compare July 12, 2026 22:16

tomcur commented Jul 12, 2026

View reviewed changes

taj-p approved these changes Jul 17, 2026

View reviewed changes

taj-p reviewed Jul 17, 2026

View reviewed changes

tomcur added 4 commits July 21, 2026 11:53

Fmt, typo

5277f1d

Fix range

2a683ad

Dedup font metrics

50b07ac

tomcur force-pushed the parley-core-shapedtext branch from f94e80e to 50b07ac Compare July 21, 2026 10:01

tomcur added 3 commits July 21, 2026 12:10

Drop unused ShapedText::features for now

3691178

Clear runs

9b07375

Actually get cluster index from clusters range

cf7ead6

tomcur mentioned this pull request Jul 21, 2026

Test reusing a parley::Layout #701

Merged

Update shape_item docs

d38f258

tomcur added this pull request to the merge queue Jul 21, 2026

Merged via the queue into linebender:main with commit 91388dc Jul 21, 2026
24 checks passed

tomcur deleted the parley-core-shapedtext branch July 21, 2026 16:45

tomcur mentioned this pull request Jul 21, 2026

Pre-allocate some ShapedText storage #702

Merged

taj-p pushed a commit to taj-p/parley that referenced this pull request Jul 22, 2026

Test reusing a parley::Layout (linebender#701)

0ce4f19

To catch issues like linebender#679 (comment).

tomcur mentioned this pull request Jul 22, 2026

Derive ShapedRun::advance from clusters #706

Merged

		let cluster = run.range.char_range.end;
		let text = run.range.byte_range.end;

		let normalized_coords =
		&Vec::from_iter(run.normalized_coords().iter().map(\|c\| c.to_bits()));

		let clusters =
		&mut layout.shaped_text.clusters_mut()[line_item.cluster_range.clone()];

Uh oh!

Conversation

tomcur commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoburns commented Jul 9, 2026

Uh oh!

tomcur commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taj-p left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomcur Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tomcur commented Jul 9, 2026 •

edited

Loading

tomcur Jul 21, 2026 •

edited

Loading