Skip to content

Commit 2e17b6e

Browse files
Derive ShapedRun::advance from clusters (#706)
`process_clusters` updated its `run_advance` accumulator only when it encountered the next cluster. It therefore omitted the final cluster, and a run containing one cluster reported zero advance. Derive `ShapedRun::advance` from finalized `ClusterData::advance` values instead. This makes cluster data the source of truth and removes the parallel accumulator from `process_clusters`. Using finalized `ClusterData::advance` values as the source of truth also respects transformations applied while constructing clusters, such as stripping newline glyph contribution. Add a regression test for a one-cluster run. ## Performance A repeated paired Tango comparison against `fed192a` found no statistically significant regressions: ```text Default Style - arabic 20 characters -0.74% Default Style - latin 20 characters -0.06% Default Style - japanese 20 characters -0.42% Default Style - arabic 1 paragraph -0.60% Default Style - latin 1 paragraph +0.21% Default Style - japanese 1 paragraph -0.90% Default Style - arabic 4 paragraph -0.65% Default Style - latin 4 paragraph -0.44% Default Style - japanese 4 paragraph -1.10%* Styled - arabic 20 characters +0.22% Styled - latin 20 characters -0.74% Styled - japanese 20 characters +0.73% Styled - arabic 1 paragraph +0.44% Styled - latin 1 paragraph +0.15% Styled - japanese 1 paragraph +0.30% Styled - arabic 4 paragraph +0.52% Styled - latin 4 paragraph +0.68% Styled - japanese 4 paragraph -0.94% ```
1 parent fed192a commit 2e17b6e

1 file changed

Lines changed: 73 additions & 11 deletions

File tree

parley_engine/src/shape/shaped_text.rs

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ impl ShapedText {
265265
let is_rtl = !item.bidi_level.is_multiple_of(2);
266266

267267
let glyphs_start = self.glyphs.len();
268-
let run_advance = if !is_rtl {
268+
if !is_rtl {
269269
process_clusters(
270270
Direction::Ltr,
271271
&mut self.clusters,
@@ -276,9 +276,9 @@ impl ShapedText {
276276
&char_info[range.char_range.clone()],
277277
&options.char_style_indices[range.char_range.clone()],
278278
text[range.byte_range.clone()].char_indices(),
279-
)
279+
);
280280
} else {
281-
let run_advance = process_clusters(
281+
process_clusters(
282282
Direction::Rtl,
283283
&mut self.clusters,
284284
&mut self.glyphs,
@@ -292,14 +292,19 @@ impl ShapedText {
292292
// Reverse clusters into logical order for RTL
293293
let clusters_len = self.clusters.len();
294294
self.clusters[clusters_start..clusters_len].reverse();
295-
run_advance
296-
};
295+
}
296+
297+
let clusters_range = clusters_start..self.clusters.len();
298+
let run_advance = self.clusters[clusters_range.clone()]
299+
.iter()
300+
.map(|cluster| cluster.advance)
301+
.sum();
297302

298303
self.runs.push(ShapedRun {
299304
range,
300305
font_size: options.font_size,
301306
font_index,
302-
clusters_range: clusters_start..self.clusters.len(),
307+
clusters_range,
303308
glyphs_range: glyphs_start..self.glyphs.len(),
304309
normalized_coords_range,
305310
bidi_level: item.bidi_level,
@@ -364,7 +369,7 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>(
364369
char_infos: &[CharInfo],
365370
char_style_indices: &[u16],
366371
char_indices_iter: I,
367-
) -> f32 {
372+
) {
368373
let char_info_at = |i: usize| (char_infos[i], char_style_indices[i]);
369374
let mut char_indices_iter = char_indices_iter.peekable();
370375
let mut cluster_start_char = char_indices_iter.next().unwrap();
@@ -373,7 +378,6 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>(
373378
let start_cluster_id = glyph_infos.first().unwrap().cluster;
374379
let mut cluster_id = start_cluster_id;
375380
let mut char_info = char_info_at(cluster_id as usize);
376-
let mut run_advance = 0.0;
377381
let mut cluster_advance = 0.0;
378382
// If the current cluster might be a single-glyph, zero-offset cluster, we defer
379383
// pushing the first glyph to `glyphs` because it might be inlined into `ClusterData`.
@@ -418,7 +422,6 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>(
418422
for (glyph_info, glyph_pos) in glyph_infos.iter().zip(glyph_positions.iter()) {
419423
// Flush previous cluster if we've reached a new cluster
420424
if cluster_id != glyph_info.cluster {
421-
run_advance += cluster_advance;
422425
let num_components = num_components(glyph_info.cluster, cluster_id, last_cluster_id);
423426
cluster_advance /= num_components as f32;
424427
let is_newline = to_whitespace(cluster_start_char.1) == Whitespace::Newline;
@@ -594,8 +597,6 @@ fn process_clusters<I: Iterator<Item = (usize, char)>>(
594597
);
595598
}
596599
}
597-
598-
run_advance
599600
}
600601

601602
#[derive(PartialEq)]
@@ -669,3 +670,64 @@ fn push_cluster(
669670
advance: final_advance,
670671
});
671672
}
673+
674+
#[cfg(test)]
675+
mod tests {
676+
use alloc::{sync::Arc, vec};
677+
678+
use fontique::Synthesis;
679+
use linebender_resource_handle::{Blob, FontData};
680+
681+
use crate::{Analysis, AnalysisOptions, Analyzer, FontInstance, ShapeOptions, Shaper};
682+
683+
use super::ShapedText;
684+
685+
const ROBOTO: &[u8] =
686+
include_bytes!("../../../parley_dev/assets/fonts/roboto_fonts/Roboto-Regular.ttf");
687+
688+
fn shape(text: &str) -> ShapedText {
689+
let mut analysis = Analysis::new();
690+
Analyzer::new().analyze(
691+
text,
692+
&AnalysisOptions {
693+
word_break: &[],
694+
line_break_override: None,
695+
},
696+
&mut analysis,
697+
);
698+
let font = FontInstance {
699+
font: FontData::new(Blob::new(Arc::new(ROBOTO)), 0),
700+
synthesis: Synthesis::default(),
701+
};
702+
let char_style_indices = vec![0; text.chars().count()];
703+
let mut shaper = Shaper::default();
704+
let mut shaped = ShapedText::new();
705+
for item in analysis.itemize(text, |_| false) {
706+
shaper.shape_item(
707+
text,
708+
&analysis,
709+
&item,
710+
&ShapeOptions {
711+
font_size: 32.0,
712+
language: None,
713+
features: &[],
714+
variations: &[],
715+
char_style_indices: &char_style_indices,
716+
},
717+
|_| Some(font.clone()),
718+
&mut shaped,
719+
);
720+
}
721+
shaped
722+
}
723+
724+
#[test]
725+
fn single_cluster_run_advance_matches_its_cluster() {
726+
let shaped = shape("A");
727+
let run = &shaped.runs()[0];
728+
let cluster = &shaped.clusters()[run.clusters_range.clone()][0];
729+
730+
assert!(cluster.advance > 0.0);
731+
assert_eq!(run.advance, cluster.advance);
732+
}
733+
}

0 commit comments

Comments
 (0)