Merge pull request #6 from bnema/fix/fade-multi-output
fix(visual): keep fade redraws until every output drew its end
2 files · +59 -15
M internal/adapters/visual/fade.go +38 -15
| @@ -18,12 +18,18 @@ type fadeCmd struct { | |||
| 18 | 18 | dur time.Duration | |
| 19 | 19 | } | |
| 20 | 20 | ||
| 21 | - | // fadeModel is the overlay's animation state. The view (NeferGUI owner loop) | |
| 22 | - | // owns the fields below "owned"; other goroutines only use send. | |
| 21 | + | // fadeModel is the overlay's animation state, shared by the surfaces of every | |
| 22 | + | // output. The view (NeferGUI owner loop) owns the fields below "owned"; other | |
| 23 | + | // goroutines only use send and animatingAt. | |
| 23 | 24 | type fadeModel struct { | |
| 24 | - | cmds chan fadeCmd | |
| 25 | - | wake chan struct{} | |
| 26 | - | animating atomic.Bool | |
| 25 | + | cmds chan fadeCmd | |
| 26 | + | wake chan struct{} | |
| 27 | + | // base is the monotonic origin for tickUntil; fixed at construction. | |
| 28 | + | base time.Time | |
| 29 | + | // tickUntil keeps redraws coming past the fade end, as nanoseconds since | |
| 30 | + | // base (monotonic, like fadeAlpha). It must not depend on which surface | |
| 31 | + | // drew last: a wake redraws all surfaces, but each draws at its own instant. | |
| 32 | + | tickUntil atomic.Int64 | |
| 27 | 33 | // finish ends the run once a reveal completed; set by the runner before | |
| 28 | 34 | // the loop starts. | |
| 29 | 35 | finish context.CancelFunc | |
| @@ -36,7 +42,7 @@ type fadeModel struct { | |||
| 36 | 42 | } | |
| 37 | 43 | ||
| 38 | 44 | func newFadeModel() *fadeModel { | |
| 39 | - | return &fadeModel{cmds: make(chan fadeCmd, 4), wake: make(chan struct{}, 1)} | |
| 45 | + | return &fadeModel{cmds: make(chan fadeCmd, 4), wake: make(chan struct{}, 1), base: time.Now()} | |
| 40 | 46 | } | |
| 41 | 47 | ||
| 42 | 48 | // send queues a retarget (dropping the oldest when full) and requests a redraw. | |
| @@ -76,9 +82,22 @@ func fadeCSS(a float64) string { | |||
| 76 | 82 | return fmt.Sprintf("width:100%%;height:100%%;background-color:rgb(0 0 0 / %.1f%%)", a*100) | |
| 77 | 83 | } | |
| 78 | 84 | ||
| 85 | + | // animatingAt reports whether the ticker must still request redraws. | |
| 86 | + | func (m *fadeModel) animatingAt(now time.Time) bool { | |
| 87 | + | return int64(now.Sub(m.base)) < m.tickUntil.Load() | |
| 88 | + | } | |
| 89 | + | ||
| 79 | 90 | // fadeView applies queued retargets, then draws the overlay for this instant. | |
| 80 | 91 | func fadeView(f *nefergui.Frame, m *fadeModel) { | |
| 81 | - | now := time.Now() | |
| 92 | + | a, done := m.step(time.Now()) | |
| 93 | + | f.Root(nefergui.Inline(fadeCSS(a))) | |
| 94 | + | if done && m.started && m.to == 0 && m.finish != nil { | |
| 95 | + | m.finish() | |
| 96 | + | } | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | // step applies queued retargets and returns the opacity at now. | |
| 100 | + | func (m *fadeModel) step(now time.Time) (float64, bool) { | |
| 82 | 101 | for { | |
| 83 | 102 | select { | |
| 84 | 103 | case c := <-m.cmds: | |
| @@ -88,17 +107,16 @@ func fadeView(f *nefergui.Frame, m *fadeModel) { | |||
| 88 | 107 | m.to = 1 | |
| 89 | 108 | } | |
| 90 | 109 | m.start, m.dur, m.started = now, c.dur, true | |
| 110 | + | // Any wake at or after the end marks every surface dirty until it | |
| 111 | + | // draws, so that draw shows the final opacity. The extra ticks are | |
| 112 | + | // margin for ticker jitter. | |
| 113 | + | m.tickUntil.Store(int64(now.Add(c.dur + 2*fadeTick).Sub(m.base))) | |
| 91 | 114 | continue | |
| 92 | 115 | default: | |
| 93 | 116 | } | |
| 94 | 117 | break | |
| 95 | 118 | } | |
| 96 | - | a, done := fadeAlpha(m.from, m.to, m.start, m.dur, now) | |
| 97 | - | m.animating.Store(!done) | |
| 98 | - | f.Root(nefergui.Inline(fadeCSS(a))) | |
| 99 | - | if done && m.started && m.to == 0 && m.finish != nil { | |
| 100 | - | m.finish() | |
| 101 | - | } | |
| 119 | + | return fadeAlpha(m.from, m.to, m.start, m.dur, now) | |
| 102 | 120 | } | |
| 103 | 121 | ||
| 104 | 122 | // fadeRunner runs the overlay until ctx ends or a reveal completes. | |
| @@ -116,14 +134,19 @@ func (guiFader) RunFade(ctx context.Context, m *fadeModel) error { | |||
| 116 | 134 | go func() { | |
| 117 | 135 | t := time.NewTicker(fadeTick) | |
| 118 | 136 | defer t.Stop() | |
| 137 | + | was := false | |
| 119 | 138 | for { | |
| 120 | 139 | select { | |
| 121 | 140 | case <-ctx.Done(): | |
| 122 | 141 | return | |
| 123 | - | case <-t.C: | |
| 124 | - | if m.animating.Load() { | |
| 142 | + | case now := <-t.C: | |
| 143 | + | // One more poke when animation stops: even a starved ticker | |
| 144 | + | // then wakes every surface at least once past the end. | |
| 145 | + | animating := m.animatingAt(now) | |
| 146 | + | if animating || was { | |
| 125 | 147 | m.poke() | |
| 126 | 148 | } | |
| 149 | + | was = animating | |
| 127 | 150 | } | |
| 128 | 151 | } | |
| 129 | 152 | }() | |
| @@ -18,12 +18,18 @@ type fadeCmd struct { | |||||
| 18 | dur time.Duration | 18 | dur time.Duration | ||
| 19 | } | 19 | } | ||
| 20 | 20 | ||||
| 21 | - | // fadeModel is the overlay's animation state. The view (NeferGUI owner loop) | 21 | + | // fadeModel is the overlay's animation state, shared by the surfaces of every |
| 22 | - | // owns the fields below "owned"; other goroutines only use send. | 22 | + | // output. The view (NeferGUI owner loop) owns the fields below "owned"; other |
| 23 | + | // goroutines only use send and animatingAt. | |||
| 23 | type fadeModel struct { | 24 | type fadeModel struct { | ||
| 24 | - | cmds chan fadeCmd | 25 | + | cmds chan fadeCmd |
| 25 | - | wake chan struct{} | 26 | + | wake chan struct{} |
| 26 | - | animating atomic.Bool | 27 | + | // base is the monotonic origin for tickUntil; fixed at construction. |
| 28 | + | base time.Time | |||
| 29 | + | // tickUntil keeps redraws coming past the fade end, as nanoseconds since | |||
| 30 | + | // base (monotonic, like fadeAlpha). It must not depend on which surface | |||
| 31 | + | // drew last: a wake redraws all surfaces, but each draws at its own instant. | |||
| 32 | + | tickUntil atomic.Int64 | |||
| 27 | // finish ends the run once a reveal completed; set by the runner before | 33 | // finish ends the run once a reveal completed; set by the runner before | ||
| 28 | // the loop starts. | 34 | // the loop starts. | ||
| 29 | finish context.CancelFunc | 35 | finish context.CancelFunc | ||
| @@ -36,7 +42,7 @@ type fadeModel struct { | |||||
| 36 | } | 42 | } | ||
| 37 | 43 | ||||
| 38 | func newFadeModel() *fadeModel { | 44 | func newFadeModel() *fadeModel { | ||
| 39 | - | return &fadeModel{cmds: make(chan fadeCmd, 4), wake: make(chan struct{}, 1)} | 45 | + | return &fadeModel{cmds: make(chan fadeCmd, 4), wake: make(chan struct{}, 1), base: time.Now()} |
| 40 | } | 46 | } | ||
| 41 | 47 | ||||
| 42 | // send queues a retarget (dropping the oldest when full) and requests a redraw. | 48 | // send queues a retarget (dropping the oldest when full) and requests a redraw. | ||
| @@ -76,9 +82,22 @@ func fadeCSS(a float64) string { | |||||
| 76 | return fmt.Sprintf("width:100%%;height:100%%;background-color:rgb(0 0 0 / %.1f%%)", a*100) | 82 | return fmt.Sprintf("width:100%%;height:100%%;background-color:rgb(0 0 0 / %.1f%%)", a*100) | ||
| 77 | } | 83 | } | ||
| 78 | 84 | ||||
| 85 | + | // animatingAt reports whether the ticker must still request redraws. | |||
| 86 | + | func (m *fadeModel) animatingAt(now time.Time) bool { | |||
| 87 | + | return int64(now.Sub(m.base)) < m.tickUntil.Load() | |||
| 88 | + | } | |||
| 89 | + | ||||
| 79 | // fadeView applies queued retargets, then draws the overlay for this instant. | 90 | // fadeView applies queued retargets, then draws the overlay for this instant. | ||
| 80 | func fadeView(f *nefergui.Frame, m *fadeModel) { | 91 | func fadeView(f *nefergui.Frame, m *fadeModel) { | ||
| 81 | - | now := time.Now() | 92 | + | a, done := m.step(time.Now()) |
| 93 | + | f.Root(nefergui.Inline(fadeCSS(a))) | |||
| 94 | + | if done && m.started && m.to == 0 && m.finish != nil { | |||
| 95 | + | m.finish() | |||
| 96 | + | } | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | // step applies queued retargets and returns the opacity at now. | |||
| 100 | + | func (m *fadeModel) step(now time.Time) (float64, bool) { | |||
| 82 | for { | 101 | for { | ||
| 83 | select { | 102 | select { | ||
| 84 | case c := <-m.cmds: | 103 | case c := <-m.cmds: | ||
| @@ -88,17 +107,16 @@ func fadeView(f *nefergui.Frame, m *fadeModel) { | |||||
| 88 | m.to = 1 | 107 | m.to = 1 | ||
| 89 | } | 108 | } | ||
| 90 | m.start, m.dur, m.started = now, c.dur, true | 109 | m.start, m.dur, m.started = now, c.dur, true | ||
| 110 | + | // Any wake at or after the end marks every surface dirty until it | |||
| 111 | + | // draws, so that draw shows the final opacity. The extra ticks are | |||
| 112 | + | // margin for ticker jitter. | |||
| 113 | + | m.tickUntil.Store(int64(now.Add(c.dur + 2*fadeTick).Sub(m.base))) | |||
| 91 | continue | 114 | continue | ||
| 92 | default: | 115 | default: | ||
| 93 | } | 116 | } | ||
| 94 | break | 117 | break | ||
| 95 | } | 118 | } | ||
| 96 | - | a, done := fadeAlpha(m.from, m.to, m.start, m.dur, now) | 119 | + | return fadeAlpha(m.from, m.to, m.start, m.dur, now) |
| 97 | - | m.animating.Store(!done) | |||
| 98 | - | f.Root(nefergui.Inline(fadeCSS(a))) | |||
| 99 | - | if done && m.started && m.to == 0 && m.finish != nil { | |||
| 100 | - | m.finish() | |||
| 101 | - | } | |||
| 102 | } | 120 | } | ||
| 103 | 121 | ||||
| 104 | // fadeRunner runs the overlay until ctx ends or a reveal completes. | 122 | // fadeRunner runs the overlay until ctx ends or a reveal completes. | ||
| @@ -116,14 +134,19 @@ func (guiFader) RunFade(ctx context.Context, m *fadeModel) error { | |||||
| 116 | go func() { | 134 | go func() { | ||
| 117 | t := time.NewTicker(fadeTick) | 135 | t := time.NewTicker(fadeTick) | ||
| 118 | defer t.Stop() | 136 | defer t.Stop() | ||
| 137 | + | was := false | |||
| 119 | for { | 138 | for { | ||
| 120 | select { | 139 | select { | ||
| 121 | case <-ctx.Done(): | 140 | case <-ctx.Done(): | ||
| 122 | return | 141 | return | ||
| 123 | - | case <-t.C: | 142 | + | case now := <-t.C: |
| 124 | - | if m.animating.Load() { | 143 | + | // One more poke when animation stops: even a starved ticker |
| 144 | + | // then wakes every surface at least once past the end. | |||
| 145 | + | animating := m.animatingAt(now) | |||
| 146 | + | if animating || was { | |||
| 125 | m.poke() | 147 | m.poke() | ||
| 126 | } | 148 | } | ||
| 149 | + | was = animating | |||
| 127 | } | 150 | } | ||
| 128 | } | 151 | } | ||
| 129 | }() | 152 | }() | ||
M internal/adapters/visual/view_test.go +21 -0
| @@ -20,6 +20,27 @@ func TestFadeAlphaInterpolatesAndArrives(t *testing.T) { | |||
| 20 | 20 | require.True(t, done) | |
| 21 | 21 | } | |
| 22 | 22 | ||
| 23 | + | // Surfaces share the model but draw at different instants: a surface that | |
| 24 | + | // drew just after the end must not stop redraws for one that drew just before. | |
| 25 | + | func TestFadeKeepsTickingUntilEverySurfaceDrewTheEnd(t *testing.T) { | |
| 26 | + | m := newFadeModel() | |
| 27 | + | m.send(fadeCmd{black: true, dur: time.Second}) | |
| 28 | + | t0 := m.base.Add(time.Minute) | |
| 29 | + | a, done := m.step(t0) | |
| 30 | + | require.Equal(t, 0.0, a) | |
| 31 | + | require.False(t, done) | |
| 32 | + | ||
| 33 | + | before := t0.Add(time.Second - time.Millisecond) | |
| 34 | + | after := t0.Add(time.Second + time.Millisecond) | |
| 35 | + | _, done = m.step(before) // surface A | |
| 36 | + | require.False(t, done) | |
| 37 | + | a, done = m.step(after) // surface B reaches the end | |
| 38 | + | require.Equal(t, 1.0, a) | |
| 39 | + | require.True(t, done) | |
| 40 | + | require.True(t, m.animatingAt(after), "surface A still needs a redraw") | |
| 41 | + | require.False(t, m.animatingAt(t0.Add(time.Hour))) | |
| 42 | + | } | |
| 43 | + | ||
| 23 | 44 | func TestFadeLayerIsClickThroughOverlayOnEveryOutput(t *testing.T) { | |
| 24 | 45 | l := fadeLayer() | |
| 25 | 46 | require.True(t, l.AllOutputs) | |
| @@ -20,6 +20,27 @@ func TestFadeAlphaInterpolatesAndArrives(t *testing.T) { | |||||
| 20 | require.True(t, done) | 20 | require.True(t, done) | ||
| 21 | } | 21 | } | ||
| 22 | 22 | ||||
| 23 | + | // Surfaces share the model but draw at different instants: a surface that | |||
| 24 | + | // drew just after the end must not stop redraws for one that drew just before. | |||
| 25 | + | func TestFadeKeepsTickingUntilEverySurfaceDrewTheEnd(t *testing.T) { | |||
| 26 | + | m := newFadeModel() | |||
| 27 | + | m.send(fadeCmd{black: true, dur: time.Second}) | |||
| 28 | + | t0 := m.base.Add(time.Minute) | |||
| 29 | + | a, done := m.step(t0) | |||
| 30 | + | require.Equal(t, 0.0, a) | |||
| 31 | + | require.False(t, done) | |||
| 32 | + | ||||
| 33 | + | before := t0.Add(time.Second - time.Millisecond) | |||
| 34 | + | after := t0.Add(time.Second + time.Millisecond) | |||
| 35 | + | _, done = m.step(before) // surface A | |||
| 36 | + | require.False(t, done) | |||
| 37 | + | a, done = m.step(after) // surface B reaches the end | |||
| 38 | + | require.Equal(t, 1.0, a) | |||
| 39 | + | require.True(t, done) | |||
| 40 | + | require.True(t, m.animatingAt(after), "surface A still needs a redraw") | |||
| 41 | + | require.False(t, m.animatingAt(t0.Add(time.Hour))) | |||
| 42 | + | } | |||
| 43 | + | ||||
| 23 | func TestFadeLayerIsClickThroughOverlayOnEveryOutput(t *testing.T) { | 44 | func TestFadeLayerIsClickThroughOverlayOnEveryOutput(t *testing.T) { | ||
| 24 | l := fadeLayer() | 45 | l := fadeLayer() | ||
| 25 | require.True(t, l.AllOutputs) | 46 | require.True(t, l.AllOutputs) | ||