Mostly to show that TakeOwnership and ReleaseOwnership are based on VidPN ownership.
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/dxgi/tests/dxgi.c | 178 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 177 insertions(+), 1 deletion(-)
diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 79e4bc0c15..5e1fb43b7e 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -17,11 +17,15 @@ */
#include <assert.h> +#include "ntstatus.h" +#define WIN32_NO_STATUS #define COBJMACROS #include "initguid.h" #include "dxgi1_6.h" #include "d3d11.h" #include "d3d12.h" +#include "winternl.h" +#include "ddk/d3dkmthk.h" #include "wine/heap.h" #include "wine/test.h"
@@ -36,6 +40,10 @@ static DEVMODEW registry_mode; static HRESULT (WINAPI *pCreateDXGIFactory1)(REFIID iid, void **factory); static HRESULT (WINAPI *pCreateDXGIFactory2)(UINT flags, REFIID iid, void **factory);
+static NTSTATUS (WINAPI *pD3DKMTCheckVidPnExclusiveOwnership)(const D3DKMT_CHECKVIDPNEXCLUSIVEOWNERSHIP *); +static NTSTATUS (WINAPI *pD3DKMTCloseAdapter)(const D3DKMT_CLOSEADAPTER *); +static NTSTATUS (WINAPI *pD3DKMTOpenAdapterFromGdiDisplayName)(D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME *); + static PFN_D3D12_CREATE_DEVICE pD3D12CreateDevice; static PFN_D3D12_GET_DEBUG_INTERFACE pD3D12GetDebugInterface;
@@ -4985,6 +4993,167 @@ done: ok(!refcount, "Factory has %u references left.\n", refcount); }
+static void test_output_ownership(IUnknown *device, BOOL is_d3d12) +{ + static const DWORD timeout = 1000; + static const WCHAR display1W[] = {'\','\','.','\','D','I','S','P','L','A','Y','1',0}; + D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME open_adapter_gdi_desc; + D3DKMT_CHECKVIDPNEXCLUSIVEOWNERSHIP check_ownership_desc; + D3DKMT_CLOSEADAPTER close_adapter_desc; + DXGI_SWAP_CHAIN_DESC swapchain_desc; + IDXGISwapChain *swapchain; + IDXGIFactory *factory; + IDXGIOutput *output; + BOOL fullscreen; + ULONG refcount; + NTSTATUS status; + HRESULT hr; + + if (!pD3DKMTCheckVidPnExclusiveOwnership || pD3DKMTCheckVidPnExclusiveOwnership(NULL) == STATUS_PROCEDURE_NOT_FOUND + || !pD3DKMTCloseAdapter || !pD3DKMTOpenAdapterFromGdiDisplayName) + { + skip("Required functions are unavailable.\n"); + return; + } + + lstrcpyW(open_adapter_gdi_desc.DeviceName, display1W); + status = pD3DKMTOpenAdapterFromGdiDisplayName(&open_adapter_gdi_desc); + ok(status == STATUS_SUCCESS, "Got unexpected status %#x.\n", status); + + check_ownership_desc.hAdapter = open_adapter_gdi_desc.hAdapter; + check_ownership_desc.VidPnSourceId = open_adapter_gdi_desc.VidPnSourceId; + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", status); + + get_factory(device, is_d3d12, &factory); + + swapchain_desc.BufferDesc.Width = 800; + swapchain_desc.BufferDesc.Height = 600; + swapchain_desc.BufferDesc.RefreshRate.Numerator = 60; + swapchain_desc.BufferDesc.RefreshRate.Denominator = 60; + swapchain_desc.BufferDesc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; + swapchain_desc.BufferDesc.ScanlineOrdering = DXGI_MODE_SCANLINE_ORDER_UNSPECIFIED; + swapchain_desc.BufferDesc.Scaling = DXGI_MODE_SCALING_UNSPECIFIED; + swapchain_desc.SampleDesc.Count = 1; + swapchain_desc.SampleDesc.Quality = 0; + swapchain_desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT; + swapchain_desc.BufferCount = is_d3d12 ? 2 : 1; + swapchain_desc.OutputWindow = CreateWindowA("static", "dxgi_test", 0, 0, 0, 400, 200, 0, 0, 0, 0); + swapchain_desc.Windowed = TRUE; + swapchain_desc.SwapEffect = is_d3d12 ? DXGI_SWAP_EFFECT_FLIP_DISCARD : DXGI_SWAP_EFFECT_DISCARD; + swapchain_desc.Flags = 0; + + hr = IDXGIFactory_CreateSwapChain(factory, device, &swapchain_desc, &swapchain); + ok(hr == S_OK, "Failed to create swapchain, hr %#x.\n", hr); + + /* 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); + output = NULL; + fullscreen = FALSE; + hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, &output); + todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + todo_wine_if(is_d3d12) ok(fullscreen, "Unexpected fullscreen state.\n"); + todo_wine_if(is_d3d12) ok(output != NULL, "Expect output not null.\n"); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + if (is_d3d12) + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", + status); + else + todo_wine ok(status == STATUS_GRAPHICS_PRESENT_OCCLUDED, "Got unexpected status %#x.\n", status); + if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : E_INVALIDARG), "Got unexpected hr %#x.\n", hr); + if (output) hr = IDXGIOutput_TakeOwnership(output, device, TRUE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : S_OK), "Got unexpected hr %#x.\n", hr); + /* Calling IDXGIOutput_ReleaseOwnership makes it unoccluded. So we can be confident about IDXGIOutput_TakeOwnership + * was called in IDXGISwapChain_SetFullscreenState */ + if (output) IDXGIOutput_ReleaseOwnership(output); + Sleep(timeout); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", + status); + + if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : DXGI_ERROR_NOT_CURRENTLY_AVAILABLE), "Got unexpected hr %#x.\n", hr); + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", + status); + if (output) IDXGIOutput_ReleaseOwnership(output); + + if (output) hr = IDXGIOutput_TakeOwnership(output, device, TRUE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : S_OK), "Got unexpected hr %#x.\n", hr); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + /* So the following results shows that IDXGIOutput_TakeOwnership(output, device, TRUE) is used in + * SetFullscreenState(swapchain, TRUE, NULL) + * + * This make me believe the MSDN documentation is saying the opposite about the last parameter, + * "HRESULT TakeOwnership(IUnknown *pDevice, BOOL Exclusive); + * Name: Exclusive Type: BOOL + * Set to TRUE to enable other threads or applications to take ownership of the device; otherwise, set to FALSE." + * + * Reasons: + * 1. The parameter name is called 'Exclusive' + * 2. D3DKMTSetVidPnSourceOwner(D3DKMT_VIDPNSOURCEOWNER_EXCLUSIVE) makes D3DKMTCheckVidPnExclusiveOwnership return + * STATUS_GRAPHICS_PRESENT_OCCLUDED. And D3DKMTSetVidPnSourceOwner(D3DKMT_VIDPNSOURCEOWNER_SHARED) makes + * D3DKMTCheckVidPnExclusiveOwnership return STATUS_GRAPHICS_VIDPN_SOURCE_IN_USE. So the opposite mapping of + * what MSDN is saying is consistent with the tests. + */ + if (is_d3d12) + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", + status); + else + todo_wine ok(status == STATUS_GRAPHICS_PRESENT_OCCLUDED, "Got unexpected status %#x.\n", status); + if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : E_INVALIDARG), "Got unexpected hr %#x.\n", hr); + if (output) IDXGIOutput_ReleaseOwnership(output); + + /* Swapchain in windowed mode */ + hr = IDXGISwapChain_SetFullscreenState(swapchain, FALSE, NULL); + todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + Sleep(timeout); + fullscreen = TRUE; + hr = IDXGISwapChain_GetFullscreenState(swapchain, &fullscreen, NULL); + todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + todo_wine_if(is_d3d12) ok(!fullscreen, "Unexpected fullscreen state.\n"); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + ok(status == STATUS_SUCCESS, "Got unexpected status %#x.\n", status); + + if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : DXGI_ERROR_NOT_CURRENTLY_AVAILABLE), "Got unexpected hr %#x.\n", hr); + + if (output) hr = IDXGIOutput_TakeOwnership(output, device, TRUE); + todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : S_OK), "Got unexpected hr %#x.\n", hr); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + if (is_d3d12) + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", + status); + else + todo_wine ok(status == STATUS_GRAPHICS_PRESENT_OCCLUDED, "Got unexpected status %#x.\n", status); + if (output) IDXGIOutput_ReleaseOwnership(output); + status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc); + ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n", status); + + if (output) IDXGIOutput_Release(output); + +done: + wait_device_idle(device); + + IDXGISwapChain_Release(swapchain); + DestroyWindow(swapchain_desc.OutputWindow); + refcount = IDXGIFactory_Release(factory); + ok(refcount == !is_d3d12, "Got unexpected refcount %u.\n", refcount); + + close_adapter_desc.hAdapter = open_adapter_gdi_desc.hAdapter; + status = pD3DKMTCloseAdapter(&close_adapter_desc); + ok(status == STATUS_SUCCESS, "Got unexpected status %#x.\n", status); +} + static void run_on_d3d10(void (*test_func)(IUnknown *device, BOOL is_d3d12)) { IDXGIDevice *device; @@ -5028,7 +5197,7 @@ static void run_on_d3d12(void (*test_func)(IUnknown *device, BOOL is_d3d12))
START_TEST(dxgi) { - HMODULE dxgi_module, d3d12_module; + HMODULE dxgi_module, d3d12_module, gdi32_module; BOOL enable_debug_layer = FALSE; unsigned int argc, i; ID3D12Debug *debug; @@ -5038,6 +5207,11 @@ START_TEST(dxgi) pCreateDXGIFactory1 = (void *)GetProcAddress(dxgi_module, "CreateDXGIFactory1"); pCreateDXGIFactory2 = (void *)GetProcAddress(dxgi_module, "CreateDXGIFactory2");
+ gdi32_module = GetModuleHandleA("gdi32.dll"); + pD3DKMTCheckVidPnExclusiveOwnership = (void *)GetProcAddress(gdi32_module, "D3DKMTCheckVidPnExclusiveOwnership"); + pD3DKMTCloseAdapter = (void *)GetProcAddress(gdi32_module, "D3DKMTCloseAdapter"); + pD3DKMTOpenAdapterFromGdiDisplayName = (void *)GetProcAddress(gdi32_module, "D3DKMTOpenAdapterFromGdiDisplayName"); + registry_mode.dmSize = sizeof(registry_mode); ok(EnumDisplaySettingsW(NULL, ENUM_REGISTRY_SETTINGS, ®istry_mode), "Failed to get display mode.\n");
@@ -5088,6 +5262,7 @@ START_TEST(dxgi) run_on_d3d10(test_swapchain_present); run_on_d3d10(test_swapchain_backbuffer_index); run_on_d3d10(test_swapchain_formats); + run_on_d3d10(test_output_ownership);
if (!(d3d12_module = LoadLibraryA("d3d12.dll"))) { @@ -5108,6 +5283,7 @@ START_TEST(dxgi) run_on_d3d12(test_swapchain_present); run_on_d3d12(test_swapchain_backbuffer_index); run_on_d3d12(test_swapchain_formats); + run_on_d3d12(test_output_ownership);
FreeLibrary(d3d12_module); }
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=52271
Your paranoid android.
=== w2008s64 (64 bit report) ===
dxgi: dxgi.c:4523: Test failed: Got unexpected message 0x46, hwnd 00000000000E0104, wparam 0, lparam 0x22f960. dxgi.c:4523: Test failed: Got unexpected message 0x24, hwnd 00000000000E0104, wparam 0, lparam 0x22f5d0. dxgi.c:4523: Test failed: Got unexpected message 0x1a, hwnd 00000000000E0104, wparam 0x18, lparam 0x22f988. dxgi.c:4523: Test failed: Got unexpected message 0x1a, hwnd 00000000000E0104, wparam 0x2f, lparam 0x22f978.
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.)
- 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?
- /* 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?
- /* Calling IDXGIOutput_ReleaseOwnership makes it unoccluded. So we can be confident about IDXGIOutput_TakeOwnership
* was called in IDXGISwapChain_SetFullscreenState */- if (output) IDXGIOutput_ReleaseOwnership(output);
- Sleep(timeout);
Likewise.
- status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc);
- ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n",
status);- if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE);
- todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : DXGI_ERROR_NOT_CURRENTLY_AVAILABLE), "Got unexpected hr %#x.\n", hr);
- ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n",
status);
This ok() seems redundant.
- /* Swapchain in windowed mode */
- hr = IDXGISwapChain_SetFullscreenState(swapchain, FALSE, NULL);
- todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
- Sleep(timeout);
And another.
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?
- 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.
Or may be I could use D3DKMTOpenAdapterFromGdiDisplayName only when testing on wine?
- /* 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.
- /* Calling IDXGIOutput_ReleaseOwnership makes it unoccluded. So we can be confident about IDXGIOutput_TakeOwnership
* was called in IDXGISwapChain_SetFullscreenState */- if (output) IDXGIOutput_ReleaseOwnership(output);
- Sleep(timeout);
Likewise.
- status = pD3DKMTCheckVidPnExclusiveOwnership(&check_ownership_desc);
- ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n",
status);- if (output) hr = IDXGIOutput_TakeOwnership(output, device, FALSE);
- todo_wine ok(hr == (is_d3d12 ? E_NOINTERFACE : DXGI_ERROR_NOT_CURRENTLY_AVAILABLE), "Got unexpected hr %#x.\n", hr);
- ok(status == STATUS_SUCCESS || status == STATUS_GRAPHICS_PRESENT_UNOCCLUDED, "Got unexpected status %#x.\n",
status);This ok() seems redundant.
- /* Swapchain in windowed mode */
- hr = IDXGISwapChain_SetFullscreenState(swapchain, FALSE, NULL);
- todo_wine_if(is_d3d12) ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
- Sleep(timeout);
And another.
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.
On 5/17/19 10:58 PM, Henri Verbeet wrote:
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.
Sure. That should save a few more seconds.
-
Henri Verbeet -
Marvin -
Zhiyi Zhang