Skip to content

[Text input] Measure the caret outside the text layout getter - #22176

Merged
MrJul merged 3 commits into
mainfrom
stx/0-preedit-reentrancy
Sep 14, 2026
Merged

[Text input] Measure the caret outside the text layout getter#22176
MrJul merged 3 commits into
mainfrom
stx/0-preedit-reentrancy

Conversation

@Gillibald

@Gillibald Gillibald commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does the pull request do?

Part of the structured text input work tracked in #22168.

Takes the caret measurement out of the TextPresenter.TextLayout getter, so building a layout raises no events and a preedit key press builds one layout instead of three. Hiding the caret stops rebuilding the layout as well.

What is the current behavior?

The getter builds the layout and then runs UpdateCaret, which raises CaretBoundsChanged. A handler that invalidates the layout - an IME candidate window, or caret-following code that forces a layout pass - disposes the layout the getter is about to return, and the getter hands back a field the handler has already cleared. The caller then dereferences null: MeasureOverride throws on the first preedit update while such a handler is attached.

The same path is wasteful. Counting CreateTextLayout calls per preedit key press today gives three: one from OnPreeditChanged, which the PreeditText case of the invalidation switch disposes immediately after; one from the cursor-position property that follows it; and one from the measure pass that actually renders.

OnPreeditChanged also builds a raw CharacterHit at CaretIndex + cursor and measures it against whatever layout exists, which is either one that does not contain the preedit yet or a position on the seam of the preedit's fallback-font run. Both are outside what hit-testing accepts.

What is the updated/expected behavior with this PR?

The getter builds the layout, marks the caret dirty, and returns, raising nothing. EnsureCaretBounds measures the caret afterwards against the layout field rather than the property, so it can neither build a layout nor recurse; a handler that invalidates from there only schedules the next measure pass. The measure pass builds the layout once and settles the caret against that build, and a preedit change records the position it wants for that pass to place, normalized through the line's caret hit walkers.

Checklist

Breaking changes

None. CaretBoundsChanged now fires from the measure pass or from an explicit caret move rather than from an arbitrary layout access, and HideCaret no longer invalidates the layout. Both are internal timing changes in the presenter.

Obsoletions / Deprecations

None.

Fixed issues

Part of #22168

🤖 Generated with Claude Code

@Gillibald
Add failing test: preedit crashes when a CaretBoundsChanged handler i…
e22407b 
…nvalidates layout

TextPresenter's TextLayout getter runs UpdateCaret after creating the
layout, and UpdateCaret raises CaretBoundsChanged. A subscriber that
synchronously invalidates the text layout - the IME candidate window or
any caret-following code forcing a layout pass does this in practice -
nulls the field before the getter returns it, so the surrounding
computation (the preedit update, or the measure pass materializing the
layout) dereferences null. Found live by TextInputDebugger on its first
Japanese IME session: the trace viewer's scroll-into-view pumped a layout
pass from inside the caret callback and every composition update crashed.
@Gillibald Gillibald changed the title Fix preedit caret crash when a caret-bounds handler invalidates layout [Text input] Fix preedit caret crash when a caret-bounds handler invalidates layout Sep 8, 2026
@Gillibald Gillibald mentioned this pull request Sep 8, 2026
Open
@avaloniaui-bot

avaloniaui-bot commented Sep 8, 2026

Copy link
Copy Markdown

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

@MrJul MrJul added bug backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Sep 8, 2026
MrJul
MrJul requested changes Sep 9, 2026
View reviewed changes
Comment thread src/Avalonia.Controls/Presenters/TextPresenter.cs
Comment thread src/Avalonia.Controls/Presenters/TextPresenter.cs Outdated
@Gillibald
Gillibald force-pushed the stx/0-preedit-reentrancy branch from b24c23a to 1d92065 Compare September 9, 2026 14:40
@Gillibald Gillibald changed the title [Text input] Fix preedit caret crash when a caret-bounds handler invalidates layout [Text input] Measure the caret outside the text layout getter Sep 9, 2026
Gillibald and others added 2 commits September 9, 2026 16:50
@Gillibald @claude
Measure the caret outside the text layout getter
a515cff 
The getter built the layout and then ran UpdateCaret, which raises
CaretBoundsChanged. A handler that invalidates the layout - an IME candidate
window, or caret-following code forcing a layout pass - disposed the layout the
getter was about to return, so the getter handed back a field the handler had
already cleared and the caller dereferenced null.

- The getter builds the layout, marks the caret dirty and returns. It raises no
  events, so nothing invalidates the layout while it is being built.
- EnsureCaretBounds measures the caret against the layout field rather than the
  property, so it can neither build a layout nor recurse. A handler invalidating
  from there only schedules the next measure pass.
- The measure pass builds the layout once and settles the caret against it, so a
  preedit key press builds one layout where it built three.
- A preedit change records the caret position it wants and lets that pass place
  it, normalized through the line's caret hit walkers so a position on the seam
  of a fallback-font run resolves to a hit the layout can measure.
- The declaration of the layout field now names the five members that own its
  lifetime.

Co-Authored-By: Claude Opus 5
@Gillibald @claude
Repaint instead of rebuilding the layout to hide the caret
743cb64 
HideCaret disposed the text layout and reshaped the text on the next access,
which is work the caret does not need: it blinks over the text rather than
taking part in it. Anything holding the layout across the call also lost it.

Co-Authored-By: Claude Opus 5
@Gillibald
Gillibald force-pushed the stx/0-preedit-reentrancy branch from 1d92065 to 743cb64 Compare September 9, 2026 14:52
@avaloniaui-bot

avaloniaui-bot commented Sep 9, 2026

Copy link
Copy Markdown

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

MrJul
MrJul approved these changes Sep 14, 2026
View reviewed changes

@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 this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit a4fe71e Sep 14, 2026
10 checks passed
@MrJul
MrJul deleted the stx/0-preedit-reentrancy branch September 14, 2026 10:47
MrJul pushed a commit to MrJul/Avalonia that referenced this pull request Sep 22, 2026
@Gillibald @claude @MrJul
[Text input] Measure the caret outside the text layout getter (Avalon…
e155d3d 
…iaUI#22176)

* Add failing test: preedit crashes when a CaretBoundsChanged handler invalidates layout

TextPresenter's TextLayout getter runs UpdateCaret after creating the
layout, and UpdateCaret raises CaretBoundsChanged. A subscriber that
synchronously invalidates the text layout - the IME candidate window or
any caret-following code forcing a layout pass does this in practice -
nulls the field before the getter returns it, so the surrounding
computation (the preedit update, or the measure pass materializing the
layout) dereferences null. Found live by TextInputDebugger on its first
Japanese IME session: the trace viewer's scroll-into-view pumped a layout
pass from inside the caret callback and every composition update crashed.

* Measure the caret outside the text layout getter

The getter built the layout and then ran UpdateCaret, which raises
CaretBoundsChanged. A handler that invalidates the layout - an IME candidate
window, or caret-following code forcing a layout pass - disposed the layout the
getter was about to return, so the getter handed back a field the handler had
already cleared and the caller dereferenced null.

- The getter builds the layout, marks the caret dirty and returns. It raises no
  events, so nothing invalidates the layout while it is being built.
- EnsureCaretBounds measures the caret against the layout field rather than the
  property, so it can neither build a layout nor recurse. A handler invalidating
  from there only schedules the next measure pass.
- The measure pass builds the layout once and settles the caret against it, so a
  preedit key press builds one layout where it built three.
- A preedit change records the caret position it wants and lets that pass place
  it, normalized through the line's caret hit walkers so a position on the seam
  of a fallback-font run resolves to a hit the layout can measure.
- The declaration of the layout field now names the five members that own its
  lifetime.

Co-Authored-By: Claude Opus 5 

* Repaint instead of rebuilding the layout to hide the caret

HideCaret disposed the text layout and reshaped the text on the next access,
which is work the caret does not need: it blinks over the text rather than
taking part in it. Anything holding the layout across the call also lost it.

Co-Authored-By: Claude Opus 5 

---------

Co-authored-by: Claude Opus 5
@MrJul MrJul added backported-12.1.x and removed backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Sep 22, 2026
@chenjt2001 chenjt2001 mentioned this pull request Sep 29, 2026
Merged
1 of 3 tasks
grokys pushed a commit to Evan260/Avalonia that referenced this pull request Oct 7, 2026
@Gillibald @claude @grokys
[Text input] Measure the caret outside the text layout getter (Avalon…
4f419a3 
…iaUI#22176)

* Add failing test: preedit crashes when a CaretBoundsChanged handler invalidates layout

TextPresenter's TextLayout getter runs UpdateCaret after creating the
layout, and UpdateCaret raises CaretBoundsChanged. A subscriber that
synchronously invalidates the text layout - the IME candidate window or
any caret-following code forcing a layout pass does this in practice -
nulls the field before the getter returns it, so the surrounding
computation (the preedit update, or the measure pass materializing the
layout) dereferences null. Found live by TextInputDebugger on its first
Japanese IME session: the trace viewer's scroll-into-view pumped a layout
pass from inside the caret callback and every composition update crashed.

* Measure the caret outside the text layout getter

The getter built the layout and then ran UpdateCaret, which raises
CaretBoundsChanged. A handler that invalidates the layout - an IME candidate
window, or caret-following code forcing a layout pass - disposed the layout the
getter was about to return, so the getter handed back a field the handler had
already cleared and the caller dereferenced null.

- The getter builds the layout, marks the caret dirty and returns. It raises no
  events, so nothing invalidates the layout while it is being built.
- EnsureCaretBounds measures the caret against the layout field rather than the
  property, so it can neither build a layout nor recurse. A handler invalidating
  from there only schedules the next measure pass.
- The measure pass builds the layout once and settles the caret against it, so a
  preedit key press builds one layout where it built three.
- A preedit change records the caret position it wants and lets that pass place
  it, normalized through the line's caret hit walkers so a position on the seam
  of a fallback-font run resolves to a hit the layout can measure.
- The declaration of the layout field now names the five members that own its
  lifetime.

Co-Authored-By: Claude Opus 5 

* Repaint instead of rebuilding the layout to hide the caret

HideCaret disposed the text layout and reshaped the text on the next access,
which is work the caret does not need: it blinks over the text rather than
taking part in it. Anything holding the layout across the call also lost it.

Co-Authored-By: Claude Opus 5 

---------

Co-authored-by: Claude Opus 5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrJul MrJul MrJul approved these changes

Assignees

No one assigned

Labels

area-textprocessing backported-12.1.x bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Gillibald @avaloniaui-bot @MrJul