[PATCH v5 2/6] ddraw: Use WINED3D_SWAPCHAIN_NO_WINDOW_CHANGES also if DDSCL_NOWINDOWCHANGES is set.

[Gabriel Ivăncescu]

On 13/01/2022 14:14, Stefan Dösinger wrote:
> This is certainly interesting behavior. I'm asking the questions below to check if I grok the test results right:
> 
>> Am 12.01.2022 um 19:20 schrieb Gabriel Ivăncescu <gabrielopcode at gmail.com>:
>>
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN | DDSCL_NOWINDOWCHANGES);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    todo_wine ok(tmp == style, "Expected window style %#x, got %#x.\n", style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    todo_wine ok(tmp == exstyle, "Expected window extended style %#x, got %#x.\n", exstyle, tmp);
> 
> So far so sensible
> 
>> +
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    expected_style = style | WS_VISIBLE;
>> +    todo_wine ok(tmp == expected_style, "Expected window style %#x, got %#x.\n", expected_style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    expected_style = exstyle | WS_EX_TOPMOST;
>> +    todo_wine ok(tmp == expected_style, "Expected window extended style %#x, got %#x.\n", expected_style, tmp);
> 
> Sensible too, window is made visible and topmost.
> 
>> +    ShowWindow(window, SW_HIDE);
> 
> Why do you hide the window here? To show that the below call does not make it visible? Makes sense, although it also convolutes things with the different treatment of hidden windows.
> 
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    todo_wine ok(tmp == style, "Expected window style %#x, got %#x.\n", style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    expected_style = exstyle | WS_EX_TOPMOST;
>> +    todo_wine ok(tmp == expected_style, "Expected window extended style %#x, got %#x.\n", expected_style, tmp);
>> +
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_NORMAL | DDSCL_NOWINDOWCHANGES);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    ok(tmp == style, "Expected window style %#x, got %#x.\n", style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    expected_style = exstyle | WS_EX_TOPMOST;
>> +    ok(tmp == expected_style, "Expected window extended style %#x, got %#x.\n", expected_style, tmp);
> 
> That's the surprising behavior you are trying to show - the NOWINDOWCHANGES matters on "exiting" fullscreen.
> 
> For my curiosity I tested what destroying the ddraw interface does here. It doesn't remove WS_EX_TOPMOST either, which suggests that either we have to get rid of our DD_SCL call on destruction or add NOWINDOWCHANGES. No need to take care of this in this patch series though unless we find a game that needs it.
>  >> +
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_NORMAL);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    ok(tmp == style, "Expected window style %#x, got %#x.\n", style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    expected_style = exstyle | WS_EX_TOPMOST;
>> +    ok(tmp == expected_style, "Expected window extended style %#x, got %#x.\n", expected_style, tmp);
> 
> Why does this not change the style? Because ddraw considers it a redundant SCL normal->normal call? The above fullscreen|exclusive -> fullscreen|exclusive call does add WS_EX_TOPMOST.
> 

I don't know why honestly, but we already have the 
"restore_mode_on_normal" thing, so I think it possibly has similar code 
path on native? That's why I added it there on patch 3, since it's done 
when going from exclusive to normal only, not any call to normal.

It's obviously inconsistent and somewhat badly designed but sadly we 
have to match it (or at least test for it).

>> +
>> +    ret = SetForegroundWindow(window);
>> +    ok(ret, "Failed to set foreground window.\n");
>> +
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    expected_style = style | WS_VISIBLE;
>> +    todo_wine ok(tmp == expected_style, "Expected window style %#x, got %#x.\n", expected_style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    expected_style = exstyle | WS_EX_TOPMOST;
>> +    todo_wine ok(tmp == expected_style, "Expected window extended style %#x, got %#x.\n", expected_style, tmp);
>> +
>> +    ShowWindow(window, SW_HIDE);
>> +    hr = IDirectDraw_SetCooperativeLevel(ddraw, window, DDSCL_NORMAL);
>> +    ok(SUCCEEDED(hr), "SetCooperativeLevel failed, hr %#x.\n", hr);
>> +
>> +    tmp = GetWindowLongA(window, GWL_STYLE);
>> +    ok(tmp == style, "Expected window style %#x, got %#x.\n", style, tmp);
>> +    tmp = GetWindowLongA(window, GWL_EXSTYLE);
>> +    todo_wine ok(tmp == exstyle, "Expected window extended style %#x, got %#x.\n", exstyle, tmp);
> 
> And afaiu here you try to show that the window behing hidden doesn't have any unintended side effects. Topmost is removed anyway, although WS_VISIBLE is not added (presumably cause it is DDSCL_NORMAL)
> 

Yeah, I actually just tried to test most corner cases I could think of 
to get a picture of how native works. I doubt that they put that much 
thought into it, so it's probably bound to have quirks and such. (note, 
I'm not saying we necessarily have to fix all of them, these are just 
tests).

If you think some tests are redundant or too overkill, I'll happily 
remove them (or, if you have suggestions to add even more...).


Also I'll fix the dumb window2 leak, sorry. Let me know if there's 
anything else though, before I resend (but probably will do tomorrow).

Thanks for the reviews!