Skip to content

Move char_style_indices out of the per-item ShapeOptions #741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tomcur merged 3 commits into from
Aug 10, 2026
Merged

Move char_style_indices out of the per-item ShapeOptions #741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
797c8bc
Move `char_style_indices` out of the per-item `ShapeOptions`
tomcur Aug 9, 2026
bc8f882
Take style index from `Character` in `process_shaped_clusters`
tomcur Aug 9, 2026
d48ff5f
Debug-assert length
tomcur Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion parley/src/shape/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ pub(crate) fn shape_text<'a, B: Brush>(
font_size: item_style.font_size,
features: &style_features[item_style_index as usize],
variations: rcx.variations(item_style.font_variations).unwrap_or(&[]),
char_style_indices,
},
})
})
Expand All @@ -184,6 +183,7 @@ pub(crate) fn shape_text<'a, B: Brush>(
scx.shape_text(
text,
analysis,
char_style_indices,
items,
font_selector,
&mut layout.data.shaped_text,
Expand Down
10 changes: 8 additions & 2 deletions parley_engine/src/shape/atom.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,10 +858,16 @@ mod tests {
language: None,
features: &[],
variations: &[],
char_style_indices: &char_style_indices,
},
}];
shaper.shape_text(text, &analysis, items, SingleFont(font), &mut shaped);
shaper.shape_text(
text,
&analysis,
&char_style_indices,
items,
SingleFont(font),
&mut shaped,
);
shaped
}

Expand Down
49 changes: 21 additions & 28 deletions parley_engine/src/shape/shaped_text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ impl ShapedText {
item: &Segment,
options: &ShapeOptions<'_>,
char_info: &[CharInfo],
char_style_indices: &[u16],
font: &FontInstance,
glyph_buffer: &harfrust::GlyphBuffer,
normalized_coords: &[harfrust::NormalizedCoord],
Expand Down Expand Up @@ -318,7 +319,7 @@ impl ShapedText {
for (((byte_offset, ch), info), style_index) in text[range.byte_range.clone()]
.char_indices()
.zip(&char_info[range.char_range.clone()])
.zip(&options.char_style_indices[range.char_range.clone()])
.zip(&char_style_indices[range.char_range.clone()])
{
self.characters.push(Character {
text_byte_start: (range.byte_range.start + byte_offset) as u32,
Expand All @@ -340,7 +341,6 @@ impl ShapedText {
scale_factor,
glyph_infos.iter(),
glyph_positions.iter(),
&options.char_style_indices[range.char_range.clone()],
&self.characters,
characters_start,
);
Expand All @@ -351,7 +351,6 @@ impl ShapedText {
scale_factor,
glyph_infos.iter().rev(),
glyph_positions.iter().rev(),
&options.char_style_indices[range.char_range.clone()],
&self.characters,
characters_start,
);
Expand Down Expand Up @@ -429,7 +428,6 @@ pub struct ShapedRun {
/// * `glyph_infos` - `HarfRust` glyph information in logical order (i.e., reversed for RTL runs).
/// * `glyph_positions` - `HarfRust` glyph positioning data in logical order (i.e., reversed for RTL
/// runs).
/// * `char_style_indices` - The run's slice of per-character style indices, indexed by cluster ID.
/// * `characters` must contain the shaped characters whose clusters we're now processing, starting at
/// index `characters_start`.
/// * `characters_start` - See `characters`.
Expand All @@ -439,7 +437,6 @@ fn process_shaped_clusters<'a>(
scale_factor: f32,
glyph_infos: impl Iterator<Item = &'a harfrust::GlyphInfo>,
glyph_positions: impl Iterator<Item = &'a harfrust::GlyphPosition>,
char_style_indices: &[u16],
characters: &[Character],
characters_start: usize,
) {
Expand All @@ -457,7 +454,6 @@ fn process_shaped_clusters<'a>(
fn flush(
cluster: &mut Cluster,
char_end: usize,
style_index: u16,
characters: &[Character],
shaped_clusters: &mut Vec<ShapedCluster>,
) {
Expand All @@ -479,7 +475,7 @@ fn process_shaped_clusters<'a>(

shaped_clusters.push(ShapedCluster {
chars_range: (cluster.characters_start as u32, char_end as u32),
style_index,
style_index: first_character.style_index,
flags: ShapedClusterFlags::new(glyph_len)
.with_grapheme_start(first_character.grapheme_start)
// TODO: fill with actual shaping data (`parley` currently just ignores this)
Expand All @@ -502,14 +498,7 @@ fn process_shaped_clusters<'a>(
for (glyph_info, glyph_pos) in glyph_infos.zip(glyph_positions) {
if glyph_info.cluster != cluster.id {
let char_end = characters_start + glyph_info.cluster as usize;
let style_index = char_style_indices[cluster.id as usize];
flush(
&mut cluster,
char_end,
style_index,
characters,
shaped_clusters,
);
flush(&mut cluster, char_end, characters, shaped_clusters);

cluster = Cluster {
id: glyph_info.cluster,
Expand Down Expand Up @@ -542,14 +531,7 @@ fn process_shaped_clusters<'a>(
}

// Flush the final cluster.
let style_index = char_style_indices[cluster.id as usize];
flush(
&mut cluster,
characters.len(),
style_index,
characters,
shaped_clusters,
);
flush(&mut cluster, characters.len(), characters, shaped_clusters);
}

#[cfg(test)]
Expand Down Expand Up @@ -621,10 +603,16 @@ mod tests {
language: None,
features: &[],
variations: &[],
char_style_indices: &char_style_indices,
},
}];
shaper.shape_text(text, &analysis, items, SingleFont(font), &mut shaped);
shaper.shape_text(
text,
&analysis,
&char_style_indices,
items,
SingleFont(font),
&mut shaped,
);
shaped
}

Expand Down Expand Up @@ -732,7 +720,6 @@ mod tests {
language: None,
features: &[],
variations: &[],
char_style_indices: &char_style_indices,
},
},
Item {
Expand All @@ -742,11 +729,17 @@ mod tests {
language: None,
features: &[],
variations: &[],
char_style_indices: &char_style_indices,
},
},
];
shaper.shape_text(text, &analysis, items, SingleFont(font), &mut shaped);
shaper.shape_text(
text,
&analysis,
&char_style_indices,
items,
SingleFont(font),
&mut shaped,
);

let grapheme_starts: Vec<bool> =
shaped.characters.iter().map(|c| c.grapheme_start).collect();
Expand Down
22 changes: 16 additions & 6 deletions parley_engine/src/shape/shaper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ pub struct ShapeOptions<'a> {
pub features: &'a [FontFeature],
/// The font variations that are constant over an item.
pub variations: &'a [FontVariation],
/// The per-character style indices.
// TODO: rename to something like `user_data` (s.t. we don't assume it's a style per se).
// TODO: probably move this out of `ShapeOptions`, and supply it as a parameter on
// `Shaper::shape_text`.
pub char_style_indices: &'a [u16],
}

/// The font instance to shape an item with.
Expand Down Expand Up @@ -92,6 +87,10 @@ impl Shaper {
/// split on properties like shaping-relevant style changes (e.g., font size) or properties like
/// language.
///
/// `char_style_indices` holds per-character style indices; these are copied onto
/// [`Character`][super::Character`] and [`ShapedCluster`][super::ShapedCluster]. A shaped
/// cluster's style index is that of its logically first constituent character.
Comment on lines +90 to +92

@tomcur tomcur Aug 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's now an argument for dropping style_indices from parley_engine completely, as it doesn't actually read them. Since #727, the full text is shaped, so character indices exposed by parley_engine can be used by users to index into some per-character slice directly.

///
/// Characters that don't have a particular script have their script resolved based on
/// surrounding context (see [`Segment::script`]).
///
Expand All @@ -106,6 +105,8 @@ impl Shaper {
&mut self,
text: &str,
analysis: &Analysis,
// TODO: rename to something like `user_data` (s.t. we don't assume it's a style per se).
char_style_indices: &[u16],
items: impl IntoIterator<Item = Item<'options>>,
mut select_font: impl FontSelector,
shaped_text: &mut ShapedText,
Expand All @@ -114,6 +115,12 @@ impl Shaper {
shaped_text.reserve(text.len());

let char_count = analysis.char_info().len();
debug_assert_eq!(
char_style_indices.len(),
char_count,
"The number of character style indices must be equal to the character count"
);

let mut previous_item_end = 0;
let mut itemizer = analysis.itemize(text);

Expand Down Expand Up @@ -142,6 +149,7 @@ impl Shaper {
&item.options,
&mut select_font,
analysis.char_info(),
char_style_indices,
shaped_text,
)
.is_err()
Expand Down Expand Up @@ -182,6 +190,7 @@ fn shape_segment(
options: &ShapeOptions<'_>,
select_font: &mut impl FontSelector,
char_info: &[CharInfo],
char_style_indices: &[u16],
shaped_text: &mut ShapedText,
) -> Result<(), ()> {
select_font.begin_segment(item, options);
Expand All @@ -193,7 +202,7 @@ fn shape_segment(

// Only process current item
let item_char_info = &char_info[char_range.start..char_range.end];
let item_char_style_indices = &options.char_style_indices[char_range.start..char_range.end];
let item_char_style_indices = &char_style_indices[char_range.start..char_range.end];

if item_text.is_empty() {
return Ok(()); // No clusters
Expand Down Expand Up @@ -378,6 +387,7 @@ fn shape_segment(
item,
options,
char_info,
char_style_indices,
&font,
&glyph_buffer,
harf_shaper.coords(),
Expand Down
Loading