fix(core): cancel stale focus flashes and track cooldown per target
- commit
- ad49c58ac2874627793052d0375933ab9d4d4ce9
- author
- brice · · 2026-10-19 04:47 UTC
- parent
- 3b72e8e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +117 -12
M internal/core/flash.go +47 -12
| @@ -25,16 +25,15 @@ type flashTarget struct { | |||
| 25 | 25 | } | |
| 26 | 26 | ||
| 27 | 27 | type focusFlash struct { | |
| 28 | - | pending *flashTarget | |
| 29 | - | since time.Time | |
| 30 | - | timerC <-chan time.Time | |
| 31 | - | timerStop func() bool | |
| 32 | - | running *flashTarget | |
| 33 | - | rect Rect | |
| 34 | - | start time.Time | |
| 35 | - | value float64 // Retained between this output's own flips. | |
| 36 | - | last flashTarget | |
| 37 | - | lastAt time.Time | |
| 28 | + | pending *flashTarget | |
| 29 | + | since time.Time | |
| 30 | + | timerC <-chan time.Time | |
| 31 | + | timerStop func() bool | |
| 32 | + | running *flashTarget | |
| 33 | + | rect Rect | |
| 34 | + | start time.Time | |
| 35 | + | value float64 // Retained between this output's own flips. | |
| 36 | + | lastStarts map[flashTarget]time.Time | |
| 38 | 37 | } | |
| 39 | 38 | ||
| 40 | 39 | func (c *Core) flashOn() bool { return c.animOn() && c.cfg.Focus.Flash && !c.protectionRequested() } | |
| @@ -112,10 +111,19 @@ func (c *Core) flashTick() bool { | |||
| 112 | 111 | } | |
| 113 | 112 | } | |
| 114 | 113 | } | |
| 115 | - | if target == f.last && now.Sub(f.lastAt) < flashCooldown { | |
| 114 | + | if at, ok := f.lastStarts[target]; ok && now.Sub(at) < flashCooldown { | |
| 116 | 115 | return false | |
| 117 | 116 | } | |
| 118 | - | f.running, f.rect, f.start, f.last, f.lastAt = &target, rect, now, target, now | |
| 117 | + | if f.lastStarts == nil { | |
| 118 | + | f.lastStarts = make(map[flashTarget]time.Time) | |
| 119 | + | } | |
| 120 | + | for key, at := range f.lastStarts { | |
| 121 | + | if now.Sub(at) >= flashCooldown { | |
| 122 | + | delete(f.lastStarts, key) | |
| 123 | + | } | |
| 124 | + | } | |
| 125 | + | f.lastStarts[target] = now | |
| 126 | + | f.running, f.rect, f.start = &target, rect, now | |
| 119 | 127 | f.value = c.cfg.Focus.FlashOpacity | |
| 120 | 128 | return true | |
| 121 | 129 | } | |
| @@ -129,6 +137,11 @@ func (c *Core) advanceFlash(only *screen) { | |||
| 129 | 137 | c.stopFlash() | |
| 130 | 138 | return | |
| 131 | 139 | } | |
| 140 | + | sc := c.screens[c.screenIndex(f.running.output)] | |
| 141 | + | if !c.flashVisible(*f.running, sc) { | |
| 142 | + | c.stopFlash() | |
| 143 | + | return | |
| 144 | + | } | |
| 132 | 145 | if only != nil && only.name() != f.running.output { | |
| 133 | 146 | return | |
| 134 | 147 | } | |
| @@ -142,6 +155,28 @@ func (c *Core) advanceFlash(only *screen) { | |||
| 142 | 155 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | |
| 143 | 156 | } | |
| 144 | 157 | ||
| 158 | + | // flashVisible checks the current layout, not leaving placements retained for | |
| 159 | + | // transitions: an old border must not outlive its target's visibility. | |
| 160 | + | func (c *Core) flashVisible(target flashTarget, sc *screen) bool { | |
| 161 | + | if sc.mon.ov.open { | |
| 162 | + | return false | |
| 163 | + | } | |
| 164 | + | if target.level == flashMonitor { | |
| 165 | + | return true | |
| 166 | + | } | |
| 167 | + | owner, _ := c.screenOf(target.window) | |
| 168 | + | if owner != sc || c.windows.lookup(target.window).client.ID == 0 { | |
| 169 | + | return false | |
| 170 | + | } | |
| 171 | + | view := sc.mon.viewport() | |
| 172 | + | for _, p := range sc.mon.Layout() { | |
| 173 | + | if p.ID == target.window { | |
| 174 | + | return !p.Fullscreen && p.Preview == 0 && view.shows(p) | |
| 175 | + | } | |
| 176 | + | } | |
| 177 | + | return false | |
| 178 | + | } | |
| 179 | + | ||
| 145 | 180 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | |
| 146 | 181 | f := &c.flash | |
| 147 | 182 | if !c.flashing(sc) { | |
| @@ -25,16 +25,15 @@ type flashTarget struct { | |||||
| 25 | } | 25 | } | ||
| 26 | 26 | ||||
| 27 | type focusFlash struct { | 27 | type focusFlash struct { | ||
| 28 | - | pending *flashTarget | 28 | + | pending *flashTarget |
| 29 | - | since time.Time | 29 | + | since time.Time |
| 30 | - | timerC <-chan time.Time | 30 | + | timerC <-chan time.Time |
| 31 | - | timerStop func() bool | 31 | + | timerStop func() bool |
| 32 | - | running *flashTarget | 32 | + | running *flashTarget |
| 33 | - | rect Rect | 33 | + | rect Rect |
| 34 | - | start time.Time | 34 | + | start time.Time |
| 35 | - | value float64 // Retained between this output's own flips. | 35 | + | value float64 // Retained between this output's own flips. |
| 36 | - | last flashTarget | 36 | + | lastStarts map[flashTarget]time.Time |
| 37 | - | lastAt time.Time | |||
| 38 | } | 37 | } | ||
| 39 | 38 | ||||
| 40 | func (c *Core) flashOn() bool { return c.animOn() && c.cfg.Focus.Flash && !c.protectionRequested() } | 39 | func (c *Core) flashOn() bool { return c.animOn() && c.cfg.Focus.Flash && !c.protectionRequested() } | ||
| @@ -112,10 +111,19 @@ func (c *Core) flashTick() bool { | |||||
| 112 | } | 111 | } | ||
| 113 | } | 112 | } | ||
| 114 | } | 113 | } | ||
| 115 | - | if target == f.last && now.Sub(f.lastAt) < flashCooldown { | 114 | + | if at, ok := f.lastStarts[target]; ok && now.Sub(at) < flashCooldown { |
| 116 | return false | 115 | return false | ||
| 117 | } | 116 | } | ||
| 118 | - | f.running, f.rect, f.start, f.last, f.lastAt = &target, rect, now, target, now | 117 | + | if f.lastStarts == nil { |
| 118 | + | f.lastStarts = make(map[flashTarget]time.Time) | |||
| 119 | + | } | |||
| 120 | + | for key, at := range f.lastStarts { | |||
| 121 | + | if now.Sub(at) >= flashCooldown { | |||
| 122 | + | delete(f.lastStarts, key) | |||
| 123 | + | } | |||
| 124 | + | } | |||
| 125 | + | f.lastStarts[target] = now | |||
| 126 | + | f.running, f.rect, f.start = &target, rect, now | |||
| 119 | f.value = c.cfg.Focus.FlashOpacity | 127 | f.value = c.cfg.Focus.FlashOpacity | ||
| 120 | return true | 128 | return true | ||
| 121 | } | 129 | } | ||
| @@ -129,6 +137,11 @@ func (c *Core) advanceFlash(only *screen) { | |||||
| 129 | c.stopFlash() | 137 | c.stopFlash() | ||
| 130 | return | 138 | return | ||
| 131 | } | 139 | } | ||
| 140 | + | sc := c.screens[c.screenIndex(f.running.output)] | |||
| 141 | + | if !c.flashVisible(*f.running, sc) { | |||
| 142 | + | c.stopFlash() | |||
| 143 | + | return | |||
| 144 | + | } | |||
| 132 | if only != nil && only.name() != f.running.output { | 145 | if only != nil && only.name() != f.running.output { | ||
| 133 | return | 146 | return | ||
| 134 | } | 147 | } | ||
| @@ -142,6 +155,28 @@ func (c *Core) advanceFlash(only *screen) { | |||||
| 142 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | 155 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | ||
| 143 | } | 156 | } | ||
| 144 | 157 | ||||
| 158 | + | // flashVisible checks the current layout, not leaving placements retained for | |||
| 159 | + | // transitions: an old border must not outlive its target's visibility. | |||
| 160 | + | func (c *Core) flashVisible(target flashTarget, sc *screen) bool { | |||
| 161 | + | if sc.mon.ov.open { | |||
| 162 | + | return false | |||
| 163 | + | } | |||
| 164 | + | if target.level == flashMonitor { | |||
| 165 | + | return true | |||
| 166 | + | } | |||
| 167 | + | owner, _ := c.screenOf(target.window) | |||
| 168 | + | if owner != sc || c.windows.lookup(target.window).client.ID == 0 { | |||
| 169 | + | return false | |||
| 170 | + | } | |||
| 171 | + | view := sc.mon.viewport() | |||
| 172 | + | for _, p := range sc.mon.Layout() { | |||
| 173 | + | if p.ID == target.window { | |||
| 174 | + | return !p.Fullscreen && p.Preview == 0 && view.shows(p) | |||
| 175 | + | } | |||
| 176 | + | } | |||
| 177 | + | return false | |||
| 178 | + | } | |||
| 179 | + | ||||
| 145 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | 180 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | ||
| 146 | f := &c.flash | 181 | f := &c.flash | ||
| 147 | if !c.flashing(sc) { | 182 | if !c.flashing(sc) { | ||
M internal/core/flash_test.go +70 -0
| @@ -397,3 +397,73 @@ func TestFlashNoImplicitFocusTriggers(t *testing.T) { | |||
| 397 | 397 | t.Fatal("pointer focus flashed") | |
| 398 | 398 | } | |
| 399 | 399 | } | |
| 400 | + | ||
| 401 | + | func TestFlashCancelsStaleRunningTargets(t *testing.T) { | |
| 402 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |
| 403 | + | for _, change := range []string{"overview", "output gone", "fullscreen", "unmapped", "hidden", "other output", "workspace changed"} { | |
| 404 | + | t.Run(string(rune('0'+level))+"/"+change, func(t *testing.T) { | |
| 405 | + | c, clock := flashCore(t) | |
| 406 | + | a := c.cur() | |
| 407 | + | c.addScreen(ports.OutputInfo{Name: "B", Width: 300, Height: 200}) | |
| 408 | + | b := c.screens[1] | |
| 409 | + | startFlash(t, c, clock, level, 3) | |
| 410 | + | if indicatorScene(t, c).Flash == nil { | |
| 411 | + | t.Fatal("setup: no running flash") | |
| 412 | + | } | |
| 413 | + | switch change { | |
| 414 | + | case "overview": | |
| 415 | + | a.mon.ToggleOverview() | |
| 416 | + | case "output gone": | |
| 417 | + | c.screens = []*screen{b} | |
| 418 | + | c.focusScreen = 0 | |
| 419 | + | case "fullscreen": | |
| 420 | + | a.mon.ToggleFullscreen() | |
| 421 | + | case "unmapped": | |
| 422 | + | c.windows.drop(3) | |
| 423 | + | case "hidden": | |
| 424 | + | w := a.mon.Current() | |
| 425 | + | w.ToggleWindowStash() | |
| 426 | + | w.ToggleStashVisible() | |
| 427 | + | case "other output": | |
| 428 | + | a.mon.RemoveWindow(3) | |
| 429 | + | b.mon.AddWindow(3) | |
| 430 | + | case "workspace changed": | |
| 431 | + | a.mon.Focus(a.mon.Active + 1) | |
| 432 | + | } | |
| 433 | + | // Cancellation must not wait for the target output's next flip. | |
| 434 | + | scenes := stepScenes(t, c, b) | |
| 435 | + | shouldCancel := level != flashMonitor || change == "overview" || change == "output gone" | |
| 436 | + | if shouldCancel { | |
| 437 | + | if scenes["A"].Flash != nil || c.flash.running != nil || c.animating() { | |
| 438 | + | t.Fatalf("stale flash retained: %+v, animating=%v", scenes["A"].Flash, c.animating()) | |
| 439 | + | } | |
| 440 | + | } else if scenes["A"].Flash == nil || !c.animating() { | |
| 441 | + | t.Fatal("monitor flash cancelled for a window change") | |
| 442 | + | } | |
| 443 | + | }) | |
| 444 | + | } | |
| 445 | + | } | |
| 446 | + | } | |
| 447 | + | ||
| 448 | + | func TestFlashCooldownRemembersEveryTarget(t *testing.T) { | |
| 449 | + | c, clock := flashCore(t) | |
| 450 | + | startFlash(t, c, clock, flashWindow, 3) | |
| 451 | + | first := c.flash.start | |
| 452 | + | startFlash(t, c, clock, flashWindow, 2) | |
| 453 | + | c.requestFlash(flashWindow, 3) | |
| 454 | + | clock.now = clock.now.Add(flashSettle) | |
| 455 | + | if c.flashTick() || c.flash.running != nil { | |
| 456 | + | t.Fatal("A flashed again after B within cooldown") | |
| 457 | + | } | |
| 458 | + | clock.now = first.Add(flashCooldown) | |
| 459 | + | startFlash(t, c, clock, flashWindow, 3) | |
| 460 | + | if c.flash.running.window != 3 { | |
| 461 | + | t.Fatal("A did not flash after cooldown") | |
| 462 | + | } | |
| 463 | + | // Inserting after all previous starts expire prunes old target keys. | |
| 464 | + | clock.now = clock.now.Add(flashCooldown) | |
| 465 | + | startFlash(t, c, clock, flashMonitor, 0) | |
| 466 | + | if len(c.flash.lastStarts) != 1 { | |
| 467 | + | t.Fatalf("expired cooldown entries retained: %v", c.flash.lastStarts) | |
| 468 | + | } | |
| 469 | + | } | |
| @@ -397,3 +397,73 @@ func TestFlashNoImplicitFocusTriggers(t *testing.T) { | |||||
| 397 | t.Fatal("pointer focus flashed") | 397 | t.Fatal("pointer focus flashed") | ||
| 398 | } | 398 | } | ||
| 399 | } | 399 | } | ||
| 400 | + | ||||
| 401 | + | func TestFlashCancelsStaleRunningTargets(t *testing.T) { | |||
| 402 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |||
| 403 | + | for _, change := range []string{"overview", "output gone", "fullscreen", "unmapped", "hidden", "other output", "workspace changed"} { | |||
| 404 | + | t.Run(string(rune('0'+level))+"/"+change, func(t *testing.T) { | |||
| 405 | + | c, clock := flashCore(t) | |||
| 406 | + | a := c.cur() | |||
| 407 | + | c.addScreen(ports.OutputInfo{Name: "B", Width: 300, Height: 200}) | |||
| 408 | + | b := c.screens[1] | |||
| 409 | + | startFlash(t, c, clock, level, 3) | |||
| 410 | + | if indicatorScene(t, c).Flash == nil { | |||
| 411 | + | t.Fatal("setup: no running flash") | |||
| 412 | + | } | |||
| 413 | + | switch change { | |||
| 414 | + | case "overview": | |||
| 415 | + | a.mon.ToggleOverview() | |||
| 416 | + | case "output gone": | |||
| 417 | + | c.screens = []*screen{b} | |||
| 418 | + | c.focusScreen = 0 | |||
| 419 | + | case "fullscreen": | |||
| 420 | + | a.mon.ToggleFullscreen() | |||
| 421 | + | case "unmapped": | |||
| 422 | + | c.windows.drop(3) | |||
| 423 | + | case "hidden": | |||
| 424 | + | w := a.mon.Current() | |||
| 425 | + | w.ToggleWindowStash() | |||
| 426 | + | w.ToggleStashVisible() | |||
| 427 | + | case "other output": | |||
| 428 | + | a.mon.RemoveWindow(3) | |||
| 429 | + | b.mon.AddWindow(3) | |||
| 430 | + | case "workspace changed": | |||
| 431 | + | a.mon.Focus(a.mon.Active + 1) | |||
| 432 | + | } | |||
| 433 | + | // Cancellation must not wait for the target output's next flip. | |||
| 434 | + | scenes := stepScenes(t, c, b) | |||
| 435 | + | shouldCancel := level != flashMonitor || change == "overview" || change == "output gone" | |||
| 436 | + | if shouldCancel { | |||
| 437 | + | if scenes["A"].Flash != nil || c.flash.running != nil || c.animating() { | |||
| 438 | + | t.Fatalf("stale flash retained: %+v, animating=%v", scenes["A"].Flash, c.animating()) | |||
| 439 | + | } | |||
| 440 | + | } else if scenes["A"].Flash == nil || !c.animating() { | |||
| 441 | + | t.Fatal("monitor flash cancelled for a window change") | |||
| 442 | + | } | |||
| 443 | + | }) | |||
| 444 | + | } | |||
| 445 | + | } | |||
| 446 | + | } | |||
| 447 | + | ||||
| 448 | + | func TestFlashCooldownRemembersEveryTarget(t *testing.T) { | |||
| 449 | + | c, clock := flashCore(t) | |||
| 450 | + | startFlash(t, c, clock, flashWindow, 3) | |||
| 451 | + | first := c.flash.start | |||
| 452 | + | startFlash(t, c, clock, flashWindow, 2) | |||
| 453 | + | c.requestFlash(flashWindow, 3) | |||
| 454 | + | clock.now = clock.now.Add(flashSettle) | |||
| 455 | + | if c.flashTick() || c.flash.running != nil { | |||
| 456 | + | t.Fatal("A flashed again after B within cooldown") | |||
| 457 | + | } | |||
| 458 | + | clock.now = first.Add(flashCooldown) | |||
| 459 | + | startFlash(t, c, clock, flashWindow, 3) | |||
| 460 | + | if c.flash.running.window != 3 { | |||
| 461 | + | t.Fatal("A did not flash after cooldown") | |||
| 462 | + | } | |||
| 463 | + | // Inserting after all previous starts expire prunes old target keys. | |||
| 464 | + | clock.now = clock.now.Add(flashCooldown) | |||
| 465 | + | startFlash(t, c, clock, flashMonitor, 0) | |||
| 466 | + | if len(c.flash.lastStarts) != 1 { | |||
| 467 | + | t.Fatalf("expired cooldown entries retained: %v", c.flash.lastStarts) | |||
| 468 | + | } | |||
| 469 | + | } | |||