Improve inline spinner performance (#5592)

As a followup to #5571, improve performance for inline spinners too.

This requires a bit of preparation; in particular, we change
ListContextTrait.HandleRender to no longer force a UI layout/redraw, so
callers need to take care of this themselves where needed. This requires
careful testing to see that we didn't miss any situation where redrawing
only worked by accident; see the first commit of the branch for an
example.

pkg/gocui/flush_test.go

@@ -1,6 +1,7 @@
 package gocui
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -200,3 +201,92 @@ func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) {
 	assert.NoError(t, err)
 	assert.True(t, contentOnly, "should return true when no events are queued")
 }
+
+// Ensure an overlapping view that is not tainted does not get overdrawn
+func TestFlushContentOnly_DoesNotOverdrawHigherZViews(t *testing.T) {
+	g := newTestGui(t)
+
+	// Base view
+	list, _ := g.SetView("list", 0, 0, 79, 23, 0)
+	list.Frame = false
+	list.SetContent(strings.Repeat("LIST LINE FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER FILLER\n", 22))
+
+	// Overlapping 'popup'
+	popup, _ := g.SetView("popup", 20, 8, 60, 16, 0)
+	popup.Frame = false
+	popupLine := strings.Repeat("P", 60)
+	popup.SetContent(strings.Repeat(popupLine+"\n", 16))
+
+	// Full flush — popup ends up on top.
+	assert.NoError(t, g.flush())
+
+	cellAt := func(x, y int) string {
+		s, _, _ := g.screen.Get(x, y)
+		return s
+	}
+
+	// Taint only the list view
+	list.SetContent(strings.Repeat(strings.Repeat("X", 80)+"\n", 22))
+	assert.True(t, list.IsTainted(), "list should be tainted after SetContent")
+	assert.False(t, popup.IsTainted(), "popup should not be tainted")
+
+	// flushContentOnly is what spinner ticks ultimately invoke.
+	assert.NoError(t, g.flushContentOnly(g.views))
+
+	assert.Equal(t, "P", cellAt(21, 9),
+		"popup region must still show popup content after flushContentOnly; "+
+			"if this fails the popup-overdraw bug is present")
+
+	// Additional checks to be sure
+	assert.Equal(t, "P", cellAt(40, 11), "interior popup cell should still show popup content")
+	assert.Equal(t, "P", cellAt(58, 14), "near-edge popup cell should still show popup content")
+
+	// Ensure tainted view was updated
+	assert.Equal(t, "X", cellAt(5, 5), "list cell outside popup should show new list content")
+	assert.Equal(t, "X", cellAt(70, 20), "list cell outside popup should show new list content")
+}
+
+// Ensure transitive overlap: with views in z-order [a, b, c] where b overlaps a
+// and c overlaps b but c does NOT overlap a, tainting a must redraw all three —
+// otherwise b's redraw paints over c.
+func TestFlushContentOnly_RedrawsTransitivelyOverlappingViews(t *testing.T) {
+	g := newTestGui(t)
+
+	// Geometry: b straddles a and c; a and c are disjoint.
+	//   a: (0,0)-(40,10)    b: (30,5)-(60,15)    c: (50,12)-(75,20)
+	a, _ := g.SetView("a", 0, 0, 40, 10, 0)
+	a.Frame = false
+	a.SetContent(strings.Repeat(strings.Repeat("A", 60)+"\n", 20))
+
+	b, _ := g.SetView("b", 30, 5, 60, 15, 0)
+	b.Frame = false
+	b.SetContent(strings.Repeat(strings.Repeat("B", 60)+"\n", 20))
+
+	c, _ := g.SetView("c", 50, 12, 75, 20, 0)
+	c.Frame = false
+	c.SetContent(strings.Repeat(strings.Repeat("C", 60)+"\n", 20))
+
+	assert.NoError(t, g.flush())
+
+	cellAt := func(x, y int) string {
+		s, _, _ := g.screen.Get(x, y)
+		return s
+	}
+
+	// Taint only a.
+	a.SetContent(strings.Repeat(strings.Repeat("X", 60)+"\n", 20))
+	assert.True(t, a.IsTainted())
+	assert.False(t, b.IsTainted())
+	assert.False(t, c.IsTainted())
+
+	assert.NoError(t, g.flushContentOnly(g.views))
+
+	// a redrawn (direct).
+	assert.Equal(t, "X", cellAt(5, 5), "a should be redrawn (tainted)")
+	// b redrawn (overlaps a).
+	assert.Equal(t, "B", cellAt(45, 7), "b should be redrawn (overlaps a)")
+	// c redrawn transitively (overlaps b, which overlaps a). Without the
+	// transitive case, b's redraw would paint over c at this cell.
+	assert.Equal(t, "C", cellAt(55, 14),
+		"c should be redrawn transitively; if 'B' here, b's redraw painted over c")
+}

pkg/gocui/gui.go

@@ -13,7 +13,9 @@ import (
 
 	"github.com/gdamore/tcell/v3"
 	"github.com/go-errors/errors"
+	"github.com/jesseduffield/generics/set"
 	"github.com/rivo/uniseg"
+	"github.com/samber/lo"
 )
 
 // OutputMode represents an output mode, which determines how colors
@@ -1170,11 +1172,9 @@ func (g *Gui) flush() error {
 // Redraws only tainted views and skips the layout pass.
 // tcell's cell-level dirty tracking ensures only
 // actually-changed cells are emitted to the terminal.
+// Will also redraw any views that overlap tainted views
 func (g *Gui) flushContentOnly(views []*View) error {
-	for _, v := range views {
+	for _, v := range viewsToRedrawContentOnly(views) {
-		if !v.tainted {
-			continue
-		}
 		if err := g.draw(v); err != nil {
 			return err
 		}
@@ -1184,6 +1184,36 @@ func (g *Gui) flushContentOnly(views []*View) error {
 	return nil
 }
 
+func viewsToRedrawContentOnly(views []*View) []*View {
+	redrawIndexes := set.New[int]()
+
+	for i, v := range views {
+		if !v.tainted && !redrawIndexes.Includes(i) {
+			continue
+		}
+
+		redrawIndexes.Add(i)
+
+		for j, above := range views[i+1:] {
+			aboveIndex := i + 1 + j
+			if !redrawIndexes.Includes(aboveIndex) && rectsOverlap(v, above) {
+				redrawIndexes.Add(aboveIndex)
+			}
+		}
+	}
+
+	return lo.FilterMap(views, func(view *View, i int) (*View, bool) {
+		return view, redrawIndexes.Includes(i)
+	})
+}
+
+// Reports whether two views' rectangles share at least one cell.
+func rectsOverlap(a, b *View) bool {
+	ax0, ay0, ax1, ay1 := a.Dimensions()
+	bx0, by0, bx1, by1 := b.Dimensions()
+	return ax0 <= bx1 && ax1 >= bx0 && ay0 <= by1 && ay1 >= by0
+}
+
 func (g *Gui) ForceLayoutAndRedraw() error {
 	return g.flush()
 }

pkg/gui/context/list_context_trait.go

@@ -124,7 +124,6 @@ func (self *ListContextTrait) HandleRender() {
 		content := self.renderLines(-1, -1)
 		self.GetViewTrait().SetContent(content)
 	}
-	self.c.Render()
 	self.setFooter()
 }
 

pkg/gui/context/suggestions_context.go

@@ -67,10 +67,17 @@ func NewSuggestionsContext(
 }
 
 func (self *SuggestionsContext) SetSuggestions(suggestions []*types.Suggestion) {
-	self.State.Suggestions = suggestions
+	// SetSuggestions is invoked from AsyncHandler (a worker goroutine) when
-	self.SetSelection(0)
+	// the prompt input changes, as well as from prepareConfirmationPanel on
-	self.c.ResetViewOrigin(self.GetView())
+	// the UI thread. Bounce to the UI thread either way so the worker path
-	self.HandleRender()
+	// keeps flushing once HandleRender stops calling Render() itself.
+	self.c.OnUIThread(func() error {
+		self.State.Suggestions = suggestions
+		self.SetSelection(0)
+		self.c.ResetViewOrigin(self.GetView())
+		self.HandleRender()
+		return nil
+	})
 }
 
 func (self *SuggestionsContext) RefreshSuggestions() {

pkg/gui/controllers/commits_files_controller.go

@@ -476,7 +476,11 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm
 				self.c.Git().Patch.PatchBuilder.Reset()
 			}
 
-			self.c.PostRefreshUpdate(self.context())
+			self.c.OnUIThread(func() error {
+				self.c.PostRefreshUpdate(self.context())
+				return nil
+			})
+
 			return nil
 		})
 	}

pkg/gui/controllers/helpers/app_status_helper.go

@@ -6,6 +6,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/gocui"
 	"github.com/jesseduffield/lazygit/pkg/gui/status"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
 type AppStatusHelper struct {
@@ -93,13 +94,24 @@ func (self *AppStatusHelper) renderAppStatus() {
 	self.c.OnWorker(func(_ gocui.Task) error {
 		ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate))
 		defer ticker.Stop()
+		prevAppStatus := ""
 		for range ticker.C {
 			appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig())
-			self.c.Views().AppStatus.FgColor = color
+
-			self.c.OnUIThreadContentOnly(func() error {
+			update := self.c.OnUIThreadContentOnly
+			if utils.StringWidth(appStatus) != utils.StringWidth(prevAppStatus) {
+				// Need a full layout whenever the width of the status string changes. This can't
+				// happen during normal spinning because we validate that all spinner frames have
+				// the same width, so typically this will only be triggered at the beginning and end
+				// of a status, or if the status string changes midway for some reason.
+				update = self.c.OnUIThread
+			}
+			update(func() error {
+				self.c.Views().AppStatus.FgColor = color
 				self.c.SetViewContent(self.c.Views().AppStatus, appStatus)
 				return nil
 			})
+			prevAppStatus = appStatus
 
 			if appStatus == "" {
 				break

pkg/gui/controllers/helpers/inline_status_helper.go

@@ -149,7 +149,7 @@ func (self *InlineStatusHelper) stop(opts InlineStatusOpts) {
 }
 
 func (self *InlineStatusHelper) renderContext(contextKey types.ContextKey) {
-	self.c.OnUIThread(func() error {
+	self.c.OnUIThreadContentOnly(func() error {
 		self.c.ContextForKey(contextKey).HandleRender()
 		return nil
 	})

pkg/gui/controllers/helpers/refresh_helper.go

@@ -532,9 +532,12 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele
 
 	// Need to re-render the commits view because the visualization of local
 	// branch heads might have changed
-	self.c.Mutexes().LocalCommitsMutex.Lock()
+	self.c.OnUIThread(func() error {
-	self.c.Contexts().LocalCommits.HandleRender()
+		self.c.Mutexes().LocalCommitsMutex.Lock()
-	self.c.Mutexes().LocalCommitsMutex.Unlock()
+		self.c.Contexts().LocalCommits.HandleRender()
+		self.c.Mutexes().LocalCommitsMutex.Unlock()
+		return nil
+	})
 
 	self.refreshStatus()
 }
@@ -780,22 +783,27 @@ func (self *RefreshHelper) refForLog() string {
 }
 
 func (self *RefreshHelper) refreshView(context types.Context) {
-	// Re-applying the filter must be done before re-rendering the view, so that
+	// refreshView is called from the worker goroutine that drives async
-	// the filtered list model is up to date for rendering.
+	// refreshes, so bounce to the UI thread before mutating view content.
-	self.searchHelper.ReApplyFilter(context)
+	self.c.OnUIThread(func() error {
+		// Re-applying the filter must be done before re-rendering the view, so that
+		// the filtered list model is up to date for rendering.
+		self.searchHelper.ReApplyFilter(context)
 
-	self.c.PostRefreshUpdate(context)
+		self.c.PostRefreshUpdate(context)
 
-	self.c.AfterLayout(func() error {
+		self.c.AfterLayout(func() error {
-		// Re-applying the search must be done after re-rendering the view though,
+			// Re-applying the search must be done after re-rendering the view though,
-		// so that the "x of y" status is shown correctly.
+			// so that the "x of y" status is shown correctly.
-		//
+			//
-		// Also, it must be done after layout, because otherwise FocusPoint
+			// Also, it must be done after layout, because otherwise FocusPoint
-		// hasn't been called yet (see ListContextTrait.FocusLine), which means
+			// hasn't been called yet (see ListContextTrait.FocusLine), which means
-		// that the scroll position might be such that the entire visible
+			// that the scroll position might be such that the entire visible
-		// content is outside the viewport. And this would cause problems in
+			// content is outside the viewport. And this would cause problems in
-		// searchModelCommits.
+			// searchModelCommits.
-		self.searchHelper.ReApplySearch(context)
+			self.searchHelper.ReApplySearch(context)
+			return nil
+		})
 		return nil
 	})
 }
@@ -941,7 +949,11 @@ func (self *RefreshHelper) setGithubPullRequests(authToken string, baseRemote *m
 	self.savePullRequestsToCache(prs)
 	self.rebuildPullRequestsMap()
 
-	self.c.PostRefreshUpdate(self.c.Contexts().Branches)
+	self.c.OnUIThread(func() error {
+		self.c.PostRefreshUpdate(self.c.Contexts().Branches)
+		return nil
+	})
+
 	return nil
 }
 

pkg/gui/layout.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/jesseduffield/lazygit/pkg/gocui"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/samber/lo"
 )
 
@@ -23,7 +24,11 @@ func (gui *Gui) layout(g *gocui.Gui) error {
 
 	informationStr := gui.informationStr()
 
-	appStatus := gui.helpers.AppStatus.GetStatusString()
+	var appStatus string
+	appStatusView, err := g.View("appStatus")
+	if err == nil {
+		appStatus = utils.Decolorise(appStatusView.Buffer())
+	}
 
 	viewDimensions := gui.getWindowDimensions(informationStr, appStatus)