v2: viewport: refactor softwrap; improve perf; various bug fixes

[lrstanley]

Describe your changes

• fixes GetVisibleLines() returning more results than is rendered in the viewport.
  • capped softWrap slice to maxHeight, and accounting for the correct virtual offset (see below).
  • visibleLines() now also returns exactly what is needed for visibility, where it was returning ALL lines after the visible real-index.
• fixes GetTotalLines() not returning the correct wrapped results with SoftWrap.
  • division over int was rounding down and mis-calculating lines, so we now use float64 + math.Ceil.
• fixes a bug when SoftWrap is enabled, which caused lines that are exactly the width of the viewport to add an additional empty line to the output.
  • ansi.Cut at the exact length returns an empty string as the remaining, which wasn't being accounted for.
• fixes a bug that prevented the first and last line to be scrolled to in certain situations, both in SoftWrap and non-SoftWrap modes.
  • new internal "virtual offset" is now tracked, which accurately determines the offset from the "real" index that is actually being rendered.
• fixes scrolling when SoftWrap is enabled, so it scrolls through the wrapped lines, rather than scrolling only "real" lines, which previously felt more jagged.
  • virtual offset also adds this feature/fix.
  • before, this also meant you couldn't scroll through wrapped lines when the viewport was 1 height tall.
• fixes an issue when 1 SoftWrap'd line spans greater than the entire height of the viewport, where softwrapping would previously not work correctly/cut off lines.
• fixes a panic when viewport is initialized with content, and a size hasn't been set yet
  • was also due to the int division rounding.
• fixes a panic when viewport height or width is 0 (was previously returning -1 values)
  • max() in a few places.
• fixes a bug where SetContentLines() didn't do CRLF normalization (even though it technically allows newlines in the provided string slice).
• fixes a bug where SetContentLines() cause total and visible line calculations to be incorrect.
• fixes gutter size being included twice, once in Model.maxWidth() and also in Model.calculateLines(), resulting in slightly off visible line calculations.
• Added clarification on LeftGutterFunc that the resulting width of the gutter must be stable, as this is what the original, and updated code expects when calculating visible/total height (both in non-softwrapping and when using SoftWrap).
• Added golden tests for various sizing scenarios, which should help isolate issues in the future if any of the logic changes.
• Fixed misc spelling/grammar issues.

Performance

Also took a little bit of time to improve performance, given viewport is commonly used to house other components, and performance hits here can affect other core parts of folks apps, especially if they are actively scrolling. There were quite a few inefficient allocations, re-invocations of heavy methods in multiple places, as well as re-processing and re-stitching of data multiple times. The changes made should have little to no readability overhead, but result in the following perf wins (tl;dr: 35% less CPU usage, 38% less memory usage, and 38% less allocations on average, with more/less depending on if SoftWrap is enabled or not. upwards of 55%, 58%, and 51% respectively depending on the usecase):

goos: linux
goarch: amd64
pkg: github.com/charmbracelet/bubbles/v2/viewport
cpu: AMD Ryzen 9 5900X 12-Core Processor            
                              │ benchmark-v2-exp-head  │          benchmark-pr-823           │
                              │         sec/op         │   sec/op     vs base                │
View/view-30x15-12                         31.17µ ± 2%   25.36µ ± 3%  -18.65% (p=0.000 n=10)
View/view-100x100-12                       142.7µ ± 2%   101.0µ ± 2%  -29.23% (p=0.000 n=10)
View/view-30x15-softwrap-12                252.9µ ± 2%   114.4µ ± 2%  -54.78% (p=0.000 n=10)
View/view-100x100-softwrap-12              226.6µ ± 3%   158.7µ ± 3%  -29.97% (p=0.000 n=10)
geomean                                    126.3µ        82.56µ       -34.66%

                              │ benchmark-v2-exp-head  │           benchmark-pr-823           │
                              │          B/op          │     B/op      vs base                │
View/view-30x15-12                       12.357Ki ± 4%   8.884Ki ± 3%  -28.10% (p=0.000 n=10)
View/view-100x100-12                      162.7Ki ± 3%   105.3Ki ± 5%  -35.31% (p=0.000 n=10)
View/view-30x15-softwrap-12              134.13Ki ± 5%   56.29Ki ± 4%  -58.03% (p=0.000 n=10)
View/view-100x100-softwrap-12             198.5Ki ± 8%   153.1Ki ± 6%  -22.90% (p=0.000 n=10)
geomean                                   85.54Ki        53.28Ki       -37.71%

                              │ benchmark-v2-exp-head  │          benchmark-pr-823          │
                              │       allocs/op        │ allocs/op   vs base                │
View/view-30x15-12                         103.00 ± 0%   50.00 ± 0%  -51.46% (p=0.000 n=10)
View/view-100x100-12                       138.00 ± 0%   80.00 ± 0%  -42.03% (p=0.000 n=10)
View/view-30x15-softwrap-12                 461.0 ± 0%   348.0 ± 0%  -24.51% (p=0.000 n=10)
View/view-100x100-softwrap-12               213.0 ± 0%   144.0 ± 0%  -32.39% (p=0.000 n=10)
geomean                                     193.3        119.0       -38.44

Related issue/discussions

• closes: v2: viewport: VisibleLineCount and TotalLineCount incorrect when using SoftWrap and wrapping #819
• closes: v2: viewport: panic when enabling SoftWrap, but not receiving window size yet #820

Checklist before requesting a review

• I have read CONTRIBUTING.md
• I have performed a self-review of my code

---

This was one of the example usecases that when tested with the current v2-exp HEAD, has all sorts of rendering issues, and visible/total line calculation issues:

package main

import (
	"fmt"
	"log/slog"
	"os"
	"strings"

	"github.com/charmbracelet/bubbles/v2/viewport"
	tea "github.com/charmbracelet/bubbletea/v2"
	"github.com/charmbracelet/lipgloss/v2"
)

const randomText = `
1 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
2 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
3 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
4 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
5 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
6 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
7 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
8 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
9 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
10 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
11 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
12 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
13 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
14 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
15 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
16 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
17 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
18 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
19 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
20 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
21 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
22 Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
23 Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
24 Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. LAST LINE
`

type Model struct {
	width  int
	height int

	viewport viewport.Model
}

func (m Model) Init() tea.Cmd {
	return nil
}

func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	var cmd tea.Cmd
	var cmds []tea.Cmd

	switch msg := msg.(type) {
	case tea.WindowSizeMsg:
		m.width = msg.Width
		m.height = msg.Height
		m.viewport.SetWidth(msg.Width - 2)   // -2=border.
		m.viewport.SetHeight(msg.Height - 3) // -2=border, -1=status bar.
		return m, nil
	case tea.KeyMsg:
		switch msg.String() {
		case "q", "ctrl+c":
			return m, tea.Quit
		}
	}

	m.viewport, cmd = m.viewport.Update(msg)
	cmds = append(cmds, cmd)

	return m, tea.Batch(cmds...)
}

func (m Model) View() string {
	return lipgloss.JoinVertical(
		lipgloss.Top,
		lipgloss.NewStyle().
			Border(lipgloss.RoundedBorder()).
			Width(m.width).
			Height(m.height-1).
			Render(m.viewport.View()),
		lipgloss.NewStyle().
			Width(m.width).
			Height(1).
			Align(lipgloss.Center).
			Foreground(lipgloss.Color("white")).
			Render(fmt.Sprintf("viewport: %d visible lines, %d total lines", m.viewport.VisibleLineCount(), m.viewport.TotalLineCount())),
	)
}

func main() {
	m := Model{viewport: viewport.New()}

	m.viewport.SoftWrap = true
	m.viewport.SetContent(strings.TrimSpace(randomText))

	p := tea.NewProgram(m, tea.WithAltScreen())
	if _, err := p.Run(); err != nil {
		slog.Error("error running program", "error", err)
		os.Exit(1)
	}
}

Reviewer Notes

Currently, with soft wrapping, if you have a lot of wrapped lines (more lines added virtually, than your viewport height, if you resize the window larger, it can easily leave the yoffset in a state where it's beyond the visible lines completely. Should we always check Model.PastBottom() inside of Model.SetWidth() and Model.SetHeight()? Feel like that's a sane/reasonable default, as I can't imagine someone not wanting it.

[meowgorithm]

This PR is so awesome. @lrstanley, it looks like the golden files need to be updated. Do you mind doing that?

[lrstanley]

This PR is so awesome. @lrstanley, it looks like the golden files need to be updated. Do you mind doing that?

Thanks, didn't catch the table tests failing. Looking at those tests, many don't have height or width set, and then take an immediate snapshot to render the table. What's the expected outcome in that situation? Right now with this PR, it's expected that viewport won't render anything until width/height are set, which means many of those tests effectively render nothing because they are missing width and/or height.

Should the table tests always have WithWidth() and WithHeight()? Or is it expected that viewport will just render max potential width/height when no width/height are set? (feels like it would be a huge pain?)

[caarlos0]

PR looks great! Thanks @lrstanley 🙏🏻

Regarding the failed tests, I think we could probably set a width and height for all the tests 🤔

...Or, if width/height isn't set, render everything - not sure if this is the best behavior though

[lrstanley]

Regarding the failed tests, I think we could probably set a width and height for all the tests 🤔

I've done this for now, mostly just matching up the explicit dimensions to how it was previously rendered to reduce the diffs, though:

...Or, if width/height isn't set, render everything - not sure if this is the best behavior though

Right now, table has a built-in default height of 20, which feels weird, as no other components have this behavior that I can see from some basic skimming. Because there is no default width, it creates an odd scenario for both the table component (and previously the viewport, before this PR), which would just use up as much space as needed (which can create all sorts of weird bugs for downstream apps if they don't explicitly set dimensions because of those default behaviors). It's also clear that there are still quite a few bugs with the current table implementation, given some of the TODOs in the table tests. The rewrite in #772 might help (though I think that still needs quite a bit of work).

I think that viewport should not have a "free form" mode (width or height), because I think it defeats the purpose of a viewport in the first place (someone is using it because of the size constraints, be it scrolling or wrapping, so they should know the constraints), and makes wrapping (which is new to v2), impossible to calculate correctly. Thoughts?

[meowgorithm]

@aymanbagabas and @caarlos0: are we good to merge this one?