On Fri, 17 May 2019 at 07:06, Zhiyi Zhang zzhang@codeweavers.com wrote:
On 5/17/19 4:28 AM, Henri Verbeet wrote:
On Wed, 15 May 2019 at 18:13, Zhiyi Zhang zzhang@codeweavers.com wrote:
- if (!pD3DKMTCheckVidPnExclusiveOwnership || pD3DKMTCheckVidPnExclusiveOwnership(NULL) == STATUS_PROCEDURE_NOT_FOUND
|| !pD3DKMTCloseAdapter || !pD3DKMTOpenAdapterFromGdiDisplayName)Formatting. (Double indent for line continuations.)
Eh, I keep forgetting this when writing d3d code. Are you using some kind of coding style format file? For example, clang-format? If it's, could you share it so I won't forget about it next time?
I'm not aware of anyone having something like that, sorry.
- lstrcpyW(open_adapter_gdi_desc.DeviceName, display1W);
- status = pD3DKMTOpenAdapterFromGdiDisplayName(&open_adapter_gdi_desc);
- ok(status == STATUS_SUCCESS, "Got unexpected status %#x.\n", status);
Wouldn't it make more sense to use D3DKMTOpenAdapterFromLuid(), so that we can use the adapter corresponding to the device?
The problem with D3DKMTOpenAdapterFromLuid is that currently there is no way to uniquely correlate dxgi adapter, gdi adapter, opengl adapter and vulkan adapter, as previously discussed when trying to support LUID. But I think it could be partially implemented for dxgi adapter <-> gdi adapter, seeing that dxgi currently use EnumDisplayDevice to enumerate adapters.
Sure, but that only starts being an issue once winex11/gdi32 actually supports multiple adapters. I.e., D3DKMTOpenAdapterFromLuid() always opening \.\DISPLAY1 would be no worse than the current behaviour.
- /* Swapchain in fullscreen mode */
- hr = IDXGISwapChain_SetFullscreenState(swapchain, TRUE, NULL);
- /* DXGI_ERROR_NOT_CURRENTLY_AVAILABLE on some machines. DXGI_ERROR_UNSUPPORTED on Win 7 testbot. */
- if (hr == DXGI_ERROR_NOT_CURRENTLY_AVAILABLE || broken(hr == DXGI_ERROR_UNSUPPORTED))
- {
skip("Failed to change fullscreen state.\n");goto done;- }
- todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
- Sleep(timeout);
Is that Sleep() really required?
Yes. I think I have it commented somewhere in the patch series. I reordered then so it must be in later patches.
The Sleep() is used so that the tests can get consistent results. Without the Sleep(), the tests suffers from timing issues, getting different results sometimes. For example, on Windows, it takes a few seconds for a fullscreen window to actually exited the fullscreen mode. For other places, I expect some IPC involved, so the state is not updated in real time. I tried to remove as many Sleep() calls as I can. So yes, any Sleep() calls you are seeing are necessary.
Would it be possible to poll for the change instead? E.g., by calling D3DKMTCheckVidPnExclusiveOwnership() in loop with a timeout until either the state changes, or the timeout expires.