fix(wayland): skip dead queued buffers without unmapping, keep frame callbacks on role destroy
- commit
- e8dd46cc7fd34a3151ddac398874b4aa4f22ce0b
- author
- brice · · 2026-09-25 19:17 UTC
- parent
- b754135
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +45 -5
M internal/adapters/wayland/fifo.go +12 -5
| @@ -127,7 +127,8 @@ func (h *timerHandler) SetTimestamp(r *committiming.WpCommitTimerV1, secHi, secL | |||
| 127 | 127 | } | |
| 128 | 128 | func (*timerHandler) Destroy(*committiming.WpCommitTimerV1) {} | |
| 129 | 129 | ||
| 130 | - | // maxTimestampAhead bounds how far a commit timestamp can hold a commit. | |
| 130 | + | // maxTimestampAhead bounds how far a commit timestamp can hold a commit; | |
| 131 | + | // a later one applies early (a deviation from "not before"). | |
| 131 | 132 | const maxTimestampAhead = time.Second | |
| 132 | 133 | ||
| 133 | 134 | // monotonicTime converts a CLOCK_MONOTONIC time to wall time, at most | |
| @@ -294,9 +295,10 @@ func (s *surface) queueUpdate() { | |||
| 294 | 295 | ||
| 295 | 296 | // applyUpdate applies a queued update, keeping the requests made since. | |
| 296 | 297 | func (s *surface) applyUpdate(u update) { | |
| 297 | - | // A buffer destroyed while queued (a swapchain resize) shows nothing. | |
| 298 | + | // A buffer destroyed while queued (a swapchain resize) is skipped: the | |
| 299 | + | // surface keeps its content, and is not unmapped behind the client. | |
| 298 | 300 | if u.buffer != nil && !u.buffer.Resource.Alive() { | |
| 299 | - | u.buffer = nil | |
| 301 | + | u.buffer, u.attached = nil, false | |
| 300 | 302 | } | |
| 301 | 303 | later := s.takePending() | |
| 302 | 304 | s.putPending(u) | |
| @@ -305,11 +307,16 @@ func (s *surface) applyUpdate(u update) { | |||
| 305 | 307 | } | |
| 306 | 308 | ||
| 307 | 309 | // dropQueue discards the queued updates of a destroyed surface or role. | |
| 310 | + | // A live surface still gets its queued frame callbacks. | |
| 308 | 311 | func (s *surface) dropQueue() { | |
| 309 | 312 | released := map[*server.Resource]bool{} | |
| 310 | 313 | for _, u := range s.queue { | |
| 311 | - | for _, cb := range u.callbacks { | |
| 312 | - | cb.Destroy() | |
| 314 | + | if s.destroyed { | |
| 315 | + | for _, cb := range u.callbacks { | |
| 316 | + | cb.Destroy() | |
| 317 | + | } | |
| 318 | + | } else if len(u.callbacks) > 0 { | |
| 319 | + | s.server.queueFrames(s.server.frameOutput(s), u.callbacks) | |
| 313 | 320 | } | |
| 314 | 321 | if u.buffer != nil && !sameBuffer(u.buffer, s.current) && !released[u.buffer.Resource] && u.buffer.Resource.Alive() { | |
| 315 | 322 | released[u.buffer.Resource] = true | |
| @@ -127,7 +127,8 @@ func (h *timerHandler) SetTimestamp(r *committiming.WpCommitTimerV1, secHi, secL | |||||
| 127 | } | 127 | } | ||
| 128 | func (*timerHandler) Destroy(*committiming.WpCommitTimerV1) {} | 128 | func (*timerHandler) Destroy(*committiming.WpCommitTimerV1) {} | ||
| 129 | 129 | ||||
| 130 | - | // maxTimestampAhead bounds how far a commit timestamp can hold a commit. | 130 | + | // maxTimestampAhead bounds how far a commit timestamp can hold a commit; |
| 131 | + | // a later one applies early (a deviation from "not before"). | |||
| 131 | const maxTimestampAhead = time.Second | 132 | const maxTimestampAhead = time.Second | ||
| 132 | 133 | ||||
| 133 | // monotonicTime converts a CLOCK_MONOTONIC time to wall time, at most | 134 | // monotonicTime converts a CLOCK_MONOTONIC time to wall time, at most | ||
| @@ -294,9 +295,10 @@ func (s *surface) queueUpdate() { | |||||
| 294 | 295 | ||||
| 295 | // applyUpdate applies a queued update, keeping the requests made since. | 296 | // applyUpdate applies a queued update, keeping the requests made since. | ||
| 296 | func (s *surface) applyUpdate(u update) { | 297 | func (s *surface) applyUpdate(u update) { | ||
| 297 | - | // A buffer destroyed while queued (a swapchain resize) shows nothing. | 298 | + | // A buffer destroyed while queued (a swapchain resize) is skipped: the |
| 299 | + | // surface keeps its content, and is not unmapped behind the client. | |||
| 298 | if u.buffer != nil && !u.buffer.Resource.Alive() { | 300 | if u.buffer != nil && !u.buffer.Resource.Alive() { | ||
| 299 | - | u.buffer = nil | 301 | + | u.buffer, u.attached = nil, false |
| 300 | } | 302 | } | ||
| 301 | later := s.takePending() | 303 | later := s.takePending() | ||
| 302 | s.putPending(u) | 304 | s.putPending(u) | ||
| @@ -305,11 +307,16 @@ func (s *surface) applyUpdate(u update) { | |||||
| 305 | } | 307 | } | ||
| 306 | 308 | ||||
| 307 | // dropQueue discards the queued updates of a destroyed surface or role. | 309 | // dropQueue discards the queued updates of a destroyed surface or role. | ||
| 310 | + | // A live surface still gets its queued frame callbacks. | |||
| 308 | func (s *surface) dropQueue() { | 311 | func (s *surface) dropQueue() { | ||
| 309 | released := map[*server.Resource]bool{} | 312 | released := map[*server.Resource]bool{} | ||
| 310 | for _, u := range s.queue { | 313 | for _, u := range s.queue { | ||
| 311 | - | for _, cb := range u.callbacks { | 314 | + | if s.destroyed { |
| 312 | - | cb.Destroy() | 315 | + | for _, cb := range u.callbacks { |
| 316 | + | cb.Destroy() | |||
| 317 | + | } | |||
| 318 | + | } else if len(u.callbacks) > 0 { | |||
| 319 | + | s.server.queueFrames(s.server.frameOutput(s), u.callbacks) | |||
| 313 | } | 320 | } | ||
| 314 | if u.buffer != nil && !sameBuffer(u.buffer, s.current) && !released[u.buffer.Resource] && u.buffer.Resource.Alive() { | 321 | if u.buffer != nil && !sameBuffer(u.buffer, s.current) && !released[u.buffer.Resource] && u.buffer.Resource.Alive() { | ||
| 315 | released[u.buffer.Resource] = true | 322 | released[u.buffer.Resource] = true | ||
M internal/adapters/wayland/fifo_test.go +33 -0
| @@ -170,3 +170,36 @@ func TestTickFifoClearsBarrierOnFlip(t *testing.T) { | |||
| 170 | 170 | t.Fatal("barrier kept after the flip") | |
| 171 | 171 | } | |
| 172 | 172 | } | |
| 173 | + | ||
| 174 | + | // A queued commit whose buffer is destroyed (a swapchain resize) is skipped | |
| 175 | + | // without unmapping the window. | |
| 176 | + | func TestFifoQueuedBufferDestroyed(t *testing.T) { | |
| 177 | + | s, events, _, contents, dir := contentServer(t) | |
| 178 | + | c := protocolClient(t, s, dir) | |
| 179 | + | _, surf, _ := surfaceMapper(t, c, events)() | |
| 180 | + | manager := bindProtocol(t, c, "wp_fifo_manager_v1") | |
| 181 | + | f := c.AllocateID() | |
| 182 | + | registerProtocol(t, c, f) | |
| 183 | + | requestProtocol(t, c, manager, fifo.WpFifoManagerV1RequestGetFifo, f, surf) | |
| 184 | + | buf := shmBuffer(t, c) | |
| 185 | + | requestProtocol(t, c, f, fifo.WpFifoV1RequestSetBarrier) | |
| 186 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestAttach, buf, int32(0), int32(0)) | |
| 187 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestCommit) | |
| 188 | + | // Queued behind the barrier, then its buffer goes away. | |
| 189 | + | requestProtocol(t, c, f, fifo.WpFifoV1RequestWaitBarrier) | |
| 190 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestAttach, buf, int32(0), int32(0)) | |
| 191 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestCommit) | |
| 192 | + | requestProtocol(t, c, buf, wayland.BufferRequestDestroy) | |
| 193 | + | if err := c.Roundtrip(); err != nil { | |
| 194 | + | t.Fatal(err) | |
| 195 | + | } | |
| 196 | + | drainContents(contents) | |
| 197 | + | time.Sleep(3 * defaultFramePeriod) | |
| 198 | + | select { | |
| 199 | + | case ev := <-events: | |
| 200 | + | if _, ok := ev.(ports.WindowUnmapped); ok { | |
| 201 | + | t.Fatal("window unmapped by a dead queued buffer") | |
| 202 | + | } | |
| 203 | + | default: | |
| 204 | + | } | |
| 205 | + | } | |
| @@ -170,3 +170,36 @@ func TestTickFifoClearsBarrierOnFlip(t *testing.T) { | |||||
| 170 | t.Fatal("barrier kept after the flip") | 170 | t.Fatal("barrier kept after the flip") | ||
| 171 | } | 171 | } | ||
| 172 | } | 172 | } | ||
| 173 | + | ||||
| 174 | + | // A queued commit whose buffer is destroyed (a swapchain resize) is skipped | |||
| 175 | + | // without unmapping the window. | |||
| 176 | + | func TestFifoQueuedBufferDestroyed(t *testing.T) { | |||
| 177 | + | s, events, _, contents, dir := contentServer(t) | |||
| 178 | + | c := protocolClient(t, s, dir) | |||
| 179 | + | _, surf, _ := surfaceMapper(t, c, events)() | |||
| 180 | + | manager := bindProtocol(t, c, "wp_fifo_manager_v1") | |||
| 181 | + | f := c.AllocateID() | |||
| 182 | + | registerProtocol(t, c, f) | |||
| 183 | + | requestProtocol(t, c, manager, fifo.WpFifoManagerV1RequestGetFifo, f, surf) | |||
| 184 | + | buf := shmBuffer(t, c) | |||
| 185 | + | requestProtocol(t, c, f, fifo.WpFifoV1RequestSetBarrier) | |||
| 186 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestAttach, buf, int32(0), int32(0)) | |||
| 187 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestCommit) | |||
| 188 | + | // Queued behind the barrier, then its buffer goes away. | |||
| 189 | + | requestProtocol(t, c, f, fifo.WpFifoV1RequestWaitBarrier) | |||
| 190 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestAttach, buf, int32(0), int32(0)) | |||
| 191 | + | requestProtocol(t, c, surf, wayland.SurfaceRequestCommit) | |||
| 192 | + | requestProtocol(t, c, buf, wayland.BufferRequestDestroy) | |||
| 193 | + | if err := c.Roundtrip(); err != nil { | |||
| 194 | + | t.Fatal(err) | |||
| 195 | + | } | |||
| 196 | + | drainContents(contents) | |||
| 197 | + | time.Sleep(3 * defaultFramePeriod) | |||
| 198 | + | select { | |||
| 199 | + | case ev := <-events: | |||
| 200 | + | if _, ok := ev.(ports.WindowUnmapped); ok { | |||
| 201 | + | t.Fatal("window unmapped by a dead queued buffer") | |||
| 202 | + | } | |||
| 203 | + | default: | |||
| 204 | + | } | |||
| 205 | + | } | |||