fix(selection): build every overlay before starting any so Tab and F never race the wake list
- commit
- 753c87f322f1db824ed97d679189a33ffc623941
- author
- brice · · 2026-10-18 09:36 UTC
- parent
- 07981f5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- feat/freeze-key
3 files · +39 -2
M Makefile +1 -1
| @@ -48,7 +48,7 @@ arch: | |||
| 48 | 48 | # Steady-state allocation guards. PERF_TESTS lists exact test names and PERF_PKGS | |
| 49 | 49 | # the packages holding them. Every named test must exist, so the guard cannot | |
| 50 | 50 | # pass vacuously when a frame-path test is removed or renamed. | |
| 51 | - | PERF_TESTS := TestFrameAllocations TestSelectionAllocations TestPickerAllocations TestMonitorGridAllocations TestMonitorGridPhysicalAllocations TestGridAllocations TestGridModeAllocations TestSelectorAllocations TestModeToggleAllocations TestEffectsAllocations TestRefreshAllocations TestVideoRowsAllocations TestWriteSteadyStateAllocations TestResolveOutputAllocations TestCaptureAllocations TestCaptureAllocationsCancellable TestCropAllocations | |
| 51 | + | PERF_TESTS := TestFrameAllocations TestSelectionAllocations TestPickerAllocations TestMonitorGridAllocations TestMonitorGridPhysicalAllocations TestGridAllocations TestGridModeAllocations TestSelectorAllocations TestModeToggleAllocations TestFreezeKeyAllocations TestEffectsAllocations TestRefreshAllocations TestVideoRowsAllocations TestWriteSteadyStateAllocations TestResolveOutputAllocations TestCaptureAllocations TestCaptureAllocationsCancellable TestCropAllocations | |
| 52 | 52 | PERF_PKGS := ./internal/ports ./internal/core ./internal/adapters/selection ./internal/adapters/indicator ./internal/adapters/ffmpeg ./internal/app ./internal/adapters/wayland | |
| 53 | 53 | perf_empty := | |
| 54 | 54 | perf_space := $(perf_empty) $(perf_empty) | |
| @@ -48,7 +48,7 @@ arch: | |||||
| 48 | # Steady-state allocation guards. PERF_TESTS lists exact test names and PERF_PKGS | 48 | # Steady-state allocation guards. PERF_TESTS lists exact test names and PERF_PKGS | ||
| 49 | # the packages holding them. Every named test must exist, so the guard cannot | 49 | # the packages holding them. Every named test must exist, so the guard cannot | ||
| 50 | # pass vacuously when a frame-path test is removed or renamed. | 50 | # pass vacuously when a frame-path test is removed or renamed. | ||
| 51 | - | PERF_TESTS := TestFrameAllocations TestSelectionAllocations TestPickerAllocations TestMonitorGridAllocations TestMonitorGridPhysicalAllocations TestGridAllocations TestGridModeAllocations TestSelectorAllocations TestModeToggleAllocations TestEffectsAllocations TestRefreshAllocations TestVideoRowsAllocations TestWriteSteadyStateAllocations TestResolveOutputAllocations TestCaptureAllocations TestCaptureAllocationsCancellable TestCropAllocations | 51 | + | PERF_TESTS := TestFrameAllocations TestSelectionAllocations TestPickerAllocations TestMonitorGridAllocations TestMonitorGridPhysicalAllocations TestGridAllocations TestGridModeAllocations TestSelectorAllocations TestModeToggleAllocations TestFreezeKeyAllocations TestEffectsAllocations TestRefreshAllocations TestVideoRowsAllocations TestWriteSteadyStateAllocations TestResolveOutputAllocations TestCaptureAllocations TestCaptureAllocationsCancellable TestCropAllocations |
| 52 | PERF_PKGS := ./internal/ports ./internal/core ./internal/adapters/selection ./internal/adapters/indicator ./internal/adapters/ffmpeg ./internal/app ./internal/adapters/wayland | 52 | PERF_PKGS := ./internal/ports ./internal/core ./internal/adapters/selection ./internal/adapters/indicator ./internal/adapters/ffmpeg ./internal/app ./internal/adapters/wayland | ||
| 53 | perf_empty := | 53 | perf_empty := | ||
| 54 | perf_space := $(perf_empty) $(perf_empty) | 54 | perf_space := $(perf_empty) $(perf_empty) | ||
M internal/adapters/selection/backdrop_test.go +31 -0
| @@ -94,6 +94,37 @@ func TestFreezeKeyTogglesEveryOverlay(t *testing.T) { | |||
| 94 | 94 | assert.False(t, sess.acceptedFrozen(), "a pick on the live screen is captured live") | |
| 95 | 95 | } | |
| 96 | 96 | ||
| 97 | + | // The live state survives a round trip through record mode, where F is | |
| 98 | + | // ignored. | |
| 99 | + | func TestFreezeKeyWithTab(t *testing.T) { | |
| 100 | + | sess, _ := testSession() | |
| 101 | + | sess.canFreeze = true | |
| 102 | + | m := frozenModel(t, 0, sess) | |
| 103 | + | m.toggle = true | |
| 104 | + | m.input(key("f")) | |
| 105 | + | m.input(key("Tab")) | |
| 106 | + | require.Equal(t, ports.Record, m.mode) | |
| 107 | + | assert.False(t, m.input(key("f")), "F is ignored in record mode") | |
| 108 | + | m.input(key("Tab")) | |
| 109 | + | assert.False(t, drawsBackdrop(m), "still live after the round trip") | |
| 110 | + | assert.Contains(t, m.footerText, "F freeze") | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | // Like Tab, F allocates only the rebuilt footer text. | |
| 114 | + | func TestFreezeKeyAllocations(t *testing.T) { | |
| 115 | + | if raceEnabled { | |
| 116 | + | t.Skip("the race detector adds allocations") | |
| 117 | + | } | |
| 118 | + | sess, _ := testSession() | |
| 119 | + | sess.canFreeze = true | |
| 120 | + | m := frozenModel(t, 0, sess) | |
| 121 | + | n := testing.AllocsPerRun(100, func() { | |
| 122 | + | m.input(key("f")) | |
| 123 | + | _ = m.scene() | |
| 124 | + | }) | |
| 125 | + | assert.LessOrEqual(t, n, 1.0, "an F press allocates only the rebuilt legend text") | |
| 126 | + | } | |
| 127 | + | ||
| 97 | 128 | // Without a frozen screen, or in record mode, F does nothing and is not shown. | |
| 98 | 129 | func TestFreezeKeyHiddenWithoutBackdrop(t *testing.T) { | |
| 99 | 130 | m := newTestModel(t) | |
| @@ -94,6 +94,37 @@ func TestFreezeKeyTogglesEveryOverlay(t *testing.T) { | |||||
| 94 | assert.False(t, sess.acceptedFrozen(), "a pick on the live screen is captured live") | 94 | assert.False(t, sess.acceptedFrozen(), "a pick on the live screen is captured live") | ||
| 95 | } | 95 | } | ||
| 96 | 96 | ||||
| 97 | + | // The live state survives a round trip through record mode, where F is | |||
| 98 | + | // ignored. | |||
| 99 | + | func TestFreezeKeyWithTab(t *testing.T) { | |||
| 100 | + | sess, _ := testSession() | |||
| 101 | + | sess.canFreeze = true | |||
| 102 | + | m := frozenModel(t, 0, sess) | |||
| 103 | + | m.toggle = true | |||
| 104 | + | m.input(key("f")) | |||
| 105 | + | m.input(key("Tab")) | |||
| 106 | + | require.Equal(t, ports.Record, m.mode) | |||
| 107 | + | assert.False(t, m.input(key("f")), "F is ignored in record mode") | |||
| 108 | + | m.input(key("Tab")) | |||
| 109 | + | assert.False(t, drawsBackdrop(m), "still live after the round trip") | |||
| 110 | + | assert.Contains(t, m.footerText, "F freeze") | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | // Like Tab, F allocates only the rebuilt footer text. | |||
| 114 | + | func TestFreezeKeyAllocations(t *testing.T) { | |||
| 115 | + | if raceEnabled { | |||
| 116 | + | t.Skip("the race detector adds allocations") | |||
| 117 | + | } | |||
| 118 | + | sess, _ := testSession() | |||
| 119 | + | sess.canFreeze = true | |||
| 120 | + | m := frozenModel(t, 0, sess) | |||
| 121 | + | n := testing.AllocsPerRun(100, func() { | |||
| 122 | + | m.input(key("f")) | |||
| 123 | + | _ = m.scene() | |||
| 124 | + | }) | |||
| 125 | + | assert.LessOrEqual(t, n, 1.0, "an F press allocates only the rebuilt legend text") | |||
| 126 | + | } | |||
| 127 | + | ||||
| 97 | // Without a frozen screen, or in record mode, F does nothing and is not shown. | 128 | // Without a frozen screen, or in record mode, F does nothing and is not shown. | ||
| 98 | func TestFreezeKeyHiddenWithoutBackdrop(t *testing.T) { | 129 | func TestFreezeKeyHiddenWithoutBackdrop(t *testing.T) { | ||
| 99 | m := newTestModel(t) | 130 | m := newTestModel(t) | ||
M internal/adapters/selection/selector.go +7 -1
| @@ -171,7 +171,9 @@ func (s *Selector) Select(ctx context.Context, outputs []ports.Output) (ports.Se | |||
| 171 | 171 | sess := newSession(cancel) | |
| 172 | 172 | sess.canFreeze = len(s.backdrops) > 0 | |
| 173 | 173 | s.frozen = false | |
| 174 | - | var wg sync.WaitGroup | |
| 174 | + | // Build every model and the complete wakes slice before any overlay | |
| 175 | + | // starts: a running overlay reads sess.wakes on Tab or F. | |
| 176 | + | models := make([]*model, len(outputs)) | |
| 175 | 177 | for i := range outputs { | |
| 176 | 178 | m := newModel(s.mode, outputs, i, sess, s.grid) | |
| 177 | 179 | m.setWorkspaces(s.workspaces) | |
| @@ -187,6 +189,10 @@ func (s *Selector) Select(ctx context.Context, outputs []ports.Output) (ports.Se | |||
| 187 | 189 | sess.wakes = append(sess.wakes, m.wake) | |
| 188 | 190 | } | |
| 189 | 191 | m.footerText = footerText(m.mode, m.footerKind, m.footerWorkspace, m.toggle, m.noIndicator, m.freezeHint()) | |
| 192 | + | models[i] = m | |
| 193 | + | } | |
| 194 | + | var wg sync.WaitGroup | |
| 195 | + | for i, m := range models { | |
| 190 | 196 | wg.Add(1) | |
| 191 | 197 | go func() { | |
| 192 | 198 | defer wg.Done() | |
| @@ -171,7 +171,9 @@ func (s *Selector) Select(ctx context.Context, outputs []ports.Output) (ports.Se | |||||
| 171 | sess := newSession(cancel) | 171 | sess := newSession(cancel) | ||
| 172 | sess.canFreeze = len(s.backdrops) > 0 | 172 | sess.canFreeze = len(s.backdrops) > 0 | ||
| 173 | s.frozen = false | 173 | s.frozen = false | ||
| 174 | - | var wg sync.WaitGroup | 174 | + | // Build every model and the complete wakes slice before any overlay |
| 175 | + | // starts: a running overlay reads sess.wakes on Tab or F. | |||
| 176 | + | models := make([]*model, len(outputs)) | |||
| 175 | for i := range outputs { | 177 | for i := range outputs { | ||
| 176 | m := newModel(s.mode, outputs, i, sess, s.grid) | 178 | m := newModel(s.mode, outputs, i, sess, s.grid) | ||
| 177 | m.setWorkspaces(s.workspaces) | 179 | m.setWorkspaces(s.workspaces) | ||
| @@ -187,6 +189,10 @@ func (s *Selector) Select(ctx context.Context, outputs []ports.Output) (ports.Se | |||||
| 187 | sess.wakes = append(sess.wakes, m.wake) | 189 | sess.wakes = append(sess.wakes, m.wake) | ||
| 188 | } | 190 | } | ||
| 189 | m.footerText = footerText(m.mode, m.footerKind, m.footerWorkspace, m.toggle, m.noIndicator, m.freezeHint()) | 191 | m.footerText = footerText(m.mode, m.footerKind, m.footerWorkspace, m.toggle, m.noIndicator, m.freezeHint()) | ||
| 192 | + | models[i] = m | |||
| 193 | + | } | |||
| 194 | + | var wg sync.WaitGroup | |||
| 195 | + | for i, m := range models { | |||
| 190 | wg.Add(1) | 196 | wg.Add(1) | ||
| 191 | go func() { | 197 | go func() { | ||
| 192 | defer wg.Done() | 198 | defer wg.Done() | ||