Skip to content

Derive ShapedRun::advance from clusters#706

Merged
waywardmonkeys merged 1 commit into
linebender:mainfrom
waywardmonkeys:core/derive-shaped-run-advance
Jul 22, 2026
Merged

Derive ShapedRun::advance from clusters#706
waywardmonkeys merged 1 commit into
linebender:mainfrom
waywardmonkeys:core/derive-shaped-run-advance

Conversation

@waywardmonkeys

Copy link
Copy Markdown
Contributor

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:

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%

`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`.

Add a regression test for a one-cluster run.
@waywardmonkeys

Copy link
Copy Markdown
Contributor Author

An LLM (GPT 5.6, Sol, xhigh) was involved in this with finding / identifying the issue and writing the test.

@tomcur tomcur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For additional context, see: #679 (comment).

@taj-p taj-p left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!! Thank you 🙏

@waywardmonkeys
waywardmonkeys added this pull request to the merge queue Jul 22, 2026
Merged via the queue into linebender:main with commit 2e17b6e Jul 22, 2026
24 checks passed
@waywardmonkeys
waywardmonkeys deleted the core/derive-shaped-run-advance branch July 22, 2026 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants