refactor(wayland): drop test-only branches from the pacer
signalPacer no longer returns early when the server has no display, and paceNow/paceTick no longer special-case a missing pace timer: production always has both, since New registers them. paceFixture now builds what New provides for pacing (a display that never runs, so posted ticks stay queued, and the registered pace timer through openPaceTimer). TestCommitWakesPacerWithoutTimer covered the removed fallback and is dropped; the timer-armed cases keep their coverage.
- commit
- 4e9be6c7014515508e38da47733e6e67d1ca73ff
- author
- brice · · 2026-10-14 03:42 UTC
- committer
- brice · · 2026-10-14 04:34 UTC
- parent
- ba55b3c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +22 -38
M internal/adapters/wayland/fifo.go +0 -3
| @@ -772,8 +772,5 @@ func (s *Server) signalPacer() { | |||
| 772 | 772 | if !s.paceSignal.CompareAndSwap(false, true) { | |
| 773 | 773 | return | |
| 774 | 774 | } | |
| 775 | - | if s.display == nil { // unit tests that build a Server without a display | |
| 776 | - | return | |
| 777 | - | } | |
| 778 | 775 | s.display.Post(s.paceSignalFn) | |
| 779 | 776 | } | |
| @@ -772,8 +772,5 @@ func (s *Server) signalPacer() { | |||||
| 772 | if !s.paceSignal.CompareAndSwap(false, true) { | 772 | if !s.paceSignal.CompareAndSwap(false, true) { | ||
| 773 | return | 773 | return | ||
| 774 | } | 774 | } | ||
| 775 | - | if s.display == nil { // unit tests that build a Server without a display | |||
| 776 | - | return | |||
| 777 | - | } | |||
| 778 | s.display.Post(s.paceSignalFn) | 775 | s.display.Post(s.paceSignalFn) | ||
| 779 | } | 776 | } | ||
M internal/adapters/wayland/frames.go +2 -12
| @@ -31,15 +31,8 @@ const suspendedFrameQueue = "\x00suspended" // never a wl_output name | |||
| 31 | 31 | const maxPacePasses = 3 | |
| 32 | 32 | ||
| 33 | 33 | // paceNow runs a tick from code already on the display goroutine (a client | |
| 34 | - | // commit). Without a registered pace timer (unit tests that build a Server | |
| 35 | - | // without a display) it falls back to signalling a tick. | |
| 36 | - | func (s *Server) paceNow() { | |
| 37 | - | if !s.paceReady { | |
| 38 | - | s.signalPacer() | |
| 39 | - | return | |
| 40 | - | } | |
| 41 | - | s.paceTick(nil, nil) | |
| 42 | - | } | |
| 34 | + | // commit). | |
| 35 | + | func (s *Server) paceNow() { s.paceTick(nil, nil) } | |
| 43 | 36 | ||
| 44 | 37 | // paceTolerance is how much earlier than the armed deadline a new one may be | |
| 45 | 38 | // and still reuse it: the ticks of one frame compute the same deadline from | |
| @@ -144,9 +137,6 @@ func (s *Server) paceTick(flipped map[string]bool, reports []ports.OutputPresent | |||
| 144 | 137 | break | |
| 145 | 138 | } | |
| 146 | 139 | } | |
| 147 | - | if !s.paceReady { | |
| 148 | - | return | |
| 149 | - | } | |
| 150 | 140 | if idle { | |
| 151 | 141 | // Nothing waits: sleep until a commit queues a callback | |
| 152 | 142 | // (wakePacer) or a flip. | |
| @@ -31,15 +31,8 @@ const suspendedFrameQueue = "\x00suspended" // never a wl_output name | |||||
| 31 | const maxPacePasses = 3 | 31 | const maxPacePasses = 3 | ||
| 32 | 32 | ||||
| 33 | // paceNow runs a tick from code already on the display goroutine (a client | 33 | // paceNow runs a tick from code already on the display goroutine (a client | ||
| 34 | - | // commit). Without a registered pace timer (unit tests that build a Server | 34 | + | // commit). |
| 35 | - | // without a display) it falls back to signalling a tick. | 35 | + | func (s *Server) paceNow() { s.paceTick(nil, nil) } |
| 36 | - | func (s *Server) paceNow() { | |||
| 37 | - | if !s.paceReady { | |||
| 38 | - | s.signalPacer() | |||
| 39 | - | return | |||
| 40 | - | } | |||
| 41 | - | s.paceTick(nil, nil) | |||
| 42 | - | } | |||
| 43 | 36 | ||||
| 44 | // paceTolerance is how much earlier than the armed deadline a new one may be | 37 | // paceTolerance is how much earlier than the armed deadline a new one may be | ||
| 45 | // and still reuse it: the ticks of one frame compute the same deadline from | 38 | // and still reuse it: the ticks of one frame compute the same deadline from | ||
| @@ -144,9 +137,6 @@ func (s *Server) paceTick(flipped map[string]bool, reports []ports.OutputPresent | |||||
| 144 | break | 137 | break | ||
| 145 | } | 138 | } | ||
| 146 | } | 139 | } | ||
| 147 | - | if !s.paceReady { | |||
| 148 | - | return | |||
| 149 | - | } | |||
| 150 | if idle { | 140 | if idle { | ||
| 151 | // Nothing waits: sleep until a commit queues a callback | 141 | // Nothing waits: sleep until a commit queues a callback | ||
| 152 | // (wakePacer) or a flip. | 142 | // (wakePacer) or a flip. | ||
M internal/adapters/wayland/pace_inline_test.go +20 -23
| @@ -6,15 +6,24 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/bnema/go-wayland-bindings/server/wayland" | |
| 8 | 8 | "github.com/bnema/neferwl/internal/ports" | |
| 9 | + | "github.com/bnema/purego-libwayland/server" | |
| 9 | 10 | "golang.org/x/sys/unix" | |
| 10 | 11 | ) | |
| 11 | 12 | ||
| 12 | 13 | // paceFixture is a server whose surface waits on an output whose next | |
| 13 | - | // frame is an hour away: a commit queues its callback and nothing fires. | |
| 14 | - | func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | |
| 14 | + | // frame is an hour away: a commit queues its callback and nothing fires. It | |
| 15 | + | // has what New gives a server for pacing: a display (never run, so posted | |
| 16 | + | // ticks stay queued) and the registered pace timer. | |
| 17 | + | func paceFixture(t *testing.T) (*Server, *surface) { | |
| 15 | 18 | t.Helper() | |
| 19 | + | display, err := server.NewDisplay() | |
| 20 | + | if err != nil { | |
| 21 | + | t.Fatal(err) | |
| 22 | + | } | |
| 23 | + | t.Cleanup(display.Close) | |
| 16 | 24 | out := &output{place: ports.OutputPlacement{Info: ports.OutputInfo{Name: "DP-2", RefreshMilli: 60000}}} | |
| 17 | 25 | s := &Server{ | |
| 26 | + | display: display, | |
| 18 | 27 | outputs: []*output{out}, | |
| 19 | 28 | awaiting: map[string][]*wayland.Callback{}, | |
| 20 | 29 | frameDue: map[string]time.Time{"DP-2": time.Now().Add(time.Hour)}, | |
| @@ -23,14 +32,11 @@ func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | |||
| 23 | 32 | flips: map[string]ports.FlipInfo{}, | |
| 24 | 33 | reports: map[string]ports.OutputPresented{}, | |
| 25 | 34 | } | |
| 26 | - | if timer { | |
| 27 | - | fd, err := unix.TimerfdCreate(unix.CLOCK_MONOTONIC, unix.TFD_NONBLOCK|unix.TFD_CLOEXEC) | |
| 28 | - | if err != nil { | |
| 29 | - | t.Fatal(err) | |
| 30 | - | } | |
| 31 | - | t.Cleanup(func() { unix.Close(fd) }) | |
| 32 | - | s.paceFD, s.paceReady = fd, true | |
| 35 | + | s.paceSignalFn = s.paceSignalled | |
| 36 | + | if err := s.openPaceTimer(); err != nil { | |
| 37 | + | t.Fatal(err) | |
| 33 | 38 | } | |
| 39 | + | t.Cleanup(func() { unix.Close(s.paceFD) }) | |
| 34 | 40 | w := &window{hasLast: true, last: ports.ConfigureWindow{Output: "DP-2", Visible: true}} | |
| 35 | 41 | surf := &surface{server: s, xdg: &xdgSurface{window: w}} | |
| 36 | 42 | surf.next.callbacks = []*wayland.Callback{new(wayland.Callback)} | |
| @@ -41,7 +47,7 @@ func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | |||
| 41 | 47 | // pace timer itself and posts no tick, which would only run later on the | |
| 42 | 48 | // display goroutine it is already on. | |
| 43 | 49 | func TestCommitTicksPacerInline(t *testing.T) { | |
| 44 | - | s, surf := paceFixture(t, true) | |
| 50 | + | s, surf := paceFixture(t) | |
| 45 | 51 | surf.queueAndTick() | |
| 46 | 52 | if len(s.awaiting["DP-2"]) != 1 { | |
| 47 | 53 | t.Fatalf("callback not queued: %+v", s.awaiting) | |
| @@ -55,18 +61,9 @@ func TestCommitTicksPacerInline(t *testing.T) { | |||
| 55 | 61 | } | |
| 56 | 62 | } | |
| 57 | 63 | ||
| 58 | - | // Without a registered pace timer the wake falls back to a posted tick. | |
| 59 | - | func TestCommitWakesPacerWithoutTimer(t *testing.T) { | |
| 60 | - | s, surf := paceFixture(t, false) | |
| 61 | - | surf.queueAndTick() | |
| 62 | - | if !s.paceSignal.Load() { | |
| 63 | - | t.Fatal("tick not signalled") | |
| 64 | - | } | |
| 65 | - | } | |
| 66 | - | ||
| 67 | 64 | // Wakes inside a tick coalesce into one more pass, not one posted tick each. | |
| 68 | 65 | func TestWakePacerDefersInsideTick(t *testing.T) { | |
| 69 | - | s, _ := paceFixture(t, true) | |
| 66 | + | s, _ := paceFixture(t) | |
| 70 | 67 | s.paceDepth = 1 | |
| 71 | 68 | s.wakePacer() | |
| 72 | 69 | s.wakePacer() | |
| @@ -82,7 +79,7 @@ func TestWakePacerDefersInsideTick(t *testing.T) { | |||
| 82 | 79 | ||
| 83 | 80 | // A commit that waits (a fifo barrier) is timed by the inline tick. | |
| 84 | 81 | func TestCommitWaitingOnBarrierArmsTimer(t *testing.T) { | |
| 85 | - | s, surf := paceFixture(t, true) | |
| 82 | + | s, surf := paceFixture(t) | |
| 86 | 83 | surf.next.callbacks = nil | |
| 87 | 84 | surf.barrier, surf.barrierAt = true, time.Now() | |
| 88 | 85 | surf.next.wait = true | |
| @@ -113,7 +110,7 @@ func waitArmed(t *testing.T, s *Server, msg string) { | |||
| 113 | 110 | // A synchronized subsurface commit waits for its parent's commit: it does | |
| 114 | 111 | // not tick the pacer, so a burst of child commits costs no ticks. | |
| 115 | 112 | func TestSyncedCommitDoesNotTick(t *testing.T) { | |
| 116 | - | s, surf := paceFixture(t, true) | |
| 113 | + | s, surf := paceFixture(t) | |
| 117 | 114 | surf.sub.parent = &surface{server: s} | |
| 118 | 115 | surf.sub.synced = true | |
| 119 | 116 | surf.queueAndTick() | |
| @@ -133,7 +130,7 @@ func TestSyncedCommitDoesNotTick(t *testing.T) { | |||
| 133 | 130 | // timer; another deadline re-arms it; an expiry consumed by onPaceTimer, or a | |
| 134 | 131 | // stop, leaves it disarmed, and a stop of a disarmed timer does nothing. | |
| 135 | 132 | func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | |
| 136 | - | s, _ := paceFixture(t, true) | |
| 133 | + | s, _ := paceFixture(t) | |
| 137 | 134 | remaining := func() time.Duration { | |
| 138 | 135 | var cur unix.ItimerSpec | |
| 139 | 136 | if err := unix.TimerfdGettime(s.paceFD, &cur); err != nil { | |
| @@ -6,15 +6,24 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/bnema/go-wayland-bindings/server/wayland" | 7 | "github.com/bnema/go-wayland-bindings/server/wayland" | ||
| 8 | "github.com/bnema/neferwl/internal/ports" | 8 | "github.com/bnema/neferwl/internal/ports" | ||
| 9 | + | "github.com/bnema/purego-libwayland/server" | |||
| 9 | "golang.org/x/sys/unix" | 10 | "golang.org/x/sys/unix" | ||
| 10 | ) | 11 | ) | ||
| 11 | 12 | ||||
| 12 | // paceFixture is a server whose surface waits on an output whose next | 13 | // paceFixture is a server whose surface waits on an output whose next | ||
| 13 | - | // frame is an hour away: a commit queues its callback and nothing fires. | 14 | + | // frame is an hour away: a commit queues its callback and nothing fires. It |
| 14 | - | func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | 15 | + | // has what New gives a server for pacing: a display (never run, so posted |
| 16 | + | // ticks stay queued) and the registered pace timer. | |||
| 17 | + | func paceFixture(t *testing.T) (*Server, *surface) { | |||
| 15 | t.Helper() | 18 | t.Helper() | ||
| 19 | + | display, err := server.NewDisplay() | |||
| 20 | + | if err != nil { | |||
| 21 | + | t.Fatal(err) | |||
| 22 | + | } | |||
| 23 | + | t.Cleanup(display.Close) | |||
| 16 | out := &output{place: ports.OutputPlacement{Info: ports.OutputInfo{Name: "DP-2", RefreshMilli: 60000}}} | 24 | out := &output{place: ports.OutputPlacement{Info: ports.OutputInfo{Name: "DP-2", RefreshMilli: 60000}}} | ||
| 17 | s := &Server{ | 25 | s := &Server{ | ||
| 26 | + | display: display, | |||
| 18 | outputs: []*output{out}, | 27 | outputs: []*output{out}, | ||
| 19 | awaiting: map[string][]*wayland.Callback{}, | 28 | awaiting: map[string][]*wayland.Callback{}, | ||
| 20 | frameDue: map[string]time.Time{"DP-2": time.Now().Add(time.Hour)}, | 29 | frameDue: map[string]time.Time{"DP-2": time.Now().Add(time.Hour)}, | ||
| @@ -23,14 +32,11 @@ func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | |||||
| 23 | flips: map[string]ports.FlipInfo{}, | 32 | flips: map[string]ports.FlipInfo{}, | ||
| 24 | reports: map[string]ports.OutputPresented{}, | 33 | reports: map[string]ports.OutputPresented{}, | ||
| 25 | } | 34 | } | ||
| 26 | - | if timer { | 35 | + | s.paceSignalFn = s.paceSignalled |
| 27 | - | fd, err := unix.TimerfdCreate(unix.CLOCK_MONOTONIC, unix.TFD_NONBLOCK|unix.TFD_CLOEXEC) | 36 | + | if err := s.openPaceTimer(); err != nil { |
| 28 | - | if err != nil { | 37 | + | t.Fatal(err) |
| 29 | - | t.Fatal(err) | |||
| 30 | - | } | |||
| 31 | - | t.Cleanup(func() { unix.Close(fd) }) | |||
| 32 | - | s.paceFD, s.paceReady = fd, true | |||
| 33 | } | 38 | } | ||
| 39 | + | t.Cleanup(func() { unix.Close(s.paceFD) }) | |||
| 34 | w := &window{hasLast: true, last: ports.ConfigureWindow{Output: "DP-2", Visible: true}} | 40 | w := &window{hasLast: true, last: ports.ConfigureWindow{Output: "DP-2", Visible: true}} | ||
| 35 | surf := &surface{server: s, xdg: &xdgSurface{window: w}} | 41 | surf := &surface{server: s, xdg: &xdgSurface{window: w}} | ||
| 36 | surf.next.callbacks = []*wayland.Callback{new(wayland.Callback)} | 42 | surf.next.callbacks = []*wayland.Callback{new(wayland.Callback)} | ||
| @@ -41,7 +47,7 @@ func paceFixture(t *testing.T, timer bool) (*Server, *surface) { | |||||
| 41 | // pace timer itself and posts no tick, which would only run later on the | 47 | // pace timer itself and posts no tick, which would only run later on the | ||
| 42 | // display goroutine it is already on. | 48 | // display goroutine it is already on. | ||
| 43 | func TestCommitTicksPacerInline(t *testing.T) { | 49 | func TestCommitTicksPacerInline(t *testing.T) { | ||
| 44 | - | s, surf := paceFixture(t, true) | 50 | + | s, surf := paceFixture(t) |
| 45 | surf.queueAndTick() | 51 | surf.queueAndTick() | ||
| 46 | if len(s.awaiting["DP-2"]) != 1 { | 52 | if len(s.awaiting["DP-2"]) != 1 { | ||
| 47 | t.Fatalf("callback not queued: %+v", s.awaiting) | 53 | t.Fatalf("callback not queued: %+v", s.awaiting) | ||
| @@ -55,18 +61,9 @@ func TestCommitTicksPacerInline(t *testing.T) { | |||||
| 55 | } | 61 | } | ||
| 56 | } | 62 | } | ||
| 57 | 63 | ||||
| 58 | - | // Without a registered pace timer the wake falls back to a posted tick. | |||
| 59 | - | func TestCommitWakesPacerWithoutTimer(t *testing.T) { | |||
| 60 | - | s, surf := paceFixture(t, false) | |||
| 61 | - | surf.queueAndTick() | |||
| 62 | - | if !s.paceSignal.Load() { | |||
| 63 | - | t.Fatal("tick not signalled") | |||
| 64 | - | } | |||
| 65 | - | } | |||
| 66 | - | ||||
| 67 | // Wakes inside a tick coalesce into one more pass, not one posted tick each. | 64 | // Wakes inside a tick coalesce into one more pass, not one posted tick each. | ||
| 68 | func TestWakePacerDefersInsideTick(t *testing.T) { | 65 | func TestWakePacerDefersInsideTick(t *testing.T) { | ||
| 69 | - | s, _ := paceFixture(t, true) | 66 | + | s, _ := paceFixture(t) |
| 70 | s.paceDepth = 1 | 67 | s.paceDepth = 1 | ||
| 71 | s.wakePacer() | 68 | s.wakePacer() | ||
| 72 | s.wakePacer() | 69 | s.wakePacer() | ||
| @@ -82,7 +79,7 @@ func TestWakePacerDefersInsideTick(t *testing.T) { | |||||
| 82 | 79 | ||||
| 83 | // A commit that waits (a fifo barrier) is timed by the inline tick. | 80 | // A commit that waits (a fifo barrier) is timed by the inline tick. | ||
| 84 | func TestCommitWaitingOnBarrierArmsTimer(t *testing.T) { | 81 | func TestCommitWaitingOnBarrierArmsTimer(t *testing.T) { | ||
| 85 | - | s, surf := paceFixture(t, true) | 82 | + | s, surf := paceFixture(t) |
| 86 | surf.next.callbacks = nil | 83 | surf.next.callbacks = nil | ||
| 87 | surf.barrier, surf.barrierAt = true, time.Now() | 84 | surf.barrier, surf.barrierAt = true, time.Now() | ||
| 88 | surf.next.wait = true | 85 | surf.next.wait = true | ||
| @@ -113,7 +110,7 @@ func waitArmed(t *testing.T, s *Server, msg string) { | |||||
| 113 | // A synchronized subsurface commit waits for its parent's commit: it does | 110 | // A synchronized subsurface commit waits for its parent's commit: it does | ||
| 114 | // not tick the pacer, so a burst of child commits costs no ticks. | 111 | // not tick the pacer, so a burst of child commits costs no ticks. | ||
| 115 | func TestSyncedCommitDoesNotTick(t *testing.T) { | 112 | func TestSyncedCommitDoesNotTick(t *testing.T) { | ||
| 116 | - | s, surf := paceFixture(t, true) | 113 | + | s, surf := paceFixture(t) |
| 117 | surf.sub.parent = &surface{server: s} | 114 | surf.sub.parent = &surface{server: s} | ||
| 118 | surf.sub.synced = true | 115 | surf.sub.synced = true | ||
| 119 | surf.queueAndTick() | 116 | surf.queueAndTick() | ||
| @@ -133,7 +130,7 @@ func TestSyncedCommitDoesNotTick(t *testing.T) { | |||||
| 133 | // timer; another deadline re-arms it; an expiry consumed by onPaceTimer, or a | 130 | // timer; another deadline re-arms it; an expiry consumed by onPaceTimer, or a | ||
| 134 | // stop, leaves it disarmed, and a stop of a disarmed timer does nothing. | 131 | // stop, leaves it disarmed, and a stop of a disarmed timer does nothing. | ||
| 135 | func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | 132 | func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | ||
| 136 | - | s, _ := paceFixture(t, true) | 133 | + | s, _ := paceFixture(t) |
| 137 | remaining := func() time.Duration { | 134 | remaining := func() time.Duration { | ||
| 138 | var cur unix.ItimerSpec | 135 | var cur unix.ItimerSpec | ||
| 139 | if err := unix.TimerfdGettime(s.paceFD, &cur); err != nil { | 136 | if err := unix.TimerfdGettime(s.paceFD, &cur); err != nil { | ||