The comment above wayland_surface_clear_role says that we are only allowed to give it the same role, would that still hold with this change which introduces different roles? It's not very obvious that it is from the code, when windows are getting reparented for instance.
To keep this commit more simple and focused, it introduces support for different roles, but not for role changes. In this commit if we detect a role change we bail out to avoid getting disconnected by the compositor:
``` * TODO: Recreate the surface to allow role change. */ if (surface && role && surface->role && role != surface->role) goto out;``` ```
Proper support for role changes is added a few commits later: https://gitlab.winehq.org/wine/wine/-/merge_requests/6107/diffs?commit_id=f3...
I think it would be cleaner to better separate the two wayland surface use cases - one which uses top-level wayland surfaces for windowing purposes, and the other one which uses wayland sub-surfaces in OpenGL/Vulkan only. Maybe it would even be better to not share that much code between the two, they are pretty much separate and GL/VK doesn't need any of the windowing logic.
OpenGL/Vulkan are already using a separate mechanism, the `wayland_client_surface`, that shields them from all the unnecessary `wayland_surface` details (e.g., child vs toplevel is completely transparent). While developing this MR I experimented with using that client surface mechanism directly to support accel. child rendering, but that turned to be unproductive because of all the details that still need to be handled for proper sync of child windows and their corresponding surfaces (e.g., nesting and transitive positioning) but also reparenting and locking. I found it more straightforward to manage the complexity by dealing with a single `wayland_surface` that abstracts most of the differences.
The other benefit of the proposed approach, is that we can trivially use the same mechanism to support proper positioning of tooltips/menus by giving them the subsurface role (I already have a prototype based on this MR that I was hoping to refine and propose).
Generally speaking I don't think we want to create dedicated host surfaces for child windows, the Win32 compositing rules are already implemented more or less correctly and in a driver-independent way. This also means that some of the later changes in this MR aren't going to be necessary.
The main function of the child subsurfaces in this MR is to be available in case their respective child windows become the target of accelerated rendering, but are mostly inert otherwise. Only the [last commit](https://gitlab.winehq.org/wine/wine/-/merge_requests/6107/diffs?commit_id=ab...) uses them to display some GDI contents, but as [discussed](https://gitlab.winehq.org/wine/wine/-/merge_requests/6248#note_78531) we can probably drop it.
Later in the series (https://gitlab.winehq.org/wine/wine/-/merge_requests/6107/diffs?commit_id=38...) we even switch to creating child subsurface only on demand to avoid polluting the compositor with inert objects.