[PATCH 1/3] dxgi/tests: Test removing WS_EX_TOPMOST on a swapchain's fullscreen state output window.

[Gabriel Ivăncescu]

Hi Zhiyi,

Thanks for the review.

On 29/01/2021 04:34, Zhiyi Zhang wrote:
> 
> 
> On 1/28/21 9:51 PM, Gabriel Ivăncescu wrote:
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>   dlls/dxgi/tests/dxgi.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c
>> index 2d8826f..14cb555 100644
>> --- a/dlls/dxgi/tests/dxgi.c
>> +++ b/dlls/dxgi/tests/dxgi.c
>> @@ -2826,10 +2826,47 @@ static void test_set_fullscreen(IUnknown *device, BOOL is_d3d12)
>>       refcount = IDXGISwapChain_Release(swapchain);
>>       ok(!refcount, "IDXGISwapChain has %u references left.\n", refcount);
>>   
>> -    swapchain_desc.OutputWindow = CreateWindowA("static", "dxgi_test", 0, 0, 0, 400, 200, 0, 0, 0, 0);
>> +    swapchain_desc.OutputWindow = CreateWindowA("static", "dxgi_test", WS_VISIBLE, 0, 0, 400, 200, 0, 0, 0, 0);
> test_swapchain_present() uses an invisible window to test Present() in fullscreen mode. So either this change
> is unnecessary or you need to handle the case of invisible windows. It would be helpful to test the existence
> of WS_EX_TOPMOST even for an invisible window for a fullscreen swapchain.
> 

When I tested on invisible windows the results were unreliable on 
Windows. Most of the time, it actually didn't exit fullscreen, but 
sometimes it did. I'm not sure what's the best way to handle that.

I don't think it matters that much in practice, but…

>>       check_window_fullscreen_state(swapchain_desc.OutputWindow, &initial_state.fullscreen_state);
>>       swapchain_desc.Flags = DXGI_SWAP_CHAIN_FLAG_ALLOW_MODE_SWITCH;
>>       hr = IDXGIFactory_CreateSwapChain(factory, device, &swapchain_desc, &swapchain);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    hr = IDXGISwapChain_SetFullscreenState(swapchain, TRUE, NULL);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    ok(!!fullscreen, "Got unexpected fullscreen %#x.\n", fullscreen);
> Please test the existence of WS_EX_TOPMOST first.
> 
>> +
>> +    /* Removing topmost style makes the swapchain lose fullscreen state on next Present */
>> +    SetWindowPos(swapchain_desc.OutputWindow, HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOSIZE | SWP_NOMOVE | SWP_NOACTIVATE);
>> +    ok(!(GetWindowLongA(swapchain_desc.OutputWindow, GWL_EXSTYLE) & WS_EX_TOPMOST), "Got unexpected WS_EX_TOPMOST.\n");
>> +    flush_events();
>> +
>> +    hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    ok(!!fullscreen, "Got unexpected fullscreen %#x.\n", fullscreen);
>> +    hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    ok(!!fullscreen, "Got unexpected fullscreen %#x.\n", fullscreen);
> Do you need to call IDXGISwapChain_GetFullscreenState() twice?
> 

I called it twice to test the fact that GetFullscreenState does not 
remove the fullscreen state itself. (iirc Stefan had a theory that it 
does, so I wanted to write a test for it)

>> +    hr = IDXGISwapChain_Present(swapchain, 0, 0);
> Please test if DXGI_PRESENT_TEST flag also make the swapchain exit fullscreen.
> 
> 
>> +    todo_wine ok(hr == DXGI_STATUS_OCCLUDED || broken(hr == DXGI_ERROR_INVALID_CALL) /* Win10 1809 */, "Got unexpected hr %#x.\n", hr);
> Does calling IDXGISwapChain_ResizeBuffers() before Present() help to remove this DXGI_ERROR_INVALID_CALL error?
> 
>> +    if (hr != DXGI_ERROR_INVALID_CALL)
>> +    {
>> +        hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL);
>> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +        todo_wine ok(!fullscreen, "Got unexpected fullscreen %#x.\n", fullscreen);
>> +        hr = IDXGISwapChain_Present(swapchain, 0, 0);
>> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    }
>> +    hr = IDXGISwapChain_SetFullscreenState(swapchain, FALSE, NULL);
>> +    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
>> +    refcount = IDXGISwapChain_Release(swapchain);
>> +    ok(!refcount, "IDXGISwapChain has %u references left.\n", refcount);
>> +
>> +    DestroyWindow(swapchain_desc.OutputWindow);
>> +    swapchain_desc.OutputWindow = CreateWindowA("static", "dxgi_test", 0, 0, 0, 400, 200, 0, 0, 0, 0);
>> +    check_window_fullscreen_state(swapchain_desc.OutputWindow, &initial_state.fullscreen_state);
>> +    hr = IDXGIFactory_CreateSwapChain(factory, device, &swapchain_desc, &swapchain);
>>       ok(SUCCEEDED(hr), "CreateSwapChain failed, hr %#x.\n", hr);
>>       check_swapchain_fullscreen_state(swapchain, &initial_state);
>>       test_swapchain_fullscreen_state(swapchain, adapter, &initial_state);
> 
> 

Right, I'll look into those.

Thanks,
Gabriel