On Wed Aug 28 08:37:08 2024 +0000, Alexandros Frantzis wrote:
Thanks, this approach is serializing access across all Wayland surfaces, but I don't think that's a big loss and the simplicity it brings seems to be worth it. This broader locking scheme has introduced a new deadlock situation, though, between `window_surface->mutex` and `win_data_mutex`: Thread 1: **window_surface_lock** -> window_surface_flush -> {get,set}_window_surface_contents -> **wayland_win_data_get** Thread 2: WAYLAND_WindowPosChanged -> **wayland_win_data_get** -> wayland_window_surface_set_visible_rect -> **window_surface_lock** An example where I am seeing this consistently is while resizing firefox. Moving the call to `wayland_window_surface_set_visible_rect` after `wayland_win_data_release` in `WAYLAND_WindowPosChanged` seems to be a reasonable fix?
Sounds reasonable, then I'm tempted to get rid of this window surface buffer queue recreation entirely, removing the need for `wayland_window_surface_set_visible_rect` altogether. The only reason we do it I believe, is because buffers need to match the surface dimensions at the moment, but what about using `wp_viewport_set_source` to crop the buffers instead?
This would let us use the surface dimensions for the buffers, and only reallocate the queue and buffers when surface needs to be re-created, which seems to be its intended usage (we round the surface rectangles to multiple of 128 for that purpose).
It seems that the only thing preventing that is that wp_viewporter is currently optional, do you think it would be an issue to make it a requirement? I think it will let us make the code much simpler, and even allow further integration into win32u, allowing win32u to draw directly to the buffer for instance.