[PATCH 01/10] dxgi/tests: Add IDXGIOutput ownership tests.

[Henri Verbeet]

On Fri, 17 May 2019 at 07:06, Zhiyi Zhang <zzhang at codeweavers.com> wrote:
> On 5/17/19 4:28 AM, Henri Verbeet wrote:
> > On Wed, 15 May 2019 at 18:13, Zhiyi Zhang <zzhang at 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.