Skip to content

Commit e5443c1

Browse files
geevensinghCopilot
andcommitted
Fix inline-mode click-to-jump jiggle via predicted-settle ghost
In inline mode the editor's ExtentHeight covers the inline document (including deletion lines), but the overview bar paints the viewport band from the right/left source-line counts. Sending raction * ExtentHeight to the editor and pinning the ghost at the cursor's bar position therefore landed at two different coordinate systems: the editor settled at one inline line, while the bar's band repainted at the corresponding RightFirstLine, which could be 25-30 px off the cursor in a mid-sized file with deletions interleaved. Add DiffPaneViewModel.PredictBandTopFractionForScroll, which walks InlineLineToSourceLines forward from the predicted first inline line to find what (LeftFirstLine, RightFirstLine) the bar will paint after settle, and returns min(leftFrac, rightFrac) to match how the geometry layer's GetBandTopY picks the band's top. The bar uses this prediction to place the ghost where the band will actually settle, so MouseDown -> MouseUp -> editor settle all land on the same pixel. In side-by-side mode the prediction is identity and behaviour is unchanged. Remove the deferred ghost-clear scaffolding (_ghostClearPending, _ghostClearTimer, GhostClearTimeout, EnsureGhostClearTimer, OnGhostClearTimerTick, the pending-clear branch in OnMouseLeftButtonDown, the deferred-clear path in ResetInteraction): it was a 250 ms timer-based workaround for this same root cause and is dead weight now that the ghost and settled positions coincide. Restore the empty-space click path to set the ghost via EmitDragScroll, replacing the previous skip-ghost workaround (ef8757e) that pre-emptively hid the band on click to mask the same jiggle. Four new unit tests cover the prediction directly via the internal static helper: side-by-side identity, empty-map passthrough, the pure-delete-in-middle case (validating min(leftFrac, rightFrac)), walking past a leading delete to find the first non-null newLine, and NaN / out-of-range input normalization. AI-Local-Session: b53d5fe5-38cc-41dd-b987-3def5ef3f0d3 AI-Cloud-Session: 744dd836-01e5-48af-91db-daa8241a01d7 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ef8757e commit e5443c1

4 files changed

Lines changed: 254 additions & 52 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,14 @@ body. Keep section headings exact and write notes in Markdown.
4141
indicator) recenters the indicator on the cursor and starts a
4242
drag in the same gesture — minimap-style "click anywhere to jump
4343
there", and a press-and-drag from empty space scrubs without
44-
needing a second click. Empty-space clicks land the indicator at
45-
its actual line-snapped position immediately rather than locking
46-
to the cursor on press and snapping on release — no more jiggle
47-
when the press and release land at the same cursor position but
48-
the editor's per-line scroll granularity doesn't divide the bar's
49-
pixel height evenly.
44+
needing a second click. The indicator lands at the editor's
45+
natural settle position for the click — in side-by-side mode that
46+
always equals the cursor position, in inline mode it can be
47+
slightly offset because the inline document interleaves
48+
deletion lines (which advance the editor's scroll but not the
49+
right-side line count); picking the post-settle position up
50+
front keeps the indicator stable across MouseUp instead of
51+
visibly jumping when the editor's viewport catches up.
5052
- Language-aware syntax highlighting for TypeScript (`.ts`, `.tsx`),
5153
YAML (`.yaml`, `.yml`), Go (`.go`), Rust (`.rs`), Ruby (`.rb`),
5254
Bash / shell (`.sh`, `.bash`, `.zsh`), and TOML (`.toml`). Combined

DiffViewer.Tests/ViewModels/DiffPaneViewModelTests.cs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,4 +1735,117 @@ public ImageDecodeResult Decode(byte[] bytes, string? path)
17351735
return DecodeFunc(bytes, path);
17361736
}
17371737
}
1738+
1739+
[Fact]
1740+
public void PredictBandTopFraction_SideBySide_ReturnsScrollFractionVerbatim()
1741+
{
1742+
// In side-by-side mode the right editor's ExtentHeight maps 1:1
1743+
// to RightDocument.LineCount * lineHeight, so the predicted
1744+
// band-top fraction equals the scroll fraction. The inline map
1745+
// is irrelevant for this mode.
1746+
var emptyMap = Array.Empty<(int?, int?)>();
1747+
DiffPaneViewModel.PredictBandTopFraction(
1748+
scrollFraction: 0.42,
1749+
map: emptyMap,
1750+
leftTotal: 100,
1751+
rightTotal: 100,
1752+
isSideBySide: true)
1753+
.Should().Be(0.42);
1754+
}
1755+
1756+
[Fact]
1757+
public void PredictBandTopFraction_Inline_EmptyMap_ReturnsScrollFractionVerbatim()
1758+
{
1759+
// Before the inline diff is built (e.g. during initial load) the
1760+
// map is empty and there's nothing to project through; pass the
1761+
// scroll fraction through so the bar still has a sensible ghost
1762+
// position instead of collapsing to 0 or 1.
1763+
DiffPaneViewModel.PredictBandTopFraction(
1764+
scrollFraction: 0.7,
1765+
map: Array.Empty<(int?, int?)>(),
1766+
leftTotal: 50,
1767+
rightTotal: 50,
1768+
isSideBySide: false)
1769+
.Should().Be(0.7);
1770+
}
1771+
1772+
[Fact]
1773+
public void PredictBandTopFraction_Inline_PureDeleteInMiddle_ShiftsAwayFromCursor()
1774+
{
1775+
// Left: a b c d e (5 lines) Right: a c d e (4 lines)
1776+
// Inline mapping (1-indexed inline line):
1777+
// 1: a (1, 1)
1778+
// 2: -b (2, null)
1779+
// 3: c (3, 2)
1780+
// 4: d (4, 3)
1781+
// 5: e (5, 4)
1782+
// Scroll fraction 0.4 of inlineTotal=5 -> firstInlineIndex=2,
1783+
// i.e. inline line 3 ("c") at top. That maps to leftLine=3,
1784+
// rightLine=2, so leftFrac=2/5=0.4, rightFrac=1/4=0.25, and
1785+
// the predicted band-top fraction is min(0.4, 0.25) = 0.25.
1786+
// (Without this prediction the bar would ghost at 0.4 and then
1787+
// visibly jump down to 0.25 on settle — the inline-mode jiggle.)
1788+
var map = new (int? OldLine, int? NewLine)[]
1789+
{
1790+
(1, 1),
1791+
(2, null),
1792+
(3, 2),
1793+
(4, 3),
1794+
(5, 4),
1795+
};
1796+
DiffPaneViewModel.PredictBandTopFraction(
1797+
scrollFraction: 0.4,
1798+
map: map,
1799+
leftTotal: 5,
1800+
rightTotal: 4,
1801+
isSideBySide: false)
1802+
.Should().BeApproximately(0.25, 1e-9);
1803+
}
1804+
1805+
[Fact]
1806+
public void PredictBandTopFraction_Inline_FirstInlineLineIsDelete_WalksPastIt()
1807+
{
1808+
// Inline line 1 is a pure delete (oldLine present, newLine null).
1809+
// The prediction must walk forward to find the first non-null
1810+
// newLine — otherwise rightFrac would default to 1.0 (the
1811+
// "missing" sentinel) and the predicted band-top would collapse
1812+
// to leftFrac, which would visibly skew the ghost.
1813+
var map = new (int? OldLine, int? NewLine)[]
1814+
{
1815+
(1, null), // pure delete at the top of the inline doc
1816+
(2, 1), // first context line: leftLine=2, rightLine=1
1817+
(3, 2),
1818+
(4, 3),
1819+
};
1820+
// scrollFraction=0 -> firstInlineIndex=0. Walk: map[0] gives
1821+
// predictedLeft=1, predictedRight=null. map[1] gives
1822+
// predictedRight=1. leftFrac=(1-1)/4=0, rightFrac=(1-1)/3=0.
1823+
// min(0,0) = 0.
1824+
DiffPaneViewModel.PredictBandTopFraction(
1825+
scrollFraction: 0.0,
1826+
map: map,
1827+
leftTotal: 4,
1828+
rightTotal: 3,
1829+
isSideBySide: false)
1830+
.Should().Be(0.0);
1831+
}
1832+
1833+
[Theory]
1834+
[InlineData(double.NaN, 0.0)]
1835+
[InlineData(-0.5, 0.0)]
1836+
[InlineData(1.5, 1.0)]
1837+
public void PredictBandTopFraction_ClampsAndNormalizesInput(
1838+
double scrollFraction, double expectedNormalized)
1839+
{
1840+
// Out-of-range / NaN inputs are normalized before any further
1841+
// logic. In side-by-side mode that normalized value is returned
1842+
// verbatim, giving us a tidy way to assert the normalization.
1843+
DiffPaneViewModel.PredictBandTopFraction(
1844+
scrollFraction: scrollFraction,
1845+
map: Array.Empty<(int?, int?)>(),
1846+
leftTotal: 10,
1847+
rightTotal: 10,
1848+
isSideBySide: true)
1849+
.Should().Be(expectedNormalized);
1850+
}
17381851
}

DiffViewer/Rendering/HunkOverviewBar.cs

Lines changed: 49 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,24 @@ private enum BarInteractionState
9898

9999
/// <summary>
100100
/// Optimistic band-top Y while a drag is active, in bar coords.
101-
/// Set on every MouseMove from the cursor position; cleared on drag
102-
/// end. When set, <see cref="OnRender"/> translates the computed
103-
/// viewport band so its top sits here — decouples the band's
104-
/// painted position from the editor's actual scroll, which trails
105-
/// MouseMove by a layout pass and can fall many frames behind
106-
/// under fast drag. The editor scroll still catches up via the
107-
/// normal <see cref="DiffPaneViewModel.RequestScrollByVerticalFraction"/>
101+
/// Set on every MouseMove (and on empty-space MouseDown) to the
102+
/// predicted post-settle position for the requested scroll; cleared
103+
/// on drag end. When set, <see cref="OnRender"/> translates the
104+
/// computed viewport band so its top sits here — decouples the
105+
/// band's painted position from the editor's actual scroll, which
106+
/// trails MouseMove by a layout pass and can fall many frames
107+
/// behind under fast drag. The editor scroll still catches up via
108+
/// the normal <see cref="DiffPaneViewModel.RequestScrollByVerticalFraction"/>
108109
/// path; the ghost just makes the bar's visual feedback frame-
109110
/// instant so the band feels truly stuck to the cursor.
111+
/// <para>The "predicted" qualifier matters in inline mode, where
112+
/// the editor's <c>ExtentHeight</c> covers the inline document
113+
/// (including deletion lines) but the bar paints from the right/
114+
/// left source line counts. The ghost uses
115+
/// <see cref="DiffPaneViewModel.PredictBandTopFractionForScroll"/>
116+
/// so it lands at the same position the band will paint after
117+
/// settle, eliminating the visible snap when the ghost is later
118+
/// cleared.</para>
110119
/// </summary>
111120
private double? _dragGhostBandTop;
112121

@@ -363,26 +372,13 @@ protected override void OnMouseLeftButtonDown(MouseButtonEventArgs e)
363372
bool inBand = band is not null && HunkOverviewBarGeometry.IsInsideBand(band, p);
364373

365374
// Empty press: minimap-style "click anywhere to jump there".
366-
// Send the scroll request immediately so the band lands at the
367-
// cursor, but do NOT set the optimistic ghost band — let the
368-
// natural ViewportState propagation paint the band at its
369-
// actual settled (line-snapped) position on the next frame.
370-
// Using the ghost path here would lock the band visually to
371-
// the cursor's exact Y, then snap to the line-snapped position
372-
// on MouseUp when the ghost clears — visible as a jiggle on
373-
// smaller files where one line of editor scroll spans several
374-
// bar pixels. The ghost's value-add is smoothing fast drag
375-
// (decoupling the bar paint from editor scroll lag); a
376-
// stationary click has nothing to smooth.
377-
//
378-
// If the user follows up with motion, the first MouseMove
379-
// activates the ghost via EmitDragScroll and we're back on
380-
// the smooth-drag path. The transition is a small one-time
381-
// snap to cursor, masked by the motion that caused it.
382-
//
383-
// Still go directly into Dragging (skipping PendingClick) so
384-
// MouseUp doesn't try to fire a JumpToHunk — there's no hunk
385-
// to commit to, and the scroll has already been sent.
375+
// Set the ghost at the predicted post-settle position so the
376+
// band feels stuck to the press, send the scroll request, and
377+
// enter Dragging so the same gesture can scrub by moving the
378+
// cursor. ResetInteraction clears the ghost on MouseUp; since
379+
// the ghost is at the predicted-settle position (not the raw
380+
// cursor), the clear lands on the same frame the natural band
381+
// repaints at that same position — no visible snap.
386382
if (idx < 0 && !inBand)
387383
{
388384
if (band is null) return;
@@ -393,9 +389,7 @@ protected override void OnMouseLeftButtonDown(MouseButtonEventArgs e)
393389
_interaction = BarInteractionState.Dragging;
394390
ToolTip = null;
395391
CaptureMouse();
396-
double targetBandTopY = p.Y - _bandDragOffsetFromTop;
397-
double fraction = ActualHeight <= 0 ? 0 : targetBandTopY / ActualHeight;
398-
_vm.RequestScrollByVerticalFraction(fraction);
392+
EmitDragScroll(p);
399393
e.Handled = true;
400394
return;
401395
}
@@ -459,10 +453,6 @@ private void ResetInteraction()
459453
if (IsMouseCaptured) ReleaseMouseCapture();
460454
if (_dragGhostBandTop is not null)
461455
{
462-
// Clear the ghost first, then invalidate so the next paint
463-
// shows the band where the editor actually settled. If the
464-
// editor hasn't quite caught up yet, the band may snap by a
465-
// pixel or two — fine, drag is over.
466456
_dragGhostBandTop = null;
467457
InvalidateVisual();
468458
}
@@ -578,20 +568,33 @@ private void EmitDragScroll(Point p)
578568
if (_vm is null) return;
579569
double targetBandTopY = p.Y - _bandDragOffsetFromTop;
580570

581-
// Paint the band at the cursor on the very next render tick,
582-
// without waiting for the editor's scroll → layout → ScrollChanged
583-
// → ViewportState → PropertyChanged round-trip. Without this
584-
// decoupling, the band visibly trails the cursor by however long
585-
// the editor takes to settle its layout pass; on large files
586-
// that's enough to drop the band several frames behind a fast
587-
// drag, which reads as the band "sticking" to lower-than-cursor
588-
// positions. The editor still catches up via the normal scroll
589-
// path below; the ghost just makes the bar's visual frame-instant.
590-
_dragGhostBandTop = ClampGhostTop(targetBandTopY);
571+
// Paint the band at the predicted post-settle position on the
572+
// very next render tick, without waiting for the editor's scroll
573+
// → layout → ScrollChanged → ViewportState → PropertyChanged
574+
// round-trip. Without this decoupling, the band visibly trails
575+
// the cursor by however long the editor takes to settle its
576+
// layout pass; on large files that's enough to drop the band
577+
// several frames behind a fast drag, which reads as the band
578+
// "sticking" to lower-than-cursor positions.
579+
//
580+
// In side-by-side mode the editor's scroll fraction maps 1:1 to
581+
// the bar's band-top fraction, so the predicted position equals
582+
// <c>targetBandTopY</c> and the band lands under the cursor.
583+
// In inline mode the editor's extent is the inline document
584+
// (which includes deletion lines) while the bar paints from the
585+
// right/left source line counts; the prediction (via the VM's
586+
// <see cref="DiffPaneViewModel.PredictBandTopFractionForScroll"/>)
587+
// walks the inline-to-source map to find where the band will
588+
// actually settle. Setting the ghost to the predicted position
589+
// (instead of the cursor) ensures there's no visible jump when
590+
// the editor's Viewport state catches up.
591+
double rawFraction = ActualHeight <= 0 ? 0 : targetBandTopY / ActualHeight;
592+
double predictedFraction = _vm.PredictBandTopFractionForScroll(rawFraction);
593+
double predictedBandTopY = predictedFraction * ActualHeight;
594+
_dragGhostBandTop = ClampGhostTop(predictedBandTopY);
591595
InvalidateVisual();
592596

593-
double fraction = ActualHeight <= 0 ? 0 : targetBandTopY / ActualHeight;
594-
_vm.RequestScrollByVerticalFraction(fraction);
597+
_vm.RequestScrollByVerticalFraction(rawFraction);
595598
}
596599

597600
/// <summary>

DiffViewer/ViewModels/DiffPaneViewModel.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,90 @@ public void RequestScrollByVerticalFraction(double fraction)
11981198
this, new ScrollByVerticalFractionRequestedEventArgs(fraction));
11991199
}
12001200

1201+
/// <summary>
1202+
/// Predict the viewport-band top fraction (0..1 of bar height) the
1203+
/// overview bar will paint at after the editor settles at
1204+
/// <paramref name="scrollFraction"/>. Used by the bar to position the
1205+
/// sticky-thumb ghost so it lands at the post-settle position rather
1206+
/// than the raw cursor position — without this prediction, inline
1207+
/// mode produces a visible "jiggle" on click-to-jump because the
1208+
/// editor's ExtentHeight is based on the inline document (which
1209+
/// includes deletion lines) while the bar paints the band based on
1210+
/// <c>(RightFirstLine − 1) / RightTotalLines × barHeight</c>. The two
1211+
/// coordinate systems coincide only in side-by-side mode.
1212+
///
1213+
/// <para>In side-by-side mode the right editor's extent maps 1:1 to
1214+
/// the right document's line count, so the prediction is identity.
1215+
/// In inline mode the prediction walks
1216+
/// <see cref="InlineLineToSourceLines"/> forward from the predicted
1217+
/// first inline line to find what
1218+
/// <c>(LeftFirstLine, RightFirstLine)</c> will be after settle, then
1219+
/// returns <c>min(leftFrac, rightFrac)</c> to match how
1220+
/// <c>HunkOverviewBarGeometry.GetBandTopY</c> picks the band's top.
1221+
/// </para>
1222+
/// </summary>
1223+
public double PredictBandTopFractionForScroll(double scrollFraction) =>
1224+
PredictBandTopFraction(
1225+
scrollFraction,
1226+
InlineLineToSourceLines,
1227+
LeftDocument.LineCount,
1228+
RightDocument.LineCount,
1229+
IsSideBySide);
1230+
1231+
/// <summary>
1232+
/// Pure-function core of <see cref="PredictBandTopFractionForScroll"/>
1233+
/// for unit-testing without spinning up an editor or loading a real
1234+
/// diff. NaN and out-of-range inputs are normalized to <c>[0, 1]</c>.
1235+
/// Returns <paramref name="scrollFraction"/> verbatim when the map
1236+
/// is empty (no inline diff loaded yet) or when
1237+
/// <paramref name="isSideBySide"/> is true.
1238+
/// </summary>
1239+
internal static double PredictBandTopFraction(
1240+
double scrollFraction,
1241+
IReadOnlyList<(int? OldLine, int? NewLine)> map,
1242+
int leftTotal,
1243+
int rightTotal,
1244+
bool isSideBySide)
1245+
{
1246+
if (double.IsNaN(scrollFraction)) scrollFraction = 0;
1247+
if (scrollFraction < 0) scrollFraction = 0;
1248+
if (scrollFraction > 1) scrollFraction = 1;
1249+
1250+
if (isSideBySide) return scrollFraction;
1251+
if (map is null || map.Count == 0) return scrollFraction;
1252+
1253+
int safeLeftTotal = Math.Max(1, leftTotal);
1254+
int safeRightTotal = Math.Max(1, rightTotal);
1255+
int inlineTotal = map.Count;
1256+
1257+
int firstInlineIndex = (int)Math.Floor(scrollFraction * inlineTotal);
1258+
if (firstInlineIndex < 0) firstInlineIndex = 0;
1259+
if (firstInlineIndex >= inlineTotal) firstInlineIndex = inlineTotal - 1;
1260+
1261+
int? predictedLeft = null;
1262+
int? predictedRight = null;
1263+
for (int i = firstInlineIndex; i < inlineTotal; i++)
1264+
{
1265+
var (oldLn, newLn) = map[i];
1266+
if (predictedLeft is null && oldLn is int o) predictedLeft = o;
1267+
if (predictedRight is null && newLn is int n) predictedRight = n;
1268+
if (predictedLeft is not null && predictedRight is not null) break;
1269+
}
1270+
1271+
if (predictedLeft is null && predictedRight is null)
1272+
{
1273+
return scrollFraction;
1274+
}
1275+
1276+
double leftFrac = predictedLeft is int pl
1277+
? (double)(pl - 1) / safeLeftTotal
1278+
: 1.0;
1279+
double rightFrac = predictedRight is int pr
1280+
? (double)(pr - 1) / safeRightTotal
1281+
: 1.0;
1282+
return Math.Min(leftFrac, rightFrac);
1283+
}
1284+
12011285
public void Dispose()
12021286
{
12031287
if (_settingsService is not null)

0 commit comments

Comments
 (0)