fix(text): don't shape stale content when TextLayout is built outside measure - #21904
Closed
gentledepp wants to merge 1 commit into
Closed
fix(text): don't shape stale content when TextLayout is built outside measure#21904gentledepp wants to merge 1 commit into
gentledepp wants to merge 1 commit into
Conversation
gentledepp
force-pushed
the
fix/21902_textblock-textruncache-blank
branch
from
July 31, 2026 09:47
76da4cb to
587fa70
Compare
|
You can test this PR using the following package version. |
Member
|
@gentledepp what's the status of this PR? Is it ready for review? |
|
I am running into the symptoms as described on Avalonia 12.1.2 and prior 12.* versions. TextBlock with inlines sometimes ends up with Width=0 / empty, after IsVisible is toggled on one of the parents. Invalidating measure/arrange does not help, only touching the Inlines, FontSize, ... helps. Will this get merged? Thanks! |
Member
|
@yself - we're waiting for the PR author to respond; the PR is still marked as draft. Maybe @Gillibald may have some input though. |
3 tasks
Member
|
Closing in favor of #22149 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #21902.
Check out the before- and after video I added to the corresponding issue:
#21902
What's broken
A
TextBlockwhose content lives inInlinesrather thanTextcan end up rendering nothing,permanently. It keeps its height, it's visible,
Inlines.Count > 0,Boundsis ample — butDesiredSize.Widthis0. Re-measuring doesn't fix it, forcing a full layout pass on an ancestordoesn't fix it, and replacing the whole
InlineCollectiondoesn't reliably fix it either. SettingFontSizefixes it instantly, which is the clue that cracked it:FontSizeis one of theproperties routed through
InvalidateTextLayout(), and that is the only code path that calls_textRunCache?.Invalidate().So it's the run cache from #21030, and here's the chain:
TextRunCachekeys its entries onfirstTextSourceIndexalone — there's nothing in an entry thatsays which content produced it. And
TextFormatterImpl.FormatLinechecks the cache before itfetches anything from the text source, so an entry at index 0 beats whatever the
TextBlockactually holds right now. That's fine as long as nothing ever caches the wrong thing, and normally
nothing does, because every content mutation routes through
InvalidateTextLayout().The hole is that
TextBlockcan build a layout from the wrong text source in the first place:TextLayoutis lazy and public:_textLayout ??= CreateTextLayout(Text).CreateTextLayoutuses_textRunsif it has them, and otherwise silently falls back tonew SimpleTextSource(text ?? "")._textRunsis only ever built inMeasureOverride, and it's set back tonullinOnMeasureInvalidated.Put those together and there's a window between a measure invalidation and the next measure pass
where
_textRunsisnullwhileInlinesis non-empty. If the content lives entirely inInlines, thenTextisnull, so the fallback happily shapes the empty string — and that emptyresult gets written into the cache at key 0. The
fetchedRuns.Count == 0early-out doesn't catchit, because
FetchTextRunsadds theTextEndOfParagraphrun before it breaks, so the count is 1,not 0. From then on every measure and every arrange hits the poisoned entry and gets an empty line
back, forever.
Reading
TextLayoutin that window is not exotic — it's a public property andRenderTextLayoutreads it, so a render pass that runs before a queued measure is enough. The realistic trigger is
any binding that assigns
Inlinesasynchronously (converter yields an emptyInlineCollectionuntil the VM delivers the real value a dispatcher turn later).
The fix
Build the runs on demand in the
TextLayoutgetter instead of letting it fall back toText:CreateTextLayoutCorebuilds_textRunsfromInlinesif they're missing and we have complexcontent, then delegates to
CreateTextLayout(Text)as before. Net effect: a layout created outsidea measure pass can no longer shape the wrong content, so it can no longer cache the wrong content.
Two notes on the shape of it:
CreateTextLayout. That's deliberate —AccessTextandSelectableTextBlockboth overrideCreateTextLayoutand inherit the same fallback, and puttingthe guard in the getter covers them too without touching either class.
HasComplexContent, so the pure-Textpath is completely unaffected and there'sno extra work in the common case.
Why not one of the other options
I tried a few things first, in case any of these is the reviewer's first instinct:
_textRunCacheinOnMeasureInvalidated, where_textRunsgets nulled. Doesn'twork on its own: the poisoning read happens after that point, so the cache just gets re-poisoned
before the next measure. It would also throw away the measure→arrange reuse that Implement TextRunCache #21030 exists to
provide.
_textLayoutinMeasureOverridewhenever_textRunsis rebuilt. Fixes the stalelayout but not the cache. Test 3 below still fails, because the poisoned entry outlives the
layout being dropped.
TextRunCacheentries a content identity — e.g. a generation counter bumped whenever thetext source changes. Honestly I think this is the right long-term shape, and it would close the
class of bug rather than this one instance. But it's a much bigger change and touches the caching
contract, so I went with the narrow fix that closes the reachable hole in
TextBlock. Happy totake a run at the bigger version instead if you'd prefer that.
Tests
Three tests added to
TextBlockTests. Onmainwithout the fix, tests 2 and 3 fail withDesiredSize was 0, 15; all three pass with it.The third one (
..._Across_Constraints) is the one that actually pins the diagnosis: it measures ata different constraint, which forces
MeasureOverrideto drop_textLayout. It still failswithout the fix, so the problem can't be explained by layout caching — it's the run cache.
Full suites on
mainwith the patch, no regressions:Avalonia.Controls.UnitTestsAvalonia.Base.UnitTestsOne thing I noticed but did not fix
CachedShapingResultalso holds non-shaped runs likeEmbeddedControlRunfromInlineUIContainer.Those get measured against whatever constraint was in force when
Inline.BuildTextRunran, andMeasureOverriderebuilding_textRunsat a new constraint doesn't invalidate the cache. Thatlooks like a second, independent staleness path for
InlineUIContainersizing. It predates thischange and I didn't dig into it — flagging it in case it's news.
Draft because I'd like a maintainer's read on the narrow-vs-generation-counter question before this
is considered final.
@Gillibald since you are the master of text caching and this seems to be a regression, can you please have a short look and let me know if this at all makess sense? It does fix my rendering issue. But I am not sure if I break something with it