[PATCH] wined3d: Restore the Topmost state upon exiting fullscreen, when used from DXGI.
DXGI, unlike D3D9 and below, restores the topmost state when exiting fullscreen. Unreal Engine 4 games (e.g. Deep Rock Galactic) depend on this because they don't unset it manually, so the window remains topmost on Wine.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
In fact, we already have tests for this in DXGI, but they've been marked todo_wine, which this patch fixes.
dlls/dxgi/swapchain.c | 2 ++ dlls/dxgi/tests/dxgi.c | 2 -- dlls/wined3d/swapchain.c | 8 +++++++- include/wine/wined3d.h | 1 + 4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/dlls/dxgi/swapchain.c b/dlls/dxgi/swapchain.c index f37aee9..2895e0e 100644 --- a/dlls/dxgi/swapchain.c +++ b/dlls/dxgi/swapchain.c @@ -416,6 +416,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d11_swapchain_SetFullscreen wined3d_swapchain_get_desc(swapchain->wined3d_swapchain, &swapchain_desc); swapchain_desc.output = dxgi_output->wined3d_output; swapchain_desc.windowed = !fullscreen; + swapchain_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(state, &swapchain_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr)) @@ -2243,6 +2244,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d12_swapchain_SetFullscreen goto fail; wined3d_mutex_lock(); wined3d_desc.windowed = !fullscreen; + wined3d_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(swapchain->state, &wined3d_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr)) diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 8f52e1d..8d243e2 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -5859,7 +5859,6 @@ static void test_swapchain_window_styles(void) todo_wine_if(!(tests[i].expected_style & WS_VISIBLE)) ok(style == tests[i].expected_style, "Test %u: Got style %#x, expected %#x.\n", i, style, tests[i].expected_style); - todo_wine_if(!(tests[i].expected_exstyle & WS_EX_TOPMOST)) ok(exstyle == tests[i].expected_exstyle, "Test %u: Got exstyle %#x, expected %#x.\n", i, exstyle, tests[i].expected_exstyle);
@@ -5871,7 +5870,6 @@ static void test_swapchain_window_styles(void) todo_wine_if(!(tests[i].expected_style & WS_VISIBLE)) ok(style == tests[i].expected_style, "Test %u: Got style %#x, expected %#x.\n", i, style, tests[i].expected_style); - todo_wine_if(!(tests[i].expected_exstyle & WS_EX_TOPMOST)) ok(exstyle == tests[i].expected_exstyle, "Test %u: Got exstyle %#x, expected %#x.\n", i, exstyle, tests[i].expected_exstyle);
diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index e9d6272..84aba06 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -2114,6 +2114,10 @@ void wined3d_swapchain_state_restore_from_fullscreen(struct wined3d_swapchain_st if (!state->style && !state->exstyle) return;
+ /* DXGI restores WS_EX_TOPMOST, unlike Direct3D 9 and below */ + if ((state->desc.flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) && !(state->exstyle & WS_EX_TOPMOST)) + window_pos_flags &= ~SWP_NOZORDER; + style = GetWindowLongW(window, GWL_STYLE); exstyle = GetWindowLongW(window, GWL_EXSTYLE);
@@ -2145,7 +2149,7 @@ void wined3d_swapchain_state_restore_from_fullscreen(struct wined3d_swapchain_st rect = *window_rect; else window_pos_flags |= (SWP_NOMOVE | SWP_NOSIZE); - SetWindowPos(window, 0, rect.left, rect.top, + SetWindowPos(window, HWND_NOTOPMOST, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, window_pos_flags);
wined3d_filter_messages(window, filter); @@ -2243,6 +2247,8 @@ HRESULT CDECL wined3d_swapchain_state_set_fullscreen(struct wined3d_swapchain_st RECT *window_rect = NULL; if (state->desc.flags & WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT) window_rect = &state->original_window_rect; + if (swapchain_desc->flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) + state->desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; wined3d_swapchain_state_restore_from_fullscreen(state, state->device_window, window_rect); }
diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
#define WINED3DDP_MAXTEXCOORD 8
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=74661
Your paranoid android.
=== debiant (32 bit report) ===
dxgi: dxgi: Timeout
On 2020-07-02 14:41, Gabriel Ivăncescu wrote:
DXGI, unlike D3D9 and below, restores the topmost state when exiting fullscreen. Unreal Engine 4 games (e.g. Deep Rock Galactic) depend on this because they don't unset it manually, so the window remains topmost on Wine.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
In fact, we already have tests for this in DXGI, but they've been marked todo_wine, which this patch fixes.
dlls/dxgi/swapchain.c | 2 ++ dlls/dxgi/tests/dxgi.c | 2 -- dlls/wined3d/swapchain.c | 8 +++++++- include/wine/wined3d.h | 1 + 4 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/dlls/dxgi/swapchain.c b/dlls/dxgi/swapchain.c index f37aee9..2895e0e 100644 --- a/dlls/dxgi/swapchain.c +++ b/dlls/dxgi/swapchain.c @@ -416,6 +416,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d11_swapchain_SetFullscreen wined3d_swapchain_get_desc(swapchain->wined3d_swapchain, &swapchain_desc); swapchain_desc.output = dxgi_output->wined3d_output; swapchain_desc.windowed = !fullscreen;
- swapchain_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(state, &swapchain_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
@@ -2243,6 +2244,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d12_swapchain_SetFullscreen goto fail; wined3d_mutex_lock(); wined3d_desc.windowed = !fullscreen;
- wined3d_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(swapchain->state, &wined3d_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 8f52e1d..8d243e2 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -5859,7 +5859,6 @@ static void test_swapchain_window_styles(void) todo_wine_if(!(tests[i].expected_style & WS_VISIBLE)) ok(style == tests[i].expected_style, "Test %u: Got style %#x, expected %#x.\n", i, style, tests[i].expected_style);
todo_wine_if(!(tests[i].expected_exstyle & WS_EX_TOPMOST)) ok(exstyle == tests[i].expected_exstyle, "Test %u: Got exstyle %#x, expected %#x.\n", i, exstyle, tests[i].expected_exstyle);@@ -5871,7 +5870,6 @@ static void test_swapchain_window_styles(void) todo_wine_if(!(tests[i].expected_style & WS_VISIBLE)) ok(style == tests[i].expected_style, "Test %u: Got style %#x, expected %#x.\n", i, style, tests[i].expected_style);
todo_wine_if(!(tests[i].expected_exstyle & WS_EX_TOPMOST)) ok(exstyle == tests[i].expected_exstyle, "Test %u: Got exstyle %#x, expected %#x.\n", i, exstyle, tests[i].expected_exstyle);diff --git a/dlls/wined3d/swapchain.c b/dlls/wined3d/swapchain.c index e9d6272..84aba06 100644 --- a/dlls/wined3d/swapchain.c +++ b/dlls/wined3d/swapchain.c @@ -2114,6 +2114,10 @@ void wined3d_swapchain_state_restore_from_fullscreen(struct wined3d_swapchain_st if (!state->style && !state->exstyle) return;
- /* DXGI restores WS_EX_TOPMOST, unlike Direct3D 9 and below */
- if ((state->desc.flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) && !(state->exstyle & WS_EX_TOPMOST))
window_pos_flags &= ~SWP_NOZORDER;style = GetWindowLongW(window, GWL_STYLE); exstyle = GetWindowLongW(window, GWL_EXSTYLE);@@ -2145,7 +2149,7 @@ void wined3d_swapchain_state_restore_from_fullscreen(struct wined3d_swapchain_st rect = *window_rect; else window_pos_flags |= (SWP_NOMOVE | SWP_NOSIZE);
- SetWindowPos(window, 0, rect.left, rect.top,
SetWindowPos(window, HWND_NOTOPMOST, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, window_pos_flags);
wined3d_filter_messages(window, filter);
@@ -2243,6 +2247,8 @@ HRESULT CDECL wined3d_swapchain_state_set_fullscreen(struct wined3d_swapchain_st RECT *window_rect = NULL; if (state->desc.flags & WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT) window_rect = &state->original_window_rect;
if (swapchain_desc->flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE)state->desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; wined3d_swapchain_state_restore_from_fullscreen(state, state->device_window, window_rect); }diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
#define WINED3DDP_MAXTEXCOORD 8
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=74663
Your paranoid android.
=== w1064v1809 (64 bit report) ===
d3d9: device.c:5041: Test failed: Failed to set foreground window. device.c:5067: Test failed: Failed to set foreground window. device.c:5070: Test failed: Failed to reset device, hr 0x88760868. device.c:5041: Test failed: Failed to set foreground window. device.c:5067: Test failed: Failed to set foreground window. device.c:11438: Test failed: Failed to set foreground window. device.c:11440: Test failed: Got unexpected hr 0. device.c:11442: Test failed: Got unexpected hr 0. device.c:11447: Test failed: Failed to set foreground window. device.c:11449: Test failed: Got unexpected hr 0. device.c:11451: Test failed: Got unexpected hr 0. device.c:11469: Test failed: Failed to set foreground window. device.c:11478: Test failed: Failed to set foreground window. device.c:11493: Test failed: Failed to set foreground window. device.c:11495: Test failed: Got unexpected hr 0. device.c:11499: Test failed: Failed to set foreground window. device.c:14087: Test failed: Adapter 0: SetForegroundWindow failed, error 0. device.c:14093: Test failed: Adapter 0: SetForegroundWindow failed, error 0.
=== w1064v1809_zh_CN (32 bit report) ===
ddraw: ddraw1.c:11238: Test failed: Got unexpected color 0x00ffffff.
=== w1064v1809_2scr (32 bit report) ===
ddraw: ddraw2.c:13377: Test failed: Expected 22 surfaces, got 21.
=== debiant (32 bit report) ===
d3d8: device.c:10314: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
d3d9: device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5046: Test succeeded inside todo block: Expected device window style 0x14cf0000, got 0x14cf0000, i=3. device.c:14067: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
=== debiant (32 bit French report) ===
d3d9: device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5046: Test succeeded inside todo block: Expected device window style 0x14cf0000, got 0x14cf0000, i=3.
=== debiant (32 bit Japanese:Japan report) ===
d3d8: device.c:10314: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
d3d9: device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5046: Test succeeded inside todo block: Expected device window style 0x14cf0000, got 0x14cf0000, i=3. device.c:14067: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
=== debiant (32 bit Chinese:China report) ===
d3d8: device.c:10314: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
d3d9: Unhandled exception: floating point underflow in 32-bit code (0xf79a328f).
Report validation errors: d3d9:device crashed (c0000093)
=== debiant (32 bit WoW report) ===
d3d8: device: Timeout stateblock: Timeout visual: Timeout
=== debiant (64 bit WoW report) ===
d3d8: device.c:10314: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,23)-(1020,764).
d3d9: device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=1. device.c:4992: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5027: Test failed: Expected device window extended style 0x100, got 0x108, i=3. device.c:5046: Test succeeded inside todo block: Expected device window style 0x14cf0000, got 0x14cf0000, i=3.
Hi Rémi,
On 02/07/2020 16:14, Rémi Bernon wrote:
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Thanks for the patches, I'll take a look at them. Though I think WS_EX_TOPMOST deserves special handling, because it can't be modified with SetWindowLong like other styles.
BTW, I can't find where WINED3DCREATE_NOWINDOWCHANGES is set at all. It's checked in a couple of places but not set anywhere in whole wine tree. Is it even used anymore?
On 2020-07-02 17:14, Gabriel Ivăncescu wrote:
Hi Rémi,
On 02/07/2020 16:14, Rémi Bernon wrote:
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Thanks for the patches, I'll take a look at them. Though I think WS_EX_TOPMOST deserves special handling, because it can't be modified with SetWindowLong like other styles.
Sure, there's also window visibility that may be changed or restored in the same way (not sure if it can be controlled through styles).
BTW, I can't find where WINED3DCREATE_NOWINDOWCHANGES is set at all. It's checked in a couple of places but not set anywhere in whole wine tree. Is it even used anymore?
It's not used directly IIRC but it has the same value as some corresponding D3D flag.
On 02/07/2020 18:16, Rémi Bernon wrote:
On 2020-07-02 17:14, Gabriel Ivăncescu wrote:
Hi Rémi,
On 02/07/2020 16:14, Rémi Bernon wrote:
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Thanks for the patches, I'll take a look at them. Though I think WS_EX_TOPMOST deserves special handling, because it can't be modified with SetWindowLong like other styles.
Sure, there's also window visibility that may be changed or restored in the same way (not sure if it can be controlled through styles).
BTW, I can't find where WINED3DCREATE_NOWINDOWCHANGES is set at all. It's checked in a couple of places but not set anywhere in whole wine tree. Is it even used anymore?
It's not used directly IIRC but it has the same value as some corresponding D3D flag.
I've had a quick glance at your patches, since they seem very thorough, and while they add a lot of new flags, I couldn't find a way to trim most of them down because of the various behavior between ddraw/d3d9/dxgi.
The only thing is WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE. This flag seems redundant, because you can flip the meaning of the NO_ZORDER and NO_VISIBILITY flags instead. For example, in DXGI you use NO_VISIBILITY_CHANGES and RESTORE_WINDOW_STATE, so you could just use the RESTORE_ZORDER flag. And where you set the NO_*_CHANGES flags you just clear the RESTORE_* flags instead. Unless I am missing something?
But, was there a specific blocker in particular? I feel like if I add the special case for dxgi with the WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT rename as Henri suggested, it will complicate the logic in your patches since the state restore thing seems to also apply to d3d9 if extended.
Or was there another reason?
Thanks, Gabriel
On 2020-07-02 17:54, Gabriel Ivăncescu wrote:
On 02/07/2020 18:16, Rémi Bernon wrote:
On 2020-07-02 17:14, Gabriel Ivăncescu wrote:
Hi Rémi,
On 02/07/2020 16:14, Rémi Bernon wrote:
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Thanks for the patches, I'll take a look at them. Though I think WS_EX_TOPMOST deserves special handling, because it can't be modified with SetWindowLong like other styles.
Sure, there's also window visibility that may be changed or restored in the same way (not sure if it can be controlled through styles).
BTW, I can't find where WINED3DCREATE_NOWINDOWCHANGES is set at all. It's checked in a couple of places but not set anywhere in whole wine tree. Is it even used anymore?
It's not used directly IIRC but it has the same value as some corresponding D3D flag.
I've had a quick glance at your patches, since they seem very thorough, and while they add a lot of new flags, I couldn't find a way to trim most of them down because of the various behavior between ddraw/d3d9/dxgi.
The only thing is WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE. This flag seems redundant, because you can flip the meaning of the NO_ZORDER and NO_VISIBILITY flags instead. For example, in DXGI you use NO_VISIBILITY_CHANGES and RESTORE_WINDOW_STATE, so you could just use the RESTORE_ZORDER flag. And where you set the NO_*_CHANGES flags you just clear the RESTORE_* flags instead. Unless I am missing something?
But, was there a specific blocker in particular? I feel like if I add the special case for dxgi with the WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT rename as Henri suggested, it will complicate the logic in your patches since the state restore thing seems to also apply to d3d9 if extended.
Or was there another reason?
Thanks, Gabriel
To be honest it's been a while and I'm not sure anymore why all of these were required. I simply remember that the tests showed the individual aspects (z-order, visibility, style), and that it was dependent on the API used.
I'm not even sure if it's entirely correct either, it looks like the testbot run from earlier indicated some mistakes in extended style for instance. I think the tests are though, and the few errors in the test run seem unrelated and pre-existing.
Hi Rémi,
On 02/07/2020 19:06, Rémi Bernon wrote:
On 2020-07-02 17:54, Gabriel Ivăncescu wrote:
On 02/07/2020 18:16, Rémi Bernon wrote:
On 2020-07-02 17:14, Gabriel Ivăncescu wrote:
Hi Rémi,
On 02/07/2020 16:14, Rémi Bernon wrote:
Hi,
I investigated this logic a while ago, and I believe it's a little bit more convoluted. I think there's, for instance, some interaction with the WINED3DCREATE_NOWINDOWCHANGES flag, but the behavior also varies between ddraw, d3d <= 9, d3d >= 9ex and dxgi.
AFAICS there's also the same kind of change / restore logic with window visibility, and of course window styles -- which wined3d incorrectly modifies.
I have some patches with some more tests showing that, but it was based on some other changes to remove the window styles updates, that I sent a while ago but didn't try sending again since then.
I'm attaching my whole series if you want to have a look, potentially the additional tests can be interesting. Feel free to take over it too.
Thanks for the patches, I'll take a look at them. Though I think WS_EX_TOPMOST deserves special handling, because it can't be modified with SetWindowLong like other styles.
Sure, there's also window visibility that may be changed or restored in the same way (not sure if it can be controlled through styles).
BTW, I can't find where WINED3DCREATE_NOWINDOWCHANGES is set at all. It's checked in a couple of places but not set anywhere in whole wine tree. Is it even used anymore?
It's not used directly IIRC but it has the same value as some corresponding D3D flag.
I've had a quick glance at your patches, since they seem very thorough, and while they add a lot of new flags, I couldn't find a way to trim most of them down because of the various behavior between ddraw/d3d9/dxgi.
The only thing is WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE. This flag seems redundant, because you can flip the meaning of the NO_ZORDER and NO_VISIBILITY flags instead. For example, in DXGI you use NO_VISIBILITY_CHANGES and RESTORE_WINDOW_STATE, so you could just use the RESTORE_ZORDER flag. And where you set the NO_*_CHANGES flags you just clear the RESTORE_* flags instead. Unless I am missing something?
But, was there a specific blocker in particular? I feel like if I add the special case for dxgi with the WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT rename as Henri suggested, it will complicate the logic in your patches since the state restore thing seems to also apply to d3d9 if extended.
Or was there another reason?
Thanks, Gabriel
To be honest it's been a while and I'm not sure anymore why all of these were required. I simply remember that the tests showed the individual aspects (z-order, visibility, style), and that it was dependent on the API used.
I'm not even sure if it's entirely correct either, it looks like the testbot run from earlier indicated some mistakes in extended style for instance. I think the tests are though, and the few errors in the test run seem unrelated and pre-existing.
So it appears I will need the RESTORE_WINDOW_STATE flag as separate, because d3d9ex uses it but it doesn't restore the rect.
I will later send most of the patches rebased, and most which are unmodified from yours, the others are slightly tweaked to also fix some small mistakes / dead code.
I haven't touched the following parts, though, so I'll leave those to you if you decide to pick them up again:
The NO_STYLE_CHANGES flag, which is quite invasive. Adding the NO_VISIBILITY_CHANGES flag in dxgi. I wasn't able to test this, so I left it out—but it should be trivial to add.
Thanks, Gabriel
On Thu, 2 Jul 2020 at 17:14, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
@@ -416,6 +416,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d11_swapchain_SetFullscreen wined3d_swapchain_get_desc(swapchain->wined3d_swapchain, &swapchain_desc); swapchain_desc.output = dxgi_output->wined3d_output; swapchain_desc.windowed = !fullscreen;
- swapchain_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(state, &swapchain_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
@@ -2243,6 +2244,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d12_swapchain_SetFullscreen goto fail; wined3d_mutex_lock(); wined3d_desc.windowed = !fullscreen;
- wined3d_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(swapchain->state, &wined3d_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
Any reason to not add this to DXGI_WINED3D_SWAPCHAIN_FLAGS?
- /* DXGI restores WS_EX_TOPMOST, unlike Direct3D 9 and below */
- if ((state->desc.flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) && !(state->exstyle & WS_EX_TOPMOST))
window_pos_flags &= ~SWP_NOZORDER;
Wined3d doesn't particularly care about DXGI or d3d9. If you wanted to add a comment to this effect you'd add it to the place that sets the flag, but the flag also seems straightforward enough on its own.
@@ -2243,6 +2247,8 @@ HRESULT CDECL wined3d_swapchain_state_set_fullscreen(struct wined3d_swapchain_st RECT *window_rect = NULL; if (state->desc.flags & WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT) window_rect = &state->original_window_rect;
if (swapchain_desc->flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE)state->desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE;
We currently ignore flag changes in wined3d_swapchain_state_set_fullscreen(). That could be changed, but then you'd have to handle other flags potentially changing as well, and I don't think there's a need.
diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
Ideally WINED3D_SWAPCHAIN_TOPMOST_RESTORE would simply be merged with WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT, but the naming should at least be consistent.
Hi Henri,
Thanks for the review!
On 02/07/2020 17:08, Henri Verbeet wrote:
On Thu, 2 Jul 2020 at 17:14, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
@@ -416,6 +416,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d11_swapchain_SetFullscreen wined3d_swapchain_get_desc(swapchain->wined3d_swapchain, &swapchain_desc); swapchain_desc.output = dxgi_output->wined3d_output; swapchain_desc.windowed = !fullscreen;
- swapchain_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(state, &swapchain_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
@@ -2243,6 +2244,7 @@ static HRESULT STDMETHODCALLTYPE DECLSPEC_HOTPATCH d3d12_swapchain_SetFullscreen goto fail; wined3d_mutex_lock(); wined3d_desc.windowed = !fullscreen;
- wined3d_desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE; hr = wined3d_swapchain_state_set_fullscreen(swapchain->state, &wined3d_desc, NULL); wined3d_mutex_unlock(); if (FAILED(hr))
Any reason to not add this to DXGI_WINED3D_SWAPCHAIN_FLAGS?
No reason, I'll do that, thanks. I'm not very familiar with the code as you can probably figure it out. :-)
- /* DXGI restores WS_EX_TOPMOST, unlike Direct3D 9 and below */
- if ((state->desc.flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE) && !(state->exstyle & WS_EX_TOPMOST))
window_pos_flags &= ~SWP_NOZORDER;Wined3d doesn't particularly care about DXGI or d3d9. If you wanted to add a comment to this effect you'd add it to the place that sets the flag, but the flag also seems straightforward enough on its own.
Noted.
@@ -2243,6 +2247,8 @@ HRESULT CDECL wined3d_swapchain_state_set_fullscreen(struct wined3d_swapchain_st RECT *window_rect = NULL; if (state->desc.flags & WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT) window_rect = &state->original_window_rect;
if (swapchain_desc->flags & WINED3D_SWAPCHAIN_TOPMOST_RESTORE)state->desc.flags |= WINED3D_SWAPCHAIN_TOPMOST_RESTORE;We currently ignore flag changes in wined3d_swapchain_state_set_fullscreen(). That could be changed, but then you'd have to handle other flags potentially changing as well, and I don't think there's a need.
diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
Ideally WINED3D_SWAPCHAIN_TOPMOST_RESTORE would simply be merged with WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT, but the naming should at least be consistent.
So in that case I'd simply check to see if the rect is NULL or not, to restore TOPMOST, within the function that restores it, right?
As for the name, how about WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE? (I'll of course send it as a separate patch)
Thanks, Gabriel
On Thu, 2 Jul 2020 at 19:50, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
Ideally WINED3D_SWAPCHAIN_TOPMOST_RESTORE would simply be merged with WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT, but the naming should at least be consistent.
So in that case I'd simply check to see if the rect is NULL or not, to restore TOPMOST, within the function that restores it, right?
No, you'd rename WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT to something like WINED3D_SWAPCHAIN_RESTORE_GEOMETRY (although it's perhaps a bit of a stretch to include the stacking order in "geometry"), and then check for that flag the same way the current patch checks for WINED3D_SWAPCHAIN_TOPMOST_RESTORE.
As for the name, how about WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE? (I'll of course send it as a separate patch)
I could live with it, but "state" is very broad.
On 02/07/2020 18:44, Henri Verbeet wrote:
On Thu, 2 Jul 2020 at 19:50, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 346d1d0..cee46d3 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -905,6 +905,7 @@ enum wined3d_shader_type #define WINED3D_SWAPCHAIN_GDI_COMPATIBLE 0x00008000u #define WINED3D_SWAPCHAIN_IMPLICIT 0x00010000u #define WINED3D_SWAPCHAIN_HOOK 0x00020000u +#define WINED3D_SWAPCHAIN_TOPMOST_RESTORE 0x00040000u
Ideally WINED3D_SWAPCHAIN_TOPMOST_RESTORE would simply be merged with WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT, but the naming should at least be consistent.
So in that case I'd simply check to see if the rect is NULL or not, to restore TOPMOST, within the function that restores it, right?
No, you'd rename WINED3D_SWAPCHAIN_RESTORE_WINDOW_RECT to something like WINED3D_SWAPCHAIN_RESTORE_GEOMETRY (although it's perhaps a bit of a stretch to include the stacking order in "geometry"), and then check for that flag the same way the current patch checks for WINED3D_SWAPCHAIN_TOPMOST_RESTORE.
As for the name, how about WINED3D_SWAPCHAIN_RESTORE_WINDOW_STATE? (I'll of course send it as a separate patch)
I could live with it, but "state" is very broad.
Yeah, I was basing it off Rémi's patch series to ease it later when he rebases them. From that perspective, I think it's better if I add a new flag, since it will be needed later by his patches anyway (for ddraw, d3d9 and dxgi different behaviors).
Currently I'm thinking of something like WINED3D_SWAPCHAIN_RESTORE_ZORDER. I'll do some more testing to see if it works with his patches, though, otherwise there's not much point to it.
Unless that's not desirable then I'll go with the WINED3D_SWAPCHAIN_RESTORE_GEOMETRY thing (which only applies to dxgi).
-
Gabriel Ivăncescu -
Henri Verbeet -
Marvin -
Rémi Bernon