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

Zhiyi Zhang zzhang at codeweavers.com
Fri May 17 10:12:45 CDT 2019



On 5/17/19 10:58 PM, Henri Verbeet wrote:
> 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.
Sure. That should save a few more seconds.



More information about the wine-devel mailing list