From cbf220c49781bb78f96820b24b68b8e76a3d025b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 8 Jul 2026 19:12:49 +0200 Subject: [PATCH] Read lines based on scroll position instead of a fixed per-notch delta When scrolling a lazy-loaded view (a diff in the main view, the command log, etc.), we top up the view's line buffer by reading more lines from the still-running task. This was driven by asking the task to read a fixed number of *additional* lines on every scroll event, which had two problems: - It was decoupled from the scroll position. Scrolling down, back up, and down again re-read lines that had already been read, so the buffer crept towards the end of the input regardless of where the user actually scrolled. - A single wheel notch only bought a single notch worth of runway, so fast scrolling constantly outran the reader and had to wait for the next read (and re-render) on every notch. Make ReadLines take an absolute target total instead of a delta: the task tracks how many lines it has read and only reads the shortfall, so requests are idempotent. Callers now ask to fill the viewport at the current scroll position plus a few screenfuls of read-ahead, which gives scrolling enough runway to stay smooth. The four call sites all wanted the same "fill this view" computation, so consolidate them into a single ReadLinesToFillView helper. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/vertical_scroll_controller.go | 8 ++----- .../controllers/view_selection_controller.go | 7 +----- pkg/gui/global_handlers.go | 8 ++----- pkg/gui/gui.go | 17 +++++++++++++ pkg/gui/gui_common.go | 4 ++++ pkg/gui/layout.go | 13 ++++++---- pkg/gui/types/common.go | 4 ++++ pkg/tasks/tasks.go | 24 +++++++++++++++---- 8 files changed, 58 insertions(+), 27 deletions(-) diff --git a/pkg/gui/controllers/vertical_scroll_controller.go b/pkg/gui/controllers/vertical_scroll_controller.go index 1db9bb76e..84e312c1d 100644 --- a/pkg/gui/controllers/vertical_scroll_controller.go +++ b/pkg/gui/controllers/vertical_scroll_controller.go @@ -66,12 +66,8 @@ func (self *VerticalScrollController) HandleScrollUp() error { } func (self *VerticalScrollController) HandleScrollDown() error { - scrollHeight := self.c.UserConfig().Gui.ScrollHeight - self.context.GetViewTrait().ScrollDown(scrollHeight) - - if manager := self.c.GetViewBufferManagerForView(self.context.GetView()); manager != nil { - manager.ReadLines(scrollHeight) - } + self.context.GetViewTrait().ScrollDown(self.c.UserConfig().Gui.ScrollHeight) + self.c.ReadLinesToFillView(self.context.GetView()) return nil } diff --git a/pkg/gui/controllers/view_selection_controller.go b/pkg/gui/controllers/view_selection_controller.go index 31cbd3695..1a97a9a30 100644 --- a/pkg/gui/controllers/view_selection_controller.go +++ b/pkg/gui/controllers/view_selection_controller.go @@ -50,17 +50,12 @@ func (self *ViewSelectionController) GetMouseKeybindings(opts types.KeybindingsO } func (self *ViewSelectionController) handleLineChange(delta int) { - if delta > 0 { - if manager := self.c.GetViewBufferManagerForView(self.context.GetView()); manager != nil { - manager.ReadLines(delta) - } - } - v := self.Context().GetView() if delta < 0 { v.ScrollUp(-delta) } else { v.ScrollDown(delta) + self.c.ReadLinesToFillView(v) } } diff --git a/pkg/gui/global_handlers.go b/pkg/gui/global_handlers.go index 3c4896af4..a5e59a84e 100644 --- a/pkg/gui/global_handlers.go +++ b/pkg/gui/global_handlers.go @@ -17,12 +17,8 @@ func (gui *Gui) scrollUpView(view *gocui.View) { } func (gui *Gui) scrollDownView(view *gocui.View) { - scrollHeight := gui.c.UserConfig().Gui.ScrollHeight - view.ScrollDown(scrollHeight) - - if manager := gui.getViewBufferManagerForView(view); manager != nil { - manager.ReadLines(scrollHeight) - } + view.ScrollDown(gui.c.UserConfig().Gui.ScrollHeight) + gui.readLinesToFillView(view) } func (gui *Gui) scrollUpMain() error { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index d77673e9c..1e6e8f9c3 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -691,6 +691,23 @@ func (gui *Gui) getViewBufferManagerForView(view *gocui.View) *tasks.ViewBufferM return manager } +// When scrolling a lazy-loaded view, we read enough lines to fill the viewport +// plus this many extra screenfuls, so that further scrolling has some runway +// and doesn't have to block on reading (and re-rendering) more lines on every +// wheel notch. +const scrollReadAheadScreenfuls = 3 + +// readLinesToFillView reads enough lines into the view's buffer to cover +// everything currently scrolled into view, plus a few screenfuls of read-ahead. +// Reading is idempotent (see ViewBufferManager.ReadLines), so if the buffer +// already extends far enough this does nothing. +func (gui *Gui) readLinesToFillView(view *gocui.View) { + if manager := gui.getViewBufferManagerForView(view); manager != nil { + viewportBottom := view.OriginY() + view.InnerHeight() + manager.ReadLines(viewportBottom + scrollReadAheadScreenfuls*view.InnerHeight()) + } +} + func (gui *Gui) initialWindowViewNameMap(contextTree *context.ContextTree) *utils.ThreadSafeMap[string, string] { result := utils.NewThreadSafeMap[string, string]() diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index c8de7545d..69ec44781 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -165,6 +165,10 @@ func (self *guiCommon) GetViewBufferManagerForView(view *gocui.View) *tasks.View return self.gui.getViewBufferManagerForView(view) } +func (self *guiCommon) ReadLinesToFillView(view *gocui.View) { + self.gui.readLinesToFillView(view) +} + func (self *guiCommon) State() types.IStateAccessor { return self.gui.stateAccessor } diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 6f7dc7187..de3bdbe9b 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -37,13 +37,18 @@ func (gui *Gui) layout(g *gocui.Gui) error { if prevMainView != nil { prevMainHeight := prevMainView.Height() newMainHeight := viewDimensions["main"].Y1 - viewDimensions["main"].Y0 + 1 - heightDiff := newMainHeight - prevMainHeight - if heightDiff > 0 { + if newMainHeight > prevMainHeight { + // The main views have grown taller, so make sure enough lines are + // loaded to fill them. The views haven't been resized yet at this + // point, so we can't rely on their current height; compute the target + // total from the new height instead. (Reading past the actual content + // is harmless: ReadLines stops at the end of input.) + linesToRead := prevMainView.OriginY() + newMainHeight if manager := gui.getViewBufferManagerForView(gui.Views.Main); manager != nil { - manager.ReadLines(heightDiff) + manager.ReadLines(linesToRead) } if manager := gui.getViewBufferManagerForView(gui.Views.Secondary); manager != nil { - manager.ReadLines(heightDiff) + manager.ReadLines(linesToRead) } } } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 4ced8bd79..08ff53bb0 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -58,6 +58,10 @@ type IGuiCommon interface { // return the view buffer manager for the given view, or nil if it doesn't have one GetViewBufferManagerForView(view *gocui.View) *tasks.ViewBufferManager + // read enough lines into the given view's buffer to fill it at its current + // scroll position, plus some read-ahead for smooth scrolling + ReadLinesToFillView(view *gocui.View) + // returns true if command completed successfully RunSubprocess(cmdObj *oscommands.CmdObj) (bool, error) RunSubprocessAndRefresh(*oscommands.CmdObj) error diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 26145c784..bc08013ed 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -82,7 +82,11 @@ type ViewBufferManager struct { } type LinesToRead struct { - // Total number of lines to read + // The total number of lines the task should have read once this request is + // satisfied. This is an absolute count from the start of the task, not a + // delta: the task keeps track of how many lines it has already read and only + // reads the shortfall, so a request for a total at or below what has already + // been read reads nothing. -1 means read all the way to the end. Total int // Number of lines after which we have read enough to fill the view, and can @@ -119,10 +123,14 @@ func NewViewBufferManager( } } -func (self *ViewBufferManager) ReadLines(n int) { +// ReadLines asks the task to ensure it has read at least totalLines lines in +// total. Because the count is absolute rather than a delta, repeated requests +// (e.g. as the user scrolls down, back up, and down again) don't re-read lines +// that have already been read: the task only ever reads the shortfall. +func (self *ViewBufferManager) ReadLines(totalLines int) { if self.readLines != nil { go utils.Safe(func() { - self.readLines <- LinesToRead{Total: n, InitialRefreshAfter: -1} + self.readLines <- LinesToRead{Total: totalLines, InitialRefreshAfter: -1} }) } } @@ -283,6 +291,11 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix } } + // The total number of lines we have read so far. Requests specify an + // absolute target total (see LinesToRead.Total), so we compare against + // this to work out how many more lines, if any, we still need to read. + linesRead := 0 + outer: for { if stopped() { @@ -297,7 +310,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix linesToRead.Then() } } - for i := 0; linesToRead.Total == -1 || i < linesToRead.Total; i++ { + for linesToRead.Total == -1 || linesRead < linesToRead.Total { if stopped() { callThen() break outer @@ -331,8 +344,9 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix } writeToView(append(line, '\n')) lineWrittenChan <- struct{}{} + linesRead++ - if i+1 == linesToRead.InitialRefreshAfter { + if linesRead == linesToRead.InitialRefreshAfter { // We have read enough lines to fill the view, so do a first refresh // here to show what we have. Continue reading and refresh again at // the end to make sure the scrollbar has the right size.