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
-----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.
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@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-----
-----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 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...
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@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-----
-
Aaryaman Vasishta -
Stefan Dösinger