Skip to content

fix: scroll to the first change using the real text layout in diff view - #2700

Closed
weiningwei wants to merge 1 commit into
sourcegit-scm:developfrom
weiningwei:fix/diff-autoscroll-first-change
Closed

fix: scroll to the first change using the real text layout in diff view#2700
weiningwei wants to merge 1 commit into
sourcegit-scm:developfrom
weiningwei:fix/diff-autoscroll-first-change

Conversation

@weiningwei

@weiningwei weiningwei commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Problem

With "Line Word Wrap" enabled, opening a changed file does not scroll to the first change — the view stays at the top of the diff, above the change.

AutoScrollToFirstChange() estimates the offset as lineHeight * (curBlock.Start - 1), which assumes one visual row per line. Wrapped long lines break that assumption, so the estimate is several times too small: the view either stays at the top of the diff or lands short of the change, instead of centering it.

Before / After

src/Resources/Icons.axaml from 90d9e2ddc2, word wrap enabled:

Before — the view stays at the top of the diff, the first change is out of view:

before

After — the first change is brought into view (block indicator 1/1):

after

Fix

Offset calculation — ask the real text layout instead of estimating: build the visual lines above the target so their wrapped heights are recorded in the height tree, then use VisualLine.VisualTop. Lines already built by previous layouts are reused, so only the not-yet-built ones cost anything.

Timing — the calculation now depends on the document and the layout, so:

  • It used to run from OnDataContextChanged(), before the subclass writes the new text, which means it measured a stale document. It is now scheduled by the subclasses right after the text is written.
  • It is dispatched at DispatcherPriority.Background, so it runs after the layout pass, when the document, the scroll extent and the wrapped heights are all up-to-date.

Notes

  • The vOffset >= 0 guard is kept: no scrolling when the first change is already in the top half of the viewport.
  • ScrollToLine() was avoided — its 0.3 viewport minimum-scroll threshold is meant for caret following and would swallow legitimate jump corrections.

love-linger added a commit that referenced this pull request Sep 13, 2026
… change (#2700)

Signed-off-by: leo <longshuang@msn.cn>
@love-linger

Copy link
Copy Markdown
Collaborator

I've pushed my fix for this BUG which is simpler and faster

@weiningwei
weiningwei deleted the fix/diff-autoscroll-first-change branch September 13, 2026 02:38
@weiningwei

weiningwei commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

@love-linger Thanks for the simplification — but unfortunately it doesn't work on some case: maybe the modifyed line is too long?

Environment / repro

  • Windows 11, built from develop @ b5bb347620
  • "Line Word Wrap" enabled
  • Repo: https://github.com/tw93/Pake
    • c7a310ff4c925892979e6f954b05dbab8d30eed9
case1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants