mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Scan for the restore target before revealing the new content
On a position-preserving re-render the first paint swapped the off-screen content in and only then ran Apply, which for the buffer-parse backend (no pager) scans the whole diff to locate the line to land on. That scan takes tens of milliseconds on a large diff, during which the new content was already displayed at the *old* scroll position — a layout draw landing in that window showed a frame at the stale (and now out-of-range) scroll, a pronounced flicker when changing context size while scrolled down. The metadata/hyperlink backends didn't show it because they resolve the target during the load, so their Apply is instant. Let Apply own the swap: it locates the target against the still-off-screen (and, at end of input, complete) buffer first, then calls swapIn and settles the scroll. The scan now runs while the previous content is still displayed, so the new content is revealed already at the right position — matching what the early-resolving backends already did. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7f8c6f9a89
commit
fced2c616a
|
|
@ -285,18 +285,21 @@ func (self *StagingHelper) restoreDiffLinePositionOnRerender(view *gocui.View, c
|
|||
}
|
||||
return view.OffscreenLineCount() >= primaryBufferLine+view.InnerHeight()
|
||||
},
|
||||
Apply: func() {
|
||||
// The off-screen render has just been swapped in, so the displayed buffer
|
||||
// now holds it. Land on the nearest candidate it still contains: the
|
||||
// primary one if the incremental scan found it, otherwise scan the now-
|
||||
// complete content in priority order (the common path for buffer-parse,
|
||||
// which only becomes well-formed once the whole diff has loaded, at which
|
||||
// point the swap happens at end of input).
|
||||
Apply: func(swapIn func()) {
|
||||
// Find the candidate to land on, then swap the off-screen content in and
|
||||
// place it. The find runs against the *off-screen* buffer, before the swap,
|
||||
// so the (possibly whole-diff) scan happens while the previous content is
|
||||
// still displayed — otherwise the new content would be drawn at the old
|
||||
// scroll for the duration of the scan. Land on the nearest candidate the
|
||||
// content still contains: the primary one if the incremental scan found it,
|
||||
// otherwise scan the now-complete content in priority order (the common path
|
||||
// for buffer-parse, which only becomes well-formed once the whole diff has
|
||||
// loaded, so its swap is at end of input and the off-screen buffer is whole).
|
||||
matched, bufferLine := -1, primaryBufferLine
|
||||
if bufferLine != -1 {
|
||||
matched = 0
|
||||
} else {
|
||||
resolved := self.resolveDiffLines(view.DiffLineContents())
|
||||
resolved := self.resolveDiffLines(view.OffscreenDiffLineContents())
|
||||
for i, candidate := range candidates {
|
||||
if line := findResolvedDiffLine(resolved, candidate.identity, 0); line != -1 {
|
||||
matched, bufferLine = i, line
|
||||
|
|
@ -304,6 +307,9 @@ func (self *StagingHelper) restoreDiffLinePositionOnRerender(view *gocui.View, c
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
swapIn()
|
||||
|
||||
// If no candidate is there (the content changed and they're all gone),
|
||||
// leave the scroll and selection as they are rather than acting on a line
|
||||
// that no longer means what it did.
|
||||
|
|
|
|||
|
|
@ -171,20 +171,22 @@ type LinesToRead struct {
|
|||
// re-renders content the user was already looking at (returning to a focused main
|
||||
// view on escape). The render task reads the new content into an off-screen
|
||||
// buffer; RenderRestore decides, as that buffer fills, when the task has read far
|
||||
// enough to show the saved position (FirstPaintReady), and then — once the
|
||||
// off-screen buffer has been swapped in — scrolls there and restores the
|
||||
// selection (Apply). It is a predicate rather than a fixed scroll position so the
|
||||
// target can be a row matching a patch identity, located by scanning the loading
|
||||
// content, rather than a line number that the changed content may have moved.
|
||||
// enough to show the saved position (FirstPaintReady), and then locates the target
|
||||
// and reveals it (Apply). It is a predicate rather than a fixed scroll position so
|
||||
// the target can be a row matching a patch identity, located by scanning the
|
||||
// loading content, rather than a line number that the changed content may have moved.
|
||||
type RenderRestore struct {
|
||||
// FirstPaintReady reports whether the task has now read enough of the new
|
||||
// (off-screen) content to first-paint at the saved position. Evaluated after
|
||||
// each line is read.
|
||||
FirstPaintReady func() bool
|
||||
|
||||
// Apply runs once, just after the off-screen render is swapped in at the first
|
||||
// paint, to scroll to the saved position and restore the selection.
|
||||
Apply func()
|
||||
// Apply runs once at the first paint. It locates the target in the not-yet-
|
||||
// displayed off-screen content, then calls swapIn to promote that content to
|
||||
// the display and settles the scroll/selection onto the target. Doing the
|
||||
// (potentially whole-diff) scan before swapIn keeps the previous content shown
|
||||
// while it runs, so the new content is never briefly drawn at the old scroll.
|
||||
Apply func(swapIn func())
|
||||
}
|
||||
|
||||
func (self *ViewBufferManager) GetTaskKey() string {
|
||||
|
|
@ -464,11 +466,17 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
return
|
||||
}
|
||||
painted = true
|
||||
self.swapInRender()
|
||||
if restore != nil {
|
||||
restore.Apply()
|
||||
} else if linesToRead.ResetOrigin && self.resetOrigin != nil {
|
||||
self.resetOrigin()
|
||||
// Apply locates the target in the off-screen content, swaps it in,
|
||||
// and settles the scroll in one step — so the scan it may run first
|
||||
// happens while the previous content is still displayed, never the
|
||||
// new content at the old scroll.
|
||||
restore.Apply(self.swapInRender)
|
||||
} else {
|
||||
self.swapInRender()
|
||||
if linesToRead.ResetOrigin && self.resetOrigin != nil {
|
||||
self.resetOrigin()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -174,9 +174,10 @@ func TestNewCmdTask(t *testing.T) {
|
|||
// thing we want to do with the output is count the number of lines.
|
||||
// When a RenderRestore is set, the first paint is driven by its FirstPaintReady
|
||||
// predicate rather than the InitialRefreshAfter line count, and Apply runs exactly
|
||||
// once, right after the off-screen render is swapped in. This is the read-loop
|
||||
// half of the escape restore: scroll to and select the saved position as the new
|
||||
// content first appears. See RenderRestore.
|
||||
// once. Apply controls when the off-screen render is swapped in (it calls swapIn
|
||||
// after locating the target), so its scan runs while the previous content is still
|
||||
// displayed rather than revealing the new content at the old scroll. This is the
|
||||
// read-loop half of the escape restore. See RenderRestore.
|
||||
func TestNewCmdTaskRestore(t *testing.T) {
|
||||
writer := bytes.NewBuffer(nil)
|
||||
linesWritten := func() int { return strings.Count(writer.String(), "\n") }
|
||||
|
|
@ -184,7 +185,8 @@ func TestNewCmdTaskRestore(t *testing.T) {
|
|||
swapped := false
|
||||
applyCount := 0
|
||||
applyAtLines := -1
|
||||
applyAfterSwap := true
|
||||
swappedBeforeApply := false
|
||||
swappedByApply := false
|
||||
|
||||
task := gocui.NewFakeTask()
|
||||
manager := NewViewBufferManager(
|
||||
|
|
@ -204,12 +206,15 @@ func TestNewCmdTaskRestore(t *testing.T) {
|
|||
restore := &RenderRestore{
|
||||
// Ready once five lines have loaded — well before InitialRefreshAfter (30).
|
||||
FirstPaintReady: func() bool { return linesWritten() >= 5 },
|
||||
Apply: func() {
|
||||
Apply: func(swapIn func()) {
|
||||
applyCount++
|
||||
applyAtLines = linesWritten()
|
||||
if !swapped {
|
||||
applyAfterSwap = false
|
||||
// The render must not be swapped in until Apply asks for it.
|
||||
if swapped {
|
||||
swappedBeforeApply = true
|
||||
}
|
||||
swapIn()
|
||||
swappedByApply = swapped
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -230,7 +235,8 @@ func TestNewCmdTaskRestore(t *testing.T) {
|
|||
wg.Wait()
|
||||
|
||||
assert.Equal(t, 1, applyCount, "Apply should run exactly once")
|
||||
assert.True(t, applyAfterSwap, "Apply should run after the off-screen render is swapped in")
|
||||
assert.False(t, swappedBeforeApply, "the off-screen render should not be swapped in before Apply runs")
|
||||
assert.True(t, swappedByApply, "Apply should swap the off-screen render in via swapIn")
|
||||
// The first paint was driven by FirstPaintReady (>=5 lines), not by
|
||||
// InitialRefreshAfter (30).
|
||||
assert.GreaterOrEqual(t, applyAtLines, 5)
|
||||
|
|
|
|||
Loading…
Reference in a new issue