Revert "Hold the placeholder until first paint when restoring a scroll position"

This reverts the holdViewLines flicker patch (Race A). It is about to be
superseded by rendering a re-render into an off-screen buffer and swapping
it in atomically, which keeps the displayed buffer (and so every reader)
untouched until the new content is ready — a cleaner mechanism than
suppressing the view-line rebuild. Removing it on its own keeps that
upcoming change focused.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-06-09 11:21:37 +02:00
parent 0d7d16df19
commit 45af0aac30
5 changed files with 0 additions and 87 deletions

View file

@ -110,21 +110,6 @@ type View struct {
// ignore them; this is the count of the entries they may trust.
freshViewLineCount int
// While true, refreshViewLinesIfNeeded leaves the current view lines in
// place instead of rebuilding them from the (changing) buffer, so the view
// keeps drawing the previous render in full. This is a stronger form of the
// viewLines retention described above: rather than revealing new content
// line-by-line as it loads, it holds the *whole* previous render until told
// to release. It is used when re-rendering content the view is already
// scrolled into (escaping to a focused main view): the placeholder left in
// the view is held, coherent, at its scroll until the re-render task's first
// paint scrolls to the saved position and releases the hold, swapping to the
// loaded content in one step — so a layout pass landing mid-load can't draw
// half-loaded content at the not-yet-restored scroll. While held, the view
// lines are a placeholder that need not match the buffer, so the
// view-line→buffer-line mapping reports no result.
holdViewLines bool
// writeMutex protects locks the write process
writeMutex sync.Mutex
@ -1254,18 +1239,6 @@ func (v *View) FlushStaleCells() {
v.clearViewLines()
}
// SetHoldViewLines controls whether refreshViewLinesIfNeeded holds the current
// view lines instead of rebuilding them from the buffer (see holdViewLines). A
// re-render task sets it while restoring a scroll position and releases it at
// its first paint, so the view keeps showing the coherent placeholder until the
// loaded content is shown at the restored scroll in one step.
func (v *View) SetHoldViewLines(hold bool) {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
v.holdViewLines = hold
}
func (v *View) rewind() {
v.buf.ei.reset()
v.buf.ei.resetScreenCursor()
@ -1487,10 +1460,6 @@ func (v *View) refreshViewLinesIfNeeded() {
return
}
if v.holdViewLines {
return
}
maxX := v.InnerWidth()
wrap := 0
if v.Wrap {
@ -1785,13 +1754,6 @@ func (v *View) BufferLineForViewLine(y int) (int, bool) {
func (v *View) bufferLineForViewLine(y int) (int, bool) {
v.refreshViewLinesIfNeeded()
// While holding the previous render (see holdViewLines), the displayed view
// lines are a placeholder that need not correspond to the loading buffer, so
// there is no valid mapping to return.
if v.holdViewLines {
return 0, false
}
// Bound on freshViewLineCount rather than len(v.viewLines): the entries past
// it are a stale tail retained for flicker-avoidance (see freshViewLineCount)
// and don't correspond to the current buffer. Within the fresh range every

View file

@ -233,36 +233,6 @@ func TestBufferLineForViewLineStaleTail(t *testing.T) {
assert.False(t, ok)
}
// While holding the view lines, the view keeps drawing the previous render even
// as the buffer is overwritten, so a re-render that is restoring a scroll
// position can keep showing the coherent placeholder until its first paint
// reveals the loaded content in one step. See View.holdViewLines.
func TestHoldViewLines(t *testing.T) {
v := NewView("name", 0, 0, 80, 10, OutputNormal)
v.writeString("a\nb\nc")
assert.Equal(t, []string{"a", "b", "c"}, v.ViewBufferLines())
v.SetHoldViewLines(true)
// Re-render the flicker-avoidance way (rewind, then overwrite from the top).
v.Reset()
v.writeString("w\nx\ny\nz")
// The new content is in the buffer, but while held the view keeps showing the
// previous render, and the view-line→buffer-line mapping reports no result.
assert.Equal(t, []string{"a", "b", "c"}, v.ViewBufferLines())
_, ok := v.BufferLineForViewLine(0)
assert.False(t, ok)
// Releasing the hold reveals the loaded content.
v.SetHoldViewLines(false)
assert.Equal(t, []string{"w", "x", "y", "z"}, v.ViewBufferLines())
bufferLine, ok := v.BufferLineForViewLine(0)
assert.True(t, ok)
assert.Equal(t, 0, bufferLine)
}
func TestContainsColoredText(t *testing.T) {
hexColor := func(text string, hexStr string) []cell {
cells := make([]cell, len(text))

View file

@ -83,14 +83,6 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
// its first paint.
targetOriginY := gui.getManager(view).GetScrollToOriginYForNextTask()
// While restoring a scroll position, hold the placeholder currently in the
// view until the first paint (released by ApplyInitialScroll), so a layout
// pass landing mid-load can't draw half-loaded content at the not-yet-restored
// scroll. Set synchronously here (before the layout pass that drains
// afterLayout) for the same reason StartLoading is. A render without a scroll
// to restore clears any leftover hold.
view.SetHoldViewLines(targetOriginY != nil)
// Run the pty after layout so that it gets the correct size
gui.afterLayout(func() error {
// Need to get the width and the pager command again because the layout might have

View file

@ -35,13 +35,6 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
// The task clears the request and suppresses the origin reset when it starts.
targetOriginY := manager.GetScrollToOriginYForNextTask()
// While restoring a scroll position, hold the placeholder currently in the
// view until the first paint, so a layout pass landing mid-load can't draw
// half-loaded content at the not-yet-restored scroll. Released by the first
// paint (ApplyInitialScroll). A render without a scroll to restore clears any
// leftover hold so its content reveals normally.
view.SetHoldViewLines(targetOriginY != nil)
var r io.ReadCloser
start := func() (tasks.Cmd, io.Reader) {
view.SetContentWidth(contentWidth)

View file

@ -33,10 +33,6 @@ func (gui *Gui) linesToReadFromCmdTask(v *gocui.View, targetOriginY *int) tasks.
oy = *targetOriginY
applyInitialScroll = func() {
v.SetOrigin(v.OriginX(), *targetOriginY)
// Release the placeholder hold (set when this scroll-restore task
// started): the next refresh now reveals the loaded content at the
// restored scroll in one step. See View.holdViewLines.
v.SetHoldViewLines(false)
}
}