diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 47cd51ff7..2f0f10657 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -96,6 +96,49 @@ type refreshEnv struct { // the repo generation captured when the refresh started generation int + + // When non-nil, each scope's UI-thread bounce is collected here instead of + // being dispatched as it's produced, so they can all be applied in a single + // frame once the whole refresh is done (see RefreshOptions.BatchUIUpdates). + // Held by pointer so the copies of env that flow through the scope functions + // all share the one batch. + batch *refreshBounceBatch +} + +// refreshBounceBatch collects the UI-thread bounces of a batched refresh so they +// can be applied together in one frame rather than one scope at a time. The +// scopes run on separate worker goroutines and add concurrently, hence the +// mutex. Once the refresh starts flushing it closes the batch, so that any +// bounces enqueued afterwards — the nested ones a flushed bounce produces in +// turn, e.g. scrolling the selection into view — are dispatched immediately as +// ordinary follow-ups instead of being collected into a batch that nothing +// will drain. +type refreshBounceBatch struct { + mutex deadlock.Mutex + funcs []func() + closed bool +} + +// add collects f and returns true. Once the batch is closed it collects nothing +// and returns false, telling the caller to dispatch f immediately instead. +func (self *refreshBounceBatch) add(f func()) bool { + self.mutex.Lock() + defer self.mutex.Unlock() + + if self.closed { + return false + } + self.funcs = append(self.funcs, f) + return true +} + +// close marks the batch flushed and returns everything collected so far. +func (self *refreshBounceBatch) close() []func() { + self.mutex.Lock() + defer self.mutex.Unlock() + + self.closed = true + return self.funcs } func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool) { @@ -121,18 +164,15 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr ) } - // f runs on the UI thread when the refresh was initiated there, and also for - // BLOCK_UI, which dispatches f onto the UI thread regardless of the caller. - // Only a SYNC/ASYNC refresh initiated from a worker runs f on that worker. - // This, not calledFromWorker alone, is what decides whether a scope capture - // runs inline or has to hop (see captureOnUIThread). - fRunsOnUIThread := options.Mode == types.BLOCK_UI || !calledFromWorker + // f runs on the UI thread when the refresh was initiated there (Refresh); a + // refresh initiated from a worker (RefreshFromWorker) runs f on that worker. + // This decides whether a scope capture runs inline or has to hop (see + // captureOnUIThread). + fRunsOnUIThread := !calledFromWorker // Debug-only guard: every refresh must be issued from the entry point that // matches its goroutine — Refresh on the UI thread, RefreshFromWorker on a - // worker. We check the caller's own goroutine here, before a BLOCK_UI - // refresh dispatches f onto the UI thread, so it holds regardless of the - // mode. goid stays out of production control flow (debug only). + // worker. goid stays out of production control flow (debug only). if self.c.GetConfig().GetDebug() && self.c.GocuiGui().IsUIThread() == calledFromWorker { panic("Refresh called from a worker, or RefreshFromWorker called from the UI thread") } @@ -144,6 +184,9 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr background: options.Background, generation: self.c.State().GetRepoGeneration(), } + if options.BatchUIUpdates { + env.batch = &refreshBounceBatch{} + } var scopeSet *set.Set[types.RefreshableView] if len(options.Scope) == 0 { @@ -376,6 +419,20 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr wg.Wait() + if env.batch != nil { + // Apply all the scopes' collected bounces in a single UI-thread task, + // so they land in one frame: gocui drains every queued event before it + // redraws, so one task means one repaint. Bounces enqueued from within + // these (see refreshBounceBatch) run as ordinary follow-ups. + bounces := env.batch.close() + self.onUIThread(env.background, func() error { + for _, bounce := range bounces { + bounce() + } + return nil + }) + } + if options.Then != nil { // Queue Then via OnUIThread so it runs *after* the refresh-scope // functions' model-update bounces (which are already queued by @@ -387,14 +444,6 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr } } - if options.Mode == types.BLOCK_UI { - self.c.OnUIThread(func() error { - f() - return nil - }) - return - } - f() } @@ -482,8 +531,6 @@ func getModeName(mode types.RefreshMode) string { return "sync" case types.ASYNC: return "async" - case types.BLOCK_UI: - return "block-ui" default: return "unknown mode" } @@ -1057,13 +1104,21 @@ func (self *RefreshHelper) refreshFilesAndSubmodules(captured capturedFilesState // clobber the new repo's state. The generation is captured once at the start of // the refresh and carried in env (see refreshEnv). func (self *RefreshHelper) onUIThreadUnlessRepoChanged(env refreshEnv, f func()) { - self.onUIThread(env.background, func() error { + wrapper := func() { if self.c.State().GetRepoGeneration() != env.generation { - return nil + return } f() - return nil - }) + } + + // A batched refresh collects its bounces and fires them together at the end + // (see refreshBounceBatch); add reports false once the batch is flushing, so + // bounces enqueued from within a flushed bounce dispatch immediately. + if env.batch != nil && env.batch.add(wrapper) { + return + } + + self.onUIThread(env.background, func() error { wrapper(); return nil }) } // onWorker and onUIThread pick the foreground or background variant of the @@ -1094,12 +1149,11 @@ func (self *RefreshHelper) onUIThread(background bool, f func() error) { // runs on the UI thread (fRunsOnUIThread is true) fn runs inline; when it runs // on a worker, fn is dispatched to the UI thread and we block for it. // -// The inline case matters for correctness as much as the hop: a SYNC or -// BLOCK_UI refresh parks the UI thread in a wg.Wait while its scope workers -// run, so a scope worker that tried to hop to the UI thread there would +// The inline case matters for correctness as much as the hop: a SYNC refresh +// initiated on the UI thread parks that thread in a wg.Wait while its scope +// workers run, so a scope worker that tried to hop to the UI thread there would // deadlock. Capturing before those workers are spawned — inline, on the UI -// thread — avoids that entirely. This is why BLOCK_UI (which always runs on the -// UI thread, even from a worker caller) captures inline rather than hopping. +// thread — avoids that entirely. func (self *RefreshHelper) captureOnUIThread(fRunsOnUIThread bool, background bool, fn func()) { if fRunsOnUIThread { fn() diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index 5f07b8ea6..dda3d918a 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -56,7 +56,8 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions scope = append(scope, types.PULL_REQUESTS) } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + Mode: types.SYNC, + BatchUIUpdates: true, Scope: scope, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, @@ -368,7 +369,8 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest } self.c.Refresh(types.RefreshOptions{ - Mode: types.BLOCK_UI, + Mode: types.SYNC, + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, @@ -534,7 +536,8 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + Mode: types.SYNC, + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, @@ -570,7 +573,8 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri } self.c.RefreshFromWorker(types.RefreshOptions{ - Mode: types.BLOCK_UI, + Mode: types.SYNC, + BatchUIUpdates: true, BranchSelection: types.SelectCheckedOutBranch, CommitSelection: types.SelectHeadCommit, SelectTopReflogCommit: true, diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 23e94adf7..ade7d1a9a 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -604,7 +604,7 @@ func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, start return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit, "") return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( - err, types.RefreshOptions{Mode: types.BLOCK_UI}) + err, types.RefreshOptions{BatchUIUpdates: true}) }) } @@ -628,7 +628,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash()) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, - types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() error { + types.RefreshOptions{BatchUIUpdates: true, Then: func() error { todos := make([]*models.Commit, 0, len(commitsToEdit)-1) for _, c := range commitsToEdit[:len(commitsToEdit)-1] { // Merge commits can't be set to "edit", so just skip them diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index f4041bb2e..7b304b15d 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -28,9 +28,8 @@ const ( type RefreshMode int const ( - SYNC RefreshMode = iota // wait until everything is done before returning - ASYNC // return immediately, allowing each independent thing to update itself - BLOCK_UI // wrap code in an update call to ensure UI updates all at once and keybindings aren't executed till complete + SYNC RefreshMode = iota // wait until everything is done before returning + ASYNC // return immediately, allowing each independent thing to update itself ) // CommitSelectionBehavior controls which local commit is selected after the @@ -74,7 +73,12 @@ const ( type RefreshOptions struct { Then func() error Scope []RefreshableView // e.g. []RefreshableView{COMMITS, BRANCHES}. Leave empty to refresh everything - Mode RefreshMode // one of SYNC (default), ASYNC, and BLOCK_UI + Mode RefreshMode // one of SYNC (default) and ASYNC + + // If true, hold off on updating the UI until all scopes have finished + // refreshing and then apply them together in a single frame, rather than + // letting each scope update the UI as soon as it's done. + BatchUIUpdates bool // Controls which local branch is selected after the refresh. Defaults to // KeepBranchSelectionByName.