Migrate analysis from `parley` to `parley_core` by tomcur · Pull Request #649 · linebender/parley

tomcur · 2026-06-23T16:08:10Z

On top of #648.

This introduces Analyzer and Analysis as scratch, moves over analyze_text from Parley into Parley Core, as well as some types like CharInfo, Boundary. Parley now calls into Parley Core for analysis.

Analysis is unchanged, except Parley Core is given the word break config spans by Parley, where Parley previously reached into the style runs during analysis.

CharInfo has different fields in #634 (the big PR where all of shaping was migrated to parley_core), and we may move to something like that later. For now, this is probably fine.

Benches neutral as expected

$ cargo export target/benchmarks -- bench --bench=main
$ cargo bench -q --bench=main -- compare ../target/benchmarks/main -t 8.
Default Style - arabic 20 characters               [   8.7 us ...   8.7 us ]      -0.26%
Default Style - latin 20 characters                [   4.1 us ...   4.2 us ]      +1.69%*
Default Style - japanese 20 characters             [   8.1 us ...   8.2 us ]      +0.49%
Default Style - arabic 1 paragraph                 [  47.0 us ...  46.7 us ]      -0.66%
Default Style - latin 1 paragraph                  [  16.0 us ...  16.5 us ]      +2.78%*
Default Style - japanese 1 paragraph               [  69.1 us ...  68.7 us ]      -0.55%
Default Style - arabic 4 paragraph                 [ 203.1 us ... 196.7 us ]      -3.13%*
Default Style - latin 4 paragraph                  [  61.5 us ...  61.7 us ]      +0.39%
Default Style - japanese 4 paragraph               [  98.1 us ...  97.0 us ]      -1.14%*
Styled - arabic 20 characters                      [   9.7 us ...   9.7 us ]      +0.04%
Styled - latin 20 characters                       [   5.3 us ...   5.3 us ]      +1.55%*
Styled - japanese 20 characters                    [   8.6 us ...   8.6 us ]      -0.46%
Styled - arabic 1 paragraph                        [  49.3 us ...  49.0 us ]      -0.76%
Styled - latin 1 paragraph                         [  20.4 us ...  20.6 us ]      +0.93%
Styled - japanese 1 paragraph                      [  75.0 us ...  74.4 us ]      -0.90%
Styled - arabic 4 paragraph                        [ 220.1 us ... 215.6 us ]      -2.02%*
Styled - latin 4 paragraph                         [  79.5 us ...  80.0 us ]      +0.59%
Styled - japanese 4 paragraph                      [ 106.2 us ... 105.3 us ]      -0.85%

tomcur · 2026-06-24T05:53:03Z

+/// - Control characters
+/// - Format characters, unless they use the "Inherited" script
+#[inline(always)]
+pub(crate) fn contributes_to_shaping(general_category: GeneralCategory, script: Script) -> bool {


Note this function is copied over from the old parley/src/analysis/mod.rs. That original file is moved to parley_core/src/analysis.rs (with changes). The remaining parley/src/analysis/cluster.rs should also end up moving, dropping the analysis dir entirely.

Will we remove this duplicate function soon then? Should we leave a TODO?

Yep, should happen when font selection is moved to parley_core. Added a todo!

This introduces `Analyzer` and `Analysis` as scratch, moves over `analyze_text` from Parley into Parley Core, as well some types like `CharInfo`, `Boundary`. Parley now calls into Parley Core for analysis. Analysis is unchanged, except Parley Core is given word break spans by Parley, where Parley previously reached into the style runs during analysis. Quite a bit is `pub` so Parley can reach into Core's fields. `CharInfo` has different fields in linebender#634 (the big PR where all of shaping was migrated to `parley_core`), and we may move to something like that later. For now, this should be fine. Benches neutral as expected: <details> <summary>Benchmark results</summary> ``` $ cargo export target/benchmarks -- bench --bench=main $ cargo bench -q --bench=main -- compare ../target/benchmarks/main -t 8. Default Style - arabic 20 characters [ 8.7 us ... 8.7 us ] -0.26% Default Style - latin 20 characters [ 4.1 us ... 4.2 us ] +1.69%* Default Style - japanese 20 characters [ 8.1 us ... 8.2 us ] +0.49% Default Style - arabic 1 paragraph [ 47.0 us ... 46.7 us ] -0.66% Default Style - latin 1 paragraph [ 16.0 us ... 16.5 us ] +2.78%* Default Style - japanese 1 paragraph [ 69.1 us ... 68.7 us ] -0.55% Default Style - arabic 4 paragraph [ 203.1 us ... 196.7 us ] -3.13%* Default Style - latin 4 paragraph [ 61.5 us ... 61.7 us ] +0.39% Default Style - japanese 4 paragraph [ 98.1 us ... 97.0 us ] -1.14%* Styled - arabic 20 characters [ 9.7 us ... 9.7 us ] +0.04% Styled - latin 20 characters [ 5.3 us ... 5.3 us ] +1.55%* Styled - japanese 20 characters [ 8.6 us ... 8.6 us ] -0.46% Styled - arabic 1 paragraph [ 49.3 us ... 49.0 us ] -0.76% Styled - latin 1 paragraph [ 20.4 us ... 20.6 us ] +0.93% Styled - japanese 1 paragraph [ 75.0 us ... 74.4 us ] -0.90% Styled - arabic 4 paragraph [ 220.1 us ... 215.6 us ] -2.02%* Styled - latin 4 paragraph [ 79.5 us ... 80.0 us ] +0.59% Styled - japanese 4 paragraph [ 106.2 us ... 105.3 us ] -0.85% ``` </details>

taj-p

Largely LGTM! Added some comments

taj-p · 2026-06-27T20:54:11Z

+#[derive(Default)]
+pub struct Analyzer {
+    pub(crate) bidi: BidiResolver,
+    pub(crate) bidi_props: Vec<(BidiClass, BidiMirroringGlyph)>,


I think bidi_props is a leak. It's pushed once per char but never cleared

Whoops, I accidentally copied that over from the big parley_core PR, where we do use it (and clear it 😅): https://github.com/linebender/parley/pull/634/changes#diff-5bf51df3f98cf8aad5e4e8931ac4daa515f616d1ad0d5530ecad7124c360eccaR542.

I've removed the field now, because in this new PR we still carry bidi info on CharInfo.

taj-p · 2026-06-27T20:56:19Z

+/// - Control characters
+/// - Format characters, unless they use the "Inherited" script
+#[inline(always)]
+pub(crate) fn contributes_to_shaping(general_category: GeneralCategory, script: Script) -> bool {


Will we remove this duplicate function soon then? Should we leave a TODO?

taj-p · 2026-06-27T21:04:10Z

            ),
            None,
        );
+        // TODO: perhaps core::mem::swap?


Yea - I think this makes sense. Either by making bidi.levels pub(crate) or adding a helper:

pub fn swap_levels(&mut self, dst: &mut Vec<BidiLevel>) { core::mem::swap(&mut self.levels, dst); }

At some point, we might want to pass levels into bidi.resolve to circumvent this entirely

I've made it pub(crate) for now. I agree with passing levels into bidi.resolve.

taj-p · 2026-06-27T21:11:11Z

+    lcx.info
+        .extend(lcx.analysis.char_info().iter().map(|&ci| (ci, 0)));


We can avoid this additional work by storing the CharInfo and style index in parallel vecs rather than copying back to a tuple vec

That makes sense to me. It'll be a bit of a bigger diff, as the existing LayoutContext::info is read in many places, but happy to do it as part of this PR (tomorrow)!

parley/parley/src/context.rs

Line 36 in 9888508

pub(crate) info: Vec<(CharInfo, u16)>,

Whether it happens in this PR or as a separate PR, I don't mind - will leave this with you 🙏

Done :)

Seems to be a slight win

$ cargo bench -q --bench=main -- compare ../target/benchmarks/main -t 8. Default Style - arabic 20 characters [ 8.8 us ... 8.7 us ] -1.33%* Default Style - latin 20 characters [ 4.2 us ... 4.1 us ] -1.38%* Default Style - japanese 20 characters [ 8.0 us ... 8.0 us ] -0.63% Default Style - arabic 1 paragraph [ 47.5 us ... 47.1 us ] -0.82% Default Style - latin 1 paragraph [ 16.1 us ... 16.2 us ] +0.28% Default Style - japanese 1 paragraph [ 68.6 us ... 68.6 us ] -0.06% Default Style - arabic 4 paragraph [ 204.1 us ... 201.3 us ] -1.41%* Default Style - latin 4 paragraph [ 61.2 us ... 60.9 us ] -0.51% Default Style - japanese 4 paragraph [ 97.4 us ... 97.3 us ] -0.12% Styled - arabic 20 characters [ 9.8 us ... 9.7 us ] -1.23%* Styled - latin 20 characters [ 5.3 us ... 5.3 us ] -1.51%* Styled - japanese 20 characters [ 8.6 us ... 8.7 us ] +0.76% Styled - arabic 1 paragraph [ 50.1 us ... 50.0 us ] -0.23% Styled - latin 1 paragraph [ 20.7 us ... 20.5 us ] -1.06%* Styled - japanese 1 paragraph [ 74.6 us ... 75.1 us ] +0.73% Styled - arabic 4 paragraph [ 224.5 us ... 222.6 us ] -0.84% Styled - latin 4 paragraph [ 80.2 us ... 80.0 us ] -0.27% Styled - japanese 4 paragraph [ 106.0 us ... 106.1 us ] +0.17%

taj-p · 2026-06-27T21:38:31Z

-    let contiguous_word_break_substrings =
-        WordBreakSegmentIter::new(text, rest_runs.iter(), lcx, first_style_run);
    let mut global_offset = 0;
    let mut line_boundary_positions: Vec<usize> = Vec::new();


We should move this scratch space to Analyzer at some point 😅

DJMcNab · 2026-06-29T00:28:20Z

+
+    /// Bidi level for each character, parallel to `info`.
+    ///
+    /// Empty if the text is all LTR.


Is the theory that even if we do have RTL, you're likely to have some LTR numerals in there, so need bidi awareness?

Are you asking whether the Vec could also be empty in case the text is all RTL? I suppose we could do that; parley currently only optimizes the all-LTR case and this PR doesn't touch that:

parley/parley/src/shape/mod.rs

Line 126 in 9888508

let level = levels.get(char_index).copied().unwrap_or(0);

I suppose in principle that could fall back to the paragraph level (which can be RTL) instead of to 0.

taj-p

LGTM 🎉

taj-p · 2026-06-30T07:23:42Z

+    lcx.info
+        .extend(lcx.analysis.char_info().iter().map(|&ci| (ci, 0)));


Whether it happens in this PR or as a separate PR, I don't mind - will leave this with you 🙏

taj-p · 2026-06-30T07:57:10Z

+        }
+    }
+
+    impl Iterator for DenseWordBreaks<'_> {


I considered whether we could just pass an iterator of lcx.word_break to avoid a collect, but since we iterate over the entire range, I think it's probably better to "collect all at once" and pass it in (as is currently proposed).

I considered this too, it could work. It's mainly trading off simplicity and a collect versus the code either needing to be monomorphized or getting vtable indirection (and as we probably expect the collections to remain quite sparse, it's not necessarily a win).

taj-p · 2026-06-30T08:01:36Z

    let mut global_offset = 0;
    let mut line_boundary_positions: Vec<usize> = Vec::new();
+
+    let data_sources = AnalysisDataSources::new();


Thinking ahead: There might be a breaking API change in the future to pass this in because consumers may need to mutate AnalysisDataSources to pass in data at runtime. We can probably do that later - as needed - because I don't think stabilising the API ASAP is a high priority?

We can probably do that later - as needed - because I don't think stabilising the API ASAP is a high priority?

That matches my understanding!

tomcur force-pushed the parley-core-analysis branch from 9fc220c to 40fb45e Compare June 23, 2026 16:13

tomcur commented Jun 24, 2026

View reviewed changes

tomcur force-pushed the parley-core-analysis branch from 40fb45e to f9ed6e6 Compare June 26, 2026 14:41

tomcur force-pushed the parley-core-analysis branch from f9ed6e6 to 4900bf6 Compare June 27, 2026 12:56

taj-p reviewed Jun 27, 2026

View reviewed changes

DJMcNab reviewed Jun 29, 2026

View reviewed changes

tomcur added 2 commits June 29, 2026 15:58

Clean up

85066cc

Forward Cargo features

404d8c0

tomcur force-pushed the parley-core-analysis branch from be95975 to 404d8c0 Compare June 29, 2026 14:18

tomcur added 4 commits June 29, 2026 16:25

Inline methods

ad5f122

Leave note about duplicated contributes_to_shaping

7a1fc29

Bidi levels ping-pong

f993c6e

Filter

2fa1481

taj-p approved these changes Jun 30, 2026

View reviewed changes

Make char style indices parallel to char info

3ca5017

tomcur added this pull request to the merge queue Jun 30, 2026

Merged via the queue into linebender:main with commit cb0f047 Jun 30, 2026
24 checks passed

tomcur deleted the parley-core-analysis branch June 30, 2026 13:47

tomcur mentioned this pull request Jul 2, 2026

Add parley_core itemizer, refactor parley to use it #670

Merged

		lcx.info
		.extend(lcx.analysis.char_info().iter().map(\|&ci\| (ci, 0)));

Uh oh!

Conversation

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

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

tomcur Jun 29, 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!

Uh oh!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomcur commented Jun 23, 2026 •

edited

Loading

tomcur Jun 29, 2026 •

edited

Loading