[PATCH v2 resend 1/3] dxgi/tests: Add IDXGIOutput ownership tests.

[Józef Kucia]

On Mon, Jun 10, 2019 at 3:05 PM Zhiyi Zhang <zzhang at codeweavers.com> wrote:
> +/* This is to get a result from a function that returns different result if called too early */
> +#define wait_result(a, b, c, d) wait_result_(__LINE__, a, b, c, d)
> +#define wait_result_(line, func, arg, expected, todo)                                                      \
> +    do                                                                                                     \
> +    {                                                                                                      \
> +        DWORD total_time = 0;                                                                              \
> +        typeof(expected) got;                                                                              \
> +        do                                                                                                 \
> +        {                                                                                                  \
> +            got = func(arg);                                                                               \
> +            if (got == expected)                                                                           \
> +                break;                                                                                     \
> +            Sleep(wait_step);                                                                              \
> +            total_time += wait_step;                                                                       \
> +        } while (total_time < wait_timeout);                                                               \
> +        todo_wine_if(todo) ok_(__FILE__, line)(got == expected, "Expect %#x, got %#x.\n", expected, got);  \
> +    } while (0)

Do you expect that wait_result() is needed for other functions than
D3DKMTCheckVidPnExclusiveOwnership? I think we could avoid this macro
and introduce wait_vidpn_exclusive_ownership() instead.

Also if you make wait_step and wait_timeout global variables, please
try to use the same timeout constant in patch 2/3. Otherwise, it might
make sense to not make them global.

> +    if (is_d3d12)
> +    {
> +        hr = ID3D12CommandQueue_GetDevice((ID3D12CommandQueue *)device, &IID_ID3D12Device, (void **)&d3d12_device);
> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> +        luid = ID3D12Device_GetAdapterLuid(d3d12_device);
> +        ID3D12Device_Release(d3d12_device);
> +        hr = IDXGIFactory_QueryInterface(factory, &IID_IDXGIFactory4, (void **)&factory4);
> +        if (hr == E_NOINTERFACE)
> +        {
> +            skip("DXGI 1.4 unsupported.\n");
> +            IDXGIFactory_Release(factory);
> +            return;
> +        }

Is it possible to have a D3D12 device without IID_IDXGIFactory4?

> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> +        hr = IDXGIFactory4_EnumAdapterByLuid(factory4, luid, &IID_IDXGIAdapter, (void **)&adapter);
> +        IDXGIFactory4_Release(factory4);
> +        if (hr == DXGI_ERROR_NOT_FOUND)
> +        {
> +            skip("Wine doesn't support IDXGIFactory4_EnumAdapterByLuid.\n");
> +            IDXGIFactory_Release(factory);
> +            return;
> +        }
> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> +    }
> +    else
> +    {
> +        hr = IDXGIDevice_GetAdapter((IDXGIDevice *)device, &adapter);
> +        ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
> +    }

Perhaps we should introduce a helper function, e.g. IDXGIAdapter
*get_adapter(IUnknown *device);

> +    /* 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.
> +     */

I feel that the full MSDN quote is not necessary. A simple explanation
what the "exclusive" parameter means according to tests should be
enough.