[PATCH 05/12] d3drm: Implement IDirect3DRMDevice*::InitFromD3D.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Patches 1-4 look good to me, once the fixes you mentioned and we talked about on IRC are implemented.

Am 2016-07-07 um 14:52 schrieb Aaryaman Vasishta:
>  struct d3drm_frame
>  {
> +    struct d3drm_object obj;
This seems unrelated.

> +    if (device->ddraw)
> +        hr = D3DRMERR_BADOBJECT;
I guess you're not returning here right away because you want to addref d3drm later. Wouldn't it be better to move those AddRefs in front in this case?

The QueryInterface / GetRenderTarget code below is unlikely to fail, if it does something is severely wrong (A d3d device without a render target???) and getting the reference leak right is the last of your worries.

>      device->width = desc.dwWidth;
>      device->height = desc.dwHeight;
If I read the diff right this code will set width and height to 0 on an already initialized device.

>  static HRESULT WINAPI d3drm_device3_GetDirect3DDevice2(IDirect3DRMDevice3 *iface, IDirect3DDevice2 **d3d_device)
>  {
>      struct d3drm_device *device = impl_from_IDirect3DRMDevice3(iface);
> +    HRESULT hr;
>  
>      TRACE("iface %p, d3d_device %p.\n", iface, d3d_device);
>  
> -    IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device);
> +    if (FAILED(hr = IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device)))
> +        hr = D3DRMERR_BADOBJECT;
>  
> -    return D3DRM_OK;
> +    return hr;
>  }
I think this change (like the device2_GetDirect3DDevice2 one) can be moved to a separate patch.

> +    hr = IDirect3DRMDevice_InitFromD3D(device1, d3d1, d3ddevice1);
> +    ok(hr == D3DRMERR_BADOBJECT, "Expected hr == D3DRMERR_BADOBJECT, got %#x.\n", hr);
> +    ref3 = get_refcount((IUnknown *)d3ddevice1);
> +    ok(ref3 > device_ref2, "Expected ref3 > device_ref2, got ref3 = %u, device_ref2 = %u.\n", ref3, device_ref2);
> +    ref3 = get_refcount((IUnknown *)d3d1);
> +    ok(ref3 > d3d_ref2, "Expected ref3 > d3d_ref2, got ref3 = %u, d3d_ref2 = %u.\n", ref3, d3d_ref2);
You forgot to test the reference of d3drm1.

> +    hr = IDirect3DRMDevice_QueryInterface(device1, &IID_IDirect3DRMDevice2, (void **)&device2);
> +    ok(SUCCEEDED(hr), "Cannot get IDirect3DRMDevice2 Interface (hr = %x).\n", hr);
> +    hr = IDirect3DRMDevice2_GetDirect3DDevice2(device2, &d3ddevice2);
> +    ok(SUCCEEDED(hr), "Expected hr == D3DRM_OK, got %#x.\n", hr);
> +    ok(d3ddevice2 == NULL, "Expected d3ddevice2 == NULL, got %p.\n", d3ddevice2);
> +    IDirect3DRMDevice2_Release(device2);
You forgot d3ddevice2 = 0xdeadbeef here.

> +    hr = IDirect3DRM2_CreateObject(d3drm2, &CLSID_CDirect3DRMDevice, NULL, &IID_IDirect3DRMDevice2, (void **)&device2);
> +    ok(SUCCEEDED(hr), "Cannot get IDirect3DRMDevice2 interface (hr = %#x).\n", hr);
> +
> +    hr = IDirectDraw_QueryInterface(ddraw1, &IID_IDirect3D, (void **)&d3d1);
> +    ok(SUCCEEDED(hr), "Cannot get IDirect3D interface (hr = %x).\n", hr);
> +    if (SUCCEEDED(hr = IDirect3DDevice2_QueryInterface(d3ddevice2, &IID_IDirect3DDevice, (void **)&d3ddevice1)))
> +    {
> +        hr = IDirect3DRMDevice2_InitFromD3D(device2, d3d1, d3ddevice1);
> +        ok(hr == E_NOINTERFACE, "Expected hr == E_NOINTERFACE, got %#x.\n", hr);
What happens if you QI IDirect3DRMDevice1 from IDirect3DRMDevice2 and call InitFromD3D on it? I would expect it to succeed because common decency suggests that CreateObject doesn't leave a history of which interface was requested and there's no CLSID_CDirect3DRMDevice2 or CLSID_CDirect3DRMDevice3. Still worth a check though I think.

Cheers,
Stefan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXgmxrAAoJEN0/YqbEcdMw9dQP/RbovFFjwIdo5ho2BcxWi1Sm
68U7n+sX+HAPmHPc2W/fOK1scbPaQz84e3cHj+dsFClCUKNLUpZ1eGdJDu1u9Fgh
ZJVC9hgw8rzC+IDGr5vdmuzzOj65mI7KslJX9CfD9tnXigLtlvPf3me8jnhjL51b
Eq8N1rStZTh805gDBEnmr/oqgQFkRoHKgs5lLIWq2inAfF3dq0BiThBGpC+eq19H
YcihqOJWgSXrbXQYtTPjFtjp3jqqFa32+/ZQufCEAELcYUjM2gG23+O/rT5sa4d2
QuQOIhdUnTR9G6WnoZXgxKwxtJTvTu02GKlk5gaIkpWFJ2O+mPJuiBmylLoucQ6V
BkYkhORuT2WYKainbTx3qNJaxBPSa5/y4HWTpQhzYH0LryFBLkL8MiVjO+pGk7QM
4wHTeL4ZXmjtyYOs6qfuxNIEJyfDs0hU5850lWdVD6MLaj3JwZYRLedCJ9TwV7Jb
Qz3JQIK6YPOgTSqehIGTwzxgcbvjjiLP2zalEV4HOIqKeEkodcBxVS0QiXSDbuAj
gLhbqwlRMshT5Dtt1Umi2KUoGljFjh1rbe42cyVxXcoa72D+g9LQUb4d0hUP/Ppt
6lNhfq+Vf2W+FmdB2Hv/WWof7vBV59r/TwjjExrN00yRGCLNLF3BaK8lzjeJprGO
H6J9OAsscEyzMNpB+l0W
=exYP
-----END PGP SIGNATURE-----