Implement parley_core (analysis, shaping, reshaping breaks, vertical text, inline objects)

[tomcur]

The parley_core proposed here provides primitives for shaping text, including reshaping across breaks and undoing breaks. In this form, everything relevant to shaping should be in parley_core (including features like vertical writing modes), but it does not provide layout (stacking of lines, wrapping around floated boxes, alignment, etc.).

Having correct shaping and efficient reshaping across breaks probably allows most layout engines to be built on top, and as such that may be the correct seam for parley_core (and experimenting with it is what led to this PR). Whether that conceptually really is the correct seam for parley_core is an important question!

Overview

The main new functionality here is reshaping, along with vertical writing modes and improved handling of inline objects. (This PR probably needs to be split up into multiple.)

The current API looks like the following (and I'm not too attached to the exact form!).

/// Reusable scratch for [`Analyzer::analyze`].
struct Analyzer { ... }; // maybe call this `AnalysisContext`, for symmetry with `ShapeContext`

impl Analyzer {
    /// Analyze `text`, writing the result into `analysis`.
    ///
    /// This overwrites `analysis`, reusing its allocations.
    pub fn analyze(&mut self, text: &str, options: &AnalysisOptions<'_>, analysis: &mut Analysis) { ... }
}

/// The result of [`Analyzer::analyze`].
pub struct Analysis { ... }

pub struct ItemizeOptions<'a> {
    ...
    /// The paragraph's writing mode.
    pub writing_mode: WritingMode, // Horizontal or vertical. If vertical, includes the orientation.
}

impl Analysis {
    /// Returns per-character info, in source order.
    pub fn char_infos(&self) -> &[CharInfo] { ... }
    /// Returns resolved bidi levels, parallel to [`Self::char_infos`].
    pub fn bidi_levels(&self) -> &[u8] { ... }
    /// Returns the paragraph's resolved base bidi level.
    pub fn paragraph_level(&self) -> u8 { ... }

    /// Itemizes the analyzed text with an additional break predicate.
    ///
    /// The yielded [`Item`]s are maximally contiguous runs of the text in logical order of
    /// constant bidi level, script, language, and [orientation](RunOrientation)), additionally
    /// splitting any run when the `split_before` predicate returns `true`.
    ///
    /// A `U+FFFC` OBJECT REPLACEMENT CHARACTER also breaks the run on both sides and forms a
    /// one-character [`ItemKind::InlineBox`] item. Every other item is [`ItemKind::Text`].
    ///
    /// This breask runs on the same conditions as [`Self::items`] plus wherever `split_before`
    /// returns `true`. Use it to split on conditions like style changes that affect shaping.
    ///
    /// `text` must match what was passed to [`Analyzer::analyze`](crate::Analyzer::analyze).
    pub fn itemize_with<'a, F>(
        &'a self,
        text: &'a str,
        options: &ItemizeOptions<'a>,
        split_before: F,
    ) -> Itemizer<'a, F>
    where
        F: FnMut(usize, usize) -> bool,
    { ... }
}


/// Reusable shaping resources for [`ShapeContext::shape_run`].
pub struct ShapeContext { ... }

/// A run ready to shape.
#[derive(Clone, Debug)]
pub struct ShapeInput<'a> {
    /// The full paragraph text.
    pub text: &'a str,
    /// The analysis of the full paragraph text.
    pub analysis: &'a Analysis,
    /// The byte range within `text`/`analysis` to shape.
    pub text_range: Range<usize>,
    /// The char range corresponding to `text_range`, i.e., indices into [`Analysis::char_infos`],
    /// counted in `char`s.
    pub char_range: Range<usize>,
    /// The run's script.
    pub script: Script,
    /// The run's language, if known.
    pub language: Option<Language>,
    /// The bidi embedding level (its parity gives the inline direction).
    pub level: u8,
    /// The run's resolved orientation, from the paragraph's [`WritingMode`](crate::WritingMode).
    pub orientation: RunOrientation,
    /// The requested font attributes (weight, width, style).
    pub attributes: Attributes,
    /// Font size in pixels per em.
    pub font_size: f32,
    /// OpenType features to apply.
    pub features: &'a [FontFeature],
    /// Variation axis settings to apply.
    pub variations: &'a [FontVariation],
    /// Extra spacing added to each cluster advance.
    pub letter_spacing: f32,
    /// Extra spacing added to each space cluster.
    pub word_spacing: f32,
}

impl ShapeContext {
    /// Shapes one run, appending the result to `out`.
    ///
    /// May append more than one [`Run`](crate::Run), because fonts may not be able to map a
    /// character, in which case the run gets split and shaped against various fonts in `query`.
    pub fn shape_run(
        &mut self,
        input: &ShapeInput<'_>,
        query: &mut Query<'_>,
        out: &mut ShapedText,
    ) { ... }

    /// Commits a line break at byte offset `pos`, reshaping the bounded region on each side so
    /// cursive joins are severed.
    ///
    /// Undo this with [`Self::apply_concat`].
    ///
    /// Reshapes each side as necessary. It is a no-op when `pos` is a break-safe boundary.
    /// If the break severs a cursive join or splits a ligature (e.g. a hyphenation falling
    /// inside a Latin `fi` ligature), both sides are reshaped.
    pub fn apply_break(
        &mut self,
        text: &str,
        analysis: &Analysis,
        shaped: &mut ShapedText,
        pos: usize,
        query: &mut Query<'_>,
    ) { ... }

    /// Merges two adjacent shaped fragments meeting at byte offset `pos`, reshaping the bounded
    /// region as one fragment so that cursive joins and ligatures are formed across the seam.
    ///
    /// This is the reverse of [`Self::apply_break`].
    ///
    /// Reshapes each side as necessary. It is a no-op when the boundary at `pos` is already
    /// safe-to-concat.
    pub fn apply_concat(
        &mut self,
        text: &str,
        analysis: &Analysis,
        shaped: &mut ShapedText,
        pos: usize,
        query: &mut Query<'_>,
    ) { ... }
}

/// The shaped result of one paragraph.
#[derive(Clone, Debug, Default)]
pub struct ShapedText { ... }

impl ShapedText {
    /// Returns the run at `index` in logical order, if any.
    pub fn run(&self, index: usize) -> Option<Run<'_>> { ... }

    /// Iterates the runs in logical (source) order.
    pub fn runs(&self) -> impl Iterator<Item = Run<'_>> + '_ { ... }
}

The pipeline in this PR is such that later stages can be redone without redoing earlier stages, though I'm also not too attached at exactly where those seams are. First you use the Analyzer to create an Analysis. This does bidi resolution, finds segmentation boundaries, and records the per-character Unicode properties.

Next, you itemize using Analysis::itemize_with into individually shapeable runs of constant bidi, script, language, orientation. Callers like parley that style text provide their own additional predicate for splitting (e.g., if the font size changes).

These itemized runs are shaped using a ShapeContext and appended to a ShapedText. The shaped runs can be rendered.

If you want to apply a break, you use ShapeContext::apply_break. This reshapes around the break, severing cursive joins/ligatures. Undo a break using ShapeContext::apply_concat. Fancy line breaking algorithms (something like Knuth-Plass) can apply the break, remeasure the runs after reshaping, and backtrack if needed.

ShapedText allows getting/iterating the shaped runs that have the information needed by the renderer (e.g., glyphs, advances, whether the glyph needs to be rotated for vertical text, etc.).

This operates on a &str, and the unit of correct analysis and shaping is effectively an entire paragraph. If a paragraph is huge, the allocations required are correspondingly huge. I'm not sure whether this really matters in practice (the user can decide to cut into parts and forego strictly correct analysis/shaping). It might be possible to expose something like the "pre- and post-context" like what harfrust has, which I believe would at least give correct contextual substitutions across seams, but not ligatures (and also not correct bidi resolving).

Some notes

This PR implements vertical writing modes in parley_core, including text-orientation.

Inline objects are supported first-class using the u+fffc object replacement character, which gets correct analysis, bidi, etc. around the object (parley still uses its out-of-band encoding (for now?)).

parley is refactored to be on top of parley_core for its analysis and shaping. It doesn't use the reshaping in this PR yet.

parley_core does not do things like text transforms, whitespace collapsing, etc., the idea being that for clean separation of these kinds of concerns, the layer above, like parley, has a String that it builds the transformed text into (including control characters like u+fffc for inline objects and bidi overrides).

The diff is rather large, and there is new functionality here (and a bit of experimentation to get here), but I've attempted to port some of the core things over from parley without too many gratuitous changes. That also means this is punting on some clean up, but I think the core that's been brought over is mostly in an OK state.

I have some follow-ups to this if we land this, like better emoji presentation classification, which correctly chooses between color emoji or pictographs.

This benches on my machines as follows.

Desktop

$ cargo bench -q --bench=main -- compare target/benchmarks/main -p -t 4.
Default Style - arabic 20 characters               [   8.9 us ...   8.6 us ]      -3.06%*
Default Style - latin 20 characters                [   4.3 us ...   4.1 us ]      -4.20%*
Default Style - japanese 20 characters             [   8.1 us ...   8.3 us ]      +1.97%*
Default Style - arabic 1 paragraph                 [  48.1 us ...  45.2 us ]      -5.91%*
Default Style - latin 1 paragraph                  [  17.2 us ...  14.9 us ]     -13.22%*
Default Style - japanese 1 paragraph               [  70.0 us ...  69.6 us ]      -0.62%
Default Style - arabic 4 paragraph                 [ 213.2 us ... 198.3 us ]      -7.01%*
Default Style - latin 4 paragraph                  [  66.1 us ...  56.3 us ]     -14.78%*
Default Style - japanese 4 paragraph               [  99.5 us ...  98.1 us ]      -1.43%*
Styled - arabic 20 characters                      [   9.8 us ...  10.2 us ]      +4.03%*
Styled - latin 20 characters                       [   5.3 us ...   5.9 us ]     +10.23%*
Styled - japanese 20 characters                    [   8.7 us ...   9.1 us ]      +5.18%*
Styled - arabic 1 paragraph                        [  50.3 us ...  50.6 us ]      +0.53%
Styled - latin 1 paragraph                         [  21.4 us ...  21.9 us ]      +2.77%*
Styled - japanese 1 paragraph                      [  76.5 us ...  78.6 us ]      +2.78%*
Styled - arabic 4 paragraph                        [ 231.4 us ... 227.3 us ]      -1.75%*
Styled - latin 4 paragraph                         [  83.0 us ...  85.0 us ]      +2.45%*
Styled - japanese 4 paragraph                      [ 108.8 us ... 111.4 us ]      +2.37%*

Laptop

$ cargo bench -q --bench=main -- compare target/benchmarks/main -p -t 2.
Default Style - arabic 20 characters               [  20.3 us ...  20.8 us ]      +2.31%
Default Style - latin 20 characters                [   8.1 us ...   8.0 us ]      -1.09%
Default Style - japanese 20 characters             [  17.2 us ...  18.6 us ]      +7.87%*
Default Style - arabic 1 paragraph                 [ 103.0 us ...  99.0 us ]      -3.93%
Default Style - latin 1 paragraph                  [  33.3 us ...  29.6 us ]     -10.97%*
Default Style - japanese 1 paragraph               [ 148.1 us ... 155.9 us ]      +5.32%*
Default Style - arabic 4 paragraph                 [ 443.5 us ... 445.2 us ]      +0.38%
Default Style - latin 4 paragraph                  [ 130.0 us ... 120.1 us ]      -7.66%*
Default Style - japanese 4 paragraph               [ 216.5 us ... 231.4 us ]      +6.88%*
Styled - arabic 20 characters                      [  21.4 us ...  21.4 us ]      -0.15%
Styled - latin 20 characters                       [  11.4 us ...  11.4 us ]      +0.42%
Styled - japanese 20 characters                    [  18.4 us ...  18.7 us ]      +1.21%
Styled - arabic 1 paragraph                        [ 108.5 us ... 115.3 us ]      +6.23%*
Styled - latin 1 paragraph                         [  41.5 us ...  41.6 us ]      +0.08%
Styled - japanese 1 paragraph                      [ 155.1 us ... 166.4 us ]      +7.26%*
Styled - arabic 4 paragraph                        [ 463.5 us ... 462.3 us ]      -0.27%
Styled - latin 4 paragraph                         [ 161.7 us ... 155.1 us ]      -4.09%*
Styled - japanese 4 paragraph                      [ 236.6 us ... 238.1 us ]      +0.63%

Disclosure: done with some LLM assistance, especially porting. I've scrutinized this and iterated on it quite a bit, but if we want to land this general shape, I'll scrutinize it some more.

[tomcur]

process_clusters, push_cluster, to_whitespace and some helpers in this file are based on the old parley/src/layout/data.rs.

[tomcur]

Based on the old parley/src/shape/cache.rs.

[tomcur]

Old parley/src/layout/glyph.rs, but dropping style.

[tomcur]

Based on parley/src/layout/data.rs's RunData.

[tomcur]

Based on parley/src/layout/data.rs's Clusterdata.

[tomcur]

Iterator to itemize based on the run splitting in parley/src/shape/mod.rs.

[tomcur]

Some types (Boundary, Whitespace, etc.), ported from the old parley, some new types for vertical writing modes.

[tomcur]

Based on Run in parley/src/layout/run.rs.

[tomcur]

Loosely based on Cluster in parley/src/layout/cluster.rs.

[PoignardAzur]

The diff is rather large, and there is new functionality here (and a bit of experimentation to get here), but I've attempted to port some of the core things over from parley without too many gratuitous changes. That also means this is punting on some clean up, but I think the core that's been brought over is mostly in an OK state.

I think this could and should be broken up even further. As it is, the PR mixes logic changes (though I assume the behavior stays the same) with code structure changes with file moves. Maybe it makes a ton of sense to someone more familiar with the code, but as someone with minimal parley experience I have very little idea what's going on.

To give an example, you move parley/src/shape/cache.rs to parley_core/src/shape/cache.rs, but the files have subtle changes:

// OLD
pub(crate) struct ShapePlanKey<'a> {
    /// The font collection's blob ID.
    font_blob_id: u64,
    /// The font's index in the font collection.
    font_index: u32,
    synthesis: &'a fontique::Synthesis,
    direction: harfrust::Direction,
    script: harfrust::Script,
    language: Option<harfrust::Language>,
    features: &'a [harfrust::Feature],
    variations: Option<&'a [FontVariation]>,
}

// NEW
pub(super) struct ShapePlanKey<'a> {
    /// The font collection's blob ID.
    font_blob_id: u64,
    /// The font's index in the font collection.
    font_index: u32,
    coords: &'a [NormalizedCoord],
    direction: Direction,
    script: Script,
    language: Option<&'a Language>,
    features: &'a [Feature],
}

The synthesis and direction fields are gone and a coords field is added (and also the visibility goes from pub(crate) to pub(super), which I don't like on principle).

I don't know how big a change that is, but my point is I only noticed the change because I visually compared the old and new file for changes. The Github UI didn't show a diff, because it treats them as two different files.

For a refactor like this, what you want to do is first implement all the logic changes and visibility changes that you will need once items are refactored away, and then do a separate PR to move the items.

[PoignardAzur]

(I'm willing to do work on splitting up the PR, if it helps you.)

[nicoburns]

@tomcur Oh boy, another big (even bigger) diff. I've skimmed it, and can see that some of that is just moving files about in ways that seem quite reasonable, but I'm going to ask up front if this could be split into smaller chunks that we can review and land separately. In particular this seems to:

• Be a relatively significant rewrite/refactor of the core analysis pipeline
• Move the core analysis pipeline into it's own crate (parley_core)

And it seems to me that there is no reason why those two things need to be in the same PR.

---

EDIT: it seems that Olivier has the same feedback:

For a refactor like this, what you want to do is first implement all the logic changes and visibility changes that you will need once items are refactored away, and then do a separate PR to move the items.

I am in agreement with this, although you could also potentially do it the other way around: move stuff around first, and then do the logic changes (and I might actually prefer this as I think the "moving the code around" changes will be less controversial and easier to review / quicker to land). Either way, mixing actual logic changes with file moves makes the diff very difficult to review.

[PoignardAzur]

I am in agreement with this, although you could also potentially do it the other way around: move stuff around first, and then do the logic changes

The reason I suggest doing logic changes first is that sometimes logic changes (and changes in visibility, module structure, etc) are necessary before the code can be split into a separate crate.

[waywardmonkeys]

This replaces a PR that I was yet to file (in part because I was waiting to see the shape of this one). That PR would have split up the layout data into shaped and line breaking data. I'd rather rebuild what I'm doing on top of this instead.

[tomcur]

Thank you! I agree with both of you, as I also find the current state hard to review. I spent last evening and late last night (early morning...) minimizing diffs and checking things were correctly ported, and then decided to just push what I had to start the discussion. Your suggestions make sense, but not sure yet which way around is going to be most straightforward.

Before doing the work to make this more reviewable (again, very happy to do this), I'd love discussion on the general shape of the API; i.e., whether the "seam" between parley_core and parley is correct, and whether parley_core exposes the right primitives to be more generally useful for non-parley callers doing text layout.

(I'm willing to do work on splitting up the PR, if it helps you.)

Thank you for offering help splitting this up, @PoignardAzur! If we decide we want this shape we can discuss, though by default it may be most efficient if I do it.

The reason I suggest doing logic changes first is that sometimes logic changes (and changes in visibility, module structure, etc) are necessary before the code can be split into a separate crate.

This makes sense to me, and is one of the things I was struggling with, as I did move stuff around first.

(As an aside, the LLM use here was a mixed experience: it was genuinely useful to learn about some of the new concepts, esp. vertical metrics in fonts and such, it also helped experimenting, but it probably slowed down the port itself by quite a bit as I've been spending a lot of time cleaning up things like gratuitous renames and sloppily ported algorithms; had I done it manually this may have been in a better state, and been there more quickly.)

[PoignardAzur]

Thank you! I agree with both of you, as I also find the current state hard to review. I spent last evening and late last night (early morning...) minimizing diffs and checking things were correctly ported, and then decided to just push what I had to start the discussion. Your suggestions make sense, but not sure yet which way around is going to be most straightforward.

Fair enough, but then I'd suggest marking this PR as a draft.

Thank you for offering help splitting this up, @PoignardAzur! If we decide we want this shape we can discuss, though by default it may be most efficient if I do it.

If you're willing to redo some work from the ground up, I might do a zero-logic-changes refactoring PR of my own which you would work on top of. The changes I'm thinking of are stuff like splitting types across files, normalizing imports, moving stuff into a core/ module which would roughly match what you'd put into parley_core, etc.

I think it would be a better base for a refactor, but also you'd never be able to rebase this PR on top of mine, hence why I say you'd redo work from the ground up.

[PoignardAzur]

(As an aside, the LLM use here was a mixed experience: it was genuinely useful to learn about some of the new concepts, esp. vertical metrics in fonts and such, it also helped experimenting, but it probably slowed down the port itself by quite a bit as I've been spending a lot of time cleaning up things like gratuitous renames and sloppily ported algorithms; had I done it manually this may have been in a better state, and been there more quickly.)

In my experience, current agents work best when given a very tight scope, e.g. "Refactor this crate and move functions around but don't touch any logic" or "Change the algorithm in this function without touching any code outside of the file".

That approach gives you more prep work and follow-up work, but it has a much lower risk of going completely off-rails.

[taj-p]

Before doing the work to make this more reviewable (again, very happy to do this), I'd love discussion on the general shape of the API; i.e., whether the "seam" between parley_core and parley is correct, and whether parley_core exposes the right primitives to be more generally useful for non-parley callers doing text layout.

I've skimmed the API and implementation - I like it. I can review it when I'm back from holidays next week if that helps - perhaps we can have a couple reviewers?

It might be worth trying to port the proposed line breaking solution to parley (even if it's just an LLM draft) so that we can better understand how it might be integrated (and surface any points of friction).

One question that I am wondering: Do we want to force consumers to use fontique? If the intent is to enable parley_core to be consumed outside of the Linebender stack, perhaps we might want to break that dependency with a trait so external consumers can use their own font enumeration and fallback stack (and, also, ease a migration towards parley_core).

I'm not attached to the idea and it's certainly something we can do after the initial implementation. It might be worth hearing other's opinions on the matter.

[tomcur]

I can review it when I'm back from holidays next week if that helps

Neat! Enjoy your holidays. I'm starting a cycling/camping trip tonight until mid of next week. I'll be able to see messages every now and then. If it rains enough late next week I'll do some implementation work (we're staying at family), and will be back home June 18th.

It might be worth trying to port the proposed line breaking solution to parley (even if it's just an LLM draft) so that we can better understand how it might be integrated (and surface any points of friction).

Do you mean having parley call the reshaping when it breaks lines for layout (such that it shapes correctly and its behavior can be reviewed)?

(I played around a bit with splitting up the PR after Olivier's and Nico's suggestions, also implementing some of the logic inside parley before splitting it off. That'll probably help review.)

If the intent is to enable parley_core to be consumed outside of the Linebender stack, perhaps we might want to break that dependency with a trait so external consumers can use their own font enumeration and fallback stack

This sounds like a good idea to me.

@PoignardAzur

If you're willing to redo some work from the ground up, I might do a zero-logic-changes refactoring PR of my own which you would work on top of.

Feel free to do refactorings! This current PR should not block other work. If this monolithic version is used to show this shape works and we're wanting to work towards it, I'm happy to redo work. It'll be nice to have a cleaner starting state then. I'm also happy to split stuff up when I'm back at work (perhaps less churn that way, but can't get to it for one or two weeks!)

[taj-p]

This is very good! Kudos! My read is that it's the right shape barring the fontique dependency.

I've done a first preliminary pass. I am comfortable continuing with large reviews, but I realize that not everyone is happy to review a large PR. As I asked for another reviewer, I think I agree we should split it up (and my bad to suggest that).

I think I would suggest the following PR train:

1. parley_data: Simply add in the vertical orientation property. This has no cost to consumers. - should be a 5 min review.
2. Migrate analysis (just keep the internal fields public for now with a TODO to make them pub(crate))
   • Create parley_core/.../analyzer.rs
   • Migrate parley to use it
3. Migrate itemization
   • Create parley_core/.../itemize.rs
   • I think we might need an adapter to feed the current shaper with item items
   • Keep out vertical shaping
4. Migrate shaping

• Copy in parley_core/.../shaping/*
• Avoid the reshaping after breaking methods (i.e., don't include apply_break, apply_concat, etc)

5. Add in vertical writing modes to parley core
6. Add in reshaping after break functionality
7. (optional) Documentation if it makes sense to one shot rather than incrementally add to lib.rs / README.md.

My hope with the above proposal is that each PR can migrate one unit of Parley atomically while keeping LOC change relatively small that feasibly it only requires one reviewer - given the mitigated risk.

---

I think that this huge PR had to happen anyway. To be able to "see into the future" of a large change hugely reduces the risk of us incrementally building towards a bad destination.

---

An ask for this work is to copy paste old code to new locations rather than relying on AI to do the copy through memory (or prompt the AI to use copy paste semantics via CLI or similar).

[taj-p]

We've lost a lot of behaviour here from the original code. In particular, variation selector exclusion:

parley/parley/src/shape/mod.rs

Lines 243 to 271 in 5f75f53

	for ((_, ch), (info, style_index)) in segment_text.char_indices().zip(item_infos_iter.by_ref())
	{
	force_normalize |= info.force_normalize();
	// TODO - make emoji detection more complete, as per (except using composite Trie tables as
	// much as possible:
	// https://github.com/conor-93/parley/blob/4637d826732a1a82bbb3c904c7f47a16a21cceec/parley/src/shape/mod.rs#L221-L269
	is_emoji_or_pictograph |= info.is_emoji_or_pictograph();
	*code_unit_offset_in_string += ch.len_utf8();
	// TODO: Explore ignoring other modifiers in determining `contributes_to_shaping`:
	// regional indicators, subdivision flag tag sequences, skin tone modifiers
	// See also: https://github.com/google/emoji-segmenter
	// If the color emoji has a non-printing variation selector, ignore the variation selector.
	// Its presentation depends on the platform and font.
	//
	// e.g.
	// - `U+270C + U+FE0F`: `✌`, force basic presentation
	// - `U+270C + U+FE0F`: `✌️`, force emoji presentation
	//
	// <https://www.unicode.org/reports/tr37/>
	let is_emoji_with_non_printing_variation_selector =
	is_emoji_or_pictograph && info.is_variation_selector();
	let contributes_to_shaping =
	info.contributes_to_shaping() && !is_emoji_with_non_printing_variation_selector;
	if contributes_to_shaping {
	map_len += 1;
	}

[taj-p]

Have we lost emoji fallback chaining too? Is that on purpose? If it's intentionally supposed to not be in Parley Core, then we should pass it from Parley (assuming we're pushing responsibility of Query to caller via prior comments about fontique dependency)

parley/parley/src/shape/mod.rs

Lines 588 to 592 in 5f75f53

	if is_emoji {
	use core::iter::once;
	let emoji_family = QueryFamily::Generic(fontique::GenericFamily::Emoji);
	self.query.set_families(fonts.chain(once(emoji_family)));
	self.fonts_id = None;

[taj-p]

This iterates every font in the query. We previously stopped on finding an appropriate candidate. I think this is probably better, but it's worthwhile double checking with a benchmark (against consumers with long font chains where most of the time the first font is chosen).

[taj-p]

Why not re-use this scratch space by putting it in ShapeContext?

It's possible to re-use vectors that contain items with lifetimes via something like:

/// Used to reinterpret the lifetimes of a vector.
// For how this works, see:
// https://davidlattimore.github.io/posts/2025/09/02/rustforge-wild-performance-tricks.html
fn reuse_vec<T, U>(mut v: Vec<T>) -> Vec<U> {
    const {
        assert!(size_of::<T>() == size_of::<U>());
        assert!(align_of::<T>() == align_of::<U>());
    }
    v.clear();
    v.into_iter().map(|_x| unreachable!()).collect()
}

[taj-p]

This could be called many times. I wonder whether it would be better for Analysis to provide a byte -> char index vector mapping

[taj-p]

This means that Mixed becomes Sideways, but I think Mixed is supposed to be locale/context dependent rather than forced one way.

I think we either pass that context here so that we're correct or document the gap in a comment

[taj-p]

This is causing extra work to occur in Parley. Previously, as part of the loop, we were pushing glyphs and clusters into their final destinations but now we push that same data to scratch space and then copied into ShapedText and then re-iterated on the parley side to copy into the final destination. Thus, we pay for intermediate copies and an additional loop.

This kind of made me skeptical of the benchmark numbers showing neutral performance change. On my machine, I get the below for styled text.

---

My feeling is that this might be warranted because it's impacting styled text more than default (which have more runs to push and passes through harfrust) because previously the glyphs/cluster data flow was:

1. Collect glyph infos in HarfRust buffer
2. Iterate harfrust buffer, push each glyph / cluster into final destination

Whereas the proposal does:

1. Collect glyph infos in HarfRust buffer
2. Iterate harfrust buffer, push each glyph / cluster into ShapeContext::[glyphs|clusters]
3. In ShapedText::append_run, copy internal glyph / cluster buffers to ShapeContext::[glyphs|clusters]
4. In push_shaped_run, copy ShapedText glyph / cluster buffers into LayoutData::[glyphs|clusters] (most expensive copy because it re-walks every cluster and re-pushes every glyph instead of bulk copying).

---

I think there are some simple ways to avoid this:

• At step 2 / 3, push directly into ShapedText buffers from harfrust instead of requiring the additional scratch space
• Step 4 can be mitigated by parley using parley_core types directly and not re-interpreting glyphs / clusters into new structs (and store any relevant "parley" only data in parallel data types). Then we either do a bulk copy or "take" the allocations from ShapedText

[tomcur]

This suggestion actually makes some of the code nicer, too (esp. using parley_core's Glyph in parley, see d222b7c). I have some work locally that does the same for ClusterData and parley can then extend_from_slice the cluster and glyph slices. That seems to improve the benchmark delta by a few percentage points, but...

... the benchmarking itself turned out to be a bit of a rabbit hole. I kept getting unexpected results, and at some point decided to run a paired benchmark without code changes; on current main, if I run cargo export target/benchmarks -- bench --bench=main followed by cargo bench -q --bench=main -- compare target/benchmarks/main -p -t 4., I get the following!

Default Style - arabic 20 characters               [   8.9 us ...   8.9 us ]      +0.35%
Default Style - latin 20 characters                [   4.3 us ...   4.1 us ]      -3.50%*
Default Style - japanese 20 characters             [   8.2 us ...   8.1 us ]      -0.44%
Default Style - arabic 1 paragraph                 [  48.5 us ...  48.1 us ]      -0.66%
Default Style - latin 1 paragraph                  [  17.2 us ...  16.1 us ]      -6.52%*
Default Style - japanese 1 paragraph               [  71.3 us ...  69.6 us ]      -2.50%*
Default Style - arabic 4 paragraph                 [ 214.5 us ... 212.7 us ]      -0.86%
Default Style - latin 4 paragraph                  [  66.0 us ...  62.0 us ]      -5.96%*
Default Style - japanese 4 paragraph               [ 100.7 us ...  98.5 us ]      -2.18%*
Styled - arabic 20 characters                      [   9.9 us ...   9.9 us ]      +0.41%
Styled - latin 20 characters                       [   5.3 us ...   5.3 us ]      -1.01%*
Styled - japanese 20 characters                    [   8.8 us ...   8.7 us ]      -0.60%*
Styled - arabic 1 paragraph                        [  51.3 us ...  51.0 us ]      -0.60%
Styled - latin 1 paragraph                         [  21.4 us ...  20.9 us ]      -2.52%*
Styled - japanese 1 paragraph                      [  78.7 us ...  77.5 us ]      -1.52%*
Styled - arabic 4 paragraph                        [ 232.7 us ... 232.1 us ]      -0.27%
Styled - latin 4 paragraph                         [  83.2 us ...  80.6 us ]      -3.18%*
Styled - japanese 4 paragraph                      [ 109.7 us ... 108.3 us ]      -1.34%*

I.e., there are quite a few significant "improvements." This is without any code changes and isn't measurement noise. A possible explanation is there's a difference between the in-process code and the dlopen'd code performance that shows up at least on my machine. I don't think it's a P- vs E-core thing or something like that, as when I pin the threads to equally-powerful cores (same features, same max freq), the results stay like this.

[tomcur]

See #645.

[nicoburns]

I think that this huge PR had to happen anyway. To be able to "see into the future" of a large change hugely reduces the risk of us incrementally building towards a bad destination.

I agree with this

My hope with the above proposal is that each PR can migrate one unit of Parley atomically while keeping LOC change relatively small that feasibly it only requires one reviewer - given the mitigated risk.

LoC is only part of the concern for me. My other concern is that this PR mixes "moving code around" with "functional changes". I personally don't have the context to review the new pipeline standalone, so my review of that code would be in comparison to the old code. And that's really difficult to do when you don't have a proper diff.

I would ideally like to see the existing pipeline migrated to a parley_core crate as the first step (with the introduction of a ShapedText struct to enable the crate boundary). Followed by the incremental migation of the pipeline as Taj proposes.

I would be able to submit PRs for mechanical work to split out the crate if that is helpful.

[tomcur]

I've continued work locally to address some of the feedback, but that'd trigger some new discussions (things like how we encode graphemes), so I think it's best to tackle those things as we do the landable sequence.

I've started off the migration based on the suggestions. ShapedText can be introduced once parley_core does the shaping, but, by moving stuff over gradually, that'll come after analysis and itemization have been migrated (as per Taj's suggestion).

Do we want to land these PRs as we go, or do we perhaps want to review-but-leave-open until we have a full stack of PRs that we can land such that the repo is immediately in a cleanly releasable state? The latter would sort of block some types of work while the bulk of the migration is happening (which will likely take at least a few days, and probably more). The alternative is to potentially have to release alpha versions of parley_core.

[nicoburns]

@tomcur What do you think of the idea of first moving things to a core module/directory within the parley crate (with the aim being to structure things so that it could be it's own crate), and then move that directory out into it's own crate once we're done?

(the idea being that this would allow us to keep parley in an always-releasable state while we do the migration)

[tomcur]

@tomcur What do you think of the idea of first moving things to a core module/directory within the parley crate (with the aim being to structure things so that it could be it's own crate), and then move that directory out into it's own crate once we're done?

(the idea being that this would allow us to keep parley in an always-releasable state while we do the migration)

Hmm, that's possible and probably better than keeping a stack of PRs open. I suppose the main downside is it's slightly easier to miss visibility issues. If we're OK with potentially releasing 0.0.x or even alpha version of parley_core that'd be slightly less work. I'm definitely OK with us making a parley/src/core first. Wonder what @taj-p thinks.

(Because of time zones and wanting to run CI, I've pushed the next two PRs that I had ready. Happy to change them if we decide to go with a core first, of course.)

[taj-p]

@tomcur What do you think of the idea of first moving things to a core module/directory within the parley crate (with the aim being to structure things so that it could be it's own crate), and then move that directory out into it's own crate once we're done?
(the idea being that this would allow us to keep parley in an always-releasable state while we do the migration)

Hmm, that's possible and probably better than keeping a stack of PRs open. I suppose the main downside is it's slightly easier to miss visibility issues. If we're OK with potentially releasing 0.0.x or even alpha version of parley_core that'd be slightly less work. I'm definitely OK with us making a parley/src/core first. Wonder what @taj-p thinks.

(Because of time zones and wanting to run CI, I've pushed the next two PRs that I had ready. Happy to change them if we decide to go with a core first, of course.)

I think the migration needs to be land-able. Keeping PRs open invites churn because they can conflict with other changes. My preference is to merge each PR as its approved and not keep the whole train open until everything's ready.

If we're OK with potentially releasing 0.0.x or even alpha version of parley_core that'd be slightly less work. I'm definitely OK with us making a parley/src/core first. Wonder what @taj-p thinks.

There are quite a few pub(crate) in the existing parley_core implementation:

The purpose of parley_core is to define a clean seam out of parley, so developing in parley could risk reviews not identifying correct API separation from parley_core and parley - increasing review friction and author churn.

---

I think we also want to encourage development in parley_core. To do all the work in a module before moving to crate suggests that we will only move to a crate once we're happy with the API. I think we need to be realistic in that we likely will introduce a breaking change or two to parley_core after the initial migration - especially when we try to incorporate concepts like reshaping after breaking / mapping graphemes to clusters.

---

As I understand versioning, this means that whenever we bump parley_core from now on, we'll have to also bump the "first-number-after-leading-zero" of parley to prevent the types of compatibility issues we saw with glifo (#vello > Semver breaking changes in glifo 0.1.1 @ 💬)? If we do that, while also publishing a parley_core 0.0.x, then we can maintain our new goal of always being releasable right?

i.e.: The next version of parley would be 0.3.0 not 0.2.1.

^^^: All this said, if we find that we can't arrive at a solution where parley remains releasable, we should adopt the module approach. I am also not attached to the above - I favour unblocking the work here.

[nicoburns]

We can technically release whenever so long as we're happy to release parley_core. I'm pretty opposed to doing 0.0.x versions of parley_core. I think we should just sync the version with parley and fontique. If we're confident that we have capacity to see these changes through and land them relatively quickly (over say the period of a month), then my suggestion would be that we cut a release now (say tomorrow) with current main + dep bumps, and then starting landing these PRs.

[tomcur]

Quick response to part of what you said before I turn in for the night.

As I understand versioning, this means that whenever we bump parley_core from now on, we'll have to also bump the "first-number-after-leading-zero" of parley to prevent the types of compatibility issues we saw with glifo (#vello > Semver breaking changes in glifo 0.1.1 @ 💬)? If we do that, while also publishing a parley_core 0.0.x, then we can maintain our new goal of always being releasable right?

I believe that's right in the likely event we expose parley_core in parley's API (as in, it's API breaking for parley to bump to a breaking parley_core). In this PR we currently do expose parley_core in parley, e.g., there's a pub use parley_core::Glyph, and one of the other PRs does pub use parley_core::break_overrides::{..}. It's probably best to get parley_core to a 0.x.x soon. Because of parlance there's some indirection already and we could try to shim the parts of parley_core that we need in parley instead of re-exporting, cutting that semver link, unless it becomes painful.

[tomcur]

We can technically release whenever so long as we're happy to release parley_core. I'm pretty opposed to doing 0.0.x versions of parley_core. I think we should just sync the version with parley and fontique. If we're confident that we have capacity to see these changes through and land them relatively quickly (over say the period of a month), then my suggestion would be that we cut a release now (say tomorrow) with current main + dep bumps, and then starting landing these PRs.

I'm in agreement with this. If it ever gets to a point where parley_core is considerably more stable than parley or vice versa, we can always choose to desync (and/or decide at some other point whether shimming the core (#634 (comment)) would be worthwhile).

[tomcur]

Daniel suggested to move the discussion to Zulip (#647 (review)). Wrote here: #parley > parley_core PR sprint? @ 💬..