Skip to content

[Text] Fixes double reorder of cached bidi runs - #21351

Merged
MrJul merged 6 commits into
AvaloniaUI:masterfrom
Gillibald:fixes/doubleReorder
May 14, 2026
Merged

[Text] Fixes double reorder of cached bidi runs#21351
MrJul merged 6 commits into
AvaloniaUI:masterfrom
Gillibald:fixes/doubleReorder

Conversation

@Gillibald

@Gillibald Gillibald commented May 12, 2026

Copy link
Copy Markdown
Contributor

What does the pull request do?

Fix double reorder with cached bidi runs

When TextRunCache was enabled and a paragraph contained RTL text, recreating a TextLayout from the same cache would render RTL runs incorrectly.

The root cause was in how shaped runs were shared between the cache and the formatter:

The cache stored a ShapedTextRun referencing a ShapedBuffer backed by a rented GlyphInfo[] array.
The formatter created "non-owning" copies for use in the line by passing the same ArraySlice<GlyphInfo> to a new ShapedBuffer constructor; both the cached run and the line's run therefore pointed at the same backing array.
BidiReorderer.Reverse() mutated glyph order in-place on the line's copy, which also reversed the cached run's glyphs.
On the next layout from cache, the retrieved run had IsReversed = false (fresh copy) but an already-reversed buffer. BidiReorderer reversed it a second time, putting RTL glyphs back into logical (ascending-cluster) order — causing wrong visual rendering.

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

ShapedTextRun is now ref-counted (AddRef/ Dispose). The shaper produces one reference; calling AddRef() yields an additional owned reference rather than a shallow buffer copy.
BidiReorderer no longer calls Reverse() on individual runs. RTL ShapedBuffers are produced by the shaper already in visual (descending-cluster) order; the reorderer only shuffles the run sequence.
TextFormatterImpl: when adding runs to the cache, AddRef is called so both the cache and the current line hold independent, correctly-counted references. When reading from cache (FormatLineFromCache), each run's ref-count is incremented rather than copying the buffer.
TextRunCache: properly disposes the old entry (releasing its reference) when a key is overwritten, preventing leaks.

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@Gillibald Gillibald changed the title [Text] Fixes double reorder of cached bidi runs [WIP][Text] Fixes double reorder of cached bidi runs May 12, 2026
@Gillibald Gillibald changed the title [WIP][Text] Fixes double reorder of cached bidi runs [Text] Fixes double reorder of cached bidi runs May 12, 2026
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0065331-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added area-textprocessing backport-candidate-12.0.x Consider this PR for backporting to 12.0 branch labels May 14, 2026

@MrJul MrJul 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.

LGTM!

@MrJul MrJul added the bug label May 14, 2026
@MrJul
MrJul added this pull request to the merge queue May 14, 2026
Merged via the queue into AvaloniaUI:master with commit 507e167 May 14, 2026
12 checks passed
MrJul pushed a commit to MrJul/Avalonia that referenced this pull request May 28, 2026
* Fix double reorder with cached runs

* Revert change

* Add covering unit tests

* Remove worktrees

* Update API baseline
@MrJul MrJul added backported-12.0.x and removed backport-candidate-12.0.x Consider this PR for backporting to 12.0 branch labels May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants