Rebuild TextBlock text runs only when the content changes - #22149
Rebuild TextBlock text runs only when the content changes#22149Gillibald wants to merge 4 commits into
Conversation
_textRuns is built from Inlines, but OnMeasureInvalidated discarded it on any measure invalidation while Inlines still held the content. Between that point and the next measure pass, CreateTextLayout read a null _textRuns as "no inlines" and shaped Text instead, which is null whenever the content lives in Inlines. That empty result went into the TextRunCache, keyed by text source index, so every later layout reused it and the control rendered nothing until something invalidated the cache. - Discard _textRuns in InvalidateTextLayout, next to the run cache, so the runs and the cache are dropped by the same event and cannot disagree about the content. - Build the runs on demand in EnsureTextRuns, and pick the text source by HasComplexContent rather than by _textRuns being set. - Split the constraint-dependent work out of run building. Runs answer to the content alone; only an embedded control answers to the available width, so Inline.MeasureEmbeddedControls measures it and EmbeddedControlRun reports the child's DesiredSize live. Runs now survive a constraint change instead of being rebuilt every measure. SelectableTextBlock never hands the run cache to its layout, but it shared the same fallback and kept the wrong layout on _textLayout. Fixes #21902 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LineSpacing had no case in the property change switch and is not one of the properties registered with AffectsRender, so changing it left the measured size and the rendered text untouched even though CreateTextLayout feeds it into the paragraph properties. It changes line placement rather than shaping, so it belongs with LineHeight and the other properties that keep the run cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Repeated content invalidation before measurement can retain and reuse a stale TextLayout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates TextBlock to preserve text runs across layout-only invalidations while rebuilding them when content changes.
Changes:
- Builds inline runs on demand to prevent stale text-source caching.
- Remeasures embedded controls independently of run construction.
- Adds
LineSpacinginvalidation and regression tests.
File summaries
| File | Description |
|---|---|
src/Avalonia.Controls/TextBlock.cs |
Refactors run creation, embedded measurement, and invalidation. |
src/Avalonia.Controls/SelectableTextBlock.cs |
Ensures inline runs exist before layout creation. |
src/Avalonia.Controls/Documents/Inline.cs |
Separates run creation from embedded measurement. |
src/Avalonia.Controls/Documents/InlineUIContainer.cs |
Remeasures controls without rebuilding runs. |
src/Avalonia.Controls/Documents/Span.cs |
Propagates the separated operations. |
src/Avalonia.Controls/Documents/Run.cs |
Updates the run-building signature. |
src/Avalonia.Controls/Documents/LineBreak.cs |
Updates the run-building signature. |
tests/Avalonia.Controls.UnitTests/TextBlockTests.cs |
Tests caching, embedded controls, and line spacing. |
tests/Avalonia.Controls.UnitTests/SelectableTextBlockTests.cs |
Tests out-of-measure inline layout creation. |
tests/Avalonia.Controls.UnitTests/InlineTests.cs |
Updates calls to the new signature. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
You can test this PR using the following package version. |
InvalidateMeasure only raises OnMeasureInvalidated while the measure is still valid, so a second content change before the next measure pass left _textLayout holding the layout the first change had already replaced. MeasureOverride keeps that layout when the constraint has not moved, so the block measured and rendered the superseded content. Clear the layout in InvalidateTextLayout and InvalidateTextLayoutKeepCache rather than relying on OnMeasureInvalidated to run, which is what TextPresenter already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You can test this PR using the following package version. |
There was a problem hiding this comment.
🔵 Needs a closer look
An embedded child can be remeasured while an already-created text layout retains stale line metrics.
Review details
Suppressed comments (1)
src/Avalonia.Controls/TextBlock.cs:820
- If
TextLayoutis read after the block is invalidated, then an embedded child invalidates its measure before the queued measure runs, the block is already measure-invalid soOnMeasureInvalidatedis not raised again. This line remeasures the child, but line 823 reuses the layout created with the child's previous size; its cached line width, height, and wrapping therefore remain stale. Dispose the layout after remeasuring so the getter rebuilds line metrics from the currentDesiredSize.
MeasureEmbeddedControls(deflatedSize);
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
A line snapshots its metrics when it is formatted, so a layout built before a child was measured again keeps reporting the width and height that child used to have. MeasureOverride reuses the layout whenever the constraint has not moved, so a control that resizes while the block is already measure invalid never reaches the measured size. - MeasureEmbeddedControls reports whether any child came back a different size, and the layout is dropped only then rather than on every pass. - Route the remaining layout resets through DisposeTextLayout so every reset goes through one place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The cache and layout lifetimes are consistently separated, with focused regression coverage for the identified failure modes.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
You can test this PR using the following package version. |
What does the pull request do?
Rebuilds a
TextBlock's text runs only when the content they are built from changes, instead of on every measure pass. Embedded controls are re-measured in place, against the current constraint, rather than riding along on that rebuild:Inline.BuildTextRunloses itsblockSizeparameter, which onlyInlineUIContainerever used, and the measuring moves to a newInline.MeasureEmbeddedControls(Size).EmbeddedControlRunalready reports its child'sDesiredSizelive, so the existing runs stay valid across a constraint change.Also folds in a one-line fix for
TextBlock.LineSpacing, which invalidated nothing when it changed.What is the current behavior?
_textRunsis built fromInlines, butOnMeasureInvalidateddiscards it on any measure invalidation, whileInlinesstill holds the content. Between that point and the next measure pass the control disagrees with itself about what it contains, andCreateTextLayoutreads a null_textRunsas "no inlines":So the textSource itself is built on the wrong condition. The empty result is then shaped and stored in the
TextRunCacheunder the text source index, so every later layout reuses it and the control never recovers on its own._textLayouthas the same problem from the other side.InvalidateMeasureonly raisesOnMeasureInvalidatedwhile the measure is still valid, and that callback is the only thing clearing the layout, so anything happening after the first invalidation leaves it in place: a second content change, or an embedded control resizing.MeasureOverridekeeps that layout whenever the constraint has not moved, and a line snapshots its metrics when it is formatted, so the block measures and renders the superseded content.What is the updated/expected behavior with this PR?
A
TextBlockshapes and renders itsInlinesno matter when the layout is created, and aTextRunCacheentry can no longer be built from content the control does not have.The layout is dropped by the content change itself rather than by the measure-invalidation callback, and by a measure pass that finds an embedded control has resized, so neither can leave a superseded layout in place.
Checklist
Breaking changes
None.
Inline.BuildTextRunchanges signature andInline.MeasureEmbeddedControlsis new, but both areinternal, andInlinecannot be derived from outside the assembly becauseBuildTextRunis an internal abstract member.Obsoletions / Deprecations
None.
Fixed issues
Fixes #21902
Supersedes #21904
🤖 Generated with Claude Code