CreateDeviceFromD3D implementations, device QI tests and some other patches.

[Aaryaman Vasishta]

Hello!

Attached are patches related to implementations of CreateDeviceFromD3D and
device QI tests along with fixes to return values for GetDirect3DDevice2
and device QueryInterface.


Cheers,
Jam

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

In patch 1 you have an if condition you can remove now:
> -    todo_wine ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice interface (hr = %x).\n", hr);
> +    ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice interface (hr = %x).\n", hr);
>      if (FAILED(hr))
>          goto cleanup;
> The same applies to the equivalent places in patch 2.

After patch 2 I think the if (device->device) check in d3drm_device3_GetDirect3DDevice is no longer needed, and similarly in GetDirect3DDevice2.

In patch 2 you're still more or less thunking back to device 1 in d3drm3_CreateDeviceFromD3D because d3drm_device_set_ddraw_device_d3d. This is OK if you prefer it that way, but the code might look nicer if you make d3drm2_CreateDeviceFromD3D and d3drm3_CreateDeviceFromD3D QI the older interfaces and then call IDirect3DRM_CreateDeviceFromD3D.

Patch 3:
> +    hr = IDirect3DRMDevice_QueryInterface(device1, &IID_IDirect3DRMDevice2, (void **)&device2);
> +    ok(hr == DD_OK, "Cannot get IDirect3DRMDevice2 Interface (hr = %x).\n", hr);
> Minor matter of taste thing: S_OK or D3DRM_OK
> +    hr = IDirect3DRMDevice2_GetDirect3DDevice2(device2, &d3ddevice2);
> +    ok(hr == DD_OK, "Expected hr == DD_OK, got %x).\n", hr);
> D3DRM_OK. You're calling a d3drm object here. I won't block these patches based on that, S_OK, D3DRM_OK, D3D_OK and DD_OK are all the same value...

Patches 4, 5 and 6 look OK.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJV1D8jAAoJEN0/YqbEcdMwkpAP/288xtKa+gj88tVt/rzxgFjg
vRwDuusTrNjhRGE7mVuGKkRXmyBwSnM4QC4giJRDoLxYjNkzOFCFJK8lgExrU3G3
1ODD1CnayL2XUcxpNPwUu4IrLYR0oUkxnC9iDEteR9mWLmdXzeHNM9J+XRzkmLT2
maJrwRVYnk/funOqscGwkVbt3Y62qyRI1/G8+4NFuPPfpcW2L14PI91mAkiCrS80
afLyDhT9PzX6I4Mb686iia+aP5ktxm3pBtnZmTfra1PAPl9bXjVPEb24Iyj3DgkE
fZfnUYtPj3GEgWlCWko3RwNkjMUBolSGsc9nEgzCt0i0hr8NGFzvGppdv0umINWP
Mx6aghmsixBT+YXkbrKRxwwZZXyMvW/HSvW8w0QO/ZHUuNurwymkTBQlyrUrgKkD
5615g1u8zA9yUe41yJ+SKu34uugF0mvaeDOVWaHH8IgFRYSU8aJ/JGOFFMSyjCgq
1R/gHoyHexAFB+e9nwoYRtYFOdpg8hhD91EVsm6LYnqWZqj3uBsGabWqoYlKBAHk
xarpI3cYHpwMnAmjfW1gCighM7EWBvBPvlgw955NDwzOJT93GbJ4OPhSV6WBKQzv
yjJ+uNyBhkTk2drkXcM++2j2c/kG3hClDtVHNVzmDHVq3ApkEun+l648fqLmmC8a
aLr+xidkCe11hqlZ9veJ
=59AI
-----END PGP SIGNATURE-----

[Aaryaman Vasishta]

Made all the changes pointed in your mail. Attached are the modified
patches.
I've used SUCCEEDED(hr) instead of D3DRM_OK as it's independent of the
current API being used, since all their values are the same.


Cheers,
Jam

On Wed, Aug 19, 2015 at 2:02 PM, Stefan Dösinger <stefandoesinger(a)gmail.com>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> In patch 1 you have an if condition you can remove now:
> > -    todo_wine ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice interface
> > (hr = %x).\n", hr);
> > +    ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice interface (hr =
> > %x).\n", hr);
> >      if (FAILED(hr))
> >          goto cleanup;
> > The same applies to the equivalent places in patch 2.
>
> After patch 2 I think the if (device->device) check in
> d3drm_device3_GetDirect3DDevice is no longer needed, and similarly in
> GetDirect3DDevice2.
>
> In patch 2 you're still more or less thunking back to device 1 in
> d3drm3_CreateDeviceFromD3D because d3drm_device_set_ddraw_device_d3d. This
> is OK if you prefer it that way, but the code might look nicer if you make
> d3drm2_CreateDeviceFromD3D and d3drm3_CreateDeviceFromD3D QI the older
> interfaces and then call IDirect3DRM_CreateDeviceFromD3D.
>
> Patch 3:
> > +    hr = IDirect3DRMDevice_QueryInterface(device1,
> > &IID_IDirect3DRMDevice2, (void **)&device2);
> > +    ok(hr == DD_OK, "Cannot get IDirect3DRMDevice2 Interface (hr =
> > %x).\n", hr);
> > Minor matter of taste thing: S_OK or D3DRM_OK
> > +    hr = IDirect3DRMDevice2_GetDirect3DDevice2(device2, &d3ddevice2);
> > +    ok(hr == DD_OK, "Expected hr == DD_OK, got %x).\n", hr);
> > D3DRM_OK. You're calling a d3drm object here. I won't block these patches
> > based on that, S_OK, D3DRM_OK, D3D_OK and DD_OK are all the same value...
>
> Patches 4, 5 and 6 look OK.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJV1D8jAAoJEN0/YqbEcdMwkpAP/288xtKa+gj88tVt/rzxgFjg
> vRwDuusTrNjhRGE7mVuGKkRXmyBwSnM4QC4giJRDoLxYjNkzOFCFJK8lgExrU3G3
> 1ODD1CnayL2XUcxpNPwUu4IrLYR0oUkxnC9iDEteR9mWLmdXzeHNM9J+XRzkmLT2
> maJrwRVYnk/funOqscGwkVbt3Y62qyRI1/G8+4NFuPPfpcW2L14PI91mAkiCrS80
> afLyDhT9PzX6I4Mb686iia+aP5ktxm3pBtnZmTfra1PAPl9bXjVPEb24Iyj3DgkE
> fZfnUYtPj3GEgWlCWko3RwNkjMUBolSGsc9nEgzCt0i0hr8NGFzvGppdv0umINWP
> Mx6aghmsixBT+YXkbrKRxwwZZXyMvW/HSvW8w0QO/ZHUuNurwymkTBQlyrUrgKkD
> 5615g1u8zA9yUe41yJ+SKu34uugF0mvaeDOVWaHH8IgFRYSU8aJ/JGOFFMSyjCgq
> 1R/gHoyHexAFB+e9nwoYRtYFOdpg8hhD91EVsm6LYnqWZqj3uBsGabWqoYlKBAHk
> xarpI3cYHpwMnAmjfW1gCighM7EWBvBPvlgw955NDwzOJT93GbJ4OPhSV6WBKQzv
> yjJ+uNyBhkTk2drkXcM++2j2c/kG3hClDtVHNVzmDHVq3ApkEun+l648fqLmmC8a
> aLr+xidkCe11hqlZ9veJ
> =59AI
> -----END PGP SIGNATURE-----

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

- From patch 3:
> -    return IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device);
> +    *d3d_device = NULL;
> +    IDirect3DDevice_QueryInterface(device->device, &IID_IDirect3DDevice2, (void**)d3d_device);
> I don't think you need the NULL assignment here,
> d3ddevice::queryinterface does it for you in all error paths.

Other than that this looks good, feel free to send to wine-patches
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJV1ZLcAAoJEN0/YqbEcdMwfFEP/Rm4DafDGxrAzQtrVMmwx3rg
E8P7pQ4A+tpmYN6fChw1GJAAyQsWMRJaYT4+Ke/6jM2kwfJJ2AUW46Tjmyvbx/Ic
cMT6PCTTOxQsNwYbjqMM5i4u6wzbw1ONYtvXpXezDA9RslyL3rw3XT/swYz/0+xT
vJdTudtNl5njpO2AVesS/8MYv+v1nPXpRYdtGqvB5kKHs5yyRpj+zfe3j1JucplT
xEzKBMclhv6fPSGBcvjfsW57p2LqoJ6RuubYzr325Pkz60V+dmgi6N1gGlfVmKWM
y+la/7DrLigY3Ww2IJpyVDZnjDqYTzSIjzUkUlfqru9kpG6Jc1C2gJ7R9TwQSiqb
MrYxCNHCetAQ20UzXXOAgDwUic8Y3fRzBUqz+SbuoDgU7ka+TxKZd09G0YC7/mFz
ZJXH308ugDybTnCl4I155eJYhCYFXsRRFOQsaF1ZNkOHzFCZHk8uszODQa1MvztA
gfQdml8WNKCytQEl53NAcYymsNCTjsU9DBEjrklpvZh8QJ/zxkuIPQEOJV65FlJu
+QxG6rRL60g3tOGXr42WlLhihoVsOXhp9GtbHG4U7mBWYlI4Sv6XtzQ/rFoZ/Zq3
kkkRtj0b7v7P6Mormqpriy+0KCZvTZ1++smo/Ck4rrCJ+aYtYoKtRSWB8iTe5rBf
XsxiSnPfe2X4PG+oiNUC
=K4OT
-----END PGP SIGNATURE-----

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2015-08-20 um 10:42 schrieb Stefan Dösinger:
> I don't think you need the NULL assignment here, 
> d3ddevice::queryinterface does it for you in all error paths.
> Just to clarify: Setting *obj = NULL if the requested interface isn't
> supported is documented QueryInterface behavior[1]. Some interfaces
> beg to differ, but IDirect3DDevice* works according to the spec.

1:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682521(v=vs.85).a...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJV1ZTFAAoJEN0/YqbEcdMwmLUQAIj8w0Kw0pkReFSlXbPmZ9th
Cn/dOFRehwYtwCS7ZCC5L83KowOrGiTKp/CxxhX0eF+UoQo71PIzOqPe3Bxbbz0B
yDfHTvDSXkk4zBrfISL5cVioQwcmYD1pfjaQP8BBMZvBd83Ve4pYSLPMbubgBe+p
6rRShZZMQBcE2z1jxED3jISMBk6pkAeTVdNVs6ukA/Is0nCOOLNFJZ4ygXBv+FQR
OObw2k33OjWBCkjKP3ux6bdvPpsCLcrwLVbZxqamUge1JwTlkP6umzZK2GamaQJY
AF7RBIV6L2qeA4dCe30OFFThqWj/nRo1yVnKe7TJSs/USl+m2AiukTUE8D7WMQWs
Pdk0mT8/decvB8ESWgShlfdELSqqICB4OTqev8bL3ruW+s8W4PeyA3K4JN7bhYqA
T16b+meEIdY3Q7nO0EHeU99st260mdjqHzNO3toQbZZtHTR2ESLUmCEBePIqG+wG
+/9XHvScxLRoCxPbYOgMw/ujrDZUebOTwXG845O5LUFPhvAy7nTzSAKqODNyCO2n
tmHNYFYlAL6ZEm+L5RWjy/Yj120kJ2h4VtxfaTNZu/33gd+0H9kIMTqAlPW7NnZE
bg9+zos94n5ZTcMqQrie8HdF1HLYjA1hFPGZotKdg2qplThlpQ2XiH/mwehR7/Zf
JtzjAXDG/8TRd8jYeuaN
=zCME
-----END PGP SIGNATURE-----

[Aaryaman Vasishta]

Got it, made that change and sent it to wine-patches.

Cheers,
Jam

On Thu, Aug 20, 2015 at 2:20 PM, Stefan Dösinger <stefandoesinger(a)gmail.com>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Am 2015-08-20 um 10:42 schrieb Stefan Dösinger:
> > I don't think you need the NULL assignment here,
> > d3ddevice::queryinterface does it for you in all error paths.
> > Just to clarify: Setting *obj = NULL if the requested interface isn't
> > supported is documented QueryInterface behavior[1]. Some interfaces
> > beg to differ, but IDirect3DDevice* works according to the spec.
>
> 1:
>
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms682521(v=vs.85).a...
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJV1ZTFAAoJEN0/YqbEcdMwmLUQAIj8w0Kw0pkReFSlXbPmZ9th
> Cn/dOFRehwYtwCS7ZCC5L83KowOrGiTKp/CxxhX0eF+UoQo71PIzOqPe3Bxbbz0B
> yDfHTvDSXkk4zBrfISL5cVioQwcmYD1pfjaQP8BBMZvBd83Ve4pYSLPMbubgBe+p
> 6rRShZZMQBcE2z1jxED3jISMBk6pkAeTVdNVs6ukA/Is0nCOOLNFJZ4ygXBv+FQR
> OObw2k33OjWBCkjKP3ux6bdvPpsCLcrwLVbZxqamUge1JwTlkP6umzZK2GamaQJY
> AF7RBIV6L2qeA4dCe30OFFThqWj/nRo1yVnKe7TJSs/USl+m2AiukTUE8D7WMQWs
> Pdk0mT8/decvB8ESWgShlfdELSqqICB4OTqev8bL3ruW+s8W4PeyA3K4JN7bhYqA
> T16b+meEIdY3Q7nO0EHeU99st260mdjqHzNO3toQbZZtHTR2ESLUmCEBePIqG+wG
> +/9XHvScxLRoCxPbYOgMw/ujrDZUebOTwXG845O5LUFPhvAy7nTzSAKqODNyCO2n
> tmHNYFYlAL6ZEm+L5RWjy/Yj120kJ2h4VtxfaTNZu/33gd+0H9kIMTqAlPW7NnZE
> bg9+zos94n5ZTcMqQrie8HdF1HLYjA1hFPGZotKdg2qplThlpQ2XiH/mwehR7/Zf
> JtzjAXDG/8TRd8jYeuaN
> =zCME
> -----END PGP SIGNATURE-----