Apps constantly resizing

[AlanGriffiths]

I've seen an intermittent (maybe once a week) "constantly resizing" when dragging edges to resize chromium-browser windows and some electron apps. Probably the same underlying cause as #3450. (A ping-pong of compositor requested sizes and buffers of a different size being submitted.)

@tarek-y-ismail says "I don't really have issues causing the bug. I just launch bomber and resize a couple of times"

And he's right!

[RAOF]

So, a bomber resize loop:

12.9263 A: [email protected](width=421, height=257, states=Unknown: 'array[4]') ↲
12.9263 A: [email protected](serial=4460) ↲
12.9264 A: → [email protected]_window_geometry(x=0, y=0, width=421, height=257)
12.9264 A: → [email protected]_region(id=new wl_region@40aih)
12.9264 A: → [email protected](x=3, y=30, width=415, height=224)
12.9264 A: → [email protected]_opaque_region(region=wl_region@40aih)
12.9264 A: → [email protected]()
12.9264 A: → [email protected]_configure(serial=4460)
12.9264 A: [email protected]_id(id=44) -- [email protected] after 0.0007s ↲
12.9264 A: [email protected]_id(id=37) -- [email protected] after 0.0023s ↲
12.9264 A: [email protected]_id(id=32) -- [email protected] after 0.0023s ↲
12.9265 A: → [email protected]()
12.9265 A: → [email protected]()
12.9265 A: → [email protected]_pool(id=new wl_shm_pool@32acm, fd=fd 74, size=432788)
12.9265 A: → [email protected]_buffer(id=new wl_buffer@37abl, offset=0, width=421, height=257, stride=1684, format=0:argb8888)
12.9269 A: → [email protected]_buffer(x=0, y=0, width=421, height=30)
12.9269 A: → [email protected]_buffer(x=0, y=30, width=3, height=224)
12.9269 A: → [email protected]_buffer(x=418, y=30, width=3, height=224)
12.9269 A: → [email protected]_buffer(x=0, y=254, width=421, height=3)
12.9269 A: → [email protected]_buffer(x=0, y=257, width=3, height=30)
12.9269 A: → [email protected](buffer=wl_buffer@37abl, x=0, y=0)
12.9269 A: → [email protected]_buffer(x=3, y=30, width=415, height=224)
12.9269 A: → [email protected]()
12.9269 A: [email protected]() ↲
12.9269 A: [email protected](width=408, height=248, states=Unknown: 'array[4]') ↲
12.9269 A: [email protected](serial=4461) ↲
12.9270 A: → [email protected]_window_geometry(x=0, y=0, width=408, height=248)
12.9270 A: → [email protected]_region(id=new wl_region@44aim)
12.9270 A: → [email protected](x=3, y=30, width=402, height=215)
12.9270 A: → [email protected]_opaque_region(region=wl_region@44aim)
12.9270 A: → [email protected]()
12.9270 A: → [email protected]_configure(serial=4461)
12.9270 A: [email protected]_id(id=40) -- [email protected] after 0.0007s ↲
12.9270 A: [email protected]_id(id=42) -- [email protected] after 0.0023s ↲
12.9270 A: [email protected]_id(id=22) -- [email protected] after 0.0023s ↲
12.9271 A: → [email protected]()
12.9271 A: → [email protected]()
12.9271 A: → [email protected]_pool(id=new wl_shm_pool@22ahp, fd=fd 74, size=404736)
12.9271 A: → [email protected]_buffer(id=new wl_buffer@42aiq, offset=0, width=408, height=248, stride=1632, format=0:argb8888)
12.9275 A: → [email protected]_buffer(x=0, y=0, width=408, height=30)
12.9275 A: → [email protected]_buffer(x=0, y=30, width=3, height=215)
12.9275 A: → [email protected]_buffer(x=405, y=30, width=3, height=215)
12.9275 A: → [email protected]_buffer(x=0, y=245, width=408, height=3)
12.9275 A: → [email protected]_buffer(x=0, y=248, width=3, height=30)
12.9275 A: → [email protected](buffer=wl_buffer@42aiq, x=0, y=0)
12.9275 A: → [email protected]_buffer(x=3, y=30, width=402, height=215)
12.9275 A: → [email protected]()
12.9275 A: [email protected]() ↲
12.9275 A: [email protected](width=421, height=257, states=Unknown: 'array[4]') ↲
12.9275 A: [email protected](serial=4462) ↲

This is wild, because it isn't even interleaved - we're sending a configure, the client is acking it and committing the size we requested. And then we're requesting a different size, the client is acking it and committing the newly requested size, and then we...

[RAOF]

So, the underlying problem is that we have a feedback loop: the client submits a buffer, which results in the surface size changing, which notifies the window manager which results in the Wayland frontend sending a configure, and so on.

We should break this cycle by not having WaylandSurfaceObserver listen to content_resized_to. Instead, we should have an explicit Window Manager -> frontend “please resize your window” call, that results in a configure event being sent (and may, or may not, actually result in the window being resized!).

[RAOF]

Additionally, this particular behaviour does not seem to be exhibited by GTK apps. My suspicion is that this is because GTK apps call xdg_surface.set_window_geometry with values that are not the surface width & height (because GTK draws client-side shadows), whereas Qt is sending the surface width and height here (as Qt is not drawing client-side shadows).

[RAOF]

Ok! Further analysis!

Here's a log of bomber exhibiting the bug. On the left, we have Mir's view; -> foo indicates messages to the client. On the right, we have bomber's view; -> bar here indicates messages to Mir

│[2024-08-28 17:25:00.434765] < - debug - > mirserver: Calling modify_surface; pending_changes has (959×597)           ││[1099131.817] {Default Queue} xdg_toplevel#27.configure(959, 670, array[4])                                          │
│[2024-08-28 17:25:00.434788] < - debug - > OHMYGOD: Resize! From (959×670) to (959×597) (content size (959×597))      ││[1099131.819] {Default Queue} xdg_surface#26.configure(5154)                                                         │
│[1099132.279]  -> xdg_toplevel#27.configure(959, 597, array[4])                                                       ││[1099131.842] {Default Queue}  -> xdg_surface#26.set_window_geometry(0, 0, 959, 670)                                 │
│[1099132.285]  -> xdg_surface#26.configure(5155)                                                                      ││[1099131.845] {Default Queue}  -> wl_compositor#4.create_region(new id wl_region#42)                                 │
│[1099132.347] xdg_surface#26.set_window_geometry(0, 0, 959, 670)                                                      ││[1099131.848] {Default Queue}  -> wl_region#42.add(3, 30, 953, 637)                                                  │
│[1099132.352] wl_compositor#4.create_region(new id wl_region#42)                                                      ││[1099131.850] {Default Queue}  -> wl_surface#20.set_opaque_region(wl_region#42)                                      │
│[1099132.358] wl_region#42.add(3, 30, 953, 637)                                                                       ││[1099131.852] {Default Queue}  -> wl_region#42.destroy()                                                             │
│[1099132.362] wl_surface#20.set_opaque_region(wl_region#42)                                                           ││[1099131.854] {Default Queue}  -> xdg_surface#26.ack_configure(5154)                                                 │
│[1099132.366] wl_region#42.destroy()                                                                                  ││[1099132.073] {Default Queue}  -> wl_shm_pool#23.destroy()                                                           │
│[1099132.370]  -> wl_display#1.delete_id(42)                                                                          ││[1099132.079] {Default Queue}  -> wl_buffer#22.destroy()                                                             │
│[1099132.374] xdg_surface#26.ack_configure(5154)                                                                      ││[1099132.094] {Default Queue}  -> wl_shm#11.create_pool(new id wl_shm_pool#41, fd 101, 2570120)                      │
│[1099132.378] wl_shm_pool#23.destroy()                                                                                ││[1099132.097] {Default Queue}  -> wl_shm_pool#41.create_buffer(new id wl_buffer#21, 0, 959, 670, 3836, 0)            │
│[1099132.382]  -> wl_display#1.delete_id(23)                                                                          ││[1099132.319] {Display Queue} wl_display#1.delete_id(45)                                                             │
│[1099132.386] wl_buffer#22.destroy()                                                                                  ││[1099132.327] {Display Queue} wl_display#1.delete_id(46)                                                             │
│[1099132.612]  -> wl_display#1.delete_id(22)                                                                          ││[1099132.330] {Display Queue} wl_display#1.delete_id(44)                                                             │
│[1099132.617] wl_shm#11.create_pool(new id wl_shm_pool#41, fd 51, 2570120)                                            ││[1099133.479] {Default Queue}  -> wl_surface#20.damage_buffer(0, 0, 959, 30)                                         │
│[1099132.633] wl_shm_pool#41.create_buffer(new id wl_buffer#21, 0, 959, 670, 3836, 0)                                 ││[1099133.486] {Default Queue}  -> wl_surface#20.damage_buffer(0, 30, 3, 637)                                         │
│[1099133.585] wl_surface#20.damage_buffer(0, 0, 959, 30)                                                              ││[1099133.489] {Default Queue}  -> wl_surface#20.damage_buffer(956, 30, 3, 637)                                       │
│[1099133.591] wl_surface#20.damage_buffer(0, 30, 3, 637)                                                              ││[1099133.491] {Default Queue}  -> wl_surface#20.damage_buffer(0, 667, 959, 3)                                        │
│[1099133.595] wl_surface#20.damage_buffer(956, 30, 3, 637)                                                            ││[1099133.494] {Default Queue}  -> wl_surface#20.damage_buffer(0, 670, 3, 30)                                         │
│[1099133.599] wl_surface#20.damage_buffer(0, 667, 959, 3)                                                             ││[1099133.553] {Default Queue}  -> wl_surface#20.attach(wl_buffer#21, 0, 0)                                           │
│[1099133.604] wl_surface#20.damage_buffer(0, 670, 3, 30)                                                              ││[1099133.555] {Default Queue}  -> wl_surface#20.damage_buffer(3, 30, 953, 637)                                       │
│[1099133.608] wl_surface#20.attach(wl_buffer#21, 0, 0)                                                                ││[1099133.558] {Default Queue}  -> wl_surface#20.commit()                                                             │
│[1099133.613] wl_surface#20.damage_buffer(3, 30, 953, 637)                                                            ││[1099133.565] {Default Queue} wl_buffer#40.release()                                                                 │
│[1099133.617] wl_surface#20.commit()                                                                                  ││[1099133.568] {Default Queue} xdg_toplevel#27.configure(959, 597, array[4])                                          │
│[1099133.626]  -> wl_buffer#31.release()                                                                              ││[1099133.570] {Default Queue} xdg_surface#26.configure(5155)                                                         │
│[2024-08-28 17:25:00.436166] < - debug - > mirserver: Calling modify_surface; pending_changes has (959×670)           ││[1099133.592] {Default Queue}  -> xdg_surface#26.set_window_geometry(0, 0, 959, 597)                                 │
│[2024-08-28 17:25:00.436187] < - debug - > OHMYGOD: Resize! From (959×597) to (959×670) (content size (959×670))      ││[1099133.595] {Default Queue}  -> wl_compositor#4.create_region(new id wl_region#44)                                 │
│[1099133.671]  -> xdg_toplevel#27.configure(959, 670, array[4])                                                       ││[1099133.597] {Default Queue}  -> wl_region#44.add(3, 30, 953, 564)                                                  │
│[1099133.679]  -> xdg_surface#26.configure(5156)                                                                      ││[1099133.599] {Default Queue}  -> wl_surface#20.set_opaque_region(wl_region#44)                                      │
│[1099133.779] xdg_surface#26.set_window_geometry(0, 0, 959, 597)                                                      ││[1099133.602] {Default Queue}  -> wl_region#44.destroy()                                                             │
│[1099133.790] wl_compositor#4.create_region(new id wl_region#44)                                                      ││[1099133.604] {Default Queue}  -> xdg_surface#26.ack_configure(5155)                                                 │
│[1099133.796] wl_region#44.add(3, 30, 953, 564)                                                                       ││[1099133.741] {Display Queue} wl_display#1.delete_id(42)                                                             │
│[1099133.802] wl_surface#20.set_opaque_region(wl_region#44)                                                           ││[1099133.755] {Display Queue} wl_display#1.delete_id(23)                                                             │
│[1099133.807] wl_region#44.destroy()                                                                                  ││[1099133.759] {Display Queue} wl_display#1.delete_id(22)                                                             │
│[1099133.811]  -> wl_display#1.delete_id(44)                                                                          ││[1099133.823] {Default Queue}  -> wl_shm_pool#39.destroy()                                                           │
│[1099133.816] xdg_surface#26.ack_configure(5155)                                                                      ││[1099133.829] {Default Queue}  -> wl_buffer#40.destroy()                                                             │
│[1099135.319] wl_shm_pool#39.destroy()                                                                                ││[1099133.848] {Default Queue}  -> wl_shm#11.create_pool(new id wl_shm_pool#22, fd 101, 2290092)                      │
│[1099135.325]  -> wl_display#1.delete_id(39)                                                                          ││[1099133.851] {Default Queue}  -> wl_shm_pool#22.create_buffer(new id wl_buffer#23, 0, 959, 597, 3836, 0)            │
│[1099135.332] wl_buffer#40.destroy()                                                                                  ││[1099135.197] {Default Queue}  -> wl_surface#20.damage_buffer(0, 0, 959, 30)                                         │
│[1099135.592]  -> wl_display#1.delete_id(40)                                                                          ││[1099135.205] {Default Queue}  -> wl_surface#20.damage_buffer(0, 30, 3, 564)                                         │
│[1099135.598] wl_shm#11.create_pool(new id wl_shm_pool#22, fd 54, 2290092)                                            ││[1099135.207] {Default Queue}  -> wl_surface#20.damage_buffer(956, 30, 3, 564)                                       │
│[1099135.621] wl_shm_pool#22.create_buffer(new id wl_buffer#23, 0, 959, 597, 3836, 0)                                 ││[1099135.210] {Default Queue}  -> wl_surface#20.damage_buffer(0, 594, 959, 3)                                        │
│[1099135.629] wl_surface#20.damage_buffer(0, 0, 959, 30)                                                              ││[1099135.213] {Default Queue}  -> wl_surface#20.damage_buffer(0, 597, 3, 30)                                         │
│[1099135.634] wl_surface#20.damage_buffer(0, 30, 3, 564)                                                              ││[1099135.276] {Default Queue}  -> wl_surface#20.attach(wl_buffer#23, 0, 0)                                           │
│[1099135.638] wl_surface#20.damage_buffer(956, 30, 3, 564)                                                            ││[1099135.280] {Default Queue}  -> wl_surface#20.damage_buffer(3, 30, 953, 564)                                       │
│[1099135.642] wl_surface#20.damage_buffer(0, 594, 959, 3)                                                             ││[1099135.281] {Default Queue}  -> wl_surface#20.commit()                                                             │
│[1099135.648] wl_surface#20.damage_buffer(0, 597, 3, 30)                                                              ││[1099135.291] {Default Queue} wl_buffer#31.release()                                                                 │
│[1099135.653] wl_surface#20.attach(wl_buffer#23, 0, 0)                                                                ││[1099135.294] {Default Queue} xdg_toplevel#27.configure(959, 670, array[4])                                          │
│[1099135.659] wl_surface#20.damage_buffer(3, 30, 953, 564)                                                            ││[1099135.297] {Default Queue} xdg_surface#26.configure(5156)                                                         │

As you can see, from the client's perspective everything is fine, if maybe a bit strange - it receives xdg_toplevel.configure(959, 670, ...), xdg_surface.configure(5154), and then proceeds to act on them - setting the window geometry, ack_configure(5154)ing, attaching a 959×670 buffer, and committing it, upon which it immediately receives a configure xdg_surface.configure(5155), and goes about acting on that.

From Mir's perspective, though, it has received a commit() of a 959x670 surface, resulting in the WM resizing the surface and sending xdg_toplevel.configure(959, 670, ...), xdg_surface.configure(5154). But the 959x670 surface has been as a part of the client's ack_configure(5152). Immediately after sending the xdg_surface.configure(5154), Mir receives ack_configure(5153) from the client, and then the associated window state for that configure, including a 959x597 buffer, which the client commit()s, resulting in the WM resizing the surface and sending xdg_toplevel.configure(959, 597, ...), xdg_surface.configure(5155), and so the cycle continues.

Fundamentally, the problem here is that the client resizing the window by committing a buffer should not result in a xdg_toplevel.configure event - the client doesn't need to be told that it has resized the window, it just did it! This is awkward, though, because currently the mechanism for “the WM would like the client to change the size of its surface, please” is the same mechanism as “the client has submitted a buffer of a different size to last time; recalculate WM and compositor state”.

We probably don't want to radically rethink the WM infrastructure for 2.18, so as a cludge for now we should be able to do something with matching the ack_configure serial from the client with the serials of the configures we've sent, and... not send additional configures until the client has caught up? Or something along those lines.