Skip to content
Snippets Groups Projects

winewayland: Use subsurfaces for unmanaged windows.

Merged Rémi Bernon requested to merge rbernon/wine:mr/wayland-subsurfaces into master
3 unresolved threads

Merge request reports

Merge request pipeline #32524 skipped

Merge request pipeline skipped for b061022a

Merged by Alexandre JulliardAlexandre Julliard 4 months ago (Sep 27, 2024 9:10pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Rémi Bernon requested review from @afrantzis

    requested review from @afrantzis

    • Author Developer

      Btw, after this I feel like the driver is much more usable, would it be acceptable to enable it by default? Is there any other major feature missing (given that virtual display settings is being worked on)?

    • Clipboard support is completely missing which was present on the downstream driver (and exposing multiple display modes too but that may be related to virtual display stuff)

    • Does resolution scaling still ignore aspect ratio for fullscreen games (let's say the game itself is 4:3 but the Wayland driver stretches it to 16:9)? That's one small bit that I noticed while testing the Wayland driver.

      Edited by human222
    • I think it would be OK to take a small step forward and add it to the default driver list, but keep it at the end so x11/xwayland is still preferred for now if available.

    • Author Developer

      Yes, that's what I meant of course :)

    • As far as major features missing, wintab support is something that is still missing. I can't really think of much else, the winewayland driver certainly isn't perfect, but it is astonishingly good given the impedance mismatch of Wayland and Win32!

      For some reason, seeing this comment thread today inspired me to actually try to implement wintab support in the winewayland driver. I managed to get a functional WIP going—It's a pretty big mess though, I think the first order of business would be to get some patches going to clean up the winex11 wintab code and ideally move most of the actual wintab32-specific logic out of winex11. (But that's pretty off-topic here. If anyone wants to talk about that feel free to reach out directly.)

      I'd love to see winewayland get to a shape where it could potentially be preferred over winex11 on XWayland.

    • Please register or sign in to reply
  • Rémi Bernon added 6 commits

    added 6 commits

    • c2a0fab7 - winewayland: Move surface title change to wayland_surface_make_toplevel.
    • 2460f21e - winewayland: Call wayland_surface_clear_role in wayland_surface_destroy.
    • 67bf9975 - winewayland: Introduce a new wayland_surface role enumeration.
    • 35c90946 - winewayland: Introduce a new wayland_surface_reconfigure_xdg helper.
    • af65d37c - winewayland: Introduce a new update_wayland_surface_state_toplevel helper.
    • 29b85c35 - winewayland: Use subsurfaces for unmanaged windows.

    Compare with previous version

  • Author Developer

    v2: Fix linux build broken by intermediate changes.

    • Applying this MR on top of 2b32c285 (master) prevents osu! (download) from closing when the either the ingame "Fullscreen" or "Render at native resolution" options are enabled (i.e. no window frame). I don't want to speculate too much, but it seems that an invisible window just never closes, preventing Wine/the game from closing until a wineserver -k is sent. Only native dotnet48 is installed in the prefix.

      fullscreen.log (hangs) nofullscreen.log (closes properly)

      Here are logs of WINEDEBUG=+pid,+tid,+timestamp,+loaddll,+win,+waylanddrv with both fullscreen enabled and disabled. There may be more useful debug channels that I'm missing.

      Also, irrelevant to this regression, but I believe that the GetRawInputDeviceInfoA spam is due to a different bug somewhere in wine(bus|hid|usb).

      Edited by William Horvath
    • Can you try bisecting the individual MR commits? 🐸

    • It starts occurring after commit 5/6 (af65d37c Introduce update_wayland_surface_state_toplevel). Reverting to the previous commit 4/6, 35c90946 Introduce wayland_surface_reconfigure_xdg, it behaves normally.

    • The core problem seems to be that in wayland_win_data_update_wayland_state surface->role is used to determine what's the current role for the surface. However surface->role is really tracking what role was given in the past to this surface and thus can be given again in the future (because surfaces cannot change roles).

      In this particular osu! case, we have a previously toplevel surface (so surface->role = toplevel) that is now role-less, but wayland_surface_update_state_toplevel is called on it, which then unconditionally tries to access the NULL xdg_toplevel and crashes the thread while holding the 'win_data_mutex` lock.

    • One fix is to add if (!surface->xdg_toplevel) return; at the beginning of wayland_surface_update_state_toplevel(). It depends on what we want surface->role to mean. If it is "possible role" (which seems to match how other parts of the code are implemented) then this should be OK.

      Alternatively, perhaps a more explicit arrangement surface->role vs surface->possible_role would make things clearer?

    • Author Developer

      Thanks for catching this! The if was there originally, I added it back.

    • Looks like it's working fine now, thanks.

    • Please register or sign in to reply
435 457 /* Get the managed state with win_data unlocked, as is_window_managed
436 458 * may need to query win_data information about other HWNDs and thus
437 459 * acquire the lock itself internally. */
438 managed = is_window_managed(hwnd, swp_flags, &new_rects->window);
460 if (!(managed = is_window_managed(hwnd, swp_flags, &new_rects->window)) && surface)
461 {
462 toplevel = NtUserGetWindowRelative(hwnd, GW_OWNER);
463 if (toplevel) toplevel = NtUserGetAncestor(toplevel, GA_ROOT);
464 /* fallback to any window that is right below our top left corner */
465 if (!toplevel) toplevel = NtUserWindowFromPoint(new_rects->window.left - 1, new_rects->window.top - 1);
  • Rémi Bernon added 6 commits

    added 6 commits

    • faa42ef8 - winewayland: Move surface title change to wayland_surface_make_toplevel.
    • d01fdc51 - winewayland: Call wayland_surface_clear_role in wayland_surface_destroy.
    • 7a4abcfa - winewayland: Introduce a new wayland_surface role enumeration.
    • 12d3fb9f - winewayland: Introduce a new wayland_surface_reconfigure_xdg helper.
    • 51f42fef - winewayland: Introduce a new update_wayland_surface_state_toplevel helper.
    • e9d0dedc - winewayland: Use subsurfaces for unmanaged windows.

    Compare with previous version

  • Alexandros Frantzis
  • Rémi Bernon added 6 commits

    added 6 commits

    • 0e14fc8a - winewayland: Move surface title change to wayland_surface_make_toplevel.
    • 5c0e9c40 - winewayland: Call wayland_surface_clear_role in wayland_surface_destroy.
    • b2524709 - winewayland: Introduce a new wayland_surface role enumeration.
    • 8e6ca92d - winewayland: Introduce a new wayland_surface_reconfigure_xdg helper.
    • 6c502d47 - winewayland: Introduce a new update_wayland_surface_state_toplevel helper.
    • e341a6c7 - winewayland: Use subsurfaces for unmanaged windows.

    Compare with previous version

  • Looks great, thanks!

  • Alexandros Frantzis approved this merge request

    approved this merge request

  • added 65 commits

    • e341a6c7...fb256d8b - 59 commits from branch wine:master
    • 66e93ca8 - winewayland: Move surface title change to wayland_surface_make_toplevel.
    • 18b4bc7e - winewayland: Call wayland_surface_clear_role in wayland_surface_destroy.
    • e54fd15c - winewayland: Introduce a new wayland_surface role enumeration.
    • 07b6d8ef - winewayland: Introduce a new wayland_surface_reconfigure_xdg helper.
    • ffb6c0cd - winewayland: Introduce a new update_wayland_surface_state_toplevel helper.
    • b061022a - winewayland: Use subsurfaces for unmanaged windows.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading