Improve scroll performance when there is a large number of search results (#6201)

In extreme cases, when you have a really large number of search results (e.g. when searching for a single letter in a long branch log and there are like 40'000 matches), scrolling the view can become laggy. This PR improves this to match the performance without a search. Along the way we fix a bug with highlighting search results when the custom patch gutter is showing and the view is scrolled sideways.
2026-11-14 00:33:35 -04:00 · 2026-11-12 14:53:53 +01:00
parent f07cc9e6f6 b9e0861dfc
commit f794d3460d
3 changed files with 244 additions and 31 deletions
@@ -56,3 +56,132 @@ func BenchmarkWriteToSearchedView(b *testing.B) {
 		v.GetSearchStatus()
 	}
 }
+
+// drawSearchHighlights draws the view and returns how each of its rows is highlighted:
+// "#" for a search match, "*" for the current one, and "." for neither.
+func drawSearchHighlights(v *View) []string {
+	v.draw(true)
+
+	width, height := v.InnerSize()
+	rows := make([]string, 0, height)
+	for y := range height {
+		row := ""
+		for x := range width {
+			_, style, _ := Screen.Get(v.x0+x+1, v.y0+y+1)
+			switch style.GetBackground() {
+			case getTcellColor(ColorYellow, OutputNormal):
+				row += "#"
+			case getTcellColor(ColorCyan, OutputNormal):
+				row += "*"
+			default:
+				row += "."
+			}
+		}
+		rows = append(rows, row)
+	}
+	return rows
+}
+
+// The search matches on screen are highlighted, and the current one in a color of its
+// own.
+func TestDrawSearchMatches(t *testing.T) {
+	WithSimulationScreen(t, 14, 6)
+
+	v := NewView("name", 0, 0, 11, 5, OutputNormal) // InnerWidth 10, InnerHeight 4
+	writeLines(v,
+		"ab",
+		"abab xab",
+		"no match",
+		"xxxxxxxxxab xxxxx ab",
+		"ab ab",
+		"ab",
+	)
+	v.Search("ab", nil)
+	_ = v.gotoNextMatch()
+	_ = v.gotoNextMatch()
+	index, total := v.GetSearchStatus()
+	assert.Equal(t, 2, index, "the current match is the second one in the second line")
+	assert.Equal(t, 9, total)
+
+	// Scrolled down past the first line, so that the last line is off screen too.
+	v.SetOriginY(1)
+	assert.Equal(t,
+		[]string{
+			"##**..##..", // abab xab
+			"..........", // no match
+			".........#", // xxxxxxxxxa
+			"##.##.....", // ab ab
+		},
+		drawSearchHighlights(v))
+
+	// Scrolled sideways, matches show where they are partly on screen.
+	v.SetOriginX(3)
+	assert.Equal(t,
+		[]string{
+			"*..##.....", // b xab
+			"..........", // match
+			"......##..", // xxxxxxab x
+			"##........", // ab
+		},
+		drawSearchHighlights(v))
+}
+
+// Where matches overlap, the current one shows in full.
+func TestDrawOverlappingSearchMatches(t *testing.T) {
+	WithSimulationScreen(t, 14, 6)
+
+	v := NewView("name", 0, 0, 11, 2, OutputNormal) // InnerWidth 10, InnerHeight 1
+	writeLines(v, "aaaa")
+	v.Search("aa", nil)
+	_ = v.gotoNextMatch()
+	index, total := v.GetSearchStatus()
+	assert.Equal(t, 1, index, "the current match is the one in the middle")
+	assert.Equal(t, 3, total)
+
+	assert.Equal(t,
+		[]string{"#**#......"},
+		drawSearchHighlights(v))
+}
+
+// The model can find a match that the view doesn't show, for example in a commit hash
+// that the view abbreviates. Such a match is highlighted where the model says it is.
+// The other matches are highlighted where the view shows them.
+func TestDrawSearchMatchesFromTheModel(t *testing.T) {
+	WithSimulationScreen(t, 14, 6)
+
+	v := NewView("name", 0, 0, 11, 4, OutputNormal) // InnerWidth 10, InnerHeight 3
+	writeLines(v, "ab ab", "1234567", "ab")
+	v.Search("ab", []SearchPosition{
+		{XStart: 2, XEnd: 6, Y: 0},
+		{XStart: 2, XEnd: 6, Y: 1},
+		{XStart: 2, XEnd: 6, Y: 2},
+	})
+	_ = v.gotoNextMatch()
+	_ = v.gotoNextMatch()
+	index, total := v.GetSearchStatus()
+	assert.Equal(t, 2, index, "the current match is the one the view doesn't show")
+	assert.Equal(t, 4, total)
+
+	assert.Equal(t,
+		[]string{
+			"##.##.....", // ab ab
+			"..****....", // 1234567
+			"##........", // ab
+		},
+		drawSearchHighlights(v))
+}
+
+func BenchmarkDrawViewWithManySearchMatches(b *testing.B) {
+	WithSimulationScreen(b, 102, 42)
+	v := NewView("name", 0, 0, 101, 41, OutputNormal)
+	// 44000 matches, as when searching a long log for a single letter.
+	for i := range 4000 {
+		fmt.Fprintf(v, "line %d of a branch log, where we searched for every letter e\n", i)
+	}
+	v.Search("e", nil)
+	v.SetOriginY(2000)
+
+	for b.Loop() {
+		v.draw(true)
+	}
+}
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"slices"
+	"sort"
 	"strings"
 	"sync"
 	"unicode"
@@ -429,8 +430,13 @@ func (v *View) viewLineSpanOfBufferLine(bufferLine int) (int, int, bool) {
 }

 type searcher struct {
-	searchString       string
-	searchPositions    []SearchPosition
+	searchString string
+
+	// The matches, sorted by line, and within a line by the column they start in.
+	// Since all matches in a line have the same width, they are in the order of the
+	// column they end in, too.
+	searchPositions []SearchPosition
+
 	modelSearchResults []SearchPosition
 	currentSearchIndex int
 	onSelectItem       func(*View, int)
@@ -780,6 +786,11 @@ type SearchPosition struct {
 	Y      int
 }

+// contains returns whether the position covers content column x of view line y.
+func (p SearchPosition) contains(x, y int) bool {
+	return p.Y == y && p.XStart <= x && x < p.XEnd
+}
+
 type viewLine struct {
 	linesX, linesY int // coordinates relative to v.buf.lines
 	line           []cell
@@ -1034,8 +1045,12 @@ func (v *View) InclusionGutterWidthWhenShown() int {

 // setCharacter sets a character (grapheme cluster) at the given point relative to the view. It applies
 // the specified colors, taking into account if the cell must be highlighted. Also, it checks if the
-// position is valid. width is the number of columns the character takes up.
-func (v *View) setCharacter(x, y int, ch string, width int, fgColor, bgColor Attribute, isWindowFocused bool) {
+// position is valid. width is the number of columns the character takes up. match says whether
+// the character is part of a search match, and hoveredHyperlink whether it is part of the
+// hyperlink under the mouse.
+func (v *View) setCharacter(x, y int, ch string, width int, fgColor, bgColor Attribute, isWindowFocused bool,
+	match searchMatch, hoveredHyperlink bool,
+) {
 	maxX, maxY := v.Size()
 	if x < 0 || x >= maxX || y < 0 || y >= maxY {
 		return
@@ -1066,16 +1081,16 @@ func (v *View) setCharacter(x, y int, ch string, width int, fgColor, bgColor Att
 		}
 	}

-	if matched, selected := v.isPatternMatchedRune(x, y); matched {
+	if match != noSearchMatch {
 		fgColor = ColorBlack
-		if selected {
+		if match == currentSearchMatch {
 			bgColor = ColorCyan
 		} else {
 			bgColor = ColorYellow
 		}
 	}

-	if v.isHoveredHyperlink(x, y) {
+	if hoveredHyperlink {
 		fgColor |= AttrUnderline
 	}

@@ -1951,6 +1966,7 @@ func (v *View) draw(isWindowFocused bool) {
 	emptyCell := cell{chr: " ", width: 1, fgColor: ColorDefault, bgColor: ColorDefault}

 	gutterWidth := v.inclusionGutterWidth()
+	searchMatches := v.searcher.matchWalker(v.oy)

 	for y, vline := range v.viewLines[start:] {
 		if y >= maxY {
@@ -1968,13 +1984,15 @@ func (v *View) draw(isWindowFocused bool) {

 		// The inclusion gutter is blank but for the marker on a marked line, and the
 		// content begins after it. The blanks go through setCharacter like everything
-		// else, so that a selection reaching the left edge covers the gutter too.
+		// else, so that a selection reaching the left edge covers the gutter too. Search
+		// matches and the hovered hyperlink belong to the content, so they don't show in
+		// the gutter, even where the content is scrolled off under it.
 		for gx := range gutterWidth {
-			v.setCharacter(gx, y, " ", 1, v.FgColor, v.BgColor, isWindowFocused)
+			v.setCharacter(gx, y, " ", 1, v.FgColor, v.BgColor, isWindowFocused, noSearchMatch, false)
 		}
 		if gutterWidth > 0 && vline.linesY < len(v.inclusionGutterMarks) && v.inclusionGutterMarks[vline.linesY] {
 			v.setCharacter(0, y, v.InclusionGutterMarker, uniseg.StringWidth(v.InclusionGutterMarker),
-				v.InclusionGutterMarkerColor, v.BgColor, isWindowFocused)
+				v.InclusionGutterMarkerColor, v.BgColor, isWindowFocused, noSearchMatch, false)
 		}

 		// x tracks the current x position in the view, and cellIdx tracks the
@@ -2027,7 +2045,9 @@ func (v *View) draw(isWindowFocused bool) {
 				fgColor |= AttrUnderline
 			}

-			v.setCharacter(x, y, c.chr, c.width, fgColor, bgColor, isWindowFocused)
+			contentX, contentY := x-gutterWidth+v.ox, v.oy+y
+			v.setCharacter(x, y, c.chr, c.width, fgColor, bgColor, isWindowFocused,
+				searchMatches.matchAt(contentX, contentY), v.isHoveredHyperlink(contentX, contentY))

 			x += c.width
 			cellIdx++
@@ -2123,22 +2143,66 @@ func (v *View) contentX(x int) int {
 	return x - v.inclusionGutterWidth() + v.ox
 }

-func (v *View) isPatternMatchedRune(x, y int) (bool, bool) {
-	for i, pos := range v.searcher.searchPositions {
-		adjustedY := y + v.oy
-		adjustedX := v.contentX(x)
-		if adjustedY == pos.Y && adjustedX >= pos.XStart && adjustedX < pos.XEnd {
-			return true, i == v.searcher.currentSearchIndex
-		}
-	}
-	return false, false
+// searchMatch is whether a cell of the view is part of a search match, and if so,
+// whether that match is the current one.
+type searchMatch int
+
+const (
+	noSearchMatch searchMatch = iota
+	otherSearchMatch
+	currentSearchMatch
+)
+
+// searchMatchWalker tells which cells of the view are part of a search match, for drawing
+// them. It has to be asked about the cells in the order they are drawn: line by line, and
+// from left to right within a line. The matches are in the same order, so the walker steps
+// through them along with the cells.
+type searchMatchWalker struct {
+	positions    []SearchPosition
+	currentIndex int
+
+	// The first match that doesn't end before the cell asked about last.
+	index int
 }

+// matchWalker returns a searchMatchWalker for drawing the view from view line firstY
+// on.
+func (s *searcher) matchWalker(firstY int) searchMatchWalker {
+	return searchMatchWalker{
+		positions:    s.searchPositions,
+		currentIndex: s.currentSearchIndex,
+		index: sort.Search(len(s.searchPositions), func(i int) bool {
+			return s.searchPositions[i].Y >= firstY
+		}),
+	}
+}
+
+// matchAt returns whether the cell at content column x of view line y is part of a
+// search match. If it is part of several overlapping matches, and the current match is
+// one of them, it counts as part of the current one.
+func (w *searchMatchWalker) matchAt(x, y int) searchMatch {
+	for w.index < len(w.positions) {
+		pos := w.positions[w.index]
+		if pos.Y > y || (pos.Y == y && pos.XEnd > x) {
+			break
+		}
+		w.index++
+	}
+
+	if w.index == len(w.positions) || !w.positions[w.index].contains(x, y) {
+		return noSearchMatch
+	}
+	if w.positions[w.currentIndex].contains(x, y) {
+		return currentSearchMatch
+	}
+	return otherSearchMatch
+}
+
+// isHoveredHyperlink returns whether the cell at content column x of view line y is
+// part of the hyperlink under the mouse.
 func (v *View) isHoveredHyperlink(x, y int) bool {
 	if v.UnderlineHyperLinksOnlyOnHover && v.hoveredHyperlink != nil {
-		adjustedY := y + v.oy
-		adjustedX := v.contentX(x)
-		return adjustedY == v.hoveredHyperlink.Y && adjustedX >= v.hoveredHyperlink.XStart && adjustedX < v.hoveredHyperlink.XEnd
+		return y == v.hoveredHyperlink.Y && x >= v.hoveredHyperlink.XStart && x < v.hoveredHyperlink.XEnd
 	}
 	return false
 }
@@ -16,16 +16,16 @@ import (
 )

 // WithSimulationScreen swaps the package-level Screen for a tcell
-// terminfo-backed mock terminal so tests can call view.draw() and
-// inspect rendered cells via Screen.Get(). The previous Screen is
-// restored on test cleanup.
-func WithSimulationScreen(t *testing.T, width, height int) {
-	t.Helper()
+// terminfo-backed mock terminal so tests and benchmarks can call
+// view.draw() and inspect rendered cells via Screen.Get(). The previous
+// Screen is restored on test cleanup.
+func WithSimulationScreen(tb testing.TB, width, height int) {
+	tb.Helper()
 	saved := Screen
 	if err := (&Gui{}).tcellInitSimulation(width, height); err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
-	t.Cleanup(func() {
+	tb.Cleanup(func() {
 		Screen.Fini()
 		Screen = saved
 	})
@@ -1327,6 +1327,16 @@ func TestSearchMatchBesideTheInclusionGutter(t *testing.T) {
 	assert.Equal(t,
 		".....##...",
 		highlightedColumns(1))
+
+	// Scrolled sideways, a match that begins in the columns scrolled off to the left
+	// is highlighted where it shows, and not in the gutter.
+	v.UpdateSearchResults("12", nil)
+	v.SetOriginX(2)
+	v.draw(true)
+	// The row reads "✓ 23456789", so the "2" of the match is at view column 2.
+	assert.Equal(t,
+		"..#.......",
+		highlightedColumns(1))
 }

 // A hyperlink beside the inclusion gutter is underlined while the mouse is over it.
@@ -1337,7 +1347,7 @@ func TestHoveredHyperlinkBesideTheInclusionGutter(t *testing.T) {
 	v.InclusionGutterMarker = "✓"
 	v.UnderlineHyperLinksOnlyOnHover = true

-	v.writeString("ab\x1b]8;;https://example.com\x1b\\link\x1b]8;;\x1b\\cd\n")
+	v.writeString("ab\x1b]8;;https://example.com\x1b\\link\x1b]8;;\x1b\\cdefg\n")
 	v.SetInclusionGutter(true, []bool{true})

 	// The row reads "✓ ablinkcd". Point at the "l" of "link", at view column 4,
@@ -1356,6 +1366,16 @@ func TestHoveredHyperlinkBesideTheInclusionGutter(t *testing.T) {
 	assert.Equal(t,
 		"....____..",
 		underlinedColumns(1))
+
+	// Scrolled sideways, a hyperlink that begins in the columns scrolled off to the
+	// left is underlined where it shows, and not in the gutter. The row reads
+	// "✓ inkcdefg"; point at the "k" of "link", at view column 4 again.
+	v.SetOriginX(3)
+	v.onMouseMove(5, 1)
+	v.draw(true)
+	assert.Equal(t,
+		"..___.....",
+		underlinedColumns(1))
 }

 // Clicking a hyperlink beside the inclusion gutter opens it.