[PATCH v2 0/1] MR6248: winewayland: Add missing default surface fallback in WindowPosChanging().
This fixes a blank window content issue that happens even with basic Wine programs.
Fixes: b9879d5adc1 ("winewayland: Remove now unnecessary WindowPosChanging checks.")
-- v2: winewayland: Add missing default surface fallback in WindowPosChanging().
From: Aida Jonikienė aidas957@gmail.com
This fixes a blank window content issue that happens even with basic Wine programs.
Fixes: b9879d5adc1 ("winewayland: Remove now unnecessary WindowPosChanging checks.") --- dlls/winewayland.drv/window.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 7c694246fb7..815f60808c7 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -431,15 +431,18 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, UINT swp_flags, BOOL shaped, const REC const RECT *client_rect, RECT *visible_rect) { struct wayland_win_data *data = wayland_win_data_get(hwnd); + BOOL ret;
TRACE("hwnd %p, swp_flags %04x, shaped %u, window_rect %s, client_rect %s, visible_rect %s\n", hwnd, swp_flags, shaped, wine_dbgstr_rect(window_rect), wine_dbgstr_rect(client_rect), wine_dbgstr_rect(visible_rect));
if (!data && !(data = wayland_win_data_create(hwnd, window_rect, client_rect))) return FALSE; /* use default surface */ + + ret = !!data->wayland_surface; /* use default surface if we don't have a window */ wayland_win_data_release(data);
- return TRUE; + return ret; }
This merge request was approved by Rémi Bernon.
The decision to instantiate (or not) the `data->wayland_surface` member is made in the `WindowPosChanged` driver callback which is called after `WindowPosChanging`. This means we can't rely on that member at this point to make a choice about the need for window surface.
I think what is proposed in the MR may tend to work in many cases because the WindowPosChang* sequence is typically called multiple times, but the correct solution seems to me to revert to the previous check-if-window-is-child behavior.
On Mon Aug 12 08:32:34 2024 +0000, Alexandros Frantzis wrote:
The decision to instantiate (or not) the `data->wayland_surface` member is made in the `WindowPosChanged` driver callback which is called after `WindowPosChanging`. This means we can't rely on that member at this point to make a choice about the need for window surface. I think what is proposed in the MR may tend to work in many cases because the WindowPosChang* sequence is typically called multiple times, but the correct solution seems to me to revert to the previous check-if-window-is-child behavior.
I don't think it matter so much, mostly because it is exactly the same with other drivers, and now that I'm refactoring these interfaces for DPI scaling I prefer to see similar logic between them.
This could probably be factored and moved to win32u (as I incorrectly assumed it was already when I removed the checks): we already decide to use the parent window surface there, we should probably decide whether to create a host window or not there as well.
On Mon Aug 12 08:32:34 2024 +0000, Rémi Bernon wrote:
I don't think it matter so much, mostly because it is exactly the same with other drivers, and now that I'm refactoring these interfaces for DPI scaling I prefer to see similar logic between them. This could probably be factored and moved to win32u (as I incorrectly assumed it was already when I removed the checks): we already decide to use the parent window surface there, we should probably decide whether to create a host window or not there as well.
Actually you are probably right, other drivers actually create their windows during WindowPosChanging while winewayland doesn't. I will restore the check.
we already decide to use the parent window surface there, we should probably decide whether to create a host window or not there as well.
A relevant consideration here is https://gitlab.winehq.org/wine/wine/-/merge_requests/6107, which uses some driver/host specific logic to decide whether to create `window_surface`s and `wayland_surface`s for child windows. Perhaps there is some part of the logic that is general enough for core, but much of it is adapting to Wayland-specific needs, so we will need to allow the driver to play some role in the decision making process (if we want to pursue this particular approach, that is).
On Mon Aug 12 09:38:57 2024 +0000, Alexandros Frantzis wrote:
we already decide to use the parent window surface there, we should
probably decide whether to create a host window or not there as well. A relevant consideration here is https://gitlab.winehq.org/wine/wine/-/merge_requests/6107, which uses some driver/host specific logic to decide whether to create `window_surface`s and `wayland_surface`s for child windows. Perhaps there is some part of the logic that is general enough for core, but much of it is adapting to Wayland-specific needs, so we will need to allow the driver to play some role in the decision making process (if we want to pursue this particular approach, that is).
I don't think we want to allow drivers to create window surfaces for child windows, why is this needed for Wayland?
On Mon Aug 12 10:04:21 2024 +0000, Rémi Bernon wrote:
I don't think we want to allow drivers to create window surfaces for child windows, why is this needed for Wayland?
This was used in https://gitlab.winehq.org/wine/wine/-/merge_requests/6107/diffs?commit_id=ab...
In short, the client area subsurface that contains the accelerated content will obscure any GDI rendering of child windows (which by default end up on the parent window_surface behind the GL/VK subsurface). Using a separate window_surface (and wayland_surface) allows us to place the child window contents above the GL/VK subsurface.
I came across this scenario while testing your `wglchild` example program. Without this fix (or some other solution) the decoration of the child window is obscured.
On Tue Aug 13 09:30:31 2024 +0000, Alexandros Frantzis wrote:
This was used in https://gitlab.winehq.org/wine/wine/-/merge_requests/6107/diffs?commit_id=ab... In short, the client area subsurface that contains the accelerated content will obscure any GDI rendering of child windows (which by default end up on the parent window_surface behind the GL/VK subsurface). Using a separate window_surface (and wayland_surface) allows us to place the child window contents above the GL/VK subsurface. I came across this scenario while testing your `wglchild` example program. Without this fix (or some other solution) the decoration of the child window is obscured.
Yeah, then this the same thing is happening with other drivers and they implement their own offscreen rendering / manual compositing for this case (although it's not widely implemented).
Historically child windows were used in winex11 for non-accelerated rendering, however it caused many issues as Win32 events and child window compositing is quite different from what the host may be doing.
Implementing the child window composition ourselves avoids the impedance mismatch, although we then need to handle the accelerated / non-accelerated composition case. In general this is not very well supported, and you probably don't need to care much about it for winewayland.
This merge request was closed by Aida Jonikienė.
I assume !6256 fixed this issue too (but in another way) :frog:
-
Aida Jonikienė -
Aida Jonikienė (＠DodoGTA)
-
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon (＠rbernon)