TextMate: fall back to a full TextView redraw when the visible-line cache is stale#595
Merged
danipen merged 3 commits intoJun 5, 2026
Conversation
TMModel.SetGrammar early-returns when the same IGrammar instance is passed, skipping its internal InvalidateLine(0). That's the common case when Installation reloads a cached grammar after a document swap, and it leaves the model with no invalid lines to process. TransformLine then sees null tokens for the visible lines and they render plain, until something else triggers re-tokenization. Explicitly invalidate line 0 right after _model.SetGrammar(grammar) so the tokenizer always has work to do on the freshly-bound model. The extra invalidation is cheap on the common path (same content + same grammar tokenizes to the same state) but plugs the gap where the caller's intent — "use this grammar for this content" — could be silently dropped.
…ache is stale ModelTokensChanged is invoked from the tokenizer's background thread and captures _firstVisibleLineIndex/_lastVisibleLineIndex under lock to use inside the UI-thread dispatch callback. Those cached indices are reset to -1 (and _areVisualLinesValid to false) by SetModel, and only refilled when TextView_VisualLinesChanged fires after a layout pass. If a tokenizer event arrives before the first layout pass on a freshly bound model, the captured indices are still (-1, -1), so the redraw range collapses to firstLineIndexToRedraw=Math.Max(firstChanged, -1) and lastLineIndexToRedrawLine=Math.Min(lastChanged, -1)=-1, clamped to 0. _textView.Redraw(offset, length) is then called with a negative length and paints nothing. The newly-tokenized lines stay un-styled until the next layout pass (a scroll, a click, etc.) eventually triggers a fresh render. Detect that case — either the visible-line cache wasn't valid at capture time, or the clamped range came out inverted — and fall back to a full _textView.Redraw() instead of the broken partial one. The fallback is slightly heavier but only fires in the narrow window before the first layout pass; the partial-redraw path still runs in the common case.
Contributor
|
@danipen do you still need to release the multiple race condition fixes and memory leak fixes i added in TextMateSharp? i dont think they're included in the latest version of AvaloniaEdit, right? |
Collaborator
|
Let me check... |
Collaborator
|
Yes, you're right. It's worth trying TextMateSharp v2.0.4 first, to see if all the race condition changes fixes this issue. If it's fixed we can discard it. If not, I'm fine merging it. |
This reverts commit 72328dc. The defensive InvalidateLine(0) added in 72328dc was meant to work around a case where TMModel.SetGrammar early-returns on same-grammar calls and leaves the model without any invalid lines to tokenize. After looking into it, that path only mattered because the underlying tokenizer-thread race in TextMateSharp could leave the model un-tokenized in the first place. TextMateSharp 2.0.4 rewrites the tokenizer thread lifecycle and explicitly fixes those races (danipen/TextMateSharp PRs AvaloniaUI#114, AvaloniaUI#115, AvaloniaUI#121), so the local InvalidateLine is no longer load-bearing. The full TextView redraw fallback in ModelTokensChanged (43c7713) is independent of TextMateSharp - it addresses a clamping bug local to AvaloniaEdit.TextMate where the redraw range collapses to an inverted interval when the visible-line cache is stale - and stays.
alvaro-berruezo-unity3d
changed the title
Contributor
Author
|
TextMateSharp 2.0.4 removes the need for one of the fixes on the PR. The I updated the changes, leaving only the necessary fix |
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
3 participants
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.
Summary
TextMateColoringTransformer.ModelTokensChangedclamps its partial redraw range against the cached visible-line indices. WhenSetModelresets those indices to(-1, -1)and_areVisualLinesValidtofalse, any tokenizer event that arrives before the first layout pass refills the cache produces an inverted redraw range._textView.Redraw(offset, length)is then called with a negative length and paints nothing, so the newly-tokenized lines stay unstyled until the next layout pass eventually triggers a fresh render (typically a scroll or a click).This PR detects that state — either
areVisualLinesValidwasfalseat capture time, or the clamped range came out inverted — and falls back to a full_textView.Redraw()instead of the broken partial one. The fallback is slightly heavier but only fires in the narrow window before the first layout pass; the partial-redraw path still runs in the common case.How this was found
We reproduce this in a Unity Version Control diff viewer that hosts AvaloniaEdit.TextMate. Under rapid file switching the cache is stale frequently enough that the natural race produced an observable "missing syntax highlighting on one side of the diff" symptom; instrumentation logs showed ~160 invalid-range redraw events per session before the fix. A deterministic synthesis that suppresses
TextView_VisualLinesChanged's cache update on one side reproduces the failure 100% — with this PR's fallback applied, the same synthesis renders both sides correctly highlighted.Edit history
An earlier version of this PR also included a defensive
_model.InvalidateLine(0)after_model.SetGrammar(grammar)inTextMateColoringTransformer.SetGrammar, intended to work aroundTMModel.SetGrammar's same-grammar early-return. After looking into TextMateSharp 2.0.4 more carefully, that workaround turned out to be unnecessary: the underlying tokenizer-thread race it was defending against is explicitly fixed by 2.0.4'sTokenizerThreadrewrite (danipen/TextMateSharp PRs #114, #115, #121). That commit has been reverted in this branch; only the redraw-range fallback remains.Notes
AvaloniaEdit.TextMateand independent of any TextMateSharp version — applies to all consumers regardless of whichTextMateSharppackage they're on.TextMateInstallationTestscurrently covers disposal semantics, but exercising the redraw clamping deterministically would require a fairly elaborate harness (a real grammar, a tokenizer thread, a controlled layout-pass scheduler). Happy to add unit tests if there's a pattern the maintainers prefer.masterhead.