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

[Zhiyi Zhang]

On 6/11/19 9:08 PM, Józef Kucia wrote:
> On Mon, Jun 10, 2019 at 3:05 PM Zhiyi Zhang  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.

No. I will make it wait_vidpn_exclusive_ownership
> 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?

It seems to be the only way to get the corresponding adapter back from a d3d12 device.
> > -
> >    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.