Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve render performance #3686

Merged
merged 3 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/gui/context/base_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type BaseContext struct {
focusable bool
transient bool
hasControlledBounds bool
needsRerenderOnWidthChange bool
needsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel
needsRerenderOnHeightChange bool
highlightOnFocus bool

Expand All @@ -46,7 +46,7 @@ type NewBaseContextOpts struct {
Transient bool
HasUncontrolledBounds bool // negating for the sake of making false the default
HighlightOnFocus bool
NeedsRerenderOnWidthChange bool
NeedsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel
NeedsRerenderOnHeightChange bool

OnGetOptionsMap func() map[string]string
Expand Down Expand Up @@ -201,7 +201,7 @@ func (self *BaseContext) HasControlledBounds() bool {
return self.hasControlledBounds
}

func (self *BaseContext) NeedsRerenderOnWidthChange() bool {
func (self *BaseContext) NeedsRerenderOnWidthChange() types.NeedsRerenderOnWidthChangeLevel {
return self.needsRerenderOnWidthChange
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/branches_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewBranchesContext(c *ContextCommon) *BranchesContext {
Key: LOCAL_BRANCHES_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES,
})),
ListRenderer: ListRenderer{
list: viewModel,
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/local_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext {
Key: LOCAL_COMMITS_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/reflog_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewReflogCommitsContext(c *ContextCommon) *ReflogCommitsContext {
Key: REFLOG_COMMITS_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
})),
ListRenderer: ListRenderer{
list: viewModel,
Expand Down
1 change: 0 additions & 1 deletion pkg/gui/context/remote_branches_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func NewRemoteBranchesContext(
Kind: types.SIDE_CONTEXT,
Focusable: true,
Transient: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/sub_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func NewSubCommitsContext(
Kind: types.SIDE_CONTEXT,
Focusable: true,
Transient: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
7 changes: 3 additions & 4 deletions pkg/gui/controllers/helpers/window_arrangement_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/jesseduffield/lazygit/pkg/config"
"github.com/jesseduffield/lazygit/pkg/gui/types"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/mattn/go-runewidth"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -272,7 +271,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
return []*boxlayout.Box{
{
Window: "searchPrefix",
Size: runewidth.StringWidth(args.SearchPrefix),
Size: utils.StringWidth(args.SearchPrefix),
},
{
Window: "search",
Expand Down Expand Up @@ -325,7 +324,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
// app status appears very briefly in demos and dislodges the caption,
// so better not to show it at all
if args.AppStatus != "" {
result = append(result, &boxlayout.Box{Window: "appStatus", Size: runewidth.StringWidth(args.AppStatus)})
result = append(result, &boxlayout.Box{Window: "appStatus", Size: utils.StringWidth(args.AppStatus)})
}
}

Expand All @@ -338,7 +337,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
&boxlayout.Box{
Window: "information",
// unlike appStatus, informationStr has various colors so we need to decolorise before taking the length
Size: runewidth.StringWidth(utils.Decolorise(args.InformationStr)),
Size: utils.StringWidth(utils.Decolorise(args.InformationStr)),
})
}

Expand Down
27 changes: 26 additions & 1 deletion pkg/gui/controllers/screen_mode_actions.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import (
"github.com/jesseduffield/gocui"
"github.com/jesseduffield/lazygit/pkg/gui/types"
)

Expand All @@ -16,7 +17,7 @@ func (self *ScreenModeActions) Next() error {
),
)

return nil
return self.rerenderViewsWithScreenModeDependentContent()
}

func (self *ScreenModeActions) Prev() error {
Expand All @@ -27,9 +28,33 @@ func (self *ScreenModeActions) Prev() error {
),
)

return self.rerenderViewsWithScreenModeDependentContent()
}

// these views need to be re-rendered when the screen mode changes. The commits view,
// for example, will show authorship information in half and full screen mode.
func (self *ScreenModeActions) rerenderViewsWithScreenModeDependentContent() error {
for _, context := range self.c.Context().AllList() {
if context.NeedsRerenderOnWidthChange() == types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES {
if err := self.rerenderView(context.GetView()); err != nil {
return err
}
}
}

return nil
}

func (self *ScreenModeActions) rerenderView(view *gocui.View) error {
context, ok := self.c.Helpers().View.ContextForView(view.Name())
if !ok {
self.c.Log.Errorf("no context found for view %s", view.Name())
return nil
}

return context.HandleRender()
}

func nextIntInCycle(sl []types.WindowMaximisation, current types.WindowMaximisation) types.WindowMaximisation {
for i, val := range sl {
if val == current {
Expand Down
7 changes: 3 additions & 4 deletions pkg/gui/information_panel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/jesseduffield/lazygit/pkg/constants"
"github.com/jesseduffield/lazygit/pkg/gui/style"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/mattn/go-runewidth"
)

func (gui *Gui) informationStr() string {
Expand Down Expand Up @@ -34,7 +33,7 @@ func (gui *Gui) handleInfoClick() error {
width, _ := view.Size()

if activeMode, ok := gui.helpers.Mode.GetActiveMode(); ok {
if width-cx > runewidth.StringWidth(gui.c.Tr.ResetInParentheses) {
if width-cx > utils.StringWidth(gui.c.Tr.ResetInParentheses) {
return nil
}
return activeMode.Reset()
Expand All @@ -43,10 +42,10 @@ func (gui *Gui) handleInfoClick() error {
var title, url string

// if we're not in an active mode we show the donate button
if cx <= runewidth.StringWidth(gui.c.Tr.Donate) {
if cx <= utils.StringWidth(gui.c.Tr.Donate) {
url = constants.Links.Donate
title = gui.c.Tr.Donate
} else if cx <= runewidth.StringWidth(gui.c.Tr.Donate)+1+runewidth.StringWidth(gui.c.Tr.AskQuestion) {
} else if cx <= utils.StringWidth(gui.c.Tr.Donate)+1+utils.StringWidth(gui.c.Tr.AskQuestion) {
url = constants.Links.Discussions
title = gui.c.Tr.AskQuestion
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/layout.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (gui *Gui) layout(g *gocui.Gui) error {
}

mustRerender := false
if context.NeedsRerenderOnWidthChange() {
if context.NeedsRerenderOnWidthChange() == types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES {
// view.Width() returns the width -1 for some reason
oldWidth := view.Width() + 1
newWidth := dimensionsObj.X1 - dimensionsObj.X0 + 2*frameOffset
Expand Down
6 changes: 3 additions & 3 deletions pkg/gui/presentation/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func getBranchDisplayStrings(
// Recency is always three characters, plus one for the space
availableWidth := viewWidth - 4
if len(branchStatus) > 0 {
availableWidth -= runewidth.StringWidth(utils.Decolorise(branchStatus)) + 1
availableWidth -= utils.StringWidth(utils.Decolorise(branchStatus)) + 1
}
if icons.IsIconEnabled() {
availableWidth -= 2 // one for the icon, one for the space
Expand All @@ -65,7 +65,7 @@ func getBranchDisplayStrings(
availableWidth -= utils.COMMIT_HASH_SHORT_SIZE + 1
}
if checkedOutByWorkTree {
availableWidth -= runewidth.StringWidth(worktreeIcon) + 1
availableWidth -= utils.StringWidth(worktreeIcon) + 1
}

displayName := b.Name
Expand All @@ -79,7 +79,7 @@ func getBranchDisplayStrings(
}

// Don't bother shortening branch names that are already 3 characters or less
if runewidth.StringWidth(displayName) > max(availableWidth, 3) {
if utils.StringWidth(displayName) > max(availableWidth, 3) {
// Never shorten the branch name to less then 3 characters
len := max(availableWidth, 4)
displayName = runewidth.Truncate(displayName, len, "…")
Expand Down
16 changes: 14 additions & 2 deletions pkg/gui/types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ type ParentContexter interface {
GetParentContext() (Context, bool)
}

type NeedsRerenderOnWidthChangeLevel int

const (
// view doesn't render differently when its width changes
NEEDS_RERENDER_ON_WIDTH_CHANGE_NONE NeedsRerenderOnWidthChangeLevel = iota
// view renders differently when its width changes. An example is a view
// that truncates long lines to the view width, e.g. the branches view
NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES
// view renders differently only when the screen mode changes
NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES
)

type IBaseContext interface {
HasKeybindings
ParentContexter
Expand All @@ -60,8 +72,8 @@ type IBaseContext interface {
// determined independently.
HasControlledBounds() bool

// true if the view needs to be rerendered when its width changes
NeedsRerenderOnWidthChange() bool
// to what extent the view needs to be rerendered when its width changes
NeedsRerenderOnWidthChange() NeedsRerenderOnWidthChangeLevel

// true if the view needs to be rerendered when its height changes
NeedsRerenderOnHeightChange() bool
Expand Down
19 changes: 16 additions & 3 deletions pkg/utils/formatting.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package utils
import (
"fmt"
"strings"
"unicode"

"github.com/mattn/go-runewidth"
"github.com/samber/lo"
Expand All @@ -21,10 +22,22 @@ type ColumnConfig struct {
Alignment Alignment
}

func StringWidth(s string) int {
// We are intentionally not using a range loop here, because that would
// convert the characters to runes, which is unnecessary work in this case.
for i := 0; i < len(s); i++ {
if s[i] > unicode.MaxASCII {
return runewidth.StringWidth(s)
}
}

return len(s)
}

// WithPadding pads a string as much as you want
func WithPadding(str string, padding int, alignment Alignment) string {
uncoloredStr := Decolorise(str)
width := runewidth.StringWidth(uncoloredStr)
width := StringWidth(uncoloredStr)
if padding < width {
return str
}
Expand Down Expand Up @@ -144,7 +157,7 @@ func getPadWidths(stringArrays [][]string) []int {
return MaxFn(stringArrays, func(stringArray []string) int {
uncoloredStr := Decolorise(stringArray[i])

return runewidth.StringWidth(uncoloredStr)
return StringWidth(uncoloredStr)
})
})
}
Expand All @@ -161,7 +174,7 @@ func MaxFn[T any](items []T, fn func(T) int) int {

// TruncateWithEllipsis returns a string, truncated to a certain length, with an ellipsis
func TruncateWithEllipsis(str string, limit int) string {
if runewidth.StringWidth(str) > limit && limit <= 2 {
if StringWidth(str) > limit && limit <= 2 {
return strings.Repeat(".", limit)
}
return runewidth.Truncate(str, limit, "…")
Expand Down
25 changes: 25 additions & 0 deletions pkg/utils/formatting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strings"
"testing"

"github.com/mattn/go-runewidth"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -250,3 +251,27 @@ func TestRenderDisplayStrings(t *testing.T) {
assert.EqualValues(t, test.expectedColumnPositions, columnPositions)
}
}

func BenchmarkStringWidthAsciiOriginal(b *testing.B) {
for i := 0; i < b.N; i++ {
runewidth.StringWidth("some ASCII string")
}
}

func BenchmarkStringWidthAsciiOptimized(b *testing.B) {
for i := 0; i < b.N; i++ {
StringWidth("some ASCII string")
}
}

func BenchmarkStringWidthNonAsciiOriginal(b *testing.B) {
for i := 0; i < b.N; i++ {
runewidth.StringWidth("some non-ASCII string 🍉")
}
}

func BenchmarkStringWidthNonAsciiOptimized(b *testing.B) {
for i := 0; i < b.N; i++ {
StringWidth("some non-ASCII string 🍉")
}
}
Loading